Repository navigation
Invite verfication for Platform Users - #25
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds PlatformGuard and SystemAdminGuard to the API and applies per-endpoint authorization; gates frontend admin UI by platform_admin role; adds multiple Nexiom architecture and agent instruction documents and updates test suites to support auth context. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Request
participant Guard as Guard\n(SystemAdminGuard / PlatformGuard)
participant Headers as Request\nHeaders/Session
participant Provider as IdentityProvider
participant Controller as Controller
Client->>Guard: canActivate(context)
Guard->>Headers: extract session from headers
Headers-->>Guard: session or null
alt no session
Guard->>Client: throw UnauthorizedException (401)
else session present
Guard->>Provider: validateSession(session)
Provider-->>Guard: user payload (includes systemRole)
alt user lacks required role
Guard->>Client: throw ForbiddenException (403)
else user has role
Guard->>Controller: attach req.user and return true
Controller->>Client: handle request
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/src/modules/users/UserList.tsx (1)
30-38: Consider renaming the map parameter to avoid shadowinguser.The
userparameter in the map callback shadows theuserfromuseAuth()at line 17. While this doesn't cause a bug in the current implementation, it could lead to confusion in future maintenance.Suggested fix
- const users: UserTableItem[] = data?.data?.map((user) => ({ - id: user.id, - name: user.name, - email: user.email, - role: user.role, - systemRole: user.systemRole, - emailVerified: user.emailVerified, + const users: UserTableItem[] = data?.data?.map((apiUser) => ({ + id: apiUser.id, + name: apiUser.name, + email: apiUser.email, + role: apiUser.role, + systemRole: apiUser.systemRole, + emailVerified: apiUser.emailVerified, status: "active" })) || [];apps/web/src/modules/users/Users.tsx (1)
116-119: Consider renaming the map parameter to avoid shadowing the authuser.The
userparameter in the map callback (line 116) shadows theuserfromuseAuth()(line 28). This could cause confusion or bugs if the auth user is needed inside the map callback in future changes.Suggested fix
- {data.map((user) => { - const displayName = user.name || 'user'; - const isInviting = invitingIds.has(user.id); + {data.map((tableUser) => { + const displayName = tableUser.name || 'user'; + const isInviting = invitingIds.has(tableUser.id); return ( - <TableRow key={user.id}> + <TableRow key={tableUser.id}>And update all references to
userwithin the map totableUser.apps/web/src/modules/tenants/TenantList.tsx (1)
126-128: Remove duplicatekeyprop onTableRow.The
key={tenant.id}onTableRow(line 128) is redundant since the wrappingFragment(line 126) already has the same key. React only needs the key on the outermost element in a list.Suggested fix
<Fragment key={tenant.id}> <TableRow - key={tenant.id} className={cn("cursor-pointer hover:bg-muted/50", expandedTenantId === tenant.id && "bg-muted/50 border-b-0")}
🤖 Fix all issues with AI agents
In `@apps/api/src/modules/auth/platform.guard.spec.ts`:
- Around line 17-33: The beforeEach in the PlatformGuard test setup is missing a
call to jest.clearAllMocks(), which can let mockAuthProvider state leak between
tests; update the beforeEach that initializes mockAuthProvider and compiles the
TestingModule to call jest.clearAllMocks() at the start of the hook (before
reassigning mockAuthProvider) so each test gets a fresh mock state for
mockAuthProvider and PlatformGuard.
In `@apps/api/src/modules/auth/platform.guard.ts`:
- Around line 26-28: The code constructs a new Headers with a cast which loses
multi-value header arrays; replace the direct new Headers(req.headers as
Record<string,string>) in platform.guard.ts with a call to the toWebHeaders(...)
helper (from src/shared/utils/headers.util.ts) to convert Express req.headers
into proper Web Headers, then pass that result into
this.authProvider.getSessionFromHeaders(headers); follow the same pattern used
in auth.controller.ts to ensure array header values are joined/preserved
correctly.
In `@apps/web/src/modules/tenants/TenantList.spec.tsx`:
- Around line 45-57: Tests for TenantList are missing assertions for role-based
UI changes; add two tests that render TenantList via the existing
renderComponent helper while mocking AuthProvider state to return platform_admin
and platform_user roles respectively, then assert that for platform_admin the
status dropdown and action menu elements (identify by their test ids or
button/role labels used in TenantList) are present, and for platform_user the
static status badge is present and the action menu/dropdown are absent; use the
same mockData and isLoading props and restore/mock AuthProvider state around
each test to isolate roles.
In `@apps/web/src/modules/tenants/TenantList.tsx`:
- Around line 161-163: The status-change menu items (DropdownMenuItem) currently
call onStatusChange but do not stop event propagation, causing the row's onClick
expansion to also fire; update the three DropdownMenuItem handlers to accept the
click event (e) and call e.stopPropagation() before invoking
onStatusChange(tenant.id, ...) so clicks on Active/Suspended/Disabled won't
bubble and trigger row expansion; reference the DropdownMenuItem elements, the
onStatusChange callback, and tenant.id when making the change.
In `@apps/web/src/pages/admin/users/UserShow.tsx`:
- Around line 71-78: Replace the spinning Send icon with a Loader2 during
loading to match the pattern used in Users.tsx: in UserShow.tsx update the
Button render logic around handleInvite/inviteLoading so that when inviteLoading
is true you render the Loader2 icon (with the same classes used for size/margin)
instead of <Send className=... animate-spin>, and keep the Send icon for the
non-loading state; ensure you reference the inviteLoading flag and the Send and
Loader2 components so the conditional swap mirrors the Users.tsx implementation.
In `@docs/nexiom_sync_module_design.md`:
- Around line 69-77: The document mixes British and American spellings;
standardize to American English by replacing all instances of "Normalised" with
"Normalized" (e.g., change Normalised_Queue to Normalized_Queue and any
"Normalised" layer names), and ensure related identifiers like Normalized_Entity
and "Normalized Layer" remain consistent across the document (also update other
occurrences such as the Outbound Gateway Layer description and any
enum/queue/entity names).
In `@docs/nexiom_tech_stack_and_repo_plan.md`:
- Around line 1-3: Replace the second-level heading "## 1. Directory Structure"
with a top-level H1 heading to satisfy markdownlint MD041; update the line
starting with "## 1. Directory Structure" to use a single leading "#" (e.g., "#
Directory Structure" or "# 1. Directory Structure") so the file's first line is
an H1.
Comment on lines
+69
to
+77
| 4. **Persist:** Save to `Normalized_Entity`. | ||
|
|
||
| 5. **Dispatch:** Send event to `Normalised_Queue`. | ||
|
|
||
| ### Layer 4: Outbound Gateway Layer (The Gatekeeper) | ||
|
|
||
| * **Role:** Maps Canonical Data to Destination Format & Validates Business Rules. | ||
|
|
||
| * **Trigger:** Consumes `Normalised_Queue`. |
Contributor
There was a problem hiding this comment.
Inconsistent spelling: "Normalised" vs "Normalized".
The document mixes British ("Normalised") and American ("Normalized") spellings. Line 53 uses "Normalized Layer" while lines 71 and 77 use Normalised_Queue. Consider standardizing to one variant throughout.
Suggested fix (standardize to American English)
- 4. **Persist:** Save to `Normalized_Entity`.
+ 4. **Persist:** Save to `Normalized_Entity`.
- 5. **Dispatch:** Send event to `Normalised_Queue`.
+ 5. **Dispatch:** Send event to `Normalized_Queue`.
...
-* **Trigger:** Consumes `Normalised_Queue`.
+* **Trigger:** Consumes `Normalized_Queue`.Also update line 127:
-* **Internal:** `Source_Replica_Queue`, `Normalised_Queue`
+* **Internal:** `Source_Replica_Queue`, `Normalized_Queue`🧰 Tools
🪛 LanguageTool
[uncategorized] ~71-~71: Do not mix variants of the same word (‘normalise’ and ‘normalize’) within a single text.
Context: ...ity. 5. **Dispatch:** Send event to Normalised_Queue`. ### Layer 4: Outbound Gateway ...
(EN_WORD_COHERENCY)
[uncategorized] ~77-~77: Do not mix variants of the same word (‘normalise’ and ‘normalize’) within a single text.
Context: ...usiness Rules. * Trigger: Consumes Normalised_Queue. * Logic: 1. **Route Loo...
(EN_WORD_COHERENCY)
🤖 Prompt for AI Agents
In `@docs/nexiom_sync_module_design.md` around lines 69 - 77, The document mixes
British and American spellings; standardize to American English by replacing all
instances of "Normalised" with "Normalized" (e.g., change Normalised_Queue to
Normalized_Queue and any "Normalised" layer names), and ensure related
identifiers like Normalized_Entity and "Normalized Layer" remain consistent
across the document (also update other occurrences such as the Outbound Gateway
Layer description and any enum/queue/entity names).
Comment on lines
+1
to
+3
| ## 1. Directory Structure | ||
|
|
||
| The repository follows a standard **Turborepo** layout. We strictly separate **SaaS Modules (Generic)** from the **Engine (Specific)**. |
Contributor
There was a problem hiding this comment.
Use a top‑level H1 heading on Line 1.
Markdownlint MD041 requires the first line to be an H1.
🔧 Proposed fix
-## 1. Directory Structure
+# Nexiom Tech Stack and Repo Plan
+
+## 1. Directory Structure📝 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.
Suggested change
| ## 1. Directory Structure | |
| The repository follows a standard **Turborepo** layout. We strictly separate **SaaS Modules (Generic)** from the **Engine (Specific)**. | |
| # Nexiom Tech Stack and Repo Plan | |
| ## 1. Directory Structure | |
| The repository follows a standard **Turborepo** layout. We strictly separate **SaaS Modules (Generic)** from the **Engine (Specific)**. |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In `@docs/nexiom_tech_stack_and_repo_plan.md` around lines 1 - 3, Replace the
second-level heading "## 1. Directory Structure" with a top-level H1 heading to
satisfy markdownlint MD041; update the line starting with "## 1. Directory
Structure" to use a single leading "#" (e.g., "# Directory Structure" or "# 1.
Directory Structure") so the file's first line is an H1.
- Add jest.clearAllMocks() to platform.guard.spec.ts to prevent mock state leakage - Use toWebHeaders() helper in platform.guard.ts instead of direct Headers construction - Fix dropdown event propagation in TenantList.tsx to prevent row click - Improve loading state in UserShow.tsx with Loader2 icon - Add role-based UI tests to TenantList.spec.tsx for platform_admin vs platform_user
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/modules/tenants/TenantList.tsx (1)
127-231: Restrict row expansion/TenantEdit to platform admins.Non-admins can still expand rows and render
TenantEdit, which undermines the UI gating for admin-only edits. Gate row interactivity and the expanded edit view behindisPlatformAdmin, or render a read-only detail view for non-admins.🔧 Suggested fix
- <TableRow + <TableRow key={tenant.id} - className={cn("cursor-pointer hover:bg-muted/50", expandedTenantId === tenant.id && "bg-muted/50 border-b-0")} - onClick={() => handleRowClick(tenant.id)} - onKeyDown={(e) => handleRowKeyDown(e, tenant.id)} - tabIndex={0} - role="button" - aria-expanded={expandedTenantId === tenant.id} + className={cn( + isPlatformAdmin && "cursor-pointer hover:bg-muted/50", + isPlatformAdmin && expandedTenantId === tenant.id && "bg-muted/50 border-b-0" + )} + onClick={isPlatformAdmin ? () => handleRowClick(tenant.id) : undefined} + onKeyDown={isPlatformAdmin ? (e) => handleRowKeyDown(e, tenant.id) : undefined} + tabIndex={isPlatformAdmin ? 0 : -1} + role={isPlatformAdmin ? "button" : undefined} + aria-expanded={isPlatformAdmin ? expandedTenantId === tenant.id : undefined} > ... - {expandedTenantId === tenant.id && ( + {isPlatformAdmin && expandedTenantId === tenant.id && ( <TableRow className="bg-muted/30 hover:bg-muted/30">
🤖 Fix all issues with AI agents
In `@apps/web/src/modules/tenants/TenantList.spec.tsx`:
- Around line 91-191: Tests are asserting broad button roles that match
unrelated elements; update the three tests in TenantList.spec.tsx to target the
specific UI controls by their accessible names or container context instead of
generic role queries. For the platform_admin case, query the status dropdown
buttons by their specific accessible name (e.g., the status toggle aria-label or
visible label used by TenantList) or query within each tenant row (use the
tenant row container rendered by TenantList) to find the status dropdown; for
the platform_user case, assert absence using queryByRole with that same specific
accessible name and assert the read-only status via getByText for the exact
badge text; for the action-menu check, query the action menu by its real
accessible name/aria-label (e.g., "More actions" or the action button aria-label
used in TenantList) or by using a test id/within the tenant row container to
ensure the menu is not rendered for platform_user. Ensure you reference the
TenantList component and the test cases named 'shows status dropdown for
platform_admin', 'shows read-only status badge for platform_user', and 'hides
action menu for platform_user' when making these changes.
♻️ Duplicate comments (2)
apps/web/src/pages/admin/users/UserShow.tsx (1)
68-93: LGTM - Past review comment addressed.The conditional rendering correctly gates admin actions behind
isPlatformAdmin. TheLoader2icon swap during loading state now matches the pattern used inUsers.tsx, addressing the previous review feedback. The nested conditionals foremailVerified === falseandrecord?.idare appropriate defensive checks.apps/web/src/modules/tenants/TenantList.tsx (1)
212-213: Stop propagation on the Delete action item.
onDeleteis invoked withoutstopPropagation, which can re-trigger row expansion if the menu isn’t fully portaled and is inconsistent with Edit/status items.🛠️ Suggested fix
- <DropdownMenuItem className="text-red-600" onClick={(e) => onDelete?.(tenant.id, e)}> + <DropdownMenuItem + className="text-red-600" + onClick={(e) => { + e.stopPropagation(); + onDelete?.(tenant.id, e); + }} + >
- Use closest('button') method to check if status text is inside buttons
- Update action menu button query to use 'open menu' aria-label
- Platform admin: status text should be inside buttons (dropdown)
- Platform user: status text should NOT be inside buttons (read-only badge)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/modules/tenants/TenantList.spec.tsx`:
- Around line 12-19: The global prototype overrides for ResizeObserver and
HTMLElement methods (globalThis.ResizeObserver,
HTMLElement.prototype.scrollIntoView, .hasPointerCapture,
.releasePointerCapture) leak into other tests; fix by saving the original
references before mocking (e.g., const _origResizeObserver =
globalThis.ResizeObserver, const _origScrollIntoView =
HTMLElement.prototype.scrollIntoView, etc.), set the mocks as currently done,
and restore the originals in an afterAll() cleanup block to prevent cross-test
leakage.
- Around line 49-55: The test helper renderComponent currently wraps TenantList
with AuthProvider which triggers real session/network side effects; replace that
wrapper with AuthContext.Provider using the existing mockAuthValue (i.e., wrap
<TenantList data={mockData} isLoading={false} {...props} /> in
<AuthContext.Provider value={mockAuthValue}>...) so tests use the mocked auth
context like the role-based tests do; ensure AuthContext and mockAuthValue are
imported/available in the spec and remove the AuthProvider usage in
renderComponent to avoid async/act warnings and network calls.
♻️ Duplicate comments (1)
apps/web/src/modules/tenants/TenantList.spec.tsx (1)
126-161: Platform user test asserts the opposite of the intended behavior.This test currently expects buttons to exist for a
platform_user, which contradicts the read-only badge requirement. It should verify that the status text is not inside a button.✅ Corrected assertion
- const statusButtons = screen.queryAllByRole('button', { name: /active|suspended/i }); - // Should not find status dropdown buttons - expect(statusButtons.length).toBe(2); - - // Should find status text as plain text - expect(screen.getByText('Active')).toBeInTheDocument(); + const activeStatus = screen.getByText('Active'); + const suspendedStatus = screen.getByText('Suspended'); + expect(activeStatus.closest('button')).toBeNull(); + expect(suspendedStatus.closest('button')).toBeNull();
Comment on lines
+12
to
+19
| globalThis.ResizeObserver = vi.fn().mockImplementation(() => ({ | ||
| observe: vi.fn(), | ||
| unobserve: vi.fn(), | ||
| disconnect: vi.fn(), | ||
| })); | ||
| window.HTMLElement.prototype.scrollIntoView = vi.fn(); | ||
| window.HTMLElement.prototype.hasPointerCapture = vi.fn(); | ||
| window.HTMLElement.prototype.releasePointerCapture = vi.fn(); | ||
| globalThis.HTMLElement.prototype.scrollIntoView = vi.fn(); | ||
| globalThis.HTMLElement.prototype.hasPointerCapture = vi.fn(); | ||
| globalThis.HTMLElement.prototype.releasePointerCapture = vi.fn(); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Restore global mocks after the suite to avoid cross-test leakage.
These prototype overrides are global. Consider snapshotting originals and restoring them in afterAll() so other test files don’t inherit the mocks.
♻️ Proposed refactor
-import { describe, it, expect, vi, beforeAll } from 'vitest';
+import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
+let originalResizeObserver: typeof globalThis.ResizeObserver | undefined;
+let originalScrollIntoView: typeof HTMLElement.prototype.scrollIntoView;
+let originalHasPointerCapture: typeof HTMLElement.prototype.hasPointerCapture;
+let originalReleasePointerCapture: typeof HTMLElement.prototype.releasePointerCapture;
beforeAll(() => {
+ originalResizeObserver = globalThis.ResizeObserver;
+ originalScrollIntoView = globalThis.HTMLElement.prototype.scrollIntoView;
+ originalHasPointerCapture = globalThis.HTMLElement.prototype.hasPointerCapture;
+ originalReleasePointerCapture = globalThis.HTMLElement.prototype.releasePointerCapture;
+
globalThis.ResizeObserver = vi.fn().mockImplementation(() => ({
observe: vi.fn(),
unobserve: vi.fn(),
disconnect: vi.fn(),
}));
globalThis.HTMLElement.prototype.scrollIntoView = vi.fn();
globalThis.HTMLElement.prototype.hasPointerCapture = vi.fn();
globalThis.HTMLElement.prototype.releasePointerCapture = vi.fn();
});
+afterAll(() => {
+ globalThis.ResizeObserver = originalResizeObserver!;
+ globalThis.HTMLElement.prototype.scrollIntoView = originalScrollIntoView;
+ globalThis.HTMLElement.prototype.hasPointerCapture = originalHasPointerCapture;
+ globalThis.HTMLElement.prototype.releasePointerCapture = originalReleasePointerCapture;
+});📝 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.
Suggested change
| globalThis.ResizeObserver = vi.fn().mockImplementation(() => ({ | |
| observe: vi.fn(), | |
| unobserve: vi.fn(), | |
| disconnect: vi.fn(), | |
| })); | |
| window.HTMLElement.prototype.scrollIntoView = vi.fn(); | |
| window.HTMLElement.prototype.hasPointerCapture = vi.fn(); | |
| window.HTMLElement.prototype.releasePointerCapture = vi.fn(); | |
| globalThis.HTMLElement.prototype.scrollIntoView = vi.fn(); | |
| globalThis.HTMLElement.prototype.hasPointerCapture = vi.fn(); | |
| globalThis.HTMLElement.prototype.releasePointerCapture = vi.fn(); | |
| import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; | |
| let originalResizeObserver: typeof globalThis.ResizeObserver | undefined; | |
| let originalScrollIntoView: typeof HTMLElement.prototype.scrollIntoView; | |
| let originalHasPointerCapture: typeof HTMLElement.prototype.hasPointerCapture; | |
| let originalReleasePointerCapture: typeof HTMLElement.prototype.releasePointerCapture; | |
| beforeAll(() => { | |
| originalResizeObserver = globalThis.ResizeObserver; | |
| originalScrollIntoView = globalThis.HTMLElement.prototype.scrollIntoView; | |
| originalHasPointerCapture = globalThis.HTMLElement.prototype.hasPointerCapture; | |
| originalReleasePointerCapture = globalThis.HTMLElement.prototype.releasePointerCapture; | |
| globalThis.ResizeObserver = vi.fn().mockImplementation(() => ({ | |
| observe: vi.fn(), | |
| unobserve: vi.fn(), | |
| disconnect: vi.fn(), | |
| })); | |
| globalThis.HTMLElement.prototype.scrollIntoView = vi.fn(); | |
| globalThis.HTMLElement.prototype.hasPointerCapture = vi.fn(); | |
| globalThis.HTMLElement.prototype.releasePointerCapture = vi.fn(); | |
| }); | |
| afterAll(() => { | |
| globalThis.ResizeObserver = originalResizeObserver!; | |
| globalThis.HTMLElement.prototype.scrollIntoView = originalScrollIntoView; | |
| globalThis.HTMLElement.prototype.hasPointerCapture = originalHasPointerCapture; | |
| globalThis.HTMLElement.prototype.releasePointerCapture = originalReleasePointerCapture; | |
| }); |
🤖 Prompt for AI Agents
In `@apps/web/src/modules/tenants/TenantList.spec.tsx` around lines 12 - 19, The
global prototype overrides for ResizeObserver and HTMLElement methods
(globalThis.ResizeObserver, HTMLElement.prototype.scrollIntoView,
.hasPointerCapture, .releasePointerCapture) leak into other tests; fix by saving
the original references before mocking (e.g., const _origResizeObserver =
globalThis.ResizeObserver, const _origScrollIntoView =
HTMLElement.prototype.scrollIntoView, etc.), set the mocks as currently done,
and restore the originals in an afterAll() cleanup block to prevent cross-test
leakage.
Comment on lines
+49
to
+55
| <AuthProvider> | ||
| <TenantList | ||
| data={mockData} | ||
| isLoading={false} | ||
| {...props} | ||
| /> | ||
| </AuthProvider> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "TenantList.spec.tsx" -o -name "TenantList.test.tsx"Repository: pramodnarayana/nexiom
Length of output: 115
🏁 Script executed:
cat -n ./apps/web/src/modules/tenants/TenantList.spec.tsxRepository: pramodnarayana/nexiom
Length of output: 7893
🏁 Script executed:
find . -name "AuthProvider*" -type fRepository: pramodnarayana/nexiom
Length of output: 105
🏁 Script executed:
cat -n ./apps/web/src/lib/auth/AuthProvider.tsxRepository: pramodnarayana/nexiom
Length of output: 11551
Update renderComponent() to use mocked AuthContext.Provider instead of AuthProvider.
AuthProvider performs session fetching and network calls on mount (authClient.getSession(), /auth/refresh-session, and potentially /auth/provision-tenant). These side effects can cause async/act warnings and add unnecessary network latency to unit tests that don't require auth state validation.
Your test file already demonstrates the correct pattern in the role-based tests (lines 91–194) using <AuthContext.Provider value={mockAuthValue}>. Apply the same approach to the generic renderComponent() helper (lines 46–58) to avoid side effects in all tests.
Suggested fix
+const defaultAuthValue = {
+ user: null,
+ session: null,
+ isLoading: false,
+ isAuthenticated: false,
+ login: vi.fn(),
+ logout: vi.fn(),
+ signup: vi.fn(),
+ setAuthState: vi.fn(),
+};
+
const renderComponent = (props: Partial<React.ComponentProps<typeof TenantList>> = {}) => {
return render(
<BrowserRouter>
- <AuthProvider>
- <TenantList
- data={mockData}
- isLoading={false}
- {...props}
- />
- </AuthProvider>
+ <AuthContext.Provider value={defaultAuthValue}>
+ <TenantList data={mockData} isLoading={false} {...props} />
+ </AuthContext.Provider>
</BrowserRouter>
);
};📝 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.
Suggested change
| <AuthProvider> | |
| <TenantList | |
| data={mockData} | |
| isLoading={false} | |
| {...props} | |
| /> | |
| </AuthProvider> | |
| const defaultAuthValue = { | |
| user: null, | |
| session: null, | |
| isLoading: false, | |
| isAuthenticated: false, | |
| login: vi.fn(), | |
| logout: vi.fn(), | |
| signup: vi.fn(), | |
| setAuthState: vi.fn(), | |
| }; | |
| const renderComponent = (props: Partial<React.ComponentProps<typeof TenantList>> = {}) => { | |
| return render( | |
| <BrowserRouter> | |
| <AuthContext.Provider value={defaultAuthValue}> | |
| <TenantList data={mockData} isLoading={false} {...props} /> | |
| </AuthContext.Provider> | |
| </BrowserRouter> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In `@apps/web/src/modules/tenants/TenantList.spec.tsx` around lines 49 - 55, The
test helper renderComponent currently wraps TenantList with AuthProvider which
triggers real session/network side effects; replace that wrapper with
AuthContext.Provider using the existing mockAuthValue (i.e., wrap <TenantList
data={mockData} isLoading={false} {...props} /> in <AuthContext.Provider
value={mockAuthValue}>...) so tests use the mocked auth context like the
role-based tests do; ensure AuthContext and mockAuthValue are imported/available
in the spec and remove the AuthProvider usage in renderComponent to avoid
async/act warnings and network calls.
This was referenced Jan 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.