From 8c62af0410eb8d6006dd057dd57fab59dc1e1d47 Mon Sep 17 00:00:00 2001 From: Foysal Ahamed Date: Sun, 25 Feb 2024 18:22:14 +0100 Subject: [PATCH 1/6] :construction: Working through an nullable review state --- lexicons/com/atproto/admin/defs.json | 11 +++- packages/api/src/client/index.ts | 1 + packages/api/src/client/lexicons.ts | 6 ++ .../client/types/com/atproto/admin/defs.ts | 3 + packages/bsky/src/lexicon/index.ts | 1 + packages/bsky/src/lexicon/lexicons.ts | 6 ++ .../lexicon/types/com/atproto/admin/defs.ts | 3 + .../db/schema/moderation_subject_status.ts | 7 ++- packages/ozone/src/lexicon/index.ts | 1 + packages/ozone/src/lexicon/lexicons.ts | 6 ++ .../lexicon/types/com/atproto/admin/defs.ts | 3 + packages/ozone/src/mod-service/status.ts | 60 +++++++++++-------- .../ozone/tests/moderation-events.test.ts | 2 + packages/pds/src/lexicon/index.ts | 1 + packages/pds/src/lexicon/lexicons.ts | 6 ++ .../lexicon/types/com/atproto/admin/defs.ts | 3 + 16 files changed, 92 insertions(+), 28 deletions(-) diff --git a/lexicons/com/atproto/admin/defs.json b/lexicons/com/atproto/admin/defs.json index e1315eb7473..56a855f7427 100644 --- a/lexicons/com/atproto/admin/defs.json +++ b/lexicons/com/atproto/admin/defs.json @@ -449,7 +449,12 @@ }, "subjectReviewState": { "type": "string", - "knownValues": ["#reviewOpen", "#reviewEscalated", "#reviewClosed"] + "knownValues": [ + "#reviewOpen", + "#reviewEscalated", + "#reviewClosed", + "#reviewOptional" + ] }, "reviewOpen": { "type": "token", @@ -463,6 +468,10 @@ "type": "token", "description": "Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator" }, + "reviewOptional": { + "type": "token", + "description": "Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it" + }, "modEventTakedown": { "type": "object", "description": "Take down a subject permanently or temporarily", diff --git a/packages/api/src/client/index.ts b/packages/api/src/client/index.ts index 846c379b7a8..7f55f73390c 100644 --- a/packages/api/src/client/index.ts +++ b/packages/api/src/client/index.ts @@ -319,6 +319,7 @@ export const COM_ATPROTO_ADMIN = { DefsReviewOpen: 'com.atproto.admin.defs#reviewOpen', DefsReviewEscalated: 'com.atproto.admin.defs#reviewEscalated', DefsReviewClosed: 'com.atproto.admin.defs#reviewClosed', + DefsReviewOptional: 'com.atproto.admin.defs#reviewOptional', } export const COM_ATPROTO_MODERATION = { DefsReasonSpam: 'com.atproto.moderation.defs#reasonSpam', diff --git a/packages/api/src/client/lexicons.ts b/packages/api/src/client/lexicons.ts index d546b70b987..b95afb50e2a 100644 --- a/packages/api/src/client/lexicons.ts +++ b/packages/api/src/client/lexicons.ts @@ -747,6 +747,7 @@ export const schemaDict = { 'lex:com.atproto.admin.defs#reviewOpen', 'lex:com.atproto.admin.defs#reviewEscalated', 'lex:com.atproto.admin.defs#reviewClosed', + 'lex:com.atproto.admin.defs#reviewOptional', ], }, reviewOpen: { @@ -764,6 +765,11 @@ export const schemaDict = { description: 'Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator', }, + reviewOptional: { + type: 'token', + description: + 'Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it', + }, modEventTakedown: { type: 'object', description: 'Take down a subject permanently or temporarily', diff --git a/packages/api/src/client/types/com/atproto/admin/defs.ts b/packages/api/src/client/types/com/atproto/admin/defs.ts index 4e3d35a869f..b3dbb45bc23 100644 --- a/packages/api/src/client/types/com/atproto/admin/defs.ts +++ b/packages/api/src/client/types/com/atproto/admin/defs.ts @@ -497,6 +497,7 @@ export type SubjectReviewState = | 'lex:com.atproto.admin.defs#reviewOpen' | 'lex:com.atproto.admin.defs#reviewEscalated' | 'lex:com.atproto.admin.defs#reviewClosed' + | 'lex:com.atproto.admin.defs#reviewOptional' | (string & {}) /** Moderator review status of a subject: Open. Indicates that the subject needs to be reviewed by a moderator */ @@ -505,6 +506,8 @@ export const REVIEWOPEN = 'com.atproto.admin.defs#reviewOpen' export const REVIEWESCALATED = 'com.atproto.admin.defs#reviewEscalated' /** Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator */ export const REVIEWCLOSED = 'com.atproto.admin.defs#reviewClosed' +/** Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it */ +export const REVIEWOPTIONAL = 'com.atproto.admin.defs#reviewOptional' /** Take down a subject permanently or temporarily */ export interface ModEventTakedown { diff --git a/packages/bsky/src/lexicon/index.ts b/packages/bsky/src/lexicon/index.ts index cf2c613e686..d42435e2f7c 100644 --- a/packages/bsky/src/lexicon/index.ts +++ b/packages/bsky/src/lexicon/index.ts @@ -142,6 +142,7 @@ export const COM_ATPROTO_ADMIN = { DefsReviewOpen: 'com.atproto.admin.defs#reviewOpen', DefsReviewEscalated: 'com.atproto.admin.defs#reviewEscalated', DefsReviewClosed: 'com.atproto.admin.defs#reviewClosed', + DefsReviewOptional: 'com.atproto.admin.defs#reviewOptional', } export const COM_ATPROTO_MODERATION = { DefsReasonSpam: 'com.atproto.moderation.defs#reasonSpam', diff --git a/packages/bsky/src/lexicon/lexicons.ts b/packages/bsky/src/lexicon/lexicons.ts index d546b70b987..b95afb50e2a 100644 --- a/packages/bsky/src/lexicon/lexicons.ts +++ b/packages/bsky/src/lexicon/lexicons.ts @@ -747,6 +747,7 @@ export const schemaDict = { 'lex:com.atproto.admin.defs#reviewOpen', 'lex:com.atproto.admin.defs#reviewEscalated', 'lex:com.atproto.admin.defs#reviewClosed', + 'lex:com.atproto.admin.defs#reviewOptional', ], }, reviewOpen: { @@ -764,6 +765,11 @@ export const schemaDict = { description: 'Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator', }, + reviewOptional: { + type: 'token', + description: + 'Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it', + }, modEventTakedown: { type: 'object', description: 'Take down a subject permanently or temporarily', diff --git a/packages/bsky/src/lexicon/types/com/atproto/admin/defs.ts b/packages/bsky/src/lexicon/types/com/atproto/admin/defs.ts index a713a635635..9e6aa7e3269 100644 --- a/packages/bsky/src/lexicon/types/com/atproto/admin/defs.ts +++ b/packages/bsky/src/lexicon/types/com/atproto/admin/defs.ts @@ -497,6 +497,7 @@ export type SubjectReviewState = | 'lex:com.atproto.admin.defs#reviewOpen' | 'lex:com.atproto.admin.defs#reviewEscalated' | 'lex:com.atproto.admin.defs#reviewClosed' + | 'lex:com.atproto.admin.defs#reviewOptional' | (string & {}) /** Moderator review status of a subject: Open. Indicates that the subject needs to be reviewed by a moderator */ @@ -505,6 +506,8 @@ export const REVIEWOPEN = 'com.atproto.admin.defs#reviewOpen' export const REVIEWESCALATED = 'com.atproto.admin.defs#reviewEscalated' /** Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator */ export const REVIEWCLOSED = 'com.atproto.admin.defs#reviewClosed' +/** Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it */ +export const REVIEWOPTIONAL = 'com.atproto.admin.defs#reviewOptional' /** Take down a subject permanently or temporarily */ export interface ModEventTakedown { diff --git a/packages/ozone/src/db/schema/moderation_subject_status.ts b/packages/ozone/src/db/schema/moderation_subject_status.ts index 59803133bcb..4ed2751f217 100644 --- a/packages/ozone/src/db/schema/moderation_subject_status.ts +++ b/packages/ozone/src/db/schema/moderation_subject_status.ts @@ -3,6 +3,7 @@ import { REVIEWCLOSED, REVIEWOPEN, REVIEWESCALATED, + REVIEWOPTIONAL, } from '../../lexicon/types/com/atproto/admin/defs' export const subjectStatusTableName = 'moderation_subject_status' @@ -13,7 +14,11 @@ export interface ModerationSubjectStatus { recordPath: string recordCid: string | null blobCids: string[] | null - reviewState: typeof REVIEWCLOSED | typeof REVIEWOPEN | typeof REVIEWESCALATED + reviewState: + | typeof REVIEWCLOSED + | typeof REVIEWOPEN + | typeof REVIEWESCALATED + | typeof REVIEWOPTIONAL createdAt: string updatedAt: string lastReviewedBy: string | null diff --git a/packages/ozone/src/lexicon/index.ts b/packages/ozone/src/lexicon/index.ts index cf2c613e686..d42435e2f7c 100644 --- a/packages/ozone/src/lexicon/index.ts +++ b/packages/ozone/src/lexicon/index.ts @@ -142,6 +142,7 @@ export const COM_ATPROTO_ADMIN = { DefsReviewOpen: 'com.atproto.admin.defs#reviewOpen', DefsReviewEscalated: 'com.atproto.admin.defs#reviewEscalated', DefsReviewClosed: 'com.atproto.admin.defs#reviewClosed', + DefsReviewOptional: 'com.atproto.admin.defs#reviewOptional', } export const COM_ATPROTO_MODERATION = { DefsReasonSpam: 'com.atproto.moderation.defs#reasonSpam', diff --git a/packages/ozone/src/lexicon/lexicons.ts b/packages/ozone/src/lexicon/lexicons.ts index d546b70b987..b95afb50e2a 100644 --- a/packages/ozone/src/lexicon/lexicons.ts +++ b/packages/ozone/src/lexicon/lexicons.ts @@ -747,6 +747,7 @@ export const schemaDict = { 'lex:com.atproto.admin.defs#reviewOpen', 'lex:com.atproto.admin.defs#reviewEscalated', 'lex:com.atproto.admin.defs#reviewClosed', + 'lex:com.atproto.admin.defs#reviewOptional', ], }, reviewOpen: { @@ -764,6 +765,11 @@ export const schemaDict = { description: 'Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator', }, + reviewOptional: { + type: 'token', + description: + 'Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it', + }, modEventTakedown: { type: 'object', description: 'Take down a subject permanently or temporarily', diff --git a/packages/ozone/src/lexicon/types/com/atproto/admin/defs.ts b/packages/ozone/src/lexicon/types/com/atproto/admin/defs.ts index a713a635635..9e6aa7e3269 100644 --- a/packages/ozone/src/lexicon/types/com/atproto/admin/defs.ts +++ b/packages/ozone/src/lexicon/types/com/atproto/admin/defs.ts @@ -497,6 +497,7 @@ export type SubjectReviewState = | 'lex:com.atproto.admin.defs#reviewOpen' | 'lex:com.atproto.admin.defs#reviewEscalated' | 'lex:com.atproto.admin.defs#reviewClosed' + | 'lex:com.atproto.admin.defs#reviewOptional' | (string & {}) /** Moderator review status of a subject: Open. Indicates that the subject needs to be reviewed by a moderator */ @@ -505,6 +506,8 @@ export const REVIEWOPEN = 'com.atproto.admin.defs#reviewOpen' export const REVIEWESCALATED = 'com.atproto.admin.defs#reviewEscalated' /** Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator */ export const REVIEWCLOSED = 'com.atproto.admin.defs#reviewClosed' +/** Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it */ +export const REVIEWOPTIONAL = 'com.atproto.admin.defs#reviewOptional' /** Take down a subject permanently or temporarily */ export interface ModEventTakedown { diff --git a/packages/ozone/src/mod-service/status.ts b/packages/ozone/src/mod-service/status.ts index 2edba64282f..efbdbe66290 100644 --- a/packages/ozone/src/mod-service/status.ts +++ b/packages/ozone/src/mod-service/status.ts @@ -7,6 +7,7 @@ import { REVIEWOPEN, REVIEWCLOSED, REVIEWESCALATED, + REVIEWOPTIONAL, } from '../lexicon/types/com/atproto/admin/defs' import { ModerationEventRow, ModerationSubjectStatusRow } from './types' import { HOUR } from '@atproto/common' @@ -14,16 +15,22 @@ import { REASONAPPEAL } from '../lexicon/types/com/atproto/moderation/defs' import { jsonb } from '../db/types' const getSubjectStatusForModerationEvent = ({ + currentStatus, action, createdBy, createdAt, durationInHours, }: { + currentStatus?: ModerationSubjectStatusRow action: string createdBy: string createdAt: string durationInHours: number | null -}): Partial | null => { +}): Partial => { + const defaultReviewState = currentStatus + ? currentStatus.reviewState + : REVIEWOPTIONAL + switch (action) { case 'com.atproto.admin.defs#modEventAcknowledge': return { @@ -54,7 +61,9 @@ const getSubjectStatusForModerationEvent = ({ return { lastReviewedBy: createdBy, muteUntil: null, - reviewState: REVIEWOPEN, + // It's not likely to receive an unmute event that does not already have a status row + // but if it does happen, default to unnecessary + reviewState: defaultReviewState, lastReviewedAt: createdAt, } case 'com.atproto.admin.defs#modEventTakedown': @@ -70,26 +79,29 @@ const getSubjectStatusForModerationEvent = ({ case 'com.atproto.admin.defs#modEventMute': return { lastReviewedBy: createdBy, - reviewState: REVIEWOPEN, lastReviewedAt: createdAt, // By default, mute for 24hrs muteUntil: new Date( Date.now() + (durationInHours || 24) * HOUR, ).toISOString(), + // It's not likely to receive a mute event on a subject that does not already have a status row + // but if it does happen, default to unnecessary + reviewState: defaultReviewState, } case 'com.atproto.admin.defs#modEventComment': return { lastReviewedBy: createdBy, lastReviewedAt: createdAt, + reviewState: defaultReviewState, } case 'com.atproto.admin.defs#modEventTag': - return { tags: [] } + return { tags: [], reviewState: defaultReviewState } case 'com.atproto.admin.defs#modEventResolveAppeal': return { appealed: false, } default: - return null + return {} } } @@ -114,23 +126,6 @@ export const adjustModerationSubjectStatus = async ( createdAt, } = moderationEvent - const isAppealEvent = - action === 'com.atproto.admin.defs#modEventReport' && - meta?.reportType === REASONAPPEAL - - const subjectStatus = getSubjectStatusForModerationEvent({ - action, - createdBy, - createdAt, - durationInHours: moderationEvent.durationInHours, - }) - - // If there are no subjectStatus that means there are no side-effect of the incoming event - if (!subjectStatus) { - return null - } - - const now = new Date().toISOString() // If subjectUri exists, it's not a repoRef so pass along the uri to get identifier back const identifier = getStatusIdentifierFromSubject(subjectUri || subjectDid) @@ -143,12 +138,25 @@ export const adjustModerationSubjectStatus = async ( .selectAll() .executeTakeFirst() + const isAppealEvent = + action === 'com.atproto.admin.defs#modEventReport' && + meta?.reportType === REASONAPPEAL + + const subjectStatus = getSubjectStatusForModerationEvent({ + currentStatus, + action, + createdBy, + createdAt, + durationInHours: moderationEvent.durationInHours, + }) + + const now = new Date().toISOString() if ( currentStatus?.reviewState === REVIEWESCALATED && - subjectStatus.reviewState === REVIEWOPEN + subjectStatus.reviewState !== REVIEWCLOSED ) { - // If the current status is escalated and the incoming event is to open the review - // We want to keep the status as escalated + // If the current status is escalated only allow incoming events to move the state to + // reviewClosed because escalated subjects should never move to any other state subjectStatus.reviewState = REVIEWESCALATED } @@ -158,7 +166,7 @@ export const adjustModerationSubjectStatus = async ( // Defaulting reviewState to open for any event may not be the desired behavior. // For instance, if a subject never had any event and we just want to leave a comment to keep an eye on it // that shouldn't mean we want to review the subject - reviewState: REVIEWOPEN, + reviewState: REVIEWOPTIONAL, recordCid: subjectCid || null, } const newStatus = { diff --git a/packages/ozone/tests/moderation-events.test.ts b/packages/ozone/tests/moderation-events.test.ts index 12277ea77a4..305bdd11c9e 100644 --- a/packages/ozone/tests/moderation-events.test.ts +++ b/packages/ozone/tests/moderation-events.test.ts @@ -40,6 +40,7 @@ describe('moderation-events', () => { $type: 'com.atproto.admin.defs#repoRef', did: sc.dids.alice, } + console.log({ alicesAccount }) const bobsPost = { $type: 'com.atproto.repo.strongRef', uri: sc.posts[sc.dids.bob][0].ref.uriStr, @@ -358,6 +359,7 @@ describe('moderation-events', () => { { id: 1 }, { headers: network.bsky.adminAuthHeaders('moderator') }, ) + console.log(JSON.stringify(data, null, 2)) expect(forSnapshot(data)).toMatchSnapshot() }) }) diff --git a/packages/pds/src/lexicon/index.ts b/packages/pds/src/lexicon/index.ts index cf2c613e686..d42435e2f7c 100644 --- a/packages/pds/src/lexicon/index.ts +++ b/packages/pds/src/lexicon/index.ts @@ -142,6 +142,7 @@ export const COM_ATPROTO_ADMIN = { DefsReviewOpen: 'com.atproto.admin.defs#reviewOpen', DefsReviewEscalated: 'com.atproto.admin.defs#reviewEscalated', DefsReviewClosed: 'com.atproto.admin.defs#reviewClosed', + DefsReviewOptional: 'com.atproto.admin.defs#reviewOptional', } export const COM_ATPROTO_MODERATION = { DefsReasonSpam: 'com.atproto.moderation.defs#reasonSpam', diff --git a/packages/pds/src/lexicon/lexicons.ts b/packages/pds/src/lexicon/lexicons.ts index d546b70b987..b95afb50e2a 100644 --- a/packages/pds/src/lexicon/lexicons.ts +++ b/packages/pds/src/lexicon/lexicons.ts @@ -747,6 +747,7 @@ export const schemaDict = { 'lex:com.atproto.admin.defs#reviewOpen', 'lex:com.atproto.admin.defs#reviewEscalated', 'lex:com.atproto.admin.defs#reviewClosed', + 'lex:com.atproto.admin.defs#reviewOptional', ], }, reviewOpen: { @@ -764,6 +765,11 @@ export const schemaDict = { description: 'Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator', }, + reviewOptional: { + type: 'token', + description: + 'Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it', + }, modEventTakedown: { type: 'object', description: 'Take down a subject permanently or temporarily', diff --git a/packages/pds/src/lexicon/types/com/atproto/admin/defs.ts b/packages/pds/src/lexicon/types/com/atproto/admin/defs.ts index a713a635635..9e6aa7e3269 100644 --- a/packages/pds/src/lexicon/types/com/atproto/admin/defs.ts +++ b/packages/pds/src/lexicon/types/com/atproto/admin/defs.ts @@ -497,6 +497,7 @@ export type SubjectReviewState = | 'lex:com.atproto.admin.defs#reviewOpen' | 'lex:com.atproto.admin.defs#reviewEscalated' | 'lex:com.atproto.admin.defs#reviewClosed' + | 'lex:com.atproto.admin.defs#reviewOptional' | (string & {}) /** Moderator review status of a subject: Open. Indicates that the subject needs to be reviewed by a moderator */ @@ -505,6 +506,8 @@ export const REVIEWOPEN = 'com.atproto.admin.defs#reviewOpen' export const REVIEWESCALATED = 'com.atproto.admin.defs#reviewEscalated' /** Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator */ export const REVIEWCLOSED = 'com.atproto.admin.defs#reviewClosed' +/** Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it */ +export const REVIEWOPTIONAL = 'com.atproto.admin.defs#reviewOptional' /** Take down a subject permanently or temporarily */ export interface ModEventTakedown { From ccc8ab9045f85c2613e8d1cfd0b441ecfde6dff7 Mon Sep 17 00:00:00 2001 From: Foysal Ahamed Date: Tue, 27 Feb 2024 20:10:13 +0100 Subject: [PATCH 2/6] :white_check_mark: Update snapshots on some tests --- .../__snapshots__/get-record.test.ts.snap | 4 +-- .../tests/__snapshots__/get-repo.test.ts.snap | 2 +- .../moderation-events.test.ts.snap | 34 ++++++++++++++----- .../moderation-statuses.test.ts.snap | 29 ++++++++++++---- .../__snapshots__/moderation.test.ts.snap | 8 ++--- .../ozone/tests/moderation-events.test.ts | 4 +-- .../ozone/tests/moderation-statuses.test.ts | 2 +- 7 files changed, 58 insertions(+), 25 deletions(-) diff --git a/packages/ozone/tests/__snapshots__/get-record.test.ts.snap b/packages/ozone/tests/__snapshots__/get-record.test.ts.snap index decfb8f4ba4..5c977ea406b 100644 --- a/packages/ozone/tests/__snapshots__/get-record.test.ts.snap +++ b/packages/ozone/tests/__snapshots__/get-record.test.ts.snap @@ -27,7 +27,7 @@ Object { "moderation": Object { "subjectStatus": Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 1, + "id": 3, "lastReportedAt": "1970-01-01T00:00:00.000Z", "lastReviewedAt": "1970-01-01T00:00:00.000Z", "lastReviewedBy": "did:example:admin", @@ -124,7 +124,7 @@ Object { "moderation": Object { "subjectStatus": Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 1, + "id": 3, "lastReportedAt": "1970-01-01T00:00:00.000Z", "lastReviewedAt": "1970-01-01T00:00:00.000Z", "lastReviewedBy": "did:example:admin", diff --git a/packages/ozone/tests/__snapshots__/get-repo.test.ts.snap b/packages/ozone/tests/__snapshots__/get-repo.test.ts.snap index 67404b88362..36a2e602c5b 100644 --- a/packages/ozone/tests/__snapshots__/get-repo.test.ts.snap +++ b/packages/ozone/tests/__snapshots__/get-repo.test.ts.snap @@ -20,7 +20,7 @@ Object { "moderation": Object { "subjectStatus": Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 1, + "id": 5, "lastReportedAt": "1970-01-01T00:00:00.000Z", "lastReviewedAt": "1970-01-01T00:00:00.000Z", "lastReviewedBy": "did:example:admin", diff --git a/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap b/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap index ac48d862f58..a8b7f46d7c1 100644 --- a/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap +++ b/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap @@ -18,7 +18,25 @@ Object { "blobCids": Array [], "cid": "cids(0)", "indexedAt": "1970-01-01T00:00:00.000Z", - "moderation": Object {}, + "moderation": Object { + "subjectStatus": Object { + "createdAt": "1970-01-01T00:00:00.000Z", + "id": 1, + "reviewState": "com.atproto.admin.defs#reviewOptional", + "subject": Object { + "$type": "com.atproto.repo.strongRef", + "cid": "cids(0)", + "uri": "record(0)", + }, + "subjectBlobCids": Array [], + "subjectRepoHandle": "alice.test", + "tags": Array [ + "lang:und", + ], + "takendown": false, + "updatedAt": "1970-01-01T00:00:00.000Z", + }, + }, "repo": Object { "did": "user(0)", "handle": "alice.test", @@ -26,7 +44,7 @@ Object { "moderation": Object { "subjectStatus": Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 1, + "id": 5, "lastReportedAt": "1970-01-01T00:00:00.000Z", "lastReviewedAt": "1970-01-01T00:00:00.000Z", "lastReviewedBy": "user(1)", @@ -101,7 +119,7 @@ Array [ "comment": "X", "reportType": "com.atproto.moderation.defs#reasonSpam", }, - "id": 13, + "id": 15, "subject": Object { "$type": "com.atproto.admin.defs#repoRef", "did": "user(0)", @@ -120,7 +138,7 @@ Array [ ], "remove": Array [], }, - "id": 8, + "id": 10, "subject": Object { "$type": "com.atproto.admin.defs#repoRef", "did": "user(0)", @@ -137,7 +155,7 @@ Array [ "comment": "X", "reportType": "com.atproto.moderation.defs#reasonSpam", }, - "id": 7, + "id": 9, "subject": Object { "$type": "com.atproto.admin.defs#repoRef", "did": "user(0)", @@ -159,7 +177,7 @@ Array [ "comment": "X", "reportType": "com.atproto.moderation.defs#reasonSpam", }, - "id": 12, + "id": 14, "subject": Object { "$type": "com.atproto.repo.strongRef", "cid": "cids(0)", @@ -178,7 +196,7 @@ Array [ ], "remove": Array [], }, - "id": 6, + "id": 8, "subject": Object { "$type": "com.atproto.repo.strongRef", "cid": "cids(0)", @@ -196,7 +214,7 @@ Array [ "comment": "X", "reportType": "com.atproto.moderation.defs#reasonSpam", }, - "id": 5, + "id": 7, "subject": Object { "$type": "com.atproto.repo.strongRef", "cid": "cids(0)", diff --git a/packages/ozone/tests/__snapshots__/moderation-statuses.test.ts.snap b/packages/ozone/tests/__snapshots__/moderation-statuses.test.ts.snap index cf361739a6c..b2e0cabee16 100644 --- a/packages/ozone/tests/__snapshots__/moderation-statuses.test.ts.snap +++ b/packages/ozone/tests/__snapshots__/moderation-statuses.test.ts.snap @@ -4,7 +4,7 @@ exports[`moderation-statuses query statuses returns statuses filtered by subject Array [ Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 7, + "id": 9, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -23,7 +23,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 5, + "id": 7, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -46,7 +46,7 @@ exports[`moderation-statuses query statuses returns statuses for subjects that r Array [ Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 7, + "id": 9, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -65,7 +65,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 5, + "id": 7, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -83,7 +83,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 3, + "id": 5, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -101,7 +101,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 1, + "id": 3, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -116,5 +116,22 @@ Array [ "takendown": false, "updatedAt": "1970-01-01T00:00:00.000Z", }, + Object { + "createdAt": "1970-01-01T00:00:00.000Z", + "id": 1, + "reviewState": "com.atproto.admin.defs#reviewOptional", + "subject": Object { + "$type": "com.atproto.repo.strongRef", + "cid": "cids(2)", + "uri": "record(2)", + }, + "subjectBlobCids": Array [], + "subjectRepoHandle": "bob.test", + "tags": Array [ + "lang:und", + ], + "takendown": false, + "updatedAt": "1970-01-01T00:00:00.000Z", + }, ] `; diff --git a/packages/ozone/tests/__snapshots__/moderation.test.ts.snap b/packages/ozone/tests/__snapshots__/moderation.test.ts.snap index 1cd4c192081..51bc3f4b0e5 100644 --- a/packages/ozone/tests/__snapshots__/moderation.test.ts.snap +++ b/packages/ozone/tests/__snapshots__/moderation.test.ts.snap @@ -4,7 +4,7 @@ exports[`moderation reporting creates reports of a record. 1`] = ` Array [ Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 7, + "id": 9, "reasonType": "com.atproto.moderation.defs#reasonSpam", "reportedBy": "user(0)", "subject": Object { @@ -15,7 +15,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 9, + "id": 11, "reason": "defamation", "reasonType": "com.atproto.moderation.defs#reasonOther", "reportedBy": "user(1)", @@ -32,7 +32,7 @@ exports[`moderation reporting creates reports of a repo. 1`] = ` Array [ Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 3, + "id": 5, "reasonType": "com.atproto.moderation.defs#reasonSpam", "reportedBy": "user(0)", "subject": Object { @@ -42,7 +42,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 5, + "id": 7, "reason": "impersonation", "reasonType": "com.atproto.moderation.defs#reasonOther", "reportedBy": "user(2)", diff --git a/packages/ozone/tests/moderation-events.test.ts b/packages/ozone/tests/moderation-events.test.ts index 305bdd11c9e..767ca3f65b7 100644 --- a/packages/ozone/tests/moderation-events.test.ts +++ b/packages/ozone/tests/moderation-events.test.ts @@ -40,7 +40,6 @@ describe('moderation-events', () => { $type: 'com.atproto.admin.defs#repoRef', did: sc.dids.alice, } - console.log({ alicesAccount }) const bobsPost = { $type: 'com.atproto.repo.strongRef', uri: sc.posts[sc.dids.bob][0].ref.uriStr, @@ -204,7 +203,7 @@ describe('moderation-events', () => { const defaultEvents = await getPaginatedEvents() const reversedEvents = await getPaginatedEvents('asc') - expect(allEvents.data.events.length).toEqual(7) + expect(allEvents.data.events.length).toEqual(8) expect(defaultEvents.length).toEqual(allEvents.data.events.length) expect(reversedEvents.length).toEqual(allEvents.data.events.length) // First event in the reversed list is the last item in the default list @@ -359,7 +358,6 @@ describe('moderation-events', () => { { id: 1 }, { headers: network.bsky.adminAuthHeaders('moderator') }, ) - console.log(JSON.stringify(data, null, 2)) expect(forSnapshot(data)).toMatchSnapshot() }) }) diff --git a/packages/ozone/tests/moderation-statuses.test.ts b/packages/ozone/tests/moderation-statuses.test.ts index 14184454e62..38c69a5056a 100644 --- a/packages/ozone/tests/moderation-statuses.test.ts +++ b/packages/ozone/tests/moderation-statuses.test.ts @@ -136,7 +136,7 @@ describe('moderation-statuses', () => { } const list = await getPaginatedStatuses({}) - expect(list[0].id).toEqual(7) + expect(list[0].id).toEqual(11) expect(list[list.length - 1].id).toEqual(1) await emitModerationEvent({ From d7f6538003c8a55df39cd52645e70032af8497a6 Mon Sep 17 00:00:00 2001 From: Foysal Ahamed Date: Tue, 27 Feb 2024 20:22:18 +0100 Subject: [PATCH 3/6] :white_check_mark: Update snapshots on some tests --- .../__snapshots__/get-record.test.ts.snap | 4 +-- .../moderation-events.test.ts.snap | 2 +- .../moderation-statuses.test.ts.snap | 31 ++++++++++++++----- .../ozone/tests/moderation-events.test.ts | 4 ++- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/packages/ozone/tests/__snapshots__/get-record.test.ts.snap b/packages/ozone/tests/__snapshots__/get-record.test.ts.snap index 5c977ea406b..65e1a21bfd7 100644 --- a/packages/ozone/tests/__snapshots__/get-record.test.ts.snap +++ b/packages/ozone/tests/__snapshots__/get-record.test.ts.snap @@ -27,7 +27,7 @@ Object { "moderation": Object { "subjectStatus": Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 3, + "id": 5, "lastReportedAt": "1970-01-01T00:00:00.000Z", "lastReviewedAt": "1970-01-01T00:00:00.000Z", "lastReviewedBy": "did:example:admin", @@ -124,7 +124,7 @@ Object { "moderation": Object { "subjectStatus": Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 3, + "id": 5, "lastReportedAt": "1970-01-01T00:00:00.000Z", "lastReviewedAt": "1970-01-01T00:00:00.000Z", "lastReviewedBy": "did:example:admin", diff --git a/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap b/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap index a8b7f46d7c1..1615b746242 100644 --- a/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap +++ b/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap @@ -48,7 +48,7 @@ Object { "lastReportedAt": "1970-01-01T00:00:00.000Z", "lastReviewedAt": "1970-01-01T00:00:00.000Z", "lastReviewedBy": "user(1)", - "reviewState": "com.atproto.admin.defs#reviewEscalated", + "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { "$type": "com.atproto.admin.defs#repoRef", "did": "user(0)", diff --git a/packages/ozone/tests/__snapshots__/moderation-statuses.test.ts.snap b/packages/ozone/tests/__snapshots__/moderation-statuses.test.ts.snap index b2e0cabee16..bf760973d5f 100644 --- a/packages/ozone/tests/__snapshots__/moderation-statuses.test.ts.snap +++ b/packages/ozone/tests/__snapshots__/moderation-statuses.test.ts.snap @@ -4,7 +4,7 @@ exports[`moderation-statuses query statuses returns statuses filtered by subject Array [ Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 9, + "id": 11, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -23,7 +23,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 7, + "id": 9, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -46,7 +46,7 @@ exports[`moderation-statuses query statuses returns statuses for subjects that r Array [ Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 9, + "id": 11, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -65,7 +65,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 7, + "id": 9, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -83,7 +83,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 5, + "id": 7, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -101,7 +101,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 3, + "id": 5, "lastReportedAt": "1970-01-01T00:00:00.000Z", "reviewState": "com.atproto.admin.defs#reviewOpen", "subject": Object { @@ -118,7 +118,7 @@ Array [ }, Object { "createdAt": "1970-01-01T00:00:00.000Z", - "id": 1, + "id": 3, "reviewState": "com.atproto.admin.defs#reviewOptional", "subject": Object { "$type": "com.atproto.repo.strongRef", @@ -133,5 +133,22 @@ Array [ "takendown": false, "updatedAt": "1970-01-01T00:00:00.000Z", }, + Object { + "createdAt": "1970-01-01T00:00:00.000Z", + "id": 1, + "reviewState": "com.atproto.admin.defs#reviewOptional", + "subject": Object { + "$type": "com.atproto.repo.strongRef", + "cid": "cids(3)", + "uri": "record(3)", + }, + "subjectBlobCids": Array [], + "subjectRepoHandle": "alice.test", + "tags": Array [ + "lang:und", + ], + "takendown": false, + "updatedAt": "1970-01-01T00:00:00.000Z", + }, ] `; diff --git a/packages/ozone/tests/moderation-events.test.ts b/packages/ozone/tests/moderation-events.test.ts index 767ca3f65b7..267db2d0190 100644 --- a/packages/ozone/tests/moderation-events.test.ts +++ b/packages/ozone/tests/moderation-events.test.ts @@ -207,7 +207,9 @@ describe('moderation-events', () => { expect(defaultEvents.length).toEqual(allEvents.data.events.length) expect(reversedEvents.length).toEqual(allEvents.data.events.length) // First event in the reversed list is the last item in the default list - expect(reversedEvents[0].id).toEqual(defaultEvents[6].id) + expect(reversedEvents[0].id).toEqual( + defaultEvents[defaultEvents.length - 1].id, + ) }) it('returns report events matching reportType filters', async () => { From 93ac0e4a3456d8b8ebed75b8241f56758f2767d7 Mon Sep 17 00:00:00 2001 From: Foysal Ahamed Date: Tue, 27 Feb 2024 21:27:34 +0100 Subject: [PATCH 4/6] :white_check_mark: Add test for reviewOptional status mutation --- packages/ozone/src/mod-service/status.ts | 6 ++ .../ozone/tests/moderation-statuses.test.ts | 88 +++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/packages/ozone/src/mod-service/status.ts b/packages/ozone/src/mod-service/status.ts index efbdbe66290..4faac7a94fe 100644 --- a/packages/ozone/src/mod-service/status.ts +++ b/packages/ozone/src/mod-service/status.ts @@ -160,6 +160,12 @@ export const adjustModerationSubjectStatus = async ( subjectStatus.reviewState = REVIEWESCALATED } + if (currentStatus && subjectStatus.reviewState === REVIEWOPTIONAL) { + // reviewOptional is ONLY allowed when there is no current status + // If there is a current status, it should not be allowed to move back to reviewOptional + subjectStatus.reviewState = currentStatus.reviewState + } + // Set these because we don't want to override them if they're already set const defaultData = { comment: null, diff --git a/packages/ozone/tests/moderation-statuses.test.ts b/packages/ozone/tests/moderation-statuses.test.ts index 38c69a5056a..b9c857a3b5d 100644 --- a/packages/ozone/tests/moderation-statuses.test.ts +++ b/packages/ozone/tests/moderation-statuses.test.ts @@ -9,6 +9,10 @@ import { REASONMISLEADING, REASONSPAM, } from '../src/lexicon/types/com/atproto/moderation/defs' +import { + REVIEWOPEN, + REVIEWOPTIONAL, +} from '../src/lexicon/types/com/atproto/admin/defs' describe('moderation-statuses', () => { let network: TestNetwork @@ -160,6 +164,90 @@ describe('moderation-statuses', () => { }) }) + describe('reviewState changes', () => { + it('only sets state to #reviewOptional on first non-impactful event', async () => { + const bobsAccount = { + $type: 'com.atproto.admin.defs#repoRef', + did: sc.dids.bob, + } + const alicesPost = { + $type: 'com.atproto.repo.strongRef', + uri: sc.posts[sc.dids.alice][0].ref.uriStr, + cid: sc.posts[sc.dids.alice][0].ref.cidStr, + } + const getBobsAccountStatus = async () => { + const { data } = await queryModerationStatuses({ + subject: bobsAccount.did, + }) + + return data.subjectStatuses[0] + } + // Since bob's account already had a reviewState, it won't be changed by non-impactful events + const bobsAccountStatusBeforeTag = await getBobsAccountStatus() + + await Promise.all([ + emitModerationEvent({ + subject: bobsAccount, + event: { + $type: 'com.atproto.admin.defs#modEventTag', + add: ['newTag'], + remove: [], + comment: 'X', + }, + createdBy: sc.dids.alice, + }), + emitModerationEvent({ + subject: bobsAccount, + event: { + $type: 'com.atproto.admin.defs#modEventComment', + comment: 'X', + }, + createdBy: sc.dids.alice, + }), + ]) + const bobsAccountStatusAfterTag = await getBobsAccountStatus() + + expect(bobsAccountStatusBeforeTag.reviewState).toEqual( + bobsAccountStatusAfterTag.reviewState, + ) + + // Since alice's post didn't have a reviewState it is set to reviewOptional on first non-impactful event + const getAlicesPostStatus = async () => { + const { data } = await queryModerationStatuses({ + subject: alicesPost.uri, + }) + + return data.subjectStatuses[0] + } + + const alicesPostStatusBeforeTag = await getAlicesPostStatus() + expect(alicesPostStatusBeforeTag).toBeUndefined() + + await emitModerationEvent({ + subject: alicesPost, + event: { + $type: 'com.atproto.admin.defs#modEventComment', + comment: 'X', + }, + createdBy: sc.dids.alice, + }) + const alicesPostStatusAfterTag = await getAlicesPostStatus() + expect(alicesPostStatusAfterTag.reviewState).toEqual(REVIEWOPTIONAL) + + await emitModerationEvent({ + subject: alicesPost, + event: { + $type: 'com.atproto.admin.defs#modEventReport', + reportType: REASONMISLEADING, + comment: 'X', + }, + createdBy: sc.dids.alice, + }) + const alicesPostStatusAfterReport = await getAlicesPostStatus() + expect(alicesPostStatusAfterReport.reviewState).toEqual(REVIEWOPEN) + }) + }) + describe('blobs', () => { it('are tracked on takendown subject', async () => { const post = sc.posts[sc.dids.carol][0] From c5f2c10b9ebb582b816c61727a76842903535046 Mon Sep 17 00:00:00 2001 From: Foysal Ahamed Date: Thu, 29 Feb 2024 01:52:03 +0100 Subject: [PATCH 5/6] :recycle: Rename reviewOptional -> reviewNone --- lexicons/com/atproto/admin/defs.json | 4 ++-- packages/api/src/client/index.ts | 2 +- packages/api/src/client/lexicons.ts | 4 ++-- .../api/src/client/types/com/atproto/admin/defs.ts | 4 ++-- packages/bsky/src/lexicon/index.ts | 2 +- packages/bsky/src/lexicon/lexicons.ts | 4 ++-- .../bsky/src/lexicon/types/com/atproto/admin/defs.ts | 4 ++-- .../ozone/src/db/schema/moderation_subject_status.ts | 4 ++-- packages/ozone/src/lexicon/index.ts | 2 +- packages/ozone/src/lexicon/lexicons.ts | 4 ++-- .../src/lexicon/types/com/atproto/admin/defs.ts | 4 ++-- packages/ozone/src/mod-service/status.ts | 12 ++++++------ packages/ozone/tests/moderation-statuses.test.ts | 8 ++++---- packages/pds/src/lexicon/index.ts | 2 +- packages/pds/src/lexicon/lexicons.ts | 4 ++-- .../pds/src/lexicon/types/com/atproto/admin/defs.ts | 4 ++-- 16 files changed, 34 insertions(+), 34 deletions(-) diff --git a/lexicons/com/atproto/admin/defs.json b/lexicons/com/atproto/admin/defs.json index 56a855f7427..c73700474fe 100644 --- a/lexicons/com/atproto/admin/defs.json +++ b/lexicons/com/atproto/admin/defs.json @@ -453,7 +453,7 @@ "#reviewOpen", "#reviewEscalated", "#reviewClosed", - "#reviewOptional" + "#reviewNone" ] }, "reviewOpen": { @@ -468,7 +468,7 @@ "type": "token", "description": "Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator" }, - "reviewOptional": { + "reviewNone": { "type": "token", "description": "Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it" }, diff --git a/packages/api/src/client/index.ts b/packages/api/src/client/index.ts index 7f55f73390c..205a986e666 100644 --- a/packages/api/src/client/index.ts +++ b/packages/api/src/client/index.ts @@ -319,7 +319,7 @@ export const COM_ATPROTO_ADMIN = { DefsReviewOpen: 'com.atproto.admin.defs#reviewOpen', DefsReviewEscalated: 'com.atproto.admin.defs#reviewEscalated', DefsReviewClosed: 'com.atproto.admin.defs#reviewClosed', - DefsReviewOptional: 'com.atproto.admin.defs#reviewOptional', + DefsReviewNone: 'com.atproto.admin.defs#reviewNone', } export const COM_ATPROTO_MODERATION = { DefsReasonSpam: 'com.atproto.moderation.defs#reasonSpam', diff --git a/packages/api/src/client/lexicons.ts b/packages/api/src/client/lexicons.ts index b56763f9e62..a675a0b0201 100644 --- a/packages/api/src/client/lexicons.ts +++ b/packages/api/src/client/lexicons.ts @@ -747,7 +747,7 @@ export const schemaDict = { 'lex:com.atproto.admin.defs#reviewOpen', 'lex:com.atproto.admin.defs#reviewEscalated', 'lex:com.atproto.admin.defs#reviewClosed', - 'lex:com.atproto.admin.defs#reviewOptional', + 'lex:com.atproto.admin.defs#reviewNone', ], }, reviewOpen: { @@ -765,7 +765,7 @@ export const schemaDict = { description: 'Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator', }, - reviewOptional: { + reviewNone: { type: 'token', description: 'Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it', diff --git a/packages/api/src/client/types/com/atproto/admin/defs.ts b/packages/api/src/client/types/com/atproto/admin/defs.ts index b3dbb45bc23..082282505b6 100644 --- a/packages/api/src/client/types/com/atproto/admin/defs.ts +++ b/packages/api/src/client/types/com/atproto/admin/defs.ts @@ -497,7 +497,7 @@ export type SubjectReviewState = | 'lex:com.atproto.admin.defs#reviewOpen' | 'lex:com.atproto.admin.defs#reviewEscalated' | 'lex:com.atproto.admin.defs#reviewClosed' - | 'lex:com.atproto.admin.defs#reviewOptional' + | 'lex:com.atproto.admin.defs#reviewNone' | (string & {}) /** Moderator review status of a subject: Open. Indicates that the subject needs to be reviewed by a moderator */ @@ -507,7 +507,7 @@ export const REVIEWESCALATED = 'com.atproto.admin.defs#reviewEscalated' /** Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator */ export const REVIEWCLOSED = 'com.atproto.admin.defs#reviewClosed' /** Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it */ -export const REVIEWOPTIONAL = 'com.atproto.admin.defs#reviewOptional' +export const REVIEWNONE = 'com.atproto.admin.defs#reviewNone' /** Take down a subject permanently or temporarily */ export interface ModEventTakedown { diff --git a/packages/bsky/src/lexicon/index.ts b/packages/bsky/src/lexicon/index.ts index d42435e2f7c..4ace1ffbc86 100644 --- a/packages/bsky/src/lexicon/index.ts +++ b/packages/bsky/src/lexicon/index.ts @@ -142,7 +142,7 @@ export const COM_ATPROTO_ADMIN = { DefsReviewOpen: 'com.atproto.admin.defs#reviewOpen', DefsReviewEscalated: 'com.atproto.admin.defs#reviewEscalated', DefsReviewClosed: 'com.atproto.admin.defs#reviewClosed', - DefsReviewOptional: 'com.atproto.admin.defs#reviewOptional', + DefsReviewNone: 'com.atproto.admin.defs#reviewNone', } export const COM_ATPROTO_MODERATION = { DefsReasonSpam: 'com.atproto.moderation.defs#reasonSpam', diff --git a/packages/bsky/src/lexicon/lexicons.ts b/packages/bsky/src/lexicon/lexicons.ts index b56763f9e62..a675a0b0201 100644 --- a/packages/bsky/src/lexicon/lexicons.ts +++ b/packages/bsky/src/lexicon/lexicons.ts @@ -747,7 +747,7 @@ export const schemaDict = { 'lex:com.atproto.admin.defs#reviewOpen', 'lex:com.atproto.admin.defs#reviewEscalated', 'lex:com.atproto.admin.defs#reviewClosed', - 'lex:com.atproto.admin.defs#reviewOptional', + 'lex:com.atproto.admin.defs#reviewNone', ], }, reviewOpen: { @@ -765,7 +765,7 @@ export const schemaDict = { description: 'Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator', }, - reviewOptional: { + reviewNone: { type: 'token', description: 'Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it', diff --git a/packages/bsky/src/lexicon/types/com/atproto/admin/defs.ts b/packages/bsky/src/lexicon/types/com/atproto/admin/defs.ts index 9e6aa7e3269..e18381d7b58 100644 --- a/packages/bsky/src/lexicon/types/com/atproto/admin/defs.ts +++ b/packages/bsky/src/lexicon/types/com/atproto/admin/defs.ts @@ -497,7 +497,7 @@ export type SubjectReviewState = | 'lex:com.atproto.admin.defs#reviewOpen' | 'lex:com.atproto.admin.defs#reviewEscalated' | 'lex:com.atproto.admin.defs#reviewClosed' - | 'lex:com.atproto.admin.defs#reviewOptional' + | 'lex:com.atproto.admin.defs#reviewNone' | (string & {}) /** Moderator review status of a subject: Open. Indicates that the subject needs to be reviewed by a moderator */ @@ -507,7 +507,7 @@ export const REVIEWESCALATED = 'com.atproto.admin.defs#reviewEscalated' /** Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator */ export const REVIEWCLOSED = 'com.atproto.admin.defs#reviewClosed' /** Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it */ -export const REVIEWOPTIONAL = 'com.atproto.admin.defs#reviewOptional' +export const REVIEWNONE = 'com.atproto.admin.defs#reviewNone' /** Take down a subject permanently or temporarily */ export interface ModEventTakedown { diff --git a/packages/ozone/src/db/schema/moderation_subject_status.ts b/packages/ozone/src/db/schema/moderation_subject_status.ts index 4ed2751f217..45dc6df9d89 100644 --- a/packages/ozone/src/db/schema/moderation_subject_status.ts +++ b/packages/ozone/src/db/schema/moderation_subject_status.ts @@ -3,7 +3,7 @@ import { REVIEWCLOSED, REVIEWOPEN, REVIEWESCALATED, - REVIEWOPTIONAL, + REVIEWNONE, } from '../../lexicon/types/com/atproto/admin/defs' export const subjectStatusTableName = 'moderation_subject_status' @@ -18,7 +18,7 @@ export interface ModerationSubjectStatus { | typeof REVIEWCLOSED | typeof REVIEWOPEN | typeof REVIEWESCALATED - | typeof REVIEWOPTIONAL + | typeof REVIEWNONE createdAt: string updatedAt: string lastReviewedBy: string | null diff --git a/packages/ozone/src/lexicon/index.ts b/packages/ozone/src/lexicon/index.ts index d42435e2f7c..4ace1ffbc86 100644 --- a/packages/ozone/src/lexicon/index.ts +++ b/packages/ozone/src/lexicon/index.ts @@ -142,7 +142,7 @@ export const COM_ATPROTO_ADMIN = { DefsReviewOpen: 'com.atproto.admin.defs#reviewOpen', DefsReviewEscalated: 'com.atproto.admin.defs#reviewEscalated', DefsReviewClosed: 'com.atproto.admin.defs#reviewClosed', - DefsReviewOptional: 'com.atproto.admin.defs#reviewOptional', + DefsReviewNone: 'com.atproto.admin.defs#reviewNone', } export const COM_ATPROTO_MODERATION = { DefsReasonSpam: 'com.atproto.moderation.defs#reasonSpam', diff --git a/packages/ozone/src/lexicon/lexicons.ts b/packages/ozone/src/lexicon/lexicons.ts index b56763f9e62..a675a0b0201 100644 --- a/packages/ozone/src/lexicon/lexicons.ts +++ b/packages/ozone/src/lexicon/lexicons.ts @@ -747,7 +747,7 @@ export const schemaDict = { 'lex:com.atproto.admin.defs#reviewOpen', 'lex:com.atproto.admin.defs#reviewEscalated', 'lex:com.atproto.admin.defs#reviewClosed', - 'lex:com.atproto.admin.defs#reviewOptional', + 'lex:com.atproto.admin.defs#reviewNone', ], }, reviewOpen: { @@ -765,7 +765,7 @@ export const schemaDict = { description: 'Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator', }, - reviewOptional: { + reviewNone: { type: 'token', description: 'Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it', diff --git a/packages/ozone/src/lexicon/types/com/atproto/admin/defs.ts b/packages/ozone/src/lexicon/types/com/atproto/admin/defs.ts index 9e6aa7e3269..e18381d7b58 100644 --- a/packages/ozone/src/lexicon/types/com/atproto/admin/defs.ts +++ b/packages/ozone/src/lexicon/types/com/atproto/admin/defs.ts @@ -497,7 +497,7 @@ export type SubjectReviewState = | 'lex:com.atproto.admin.defs#reviewOpen' | 'lex:com.atproto.admin.defs#reviewEscalated' | 'lex:com.atproto.admin.defs#reviewClosed' - | 'lex:com.atproto.admin.defs#reviewOptional' + | 'lex:com.atproto.admin.defs#reviewNone' | (string & {}) /** Moderator review status of a subject: Open. Indicates that the subject needs to be reviewed by a moderator */ @@ -507,7 +507,7 @@ export const REVIEWESCALATED = 'com.atproto.admin.defs#reviewEscalated' /** Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator */ export const REVIEWCLOSED = 'com.atproto.admin.defs#reviewClosed' /** Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it */ -export const REVIEWOPTIONAL = 'com.atproto.admin.defs#reviewOptional' +export const REVIEWNONE = 'com.atproto.admin.defs#reviewNone' /** Take down a subject permanently or temporarily */ export interface ModEventTakedown { diff --git a/packages/ozone/src/mod-service/status.ts b/packages/ozone/src/mod-service/status.ts index 96d8c016d44..38f6253d766 100644 --- a/packages/ozone/src/mod-service/status.ts +++ b/packages/ozone/src/mod-service/status.ts @@ -7,7 +7,7 @@ import { REVIEWOPEN, REVIEWCLOSED, REVIEWESCALATED, - REVIEWOPTIONAL, + REVIEWNONE, } from '../lexicon/types/com/atproto/admin/defs' import { ModerationEventRow, ModerationSubjectStatusRow } from './types' import { HOUR } from '@atproto/common' @@ -29,7 +29,7 @@ const getSubjectStatusForModerationEvent = ({ }): Partial => { const defaultReviewState = currentStatus ? currentStatus.reviewState - : REVIEWOPTIONAL + : REVIEWNONE switch (action) { case 'com.atproto.admin.defs#modEventAcknowledge': @@ -160,9 +160,9 @@ export const adjustModerationSubjectStatus = async ( subjectStatus.reviewState = REVIEWESCALATED } - if (currentStatus && subjectStatus.reviewState === REVIEWOPTIONAL) { - // reviewOptional is ONLY allowed when there is no current status - // If there is a current status, it should not be allowed to move back to reviewOptional + if (currentStatus && subjectStatus.reviewState === REVIEWNONE) { + // reviewNone is ONLY allowed when there is no current status + // If there is a current status, it should not be allowed to move back to reviewNone subjectStatus.reviewState = currentStatus.reviewState } @@ -172,7 +172,7 @@ export const adjustModerationSubjectStatus = async ( // Defaulting reviewState to open for any event may not be the desired behavior. // For instance, if a subject never had any event and we just want to leave a comment to keep an eye on it // that shouldn't mean we want to review the subject - reviewState: REVIEWOPTIONAL, + reviewState: REVIEWNONE, recordCid: subjectCid || null, } const newStatus = { diff --git a/packages/ozone/tests/moderation-statuses.test.ts b/packages/ozone/tests/moderation-statuses.test.ts index 1df3684f1d2..d1f47c880fc 100644 --- a/packages/ozone/tests/moderation-statuses.test.ts +++ b/packages/ozone/tests/moderation-statuses.test.ts @@ -11,7 +11,7 @@ import { } from '../src/lexicon/types/com/atproto/moderation/defs' import { REVIEWOPEN, - REVIEWOPTIONAL, + REVIEWNONE, } from '../src/lexicon/types/com/atproto/admin/defs' describe('moderation-statuses', () => { @@ -165,7 +165,7 @@ describe('moderation-statuses', () => { }) describe('reviewState changes', () => { - it('only sets state to #reviewOptional on first non-impactful event', async () => { + it('only sets state to #reviewNone on first non-impactful event', async () => { const bobsAccount = { $type: 'com.atproto.admin.defs#repoRef', did: sc.dids.bob, @@ -211,7 +211,7 @@ describe('moderation-statuses', () => { bobsAccountStatusAfterTag.reviewState, ) - // Since alice's post didn't have a reviewState it is set to reviewOptional on first non-impactful event + // Since alice's post didn't have a reviewState it is set to reviewNone on first non-impactful event const getAlicesPostStatus = async () => { const { data } = await queryModerationStatuses({ subject: alicesPost.uri, @@ -232,7 +232,7 @@ describe('moderation-statuses', () => { createdBy: sc.dids.alice, }) const alicesPostStatusAfterTag = await getAlicesPostStatus() - expect(alicesPostStatusAfterTag.reviewState).toEqual(REVIEWOPTIONAL) + expect(alicesPostStatusAfterTag.reviewState).toEqual(REVIEWNONE) await emitModerationEvent({ subject: alicesPost, diff --git a/packages/pds/src/lexicon/index.ts b/packages/pds/src/lexicon/index.ts index d42435e2f7c..4ace1ffbc86 100644 --- a/packages/pds/src/lexicon/index.ts +++ b/packages/pds/src/lexicon/index.ts @@ -142,7 +142,7 @@ export const COM_ATPROTO_ADMIN = { DefsReviewOpen: 'com.atproto.admin.defs#reviewOpen', DefsReviewEscalated: 'com.atproto.admin.defs#reviewEscalated', DefsReviewClosed: 'com.atproto.admin.defs#reviewClosed', - DefsReviewOptional: 'com.atproto.admin.defs#reviewOptional', + DefsReviewNone: 'com.atproto.admin.defs#reviewNone', } export const COM_ATPROTO_MODERATION = { DefsReasonSpam: 'com.atproto.moderation.defs#reasonSpam', diff --git a/packages/pds/src/lexicon/lexicons.ts b/packages/pds/src/lexicon/lexicons.ts index b56763f9e62..a675a0b0201 100644 --- a/packages/pds/src/lexicon/lexicons.ts +++ b/packages/pds/src/lexicon/lexicons.ts @@ -747,7 +747,7 @@ export const schemaDict = { 'lex:com.atproto.admin.defs#reviewOpen', 'lex:com.atproto.admin.defs#reviewEscalated', 'lex:com.atproto.admin.defs#reviewClosed', - 'lex:com.atproto.admin.defs#reviewOptional', + 'lex:com.atproto.admin.defs#reviewNone', ], }, reviewOpen: { @@ -765,7 +765,7 @@ export const schemaDict = { description: 'Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator', }, - reviewOptional: { + reviewNone: { type: 'token', description: 'Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it', diff --git a/packages/pds/src/lexicon/types/com/atproto/admin/defs.ts b/packages/pds/src/lexicon/types/com/atproto/admin/defs.ts index 9e6aa7e3269..e18381d7b58 100644 --- a/packages/pds/src/lexicon/types/com/atproto/admin/defs.ts +++ b/packages/pds/src/lexicon/types/com/atproto/admin/defs.ts @@ -497,7 +497,7 @@ export type SubjectReviewState = | 'lex:com.atproto.admin.defs#reviewOpen' | 'lex:com.atproto.admin.defs#reviewEscalated' | 'lex:com.atproto.admin.defs#reviewClosed' - | 'lex:com.atproto.admin.defs#reviewOptional' + | 'lex:com.atproto.admin.defs#reviewNone' | (string & {}) /** Moderator review status of a subject: Open. Indicates that the subject needs to be reviewed by a moderator */ @@ -507,7 +507,7 @@ export const REVIEWESCALATED = 'com.atproto.admin.defs#reviewEscalated' /** Moderator review status of a subject: Closed. Indicates that the subject was already reviewed and resolved by a moderator */ export const REVIEWCLOSED = 'com.atproto.admin.defs#reviewClosed' /** Moderator review status of a subject: Unnecessary. Indicates that the subject does not need a review at the moment but there is probably some moderation related metadata available for it */ -export const REVIEWOPTIONAL = 'com.atproto.admin.defs#reviewOptional' +export const REVIEWNONE = 'com.atproto.admin.defs#reviewNone' /** Take down a subject permanently or temporarily */ export interface ModEventTakedown { From 5c0e20a22e63663fcb9472de4cc2ee1ce9e38d22 Mon Sep 17 00:00:00 2001 From: Foysal Ahamed Date: Mon, 4 Mar 2024 23:07:27 +0000 Subject: [PATCH 6/6] :white_check_mark: Use FOR UPDATE to respect db transactions --- packages/ozone/src/mod-service/status.ts | 2 ++ .../ozone/tests/__snapshots__/moderation-events.test.ts.snap | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/ozone/src/mod-service/status.ts b/packages/ozone/src/mod-service/status.ts index 38f6253d766..81cebe2dddb 100644 --- a/packages/ozone/src/mod-service/status.ts +++ b/packages/ozone/src/mod-service/status.ts @@ -135,6 +135,8 @@ export const adjustModerationSubjectStatus = async ( .selectFrom('moderation_subject_status') .where('did', '=', identifier.did) .where('recordPath', '=', identifier.recordPath) + // Make sure we respect other updates that may be happening at the same time + .forUpdate() .selectAll() .executeTakeFirst() diff --git a/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap b/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap index 216eb970ca5..fcb73413622 100644 --- a/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap +++ b/packages/ozone/tests/__snapshots__/moderation-events.test.ts.snap @@ -22,7 +22,7 @@ Object { "lastReportedAt": "1970-01-01T00:00:00.000Z", "lastReviewedAt": "1970-01-01T00:00:00.000Z", "lastReviewedBy": "user(1)", - "reviewState": "com.atproto.admin.defs#reviewOpen", + "reviewState": "com.atproto.admin.defs#reviewEscalated", "subject": Object { "$type": "com.atproto.admin.defs#repoRef", "did": "user(0)",