Let proposers always find their own questioned proposals#919
Merged
Conversation
Stewards are notified when their proposal is questioned and final review stays blocked until they revise it, but the steward queue's visibility scoping could hide the very submission they must act on: propose-only stewards only saw pending submissions, so a questioned proposal sitting on a submission awaiting more information, or one whose author lost the propose permission for that type, was unreachable while the notification kept pointing at it. Steward visibility now always includes submissions where the steward is the active proposer, regardless of state or current per-type permissions. The questioned-proposal notification also linked to the queue with a hard-coded pending status filter, which showed zero results whenever the submission was not in the pending state; that link now opens with the All status filter so the submission id search always resolves. ## Implementation Notes - backend/contributions/views.py: _visible_submission_queryset ORs a proposed_by=request.user clause into the steward visibility filter - backend/notifications/services.py: steward_submission_review_link takes a status parameter (default unchanged); the proposal-questioned notification passes status='' (All) - backend/contributions/tests/test_questioned_proposal_visibility.py: regression tests for the search-bar query a proposer uses, the has_proposal=true trap (excludes questioned by design), visibility on more_info_needed submissions, visibility after permission revocation, and the notification deep-link query
📝 WalkthroughWalkthroughQuestioned proposals remain visible to their proposers regardless of computed permissions or submission state. Questioned-proposal notifications link to the all-status view, frontend ChangesQuestioned proposal access
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
When the user profile had not loaded yet, the steward search bar's 'me' shortcut in assigned, reviewed, and proposed-by filters fell through to substring name matching, so 'me' silently resolved to any steward whose name contains those letters (e.g. James Medina) and the results belonged to someone else. 'me' now resolves to the current user or to nothing at all. ## Implementation Notes - frontend/src/lib/searchToParams.js: the three 'me' branches (assigned, reviewed, proposed-by) return currentUserId or null instead of falling through to the stewardsList name lookup - frontend/src/tests/searchParser.test.js: covers 'me' resolution for all three filters plus the profile-not-loaded case against a steward list containing a name with the substring 'me'
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/src/lib/searchToParams.js`:
- Around line 174-175: Add matching explanatory comments to the `reviewed` and
`proposed-by` `val === 'me'` branches, consistent with the existing `assigned`
branch, clarifying that `me` must resolve to `currentUserId` rather than fall
through to name matching.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6139bef1-e889-4afa-a86d-d97c09299ee2
📒 Files selected for processing (2)
frontend/src/lib/searchToParams.jsfrontend/src/tests/searchParser.test.js
Comment on lines
+174
to
+175
| if (val === 'me') { | ||
| reviewedValue = currentUserId || null; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Add the explanatory comment to reviewed and proposed-by me branches for consistency.
The assigned filter includes a helpful comment explaining why me must not fall through to name matching. The reviewed and proposed-by branches use the same pattern but lack the comment, making the intent less clear to future readers.
📝 Suggested comments
if (val === 'me') {
+ // Never fall through to name matching: "me" must resolve to the
+ // current user or to nothing (profile not loaded yet).
reviewedValue = currentUserId || null; } else if (val === 'me') {
+ // Never fall through to name matching: "me" must resolve to the
+ // current user or to nothing (profile not loaded yet).
proposedByValue = currentUserId || null;Also applies to: 204-205
🤖 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 `@frontend/src/lib/searchToParams.js` around lines 174 - 175, Add matching
explanatory comments to the `reviewed` and `proposed-by` `val === 'me'`
branches, consistent with the existing `assigned` branch, clarifying that `me`
must resolve to `currentUserId` rather than fall through to name matching.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A reviewer reported getting "Proposal questioned" notifications but finding zero results when clicking through or searching
proposed-by:me proposal-status:questioned. The filters themselves are correct; the problem was two-fold:state='pending'submissions, so a questioned proposal sitting on a submission awaiting more information, or one whose author had since lost the propose permission for that type, was unreachable by its author even though the portal notifies them and blocks final review until they revise it. Steward visibility now always includes submissions where the steward is the active proposer, regardless of state or current per-type permissions. Once a final decision clears the proposal fields, this extra visibility ends with it.status=pending. Questioned proposals can live onpendingormore_info_neededsubmissions, and the notification can be opened after the submission moved on; the hard-coded status filter then guaranteed "No submissions found matching your filters". The questioned-proposal link now opens with the All status filter, so the submission-id search always resolves. Other notifications keep the previous link behavior.Note for reviewers of this PR: searching
has:proposalstill intentionally excludes questioned proposals (that filter means "ready for final review"); the working query isproposal-status:questioned, and a regression test documents both behaviors.Test plan
contributions/tests/test_questioned_proposal_visibility.py: the exact search-bar query a proposer uses, the bare questioned filter, thehas_proposal=trueexclusion trap, visibility onmore_info_neededsubmissions, visibility after permission revocation, and the notification deep-link UUID searchSummary by CodeRabbit
mestrictly maps to the current user ID and doesn’t fall back to name/steward matching when unavailable.