✨ Track record/account delete and update data in subject status #2804

foysalit · 2024-09-09T17:45:44Z

This PR updates moderation subject status with 3 new properties recordUpdatedAt recordDeletedAt and recordStatus. as account/record events are received, these help moderators query subject statuses with delete/update time range and record status.

foysalit · 2024-09-09T19:29:10Z

packages/ozone/src/lexicon/lexicons.ts

+            type: 'string',
+            format: 'datetime',
+            description:
+              'Last update timestamp of the record the subject is associated with',


I feel like the language here is quite convoluted. would love suggestions to make it clearer.

Perhaps something along the lines of: "Timestamp at which the record or account was last updated."

foysalit · 2024-09-09T19:29:42Z

packages/ozone/src/lexicon/lexicons.ts

@@ -11127,7 +11145,7 @@ export const schemaDict = {
        type: 'object',
        description:
          'Logs account status related events on a repo subject. Normally captured by automod from the firehose and emitted to ozone for historical tracking.',
-        required: ['timestamp', 'status', 'active'],
+        required: ['timestamp', 'active'],


TODO: if this PR doesn't make it into the main branch, we'd wanna port this change over to the parent PR.

foysalit · 2024-09-09T19:38:41Z

packages/ozone/src/mod-service/status.ts

+      // When deactivated accounts are re-activated, we receive the event with just the active flag set to true
+      // so we want to make sure that the recordStatus is not set to an outdated value
+      if (currentStatus?.recordStatus !== 'active' && event.meta?.active) {
+        status.recordStatus = 'active'


default for recordStatus is null, may be we should fallback to that? although, doing this is maybe helpful if we want to see which accounts reactivated.

I'd recommend reflecting the exact hosting status here (where active is either null or 'active', slight preference for the latter), and then if we want to keep tract of reactivations we do so with a separate field such as reactivatedAt. That allows the status to not be dependent on past statuses, and if we want to know if something is active we don't need to check for multiple different values.

yeah the null value is basically "we haven't tracked any hosting status for this" so I think safer option is to check for actual deactivated value for status.

devinivy · 2024-09-20T21:14:01Z

packages/ozone/src/mod-service/status.ts

+
+  if (event.action === 'tools.ozone.moderation.defs#accountEvent') {
+    const status: Partial<ModerationSubjectStatusRow> = {
+      recordStatus: `${event.meta?.status}`,


Based on the types here, as written this could end-up containing the string 'undefined'.

devinivy · 2024-09-20T21:14:31Z

packages/ozone/src/mod-service/status.ts

+    }
+
+    if (event.meta?.tombstone) {
+      status.recordStatus = 'tombstoned'


I see a couple values for recordStatus in here that aren't reflected in the lexicon.

(P.S. is this separate from 'deleted'? Historically we have used the term "tombstoned" in a couple different ways, and I'm not sure which one is meant here.)

to my understanding, the tombstoned event is only emitted on identity and this is "technically" equivalend as "deleted" but not exactly the same thing.

devinivy · 2024-09-20T21:20:14Z

lexicons/tools/ozone/moderation/defs.json

+        "recordUpdatedAt": {
+          "type": "string",
+          "format": "datetime",
+          "description": "Last update timestamp of the record the subject is associated with"
+        },
+        "recordDeletedAt": {
+          "type": "string",
+          "format": "datetime",
+          "description": "Timestamp referencing when the record the subject is associated with was deleted"
+        },
+        "recordStatus": {
+          "type": "string",
+          "description": "Status of the record the subject is associated with. Statuses are different when the subject references an account vs. a record",
+          "knownValues": ["takendown", "suspended", "deleted", "deactivated"]
+        },


These names work alright if the subject is a record, but if the subject is a repository or chat message, using the term "record" here doesn't seem like the best fit. Would there be any issue with using something like subjectUpdatedAt/subjectDeletedAt instead?

I guess I was using record as a generic term here without the context of atproto records. I'm also not sure about subject prefix either since we already have updatedAt column on subject, we will end up with subject.updatedAt and subject.subjectUpdatedAt which feels odd. I think I'm onboard with the term hosting just from technical standpoint.

devinivy · 2024-09-20T22:07:36Z

lexicons/tools/ozone/moderation/defs.json

+        "recordStatus": {
+          "type": "string",
+          "description": "Status of the record the subject is associated with. Statuses are different when the subject references an account vs. a record",
+          "knownValues": ["takendown", "suspended", "deleted", "deactivated"]
+        },


This is quite a complex field, I think there are a few potential issues with it:

It's called "record status" but it may contain the hosting status of an account.

The hosting statuses of records and accounts are mixed together, which could make it seem like a record can be marked as takendown here, which isn't the case.

Potential confusion between the ozone status and the hosting status: the terms "takendown" and "suspended" are also terms used by ozone overlapping with the takendown and suspendedUntil fields.

Here's one idea: we could combine the three new fields into something more like this (using typescript just to illustrate the concept):

type SubjectStatusView = { // ... hosting?: AccountHosting | RecordHosting } type AccountHosting = { $type: 'tools.ozone.moderation.defs#accountHosting', status: 'active' | 'takendown' | 'suspended' | 'deleted' | 'deactivated' createdAt: Date updatedAt: Date deletedAt?: Date suspendedAt?: Date deactivatedAt?: Date takendownAt?: Date reactivatedAt?: Date } type RecordHosting = { $type: 'tools.ozone.moderation.defs#recordHosting' status: 'active' | 'deleted' createdAt: Date updatedAt: Date deletedAt?: Date }

This also gives us a way to make certain fields required together if we want, e.g. if hosting is present then hosting.createdAt can always be present too.

this is nice!

devinivy · 2024-09-20T22:09:10Z

lexicons/tools/ozone/moderation/queryStatuses.json

@@ -28,6 +28,31 @@
            "format": "datetime",
            "description": "Search subjects reviewed after a given timestamp"
          },
+          "recordDeletedAfter": {


My qualm with these fields is just that they are named "record" when they really may apply to any subject, e.g. an account rather than a record.

devinivy · 2024-09-20T22:10:12Z

packages/ozone/src/db/migrations/20240909T205730722Z-add-record-status-to-subject-status.ts

+  await db.schema
+    .alterTable('moderation_subject_status')
+    .addColumn('recordStatus', 'varchar')
+    .execute()
+  await db.schema
+    .alterTable('moderation_subject_status')
+    .addColumn('recordDeletedAt', 'varchar')
+    .execute()
+  await db.schema
+    .alterTable('moderation_subject_status')
+    .addColumn('recordUpdatedAt', 'varchar')
+    .execute()


I'd recommend distinguishing these as e.g. hostingStatus, hostingDeletedAt, etc.

devinivy · 2024-09-20T22:12:39Z

packages/ozone/src/lexicon/lexicons.ts

+            type: 'string',
+            format: 'datetime',
+            description:
+              'Last update timestamp of the record the subject is associated with',


Perhaps something along the lines of: "Timestamp at which the record or account was last updated."

devinivy · 2024-09-20T22:17:24Z

packages/ozone/src/mod-service/status.ts

+  const timestamp = event.meta?.timestamp
+    ? `${event.meta?.timestamp}`
+    : event.createdAt


The types here are a little funky to wrangle. To avoid ending up with a string like 'true' here, could we go with something more explicit like:

Suggested change

const timestamp = event.meta?.timestamp

? `${event.meta?.timestamp}`

: event.createdAt

const timestamp = typeof event.meta?.timestamp === 'string'

? event.meta.timestamp

: event.createdAt

devinivy · 2024-09-20T22:19:51Z

packages/ozone/src/mod-service/status.ts

+      // When deactivated accounts are re-activated, we receive the event with just the active flag set to true
+      // so we want to make sure that the recordStatus is not set to an outdated value
+      if (currentStatus?.recordStatus !== 'active' && event.meta?.active) {
+        status.recordStatus = 'active'


I'd recommend reflecting the exact hosting status here (where active is either null or 'active', slight preference for the latter), and then if we want to keep tract of reactivations we do so with a separate field such as reactivatedAt. That allows the status to not be dependent on past statuses, and if we want to know if something is active we don't need to check for multiple different values.

…oto into ozone-track-record-delete-and-update

devinivy · 2024-10-30T05:02:04Z

lexicons/tools/ozone/moderation/defs.json

+        "status": {
+          "type": "string",
+          "knownValues": ["takendown", "suspended", "deleted", "deactivated"]
+        },


From a hosting perspective individual records can't be deactivated, takendown, or suspended— only accounts can. They can be deleted, though.

Ah, I see the values I'd expect reflected in the implementation, so this probably just needs to be updated.

devinivy · 2024-10-30T05:04:07Z

lexicons/tools/ozone/moderation/defs.json

+        "status": {
+          "type": "string",
+          "knownValues": ["takendown", "suspended", "deleted", "deactivated"]
+        },


I see the value unknown used in the implementation—may be worth adding here and to the same field in #recordHosting.

devinivy

Nice! Let's just add a changeset here before merge.

foysalit · 2024-10-30T17:54:40Z

this is going to be merged into #2661 and I'll add the changeset there since that's the one landing on main.

* ✨ Add events for account and record update/delete/deactivation * ✨ Add handle change event * ✨ Reduce account events to 2 types and record events to 1 * ✨ Store metadata from account, identity and record events * ✨ Add created event for record * ✨ Add ndd the new events to allowed types in emitEvent * ✨ Use string value for record op and add tombstone flag to identity event * ✨ Add active flag on account events * ✨ Change accountStatus -> status to match with firehose event * ✨ Make active flag required * 🚨 fix prettier style issue * ✨ Track record/account delete and update data in subject status (#2804) * ✨ Store deleted/updated event data in subject_status * 🐛 Fix query for recordDeletedAt and recordUpdatedAt * ✨ Add tombstoned status * ✨ Move from record to hosting term * ✅ Add tests for hosting params * ✨ Update lexicons for hostingStatuses * ✅ Update snapshots * ✅ Update snapshots * ✅ Update snapshots * ✨ Adjust hosting statuses * 📝 Add changeset

✨ Store deleted/updated event data in subject_status

445077f

foysalit commented Sep 9, 2024

View reviewed changes

🐛 Fix query for recordDeletedAt and recordUpdatedAt

d333440

foysalit commented Sep 9, 2024

View reviewed changes

✨ Add tombstoned status

5d14dc4

foysalit requested review from devinivy and bnewbold September 11, 2024 16:23

devinivy reviewed Sep 20, 2024

View reviewed changes

foysalit added 9 commits September 30, 2024 20:52

Merge branch 'ozone-account-events' of github.com:bluesky-social/atpr…

6111890

…oto into ozone-track-record-delete-and-update

Merge branch 'ozone-account-events' of github.com:bluesky-social/atpr…

cc69076

…oto into ozone-track-record-delete-and-update

:rightwards_twisted_arrows: Merge with upstream

c7e0c31

✨ Move from record to hosting term

ecd3578

✅ Add tests for hosting params

7062a97

✨ Update lexicons for hostingStatuses

e34afca

✅ Update snapshots

13178bb

✅ Update snapshots

aacbb31

✅ Update snapshots

6febbd3

devinivy reviewed Oct 30, 2024

View reviewed changes

devinivy approved these changes Oct 30, 2024

View reviewed changes

✨ Adjust hosting statuses

2e1a1f0

foysalit merged commit b9bb009 into ozone-account-events Oct 30, 2024
10 checks passed

foysalit deleted the ozone-track-record-delete-and-update branch October 30, 2024 21:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Track record/account delete and update data in subject status #2804

✨ Track record/account delete and update data in subject status #2804

foysalit commented Sep 9, 2024

foysalit Sep 9, 2024

devinivy Sep 20, 2024

foysalit Sep 9, 2024

foysalit Sep 9, 2024

devinivy Sep 20, 2024

foysalit Oct 26, 2024

devinivy Sep 20, 2024

devinivy Sep 20, 2024

devinivy Sep 20, 2024

foysalit Oct 26, 2024

devinivy Sep 20, 2024

foysalit Oct 1, 2024

devinivy Sep 20, 2024

foysalit Oct 26, 2024

devinivy Sep 20, 2024

devinivy Sep 20, 2024

devinivy Sep 20, 2024

devinivy Sep 20, 2024

devinivy Sep 20, 2024

devinivy Oct 30, 2024

devinivy Oct 30, 2024

devinivy Oct 30, 2024

devinivy left a comment •

edited

Loading

foysalit commented Oct 30, 2024

✨ Track record/account delete and update data in subject status #2804

✨ Track record/account delete and update data in subject status #2804

Conversation

foysalit commented Sep 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devinivy left a comment • edited Loading

Choose a reason for hiding this comment

foysalit commented Oct 30, 2024

devinivy left a comment •

edited

Loading