feat(dashboard): wire team + operator + clear-demo to real backend#34
Conversation
Extend UserProfile with optional workspaces array and introduce types for TeamMember, InviteResponse, AdminGlobalKpis, AdminCustomer, AdminAlert, and ClearDemoResponse. Mirrors the existing backend response shapes in cloud/app/routes/. Co-Authored-By: Gradata <noreply@gradata.ai>
Replace the mock-team fixture on /team and /team/members with live
calls to the workspace endpoints shipped in cloud/app/routes/team.py:
- GET /workspaces/{id}/members — roster
- POST /workspaces/{id}/invites — invite by email + role
- PATCH /workspaces/{id}/members/{uid} — role updates
- DELETE /workspaces/{id}/members/{uid} — remove member
Workspace id is picked from profile.workspaces (prefers owner
role, falls back to the first). Aggregates now derive from real
last_sync_at freshness instead of mock deltas.
Extract TeamLeaderboard into src/components/team/ and pure helpers
(isMemberActive, computeTeamAggregate, formatSyncAgo, normalizeRole,
pickWorkspaceId) into src/lib/team.ts to keep them unit-testable
without hitting the supabase env-var guard.
Adds vitest coverage for the helpers and the leaderboard component.
Note: backend currently exposes no GET for listing invites or
DELETE for cancelling invites, so the Members page covers add
but not pending-invite management. Tracked for follow-up.
Co-Authored-By: Gradata <noreply@gradata.ai>
Replace the mock-operator fixture on the /operator surface with live calls to cloud/app/routes/operator.py: - GET /admin/global-kpis — MRR/ARR/churn/NRR rollup - GET /admin/customers — workspace roster w/ health tiers - GET /admin/alerts — churn-risk / failed-payment / usage-spike Backend already enforces the @gradata.ai / @sprites.ai allowlist via require_operator. The frontend no longer second-guesses that — it just renders the god-mode panel when calls succeed and falls back to a friendly empty state when any of the three endpoints returns a 403 (detected from the error detail string surfaced by useApi). Extract CustomerRow and isForbidden into src/components/operator/ so the pure render + gate logic can be unit-tested without pulling in the supabase-dependent api module. Co-Authored-By: Gradata <noreply@gradata.ai>
Adds a ClearDemoButton component that posts to the existing
/brains/{id}/clear-demo endpoint (cloud/app/routes/brains.py).
The button lives on the brain detail page beneath the details
card and opens a confirmation dialog that explains exactly what
gets deleted (seeded demo lessons, corrections, events, meta-rules
— plus the brain itself if it was seeded as a demo).
On success we show a count and reload the page so every card
reflects the cleared state. On failure we surface the server
detail string inline.
Includes vitest coverage for the happy path, singular-row copy,
and the error state (api client mocked).
Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough
WalkthroughAdds backend-driven operator access control, workspace-scoped team management, new UI components (ClearDemoButton, CustomerRow, TeamLeaderboard), supporting utilities/types (team helpers, error/format helpers, extended useApi), and comprehensive tests; integrates demo-clear UI into the brain page and adjusts small settings display logic. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying gradata-dashboard with
|
| Latest commit: |
cab7cb5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3660ec0c.gradata-dashboard.pages.dev |
| Branch Preview URL: | https://feat-dashboard-team-operator.gradata-dashboard.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cloud/dashboard/app/(dashboard)/team/members/page.tsx (1)
121-130:⚠️ Potential issue | 🟠 Major
Invite membercurrently bypasses the plan gate.The button and dialog are mounted outside
PlanGate, so a free-plan user can still open the flow and triggerPOST /workspaces/${workspaceId}/invites. Even if the backend rejects it, the UI is exposing a disallowed action; if it does not, this becomes a real entitlement bypass. Move the invite control inside the gate or block the handler whencurrentPlanlacks access.Also applies to: 202-247
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx around lines 121 - 130, The Invite member control is rendered outside PlanGate allowing free-plan users to open the invite flow; either move the Button and associated dialog into the existing PlanGate wrapper or add an explicit entitlement check in the invite handler to block actions when currentPlan lacks invites. Concretely, relocate the Button that calls setInviteStatus/setInviteError/setInviteOpen (and the invite dialog component) so they are rendered only inside PlanGate, or add a guard at the start of the invite submit function (the code that calls POST /workspaces/${workspaceId}/invites) that checks currentPlan/hasInvitePermission and returns early (and optionally sets an error) if the plan does not permit invites.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/dashboard/app/`(dashboard)/operator/page.tsx:
- Around line 31-34: The page currently masks non-403 failures from
useApi('/admin/customers') and useApi('/admin/alerts') by falling back to
customers ?? [] and alerts ?? [], so customer/alert fetch errors
(customersError, alertsError) never surface to the page-level ErrorState
(kpisError only). Update the page-level data handling to detect customersError
and alertsError (except allowed 403 cases) and propagate them into the existing
ErrorState retry path (or render a distinct error state) instead of defaulting
to empty arrays; locate the uses of useApi and the variables customers, alerts,
customersError, alertsError, kpisError, and ErrorState to implement this change.
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx:
- Around line 36-37: The profile fetch error is being swallowed and treated as
"No workspace found"; update the component logic that uses useApi<UserProfile>
and pickWorkspaceId so that if profileError exists you render or surface an
error state before checking !workspaceId: check the returned profileError from
useApi (and respect profileLoading) and short-circuit to an error UI/ retry
action instead of proceeding to compute workspaceId via
pickWorkspaceId(profile?.workspaces); ensure the error branch appears before the
existing !workspaceId branch and use the same error UI/handler used elsewhere
for consistency.
- Around line 147-182: The code casts m.role directly to MemberRole which lets
unnormalized backend values (e.g. "ADMIN") propagate into ROLE_BADGE and the
select, causing wrong UI; call normalizeRole(m.role) once (assign to role)
inside the roster.map callback and use that normalized role for ROLE_BADGE, the
owner check (role !== 'owner'), the select value mapping, and the aria-label;
ensure handleRoleChange still receives the intended InviteRole when onChange
fires.
In `@cloud/dashboard/app/`(dashboard)/team/page.tsx:
- Around line 69-73: Normalize roles before counting KPIs: instead of comparing
raw m.role in the Kpi value expressions, call normalizeRole(m.role) (from
normalizeRole in team.ts) and use that normalized string when filtering for
'owner'|'admin' and 'member'. Update the two roster.filter(...) expressions used
in the Kpi components so they pass each member's role through normalizeRole()
before the equality checks and then convert the final counts to strings,
ensuring case/format variants from the API are handled.
- Around line 14-15: The profile fetch failure isn't handled so a network/auth
error gets shown as "No workspace found"; update the useApi call to destructure
the error (e.g., const { data: profile, loading: profileLoading, error:
profileError } = useApi<UserProfile>('/users/me')) and, inside the page
component before checking !workspaceId, render the ErrorState when profileError
is truthy. Ensure you still compute workspaceId via
pickWorkspaceId(profile?.workspaces) and keep the existing loading fallback
(profileLoading) intact.
- Around line 36-44: The leaderboard currently filters members by last_sync_at
!== null, causing disagreement with computeTeamAggregate() which uses
isMemberActive(); update the leaderboard generation to reuse the shared activity
helper (isMemberActive) when filtering the roster so only truly active members
are included before sorting, keep the same sort logic (convert last_sync_at to
timestamps) and reference the leaderboard variable and roster array as the
location to change to ensure KPIs and leaderboard match computeTeamAggregate().
In `@cloud/dashboard/src/components/brain/ClearDemoButton.tsx`:
- Around line 40-46: The delayed setTimeout in ClearDemoButton that calls
setOpen(false), setResult(null) and window.location.reload() should not
hard-reload the current URL because that can land the user back on the deleted
brain detail; instead detect when the API result indicates the brain row was
deleted and perform an explicit redirect (e.g. use Next.js useRouter().push to
the brains list or a safe parent route) rather than window.location.reload(),
and replace the inline setTimeout with a cancellable timer (store the returned
id in a ref and add a useEffect cleanup to clearTimeout(timerRef.current) on
unmount) so navigating away during the 900ms delay cannot force-reload another
page; update the block that currently uses setTimeout/ setResult/ setOpen/
window.location.reload to implement these changes.
In `@cloud/dashboard/src/components/operator/CustomerRow.tsx`:
- Around line 58-71: The isForbidden helper currently overmatches by checking
loose substrings; update it to detect only true 403/operator-allowlist cases by
using structured status info from useApi (e.g., accept the error object or
status code instead of a freeform message) and match the precise backend detail
phrase ("Operator access" or the exact message the backend sets) or status ===
403; remove broad checks for 'operator' and 'not authorized' so 401s or
unrelated 500 messages don't render the no-access state (adjust call sites that
pass a string to pass the structured error/status to isForbidden or add an
overloaded helper that reads error.status).
In `@cloud/dashboard/src/types/api.ts`:
- Around line 65-72: The shared UserProfile interface is inaccurate for the
/users/me response: make the drifting fields optional or create a dedicated
backend response type so the compiler catches missing fields; specifically
update the UserProfile (or add a new UsersMeResponse) to mark email and plan as
optional (email?: string, plan?: string) or replace usage of UserProfile at
callers of /users/me with the new UsersMeResponse to reflect the real backend
payload and avoid hiding missing-field errors.
In `@cloud/dashboard/tests/OperatorPage.test.tsx`:
- Around line 17-19: The test helper function wrap currently types its parameter
as React.ReactNode without importing React types; update the file to import
ReactNode from 'react' and change the signature to use the imported ReactNode
(i.e., import { ReactNode } from 'react' and use wrap(children: ReactNode) =>
...) so the type resolves under stricter TypeScript/test typechecking
(referencing the wrap function and ReactNode type).
---
Outside diff comments:
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx:
- Around line 121-130: The Invite member control is rendered outside PlanGate
allowing free-plan users to open the invite flow; either move the Button and
associated dialog into the existing PlanGate wrapper or add an explicit
entitlement check in the invite handler to block actions when currentPlan lacks
invites. Concretely, relocate the Button that calls
setInviteStatus/setInviteError/setInviteOpen (and the invite dialog component)
so they are rendered only inside PlanGate, or add a guard at the start of the
invite submit function (the code that calls POST
/workspaces/${workspaceId}/invites) that checks currentPlan/hasInvitePermission
and returns early (and optionally sets an error) if the plan does not permit
invites.
🪄 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
Run ID: 842f31d3-f9ec-4c9c-9a81-84060ac5795f
📒 Files selected for processing (15)
cloud/dashboard/app/(dashboard)/brain/page.tsxcloud/dashboard/app/(dashboard)/operator/page.tsxcloud/dashboard/app/(dashboard)/team/members/page.tsxcloud/dashboard/app/(dashboard)/team/page.tsxcloud/dashboard/src/components/brain/ClearDemoButton.tsxcloud/dashboard/src/components/operator/CustomerRow.tsxcloud/dashboard/src/components/team/TeamLeaderboard.tsxcloud/dashboard/src/lib/fixtures/mock-operator.tscloud/dashboard/src/lib/fixtures/mock-team.tscloud/dashboard/src/lib/team.tscloud/dashboard/src/types/api.tscloud/dashboard/tests/ClearDemoButton.test.tsxcloud/dashboard/tests/OperatorPage.test.tsxcloud/dashboard/tests/TeamLeaderboard.test.tsxcloud/dashboard/tests/team.test.ts
💤 Files with no reviewable changes (2)
- cloud/dashboard/src/lib/fixtures/mock-operator.ts
- cloud/dashboard/src/lib/fixtures/mock-team.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
- useApi: expose errorStatus so callers can branch on HTTP status, not freeform message substrings. - CustomerRow.isForbidden: detect 403 via structured status; fall back to exact "Operator access" detail phrase for network-only failures. Keep a legacy string overload for back-compat. - Operator page: plumb errorStatus into isForbidden; surface non-403 customer/alert fetch failures as retryable ErrorState instead of falling through to empty lists. - Team overview: surface profileError before the empty-workspace state, reuse isMemberActive() for the leaderboard filter, normalize roles before KPI counts. - Team members: surface profileError, normalize role inside the roster map (no more admin-shown-as-member flicker), guard the invite submission against non-team plans (defense-in-depth alongside the existing PlanGate). - ClearDemoButton: if the brain row itself was deleted, router.push to /dashboard instead of reloading the same URL (which would render "Brain not found"); store the success timer in a ref and clear on unmount. - types/api.ts: mark UserProfile.email and UserProfile.plan optional to match the real /users/me payload, so callers get compiler warnings instead of silent undefineds. - Tests: import ReactNode in OperatorPage.test, rewrite isForbidden tests for the structured signature, mock next/navigation in ClearDemoButton.test so useRouter resolves under vitest. Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
- Extract shared formatRelativeAgo into src/lib/format.ts; drop the two duplicate formatAgo copies in operator/page.tsx and CustomerRow.tsx. - Extract readApiError into src/lib/errors.ts; drop duplicate in ClearDemoButton.tsx and team/members/page.tsx. - formatSyncAgo now delegates to formatRelativeAgo with the 'never synced' fallback, keeping the public API + test contract. - Trim meta-narration comments from lib/team.ts. All 78 vitest tests green; tsc --noEmit clean. Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx:
- Around line 48-49: The shared rowError state can stay visible after a later
successful operation; update the success paths in the member action handlers
(e.g., the functions that call refetch such as the removeMember and
changeMemberRole handlers referenced alongside busyUserId and setBusyUserId) to
explicitly clear rowError by calling setRowError(null) after a successful
refetch (or immediately after the operation completes successfully), or
alternatively switch to a per-row error map keyed by member id to scope errors
to a specific row; locate the handlers that set busyUserId, call refetch, and
currently only clear rowError at try start and add the explicit
setRowError(null) on the success branch.
In `@cloud/dashboard/src/components/brain/ClearDemoButton.tsx`:
- Around line 39-46: The component calls onCleared unconditionally after the
clear-demo API returns, which triggers the parent's refetch and can 404 if the
brain was deleted; update ClearDemoButton.tsx to compute brainDeleted from
res.data.by_table?.brains and only invoke onCleared(res.data) when brainDeleted
is false (i.e., skip the parent refetch when brainDeleted is true), leaving
setResult and the existing redirect/timer logic intact.
In `@cloud/dashboard/src/components/operator/CustomerRow.tsx`:
- Around line 34-37: The MRR value rendered in CustomerRow is shown as a raw
number string (customer.mrr_usd) and needs thousands separators for readability;
update the rendering in the JSX where the <div> displays ${customer.mrr_usd} to
format the numeric value using Number(customer.mrr_usd).toLocaleString() (or
customer.mrr_usd.toLocaleString() if already a number) and prepend the dollar
sign, keeping the font-mono styling and the same element that currently contains
the MRR.
In `@cloud/dashboard/src/lib/team.ts`:
- Around line 33-46: Role strings may contain surrounding whitespace so
normalizeRole and the workspace owner detection in pickWorkspaceId should trim
before lowercasing; update normalizeRole to compute const r = (role ||
'').trim().toLowerCase() and adjust pickWorkspaceId's owned lookup to use
(w.role || '').trim().toLowerCase() when comparing to 'owner' so values like '
owner ' or 'ADMIN ' are handled correctly.
In `@cloud/dashboard/tests/ClearDemoButton.test.tsx`:
- Around line 24-79: Add a new test in ClearDemoButton.test.tsx named like
"redirects to /dashboard when brain row was deleted" that uses
postMock.mockResolvedValue({ data: { deleted: 3, by_table: { lessons: 3, brains:
1 } } }), renders <ClearDemoButton brainId="brain-1" />, opens the dialog and
clicks the confirm button the same way other tests do (using screen.getAllByRole
and clicking the last match), then await waitFor(() =>
expect(pushMock).toHaveBeenCalledWith('/dashboard')); ensure you reference
postMock and pushMock and follow the existing test patterns around
Render/FireEvent/WaitFor with ClearDemoButton.
🪄 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
Run ID: 76e0b926-fb4e-4dba-a063-be247fb337b0
📒 Files selected for processing (13)
cloud/dashboard/app/(dashboard)/operator/page.tsxcloud/dashboard/app/(dashboard)/settings/page.tsxcloud/dashboard/app/(dashboard)/team/members/page.tsxcloud/dashboard/app/(dashboard)/team/page.tsxcloud/dashboard/src/components/brain/ClearDemoButton.tsxcloud/dashboard/src/components/operator/CustomerRow.tsxcloud/dashboard/src/hooks/useApi.tscloud/dashboard/src/lib/errors.tscloud/dashboard/src/lib/format.tscloud/dashboard/src/lib/team.tscloud/dashboard/src/types/api.tscloud/dashboard/tests/ClearDemoButton.test.tsxcloud/dashboard/tests/OperatorPage.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (8)
cloud/dashboard/src/hooks/useApi.ts (1)
5-12: LGTM!The
errorStatusaddition is well-implemented:
- Correctly uses
?? nullto handle undefined (network errors without a response)- Properly resets both
erroranderrorStatusat fetch start- JSDoc comment accurately documents the null case for network errors
Also applies to: 26-26, 39-48, 59-59
cloud/dashboard/src/lib/format.ts (1)
1-21: LGTM!Clean, testable utility with good defensive handling:
- Gracefully handles null/undefined/invalid inputs via fallback
- Future timestamps return 'just now' rather than negative values
- The
nowparameter enables deterministic testingcloud/dashboard/app/(dashboard)/settings/page.tsx (1)
53-53: LGTM!Consistent fallback handling for the now-optional
display_name.cloud/dashboard/src/lib/errors.ts (1)
1-11: LGTM!Well-designed error extraction utility that:
- Properly uses
axios.isAxiosError()for type-safe Axios error detection- Handles the full fallback chain:
detail→message→fallback- Correctly handles non-Error thrown values (e.g., strings, objects)
cloud/dashboard/app/(dashboard)/team/members/page.tsx (1)
26-70: LGTM on the API integration and error handling flow.The profile/members loading, error handling, and empty-state branches are well-structured. Past review feedback about surfacing
profileErrorbefore the!workspaceIdcheck has been addressed.cloud/dashboard/src/components/operator/CustomerRow.tsx (1)
50-69: LGTM on theisForbiddenhelper.The implementation correctly addresses the prior feedback:
- Uses structured
status === 403as the primary detection- String fallback is narrowly scoped to the exact backend phrase (
"Operator access")- Network failures without status codes fall through appropriately
cloud/dashboard/app/(dashboard)/team/page.tsx (2)
18-57: LGTM on the API integration and leaderboard logic.Past review feedback has been properly addressed:
- Profile error is surfaced before the
!workspaceIdcheck- Leaderboard uses
isMemberActive()for consistent filtering withcomputeTeamAggregate()- Role normalization is applied in KPI calculations
The defensive null handling in the sort (lines 54-55) is reasonable even though
isMemberActivefilters out nulllast_sync_atvalues.
78-90: LGTM on the KPI calculations.Role normalization is correctly applied via
normalizeRole(m.role)before comparing against'owner','admin', and'member', addressing the prior feedback about backend role case variants.
- TeamMembersPage: clear rowError on success after refetch in remove/role handlers - ClearDemoButton: skip onCleared when brain row was deleted (parent refetch would 404) - CustomerRow: format MRR with toLocaleString for thousands separators - team.ts: trim role strings before lowercasing in normalizeRole + pickWorkspaceId - ClearDemoButton.test: add coverage for /dashboard redirect when brain deleted Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Addressed all 5 CR actionables in ac8f9a3:
vitest green (5/5 ClearDemoButton tests). @coderabbitai full review |
|
Triggering a full review now to pick up the ac8f9a3 changes. ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx:
- Around line 73-90: handleInvite currently uses inviteEmail directly which
allows whitespace-only values and sends untrimmed emails to the backend; update
handleInvite to trim the input first (e.g., const email = inviteEmail.trim()),
use the trimmed value for the disabled/validation check and for the POST payload
and error messages, and clear the input with setInviteEmail('') as before; apply
the same trim-and-validate change to the other invite-related handler later in
the file (the second invite flow around lines 229-246) so all invite submissions
reject whitespace-only input and send only trimmed emails.
- Around line 48-49: Replace the single shared busyUserId/rowError state with
per-member maps so concurrent row actions don't clobber each other: change
busyUserId/setBusyUserId to something like busyById (useState<Record<string,
boolean>> or Set<string>) and rowError/setRowError to rowErrorById
(useState<Record<string, string | null>>) then update the mutation handlers (the
delete and role-change functions that currently call setBusyUserId and
setRowError in their try/finally blocks) to set busyById[id] = true before the
request and set busyById[id] = false in their respective finally, and set
rowErrorById[id] on error instead of a shared rowError; ensure reads (disabled
props, spinners, error messages) use busyById[id] and rowErrorById[id] so each
row tracks its own loading/error state.
In `@cloud/dashboard/src/components/operator/CustomerRow.tsx`:
- Line 12: The expression "const tier = (customer.plan === 'pro' ? 'cloud' :
customer.plan) as PlanTier" unsafely asserts customer.plan is a PlanTier; change
it to explicitly narrow and provide a safe fallback: map 'pro' to 'cloud', then
check customer.plan against the allowed PlanTier values (e.g., via a Set or an
array of PlanTier members) and use that value if valid, otherwise assign a
default tier (e.g., 'free' or whatever the app default is); update the variable
named "tier" in CustomerRow.tsx accordingly so no "as PlanTier" cast is used.
In `@cloud/dashboard/tests/ClearDemoButton.test.tsx`:
- Around line 68-84: Add an assertion to the existing test for ClearDemoButton
to verify that the onCleared callback is NOT invoked when the brain row deletion
response indicates the brain was deleted: after simulating the two clicks and
before/after the waitFor that asserts pushMock was called, assert that the
onCleared mock (the prop passed into ClearDemoButton) has not been called (e.g.,
expect(onClearedMock).not.toHaveBeenCalled()). Ensure you reference the same
onCleared mock instance used when rendering ClearDemoButton and keep the
existing postMock and pushMock behavior.
🪄 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
Run ID: 8c4cd983-c4e2-4093-9ccd-cfc624d005a9
📒 Files selected for processing (5)
cloud/dashboard/app/(dashboard)/team/members/page.tsxcloud/dashboard/src/components/brain/ClearDemoButton.tsxcloud/dashboard/src/components/operator/CustomerRow.tsxcloud/dashboard/src/lib/team.tscloud/dashboard/tests/ClearDemoButton.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (10)
cloud/dashboard/src/components/operator/CustomerRow.tsx (3)
5-9: Health-style mapping is clean and exhaustive.Typed keying against
AdminCustomer['health']keeps the badge style contract tight and prevents drift.
34-45: MRR/users/brains presentation looks solid.The numeric layout and MRR thousands-separator formatting improve operator scanability without adding complexity.
58-70:isForbiddennow matches the intended 403 behavior.The structured-status-first check plus narrow legacy message fallback resolves prior overmatching risk cleanly.
cloud/dashboard/src/components/brain/ClearDemoButton.tsx (3)
27-33: LGTM! Proper timer cleanup on unmount.The useEffect cleanup prevents the timer from force-navigating after the component unmounts, which correctly addresses the previous concern about navigating away during the 900ms delay.
35-65: Well-structured handler with proper edge case handling.The implementation correctly:
- Skips
onClearedwhen the brain is deleted (line 46-48), preventing a 404 from the parent's refetch- Redirects to
/dashboardinstead of reloading when the brain row was removed (line 53-54)- Clears any existing timer before scheduling a new one (line 49)
- Uses defensive optional chaining on
by_table?.brainseven though the backend guarantees the key
67-115: LGTM!The dialog properly resets state on open, prevents closing while busy, and disables the confirm button after success to prevent double-submission. The semantic dialog structure supports accessibility.
cloud/dashboard/tests/ClearDemoButton.test.tsx (4)
5-22: LGTM! Clean mock setup.The mock setup correctly intercepts the API client and router. Using
beforeEachto reset mocks ensures test isolation.
33-53: LGTM!The test correctly verifies the happy path: API call to the correct endpoint,
onClearedcallback invocation, and success banner display. The mock response without an explicitbrainskey is valid since the component uses nullish coalescing.
55-66: LGTM!Good coverage for the singular/plural copy edge case.
86-97: LGTM!Correctly verifies error banner rendering when the API rejects.
| const handleInvite = async () => { | ||
| if (!planAllowsInvites) { | ||
| setInviteError('Invites require the Team plan. Upgrade to enable this.') | ||
| return | ||
| } | ||
| setInviting(true) | ||
| setInviteStatus(null) | ||
| // TODO(backend): POST /workspaces/{id}/invites — fake for now | ||
| await new Promise((r) => setTimeout(r, 600)) | ||
| setInviteStatus(`Invite sent to ${inviteEmail}`) | ||
| setInviteEmail('') | ||
| setInviting(false) | ||
| setInviteError(null) | ||
| try { | ||
| const res = await api.post<InviteResponse>( | ||
| `/workspaces/${workspaceId}/invites`, | ||
| { email: inviteEmail, role: inviteRole }, | ||
| ) | ||
| setInviteStatus(`Invite sent to ${res.data.email}`) | ||
| setInviteEmail('') | ||
| refetch() | ||
| } catch (err) { | ||
| setInviteError(readApiError(err, 'Could not send invite. Try again.')) |
There was a problem hiding this comment.
Trim the invite email before validating and POSTing it.
Whitespace-only input currently passes the disabled={!inviteEmail} check, and leading/trailing spaces are sent to /invites unchanged. That makes an easy user typo hit the backend instead of being rejected locally.
Suggested fix
const handleInvite = async () => {
+ const email = inviteEmail.trim()
+
if (!planAllowsInvites) {
setInviteError('Invites require the Team plan. Upgrade to enable this.')
return
}
+ if (!email) {
+ setInviteError('Enter a valid email address.')
+ return
+ }
setInviting(true)
setInviteStatus(null)
setInviteError(null)
try {
const res = await api.post<InviteResponse>(
`/workspaces/${workspaceId}/invites`,
- { email: inviteEmail, role: inviteRole },
+ { email, role: inviteRole },
)
setInviteStatus(`Invite sent to ${res.data.email}`)
setInviteEmail('')
refetch()
@@
- disabled={inviting || !inviteEmail}
+ disabled={inviting || !inviteEmail.trim()}Also applies to: 229-246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx around lines 73 - 90,
handleInvite currently uses inviteEmail directly which allows whitespace-only
values and sends untrimmed emails to the backend; update handleInvite to trim
the input first (e.g., const email = inviteEmail.trim()), use the trimmed value
for the disabled/validation check and for the POST payload and error messages,
and clear the input with setInviteEmail('') as before; apply the same
trim-and-validate change to the other invite-related handler later in the file
(the second invite flow around lines 229-246) so all invite submissions reject
whitespace-only input and send only trimmed emails.
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Addressed CodeRabbit round 4 feedback in 6edd193:
Previously resolved (verified on HEAD): profileError handling on team pages, structured 403 in All 79 dashboard tests pass. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cloud/dashboard/app/(dashboard)/team/members/page.tsx (1)
50-51:⚠️ Potential issue | 🟠 MajorKeep row errors keyed by member instead of in one shared banner.
busyUserIdsnow supports concurrent row actions, butrowErroris still global. If one row fails and another row later succeeds, Line 119 or Line 133 clears the shared message and the failing row loses its error state. Track errors byuser_id, the same way you track busy state.Suggested direction
- const [rowError, setRowError] = useState<string | null>(null) + const [rowErrors, setRowErrors] = useState<Record<string, string | null>>({}) + const setRowErrorFor = (userId: string, message: string | null) => { + setRowErrors((prev) => { + const next = { ...prev } + if (message) next[userId] = message + else delete next[userId] + return next + }) + } - setRowError(null) + setRowErrorFor(m.user_id, null) ... - setRowError(readApiError(err, 'Could not remove member.')) + setRowErrorFor(m.user_id, readApiError(err, 'Could not remove member.'))Render
rowErrors[m.user_id]on the affected row, or aggregate multiple row errors without destructive clears.Also applies to: 112-123, 127-137, 156-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx around lines 50 - 51, The component currently uses a single shared rowError state while busyUserIds tracks per-user concurrency; change rowError to a per-user map (e.g., rowErrors: Record<string, string | null> or Map<string,string>) and replace setRowError calls with updates that set/clear only the specific user's error (use m.user_id as the key), update any error-clearing logic in the handlers that currently call setRowError(null) to instead clear only that user's entry, and render the error for each row using rowErrors[m.user_id] (or rowErrors.get(m.user_id)) so failures remain tied to the affected member and are not overwritten by other row actions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx:
- Around line 35-40: The page currently shows a full-screen spinner whenever
membersLoading is true (which flips on every refetch), so change the
blocking-spinner condition to only show when loading and there is no existing
data: use membersLoading && !members (or members == null) instead of
membersLoading alone (locate the useApi<TeamMember[]> call and the spinner
render that references membersLoading). Apply the same pattern to the other
places flagged (replace checks like XLoading with XLoading && !XData for each
useApi response) so background refetches keep the existing UI while still
showing a spinner only on the initial load.
---
Duplicate comments:
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx:
- Around line 50-51: The component currently uses a single shared rowError state
while busyUserIds tracks per-user concurrency; change rowError to a per-user map
(e.g., rowErrors: Record<string, string | null> or Map<string,string>) and
replace setRowError calls with updates that set/clear only the specific user's
error (use m.user_id as the key), update any error-clearing logic in the
handlers that currently call setRowError(null) to instead clear only that user's
entry, and render the error for each row using rowErrors[m.user_id] (or
rowErrors.get(m.user_id)) so failures remain tied to the affected member and are
not overwritten by other row actions.
🪄 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
Run ID: c20c8ca3-1047-40be-80e9-50a9b43d4937
📒 Files selected for processing (3)
cloud/dashboard/app/(dashboard)/team/members/page.tsxcloud/dashboard/src/components/operator/CustomerRow.tsxcloud/dashboard/tests/ClearDemoButton.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (9)
cloud/dashboard/tests/ClearDemoButton.test.tsx (6)
5-22: Strong mock isolation and cleanup setup.The API/router mocks are scoped cleanly, and resetting both mocks in
beforeEachkeeps tests independent and deterministic.
25-31: Good pre-confirmation guard test.This correctly verifies that opening the dialog alone does not trigger the destructive API call.
33-53: Happy-path flow is covered well.The test validates request path, callback payload, and user-visible success messaging in one coherent flow.
55-66: Nice singular-copy regression check.Explicitly asserting the singular success text is a solid UX copy safeguard.
68-89: Deleted-brain branch assertions look correct.This test now captures both required outcomes: redirect to
/dashboardand skippingonClearedto avoid stale-parent refetch behavior.
91-102: Error-state behavior is validated clearly.The rejection path assertion ensures users receive a surfaced failure message when the API call fails.
cloud/dashboard/src/components/operator/CustomerRow.tsx (3)
11-21: Good hardening of plan normalization and narrowing.Line 20 correctly avoids unsafe assertions and guarantees a valid
PlanBadgetier even with unexpected backend plan strings.
43-44: MRR formatting update looks correct.Line 43 now renders monetary values with locale separators, which improves scanability in operator lists.
66-78: 403 detection logic is now appropriately strict.Line 74 correctly keys off HTTP status, and Lines 72/77 keep legacy message fallback narrowly scoped to the operator-access phrase.
- Resolve 5 file conflicts preserving dashboard team/operator wiring (TeamMember/InviteResponse types, TeamLeaderboard, CustomerRow, isForbidden 403 handling, per-row busy state + role-change UI). - Align UserProfile type with backend: user_id (required) + optional email/plan/created_at/workspaces[]. Address CodeRabbit round-5 on cloud/dashboard/app/(dashboard)/team/members/page.tsx: - Don't blank the page on background refetch: use initialMembersLoad = !members && !membersError instead of flipping membersLoading on every refetch (Major). - Key rowError by user_id: rowErrors: Record<string, string|null> with setRowErrorFor(userId, msg). Render per-row inline instead of a shared banner that gets stomped by concurrent row actions (Major). Tests: cloud/tests 120 pass (1 deselected — pre-existing missing cloud/scripts/export_ab_proof.py in origin/main), dashboard vitest 79/79 pass, team+operator RLS suites 26/26 pass.
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Round-5 CR addressed + rebased on main (PR #44). Merge resolution — 5 conflict files, team/operator RLS isolation preserved:
CR round-5 items fixed (
Tests: cloud pytest 120/120 (1 deselected — pre-existing missing |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review — fixes pushed since last review, please re-verify |
|
✅ Actions performedReview triggered.
|
|
@coderabbitai review — CR Pro active, please re-verify. |
|
✅ Actions performedReview triggered.
|
Reconcile with PR #46 outcome-first dashboard pivot: - team/page.tsx: adopt TeamStats data model (corrections_week, avg_delta_pct, rules_graduated_30d, active_brains) from PR #46. Preserve operator bypass, profile error retry, pickWorkspaceId helper, TeamEmptyState from PR #34. - team/members/page.tsx: combine PLANS import (PR #34 gates invites on plan rank) with isOperatorEmail bypass (PR #46).
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Rebased on outcome-first dashboard + round-6 CR verification. Merge reconciliation with PR #46 (outcome-first pivot):
Round-6 CR items verified as already addressed:
Tests:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
@coderabbitai review — rebased on latest main, all prior fixes applied |
|
✅ Actions performedReview triggered.
|
|
Round-N: per-user-keyed rowErrors (no more cross-row stomping) |
|
@coderabbitai review |
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx:
- Around line 82-85: Validate and normalize profile?.plan before casting:
replace the blind cast done when computing currentPlan with logic that reads
profile?.plan, trims and lowercases it, checks whether the normalized value is a
valid key in PLANS, and only then assigns it to currentPlan; otherwise fall back
to 'free'. Update the computation used by planAllowsInvites (and any use passed
into PlanGate) to rely on this validated currentPlan so legacy/unknown strings
(e.g., 'pro', whitespace, null) cannot flow through. Apply the same
normalization/validation pattern to other occurrences mentioned
(settings/page.tsx, team/page.tsx, self-healing/page.tsx, meta-rules/page.tsx,
billing/page.tsx) so all uses of currentPlan/planAllowsInvites/PlanGate consume
only known PlanTier values.
🪄 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
Run ID: d6066edd-79c7-416e-a79c-f0ee48e7858f
📒 Files selected for processing (5)
cloud/dashboard/app/(dashboard)/operator/page.tsxcloud/dashboard/app/(dashboard)/settings/page.tsxcloud/dashboard/app/(dashboard)/team/members/page.tsxcloud/dashboard/app/(dashboard)/team/page.tsxcloud/dashboard/src/types/api.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
| const currentPlan = (profile?.plan?.toLowerCase() ?? 'free') as PlanTier | ||
| const memberList: Member[] = members ?? [] | ||
| const planAllowsInvites = | ||
| (PLANS[currentPlan]?.rank ?? 0) >= PLANS.team.rank | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 --glob 'cloud/dashboard/app/**/page.tsx' 'profile\??\.plan|as PlanTier|PLANS\[currentPlan\]|<PlanGate'
fd -i 'PlanBadge.tsx' cloud/dashboard/src/components/brain -t f -x sed -n '1,220p' {}Repository: Gradata/gradata
Length of output: 8996
Guard /users/me.plan before treating it as a PlanTier.
The cast on line 82 coerces a raw backend string to PlanTier without validation. Since profile?.plan is string | null | undefined, unexpected values (legacy tiers like pro, whitespace, or other strings) will make planAllowsInvites incorrect and flow invalid data into PlanGate at line 169. Normalize the plan string to a known tier and fall back to 'free' before using it with PLANS or PlanGate.
This pattern appears in multiple dashboard pages: settings/page.tsx:28, team/page.tsx:53, self-healing/page.tsx:91, meta-rules/page.tsx:14, and billing/page.tsx:28. All should validate backend plan data before casting.
Suggested fix
- const currentPlan = (profile?.plan?.toLowerCase() ?? 'free') as PlanTier
+ const rawPlan = profile?.plan?.trim().toLowerCase() ?? 'free'
+ const currentPlan: PlanTier =
+ rawPlan === 'pro'
+ ? 'cloud'
+ : rawPlan === 'free' || rawPlan === 'cloud' || rawPlan === 'team' || rawPlan === 'enterprise'
+ ? rawPlan
+ : 'free'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const currentPlan = (profile?.plan?.toLowerCase() ?? 'free') as PlanTier | |
| const memberList: Member[] = members ?? [] | |
| const planAllowsInvites = | |
| (PLANS[currentPlan]?.rank ?? 0) >= PLANS.team.rank | |
| const rawPlan = profile?.plan?.trim().toLowerCase() ?? 'free' | |
| const currentPlan: PlanTier = | |
| rawPlan === 'pro' | |
| ? 'cloud' | |
| : rawPlan === 'free' || rawPlan === 'cloud' || rawPlan === 'team' || rawPlan === 'enterprise' | |
| ? rawPlan | |
| : 'free' | |
| const planAllowsInvites = | |
| (PLANS[currentPlan]?.rank ?? 0) >= PLANS.team.rank | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx around lines 82 - 85,
Validate and normalize profile?.plan before casting: replace the blind cast done
when computing currentPlan with logic that reads profile?.plan, trims and
lowercases it, checks whether the normalized value is a valid key in PLANS, and
only then assigns it to currentPlan; otherwise fall back to 'free'. Update the
computation used by planAllowsInvites (and any use passed into PlanGate) to rely
on this validated currentPlan so legacy/unknown strings (e.g., 'pro',
whitespace, null) cannot flow through. Apply the same normalization/validation
pattern to other occurrences mentioned (settings/page.tsx, team/page.tsx,
self-healing/page.tsx, meta-rules/page.tsx, billing/page.tsx) so all uses of
currentPlan/planAllowsInvites/PlanGate consume only known PlanTier values.
Summary
Replaces three pages of mock fixtures in the Next.js dashboard with live backend calls and adds a destructive demo-data control on the brain page.
/team,/team/members): hits the real/workspaces/{id}/members,/workspaces/{id}/invites,PATCH/DELETE /workspaces/{id}/members/{uid}endpoints fromcloud/app/routes/team.py. Workspace id is picked fromprofile.workspaces(owner preferred, else first)./operator): hits/admin/global-kpis,/admin/customers,/admin/alertsfromcloud/app/routes/operator.py. Backend already gates onrequire_operator; the frontend surfaces a friendly empty state when any endpoint returns a 403, and stops hard-coding the allowlist client-side./brain): newClearDemoButtoncomponent posts to/brains/{id}/clear-demowith a confirmation dialog, shows deleted counts, then reloads the page.Pure helpers and presentational components were extracted into
src/lib/team.ts,src/components/team/, andsrc/components/operator/so they can be unit-tested without importing the supabase-dependent api module.The
mock-team.tsandmock-operator.tsfixtures are deleted.Changes
cloud/dashboard/src/types/api.ts— addTeamMember,InviteResponse,AdminGlobalKpis,AdminCustomer,AdminAlert,ClearDemoResponse, and optionalUserProfile.workspaces.cloud/dashboard/src/lib/team.ts— new pure helpers.cloud/dashboard/src/components/team/TeamLeaderboard.tsx— extracted.cloud/dashboard/src/components/operator/CustomerRow.tsx— extracted (includesisForbidden).cloud/dashboard/src/components/brain/ClearDemoButton.tsx— new.cloud/dashboard/tests/{team,TeamLeaderboard,OperatorPage,ClearDemoButton}.test.{ts,tsx}— 35 new vitest cases.Known gaps / follow-ups
team.pycurrently exposes no GET for listing pending invites and no DELETE for cancelling them. The Members page covers the add + remove-member + role-change flows but cannot render or revoke outstanding invites yet. Tracked for a follow-up once those endpoints land.UserProfile(noemail/planfields,workspacesarray) and the frontendUserProfiletype (hasemail/plan). I left that untouched since multiple other pages depend on the current TS shape — scoping the fix out.Test plan
npm run test:run— 76/76 passing (41 previous + 35 new).npm run build— static export succeeds, TypeScript strict clean, noany./team,/team/members,/operator,/brain?id=…against a live Railway backend once merged./operatorrenders the "only visible to the Gradata team" fallback for non-operator accounts.Remove demo databutton deletes demo rows and reloads the page.Generated with Gradata