Skip to content

feat(meetings): flag same-meeting assignment conflicts in red - #5379

Open
FranklinD10 wants to merge 6 commits into
sws2apps:mainfrom
FranklinD10:feat/same-meeting-conflict
Open

feat(meetings): flag same-meeting assignment conflicts in red#5379
FranklinD10 wants to merge 6 commits into
sws2apps:mainfrom
FranklinD10:feat/same-meeting-conflict

Conversation

@FranklinD10

@FranklinD10 FranklinD10 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Description

Part 1 of 2 for #3147.

That issue asks for two things. This PR does the additive half only: a red warning when the same person is given two roles in the same meeting, for example hall overseer and Watchtower reader in one midweek meeting. The existing orange week-level warning is left exactly as it is.

The second half (narrowing the orange so it only fires on the same assignment in consecutive weeks) is a behaviour change to something users see every week, so I have kept it in a separate PR that stacks on this one.

How it works

Meeting identity comes from the MM_/WM_ prefix that is already on every ASSIGNMENT_PATH key, so there is no new state and no new data plumbing. A new helper schedulesPersonHasMeetingConflict reads the existing assignmentsHistoryState and is shared by the brother and student selectors.

It is scoped to the current dataView, so a collision the user cannot see on screen is never flagged.

Red takes precedence over orange when both would apply, since it is the more specific message. Nothing is removed and nothing existing is reworded.

Deliberate exemptions

Three cases never trigger red, to avoid false positives:

  • Prayers. The app already ships a "link prayer assignments to meeting parts" setting, and WM_ClosingPrayer auto-displays the WM_Speaker_Part1 person, so a prayer doubled with another part is normal practice.
  • Circuit overseer fields. During a CO visit the same brother legitimately holds both his own field and a talk.
  • WM_Speaker_Outgoing. That record belongs to another congregation's meeting, not ours.

One edge worth flagging

isMeetingConflict keys off the selector's displayed value, and a few fields display a default that is not persisted (MM_Chairman_B aux counselor, WM_WTStudy_Conductor). If that defaulted person holds another role in the same meeting, red will show on a field the user has not filled in yet. I think that is correct information to surface, but it is a small difference from the orange warning, which cannot trip without a saved record. Happy to suppress it if you would rather.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

Devin Review

@vercel

vercel Bot commented Aug 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
staging-organized-app Ready Ready Preview Sep 6, 2026 1:13pm UTC
test-organized-app Ready Ready Preview Sep 6, 2026 1:13pm UTC

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 4e0d2b13-5ba7-4024-94d0-40d718c92a47

📥 Commits

Reviewing files that changed from the base of the PR and between fd08b3f and 97145d1.

📒 Files selected for processing (2)
  • src/features/meetings/person_selector/brother_selector/useBrotherSelector.tsx
  • src/features/meetings/person_selector/student_selector/useStudentSelector.tsx

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.


Walkthrough

Changes

Meeting conflict indicators

Layer / File(s) Summary
Conflict detection service
src/services/app/schedules.ts
Adds schedulesPersonHasMeetingConflict, which checks same-meeting assignments and excludes the edited and conflict-exempt assignments.
Selector conflict state
src/features/meetings/person_selector/{brother_selector,student_selector}/use*.tsx
Both selector hooks compute isMeetingConflict, return conflict helper text before existing warnings, and expose the new state.
Conflict presentation and autocomplete color
src/components/autocomplete/*, src/features/meetings/person_selector/{brother_selector,student_selector}/index.tsx
Adds configurable autocomplete decorator colors. Selectors apply conflict colors to decorators, icons, and helper text while preserving linked-part handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 97145

This change adds same-meeting assignment conflict warnings for brother and student selectors, with red conflict messaging taking precedence over existing notices. No concrete current-head merge risk is identified.

Sequence Diagram(s)

sequenceDiagram
  participant Selector
  participant SelectorHook
  participant SchedulesService
  participant Autocomplete
  Selector->>SelectorHook: select a person
  SelectorHook->>SchedulesService: check meeting assignment conflict
  SchedulesService-->>SelectorHook: return isMeetingConflict
  SelectorHook-->>Selector: return conflict state and helper text
  Selector->>Autocomplete: pass decorator and helper colors
  Autocomplete-->>Selector: render conflict styling
Loading

Suggested reviewers: ux-git

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: flagging same-meeting assignment conflicts in red.
Description check ✅ Passed The description directly explains the new red warning, conflict scope, exemptions, precedence, implementation, and relationship to the follow-up work.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 7…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 30, 2026
Add a red warning when the same person is given two roles in the same
meeting, e.g. hall overseer and Watchtower reader in one midweek meeting.
The existing orange week-level warning is untouched; red simply takes
precedence when both would apply.

Meeting identity comes from the MM_/WM_ prefix already present on every
ASSIGNMENT_PATH key, so no new state or data plumbing is needed. New
helper schedulesPersonHasMeetingConflict reads the existing
assignmentsHistoryState and is shared by the brother and student
selectors. Scoped to the current dataView so a collision the user cannot
see is never flagged.

Prayers, circuit overseer fields and WM_Speaker_Outgoing are exempt:
prayers are commonly doubled with another part (the app ships a linked
prayer setting for exactly that), the CO legitimately holds both his own
field and a talk during a visit, and an outgoing talk belongs to another
congregation's meeting.

Part of sws2apps#3147

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TRZ5tRSmSDbCpEHAbnVij9
@FranklinD10
FranklinD10 force-pushed the feat/same-meeting-conflict branch from 877d37f to 41cd9af Compare August 31, 2026 07:38
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 31, 2026
@FranklinD10

Copy link
Copy Markdown
Contributor Author

@rhahao @ux-git
Please review when you have some time

@mergify

mergify Bot commented Sep 4, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

devin-ai-integration[bot]

This comment was marked as resolved.

@FranklinD10

Copy link
Copy Markdown
Contributor Author

@rhahao @ux-git

Quick bundle check when you have some time. These four are green and ready to queue on your end: #5379, #5380, #5391 and #5415. I can't queue myself, no write permission. #5380 sits on top of #5379 so that order helps. Thanks!

The helper color and decorator turn red on same-meeting conflict alone, but the text kept the absence notice, hiding the conflict. Check conflict first in both selectors so text matches color. Addresses Devin review on sws2apps#5379.
@sonarqubecloud

sonarqubecloud Bot commented Sep 6, 2026

Copy link
Copy Markdown

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.

1 participant