Skip to content

ref(forms): Migrate admin user edit off JsonForm to scraps form#119183

Merged
priscilawebdev merged 6 commits into
masterfrom
pri/ref-forms-migrate-admin-user-edit
Jul 21, 2026
Merged

ref(forms): Migrate admin user edit off JsonForm to scraps form#119183
priscilawebdev merged 6 commits into
masterfrom
pri/ref-forms-migrate-admin-user-edit

Conversation

@priscilawebdev

@priscilawebdev priscilawebdev commented Jul 8, 2026

Copy link
Copy Markdown
Member

Migrates the admin user edit page (/manage/users/:id/) off the legacy JsonForm/FormModel to the scraps form system, and rewrites the users list off ResultGrid onto TanStack Query so opening a user hydrates instantly from the list cache (no spinner or breadcrumb flicker).

This form is not shown on SaaS

Before
image
image
image

After
image

image image

@priscilawebdev

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread static/app/views/admin/adminUserEdit.tsx
Comment thread static/app/views/admin/adminUserEdit.tsx
@priscilawebdev

Copy link
Copy Markdown
Member Author

bugbot run

@priscilawebdev
priscilawebdev force-pushed the pri/ref-forms-migrate-admin-user-edit branch from c5c0520 to 3ee5916 Compare July 17, 2026 13:39
Comment thread static/app/views/admin/adminUserEdit.tsx Outdated
Seed the detail query from the users list cache via placeholderData
instead of initialData. Because placeholderData isn't written to the
cache and keeps the query pending, the detail fetch always runs and the
form only mounts once authoritative data has loaded — so switching users
while the view stays mounted remounts the form with fresh values instead
of showing the previous user's data.

Move BreadcrumbTitle to the parent so it shows the placeholder name while
loading and the detail name once resolved. Keep the detail setQueryData
on mutation success so the breadcrumb updates without a refetch; the list
uses staleTime: 0 and refreshes on its own.
The update path reset the form from the submitted client payload while
the cache was updated from the PUT response, so the form baseline could
diverge from cached user data when the API normalizes fields. Reset from
the response instead, matching the deactivate path.
@priscilawebdev
priscilawebdev marked this pull request as ready for review July 20, 2026 06:35
@priscilawebdev
priscilawebdev requested a review from a team as a code owner July 20, 2026 06:35
Comment thread static/app/views/admin/adminUsers.tsx Outdated
The migrated admin user search debounced navigation by recreating a lodash debounce whenever location.query changed, without cancelling pending timers. A leaked timer would fire with a stale location.query snapshot and spread it over the URL, clobbering newer state such as the status filter or pagination cursor set by a competing navigation.

Replace the manual debounce with nuqs useQueryStates: it writes only the query, status, and cursor keys (no read-modify-write spread of the whole query), debounces the URL update via limitUrlUpdates, and updates state optimistically so typing stays responsive. The status filter keeps its push history entry while search continues to replace.
@priscilawebdev

Copy link
Copy Markdown
Member Author

bugbot run

<Fragment>
<BreadcrumbTitle title={user ? user.name || user.email : t('Details')} />
{isLoading || isPlaceholderData ? (
<LoadingIndicator />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Placeholder cache still shows spinner

Medium Severity

The detail query seeds placeholderData from the users list cache so the page can render immediately, but the UI treats isPlaceholderData like a loading state and keeps showing LoadingIndicator until the refetch finishes. List-cache hydration never drives the edit form, so navigation from the list still shows a spinner despite the cached user already being available.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 316393a. Configure here.

</Alert>
</Alert.Container>
) : (
<AdminUserEditForm user={user} userEndpoint={userEndpoint} />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Placeholder loading hides form

High Severity

The detail view treats isPlaceholderData like a full-page load and shows LoadingIndicator whenever list cache placeholder data is in use. That blocks the edit form during background refetch (against the instant-hydration goal) and, when the detail request returns 404, can keep the spinner up instead of the not-found alert because error handling never runs while placeholder data is active.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 316393a. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is ok

>
<JsonForm forms={[userEditForm]} />
</Form>
<form.SubmitButton>{t('Save Changes')}</form.SubmitButton>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Save allowed without changes

Low Severity

The migrated form always enables Save Changes, but the previous Form used requireChanges, which kept submit disabled until something changed. Admins can now trigger a PUT with unchanged values.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 316393a. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is intended

@priscilawebdev
priscilawebdev requested a review from a team July 21, 2026 06:00

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 4 total unresolved issues (including 3 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4945bf7. Configure here.

...defaultFormOptions,
defaultValues: toFormValues(user),
validators: {onDynamic: schema},
onSubmit: ({value}) => updateMutation.mutateAsync(value).catch(() => {}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API field errors not shown

Medium Severity

Submit errors from the user update API are no longer mapped onto fields. The legacy Form applied responseJSON field errors (such as username already in use) via handleErrorResponse, but onSubmit only swallows the rejection and onError toasts detail or a generic message, so field-level validation failures are lost.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4945bf7. Configure here.

@priscilawebdev
priscilawebdev merged commit fdf8960 into master Jul 21, 2026
73 checks passed
@priscilawebdev
priscilawebdev deleted the pri/ref-forms-migrate-admin-user-edit branch July 21, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants