Repository navigation
feat(api): add tenant infrastructure - #9
Conversation
- schema: add 'status' enum to organization table - migration: generate and apply 0003_ambitious_triathlon.sql - auth: enable 'admin' plugin in better-auth provider - test: fix provider spec by mocking 'admin' plugin
📝 WalkthroughWalkthroughAdds an organization status enum and status column, user ban fields, session impersonation field and FK, updates migrations/snapshots and journal entries, registers an admin plugin in BetterAuth, and implements a tenants admin UI with types, tests, and a dev dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant AdminApp as Admin (TenantListPage)
participant API as API Server (refine resource "organizations")
participant DB as Postgres (migrations + enum)
Browser->>AdminApp: Navigate to /admin/tenants
AdminApp->>API: GET /organizations (useTable)
API->>DB: SELECT organizations (includes status enum)
DB-->>API: rows (status values)
API-->>AdminApp: JSON organizations
AdminApp-->>Browser: Render TenantList (filters, badges)
Browser->>AdminApp: Trigger status change (UI)
AdminApp->>API: PATCH /organizations/:id (new status)
API->>DB: UPDATE organization.status
DB-->>API: success
API-->>AdminApp: updated organization
AdminApp-->>Browser: update row UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/api/src/auth/better-auth.provider.ts:
- Line 10: The admin plugin requires schema additions missing from your DB: add
three columns to the user table—banned (boolean), banReason (text), and
banExpires (timestamp)—and add impersonatedBy (text) to the session table; then
run the Better Auth CLI migration (npx @better-auth/cli migrate) or manually
alter the database and update your schema file so the imported admin plugin
(admin from 'better-auth/plugins') in better-auth.provider.ts can use banning
and impersonation features.
🧹 Nitpick comments (1)
apps/api/src/auth/better-auth.provider.ts (1)
128-133: Consider explicitly setting organization status.While the database default ensures new organizations receive 'active' status, explicitly setting it in the code would improve clarity and make the intent more obvious to future maintainers.
♻️ Proposed enhancement for clarity
await this.db.insert(schema.organization).values({ id: orgId, name: name, slug: slug, createdAt: new Date(), + status: 'active', });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/api/drizzle/0003_ambitious_triathlon.sqlapps/api/drizzle/meta/0003_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/auth/better-auth.provider.spec.tsapps/api/src/auth/better-auth.provider.tsapps/api/src/schema/better-auth.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api/src/auth/better-auth.provider.ts (1)
apps/api/src/schema/better-auth.ts (1)
organization(60-68)
🔇 Additional comments (9)
apps/api/drizzle/meta/_journal.json (1)
26-32: LGTM!The migration journal entry is properly formatted and follows the established pattern for tracking database migrations.
apps/api/drizzle/0003_ambitious_triathlon.sql (1)
1-2: LGTM! Safe migration with backward compatibility.The migration properly creates the enum type and adds the status column with a default value, ensuring existing organizations automatically receive 'active' status without data loss.
apps/api/src/schema/better-auth.ts (2)
1-7: LGTM! Enum definition aligns with SQL migration.The organizationStatusEnum is properly defined and matches the PostgreSQL enum created in the migration file, ensuring type safety between the database and application layers.
67-67: LGTM! Status field properly integrated.The status field correctly uses the enum type with appropriate default and constraint, maintaining consistency with the database schema.
apps/api/src/auth/better-auth.provider.spec.ts (1)
27-27: LGTM! Mock properly added for admin plugin.The admin plugin mock is correctly structured and aligns with the provider implementation changes, ensuring tests remain functional.
apps/api/src/auth/better-auth.provider.ts (1)
65-65: LGTM! Admin plugin properly registered.The admin plugin is correctly added to the plugins array and invoked alongside the organization plugin.
apps/api/drizzle/meta/0003_snapshot.json (3)
450-460: LGTM! Well-structured enum definition.The
organization_statusenum is correctly defined with meaningful states that cover the organization lifecycle (active, disabled, suspended).
222-229: LGTM! Status column correctly defined.The status column properly:
- References the
organization_statusenum type- Enforces NOT NULL constraint for data integrity
- Defaults to 'active' for new organizations
1-471: Snapshot is consistent with source files. The organization_status enum and status column in the snapshot correctly match the migration SQL and schema definition. No action needed.
- add user fields: banned, banReason, banExpires - add session field: impersonatedBy - migration: generate and apply 0004_safe_tattoo.sql
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @apps/api/drizzle/0004_safe_tattoo.sql:
- Line 1: The new session column impersonatedBy lacks a foreign key constraint
to user.id; alter the migration to add a foreign key (e.g., ADD COLUMN
"impersonatedBy" text REFERENCES "user"("id") ON DELETE SET NULL or similar) and
update the schema in apps/api/src/schema/better-auth.ts to add .references(() =>
user.id) on the impersonatedBy field so DB and ORM remain consistent.
In @apps/api/src/schema/better-auth.ts:
- Line 34: The impersonatedBy column in the schema needs a foreign key
constraint to user.id: update the model definition for impersonatedBy (the
text('impersonatedBy') field in better-auth schema) to reference the users
table/primary key (user.id) using your ORM/schema helper for foreign keys, then
run your migration generator to create and apply a migration that adds the FK
constraint to existing databases; ensure cascade or restrict behavior is set per
project convention and include the reference to user.id in the migration.
🧹 Nitpick comments (2)
apps/api/drizzle/0004_safe_tattoo.sql (1)
2-4: Consider adding constraints for ban-related fields.The three ban-related fields (
banned,banReason,banExpires) have no constraints enforcing their relationships, which could lead to inconsistent states such as:
- User marked as banned without a
banReasonbanReasonorbanExpiresset whenbannedis false or NULLWhile these can be enforced at the application level, database-level CHECK constraints would provide stronger guarantees.
♻️ Optional: Add CHECK constraint
ALTER TABLE "user" ADD CONSTRAINT "ban_consistency_check" CHECK ( (banned IS NOT TRUE) OR (banReason IS NOT NULL) );This ensures that if a user is banned, a reason must be provided.
apps/api/src/schema/better-auth.ts (1)
18-20: Consider adding index on banned field.If the application frequently queries for banned users (e.g., to filter them out or display admin lists), an index on the
bannedfield would improve performance.📊 Add index for banned field
After the user table definition, you could add:
export const userBannedIndex = index('user_banned_idx').on(user.banned);Note: You'll need to import
indexfromdrizzle-orm/pg-core.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/drizzle/0004_safe_tattoo.sqlapps/api/drizzle/meta/0004_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/schema/better-auth.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/drizzle/meta/_journal.json
🔇 Additional comments (6)
apps/api/src/schema/better-auth.ts (2)
1-7: LGTM!The
organizationStatusEnumdefinition is well-structured with appropriate status values for organizations.
71-71: Well-designed status field.The
statusfield is properly constrained with NOT NULL and a sensible default value. If you frequently filter organizations by status (e.g., showing only active organizations), consider adding an index.📊 Optional: Add index for status filtering
export const organizationStatusIndex = index('organization_status_idx').on(organization.status);apps/api/drizzle/meta/0004_snapshot.json (4)
222-229: Snapshot correctly reflects organization status field.The snapshot accurately captures the
organization.statusfield with the correct enum type, NOT NULL constraint, and default value.
299-304: Snapshot reflects missing foreign key constraint.This snapshot correctly reflects the current schema, where
impersonatedBylacks a foreign key constraint. Ensure this is addressed when you update the schema inapps/api/src/schema/better-auth.ts.
389-406: Snapshot accurately reflects user ban fields.The snapshot correctly captures the three ban-related fields with appropriate nullability. The lack of constraints is consistent with the current schema definition.
474-483: LGTM!The enum definition in the snapshot accurately reflects the
organizationStatusEnumwith all three status values.
- schema: add references(() => user.id) to impersonatedBy - migration: generate and apply 0005_skinny_the_anarchist.sql
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/api/drizzle/0005_skinny_the_anarchist.sql:
- Line 1: The migration adds a foreign key constraint
session_impersonatedBy_user_id_fk on session.impersonatedBy referencing user.id,
which will fail if any session.impersonatedBy values are non-NULL but do not
match an existing user.id; run the provided orphan-check query to verify
orphaned_count is zero, and if not, either update those session rows to point to
valid user ids or set impersonatedBy to NULL (or create the missing user rows)
to repair data, then re-run the migration to add the constraint.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/drizzle/0005_skinny_the_anarchist.sqlapps/api/drizzle/meta/0005_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/schema/better-auth.ts
✅ Files skipped from review due to trivial changes (1)
- apps/api/drizzle/meta/0005_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/src/schema/better-auth.ts
🔇 Additional comments (1)
apps/api/drizzle/meta/_journal.json (1)
26-46: LGTM!The three new migration journal entries are correctly formatted with sequential idx values (3, 4, 5), ascending timestamps, and consistent structure. The migration sequence is properly documented.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE "session" ADD CONSTRAINT "session_impersonatedBy_user_id_fk" FOREIGN KEY ("impersonatedBy") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action; No newline at end of file | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "(drizzle|migration)" | head -20Repository: pramodnarayana/nexiom
Length of output: 634
🏁 Script executed:
cat -n apps/api/drizzle/0005_skinny_the_anarchist.sqlRepository: pramodnarayana/nexiom
Length of output: 247
🏁 Script executed:
cat apps/api/drizzle/meta/0004_snapshot.jsonRepository: pramodnarayana/nexiom
Length of output: 11985
🏁 Script executed:
cat apps/api/drizzle/meta/0005_snapshot.jsonRepository: pramodnarayana/nexiom
Length of output: 12352
🏁 Script executed:
find . -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "impersonatedBy" 2>/dev/null | head -10Repository: pramodnarayana/nexiom
Length of output: 101
🏁 Script executed:
cat -n ./apps/api/src/schema/better-auth.tsRepository: pramodnarayana/nexiom
Length of output: 3717
🏁 Script executed:
# Search for where impersonatedBy is set in the code
rg "impersonatedBy" --type ts --type tsx -A 2 -B 2 | head -50Repository: pramodnarayana/nexiom
Length of output: 92
🏁 Script executed:
rg "impersonatedBy" -A 2 -B 2 | head -80Repository: pramodnarayana/nexiom
Length of output: 2509
🏁 Script executed:
cat -n apps/api/drizzle/0004_safe_tattoo.sqlRepository: pramodnarayana/nexiom
Length of output: 370
Verify data integrity before applying this migration.
The foreign key constraint will fail if the session table contains any non-NULL impersonatedBy values that don't correspond to existing user.id entries. Run this SQL query against your database to check for such orphaned references:
SELECT COUNT(*) as orphaned_count
FROM session s
WHERE s."impersonatedBy" IS NOT NULL
AND NOT EXISTS (
SELECT 1 FROM "user" u WHERE u.id = s."impersonatedBy"
);If the result is 0, the migration is safe to apply.
🤖 Prompt for AI Agents
In @apps/api/drizzle/0005_skinny_the_anarchist.sql at line 1, The migration adds
a foreign key constraint session_impersonatedBy_user_id_fk on
session.impersonatedBy referencing user.id, which will fail if any
session.impersonatedBy values are non-NULL but do not match an existing user.id;
run the provided orphan-check query to verify orphaned_count is zero, and if
not, either update those session rows to point to valid user ids or set
impersonatedBy to NULL (or create the missing user rows) to repair data, then
re-run the migration to add the constraint.
- modules/tenants: create TenantList with status dropdown - pages: add TenantListPage with strict OrganizationApiResponse types - tests: add unit tests (skip complex dropdown interaction, added user-event) - app: route /admin/tenants
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 (2)
apps/web/src/App.tsx (1)
33-51: Add the missing/admin/tenants/:idroute (currently broken link path).
TenantListlinks to/admin/tenants/${tenant.id}andresources.showis/admin/tenants/:id, but there’s no matching<Route path="tenants/:id" ... />.Proposed minimal fix (placeholder detail route)
@@ <Route path="/admin" element={<AdminSection />}> <Route index element={<AdminDashboardPage />} /> <Route path="users" element={<UserList />} /> <Route path="users/show/:id" element={<UserShow />} /> <Route path="users/edit/:id" element={<UserEdit />} /> <Route path="tenants" element={<TenantListPage />} /> + <Route path="tenants/:id" element={<div>Tenant details (TODO)</div>} /> <Route path="settings" element={<div>Settings Placeholder</div>} /> </Route>Also applies to: 78-86
apps/web/package.json (1)
31-58: Add explicit@testing-library/domdependency and resolve redundant DOM environment setup.@testing-library/user-event@14.6.1 requires @testing-library/dom as a peer dependency; since @testing-library/react v16 also moved it to peer dependency, it must be explicitly declared to ensure deterministic lockfile resolution and avoid transitive dependency issues.
Additionally, both jsdom and happy-dom are present—only one should be used. Configure your vitest environment explicitly (e.g.,
environment: "jsdom"orenvironment: "happy-dom"in vitest.config) and remove the unused DOM library.Note: Vitest has known issues when combining fake timers (
vi.useFakeTimers()) with user-event; if tests hang or timeout, avoid fake timers around user-event calls.
🤖 Fix all issues with AI agents
In @apps/web/src/pages/admin/tenants/TenantListPage.tsx:
- Around line 9-33: In the tenants mapping remove the redundant fallback by
using org.status directly when building TenantTableItem (drop the "|| 'active'"
from the mapping) and replace the console.log in handleStatusChange with a real
update flow: wire a mutation/update hook (e.g., useUpdate or a provided
updateOrganization API) and call it inside handleStatusChange (or, if not yet
implemented, replace the log with a clear TODO comment indicating to call the
update mutation for the organization id and new status); refer to the tenants
mapping where org.status is used and the handleStatusChange function to apply
these changes.
🧹 Nitpick comments (3)
apps/web/src/modules/tenants/TenantList.spec.tsx (1)
8-18: Make DOM/prototype mocks restorable to avoid cross-test leakage.Consider snapshotting originals and restoring in
afterAll, and usingObject.definePropertyfor properties that can be read-only in some DOM impls.Suggested adjustment
-import { describe, it, expect, vi, beforeAll } from 'vitest'; +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; @@ -beforeAll(() => { - window.ResizeObserver = vi.fn().mockImplementation(() => ({ +const originalResizeObserver = window.ResizeObserver; +const originalScrollIntoView = window.HTMLElement.prototype.scrollIntoView; + +beforeAll(() => { + Object.defineProperty(window, "ResizeObserver", { + value: 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(); + })), + configurable: true, + }); + window.HTMLElement.prototype.scrollIntoView = vi.fn(); + Object.defineProperty(window.HTMLElement.prototype, "hasPointerCapture", { value: vi.fn(), configurable: true }); + Object.defineProperty(window.HTMLElement.prototype, "releasePointerCapture", { value: vi.fn(), configurable: true }); }); + +afterAll(() => { + Object.defineProperty(window, "ResizeObserver", { value: originalResizeObserver, configurable: true }); + window.HTMLElement.prototype.scrollIntoView = originalScrollIntoView; +});apps/web/src/modules/tenants/TenantList.tsx (1)
29-35: Small cleanup: simplify filtering + add an accessible label for the icon-only action.
datais defaulted to[], sodata?.filter(...) || []can just bedata.filter(...).- Consider
const q = search.trim().toLowerCase()to avoid repeatedtoLowerCase()calls.- Icon-only “Settings” action should have an
aria-label(or sr-only text) for accessibility.Example tweak
- const filteredData = data?.filter(tenant => - tenant.name.toLowerCase().includes(search.toLowerCase()) || - tenant.slug?.toLowerCase().includes(search.toLowerCase()) - ) || []; + const q = search.trim().toLowerCase(); + const filteredData = data.filter((tenant) => + tenant.name.toLowerCase().includes(q) || + tenant.slug?.toLowerCase().includes(q) + ); @@ - <Button variant="ghost" size="icon" asChild> + <Button variant="ghost" size="icon" aria-label="Tenant details" asChild> <Link to={`/admin/tenants/${tenant.id}`}> <Settings className="h-4 w-4" /> </Link> </Button>Also applies to: 116-122
apps/web/src/modules/tenants/types.ts (1)
1-19: DRY thestatusunion into a shared exported type (and consider optional API status if needed).Suggested refactor
+export type OrganizationStatus = 'active' | 'disabled' | 'suspended'; + export interface TenantTableItem { @@ - status: 'active' | 'disabled' | 'suspended'; + status: OrganizationStatus; } @@ export interface OrganizationApiResponse { @@ - status: 'active' | 'disabled' | 'suspended'; + status: OrganizationStatus; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/web/package.jsonapps/web/src/App.tsxapps/web/src/modules/tenants/TenantList.spec.tsxapps/web/src/modules/tenants/TenantList.tsxapps/web/src/modules/tenants/types.tsapps/web/src/pages/admin/tenants/TenantListPage.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/App.tsx (1)
apps/web/src/pages/admin/tenants/TenantListPage.tsx (1)
TenantListPage(7-63)
apps/web/src/modules/tenants/TenantList.tsx (2)
apps/web/src/modules/tenants/types.ts (1)
TenantTableItem(1-9)apps/web/src/lib/utils.ts (1)
cn(4-6)
- modules/tenants: create TenantList (clean unused logs) - pages: add TenantListPage with strict TenantApiResponse types - tests: add unit tests (mocked Radix UI) - app: route /admin/tenants
1 participant
Summary by CodeRabbit
New Features
Tests
Chores
Dev
✏️ Tip: You can customize this high-level summary in your review settings.