From e1d77c88414c19cdc80f2359a1716a999bfd0365 Mon Sep 17 00:00:00 2001 From: mark Date: Fri, 11 Sep 2026 18:49:51 +0000 Subject: [PATCH 1/2] Move pending jobs to default queue instead of deleting them When a queue is deleted, obliterate() previously wiped all its BullMQ jobs regardless of status. Pending jobs (waiting/delayed/active, both default and appeals) are now copied into the org's default queue before the source queue is obliterated, and the delete confirmation modal warns the user with the pending job count. Closes #1113. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Rq5yzyBnSizpVMtWrHkBNb --- CHANGELOG.md | 1 + .../mrt/ManualReviewQueuesDashboard.tsx | 23 +++- .../modules/QueueOperations.test.ts | 66 ++++++++++ .../modules/QueueOperations.ts | 117 ++++++++++++++++++ 4 files changed, 205 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b315c3b6..4f22e1d8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ For more information about each release including git tags and artifacts, see [R - Scylla is now optional via `ITEM_INVESTIGATION_AND_STRIKES_ENABLED` ([#918](https://github.com/roostorg/coop/pull/918) by [@sunilatlas](https://github.com/sunilatlas)) - Settings "Other" tab renamed to "Partial Items" and its settings relocated ([#965](https://github.com/roostorg/coop/pull/965) by [@golden-fox07](https://github.com/golden-fox07)) - Queue deletion is refused while routing rules still reference the queue ([#808](https://github.com/roostorg/coop/pull/808) by [@reitblatt](https://github.com/reitblatt)) +- Deleting a queue now moves its pending jobs to the org's default queue instead of deleting them, and the delete confirmation warns about the pending job count ([#1174](https://github.com/roostorg/coop/pull/1174) by [@reitblatt](https://github.com/reitblatt), closes [#1113](https://github.com/roostorg/coop/issues/1113)) - Long text fields in the review console collapse behind a "Read more" control ([#903](https://github.com/roostorg/coop/pull/903) by [@taobojlen](https://github.com/taobojlen)) - Production Node images moved from Debian 11 (bullseye) to Debian 12 (bookworm) ([#1138](https://github.com/roostorg/coop/pull/1138) by [@juanmrad](https://github.com/juanmrad)) diff --git a/client/src/webpages/dashboard/mrt/ManualReviewQueuesDashboard.tsx b/client/src/webpages/dashboard/mrt/ManualReviewQueuesDashboard.tsx index 7e0940f67..583925dbd 100644 --- a/client/src/webpages/dashboard/mrt/ManualReviewQueuesDashboard.tsx +++ b/client/src/webpages/dashboard/mrt/ManualReviewQueuesDashboard.tsx @@ -443,6 +443,9 @@ export default function ManualReviewQueuesDashboard() { ); + const deleteTargetPendingJobCount = + queues?.find((it) => it.id === modalInfo?.id)?.pendingJobCount ?? 0; + const deleteModal = ( - Are you sure you want to delete this queue? This will delete all jobs - inside of this queue as well. You can't undo this action. +
+

+ Are you sure you want to delete this queue? You can't undo this + action. +

+ {deleteTargetPendingJobCount > 0 ? ( +

+ This queue has{' '} + + {deleteTargetPendingJobCount.toLocaleString('en')} pending job + {deleteTargetPendingJobCount === 1 ? '' : 's'} + + . Rather than being deleted,{' '} + {deleteTargetPendingJobCount === 1 ? 'it' : 'they'} will be moved to + your default queue. +

+ ) : null} +
); diff --git a/server/services/manualReviewToolService/modules/QueueOperations.test.ts b/server/services/manualReviewToolService/modules/QueueOperations.test.ts index b0422f29c..a1174aa4b 100644 --- a/server/services/manualReviewToolService/modules/QueueOperations.test.ts +++ b/server/services/manualReviewToolService/modules/QueueOperations.test.ts @@ -497,4 +497,70 @@ describe('QueueOperations', () => { 'block-appeals-rule', ), ); + + // Issue #1113: deleting a queue used to obliterate its pending jobs + // without warning. It now moves them into the org's default queue first. + testWithQueueAndActions()( + 'deleteManualReviewQueue moves pending jobs into the default queue', + async ({ org, user, queue, mrtService }) => { + const secondQueue = await mrtService.createManualReviewQueue({ + name: `delete-test-queue-${uid()}`, + description: null, + userIds: [user.id], + hiddenActionIds: [], + isAppealsQueue: false, + invokedBy: { + userId: user.id, + permissions: [UserPermission.EDIT_MRT_QUEUES], + orgId: org.id, + }, + }); + + const firstJob = await mrtService['queueOps']['addJob']({ + orgId: org.id, + queueId: secondQueue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + jobPayload: makeDummyMrtJobPayload(), + }); + const secondJob = await mrtService['queueOps']['addJob']({ + orgId: org.id, + queueId: secondQueue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + jobPayload: makeDummyMrtJobPayload(), + }); + + const result = await mrtService.deleteManualReviewQueue( + org.id, + secondQueue.id, + ); + expect(result).toEqual(true); + + const defaultQueuePendingCount = await mrtService.getPendingJobCount({ + orgId: org.id, + queueId: queue.id, + }); + expect(defaultQueuePendingCount).toEqual(2); + + const movedItemIds = ( + await mrtService.getAllJobsForQueue({ + orgId: org.id, + queueId: queue.id, + }) + ).map((job) => job.payload.item.itemId); + expect(movedItemIds).toEqual( + expect.arrayContaining([ + firstJob.payload.item.itemId, + secondJob.payload.item.itemId, + ]), + ); + + // The deleted queue itself is gone -- both the DB row and the Bull queue. + await expect( + mrtService.getAllJobsForQueue({ + orgId: org.id, + queueId: secondQueue.id, + }), + ).rejects.toMatchObject({ name: 'QueueDoesNotExistError' }); + }, + ); }); diff --git a/server/services/manualReviewToolService/modules/QueueOperations.ts b/server/services/manualReviewToolService/modules/QueueOperations.ts index bc515f5bd..151b68ff2 100644 --- a/server/services/manualReviewToolService/modules/QueueOperations.ts +++ b/server/services/manualReviewToolService/modules/QueueOperations.ts @@ -446,6 +446,16 @@ export default class QueueOperations { } const queue = await this.getOrCreateBullQueue({ orgId, queueId }); + // Captured before the DB row is deleted below (issue #1113): any jobs + // still pending in this queue get moved to the org's default queue + // instead of being silently wiped by `obliterate`. We need + // `isAppealsQueue` to know which default queue and job shape to use. + const queueBeingDeleted = + await this.getQueueForOrgAndDangerouslyBypassPermissioning({ + orgId, + queueId, + }); + let numDeletedRows: bigint; try { numDeletedRows = await this.transactionWithRetry(async (transaction) => { @@ -506,6 +516,31 @@ export default class QueueOperations { } if (numDeletedRows === 1n) { + if (queueBeingDeleted !== undefined) { + try { + const destinationQueueId = queueBeingDeleted.isAppealsQueue + ? await this.getDefaultAppealsQueueIdForOrg(orgId) + : defaultQueueId; + // Moving into the queue we just deleted the row for would be a + // no-op at best; this only happens if `queueBeingDeleted` was + // itself a default queue, which the guard above only rules out + // for the non-appeals case. + if (destinationQueueId !== queueId) { + await this.#moveAllJobsToQueue({ + orgId, + sourceQueueId: queueId, + destinationQueueId, + isAppealsQueue: queueBeingDeleted.isAppealsQueue, + }); + } + } catch (e) { + // Best-effort: if migrating pending jobs fails partway through, + // fall through to obliterate below rather than leaving the DB row + // deleted but the Bull queue still around. + this.tracer.logActiveSpanFailedIfAny(e); + } + } + try { await queue.obliterate({ force: true }); } catch (e) { @@ -523,6 +558,88 @@ export default class QueueOperations { return numDeletedRows === 1n; } + /** + * Copies every job currently in `sourceQueueId` (waiting, delayed, or + * active) into `destinationQueueId`, best-effort per job. Used by + * {@link deleteManualReviewQueue} so pending work isn't silently lost when + * a queue is deleted (issue #1113). Does not remove jobs from the source + * queue -- the caller obliterates it separately once this returns. + */ + async #moveAllJobsToQueue(opts: { + orgId: string; + sourceQueueId: string; + destinationQueueId: string; + isAppealsQueue: boolean; + }): Promise<{ moved: number; failed: number }> { + const { orgId, sourceQueueId, destinationQueueId, isAppealsQueue } = opts; + const concurrencyLimit = pLimit(10); + const PAGE_SIZE = 500; + + const moveOne = async ( + job: Job | Job, + ): Promise<'moved' | 'failed'> => { + try { + if (isAppealsQueue) { + const { data } = job as Job; + await this.addAppealJob({ + orgId, + queueId: destinationQueueId, + reenqueuedFrom: { jobId: data.id }, + jobPayload: { + createdAt: data.createdAt, + policyIds: data.policyIds, + payload: data.payload, + }, + enqueueSourceInfo: { kind: 'APPEAL' }, + }); + } else { + const { data } = await this.legacyJobToJob( + job as Job, + orgId, + ); + await this.addJob({ + orgId, + queueId: destinationQueueId, + reenqueuedFrom: { jobId: data.id }, + jobPayload: { + createdAt: data.createdAt, + policyIds: data.policyIds, + payload: data.payload, + }, + enqueueSourceInfo: { kind: 'MRT_JOB' }, + }); + } + return 'moved'; + } catch (e) { + this.tracer.logActiveSpanFailedIfAny(e); + return 'failed'; + } + }; + + const sourceQueue = isAppealsQueue + ? await this.getOrCreateBullAppealQueue({ orgId, queueId: sourceQueueId }) + : await this.getOrCreateBullQueue({ orgId, queueId: sourceQueueId }); + + let moved = 0; + let failed = 0; + for (let start = 0; ; start += PAGE_SIZE) { + const page = await sourceQueue.getJobs( + undefined, + start, + start + PAGE_SIZE - 1, + ); + if (page.length === 0) break; + const results = await Promise.all( + page.map(async (job) => concurrencyLimit(async () => moveOne(job))), + ); + for (const result of results) { + if (result === 'moved') moved++; + else failed++; + } + } + return { moved, failed }; + } + async deleteManualReviewQueueForTestsDO_NOT_USE( orgId: string, queueId: string, From 9ff70a53d836e4358244bad1148c4aedf8936c04 Mon Sep 17 00:00:00 2001 From: mark Date: Fri, 11 Sep 2026 18:51:44 +0000 Subject: [PATCH 2/2] Fix CHANGELOG PR number Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Rq5yzyBnSizpVMtWrHkBNb --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f22e1d8a..c1e861490 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ For more information about each release including git tags and artifacts, see [R - Scylla is now optional via `ITEM_INVESTIGATION_AND_STRIKES_ENABLED` ([#918](https://github.com/roostorg/coop/pull/918) by [@sunilatlas](https://github.com/sunilatlas)) - Settings "Other" tab renamed to "Partial Items" and its settings relocated ([#965](https://github.com/roostorg/coop/pull/965) by [@golden-fox07](https://github.com/golden-fox07)) - Queue deletion is refused while routing rules still reference the queue ([#808](https://github.com/roostorg/coop/pull/808) by [@reitblatt](https://github.com/reitblatt)) -- Deleting a queue now moves its pending jobs to the org's default queue instead of deleting them, and the delete confirmation warns about the pending job count ([#1174](https://github.com/roostorg/coop/pull/1174) by [@reitblatt](https://github.com/reitblatt), closes [#1113](https://github.com/roostorg/coop/issues/1113)) +- Deleting a queue now moves its pending jobs to the org's default queue instead of deleting them, and the delete confirmation warns about the pending job count ([#1175](https://github.com/roostorg/coop/pull/1175) by [@reitblatt](https://github.com/reitblatt), closes [#1113](https://github.com/roostorg/coop/issues/1113)) - Long text fields in the review console collapse behind a "Read more" control ([#903](https://github.com/roostorg/coop/pull/903) by [@taobojlen](https://github.com/taobojlen)) - Production Node images moved from Debian 11 (bullseye) to Debian 12 (bookworm) ([#1138](https://github.com/roostorg/coop/pull/1138) by [@juanmrad](https://github.com/juanmrad))