Repository navigation
refactor(admin): rename dto classes and cleanup imports - #18
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds tenant CRUD endpoints (create, update, delete) with Zod validation and UUIDs; expands controller logic and tests with a chainable MockDb; updates Jest mocks/config for ESM modules; adds e2e tenant tests; implements frontend tenant UI (create/edit dialogs, delete flow) and a composite dataProvider.getList. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebUI as Web UI
participant API as SystemAdmin API
participant DB as Database
User->>WebUI: Submit Create Tenant {name, slug, logo}
WebUI->>API: POST /admin/tenants {name,slug,logo}
API->>DB: SELECT FROM organization WHERE slug = slug
DB-->>API: [] (no match)
API->>DB: INSERT INTO organization {id: uuid(), name, slug, logo, status, createdAt}
DB-->>API: {id, slug, ...}
API-->>WebUI: 201 {id, slug, ...}
WebUI-->>User: show success, refresh list
sequenceDiagram
participant User
participant WebUI as Web UI
participant API as SystemAdmin API
participant DB as Database
User->>WebUI: Click Delete -> confirm
WebUI->>API: DELETE /admin/tenants/:id
API->>DB: BEGIN TRANSACTION
API->>DB: DELETE FROM member WHERE organization_id = id
API->>DB: DELETE FROM invitation WHERE organization_id = id
API->>DB: DELETE FROM organization WHERE id = id
DB-->>API: deletion result
API->>DB: COMMIT
API-->>WebUI: 200 { success: true }
WebUI-->>User: remove tenant from list, show toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 14
🤖 Fix all issues with AI agents
In `@apps/api/src/modules/system-admin/system-admin.controller.spec.ts`:
- Around line 234-252: Add a positive "happy path" unit test for updateTenant:
seed mockDb.query.organization.findFirst to first return the existing tenant
(e.g., {id: 't1', slug: 'old'}) and then null for collision check, mock
mockDb.mutation.organization.update (or the ORM update method used by
controller.updateTenant) to resolve with the updated tenant object, call
controller.updateTenant('t1', { name: 'New', slug: 'new-slug' }) and assert the
promise resolves to the expected updated tenant object and that the update
method was called with correct args; reference controller.updateTenant,
mockDb.query.organization.findFirst, and the mutation update mock to locate the
code to modify.
- Around line 254-262: Add a happy-path unit test for controller.deleteTenant
that asserts it returns { success: true } on successful deletion: mock
mockDb.query.organization.findFirst to return an organization object (so the
existence check passes), mock the deletion call (e.g.,
mockDb.mutation.organization.delete or whichever delete method your service
uses) to resolve successfully, then call await
controller.deleteTenant('some-id') and expect the result toEqual { success: true
}; ensure you restore/verify the mocks and use the same identifiers from the
spec (controller.deleteTenant, mockDb.query.organization.findFirst and the org
delete mutation) so the new test integrates with the existing suite.
In `@apps/api/src/modules/system-admin/system-admin.controller.ts`:
- Around line 88-94: Replace the unsafe cast inside the update call by
constructing a typed update object instead of spreading (input as any) into
.set(); specifically, in the method using
this.db.update(schema.organization).set(...).where(eq(schema.organization.id,
id)).returning(), map only the allowed fields from the incoming input (or
convert input to a Partial<Organization> / proper DTO) into an explicit object
and pass that to .set(), ensuring the update payload matches the schema fields
and TypeScript types rather than using an any cast.
- Around line 99-115: The deleteTenant controller performs a hard delete that
will fail on FK constraint violations when related members or invitations exist;
update the deleteTenant method to first check for dependent records using
this.db.query.member.findFirst({ where: eq(schema.member.organizationId, id) })
and this.db.query.invitation.findFirst({ where:
eq(schema.invitation.organizationId, id) }) and, if dependents exist, either
delete them first via
this.db.delete(schema.member).where(eq(schema.member.organizationId, id)) and
this.db.delete(schema.invitation).where(eq(schema.invitation.organizationId,
id)) before deleting the organization, or switch to a soft-delete flow by
setting organization.deletedAt and skipping hard delete; ensure any DB errors
are caught and translated into appropriate HTTP errors.
In `@apps/api/src/test/mocks/better-auth.mock.ts`:
- Around line 5-19: The parameter types for the test mock functions betterAuth,
drizzleAdapter, organization, and admin use any; change each parameter type to
unknown (e.g., (_options: unknown), (_db: unknown, _options: unknown)) to
tighten typing to match the pattern used for _handler/_req/_res and improve type
safety in tests while keeping the same return values.
In `@apps/api/test/system-admin.tenants.e2e-spec.ts`:
- Around line 1-4: Tests disable multiple ESLint rules due to untyped response
handling; instead define explicit response interfaces (e.g., TenantResponse and
TenantListResponse) and use them where the tests inspect HTTP responses (cast
response.body as TenantResponse or TenantListResponse in assertions), then
remove the broad /* eslint-disable */ lines; update occurrences of response,
tenantResponse, or similar variables in tests to use the typed assertions so the
linter rules no longer need to be disabled.
- Around line 30-32: The tests share mutable state via the createdTenantId
variable which creates a coupling between the "create" test and later tests; fix
this by making each test independent (generate a fresh tenant per test using
testSlug or a per-test suffix) and add a cleanup safety net in an
afterEach/afterAll hook that checks for and deletes any tenant when
createdTenantId is set, ensuring createdTenantId is only assigned after a
successful create (or guarded) so failures in creation don't cascade to
subsequent tests; update references to createdTenantId and testSlug accordingly
and ensure the cleanup hook runs unconditionally to remove leftover test
tenants.
In `@apps/web/src/pages/admin/tenants/components/CreateTenantDialog.tsx`:
- Around line 50-75: The onSubmit handler for CreateTenantDialog calls mutate
but does not invalidate the tenant list after success; update the mutate call
(or use useInvalidate) so that onSuccess either uses mutate's invalidates option
(targeting "admin/tenants") or explicitly calls the invalidate function for the
"admin/tenants" resource before/after setOpen and form.reset; locate the
onSubmit function and the mutate invocation in CreateTenantDialog.tsx and add
the invalidation step so the tenant list refreshes after a successful create.
In `@apps/web/src/pages/admin/tenants/components/EditTenantDialog.tsx`:
- Line 165: In EditTenantDialog.tsx, the Select is currently using defaultValue
which makes it uncontrolled and can show stale values after the form is reset;
change the Select to be a controlled component by replacing defaultValue with
value and pass field.value as the value prop while keeping field.onChange as the
onValueChange handler (i.e., use value={field.value} and
onValueChange={field.onChange} for the Select so react-hook-form control updates
correctly when the tenant changes).
- Around line 68-78: The useEffect in EditTenantDialog currently lists the
entire form object in its dependency array which can cause unnecessary
re-renders because useForm may return a new object reference each render; fix
this by either removing form from the dependency list and using [tenant] only,
or better extract the stable reset function (e.g., const { reset } = form) and
use [tenant, reset] as dependencies, then call reset(...) inside the effect to
safely reset the form when tenant changes.
- Around line 33-38: Frontend UpdateTenantSchema requires name and slug while
backend's UpdateTenantSchema makes all fields optional; make the frontend schema
match the backend by making fields optional (e.g., name, slug, logo, status) so
partial updates are accepted, or alternatively extract and use a shared
validation schema from system-admin.validation.ts; update the const
UpdateTenantSchema in EditTenantDialog.tsx to mirror the backend optionality (or
import the backend schema) and ensure form logic handles undefined values
accordingly.
In `@apps/web/src/pages/admin/tenants/TenantListPage.tsx`:
- Around line 40-61: Replace the native confirm() call inside handleDelete with
your app's Dialog/Modal confirmation component: add local state (e.g.,
confirmOpen and pendingDeleteId) to open the Dialog when a user attempts delete,
show the message in the Dialog, and call deleteMutate({ resource:
"admin/tenants", id: pendingDeleteId }, { onSuccess: ..., onError: ... }) when
the Dialog's confirm button is clicked; keep the existing toast calls in the
onSuccess/onError handlers and ensure the Dialog is accessible and closes on
cancel or after successful deletion.
In `@apps/web/src/providers/data-provider.ts`:
- Around line 44-47: Validate the API response structure before returning to
avoid silent failures: check that the local variable `data` is an object and
contains the expected `data` and `total` properties (e.g., Array or defined for
`data.data` and number for `data.total`) and if validation fails either throw a
clear error or return a safe fallback (empty array and zero total). Update the
return logic in the function that constructs the object with `data: data.data,
total: data.total` to perform these checks and handle malformed responses
gracefully, including updating any callers if you choose to throw an error
instead of returning defaults.
- Around line 29-41: The getList implementation (function getList) lacks error
handling and ignores Refine's filters/sorters/meta; implement mapping of the
incoming parameters (filters, sorters, meta) into queryFilters used in the
request, add try/catch around the axiosInstance.get call to handle and log/throw
a wrapped error, and ensure the returned shape matches Refine's expected format
(data array and total count). Locate getList, queryFilters, API_URL and
axiosInstance to add parameter mapping logic, wrap the HTTP call in try/catch,
and normalize the response (including total) before returning.
Comment on lines
+29
to
+41
| getList: async ({ resource, pagination }: any) => { | ||
| const { current = 1, pageSize = 10 } = pagination ?? {}; | ||
| const queryFilters = {}; // TODO: Implement filter mapping if needed | ||
|
|
||
| const url = `${API_URL}/${resource}`; | ||
|
|
||
| const { data } = await axiosInstance.get(url, { | ||
| params: { | ||
| page: current, | ||
| pageSize: pageSize, | ||
| ...queryFilters, | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Missing error handling and incomplete parameter support.
The custom getList implementation has a few issues:
- No error handling for failed requests - the error will propagate as an unhandled promise rejection
- The function ignores
filters,sorters, andmetaparameters that Refine'sgetListtypically receives - The
queryFiltersTODO suggests this is incomplete
Consider enhancing the implementation:
♻️ Suggested improvements
- // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
- getList: async ({ resource, pagination }: any) => {
+ // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
+ getList: async ({ resource, pagination, filters, sorters, meta }: any) => {
const { current = 1, pageSize = 10 } = pagination ?? {};
- const queryFilters = {}; // TODO: Implement filter mapping if needed
+ // TODO: Implement filter and sorter mapping if needed
+ const queryFilters = {};
const url = `${API_URL}/${resource}`;
- const { data } = await axiosInstance.get(url, {
- params: {
- page: current,
- pageSize: pageSize,
- ...queryFilters,
- },
- });
+ try {
+ const { data } = await axiosInstance.get(url, {
+ params: {
+ page: current,
+ pageSize,
+ ...queryFilters,
+ },
+ });
- // NestJS API returns { data: [...], total: N }
- return {
- data: data.data,
- total: data.total,
- };
+ // NestJS API returns { data: [...], total: N }
+ return {
+ data: data.data,
+ total: data.total,
+ };
+ } catch (error) {
+ // Re-throw with context for Refine's error handling
+ throw error;
+ }
},📝 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
| getList: async ({ resource, pagination }: any) => { | |
| const { current = 1, pageSize = 10 } = pagination ?? {}; | |
| const queryFilters = {}; // TODO: Implement filter mapping if needed | |
| const url = `${API_URL}/${resource}`; | |
| const { data } = await axiosInstance.get(url, { | |
| params: { | |
| page: current, | |
| pageSize: pageSize, | |
| ...queryFilters, | |
| }, | |
| }); | |
| getList: async ({ resource, pagination, filters, sorters, meta }: any) => { | |
| const { current = 1, pageSize = 10 } = pagination ?? {}; | |
| // TODO: Implement filter and sorter mapping if needed | |
| const queryFilters = {}; | |
| const url = `${API_URL}/${resource}`; | |
| try { | |
| const { data } = await axiosInstance.get(url, { | |
| params: { | |
| page: current, | |
| pageSize, | |
| ...queryFilters, | |
| }, | |
| }); | |
| // NestJS API returns { data: [...], total: N } | |
| return { | |
| data: data.data, | |
| total: data.total, | |
| }; | |
| } catch (error) { | |
| // Re-throw with context for Refine's error handling | |
| throw error; | |
| } | |
| }, |
🤖 Prompt for AI Agents
In `@apps/web/src/providers/data-provider.ts` around lines 29 - 41, The getList
implementation (function getList) lacks error handling and ignores Refine's
filters/sorters/meta; implement mapping of the incoming parameters (filters,
sorters, meta) into queryFilters used in the request, add try/catch around the
axiosInstance.get call to handle and log/throw a wrapped error, and ensure the
returned shape matches Refine's expected format (data array and total count).
Locate getList, queryFilters, API_URL and axiosInstance to add parameter mapping
logic, wrap the HTTP call in try/catch, and normalize the response (including
total) before returning.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
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/pages/admin/tenants/TenantListPage.tsx (1)
27-79: Guard against duplicate deletes by checking mutation loading state.
onConfirmDeletecan be triggered multiple times (e.g., double-click), leading to duplicate delete requests. Destructure themutationobject fromuseDelete()to access itsisLoadingflag, then guard the function and disable the button during the request.In
@refinedev/corev4.58.0, the loading state is accessed viamutation.isLoading:🛠️ Suggested fix
- const { mutate: deleteMutate } = useDelete(); + const { mutate: deleteMutate, mutation } = useDelete(); @@ - const onConfirmDelete = () => { + const onConfirmDelete = () => { + if (mutation.isLoading) return; if (!pendingDeleteId) return; @@ - <Button variant="destructive" onClick={onConfirmDelete}>Delete</Button> + <Button variant="destructive" onClick={onConfirmDelete} disabled={mutation.isLoading}> + {mutation.isLoading ? "Deleting..." : "Delete"} + </Button>
🤖 Fix all issues with AI agents
In `@apps/api/src/modules/system-admin/system-admin.controller.ts`:
- Around line 104-140: The deleteTenant handler currently performs multiple
independent DB operations (member/invitation deletes and organization delete)
which can leave partial state if one fails; modify deleteTenant to run the
member lookup+deletes and the final organization delete inside a single database
transaction (use your ORM/DB client's transaction API, e.g., this.db.transaction
or equivalent) by obtaining a transactional client and replacing calls like
this.db.query.member.findFirst, this.db.delete(schema.member).where(...),
this.db.query.invitation.findFirst, and
this.db.delete(schema.organization).where(...) with the transactional client's
query/delete methods, then commit on success and rollback/throw on error so the
entire cleanup + org delete is atomic.
- Around line 59-101: The updateTenant handler builds updatePayload from
optional fields and may be empty, causing invalid SQL; before calling
this.db.update(...) in updateTenant, validate that updatePayload has at least
one key and if not throw a BadRequestException (e.g., "No fields to update") so
empty PATCH bodies are rejected; place this guard after populating updatePayload
and before the .update(schema.organization).set(...) call.
In `@apps/web/src/pages/admin/tenants/components/EditTenantDialog.tsx`:
- Around line 70-80: The useEffect in EditTenantDialog (which calls reset({
name: tenant.name, slug: tenant.slug || "", logo: tenant.logo || "", status:
tenant.status })) only watches [tenant, reset] and can skip when reopening the
dialog for the same tenant; modify the effect to also depend on the dialog open
state (add open to the dependency array) and ensure you call reset when open is
true so the form is re-initialized each time the dialog opens.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/api/src/modules/system-admin/system-admin.controller.spec.ts`:
- Line 71: The test suite is missing specs for createTenant, listUsers, and
listTenants; remove the placeholder comment and add tests that cover all five
public controller methods (createTenant, listUsers, listTenants, updateTenant,
deleteTenant). For each method add a unit test in
system-admin.controller.spec.ts that calls the controller method (e.g.,
systemAdminController.createTenant(...), .listUsers(...), .listTenants(...))
with mocked dependencies, asserts the expected response or delegate calls, and
includes success and at least one error case where the underlying service is
stubbed to throw; reuse existing patterns in the file for mocking services and
assertions used by updateTenant/deleteTenant to keep style consistent. Ensure
tests reference the controller instance (systemAdminController) and the service
mocks used by the spec so they run independently and report clear failures.
In `@apps/api/src/modules/system-admin/system-admin.controller.ts`:
- Around line 118-145: Remove the redundant pre-checks
(tx.query.member.findFirst and tx.query.invitation.findFirst) and instead
perform the deletes directly inside the existing db.transaction block: call
tx.delete(schema.member).where(eq(schema.member.organizationId, id)) and
tx.delete(schema.invitation).where(eq(schema.invitation.organizationId, id))
without prior finds, then proceed to
tx.delete(schema.organization).where(eq(schema.organization.id, id)); keep the
entire operation inside the same this.db.transaction to preserve atomicity and
FK cleanup semantics.
In `@apps/web/src/pages/admin/tenants/components/EditTenantDialog.tsx`:
- Around line 70-71: In the EditTenantDialog React component remove the stale
duplicate comment so only the accurate comment remains — delete the redundant
"Reset form when tenant changes" line and keep the existing "Reset form when
tenant changes or dialog opens" comment near the form reset logic (inside the
EditTenantDialog component) to avoid duplicated comments.
| it('should return paginated users and total count', async () => { | ||
| const mockUsers = [{ id: '1', name: 'User 1' }]; | ||
| mockDb.query.user.findMany.mockResolvedValue(mockUsers); | ||
| // ... (previous tests match until updateTenant) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, verify that listUsers, listTenants, and createTenant tests exist
rg -n "describe\('(listUsers|listTenants|createTenant)'" apps/api/src/modules/system-admin/system-admin.controller.spec.tsRepository: pramodnarayana/nexiom
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Also check the context around line 71 to see the placeholder comment
sed -n '65,80p' apps/api/src/modules/system-admin/system-admin.controller.spec.tsRepository: pramodnarayana/nexiom
Length of output: 544
🏁 Script executed:
#!/bin/bash
# Get a broader view to understand file structure
wc -l apps/api/src/modules/system-admin/system-admin.controller.spec.tsRepository: pramodnarayana/nexiom
Length of output: 134
🏁 Script executed:
#!/bin/bash
# Find all describe blocks to see what tests are in the file
rg -n "describe\(" apps/api/src/modules/system-admin/system-admin.controller.spec.ts | head -20Repository: pramodnarayana/nexiom
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Also check for it() test blocks
rg -n "it\(" apps/api/src/modules/system-admin/system-admin.controller.spec.ts | head -20Repository: pramodnarayana/nexiom
Length of output: 525
🏁 Script executed:
#!/bin/bash
# Get the full file content (since it's only 160 lines)
cat -n apps/api/src/modules/system-admin/system-admin.controller.spec.tsRepository: pramodnarayana/nexiom
Length of output: 6474
🏁 Script executed:
#!/bin/bash
# Find the SystemAdminController file
fd "system-admin.controller.ts$" apps/api/src/modules/system-admin/Repository: pramodnarayana/nexiom
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Get the controller file size
wc -l apps/api/src/modules/system-admin/system-admin.controller.tsRepository: pramodnarayana/nexiom
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Look for method definitions in the controller
rg -n "^\s*(async\s+)?(listUsers|listTenants|createTenant|updateTenant|deleteTenant)" apps/api/src/modules/system-admin/system-admin.controller.tsRepository: pramodnarayana/nexiom
Length of output: 254
Add missing test suites for createTenant, listUsers, and listTenants.
The spec file lacks tests for three public controller methods: createTenant, listUsers, and listTenants. The placeholder comment at line 71 is misleading—it suggests previous tests exist, but there are none. Remove the placeholder and add comprehensive test coverage for all five controller methods (createTenant, listUsers, listTenants, updateTenant, deleteTenant).
🤖 Prompt for AI Agents
In `@apps/api/src/modules/system-admin/system-admin.controller.spec.ts` at line
71, The test suite is missing specs for createTenant, listUsers, and
listTenants; remove the placeholder comment and add tests that cover all five
public controller methods (createTenant, listUsers, listTenants, updateTenant,
deleteTenant). For each method add a unit test in
system-admin.controller.spec.ts that calls the controller method (e.g.,
systemAdminController.createTenant(...), .listUsers(...), .listTenants(...))
with mocked dependencies, asserts the expected response or delegate calls, and
includes success and at least one error case where the underlying service is
stubbed to throw; reuse existing patterns in the file for mocking services and
assertions used by updateTenant/deleteTenant to keep style consistent. Ensure
tests reference the controller instance (systemAdminController) and the service
mocks used by the spec so they run independently and report clear failures.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/api/src/modules/system-admin/system-admin.controller.spec.ts`:
- Around line 254-275: Update the test for controller.deleteTenant to assert
mockDb.delete was invoked for the correct tables: capture mockDb.delete calls
and verify they were called with schema.member, schema.invitation, and
schema.organization (or the exact table identifiers used in your implementation)
in the expected order/number of times; ensure you still assert
mockDb.transaction was called and that the delete calls occur inside the
transaction by inspecting the arguments passed to mockDb.delete in the test for
deleteTenant.
- Around line 103-120: The test's MockQueryBuilder defines a pseudo-thenable by
adding a then property which static analysis flags; replace the then property
with a real Promise-returning method (e.g., execute, getMany, or run) and update
the MockQueryBuilder interface and createMockBuilder accordingly: remove the
then signature, add an execute: () => Promise<unknown> (or getMany depending on
your real code), and implement createMockBuilder so execute returns
Promise.resolve(result) while other chain methods still mockReturnThis(); update
any test calls that awaited the builder to call the new execute/getMany method
instead (referencing MockQueryBuilder and createMockBuilder).
In `@apps/api/src/modules/system-admin/system-admin.controller.ts`:
- Around line 88-93: The updatePayload construction uses mixed truthiness checks
which can skip valid falsy values; change all conditional assignments to
explicit undefined checks (e.g., use if (input.name !== undefined), if
(input.slug !== undefined), if (input.logo !== undefined) (already correct), if
(input.status !== undefined), and if (input.metadata !== undefined) {
updatePayload.metadata = JSON.stringify(input.metadata); }) so updatePayload
(type Partial<typeof schema.organization.$inferInsert>) consistently includes
fields even when they are empty strings or other falsy but valid values.
In `@apps/web/src/pages/admin/tenants/components/EditTenantDialog.tsx`:
- Around line 33-38: The UpdateTenantSchema currently makes every field optional
which allows an all-empty payload; update the schema (UpdateTenantSchema) to add
a .refine(...) that checks at least one of name, slug, status is not undefined
or, for logo, not undefined and not an empty string, and return a clear error
message like "At least one field must be provided" so the form cannot submit an
empty update.
…yle(web): refine validation
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/pages/admin/tenants/components/EditTenantDialog.tsx`:
- Around line 45-48: The root-level validation error is being attached with
path: ["root"] but never rendered, so users won't see the message; update the
EditTenantDialog form to render the root error by checking
form.formState.errors.root and displaying its message (e.g., a small
destructive/error paragraph) inside the form UI—add this display near the form
controls or submit button so the root-level error attached by the validation
logic is visible to users.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/pages/admin/tenants/components/EditTenantDialog.tsx`:
- Around line 33-38: UpdateTenantSchema currently treats slug as a required
min-length string so an empty string (set by the form reset when tenant.slug is
null) fails validation; update the schema for the slug field (in
UpdateTenantSchema) to preprocess/coerce an empty string ("") into undefined
before applying the optional string checks so that a blank slug doesn't block
validation for other fields (ensure the slug still enforces min length, regex
and optional when a non-empty value is provided).
Comment on lines
+33
to
+38
| const UpdateTenantSchema = z.object({ | ||
| name: z.string().min(1, "Name is required").optional(), | ||
| slug: z.string().min(3, "Slug must be at least 3 chars").regex(/^[a-z0-9-]+$/, "Lowercase letters, numbers, and hyphens only").optional(), | ||
| logo: z.string().url("Must be a valid URL").optional().or(z.literal("")), | ||
| status: z.enum(["active", "disabled", "suspended"]).optional(), | ||
| }).refine((data) => { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "EditTenantDialog.tsx" -type fRepository: pramodnarayana/nexiom
Length of output: 131
🏁 Script executed:
cat -n ./apps/web/src/pages/admin/tenants/components/EditTenantDialog.tsxRepository: pramodnarayana/nexiom
Length of output: 9311
Allow empty slug when existing tenant has no slug.
When tenant.slug is null, the form reset sets slug to "" (line 85). However, the schema requires slug to be a non-empty string with minimum 3 characters (line 35), so empty string fails validation and blocks all form submissions unless users also fill in a slug field. This prevents updates to other fields like name or status for tenants without slugs.
Coerce empty string to undefined during validation so the optional slug doesn't block unrelated edits:
🛠️ Proposed fix
+const emptyToUndefined = (value: unknown) => (value === "" ? undefined : value);
+
const UpdateTenantSchema = z.object({
name: z.string().min(1, "Name is required").optional(),
- slug: z.string().min(3, "Slug must be at least 3 chars").regex(/^[a-z0-9-]+$/, "Lowercase letters, numbers, and hyphens only").optional(),
+ slug: z.preprocess(
+ emptyToUndefined,
+ z.string()
+ .min(3, "Slug must be at least 3 chars")
+ .regex(/^[a-z0-9-]+$/, "Lowercase letters, numbers, and hyphens only")
+ ).optional(),
logo: z.string().url("Must be a valid URL").optional().or(z.literal("")),
status: z.enum(["active", "disabled", "suspended"]).optional(),
}).refine((data) => {🤖 Prompt for AI Agents
In `@apps/web/src/pages/admin/tenants/components/EditTenantDialog.tsx` around
lines 33 - 38, UpdateTenantSchema currently treats slug as a required min-length
string so an empty string (set by the form reset when tenant.slug is null) fails
validation; update the schema for the slug field (in UpdateTenantSchema) to
preprocess/coerce an empty string ("") into undefined before applying the
optional string checks so that a blank slug doesn't block validation for other
fields (ensure the slug still enforces min length, regex and optional when a
non-empty value is provided).
This was referenced Jan 18, 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
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.