Skip to content

[404]Add "Invalidate reports from a reporter" action for MRT#644

Open
juanmrad wants to merge 4 commits into
mainfrom
invalidate-reports-from-user-at-MRT
Open

[404]Add "Invalidate reports from a reporter" action for MRT#644
juanmrad wants to merge 4 commits into
mainfrom
invalidate-reports-from-user-at-MRT

Conversation

@juanmrad
Copy link
Copy Markdown
Member

@juanmrad juanmrad commented May 29, 2026

Fixes #404

Context & Requests for Reviewers

Lets moderators with EDIT_MRT_QUEUES ( May need follow up to create a specific permission for this ) invalidate reports from a bad-faith reporter (#404), scoped to the current job or org-wide. Removing a reporter's entries strips them from the job's report history. If that empties a job that was enqueued purely from a user report, the job is removed and the moderator advances to the next item.

Screenshot 2026-05-27 at 9 44 38 PM Screenshot 2026-05-27 at 9 44 36 PM

Summary by CodeRabbit

  • New Features

    • Adds an "Invalidate reports" moderator action (modal) to remove pending reports for a reporter, scoped to the current job or org-wide; trims reason input and shows clear success/warning messages.
  • Bug Fixes

    • Safer queue/job handling to avoid mismatches, improve deletion behavior, and prevent missed or locked-job regressions.
  • Documentation

    • User guide updated with invalidation workflow and permission requirements.
  • Tests

    • Extensive tests covering UI, mutation behavior, sweep logic, and edge-case concurrency.

Copilot AI review requested due to automatic review settings May 29, 2026 15:58
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1e0aafd4-1201-4a66-b9f0-1625c6a19870

📥 Commits

Reviewing files that changed from the base of the PR and between c1a8aad and 30a31ed.

⛔ Files ignored due to path filters (2)
  • client/src/graphql/generated.ts is excluded by !**/generated.ts
  • server/graphql/generated.ts is excluded by !**/generated.ts
📒 Files selected for processing (6)
  • client/src/webpages/dashboard/mrt/manual_review_job/ManualReviewJobReview.tsx
  • server/graphql/modules/manualReviewTool.ts
  • server/services/manualReviewToolService/manualReviewToolService.ts
  • server/services/manualReviewToolService/modules/QueueOperations.ts
  • server/services/manualReviewToolService/modules/ReporterInvalidation.test.ts
  • server/services/manualReviewToolService/modules/ReporterInvalidation.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • server/graphql/modules/manualReviewTool.ts
  • server/services/manualReviewToolService/manualReviewToolService.ts
  • server/services/manualReviewToolService/modules/ReporterInvalidation.test.ts
  • server/services/manualReviewToolService/modules/QueueOperations.ts
  • client/src/webpages/dashboard/mrt/manual_review_job/ManualReviewJobReview.tsx

📝 Walkthrough

Walkthrough

Adds end-to-end reporter-scoped invalidation: UI trigger/modal, GraphQL mutation, service sweep and payload-scrub logic, safe queue operations for removal, DB migration, tests, and docs.

Changes

Report Invalidation Feature

Layer / File(s) Summary
InvalidateReportsButton component & tests
client/src/webpages/dashboard/mrt/manual_review_job/InvalidateReportsButton.tsx, client/src/webpages/dashboard/mrt/manual_review_job/InvalidateReportsButton.test.tsx
New link-style modal UI to invalidate a reporter's pending reports (job-scoped or org-wide). Sends trimmed reason, conditionally includes jobId, awaits optional onInvalidated, shows success/warning messages, and is covered by tests for UI states, mutation variables, callback ordering, and trimming.
Report components integration & tests
client/src/webpages/dashboard/mrt/manual_review_job/ReportInfoComponent.tsx, client/src/webpages/dashboard/mrt/manual_review_job/MergedReportsComponent.tsx, client/src/webpages/dashboard/mrt/manual_review_job/MergedReportsComponent.test.tsx
Wires viewer permissions and onInvalidated into report views. Renders InvalidateReportsButton per unique reporter when EditMrtQueues permission allows; tests verify presence/absence and per-reporter rendering.
ManualReviewJobReview page changes
client/src/webpages/dashboard/mrt/manual_review_job/ManualReviewJobReview.tsx
Fetches me.permissions, computes canInvalidateReports, and implements post-invalidation refetch/advance logic with redirect guards to avoid bounce when a job is removed.
GraphQL mutation schema & resolver
server/graphql/modules/manualReviewTool.ts
Adds InvalidateReportsFromReporterInput (reporter, optional jobId, optional reason) and InvalidateReportsFromReporterSuccessResponse; exposes Mutation.invalidateReportsFromReporter resolver enforcing EDIT_MRT_QUEUES and delegating to service with invokedBy.
ManualReviewToolService wiring
server/services/manualReviewToolService/manualReviewToolService.ts
Instantiates ReporterInvalidation, adds invalidateReportsFromReporter(input) with permission and org-id checks, and ensures job_creations inserts ignore duplicate id via onConflict(...).doNothing() in enqueue paths.
Queue operation helpers
server/services/manualReviewToolService/modules/QueueOperations.ts
Adds iteratePendingJobsForQueue, findPendingJobByJobId, removeJobByJobIdUnsafe, removeJobAllowingInvokerLock, and a guarded updateJobForQueue to handle race conditions and lock-aware deletions.
ReporterInvalidation service & tests
server/services/manualReviewToolService/modules/ReporterInvalidation.ts, server/services/manualReviewToolService/modules/ReporterInvalidation.test.ts
Implements org-wide or job-scoped sweep with per-process concurrency guard, scrubPayloadForReporter helper, conditional deletion vs update, aggregation of counters, and extensive unit/integration tests covering permissions, locks, pagination, scoping, and edge cases.
Database migration
db/src/scripts/api-server-pg/2026.05.27T21.33.27.widen_mrt_job_id_columns.sql
Widen id/item_id/item_type_id/job_id columns to text and recreate dependent view to permit type changes.
Documentation & minor formatting
docs/user/reports.md, server/routes/items/submitItems.ts
Adds moderator-facing docs for invalidate-reports (permissions, scope, behavior, audit span) and a small whitespace tweak in submitItems.

Sequence Diagram(s)

sequenceDiagram
  participant Moderator as Moderator(UI)
  participant InvalidateButton as InvalidateReportsButton
  participant GraphQL as GraphQL:invalidateReportsFromReporter
  participant Service as ManualReviewToolService
  participant QueueOps as ReporterInvalidation/QueueOperations
  participant Callback as onInvalidated

  Moderator->>InvalidateButton: open modal / confirm
  InvalidateButton->>GraphQL: mutate(input: reporter, reason?, jobId?)
  GraphQL->>Service: invalidateReportsFromReporter(invokedBy, input)
  Service->>QueueOps: sweep queues / scrub or delete/update jobs
  QueueOps-->>Service: aggregated counters (reportsRemoved, jobsDeleted, truncated)
  Service-->>GraphQL: return result
  GraphQL-->>InvalidateButton: mutation resolved
  InvalidateButton->>Callback: await onInvalidated()
  Callback-->>InvalidateButton: resolves
  InvalidateButton-->>Moderator: display success/warning, close modal
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • roostorg/coop#475: Modifies merged-report row construction in MergedReportsComponent; code-level related changes to per-reporter rendering.

Suggested labels

reliability, documentation

Suggested reviewers

  • cassidyjames
  • julietshen
  • vinaysrao1
  • dom-notion
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description includes context, issue reference, and visual examples, but lacks explicit test documentation and rollout considerations required by the template. Add a 'Tests' section documenting how the changes were tested, and optionally include a 'Rollout Plan' section if there are deployment considerations.
Docstring Coverage ⚠️ Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check ❓ Inconclusive Most changes are scoped to issue #404; however, the SQL migration to widen MRT job ID columns and formatting adjustments in submitItems.ts appear tangential to the core invalidation feature. Clarify whether the SQL column widening and submitItems.ts formatting are prerequisites for this feature or separate fixes that should be in a different PR.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main feature: adding an 'Invalidate reports from a reporter' action for MRT, with issue reference #404.
Linked Issues check ✅ Passed The PR fully implements issue #404 requirements: provides an action to invalidate reports from a specific user, enables moderators to remove mass-flagged reports, and allows report removal to focus on valid reports.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch invalidate-reports-from-user-at-MRT

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an MRT action to invalidate reports from a selected reporter, scoped either to the current job or to all pending jobs in the org.

Changes:

  • Adds server-side reporter invalidation logic, queue helpers, GraphQL mutation, and tests.
  • Adds client UI controls and post-invalidation navigation/refetch behavior.
  • Updates docs, generated GraphQL types, and MRT job-id DB column sizing.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
server/services/manualReviewToolService/modules/ReporterInvalidation.ts Implements report scrubbing/deletion flow.
server/services/manualReviewToolService/modules/ReporterInvalidation.test.ts Adds unit/integration coverage for invalidation behavior.
server/services/manualReviewToolService/modules/QueueOperations.ts Adds pending-job iteration, job lookup, and removal helpers.
server/services/manualReviewToolService/manualReviewToolService.ts Wires reporter invalidation into MRT service and adjusts job creation logging conflicts.
server/graphql/modules/manualReviewTool.ts Adds GraphQL mutation schema and resolver.
server/graphql/generated.ts Updates generated server GraphQL types.
client/src/webpages/dashboard/mrt/manual_review_job/InvalidateReportsButton.tsx Adds confirmation modal and mutation trigger.
client/src/webpages/dashboard/mrt/manual_review_job/InvalidateReportsButton.test.tsx Tests invalidation button behavior.
client/src/webpages/dashboard/mrt/manual_review_job/ReportInfoComponent.tsx Adds primary reporter invalidation action.
client/src/webpages/dashboard/mrt/manual_review_job/MergedReportsComponent.tsx Adds invalidation actions to merged report rows.
client/src/webpages/dashboard/mrt/manual_review_job/MergedReportsComponent.test.tsx Tests merged report invalidation buttons.
client/src/webpages/dashboard/mrt/manual_review_job/ManualReviewJobReview.tsx Handles permissions, refetching, and advancing after invalidation.
client/src/graphql/generated.ts Updates generated client GraphQL types/hooks.
docs/user/reports.md Documents the new invalidation workflow.
db/src/scripts/api-server-pg/2026.05.27T21.33.27.widen_mrt_job_id_columns.sql Widens MRT job/item id columns and recreates the flattened view.

Comment thread server/services/manualReviewToolService/modules/ReporterInvalidation.ts Outdated
Comment thread server/graphql/modules/manualReviewTool.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
docs/user/reports.md (1)

26-26: ⚡ Quick win

Consider breaking this sentence to improve readability.

The sentence contains nested parentheticals and multiple conditions, making it harder to parse. Additionally, the phrase "enqueued purely from a user report" could be clarified—readers might wonder if this distinguishes user reports from automated detection, when it actually means the job had no other reporters or report sources.

✍️ Suggested rewrite for clarity
-By default the action is scoped to the report you're viewing: it strips this reporter's entries from the current job's report history (and removes the job if no other reporters were left and the job was enqueued purely from a user report). Tick "Apply across the whole organization" in the confirmation modal to instead sweep every pending job in the org. Decided/closed jobs are not modified.
+By default the action is scoped to the current job: it strips this reporter's entries from the job's report history. If this reporter was the only source for the job, the job is removed from the queue entirely. To instead sweep every pending job in your organization, enable the checkbox in the confirmation modal. Decided or closed jobs are never modified.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/user/reports.md` at line 26, The long sentence is hard to parse; split
it into shorter sentences and remove nested parentheticals: explain first that
the default action only affects the current report by removing this reporter's
entries from the job's report history (and, if that leaves the job with no other
reporters or report sources, remove the job), then state separately that ticking
"Apply across the whole organization" in the confirmation modal instead sweeps
every pending job in the org; also replace "enqueued purely from a user report"
with an explicit phrase such as "the job has no other reporters or other report
sources (for example, automated detectors)" and make "Decided/closed jobs are
not modified" its own sentence for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/services/manualReviewToolService/manualReviewToolService.ts`:
- Around line 1183-1195: The invalidateReportsFromReporter method currently only
checks UserPermission.EDIT_MRT_QUEUES and trusts input.orgId, allowing cross-org
mutation; before calling reporterInvalidation.invalidateReportsFromReporter,
override/bind input.orgId to input.invokedBy.orgId (i.e., set input.orgId =
input.invokedBy.orgId) so the service enforces the invokedBy caller's org at the
service boundary and prevents in-process callers from operating on a different
org.

In `@server/services/manualReviewToolService/modules/ReporterInvalidation.ts`:
- Around line 76-90: The span created in tracer.addActiveSpan for operation
'invalidateReportsFromReporter' is missing the audit reason; include the
incoming input.reason (or a sanitized/encoded form) in the span attributes and
any audit sink used by this service so the "reason (optional, logged for audit)"
contract is satisfied. Specifically, update the attributes object in the call to
tracer.addActiveSpan (inside invalidateReportsFromReporter) to add a key like
'reporterInvalidation.reason' whose value is either the raw input.reason or a
jsonStringify/sanitized version, and ensure the same reason is propagated to any
audit logging function used by this module so the reason is recorded for audits.
- Around line 305-330: The DEFAULT payload branch updates reportHistory and
reportedForReasons but leaves legacy singular fields (reportedForReason,
reporterIdentifier) stale; when computing reportedForReasons and reportHistory
(using payload, filteredHistory, filteredReasons), also update or clear
payload.reportedForReason and payload.reporterIdentifier to reflect
filteredHistory[0] (or undefined/null when filteredHistory is empty) so legacy
fields are consistent with the scrubbed state.

---

Nitpick comments:
In `@docs/user/reports.md`:
- Line 26: The long sentence is hard to parse; split it into shorter sentences
and remove nested parentheticals: explain first that the default action only
affects the current report by removing this reporter's entries from the job's
report history (and, if that leaves the job with no other reporters or report
sources, remove the job), then state separately that ticking "Apply across the
whole organization" in the confirmation modal instead sweeps every pending job
in the org; also replace "enqueued purely from a user report" with an explicit
phrase such as "the job has no other reporters or other report sources (for
example, automated detectors)" and make "Decided/closed jobs are not modified"
its own sentence for clarity.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 596199c8-936d-434d-9a20-c0b3a86e3504

📥 Commits

Reviewing files that changed from the base of the PR and between 071a4b2 and 6a0d040.

⛔ Files ignored due to path filters (2)
  • client/src/graphql/generated.ts is excluded by !**/generated.ts
  • server/graphql/generated.ts is excluded by !**/generated.ts
📒 Files selected for processing (13)
  • client/src/webpages/dashboard/mrt/manual_review_job/InvalidateReportsButton.test.tsx
  • client/src/webpages/dashboard/mrt/manual_review_job/InvalidateReportsButton.tsx
  • client/src/webpages/dashboard/mrt/manual_review_job/ManualReviewJobReview.tsx
  • client/src/webpages/dashboard/mrt/manual_review_job/MergedReportsComponent.test.tsx
  • client/src/webpages/dashboard/mrt/manual_review_job/MergedReportsComponent.tsx
  • client/src/webpages/dashboard/mrt/manual_review_job/ReportInfoComponent.tsx
  • db/src/scripts/api-server-pg/2026.05.27T21.33.27.widen_mrt_job_id_columns.sql
  • docs/user/reports.md
  • server/graphql/modules/manualReviewTool.ts
  • server/services/manualReviewToolService/manualReviewToolService.ts
  • server/services/manualReviewToolService/modules/QueueOperations.ts
  • server/services/manualReviewToolService/modules/ReporterInvalidation.test.ts
  • server/services/manualReviewToolService/modules/ReporterInvalidation.ts

Comment thread server/services/manualReviewToolService/manualReviewToolService.ts
Comment thread server/services/manualReviewToolService/modules/ReporterInvalidation.ts Outdated
juanmrad added 2 commits May 29, 2026 15:24
# Conflicts:
#	client/src/webpages/dashboard/mrt/manual_review_job/ReportInfoComponent.tsx
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.

Comment thread server/services/manualReviewToolService/modules/QueueOperations.ts
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

Checklist:

  • Are as many columns marked NOT NULL as possible? If some columns can sometimes be null, but not when another column is filled in, have you considered normalizing the schema further (see https://www.guru99.com/database-normalization.html)? If that's not viable/desirable, are there adequate SQL CHECK constraints to capture the relationships between column values (e.g., which can be null together), and are these also captured using unions in the associated kysely types? (db/src/scripts/api-server-pg/2026.05.27T21.33.27.widen_mrt_job_id_columns.sql)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalidate reports for a user

2 participants