Label reversal fix (bluesky-social#809)

* Reproduce issue with empty negateLabelVals * normalize str arrays * Reproduce issue with reversing labels on repo * Use multiple labels for record labeling test * join with space --------- Co-authored-by: Devin Ivy <[email protected]>
Jazzkid0 · Apr 13, 2023 · efb1cac · efb1cac
1 parent aa46ad1
commit efb1cac
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 10 deletions.
diff --git a/packages/pds/src/api/com/atproto/admin/reverseModerationAction.ts b/packages/pds/src/api/com/atproto/admin/reverseModerationAction.ts
@@ -55,8 +55,15 @@ export default function (server: Server, ctx: AppContext) {
         }
 
         // invert creates & negates
-        const negate = result.createLabelVals?.split(' ')
-        const create = result.negateLabelVals?.split(' ')
+        const { createLabelVals, negateLabelVals } = result
+        const negate =
+          createLabelVals && createLabelVals.length > 0
+            ? createLabelVals.split(' ')
+            : undefined
+        const create =
+          negateLabelVals && negateLabelVals.length > 0
+            ? negateLabelVals.split(' ')
+            : undefined
         await labelTxn.formatAndCreate(
           ctx.cfg.labelerDid,
           result.subjectUri ?? result.subjectDid,

diff --git a/packages/pds/src/services/moderation/index.ts b/packages/pds/src/services/moderation/index.ts
@@ -192,8 +192,14 @@ export class ModerationService {
       subjectBlobCids,
       createdAt = new Date(),
     } = info
-    const createLabelVals = info.createLabelVals?.join()
-    const negateLabelVals = info.negateLabelVals?.join()
+    const createLabelVals =
+      info.createLabelVals && info.createLabelVals.length > 0
+        ? info.createLabelVals.join(' ')
+        : undefined
+    const negateLabelVals =
+      info.negateLabelVals && info.negateLabelVals.length > 0
+        ? info.negateLabelVals.join(' ')
+        : undefined
 
     // Resolve subject info
     let subjectInfo: SubjectInfo

diff --git a/packages/pds/src/services/moderation/views.ts b/packages/pds/src/services/moderation/views.ts
@@ -319,8 +319,14 @@ export class ModerationViews {
       reason: res.reason,
       createdAt: res.createdAt,
       createdBy: res.createdBy,
-      createLabelVals: res.createLabelVals?.split(' '),
-      negateLabelVals: res.negateLabelVals?.split(' '),
+      createLabelVals:
+        res.createLabelVals && res.createLabelVals.length > 0
+          ? res.createLabelVals.split(' ')
+          : undefined,
+      negateLabelVals:
+        res.negateLabelVals && res.negateLabelVals.length > 0
+          ? res.negateLabelVals.split(' ')
+          : undefined,
       reversal:
         res.reversedAt !== null &&
         res.reversedBy !== null &&

diff --git a/packages/pds/tests/moderation.test.ts b/packages/pds/tests/moderation.test.ts
@@ -751,17 +751,21 @@ describe('moderation', () => {
       )
     })
 
-    it('creates a non-existing label and reverses.', async () => {
+    it('creates non-existing labels and reverses.', async () => {
       const post = sc.posts[sc.dids.bob][0].ref
       const action = await actionWithLabels({
-        createLabelVals: ['puppies'],
+        createLabelVals: ['puppies', 'doggies'],
+        negateLabelVals: [],
         subject: {
           $type: 'com.atproto.repo.strongRef',
           uri: post.uriStr,
           cid: post.cidStr,
         },
       })
-      await expect(getRecordLabels(post.uriStr)).resolves.toEqual(['puppies'])
+      await expect(getRecordLabels(post.uriStr)).resolves.toEqual([
+        'puppies',
+        'doggies',
+      ])
       await reverse(action.id)
       await expect(getRecordLabels(post.uriStr)).resolves.toEqual([])
     })
@@ -789,7 +793,24 @@ describe('moderation', () => {
       await expect(getRecordLabels(post.uriStr)).resolves.toEqual([])
     })
 
-    it('creates and negates labels on a repo.', async () => {
+    it('creates labels on a repo and reverses.', async () => {
+      const action = await actionWithLabels({
+        createLabelVals: ['puppies', 'doggies'],
+        negateLabelVals: [],
+        subject: {
+          $type: 'com.atproto.admin.defs#repoRef',
+          did: sc.dids.bob,
+        },
+      })
+      await expect(getRepoLabels(sc.dids.bob)).resolves.toEqual([
+        'puppies',
+        'doggies',
+      ])
+      await reverse(action.id)
+      await expect(getRepoLabels(sc.dids.bob)).resolves.toEqual([])
+    })
+
+    it('creates and negates labels on a repo and reverses.', async () => {
       const { ctx } = server
       const labelingService = ctx.services.appView.label(ctx.db)
       await labelingService.formatAndCreate(