test(settings): backfill unit tests for team / billing / notifications panels (#1870)#2089
Conversation
…s panels (tinyhumansai#1870) Issue tinyhumansai#1870 — group 2 of the i18n coverage gap. Adds per-component RTL + Vitest suites for the eight settings panels the i18n PR (tinyhumansai#1518) surfaced as untested: - BillingPanel (4 tests) — auto-open flow, error state, re-open click, back-navigation wiring. - NotificationsPanel (6 tests) — DND skeleton/toggle/rollback, per-category toggles, redux preference seeding. - NotificationRoutingPanel (6 tests) — pipeline stats card, per-provider load + retry hint, threshold formatting, setNotificationSettings payload shape. - MessagingPanel (5 tests) — default-channel selector, loading + error states, ChannelSetupModal open/close, updatePreferences wiring. - TeamPanel (8 tests) — list/active badge, create/join/switch/leave, manage navigation, localized error fallback. - TeamMembersPanel (7 tests) — admin vs. non-admin affordances, role change + remove confirmations, empty state, route-context teamId resolution. - TeamInvitesPanel (6 tests) — code rendering with state badges, generate/revoke/copy flows, admin gating, empty state. - TeamManagementPanel (8 tests) — admin-only entry, not-found + access- denied branches, member/invite sub-route navigation, edit-name + delete confirm modals. All 50 new tests pass; full panels suite is 230/26. `pnpm compile` clean and ESLint clean on the new files.
📝 WalkthroughWalkthroughThis PR adds comprehensive test suites for eight settings panel components: Billing, Messaging, NotificationRouting, Notifications, TeamInvites, TeamManagement, TeamMembers, and TeamPanel. Tests verify UI rendering, user interactions, async state flows, and API integration using mocked dependencies and React Testing Library assertions. ChangesSettings Panels Test Coverage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@app/src/components/settings/panels/TeamInvitesPanel.test.tsx`:
- Around line 120-123: The test is fragile because it clicks the first "Copy
invite code" button from screen.getAllByRole; instead scope the click to the
invite row that contains the text "ACTIVE-123" so order changes won't break the
test: locate the row element for "ACTIVE-123" (e.g. via
screen.getByText('ACTIVE-123') and its closest row/container), use
testing-library's within(row) to find the button by role/name
(within(row).getByRole('button', { name: 'Copy invite code' })), then call
fireEvent.click on that button.
In `@app/src/components/settings/panels/TeamManagementPanel.test.tsx`:
- Around line 155-157: The test currently clicks the last element from
screen.findAllByRole('button', { name: 'Delete Team' }) which is fragile;
instead scope the query to the confirmation modal/dialog: locate the dialog
(e.g., via screen.findByRole('dialog') or byAccessibleName if given), then use
within(dialog).getByRole('button', { name: 'Delete Team' }) or
within(dialog).findByRole to target and click the confirm button. Replace the
array-index click with this scoped query (referencing screen.findAllByRole,
fireEvent.click and using within from `@testing-library/react` to scope to the
dialog).
🪄 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: CHILL
Plan: Pro
Run ID: c4af7aa7-8dc6-4475-ab78-1a021f694a5e
📒 Files selected for processing (8)
app/src/components/settings/panels/BillingPanel.test.tsxapp/src/components/settings/panels/MessagingPanel.test.tsxapp/src/components/settings/panels/NotificationRoutingPanel.test.tsxapp/src/components/settings/panels/NotificationsPanel.test.tsxapp/src/components/settings/panels/TeamInvitesPanel.test.tsxapp/src/components/settings/panels/TeamManagementPanel.test.tsxapp/src/components/settings/panels/TeamMembersPanel.test.tsxapp/src/components/settings/panels/TeamPanel.test.tsx
| // Active invite is the first one in the list; its Copy button is the | ||
| // first "Copy invite code" button. | ||
| const copyBtns = screen.getAllByRole('button', { name: 'Copy invite code' }); | ||
| fireEvent.click(copyBtns[0]); |
There was a problem hiding this comment.
Avoid order-coupled selection for the copy action.
Line 122 + Line 123 rely on the first matching button, which can silently target the wrong invite if list order changes. Prefer scoping the click to the row containing ACTIVE-123 so this test stays stable.
As per coding guidelines, “Keep tests deterministic: avoid real network calls, time-sensitive flakes, or hidden global state.”
🤖 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 `@app/src/components/settings/panels/TeamInvitesPanel.test.tsx` around lines
120 - 123, The test is fragile because it clicks the first "Copy invite code"
button from screen.getAllByRole; instead scope the click to the invite row that
contains the text "ACTIVE-123" so order changes won't break the test: locate the
row element for "ACTIVE-123" (e.g. via screen.getByText('ACTIVE-123') and its
closest row/container), use testing-library's within(row) to find the button by
role/name (within(row).getByRole('button', { name: 'Copy invite code' })), then
call fireEvent.click on that button.
| const buttons = await screen.findAllByRole('button', { name: 'Delete Team' }); | ||
| // The last one rendered is the confirmation button inside the modal. | ||
| fireEvent.click(buttons[buttons.length - 1]); |
There was a problem hiding this comment.
Target the confirm button via dialog scope, not array index.
Line 157 clicks the last "Delete Team" button found, which is fragile if button order changes. Scope the query to the confirmation modal/dialog and click that specific action.
As per coding guidelines, “Keep tests deterministic: avoid real network calls, time-sensitive flakes, or hidden global state.”
🤖 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 `@app/src/components/settings/panels/TeamManagementPanel.test.tsx` around lines
155 - 157, The test currently clicks the last element from
screen.findAllByRole('button', { name: 'Delete Team' }) which is fragile;
instead scope the query to the confirmation modal/dialog: locate the dialog
(e.g., via screen.findByRole('dialog') or byAccessibleName if given), then use
within(dialog).getByRole('button', { name: 'Delete Team' }) or
within(dialog).findByRole to target and click the confirm button. Replace the
array-index click with this scoped query (referencing screen.findAllByRole,
fireEvent.click and using within from `@testing-library/react` to scope to the
dialog).
Closes #1870
Summary
Issue #1870 — group 2 of the i18n coverage gap.
PR #1518 (Chinese i18n) surfaced a pre-existing testing gap: many settings panels had zero test files. This PR addresses the team / billing / notifications subset called out in the issue's suggested decomposition.
Adds per-component RTL + Vitest suites for 8 panels:
BillingPanelNotificationsPanelNotificationRoutingPanelMessagingPanelChannelSetupModalopen/close,updatePreferenceswiringTeamPanelTeamMembersPanelteamIdTeamInvitesPanelTeamManagementPanel50 new tests total. Behavior-focused per the in-repo convention — exercising RPC payload shapes, modal confirmation flows, optimistic-UI rollback, and i18n-resolved labels rather than implementation details.
Test plan
pnpm test src/components/settings/panels— 230/230 pass across 26 filespnpm compile— cleanRemaining work (tracked separately)
Per #1870, groups 3–5 are still untested and will land as separate PRs:
MemoryDebugPanel,VoiceDebugPanel,WebhooksDebugPanel,LocalModelDebugPanel,ScreenAwarenessDebugPanel,AutocompleteDebugPanel)Accounts,Conversations,Intelligence,Invites,Notifications,Webhooks)BetaBanner,ChatProviderPage,ReferralApplyStep)Summary by CodeRabbit