feat(custom-fields): add sorting and grouping for asset custom fields - #2737
feat(custom-fields): add sorting and grouping for asset custom fields#2737Pallavikumarimdb wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (9)
WalkthroughCustom fields now support organization-scoped groups and explicit positions. Users can manage groups, assign groups and positions while creating or editing fields, and view asset fields grouped and ordered accordingly. ChangesCustom field grouping and ordering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant CustomFieldGroupsPage
participant CustomFieldService
participant PrismaDatabase
Admin->>CustomFieldGroupsPage: submit group create, delete, or reorder
CustomFieldGroupsPage->>CustomFieldService: invoke group operation
CustomFieldService->>PrismaDatabase: persist group positions or records
PrismaDatabase-->>CustomFieldService: return operation result
CustomFieldService-->>CustomFieldGroupsPage: return success or error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (3)
apps/webapp/app/modules/custom-field/service.server.ts (1)
753-894: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd JSDoc to the new exported helpers.
The new group CRUD and reorder exports need parameter, return-value, and error documentation.
As per coding guidelines, new exports must include inline documentation and JSDoc comments.
🤖 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 `@apps/webapp/app/modules/custom-field/service.server.ts` around lines 753 - 894, Add JSDoc comments to the exported helpers createCustomFieldGroup, updateCustomFieldGroup, deleteCustomFieldGroup, getCustomFieldGroups, reorderCustomFieldGroups, and reorderCustomFields, documenting their parameters, return values, and errors while preserving the existing implementations.Source: Coding guidelines
apps/webapp/app/routes/_layout+/settings.custom-fields.groups.tsx (2)
1-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew file has no JSDoc/file-level documentation.
loader,action, and the default component are all new exports with no file-level purpose doc or per-export documentation.As per path instructions, "For new files and new exports, include inline documentation and JSDoc comments: file-level purpose docs, exported function/component/type docs, parameter/return/error descriptions, and inline comments for non-obvious business logic."
🤖 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 `@apps/webapp/app/routes/_layout`+/settings.custom-fields.groups.tsx around lines 1 - 27, Document the new custom-field-groups route file with a file-level purpose comment, and add JSDoc for the exported loader, action, and default component, including their parameters, return values, and possible errors; add inline comments only where the business logic is not self-evident.Source: Path instructions
131-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer refs/
useFetcheroverdocument.getElementById/querySelector.
handleMovereaches into the DOM directly to mutate a hidden input and submit a form, bypassing React's data flow. AuseReffor the hidden input, or better,useFetcher().submit(...), would be more idiomatic for this codebase and avoid a full-page navigation for a background reorder.If you switch to
useFetcher, remember to add its fetcher key toexcludeFetchersinapps/webapp/app/hooks/use-nprogress.tsif you make the reorder optimistic, per the webapp's fetcher/loading-bar convention.🤖 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 `@apps/webapp/app/routes/_layout`+/settings.custom-fields.groups.tsx around lines 131 - 156, Replace the DOM-based submission in handleMove with the established React data flow, preferably by using useFetcher().submit with the reorder intent and groupIds payload so the reorder occurs without full-page navigation. Remove the unused FormData and document.getElementById/querySelector logic; if the reorder is optimistic, add the fetcher key to excludeFetchers in use-nprogress.ts.
🤖 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 `@apps/webapp/app/components/assets/custom-fields-inputs.tsx`:
- Around line 257-349: Extract the duplicated FormRow and input-fallback JSX
into a typed renderCustomFieldRow helper that accepts a CustomField and renders
the shared field behavior. Replace the grouped, required, and optional field
maps with this helper, preserving each section’s existing field source and the
grouped branch’s type cast.
- Around line 203-223: Replace the any annotations in hasGroups and
groupedFields with a type derived from the customFields loader payload,
including the group relation shape. If the loader does not return group, update
its payload/type to include it rather than casting locally, while preserving the
existing grouping behavior.
In `@apps/webapp/app/components/custom-fields/form.tsx`:
- Around line 356-395: Update the Group Select and Position Input in the form
component to display their server-side validation errors, using the existing
useActionData, getValidationErrors, and DataOrErrorResponse patterns used by the
other fields. Pass the groupId and position error fallbacks into the
corresponding field components while preserving their current values and
disabled behavior.
- Around line 127-132: Replace the any generic in useLoaderData within the form
component with a local type describing an optional groups array of objects
containing string id and name fields. Remove the redundant cast where possible,
while preserving the existing empty-array fallback when groups is absent.
- Around line 63-66: Update the position field transform in the form schema to
validate the parsed value and reject or replace non-numeric input before it
reaches createCustomField or updateCustomField; preserve the existing optional
and empty-value behavior while ensuring invalid strings produce a clean
validation result rather than NaN.
In `@apps/webapp/app/modules/custom-field/service.server.ts`:
- Around line 639-643: Update the custom-field group ordering and allocation
logic around getActiveCustomFields and getCustomFieldGroups: add a deterministic
secondary sort key (such as group name or ID) after group.position wherever
groups are ordered, and replace the read-then-create position assignment with an
atomic allocation mechanism that remains safe under concurrent creation.
Preserve ascending position ordering while ensuring fields from
colliding-position groups are not interleaved unpredictably.
- Around line 52-53: Keep custom-field/group links organization-scoped: in
apps/webapp/app/modules/custom-field/service.server.ts lines 52-53, validate the
group by both groupId and organizationId before connecting during creation;
apply the same validation at lines 309-314 during updates. In
packages/database/prisma/schema.prisma lines 1119-1143, encode the
organization-scoped relation or equivalent constraint, and update
packages/database/prisma/migrations/20260722175922_add_custom_field_group/migration.sql
lines 26-30 with the corresponding database constraint.
In `@apps/webapp/app/routes/_layout`+/settings.custom-fields.groups.tsx:
- Around line 183-259: Update the Create Group, reorder, and Delete Group submit
controls in the settings groups component to use the existing useDisabled hook
from ~/hooks/use-disabled, applying its disabled state to each relevant Button
during form submission. Ensure rapid clicks cannot submit any of these forms
more than once.
- Around line 226-258: Add explicit aria-label attributes to the icon-only “Move
Down” and “Delete Group” Button elements in the group controls, using accessible
labels that match their existing titles. Keep the current click behavior,
disabled state, and visual styling unchanged.
- Around line 93-99: Update the create flow in the route’s body.intent ===
"create" branch and createCustomFieldGroup service so the new group uses a
gap-safe next position based on max(existing positions) + 1, not
existing.length. Prefer moving this calculation into createCustomFieldGroup with
an atomic aggregate/max query to avoid concurrent-create collisions, while
preserving sequential ordering after deletions.
- Around line 79-89: Update the action in settings.custom-fields.groups.tsx to
select permissions based on the submitted intent: use PermissionAction.create
for create-group operations, PermissionAction.delete for delete-group
operations, and retain PermissionAction.update only for reorder or visibility
edits. Ensure each intent performs its own requirePermission check for
PermissionEntity.customField instead of relying on one update check for all
actions.
- Around line 116-119: Update the reorder branch handling body.intent ===
"reorder" to validate that the submitted groupIds represent the complete set of
custom-field groups for the organization before calling
reorderCustomFieldGroups. Reject incomplete, unknown, or duplicate IDs, and only
invoke reorderCustomFieldGroups when the validated set matches the
organization’s existing groups.
- Around line 247-259: Update the custom field group deletion UI around the
trash Button to use the existing DeleteCustomFieldDialog confirmation pattern
before submitting the delete intent. Keep the group id and delete submission
wired through the confirmed action, ensuring deletion no longer occurs
immediately and clearly warns that assigned fields lose their grouping.
In
`@packages/database/prisma/migrations/20260722175922_add_custom_field_group/migration.sql`:
- Around line 1-3: Update the migration for the CustomField table to avoid
blocking live writes: create the groupId index with CREATE INDEX CONCURRENTLY,
and add the foreign key as NOT VALID followed by a separate VALIDATE CONSTRAINT
step. Ensure the migration structure supports concurrent index creation outside
a transaction, or explicitly require a maintenance window if that is not
possible.
In `@packages/database/prisma/schema.prisma`:
- Around line 1119-1121: Update the CustomField–CustomFieldGroup relation around
groupId and organizationId so references are organization-scoped and cannot link
records from different organizations. Prefer adding a composite
relation/constraint using the existing organization identifiers, and apply the
same change to the corresponding relation at the additionally affected schema
section.
---
Nitpick comments:
In `@apps/webapp/app/modules/custom-field/service.server.ts`:
- Around line 753-894: Add JSDoc comments to the exported helpers
createCustomFieldGroup, updateCustomFieldGroup, deleteCustomFieldGroup,
getCustomFieldGroups, reorderCustomFieldGroups, and reorderCustomFields,
documenting their parameters, return values, and errors while preserving the
existing implementations.
In `@apps/webapp/app/routes/_layout`+/settings.custom-fields.groups.tsx:
- Around line 1-27: Document the new custom-field-groups route file with a
file-level purpose comment, and add JSDoc for the exported loader, action, and
default component, including their parameters, return values, and possible
errors; add inline comments only where the business logic is not self-evident.
- Around line 131-156: Replace the DOM-based submission in handleMove with the
established React data flow, preferably by using useFetcher().submit with the
reorder intent and groupIds payload so the reorder occurs without full-page
navigation. Remove the unused FormData and document.getElementById/querySelector
logic; if the reorder is optimistic, add the fetcher key to excludeFetchers in
use-nprogress.ts.
🪄 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 Plus
Run ID: af8bfcdf-332d-4457-8d99-085fc7dc8ad2
📒 Files selected for processing (11)
apps/webapp/app/components/assets/custom-fields-inputs.tsxapps/webapp/app/components/custom-fields/form.tsxapps/webapp/app/modules/custom-field/service.server.tsapps/webapp/app/modules/custom-field/types.tsapps/webapp/app/routes/_layout+/settings.custom-fields.$fieldId_.edit.tsxapps/webapp/app/routes/_layout+/settings.custom-fields.groups.tsxapps/webapp/app/routes/_layout+/settings.custom-fields.index.tsxapps/webapp/app/routes/_layout+/settings.custom-fields.new.tsxapps/webapp/test/utils/custom-fields.test.tspackages/database/prisma/migrations/20260722175922_add_custom_field_group/migration.sqlpackages/database/prisma/schema.prisma
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/routes/_layout+/settings.custom-fields.groups.tsx (1)
299-324: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRender server-side validation errors for group creation.
The form only relies on native
required; a whitespace-only name or a server-side uniqueness failure returns an action error with no field-level feedback. ConsumeuseActionDataviaDataOrErrorResponseandgetValidationErrors, then pass thenameerror toInput.As per coding guidelines, “All forms must display server-side validation errors as a fallback, using
useActionData,getValidationErrors, andDataOrErrorResponse.”🤖 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 `@apps/webapp/app/routes/_layout`+/settings.custom-fields.groups.tsx around lines 299 - 324, Update the group-creation form around the Form and Input components to consume action results with useActionData typed as DataOrErrorResponse, derive field errors via getValidationErrors, and pass the name error to the Input. Preserve the existing required and disabled behavior while displaying server-side whitespace and uniqueness validation feedback.Source: Coding guidelines
🤖 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 `@apps/webapp/app/routes/_layout`+/settings.custom-fields.groups.tsx:
- Around line 195-206: Type the fetcher used by the delete form in the
surrounding component using the route action type or DataOrErrorResponse<{
success: true }>. Replace the any-cast error check in the useEffect with a
type-safe fetcher.data?.error narrowing while preserving the existing idle,
successful-response, and setOpen(false) behavior.
In
`@packages/database/prisma/migrations/20260722175922_add_custom_field_group/migration.sql`:
- Around line 27-35: Rework the migration steps around the
CustomField_groupId_organizationId_idx index and
CustomField_groupId_organizationId_fkey constraint so they run across separate
transaction boundaries: commit the NOT VALID foreign-key addition first, execute
CREATE INDEX CONCURRENTLY outside any transaction, then validate the constraint
in a later transaction. Preserve the CONCURRENTLY keyword and do not combine
these operations in one transactional migration.
- Line 32: Update the CustomField_groupId_organizationId_fkey definition so
deleting a CustomFieldGroup nulls only groupId while preserving the required
organizationId. Use a supported column-specific ON DELETE action, or add
migration-specific cleanup that clears groupId before removing the referenced
group, without changing the tenant-scope constraint.
In `@packages/database/prisma/schema.prisma`:
- Around line 1119-1121: Change the CustomField.group relation’s onDelete
behavior from SetNull to NoAction because its composite key includes required
organizationId. Ensure group deletion first clears CustomField.groupId, or
manage the database constraint outside Prisma so only the nullable groupId is
cleared.
---
Outside diff comments:
In `@apps/webapp/app/routes/_layout`+/settings.custom-fields.groups.tsx:
- Around line 299-324: Update the group-creation form around the Form and Input
components to consume action results with useActionData typed as
DataOrErrorResponse, derive field errors via getValidationErrors, and pass the
name error to the Input. Preserve the existing required and disabled behavior
while displaying server-side whitespace and uniqueness validation feedback.
🪄 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 Plus
Run ID: f997736a-709a-41a7-9696-5d1df6130989
📒 Files selected for processing (6)
apps/webapp/app/components/assets/custom-fields-inputs.tsxapps/webapp/app/components/custom-fields/form.tsxapps/webapp/app/modules/custom-field/service.server.tsapps/webapp/app/routes/_layout+/settings.custom-fields.groups.tsxpackages/database/prisma/migrations/20260722175922_add_custom_field_group/migration.sqlpackages/database/prisma/schema.prisma
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/webapp/app/components/assets/custom-fields-inputs.tsx
- apps/webapp/app/components/custom-fields/form.tsx
- apps/webapp/app/modules/custom-field/service.server.ts
2881e56 to
7fcf186
Compare
Description
Implements the ability to group and sort custom fields on asset forms. This introduces a new
CustomFieldGroupmodel, settings UI for managing groups, and updates the asset creation/edit forms to render custom fields grouped under their respective headers and sorted by position.changes
CustomFieldGroupmodel and associated it withCustomField./settings/custom-fields/groupsto CRUD groups and reorder them via drag-and-drop.custom-fields-inputs.tsxto group and sort active custom fields by their assigned groups and specified positions before rendering.Closes: #2606
Summary by CodeRabbit