476 extend assignment indicator to reflect current stage assignee - #502
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChapter ownership now resolves from the workflow stage and the relevant assignee. Project and My Work queries fetch peer-checker data. Chapter rows show either the conflict indicator or the ownership indicator, with tests covering both states. ChangesChapter ownership behavior
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature Merge Risk: ⚪ Minimal · up to Stage-aware ownership and conflict indicator behavior are covered across the updated utility, queries, components, and tests; no merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 full review |
✅ Action performedFull review finished. |
…urrent-stage-assignee
TLDR
Makes the chapter list's ownership icon stage-aware — it now shows the assignee relevant to the item's current stage (drafter at Drafting, PM-assigned peer checker at Peer Check, nothing at Community Review/Advanced/Complete) instead of always reading Drafting's
assigned_user_id. Also fixes a pre-existing bug where the conflict icon and ownership icon could render simultaneously in the same slot.Reviewer checklist
Refs #NNN)Details
Refs #476
deriveChapterOwnershipStatepreviously took onlyassigned_user_id, so it always evaluated Drafting's assignee regardless of the item's actual stage. AddedresolveStageAssigneeIdto pick the correct field (assigned_user_idat Drafting,peer_checker_idat Peer Check,nullotherwise) before handing off to the existing three-state derivation.peer_checker_idwasn't previously selected by the two list queries (getProjectChapters,getMyWorkChapters) — only by the single-recordgetChapterAssignmentById— so it's now added to both.Peer Check items with no
peer_checker_idshow no icon. This is consistent with #442's open-Peer-Check model once that ships, but #476 has no functional dependency on #442 — the null-check works correctly against today's data regardless of #442's status.Scope expansion: While implementing this, found
ChapterConflictIndicator(#260) andChapterOwnershipIndicatorrendering as two independent, unconditional elements in bothMyWorkRowandProjectChapterRow— no actual precedence existed, despite this ticket's own NFR assuming conflict "continues to take precedence in the same slot." Since #476 makes the ownership icon appear in more cases (any Peer-Check item with a PM-assigned checker), the collision becomes more likely, so fixed it as part of this PR: conflict and ownership now render as a single ternary per row, conflict wins. This also required updating two existing tests (renders indicators in the shared cloud, conflict, ownership order) that had encoded the old dual-render bug as expected behavior.Needs QA?
Type of change:
Technical changes
src/utils/chapterOwnershipState.ts— addedresolveStageAssigneeId(status, assignedUserId, peerCheckerId); existingderiveChapterOwnershipStateunchanged.src/db/queries.ts— addedca.peer_checker_idtogetProjectChaptersandgetMyWorkChaptersSELECTs;mapChapterRowCorenow callsresolveStageAssigneeIdbeforederiveChapterOwnershipState.src/types/db/types.ts— addedpeer_checker_idtoProjectChapterRowandMyWorkChapterRow.src/components/ui/MyWorkRow.tsx,src/components/ui/ProjectChapterRow.tsx— conflict/ownership icon precedence fixed to a single ternary (scope expansion, see above).src/utils/chapterOwnershipState.test.ts— coverage for allresolveStageAssigneeIdbranches (Drafting with/without assignee, Peer Check with/without PM assignment, no-assignee stages, unrecognized/null status).src/components/ui/MyWorkRow.test.tsx,src/components/ui/ProjectChapterRow.test.tsx— replaced the outdated dual-render assertion with precedence-focused tests (conflict wins when both apply; ownership shows when no conflict).Testing
npm run format:check && npm run lint && npm run typecheck && npm test -- --ci— all passing except a pre-existing, unrelated flaky timeout inusePrepareOfflineDownload.test.ts(not touched by this PR).How to verify
npm run format:check && npm run lint && npm run typecheck && npm test -- --cihas_conflict = true; confirm only the conflict icon renders, not both.Expected: Icon reflects the current-stage assignee only; no icon past Peer Check; conflict icon always wins when both conditions are true.
Follow-ups
community_checklabel vs API'scommunity_reviewstatus value is a naming-only mismatch inSTAGE_BY_STATUS— functionally correct today, no ticket filed yet, tracked verbally with team.getChapterAssignmentsWithBooks/ChapterListItemconfirmed dead code (grep shows zero call sites beyond their own definitions) — left untouched here for scope discipline; candidate for a separate cleanup PR.Summary by CodeRabbit