Territory transfers (#878)

* Allow founders to transfer territories

* Log territory transfers in new AuditLog table

* Add territory transfer notifications

* Use polymorphic AuditEvent table

* Add setting for territory transfer notifications

* Add push notification

* Rename label from user to stacker

* More space between cancel and confirm button

* Remove AuditEvent table

The audit table is not necessary for territory transfers and only adds complexity and unrelated discussion to this PR.

Thinking about a future-proof schema for territory transfers and how/what to audit at the same time made my head spin.

Some thoughts I had:

1. Maybe using polymorphism for an audit log / audit events is not a good idea

Using polymorphism as is currently used in the code base (user wallets) means that every generic event must map to exactly one specialized event.

Is this a good requirement/assumption? It already didn't work well for naive auditing of territory transfers since we want events to be indexable by user (no array column) so every event needs to point to a single user but a territory transfer involves multiple users.

This made me wonder: Do we even need a table? Maybe the audit log for a user can be implemented using a view? This would also mean no data denormalization.

2. What to audit and how and why?

Most actions are already tracked in some way by necessity: zaps, items, mutes, payments, ...

In that case: what is the benefit of tracking these things individually in a separate table?

Denormalize simply for convenience or performance? Why no view (see previous point)? Use case needs to be more clearly defined before speccing out a schema.

* Fix territory transfer notification id conflict

* Use include instead of two separate queries

* Drop territory transfer setting

* Remove trigger usage

* Prevent transfers to yourself
This commit is contained in:
ekzyis 2024-03-05 20:56:02 +01:00 committed by GitHub
parent 6573ce666b
commit b379e7467f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 266 additions and 4 deletions

View File

@ -168,6 +168,17 @@ export default {
LIMIT ${LIMIT}+$3)`
)
// territory transfers
queries.push(
`(SELECT "TerritoryTransfer".id::text, "TerritoryTransfer"."created_at" AS "sortTime", NULL as "earnedSats",
'TerritoryTransfer' AS type
FROM "TerritoryTransfer"
WHERE "TerritoryTransfer"."newUserId" = $1
AND "TerritoryTransfer"."created_at" <= $2
ORDER BY "sortTime" DESC
LIMIT ${LIMIT}+$3)`
)
if (meFull.noteItemSats) {
queries.push(
`(SELECT "Item".id::TEXT, MAX("ItemAct".created_at) AS "sortTime",
@ -367,6 +378,12 @@ export default {
TerritoryPost: {
item: async (n, args, { models, me }) => getItem(n, { id: n.id }, { models, me })
},
TerritoryTransfer: {
sub: async (n, args, { models, me }) => {
const transfer = await models.territoryTransfer.findUnique({ where: { id: Number(n.id) }, include: { sub: true } })
return transfer.sub
}
},
JobChanged: {
item: async (n, args, { models, me }) => getItem(n, { id: n.id }, { models, me })
},

View File

@ -6,6 +6,7 @@ import { ssValidate, territorySchema } from '../../lib/validate'
import { nextBilling, proratedBillingCost } from '../../lib/territory'
import { decodeCursor, LIMIT, nextCursorEncoded } from '../../lib/cursor'
import { subViewGroup } from './growth'
import { notifyTerritoryTransfer } from '../../lib/push-notifications'
export function paySubQueries (sub, models) {
if (sub.billingType === 'ONCE') {
@ -280,6 +281,40 @@ export default {
await models.subSubscription.create({ data: lookupData })
return true
}
},
transferTerritory: async (parent, { subName, userName }, { me, models }) => {
if (!me) {
throw new GraphQLError('you must be logged in', { extensions: { code: 'UNAUTHENTICATED' } })
}
const sub = await models.sub.findUnique({
where: {
name: subName
}
})
if (!sub) {
throw new GraphQLError('sub not found', { extensions: { code: 'BAD_INPUT' } })
}
if (sub.userId !== me.id) {
throw new GraphQLError('you do not own this sub', { extensions: { code: 'BAD_INPUT' } })
}
const user = await models.user.findFirst({ where: { name: userName } })
if (!user) {
throw new GraphQLError('user not found', { extensions: { code: 'BAD_INPUT' } })
}
if (user.id === me.id) {
throw new GraphQLError('cannot transfer territory to yourself', { extensions: { code: 'BAD_INPUT' } })
}
const [, updatedSub] = await models.$transaction([
models.territoryTransfer.create({ data: { subName, oldUserId: me.id, newUserId: user.id } }),
models.sub.update({ where: { name: subName }, data: { userId: user.id } })
])
notifyTerritoryTransfer({ models, sub, to: user })
return updatedSub
}
},
Sub: {

View File

@ -108,9 +108,16 @@ export default gql`
sortTime: Date!
}
type TerritoryTransfer {
id: ID!
sub: Sub!
sortTime: Date!
}
union Notification = Reply | Votification | Mention
| Invitification | Earn | JobChanged | InvoicePaid | Referral
| Streak | FollowActivity | ForwardedVotification | Revenue | SubStatus | TerritoryPost
| Streak | FollowActivity | ForwardedVotification | Revenue | SubStatus
| TerritoryPost | TerritoryTransfer
type Notifications {
lastChecked: Date

View File

@ -22,6 +22,7 @@ export default gql`
paySub(name: String!, hash: String, hmac: String): Sub
toggleMuteSub(name: String!): Boolean!
toggleSubSubscription(name: String!): Boolean!
transferTerritory(subName: String!, userName: String!): Sub
}
type Sub {

View File

@ -49,7 +49,8 @@ function Notification ({ n, fresh }) {
(type === 'Reply' && <Reply n={n} />) ||
(type === 'SubStatus' && <SubStatus n={n} />) ||
(type === 'FollowActivity' && <FollowActivity n={n} />) ||
(type === 'TerritoryPost' && <TerritoryPost n={n} />)
(type === 'TerritoryPost' && <TerritoryPost n={n} />) ||
(type === 'TerritoryTransfer' && <TerritoryTransfer n={n} />)
}
</NotificationLayout>
)
@ -96,6 +97,7 @@ const defaultOnClick = n => {
if (type === 'InvoicePaid') return { href: `/invoices/${n.invoice.id}` }
if (type === 'Referral') return { href: '/referrals/month' }
if (type === 'Streak') return {}
if (type === 'TerritoryTransfer') return { href: `/~${n.sub.name}` }
// Votification, Mention, JobChanged, Reply all have item
if (!n.item.title) {
@ -426,6 +428,17 @@ function TerritoryPost ({ n }) {
)
}
function TerritoryTransfer ({ n }) {
return (
<>
<div className='fw-bold text-info ms-2'>
~{n.sub.name} was transferred to you
<small className='text-muted ms-1 fw-normal' suppressHydrationWarning>{timeSince(new Date(n.sortTime))}</small>
</div>
</>
)
}
export function NotificationAlert () {
const [showAlert, setShowAlert] = useState(false)
const [hasSubscription, setHasSubscription] = useState(false)

View File

@ -11,6 +11,7 @@ import Share from './share'
import { gql, useMutation } from '@apollo/client'
import { useToast } from './toast'
import ActionDropdown from './action-dropdown'
import { TerritoryTransferDropdownItem } from './territory-transfer'
export function TerritoryDetails ({ sub }) {
return (
@ -72,6 +73,8 @@ export default function TerritoryHeader ({ sub }) {
}
)
const isMine = Number(sub.userId) === Number(me?.id)
return (
<>
<TerritoryPaymentDue sub={sub} />
@ -83,7 +86,7 @@ export default function TerritoryHeader ({ sub }) {
<Share path={`/~${sub.name}`} title={`~${sub.name} stacker news territory`} className='mx-1' />
{me &&
<>
{(Number(sub.userId) === Number(me?.id)
{(isMine
? (
<Link href={`/~${sub.name}/edit`} className='d-flex align-items-center'>
<Button variant='outline-grey border-2 rounded py-0' size='sm'>edit territory</Button>
@ -106,6 +109,12 @@ export default function TerritoryHeader ({ sub }) {
)}
<ActionDropdown>
<ToggleSubSubscriptionDropdownItem sub={sub} />
{isMine && (
<>
<Dropdown.Divider />
<TerritoryTransferDropdownItem sub={sub} />
</>
)}
</ActionDropdown>
</>}
</div>

View File

@ -0,0 +1,100 @@
import { gql, useApolloClient, useMutation } from '@apollo/client'
import { useShowModal } from './modal'
import { useToast } from './toast'
import { Button, Dropdown, InputGroup } from 'react-bootstrap'
import { Form, InputUserSuggest, SubmitButton } from './form'
import { territoryTransferSchema } from '../lib/validate'
import { useCallback } from 'react'
import Link from 'next/link'
import { useMe } from './me'
function TransferObstacle ({ sub, onClose, userName }) {
const toaster = useToast()
const [transfer] = useMutation(
gql`
mutation transferTerritory($subName: String!, $userName: String!) {
transferTerritory(subName: $subName, userName: $userName) {
name
user {
id
}
}
}
`
)
return (
<div className='text-center'>
Do you really want to transfer your territory
<div>
<Link href={`/~${sub.name}`}>~{sub.name}</Link>
{' '}to{' '}
<Link href={`/${userName}`}>@{userName}</Link>?
</div>
<div className='d-flex justify-center align-items-center mt-3 mx-auto'>
<Button className='d-flex ms-auto mx-3' variant='danger' onClick={onClose}>cancel</Button>
<Button
className='d-flex me-auto mx-3' variant='success'
onClick={
async () => {
try {
await transfer({ variables: { subName: sub.name, userName } })
onClose()
toaster.success('transfer successful')
} catch (err) {
console.error(err)
toaster.danger('failed to transfer')
}
}
}
>confirm
</Button>
</div>
</div>
)
}
function TerritoryTransferForm ({ sub, onClose }) {
const showModal = useShowModal()
const client = useApolloClient()
const me = useMe()
const schema = territoryTransferSchema({ me, client })
const onSubmit = useCallback(async (values) => {
showModal(onClose => <TransferObstacle sub={sub} onClose={onClose} {...values} />)
}, [])
return (
<Form
initial={{
userName: ''
}}
schema={schema}
onSubmit={onSubmit}
>
<h2 className='text-center'>transfer territory</h2>
<div className='d-flex align-items-center mb-2'>
<InputUserSuggest
label='stacker'
name='userName'
prepend={<InputGroup.Text>@</InputGroup.Text>}
showValid
autoFocus
/>
</div>
<SubmitButton variant='success'>transfer</SubmitButton>
</Form>
)
}
export function TerritoryTransferDropdownItem ({ sub }) {
const showModal = useShowModal()
return (
<Dropdown.Item onClick={async () =>
showModal(onClose =>
<TerritoryTransferForm sub={sub} onClose={onClose} />)}
>
transfer
</Dropdown.Item>
)
}

View File

@ -94,6 +94,13 @@ export const NOTIFICATIONS = gql`
text
}
}
... on TerritoryTransfer {
id
sortTime
sub {
...SubFields
}
}
... on Invitification {
id
sortTime

View File

@ -130,3 +130,14 @@ export const notifyZapped = async ({ models, id }) => {
console.error(err)
}
}
export const notifyTerritoryTransfer = async ({ models, sub, to }) => {
try {
await sendUserNotification(to.id, {
title: `~${sub.name} was transferred to you`,
tag: `TERRITORY_TRANSFER-${sub.name}`
})
} catch (err) {
console.error(err)
}
}

View File

@ -431,6 +431,25 @@ export function territorySchema (args) {
})
}
export function territoryTransferSchema ({ me, ...args }) {
return object({
userName: nameValidator
.test({
name: 'name',
test: async name => {
if (!name || !name.length) return false
return await usernameExists(name, args)
},
message: 'user does not exist'
})
.test({
name: 'name',
test: name => !me || me.name !== name,
message: 'cannot transfer to yourself'
})
})
}
export function userSchema (args) {
return object({
name: nameValidator

View File

@ -0,0 +1,25 @@
-- CreateTable
CREATE TABLE "TerritoryTransfer" (
"id" SERIAL NOT NULL,
"created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
"oldUserId" INTEGER NOT NULL,
"newUserId" INTEGER NOT NULL,
"subName" CITEXT NOT NULL,
CONSTRAINT "TerritoryTransfer_pkey" PRIMARY KEY ("id")
);
-- CreateIndex
CREATE INDEX "TerritoryTransfer.newUserId_index" ON "TerritoryTransfer"("created_at", "newUserId");
-- CreateIndex
CREATE INDEX "TerritoryTransfer.oldUserId_index" ON "TerritoryTransfer"("created_at", "oldUserId");
-- AddForeignKey
ALTER TABLE "TerritoryTransfer" ADD CONSTRAINT "TerritoryTransfer_oldUserId_fkey" FOREIGN KEY ("oldUserId") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE;
-- AddForeignKey
ALTER TABLE "TerritoryTransfer" ADD CONSTRAINT "TerritoryTransfer_newUserId_fkey" FOREIGN KEY ("newUserId") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE;
-- AddForeignKey
ALTER TABLE "TerritoryTransfer" ADD CONSTRAINT "TerritoryTransfer_subName_fkey" FOREIGN KEY ("subName") REFERENCES "Sub"("name") ON DELETE CASCADE ON UPDATE CASCADE;

View File

@ -114,6 +114,8 @@ model User {
SubAct SubAct[]
MuteSub MuteSub[]
Wallet Wallet[]
TerritoryTransfers TerritoryTransfer[] @relation("TerritoryTransfer_oldUser")
TerritoryReceives TerritoryTransfer[] @relation("TerritoryTransfer_newUser")
@@index([photoId])
@@index([createdAt], map: "users.created_at_index")
@ -490,6 +492,7 @@ model Sub {
SubAct SubAct[]
MuteSub MuteSub[]
SubSubscription SubSubscription[]
TerritoryTransfer TerritoryTransfer[]
@@index([parentName])
@@index([createdAt])
@ -772,6 +775,20 @@ model Log {
@@index([createdAt, name], map: "Log.name_index")
}
model TerritoryTransfer {
id Int @id @default(autoincrement())
createdAt DateTime @default(now()) @map("created_at")
oldUserId Int
newUserId Int
subName String @db.Citext
oldUser User @relation("TerritoryTransfer_oldUser", fields: [oldUserId], references: [id], onDelete: Cascade)
newUser User @relation("TerritoryTransfer_newUser", fields: [newUserId], references: [id], onDelete: Cascade)
sub Sub @relation(fields: [subName], references: [name], onDelete: Cascade)
@@index([createdAt, newUserId], map: "TerritoryTransfer.newUserId_index")
@@index([createdAt, oldUserId], map: "TerritoryTransfer.oldUserId_index")
}
enum EarnType {
POST
COMMENT

View File

@ -91,7 +91,8 @@ export function onPush (sw) {
// if there is no tag or it's a TIP, FORWARDEDTIP or EARN notification
// we don't need to merge notifications and thus the notification should be immediately shown using `showNotification`
const immediatelyShowNotification = (tag) => !tag || ['TIP', 'FORWARDEDTIP', 'EARN', 'STREAK'].includes(tag.split('-')[0])
const immediatelyShowNotification = (tag) =>
!tag || ['TIP', 'FORWARDEDTIP', 'EARN', 'STREAK', 'TERRITORY_TRANSFER'].includes(tag.split('-')[0])
const mergeAndShowNotification = async (sw, payload, currentNotifications, tag, nid) => {
// sanity check