Skip to content

feat: Restrict @predictors mentions to curators/admins with warning toast#4599

Open
SylvainChevalier wants to merge 4 commits into
mainfrom
claude/issue-4082-tagging-predictors-in-comments-should
Open

feat: Restrict @predictors mentions to curators/admins with warning toast#4599
SylvainChevalier wants to merge 4 commits into
mainfrom
claude/issue-4082-tagging-predictors-in-comments-should

Conversation

@SylvainChevalier
Copy link
Copy Markdown
Contributor

@SylvainChevalier SylvainChevalier commented Apr 8, 2026

Summary

  • @predictors autocomplete suggestion only shown to curators/admins (already existed)
  • Backend now filters to only active (non-withdrawn) predictions
  • Non-privileged users see a warning toast when using @predictors
  • Edit form now includes mention autocomplete with permission filtering

Fixes #4082


Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features

    • Added warning notifications for non-curator and non-admin users attempting to mention @predictors in comments. Only authorized users can send notifications to predictors.
  • Improvements

    • @predictors mentions now exclude users with withdrawn or expired forecasts, ensuring more accurate targeting of active forecasters.

Review Change Stack

- Backend: filter @predictors to only active (non-withdrawn) predictions
  by excluding forecasts with end_time in the past
- Frontend: show warning toast when non-admin/curator uses @predictors
  in both comment creation and editing flows
- Frontend: add mention autocomplete with permission filtering to the
  comment edit form (was previously only on new comment form)
- Add translation key for the warning message
- Add test for withdrawn forecast exclusion

Fixes #4082

Co-authored-by: Sylvain <SylvainChevalier@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 8, 2026

📝 Walkthrough

Walkthrough

Backend filtering now excludes withdrawn forecasts from @predictors mentions using timezone checks. Frontend detects @predictors in comments and displays permission-based warnings to non-curator/admin users across comment creation and editing components.

Changes

@predictors Mention Restriction

Layer / File(s) Summary
Backend filtering of withdrawn forecasts
comments/utils.py, tests/unit/test_comments/test_utils.py
Query now imports Django timezone and filters @predictors recipients to exclude forecasts with a non-null and past end_time. Test verifies only active forecasters are returned when @predictors is mentioned.
Frontend mention detection utility
front_end/src/utils/comments.ts, front_end/messages/en.json
New hasPredictorsMention() utility detects @predictors in comment text using regex (case-insensitive). Corresponding i18n warning message added for non-curator/admin users.
Comment component warning UI
front_end/src/components/comment_feed/comment.tsx
Toast import and hasPredictorsMention detection added. On successful save, shows warning when markdown contains @predictors and user is not CURATOR or ADMIN. Editor config enables mention rendering with permission context.
Comment editor warning UI
front_end/src/components/comment_feed/comment_editor.tsx
Toast import and hasPredictorsMention detection added. On form submission, shows warning when parsed markdown contains @predictors and user lacks CURATOR or ADMIN permission.

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Metaculus/metaculus#4088: Both PRs adjust the front-end handling of @predictors mentions based on user permission—main PR adds curator/admin warnings and hasPredictorsMention detection, while retrieved PR threads userPermission into the mentions plugin to gate the predictors mention.

Suggested reviewers

  • ncarazon
  • lsabor
  • cemreinanc

🐰 A mention so grand, so restricted and rare,
Only admins and curators allowed to declare!
Forecasters rejoice, with filters so keen—
Withdrawn predictions now filtered unseen. ✨

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: restricting @predictors mentions to curators/admins with a warning toast for others.
Linked Issues check ✅ Passed The pull request implements all key requirements from issue #4082: restricts @predictors mentions to curators/admins, adds warning toast for non-privileged users, and backend filters active forecasts.
Out of Scope Changes check ✅ Passed All changes are directly aligned with issue #4082's requirement to restrict @predictors mentions and add warning toasts; no out-of-scope modifications detected.

✏️ 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 claude/issue-4082-tagging-predictors-in-comments-should

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2026

🚀 Preview Environment

Your preview environment is ready!

Resource Details
🌐 Preview URL https://metaculus-pr-4599-claude-issue-4082-tagging-pred-preview.mtcl.cc
📦 Docker Image ghcr.io/metaculus/metaculus:claude-issue-4082-tagging-predictors-in-comments-should-d6cf6fe
🗄️ PostgreSQL NeonDB branch preview/pr-4599-claude-issue-4082-tagging-pred
Redis Fly Redis mtc-redis-pr-4599-claude-issue-4082-tagging-pred

Details

  • Commit: 11bb023a701a614cbe05a51f97709d02cfc91dc4
  • Branch: claude/issue-4082-tagging-predictors-in-comments-should
  • Fly App: metaculus-pr-4599-claude-issue-4082-tagging-pred

ℹ️ Preview Environment Info

Isolation:

  • PostgreSQL and Redis are fully isolated from production
  • Each PR gets its own database branch and Redis instance
  • Changes pushed to this PR will trigger a new deployment

Limitations:

  • Background workers and cron jobs are not deployed in preview environments
  • If you need to test background jobs, use Heroku staging environments

Cleanup:

  • This preview will be automatically destroyed when the PR is closed

@SylvainChevalier
Copy link
Copy Markdown
Contributor Author

@claude please fix the unit test problems.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 20, 2026

Claude encountered an error —— View job


Fixing Unit Test Problems

  • Read CLAUDE.md for repo-specific instructions
  • Identify test failures
  • Fix test issues
  • Commit and push fixes

@SylvainChevalier
Copy link
Copy Markdown
Contributor Author

@claude please fix the unit test problems.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

Claude finished @SylvainChevalier's task in 5m 20s —— View job


Fixing Unit Test Problems

  • Read Claude.md for repo-specific instructions
  • Check CI status to identify test failures
  • Investigate failing tests
  • Fix test issues
  • Commit and push fixes

Summary

Two CI failures, both fixed in 128c693:

Backendtests/unit/test_comments/test_utils.py
test_predictors_mention_excludes_withdrawn_forecasts failed with a DB IntegrityError on the end_time_after_start_time check constraint. factory_forecast auto-populates start_time to now(), so passing only end_time = now - 1 day produces end_time < start_time. Fixed by also passing start_time = now - 2 days so the withdrawn forecast spans 2 days in the past.

Frontend formattingfront_end/src/components/comment_feed/comment.tsx
Prettier flagged the new ![CURATOR, ADMIN].includes(userPermission) block. The canonical prettier shape (already present in comment_editor.tsx:175) keeps the array literal inline and puts userPermission on its own line. Reformatted to match.

Not done

I could not run the prettier check or the pytest suite locally (sandbox restrictions on shell access), so the fixes rely on CI to validate. The frontend fix mirrors the exact pattern prettier produced in the sibling file, and the backend fix addresses the precise constraint shown in the traceback.
· Branch

- tests/unit/test_comments/test_utils.py: pass start_time alongside
  end_time for the withdrawn forecast so end_time > start_time and
  the end_time_after_start_time DB check constraint is satisfied.
- front_end/src/components/comment_feed/comment.tsx: reformat the
  predictors-warning condition to match prettier's preferred shape
  (matches the equivalent block already in comment_editor.tsx).

Co-authored-by: Sylvain <SylvainChevalier@users.noreply.github.com>
@SylvainChevalier
Copy link
Copy Markdown
Contributor Author

@claude some more unit tests failing.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 19, 2026

Claude finished @SylvainChevalier's task in 2m 18s —— View job


Fixing Unit Test Problems

  • Check CI status to identify failing tests
  • Investigate failing tests
  • Fix test issues
  • Commit and push fixes

Summary

The only failure in run 25917354314 was a frontend lint error in front_end/src/components/comment_feed/comment_editor.tsx:

##[error]  5:1  error  There should be no empty line within import group  import/order

A stray blank line sat between the react and react-hot-toast imports. Removed it in e979c46 so the third-party imports form one contiguous group.

The backend test job in that run was already green, so no further changes were needed.

Not done

I could not run the linter locally (sandbox restrictions), so the fix relies on CI to validate.
· Branch

Co-authored-by: Sylvain <SylvainChevalier@users.noreply.github.com>
@claude claude Bot temporarily deployed to testing_env May 19, 2026 14:37 Inactive
@claude claude Bot temporarily deployed to testing_env May 19, 2026 14:37 Inactive
@SylvainChevalier
Copy link
Copy Markdown
Contributor Author

@claude some more unit tests failing.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 19, 2026

Claude finished @SylvainChevalier's task in 1m 31s —— View job


Fixing Unit Test Problems

  • Check CI status to identify failing tests
  • Investigate failing tests
  • Fix test issues
  • Commit and push fixes

Summary

The only failure in run 26104324382 was a frontend lint error in front_end/src/components/comment_feed/comment.tsx:

##[error]  13:1  error  There should be no empty line within import group  import/order

The previous fix in e979c46 removed the same kind of stray blank line from comment_editor.tsx, but comment.tsx had the same issue (a blank line between the react import on line 13 and the react-hot-toast import on line 15). Removed it in d6cf6fe so the third-party imports form one contiguous group.

The backend test job in that run was already green, so no further changes were needed.

Not done

I could not run the linter locally (sandbox restrictions), so the fix relies on CI to validate.
· Branch

Co-authored-by: Sylvain <SylvainChevalier@users.noreply.github.com>
@claude claude Bot temporarily deployed to testing_env May 19, 2026 15:21 Inactive
@SylvainChevalier
Copy link
Copy Markdown
Contributor Author

lgtm as far as the ui is concerned, but I couldn't test the emails.

@SylvainChevalier SylvainChevalier marked this pull request as ready for review May 19, 2026 15:57
@SylvainChevalier SylvainChevalier requested a review from ncarazon May 19, 2026 15:57
@ncarazon
Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
front_end/src/components/comment_feed/comment.tsx (1)

493-574: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add postData?.user_permission to handleSaveComment dependency array.

The callback reads postData?.user_permission on line 525 for the permission-based warning gate, but postData is not included in the dependency array. This creates a stale closure where the callback uses the old postData reference if the prop changes between renders.

Suggested fix
   }, [
     user,
     isLoading,
     commentMarkdown,
     comment.id,
+    postData?.user_permission,
     PUBLIC_MINIMAL_UI,
     t,
     setCommentMarkdown,
     setComments,
🤖 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 `@front_end/src/components/comment_feed/comment.tsx` around lines 493 - 574,
The handleSaveComment callback reads postData?.user_permission but postData is
not in its dependency array, causing a stale closure; update the dependency list
for handleSaveComment (the useCallback that defines handleSaveComment) to
include postData (or specifically postData?.user_permission) so the callback
re-creates whenever postData changes, ensuring the permission check uses the
current postData value.
🤖 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 `@front_end/src/utils/comments.ts`:
- Around line 83-88: The hasPredictorsMention function currently compares raw
capture groups (match[1] or match[2]) directly to "predictors", which misses
forms like @(predictors); update hasPredictorsMention to normalize the captured
username by trimming whitespace and any surrounding parentheses or surrounding
characters before lowercasing and comparing. Locate hasPredictorsMention (and
its use of userTagPattern and match[1]/match[2]) and apply a small normalization
step such as stripping leading/trailing parentheses and whitespace from the
chosen capture value, then compare normalizedUsername.toLowerCase() ===
"predictors".

---

Outside diff comments:
In `@front_end/src/components/comment_feed/comment.tsx`:
- Around line 493-574: The handleSaveComment callback reads
postData?.user_permission but postData is not in its dependency array, causing a
stale closure; update the dependency list for handleSaveComment (the useCallback
that defines handleSaveComment) to include postData (or specifically
postData?.user_permission) so the callback re-creates whenever postData changes,
ensuring the permission check uses the current postData value.
🪄 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

Run ID: aceda96a-12c5-4538-b92b-9200034a3ba1

📥 Commits

Reviewing files that changed from the base of the PR and between a75665e and d6cf6fe.

📒 Files selected for processing (6)
  • comments/utils.py
  • front_end/messages/en.json
  • front_end/src/components/comment_feed/comment.tsx
  • front_end/src/components/comment_feed/comment_editor.tsx
  • front_end/src/utils/comments.ts
  • tests/unit/test_comments/test_utils.py

Comment thread front_end/src/utils/comments.ts
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.

Tagging @predictors in comments should be restricted to admins and curators

2 participants