Skip to content

Attention model + course-scoping player gate (audit finale) - #965

Merged
njrini99-code merged 2 commits into
mainfrom
batch/audit-attention-coursescope
Jul 19, 2026
Merged

Attention model + course-scoping player gate (audit finale)#965
njrini99-code merged 2 commits into
mainfrom
batch/audit-attention-coursescope

Conversation

@njrini99-code

Copy link
Copy Markdown
Owner

What

Two audit findings from the Full Product Audit Swarm, integrated together.

1. One canonical coach "needs you" attention count (the deepest finding)

The coach dashboard told three different stories about what needed the coach:

  • the hero counted actionItems.length (tasks + announcements) and its copy claimed "approvals" it never counted;
  • pending roster join-request approvals were counted by nothing;
  • announcements (which have no accept/resolve step) inflated the "waiting on you" total as if they were backlog.

New pure attention-queue.ts derives one honest count from data the dashboard already holds (no new query, no RLS change): total = actionable tasks + pending approvals, announcements excluded. Wired through every surface:

  • Hero (coach-signal.ts): the number is the canonical total — approvals are finally counted, so the "approvals" copy is now true — and the body names only the parts that exist ("3 tasks and 2 approvals flagged from your roster this week.").
  • Action Items panel: badge counts actionable work only; announcements move to a clearly labelled, separately counted strip and never inflate the backlog. Row markup extracted to a shared ActionItemRow.
  • Approvals keep their dedicated banner (the act-now affordance); Signals and Team Pulse keep their own genuinely-different metrics.

2. Course-scoping: hide create/edit/delete controls from players (owner decision: "keep library open, hide player edit")

Players could browse the shared course library (intended) but four mutation controls leaked to player sessions: the library "+ Add course" button and empty-state CTA, plus (on non-library courses) "Edit course", photo upload/replace/remove, "Add tee", and the per-tee "Edit" button. All now gate on the existing canManageTeam boolean the server page already resolves from session role. Pure UI role-gate, no RLS/server-action change; same conditional path serves desktop and mobile.

Gates

  • npx eslint <10 changed files> → 0
  • npx tsc --noEmit (full) → 0
  • npx vitest run (full) → 2553 files / 23984 tests pass, 56 skipped, 0 failures

(One unrelated test, FairwayStatsCockpit.tabSwitching, flaked once under full-suite concurrency on the first run and passed deterministically in isolation and on a clean full re-run. It is untouched by this diff.)

Follow-up flagged for Nick (not in this PR)

The new-round tee picker (FairwayCoursePicker.tsx) also lets players add a course, which is the same data-pollution vector this PR closes for the library. Gating it there would be consistent with the "hide player edit" decision, but it would block a player from logging a round on a course that isn't in the coach-curated library — a real UX tradeoff outside the scoped library decision. Left open pending your call.

🤖 Generated with Claude Code

Fable Integrator and others added 2 commits July 18, 2026 22:35
The coach dashboard told three different stories about what needed the
coach: the hero counted actionItems.length (tasks + announcements) and
its copy claimed "approvals" it never counted; pending join-request
approvals were counted by nothing; and announcements (no accept/resolve
step) inflated the "waiting on you" total as if they were backlog.

Add a pure attention-queue module deriving ONE honest count from data the
dashboard already holds (no new query, no RLS): total = actionable tasks
+ pending approvals, announcements excluded. Wire it through:

- coach-signal hero: number is the canonical total (approvals finally
  counted, so the "approvals" copy is true), body names only the parts
  that exist ("3 tasks and 2 approvals ...").
- ActionItemsPanel: badge counts actionable work only; announcements move
  to a clearly-labelled, separately-counted strip and never inflate the
  backlog. Row markup extracted to a shared ActionItemRow.

Signals and Team Pulse keep their own genuinely-different metrics.

Tests: attention-queue counts/breakdown/split, coach-signal honest hero,
ActionItemsPanel announcement separation. eslint + vitest green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
…players (audit owner-decision)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@cursor

cursor Bot commented Jul 19, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@vercel

vercel Bot commented Jul 19, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
helmv3 Ignored Ignored Jul 19, 2026 2:48am

Request Review

@supabase

supabase Bot commented Jul 19, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project qmnssrrolpinvwjjnufo because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review Change Stack

Summary by CodeRabbit

  • Dashboard Updates

    • Action Items now separate actionable tasks and deadlines from informational announcements.
    • Counts and “needs your attention” messaging exclude announcements.
    • Announcements appear in a dedicated section with their own count.
    • Improved empty-state messaging for announcement-only items.
  • Access Controls

    • Course creation and editing controls are now limited to users with team-management access.
    • Players can still browse courses without seeing restricted actions.
  • Tests

    • Added coverage for dashboard attention counts, announcements, and course access permissions.

Walkthrough

The PR adds a canonical Fairway attention model that excludes announcements from actionable counts, updates dashboard signal copy, and separates announcement rendering. It also restricts golf course mutation controls to team managers with player and coach test coverage.

Changes

Fairway dashboard attention model

Layer / File(s) Summary
Canonical attention queue model
src/components/fairway/pages/dashboard/attention-queue.ts, src/components/fairway/pages/dashboard/attention-queue.test.ts
Defines actionable/announcement partitioning, task-plus-approval totals, formatted breakdown text, and associated tests.
Coach signal attention integration
src/components/fairway/pages/dashboard/coach-signal.ts, src/components/fairway/pages/dashboard/FairwayCoachDashboard.tsx, src/components/fairway/pages/dashboard/coach-signal.test.ts
Passes canonical attention counts into deriveCoachSignal, which derives totals and copy from tasks and approvals.
Action items and announcements presentation
src/components/fairway/pages/dashboard/FairwayCoachDashboard.tsx, src/components/fairway/pages/dashboard/ActionItemsPanel.test.tsx
Renders actionable items separately from announcements, updates badges and empty states, and tests mixed and announcement-only payloads.

Golf course management permission gates

Layer / File(s) Summary
Course library management gate
src/components/golf/courses/CourseLibraryClient.tsx, src/test/golf/components/CourseLibraryClientPlayerGate.test.tsx
Shows course-creation controls and instructional empty-state actions only when canManageTeam is true.
Course detail mutation gates
src/components/golf/courses/CourseDetailDrawer.tsx, src/test/golf/components/CourseDetailDrawerPlayerGate.test.tsx
Restricts course, photo, tee-set, and tee-row edit controls to team managers and verifies player/coach behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant FairwayCoachDashboard
  participant attentionQueue
  participant coachSignal
  FairwayCoachDashboard->>attentionQueue: Build counts from action items and pending approvals
  attentionQueue-->>FairwayCoachDashboard: Return canonical AttentionCounts
  FairwayCoachDashboard->>coachSignal: Derive signal with AttentionCounts
  coachSignal->>attentionQueue: Format task and approval breakdown
  attentionQueue-->>coachSignal: Return attention text
  coachSignal-->>FairwayCoachDashboard: Return CoachSignal
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 9 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title is relevant, but it does not follow Conventional Commits or include one of the required scopes. Rename it to Conventional Commits format with a required scope, e.g. feat(coachhelm): canonical attention model and player-gate updates.},{
Conventional Commits ❓ Inconclusive HEAD subject matches the Conventional Commits regex; PR title isn't available in-repo, so I can't verify it. Provide the GitHub PR title or PR metadata so the title can be checked against the convention.
✅ Passed checks (9 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
No Service-Role In Client Bundles ✅ Passed Searched all changed files; none reference SUPABASE_SERVICE_ROLE_KEY or create a Supabase client. The touched code is dashboard/course UI and tests only.
Rls Coverage On New Tables ✅ Passed No changed migration files in the PR diff vs origin/main; only src/.ts(x) files were modified.
Auth Check In Server Actions ✅ Passed No changed file matches src/app//actions//*.ts; the PR only touches dashboard and golf component/test files.
Sport-Prefixed Table Names ✅ Passed No changed TS/TSX file contains Supabase table queries or table-name literals; CourseDetailDrawer only calls actions at 17-21 and 89-187, and the rest are pure UI/tests.
No Destructive Writes ✅ Passed No changed file contains a delete-then-insert rebuild; the only mutation paths are single action calls in CourseDetailDrawer.tsx:140-188, and tests only mock them in the gate specs.
No Edits To Historical Migrations ✅ Passed No files under supabase/migrations/ changed in the PR diff, so no historical migration edits were introduced.
✨ 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 batch/audit-attention-coursescope
  • 🛠️ helm safety pass
  • 🛠️ dashboard ux pass
  • 🛠️ rls test pass

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.44.1)

ast-grep could not parse rule config: /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/components/fairway/pages/dashboard/ActionItemsPanel.test.tsx

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

src/components/fairway/pages/dashboard/FairwayCoachDashboard.tsx

ESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.

src/components/fairway/pages/dashboard/attention-queue.test.ts

ESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.

  • 7 others

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 coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@src/components/fairway/pages/dashboard/FairwayCoachDashboard.tsx`:
- Around line 1163-1199: Update the attention-item rendering around the icon and
status pill so announcement items never receive overdue treatment: require
item.type !== 'announcement' alongside item.overdue in both the icon branch and
the Overdue StatusPill branch. Preserve the existing urgent and announcement
rendering for all other cases.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: 6c1ac62d-528a-47f8-8a81-ee330e93e357

📥 Commits

Reviewing files that changed from the base of the PR and between 12f018e and bc8b837.

📒 Files selected for processing (10)
  • src/components/fairway/pages/dashboard/ActionItemsPanel.test.tsx
  • src/components/fairway/pages/dashboard/FairwayCoachDashboard.tsx
  • src/components/fairway/pages/dashboard/attention-queue.test.ts
  • src/components/fairway/pages/dashboard/attention-queue.ts
  • src/components/fairway/pages/dashboard/coach-signal.test.ts
  • src/components/fairway/pages/dashboard/coach-signal.ts
  • src/components/golf/courses/CourseDetailDrawer.tsx
  • src/components/golf/courses/CourseLibraryClient.tsx
  • src/test/golf/components/CourseDetailDrawerPlayerGate.test.tsx
  • src/test/golf/components/CourseLibraryClientPlayerGate.test.tsx

Comment on lines +1163 to +1199
<span className="mt-0.5 shrink-0">
{item.overdue ? (
<IconAlertCircle size={16} className="text-fw-danger" />
) : item.type === 'announcement' ? (
<IconBell size={16} className="text-text-tertiary" />
) : isUrgent ? (
<IconAlertCircle size={16} className="text-fw-warning" />
) : (
<IconClipboardList size={16} className="text-text-tertiary" />
)}
</span>
<span className="flex min-w-0 flex-1 flex-col gap-1">
<span
className={cn(
'truncate font-fw-sans text-body font-medium',
item.overdue ? 'text-fw-danger' : 'text-text-primary',
)}
>
{item.title}
</span>
<span className="flex flex-wrap items-center gap-2">
<span
className="font-fw-sans text-caption text-text-tertiary"
suppressHydrationWarning
>
{now ? formatRelativeDate(item.date, now) : ''}
</span>
{item.overdue ? (
<StatusPill tone="danger" dot={false} size="sm">
Overdue
</StatusPill>
) : isUrgent ? (
<StatusPill tone="warning" dot={false} size="sm">
{item.priority === 'urgent' ? 'Urgent' : 'High'}
</StatusPill>
) : null}
</span>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard the overdue treatment against announcement items.

item.overdue is checked before item.type === 'announcement', so an announcement with overdue: true would render the danger icon and "Overdue" pill — contradicting the attention model's rule that announcements are never actionable/urgent (see the doc comment at Line 1210-1215). ActionItem.overdue is untyped against type, so nothing prevents this combination upstream.

🛡️ Proposed guard
-          <span className="mt-0.5 shrink-0">
-            {item.overdue ? (
+          <span className="mt-0.5 shrink-0">
+            {item.overdue && item.type !== 'announcement' ? (
               <IconAlertCircle size={16} className="text-fw-danger" />
             ) : item.type === 'announcement' ? (
               <IconBell size={16} className="text-text-tertiary" />
             ) : isUrgent ? (
               <IconAlertCircle size={16} className="text-fw-warning" />
             ) : (
               <IconClipboardList size={16} className="text-text-tertiary" />
             )}
           </span>

Apply the same guard to the Overdue StatusPill branch below it.

🤖 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 `@src/components/fairway/pages/dashboard/FairwayCoachDashboard.tsx` around
lines 1163 - 1199, Update the attention-item rendering around the icon and
status pill so announcement items never receive overdue treatment: require
item.type !== 'announcement' alongside item.overdue in both the icon branch and
the Overdue StatusPill branch. Preserve the existing urgent and announcement
rendering for all other cases.

@njrini99-code
njrini99-code merged commit 21562fc into main Jul 19, 2026
43 checks passed
@njrini99-code
njrini99-code deleted the batch/audit-attention-coursescope branch July 19, 2026 03:09
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