feat: add update-user form #16

Great work on extracting the user management actions into separate components! This improves maintainability and readability of the page. Here are my findings:

🔴 Bug: Missing `await` in `updateUserForm`

In src/routes/admin/users/_forms.remote.ts, the updateUserForm does not await the user.updateProfile(profile) call:

const profile = { ...data, password: data._password };
user.updateProfile(profile);  // ← not awaited!

logger.info({ message: "updated user", userId: user.id });

Since updateProfile is an async method, the function returns before the database update completes. This means:

The logging happens before the update is done
Errors during the update won't be properly propagated
The form result may be returned prematurely

Fix: Add await before user.updateProfile(profile).

🟡 UX Issue: Delete dialog doesn't close after success

In src/routes/admin/users/_components/DeleteUserButton.svelte, the Form component has no onresult handler, so the dialog stays open after a successful deletion. Compare with UpdateUserButton.svelte which correctly closes the dialog:

<Form
  form={updateUserForm.preflight(updateUserFormSchema)}
  onresult={() => {
    dialogOpen = false;
  }}
>

Fix: Either bind the dialog state like UpdateUserButton does, or add an onresult handler to close the dialog after deletion.

💡 Minor: Redundant `preflight` schema

In UpdateUserButton.svelte, the form already has updateUserFormSchema baked in, yet preflight(updateUserFormSchema) is called with the same schema:

form={updateUserForm.preflight(updateUserFormSchema)}

This is consistent with AddUserButton in +page.svelte (addUserForm.preflight(addUserFormSchema)), so it's a pattern already in the codebase. Not a bug, but worth noting that preflight with the same schema is redundant — the schema is already part of the form definition.

Great work on extracting the user management actions into separate components! This improves maintainability and readability of the page. Here are my findings: ## 🔴 Bug: Missing `await` in `updateUserForm` In `src/routes/admin/users/_forms.remote.ts`, the `updateUserForm` does not `await` the `user.updateProfile(profile)` call: ```ts const profile = { ...data, password: data._password }; user.updateProfile(profile); // ← not awaited! logger.info({ message: "updated user", userId: user.id }); ``` Since `updateProfile` is an `async` method, the function returns before the database update completes. This means: - The logging happens **before** the update is done - Errors during the update won't be properly propagated - The form result may be returned prematurely **Fix:** Add `await` before `user.updateProfile(profile)`. ## 🟡 UX Issue: Delete dialog doesn't close after success In `src/routes/admin/users/_components/DeleteUserButton.svelte`, the `Form` component has no `onresult` handler, so the dialog stays open after a successful deletion. Compare with `UpdateUserButton.svelte` which correctly closes the dialog: ```svelte <Form form={updateUserForm.preflight(updateUserFormSchema)} onresult={() => { dialogOpen = false; }} > ``` **Fix:** Either bind the dialog state like `UpdateUserButton` does, or add an `onresult` handler to close the dialog after deletion. ## 💡 Minor: Redundant `preflight` schema In `UpdateUserButton.svelte`, the form already has `updateUserFormSchema` baked in, yet `preflight(updateUserFormSchema)` is called with the same schema: ```ts form={updateUserForm.preflight(updateUserFormSchema)} ``` This is consistent with `AddUserButton` in `+page.svelte` (`addUserForm.preflight(addUserFormSchema)`), so it's a pattern already in the codebase. Not a bug, but worth noting that `preflight` with the same schema is redundant — the schema is already part of the form definition.

src/routes/admin/users/_components/DeleteUserButton.svelte

					
				@ -0,0 +25,4 @@

				      <h2>Elimina utente</h2>

				      <p>Conferma di voler eliminare l'utente {user.data.username}</p>

Dialog doesn't close after successful deletion. Consider adding onresult handler or binding dialog state like UpdateUserButton does.

davide marked this conversation as resolved

src/routes/admin/users/_forms.remote.ts

					
				@ -28,4 +44,4 @@

				    invalid(issue.username("Esiste già un altro utente con questo nome"));

				  }

				  const profile = { ...data, password: data._password };

Missing await on async user.updateProfile() call — function returns before DB update completes.

davide marked this conversation as resolved

llm-bot requested changes

2026-06-27 13:53:49 +00:00

llm-bot left a comment

Great work on extracting the user management actions into separate components! This improves maintainability and readability of the page. Here are my findings:

🔴 Bug: Missing `await` in `updateUserForm`

In src/routes/admin/users/_forms.remote.ts, the updateUserForm does not await the user.updateProfile(profile) call:

const profile = { ...data, password: data._password };
user.updateProfile(profile);  // ← not awaited!

logger.info({ message: "updated user", userId: user.id });

Since updateProfile is an async method, the function returns before the database update completes. This means:

The logging happens before the update is done
Errors during the update won't be properly propagated
The form result may be returned prematurely

Fix: Add await before user.updateProfile(profile).

🟡 UX Issue: Delete dialog doesn't close after success

In src/routes/admin/users/_components/DeleteUserButton.svelte, the Form component has no onresult handler, so the dialog stays open after a successful deletion. Compare with UpdateUserButton.svelte which correctly closes the dialog:

<Form
  form={updateUserForm.preflight(updateUserFormSchema)}
  onresult={() => {
    dialogOpen = false;
  }}
>

Fix: Either bind the dialog state like UpdateUserButton does, or add an onresult handler to close the dialog after deletion.

💡 Minor: Redundant `preflight` schema

In UpdateUserButton.svelte, the form already has updateUserFormSchema baked in, yet preflight(updateUserFormSchema) is called with the same schema. This is consistent with AddUserButton in +page.svelte, so it's a pattern already in the codebase. Not a bug, but worth noting that preflight with the same schema is redundant — the schema is already part of the form definition.

Great work on extracting the user management actions into separate components! This improves maintainability and readability of the page. Here are my findings: ## 🔴 Bug: Missing `await` in `updateUserForm` In `src/routes/admin/users/_forms.remote.ts`, the `updateUserForm` does not `await` the `user.updateProfile(profile)` call: ```ts const profile = { ...data, password: data._password }; user.updateProfile(profile); // ← not awaited! logger.info({ message: "updated user", userId: user.id }); ``` Since `updateProfile` is an `async` method, the function returns before the database update completes. This means: - The logging happens **before** the update is done - Errors during the update won't be properly propagated - The form result may be returned prematurely **Fix:** Add `await` before `user.updateProfile(profile)`. ## 🟡 UX Issue: Delete dialog doesn't close after success In `src/routes/admin/users/_components/DeleteUserButton.svelte`, the `Form` component has no `onresult` handler, so the dialog stays open after a successful deletion. Compare with `UpdateUserButton.svelte` which correctly closes the dialog: ```svelte <Form form={updateUserForm.preflight(updateUserFormSchema)} onresult={() => { dialogOpen = false; }} > ``` **Fix:** Either bind the dialog state like `UpdateUserButton` does, or add an `onresult` handler to close the dialog after deletion. ## 💡 Minor: Redundant `preflight` schema In `UpdateUserButton.svelte`, the form already has `updateUserFormSchema` baked in, yet `preflight(updateUserFormSchema)` is called with the same schema. This is consistent with `AddUserButton` in `+page.svelte`, so it's a pattern already in the codebase. Not a bug, but worth noting that `preflight` with the same schema is redundant — the schema is already part of the form definition.

llm-bot requested changes

2026-06-27 13:56:09 +00:00

llm-bot left a comment

Good work extracting user actions into separate components. Two issues found:

Bug (critical): user.updateProfile(profile) in _forms.remote.ts is not awaited. Since updateProfile is async, the function returns before the DB update completes, meaning logging happens before the update and errors won't propagate properly.
UX issue: DeleteUserButton.svelte doesn't close the dialog after successful deletion (no onresult handler), unlike UpdateUserButton which correctly closes the dialog.

Good work extracting user actions into separate components. Two issues found: 1. **Bug (critical):** `user.updateProfile(profile)` in `_forms.remote.ts` is not awaited. Since `updateProfile` is async, the function returns before the DB update completes, meaning logging happens before the update and errors won't propagate properly. 2. **UX issue:** `DeleteUserButton.svelte` doesn't close the dialog after successful deletion (no `onresult` handler), unlike `UpdateUserButton` which correctly closes the dialog.

davide added 1 commit

2026-06-27 14:24:49 +00:00

chore: close dialog after user delted

check-formatter.yaml / Checks with formatters (pull_request) Successful in 11s

Details

check-linter.yaml / Checks with linters (pull_request) Successful in 15s

Details

check-yamllint.yaml / Lint yaml files (pull_request) Has been cancelled

Details

check-build.yaml / Check that project builds (pull_request) Has been cancelled

Details

check-syntax.yaml / Checks with static tools (pull_request) Has been cancelled

Details

0e533143cb

davide added 1 commit

2026-06-27 14:25:24 +00:00

fix: add await

check-formatter.yaml / Checks with formatters (pull_request) Successful in 12s

Details

check-linter.yaml / Checks with linters (pull_request) Successful in 14s

Details

check-syntax.yaml / Checks with static tools (pull_request) Successful in 22s

Details

check-yamllint.yaml / Lint yaml files (pull_request) Successful in 3s

Details

check-build.yaml / Check that project builds (pull_request) Successful in 1m2s

Details

085838d174

davide scheduled this pull request to auto merge when all checks succeed

2026-06-27 14:25:56 +00:00

davide merged commit 6a16957253 into main

2026-06-27 14:26:29 +00:00

davide deleted branch feat/update-user-form

2026-06-27 14:26:29 +00:00

davide referenced this pull request from a commit

2026-06-27 14:26:31 +00:00

feat: add update-user form (#16)

Sign in to join this conversation.

No reviewers

No labels

No milestone

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

projects/events-cash-register!16

Rows
Columns

feat: add update-user form #16

🔴 Bug: Missing await in updateUserForm

🟡 UX Issue: Delete dialog doesn't close after success

💡 Minor: Redundant preflight schema

🔴 Bug: Missing await in updateUserForm

🟡 UX Issue: Delete dialog doesn't close after success

💡 Minor: Redundant preflight schema

🔴 Bug: Missing `await` in `updateUserForm`

💡 Minor: Redundant `preflight` schema

🔴 Bug: Missing `await` in `updateUserForm`

💡 Minor: Redundant `preflight` schema