diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js index e79ee029..246383c2 100644 --- a/frontend/eslint.config.js +++ b/frontend/eslint.config.js @@ -20,7 +20,7 @@ export default [ } }, { - files: ['**/*.svelte'], + files: ['**/*.svelte', '**/*.svelte.ts', '**/*.svelte.js'], languageOptions: { parserOptions: { parser: ts.parser diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 11e2c512..d344796e 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -110,7 +110,6 @@ "client": "Client", "actor": "Actor", "unknown": "Unknown", - "account_details_updated_successfully": "Account details updated successfully", "profile_picture_updated_successfully": "Profile picture updated successfully. It may take a few minutes to update.", "account_settings": "Account Settings", "passkey_missing": "Passkey missing", @@ -136,7 +135,6 @@ "device_passkeys_only_description": "Allow only passkeys built into the user's device.", "external_security_keys_only": "External security keys only", "external_security_keys_only_description": "Allow only roaming authenticators such as USB or NFC security keys.", - "passkey_configuration_updated_successfully": "Passkey configuration updated successfully", "synced_passkeys_not_allowed": "Synced passkeys are not allowed by your administrator", "manage_your_passkeys_that_you_can_use_to_authenticate_yourself": "Manage your passkeys that you can use to authenticate yourself.", "manage_this_users_passkeys": "Manage this user's passkeys.", @@ -181,13 +179,11 @@ "are_you_sure_you_want_to_revoke_the_api_key_apikeyname": "Are you sure you want to revoke the API key \"{apiKeyName}\"? This will break any integrations using this key.", "last_used": "Last Used", "actions": "Actions", - "images_updated_successfully": "Images updated successfully. It may take a few minutes to update.", "general": "General", "configure_smtp_to_send_emails": "Enable email notifications to alert users when a login is detected from a new device or location.", "ldap": "LDAP", "configure_ldap_settings_to_sync_users_and_groups_from_an_ldap_server": "Configure LDAP settings to sync users and groups from an LDAP server.", "images": "Images", - "email_configuration_updated_successfully": "Email configuration updated successfully", "save_changes_question": "Save changes?", "you_have_to_save_the_changes_before_sending_a_test_email_do_you_want_to_save_now": "You have to save the changes before sending a test email. Do you want to save now?", "save_and_send": "Save and send", @@ -213,7 +209,6 @@ "email_login_code_from_admin": "Email Login Code from Admin", "allows_an_admin_to_send_a_login_code_to_the_user": "Allows an admin to send a login code to the user via email.", "send_test_email": "Send test email", - "application_configuration_updated_successfully": "Application configuration updated successfully", "application_name": "Application Name", "session_duration": "Session Duration", "the_duration_of_a_session_in_minutes_before_the_user_has_to_sign_in_again": "The duration of a session in minutes before the user has to sign in again.", @@ -266,9 +261,6 @@ "local": "Local", "toggle_menu": "Toggle menu", "edit": "Edit", - "user_groups_updated_successfully": "User groups updated successfully", - "user_updated_successfully": "User updated successfully", - "custom_claims_updated_successfully": "Custom claims updated successfully", "back": "Back", "user_details_firstname_lastname": "User Details {firstName} {lastName}", "manage_which_groups_this_user_belongs_to": "Manage which groups this user belongs to.", @@ -286,8 +278,6 @@ "are_you_sure_you_want_to_delete_this_user_group": "Are you sure you want to delete this user group?", "user_group_deleted_successfully": "User group deleted successfully", "user_count": "User Count", - "user_group_updated_successfully": "User group updated successfully", - "users_updated_successfully": "Users updated successfully", "user_group_details_name": "User Group Details {name}", "assign_users_to_this_group": "Assign users to this group.", "custom_claims_are_key_value_pairs_that_can_be_used_to_store_additional_information_about_a_user_prioritized": "Custom claims are key-value pairs that can be used to store additional information about a user. These claims will be included in the ID token if the scope 'profile' is requested. Custom claims defined on the user will be prioritized if there are conflicts.", @@ -320,7 +310,6 @@ "certificate_url": "Certificate URL", "enabled": "Enabled", "disabled": "Disabled", - "oidc_client_updated_successfully": "OIDC client updated successfully", "client_type": "Type", "client_type_standard": "Standard", "client_type_metadata_document": "Metadata Document", @@ -344,7 +333,6 @@ "unrestrict": "Unrestrict", "restrict": "Restrict", "user_groups_restriction_updated_successfully": "User groups restriction updated successfully", - "allowed_user_groups_updated_successfully": "Allowed user groups updated successfully", "favicon": "Favicon", "light_mode_logo": "Light Mode Logo", "dark_mode_logo": "Dark Mode Logo", @@ -468,7 +456,6 @@ "configure_user_creation": "Manage user creation settings, including signup methods and default permissions for new users.", "user_creation_groups_description": "Assign these groups automatically to new users upon signup.", "user_creation_claims_description": "Assign these custom claims automatically to new users upon signup.", - "user_creation_updated_successfully": "User creation settings updated successfully.", "signup_disabled_description": "User signups are completely disabled. Only administrators can create new user accounts.", "signup_requires_valid_token": "A valid signup token is required to create an account", "go_to_login": "Go to login", @@ -547,7 +534,6 @@ "allowed_oidc_clients_description": "Select the OIDC clients that members of this user group are allowed to sign in to.", "unrestrict_oidc_client": "Unrestrict {clientName}", "confirm_unrestrict_oidc_client_description": "Are you sure you want to unrestrict the OIDC client {#b}{clientName}{/b}? This will remove all group assignments for this client and any user will be able to sign in.", - "allowed_oidc_clients_updated_successfully": "Allowed OIDC clients updated successfully", "yes": "Yes", "no": "No", "restricted": "Restricted", @@ -556,7 +542,6 @@ "scim_endpoint": "SCIM Endpoint", "scim_token": "SCIM Token", "last_successful_sync_at": "Last successful sync: {time}", - "scim_configuration_updated_successfully": "SCIM configuration updated successfully.", "scim_enabled_successfully": "SCIM enabled successfully.", "scim_disabled_successfully": "SCIM disabled successfully.", "disable_scim_provisioning": "Disable SCIM Provisioning", @@ -603,9 +588,7 @@ "add_permission": "Add permission", "type": "Type", "api_created_successfully": "API created successfully", - "api_updated_successfully": "API updated successfully", "api_deleted_successfully": "API deleted successfully", - "api_permissions_updated_successfully": "Permissions updated successfully", "are_you_sure_you_want_to_delete_this_api": "Are you sure you want to delete this API? Clients will lose access to its permissions.", "api_access": "API access", "api_access_description": "Select which APIs this client may request tokens for on behalf of users (user-delegated access) and for itself via the client credentials grant (client access), and which permissions it may ask for.", @@ -670,5 +653,11 @@ "client_secrets_limit_reached": "An app cannot have more than 20 client secrets.", "delete_client_secret": "Delete client secret", "are_you_sure_you_want_to_delete_this_client_secret": "Are you sure you want to delete this client secret? Apps using it will no longer be able to authenticate.", - "client_secret_deleted_successfully": "Client secret deleted successfully" + "client_secret_deleted_successfully": "Client secret deleted successfully", + "you_have_unsaved_changes": "You have unsaved changes", + "changes_saved_successfully": "Changes saved successfully", + "discard": "Discard", + "discard_unsaved_changes": "Discard unsaved changes?", + "discard_unsaved_changes_confirm": "You have unsaved changes on this page. Are you sure you want to discard them?", + "please_fix_the_errors_before_saving": "Please fix the highlighted errors before saving" } diff --git a/frontend/src/app.css b/frontend/src/app.css index 5d05eee5..c7373ca5 100644 --- a/frontend/src/app.css +++ b/frontend/src/app.css @@ -20,12 +20,15 @@ ::file-selector-button { border-color: var(--color-gray-200, currentcolor); } + * { @apply border-border outline-ring/50; } + body { @apply bg-background text-foreground; } + html { @apply font-sans; } @@ -222,3 +225,105 @@ .animate-delayed-fade { animation: delayed-fade 0.5s ease-out forwards; } + +@keyframes shake { + 0%, + 100% { + transform: translateX(0); + } + + 25% { + transform: translateX(-10px); + } + + 50% { + transform: translateX(10px); + } + + 75% { + transform: translateX(-10px); + } +} + +.animate-shake { + animation: shake 0.5s ease-in-out; +} + +/* The bar shares its surface token with the cards it floats over, so it needs an elevation of its own */ +.shadow-raised { + box-shadow: + inset 0 1px 0 rgb(255 255 255 / 0.5), + 0 1px 2px rgb(0 0 0 / 0.08), + 0 12px 32px -10px rgb(0 0 0 / 0.25); +} + +.dark .shadow-raised { + box-shadow: + inset 0 1px 0 rgb(255 255 255 / 0.07), + 0 1px 2px rgb(0 0 0 / 0.6), + 0 16px 40px -12px rgb(0 0 0 / 0.9); +} + +/* Marks a moment: the bar arriving, a save settling, a blocked navigation */ +@keyframes bloom { + 0% { + opacity: 0; + } + + 30% { + opacity: 1; + } + + 100% { + opacity: 0; + } +} + +.animate-bloom { + animation: bloom 1.5s ease-out forwards; +} + +/* Dark mode has glow headroom: a luminous colour over a near-black pill reads as light spilling out of it */ +/* Light mode has none, so the same gradient washes out to almost nothing there */ +/* The light values keep the same origin below the pill, so the bloom rises the same way in both, but spread wider and carry more colour to register against white */ +.bloom-pending { + background: radial-gradient(150% 260% at 50% 140%, rgb(217 119 6 / 0.26), transparent 72%); +} + +/* Waits out the bar's 380ms motion so it blooms after the pill settles */ +/* `both` keeps it hidden during that wait */ +.bloom-success { + background: radial-gradient(150% 260% at 50% 140%, rgb(22 163 74 / 0.3), transparent 72%); + animation-delay: 380ms; + animation-fill-mode: both; +} + +.bloom-error { + background: radial-gradient( + 150% 260% at 50% 140%, + color-mix(in oklab, var(--destructive) 28%, transparent), + transparent 72% + ); +} + +.dark .bloom-pending { + background: radial-gradient(130% 150% at 50% 130%, rgb(251 191 36 / 0.24), transparent 70%); +} + +.dark .bloom-success { + background: radial-gradient(130% 150% at 50% 130%, rgb(34 197 94 / 0.24), transparent 70%); +} + +.dark .bloom-error { + background: radial-gradient( + 130% 150% at 50% 130%, + color-mix(in oklab, var(--destructive) 26%, transparent), + transparent 70% + ); +} + +@media (prefers-reduced-motion: reduce) { + .animate-bloom { + animation: none; + } +} diff --git a/frontend/src/lib/components/email-verification-state-box.svelte b/frontend/src/lib/components/email-verification-state-box.svelte index e684c180..4edc860a 100644 --- a/frontend/src/lib/components/email-verification-state-box.svelte +++ b/frontend/src/lib/components/email-verification-state-box.svelte @@ -1,4 +1,5 @@ diff --git a/frontend/src/lib/components/table/advanced-table.svelte b/frontend/src/lib/components/table/advanced-table.svelte index 56a86270..3dc4b920 100644 --- a/frontend/src/lib/components/table/advanced-table.svelte +++ b/frontend/src/lib/components/table/advanced-table.svelte @@ -1,4 +1,6 @@ diff --git a/frontend/src/lib/components/unsaved-changes-bar.svelte b/frontend/src/lib/components/unsaved-changes-bar.svelte new file mode 100644 index 00000000..c77b039c --- /dev/null +++ b/frontend/src/lib/components/unsaved-changes-bar.svelte @@ -0,0 +1,247 @@ + + + + + +{#snippet divider()} + +{/snippet} + +{#if barOpen} +
(shake = false)} + > +
+ {#if animate && bloom} + {#key bloom} + + {/key} + {/if} + + + {#if unsavedChanges.dirtyCount > 1} + + {unsavedChanges.dirtyCount} + + {/if} + {m.you_have_unsaved_changes()} + {@render divider()} + + + + {#if status?.type === 'error'} + + {/if} + {status?.message ?? ''} + {#if showingActions} + {@render divider()} + {/if} + + + + + + +
+
+{/if} diff --git a/frontend/src/lib/stores/unsaved-changes-store.svelte.ts b/frontend/src/lib/stores/unsaved-changes-store.svelte.ts new file mode 100644 index 00000000..c057f203 --- /dev/null +++ b/frontend/src/lib/stores/unsaved-changes-store.svelte.ts @@ -0,0 +1,105 @@ +import { m } from '$lib/paraglide/messages'; +import { getAxiosErrorMessage } from '$lib/utils/error-util'; +import { isAxiosError } from 'axios'; +import { SvelteSet } from 'svelte/reactivity'; + +export interface UnsavedSection { + readonly dirty: boolean; + validate?: () => boolean; + save: () => Promise; + discard: () => void; +} + +export type SaveResult = 'saved' | 'invalid' | 'failed'; + +const STATUS_DISPLAY_MS = 3000; + +const sections = new SvelteSet(); +let saving = $state(false); +let status = $state<{ type: 'success' | 'error'; message: string } | null>(null); +let statusTimeout: ReturnType | undefined; + +const dirtyCount = $derived([...sections].filter((section) => section.dirty).length); +const hasChanges = $derived(dirtyCount > 0); + +function register(section: UnsavedSection) { + sections.add(section); +} + +function unregister(section: UnsavedSection) { + sections.delete(section); +} + +function clearStatus() { + clearTimeout(statusTimeout); + status = null; +} + +function showStatus(type: 'success' | 'error', message: string) { + clearTimeout(statusTimeout); + status = { type, message }; + statusTimeout = setTimeout(() => (status = null), STATUS_DISPLAY_MS); +} + +function errorMessage(e: unknown) { + if (isAxiosError(e)) return getAxiosErrorMessage(e); + return e instanceof Error ? e.message : m.an_unknown_error_occurred(); +} + +async function saveAll(): Promise { + const dirtySections = [...sections].filter((section) => section.dirty); + if (dirtySections.length === 0) return 'saved'; + + // Every section is validated before anything is persisted, so that a validation error in one section can't leave the page half-saved. + const allValid = dirtySections.map((section) => section.validate?.() ?? true).every(Boolean); + if (!allValid) { + showStatus('error', m.please_fix_the_errors_before_saving()); + return 'invalid'; + } + + saving = true; + let failure: { error: unknown } | undefined; + // Saved one after another rather than in parallel: several sections of a page can target the same endpoint. + for (const section of dirtySections) { + try { + await section.save(); + } catch (error) { + failure ??= { error }; + } + } + saving = false; + + if (failure) { + showStatus('error', errorMessage(failure.error)); + return 'failed'; + } + showStatus('success', m.changes_saved_successfully()); + return 'saved'; +} + +function discardAll() { + clearStatus(); + for (const section of sections) { + if (section.dirty) section.discard(); + } +} + +export default { + get hasChanges() { + return hasChanges; + }, + get dirtyCount() { + return dirtyCount; + }, + get saving() { + return saving; + }, + get status() { + return status; + }, + register, + unregister, + saveAll, + discardAll, + clearStatus +}; diff --git a/frontend/src/lib/utils/form-util.ts b/frontend/src/lib/utils/form-util.ts index 68554344..a745bb7d 100644 --- a/frontend/src/lib/utils/form-util.ts +++ b/frontend/src/lib/utils/form-util.ts @@ -1,3 +1,4 @@ +import { reactiveState } from '$lib/utils/reactive-state.svelte'; import { get, writable } from 'svelte/store'; import { z } from 'zod/v4'; @@ -12,22 +13,28 @@ type FormInputs = { }; export function createForm>(schema: T, initialValues: z.infer) { - // Create a writable store for the inputs - const inputsStore = writable>>(initializeInputs(initialValues)); + // The values the inputs are compared against and restored to. Kept as a private deep copy so + // that editing an object or array field can't change what it is supposed to be compared with. + let baseline: z.infer = deepCopy(initialValues); + + // The inputs are deeply reactive, so that field components can update them in place + // (`input.value = ...`) and anything derived from them, such as the unsaved-changes + // tracking, still re-evaluates. + const inputsStore = writable>>(reactiveState(initializeInputs())); const errorsStore = writable | undefined>(); - function initializeInputs(initialValues: z.infer): FormInputs> { + function initializeInputs(): FormInputs> { const inputs: FormInputs> = {} as FormInputs>; const shape = schema instanceof z.ZodObject ? (schema.shape as Record) : {}; - for (const key in initialValues) { - if (Object.prototype.hasOwnProperty.call(initialValues, key)) { + for (const key in baseline) { + if (Object.prototype.hasOwnProperty.call(baseline, key)) { const fieldSchema = shape[key]; inputs[key as keyof z.infer] = { - value: initialValues[key as keyof z.infer], + value: deepCopy(baseline[key as keyof z.infer]), error: null, required: fieldSchema ? isRequired(fieldSchema) : false }; @@ -37,40 +44,31 @@ export function createForm>(schema: T, initialValu } function validate() { - let success = true; - inputsStore.update((inputs) => { - const values = Object.fromEntries( - Object.entries(inputs).map(([key, input]) => [key, input.value]) - ); + const inputs = get(inputsStore); + const values = Object.fromEntries( + Object.entries(inputs).map(([key, input]) => [key, trimValue(input.value)]) + ); + const result = schema.safeParse(values); + errorsStore.set(result.error); - const result = schema.safeParse(values); - errorsStore.set(result.error); - - if (!result.success) { - success = false; - for (const input of Object.keys(inputs)) { - const error = result.error.issues.find((e) => e.path[0] === input); - if (error) { - inputs[input as keyof z.infer].error = error.message; - } else { - inputs[input as keyof z.infer].error = null; - } - } - } else { - for (const input of Object.keys(inputs)) { - inputs[input as keyof z.infer].error = null; - } - } + // Set the error messages for each input based on the validation result + for (const input of Object.keys(inputs)) { + inputs[input as keyof z.infer].error = result.success + ? null + : (result.error.issues.find((e) => e.path[0] === input)?.message ?? null); + } + // If the validation succeeded, update the baseline with the new valid values + if (result.success) { for (const key in result.data) { if (Object.prototype.hasOwnProperty.call(inputs, key)) { inputs[key as keyof z.infer].value = result.data[key]; } } + } - return inputs; - }); - return success ? data() : null; + inputsStore.set(inputs); + return result.success ? result.data : null; } function data() { @@ -86,13 +84,37 @@ export function createForm>(schema: T, initialValu return values; } + function isDirty() { + const inputs = get(inputsStore); + return Object.keys(inputs).some( + (key) => !deepEqual(inputs[key as keyof z.infer].value, baseline[key as keyof z.infer]) + ); + } + + // Moves the baseline forward, e.g. after a successful save whose data isn't reflected back + // into a reactive prop that would otherwise rebuild the form. + function commit(values: z.infer) { + baseline = { ...baseline, ...deepCopy(values) }; + // Notify subscribers, so that anything comparing the inputs against the baseline + // (e.g. the unsaved-changes tracking) is re-evaluated. + inputsStore.update((inputs) => inputs); + } + + function clearErrors() { + errorsStore.set(undefined); + inputsStore.update((inputs) => { + for (const input of Object.values(inputs) as FormInput[]) input.error = null; + return inputs; + }); + } + function reset() { inputsStore.update((inputs) => { for (const input of Object.keys(inputs)) { const current = inputs[input as keyof z.infer]; inputs[input as keyof z.infer] = { ...current, - value: initialValues[input as keyof z.infer], + value: deepCopy(baseline[input as keyof z.infer]), error: null }; } @@ -154,6 +176,57 @@ export function createForm>(schema: T, initialValu data, validate, setValue, - reset + reset, + clearErrors, + isDirty, + commit }; } + +/** + * Picks the properties declared by `schema` out of a larger object, e.g. to build a form for one + * section of the application configuration. + */ +export function pickSchemaValues(schema: T, values: z.infer): z.infer { + return Object.fromEntries( + Object.keys(schema.shape).map((key) => [key, (values as Record)[key]]) + ) as z.infer; +} + +/** + * Creates a deep copy of the given value, ensuring that nested objects and arrays are also copied. + */ +export function deepCopy(value: T): T { + if (Array.isArray(value)) return value.map(deepCopy) as T; + if (value instanceof Date) return new Date(value) as T; + if (value !== null && typeof value === 'object') { + return Object.fromEntries( + Object.entries(value).map(([key, item]) => [key, deepCopy(item)]) + ) as T; + } + return value; +} + +/** + * Performs a deep comparison between two values to determine if they are equivalent. + */ +export function deepEqual(a: unknown, b: unknown): boolean { + if (Object.is(a, b)) return true; + if (typeof a !== typeof b || a === null || b === null) return false; + + if (Array.isArray(a) || Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) return false; + return a.every((item, i) => deepEqual(item, b[i])); + } + + if (typeof a === 'object' && typeof b === 'object') { + const aKeys = Object.keys(a as object); + const bKeys = Object.keys(b as object); + if (aKeys.length !== bKeys.length) return false; + return aKeys.every((key) => + deepEqual((a as Record)[key], (b as Record)[key]) + ); + } + + return false; +} diff --git a/frontend/src/lib/utils/reactive-state.svelte.ts b/frontend/src/lib/utils/reactive-state.svelte.ts new file mode 100644 index 00000000..15b30050 --- /dev/null +++ b/frontend/src/lib/utils/reactive-state.svelte.ts @@ -0,0 +1,9 @@ +/** + * Wraps a value in deeply reactive state, so that reads of it inside `$derived` and `$effect` + * are tracked even when it is mutated in place afterwards. Lives in its own module because + * runes are only available in `.svelte.ts` files. + */ +export function reactiveState(value: T): T { + const state = $state(value); + return state; +} diff --git a/frontend/src/lib/utils/unsaved-changes-util.svelte.ts b/frontend/src/lib/utils/unsaved-changes-util.svelte.ts new file mode 100644 index 00000000..584c2a0e --- /dev/null +++ b/frontend/src/lib/utils/unsaved-changes-util.svelte.ts @@ -0,0 +1,112 @@ +import { m } from '$lib/paraglide/messages'; +import unsavedChanges, { type UnsavedSection } from '$lib/stores/unsaved-changes-store.svelte'; +import { deepCopy, deepEqual } from '$lib/utils/form-util'; +import { untrack } from 'svelte'; +import { fromStore, type Readable } from 'svelte/store'; + +/** + * Registers a section of a page (e.g. a card) with the unsaved-changes bar. + * The section is unregistered when the calling component is destroyed. + */ +export function trackUnsavedSection( + dirty: () => boolean, + save: () => Promise, + discard: () => void, + options: { validate?: () => boolean } = {} +) { + const isDirty = $derived.by(dirty); + const section: UnsavedSection = { + get dirty() { + return isDirty; + }, + validate: options.validate, + save, + discard + }; + + $effect(() => { + unsavedChanges.register(section); + return () => unsavedChanges.unregister(section); + }); +} + +interface TrackedForm { + inputs: Readable; + isDirty: () => boolean; + validate: () => T | null; + commit: (data: T) => void; + reset: () => void; + clearErrors: () => void; +} + +/** + * Convenience wrapper around `trackUnsavedSection` for forms built with `createForm`. + */ +export function trackFormChanges( + getForm: () => TrackedForm, + onSave: (data: T) => Promise, + extra: { dirty?: () => boolean; discard?: () => void; enabled?: () => boolean } = {} +) { + const inputs = $derived(fromStore(getForm().inputs)); + const isDirty = $derived.by(() => { + // The inputs are deeply reactive, so edits are tracked through the reads in `isDirty()`. + // Reading the store on top of that picks up `commit()`, which moves the baseline the + // inputs are compared against without touching the inputs themselves. + void inputs.current; + if (extra.enabled && !extra.enabled()) return false; + return getForm().isDirty() || (extra.dirty?.() ?? false); + }); + + // Validation errors are only meaningful while there is something to save: a field that is + // edited back to its saved value would otherwise keep showing the error of a failed save. + $effect(() => { + if (!isDirty) untrack(() => getForm().clearErrors()); + }); + + trackUnsavedSection( + () => isDirty, + async () => { + const form = getForm(); + const data = form.validate(); + // The bar validates every section before saving, so this only happens if the inputs + // changed in between. + if (!data) throw new Error(m.please_fix_the_errors_before_saving()); + await onSave(data); + form.commit(data); + }, + () => { + getForm().reset(); + extra.discard?.(); + }, + { validate: () => getForm().validate() !== null } + ); +} + +/** + * Convenience wrapper around `trackUnsavedSection` for a value that is edited in place rather + * than through `createForm`, typically a list bound to a selection component. + */ +export function trackUnsavedValue( + read: () => T, + write: (value: T) => void, + save: (value: T) => Promise +) { + let saved = $state.raw(deepCopy(read())); + + // Reads again rather than reusing the value that was saved, so that a caller that replaces + // the value with the server's response is taken into account. + const markSaved = () => { + saved = deepCopy(read()); + }; + + trackUnsavedSection( + () => !deepEqual(read(), saved), + async () => { + await save(deepCopy(read())); + markSaved(); + }, + () => write(deepCopy(saved)) + ); + + return { markSaved }; +} diff --git a/frontend/src/routes/+layout.svelte b/frontend/src/routes/+layout.svelte index ccbd968b..e6293aca 100644 --- a/frontend/src/routes/+layout.svelte +++ b/frontend/src/routes/+layout.svelte @@ -1,11 +1,12 @@ -
+ (hasManualDisplayNameEdit = true)} /> - -
- -
diff --git a/frontend/src/routes/settings/admin/apis/+page.svelte b/frontend/src/routes/settings/admin/apis/+page.svelte index ca68697d..d6bbf65c 100644 --- a/frontend/src/routes/settings/admin/apis/+page.svelte +++ b/frontend/src/routes/settings/admin/apis/+page.svelte @@ -6,7 +6,6 @@ import { m } from '$lib/paraglide/messages'; import ApisService from '$lib/services/apis-service'; import type { ApiCreate } from '$lib/types/api.type'; - import { axiosErrorToast } from '$lib/utils/error-util'; import { LucideMinus, LucidePlus, LucideServer } from '@lucide/svelte'; import { toast } from 'svelte-sonner'; import { slide } from 'svelte/transition'; @@ -18,18 +17,9 @@ const apisService = new ApisService(); async function createApi(api: ApiCreate) { - let success = true; - await apisService - .create(api) - .then((createdApi) => { - toast.success(m.api_created_successfully()); - goto(`/settings/admin/apis/${createdApi.id}`); - }) - .catch((e) => { - axiosErrorToast(e); - success = false; - }); - return success; + const createdApi = await apisService.create(api); + toast.success(m.api_created_successfully()); + goto(`/settings/admin/apis/${createdApi.id}`); } diff --git a/frontend/src/routes/settings/admin/apis/[id]/+page.svelte b/frontend/src/routes/settings/admin/apis/[id]/+page.svelte index 9565c624..a9abd748 100644 --- a/frontend/src/routes/settings/admin/apis/[id]/+page.svelte +++ b/frontend/src/routes/settings/admin/apis/[id]/+page.svelte @@ -1,13 +1,11 @@ @@ -108,9 +93,6 @@ defaultExpanded={true} > -
- -
diff --git a/frontend/src/routes/settings/admin/apis/[id]/api-access-card.svelte b/frontend/src/routes/settings/admin/apis/[id]/api-access-card.svelte index beb7ad32..a01cc1cf 100644 --- a/frontend/src/routes/settings/admin/apis/[id]/api-access-card.svelte +++ b/frontend/src/routes/settings/admin/apis/[id]/api-access-card.svelte @@ -10,7 +10,10 @@ let { api, onCimdAccessSave - }: { api: Api; onCimdAccessSave: (update: ApiCimdAccessUpdate) => Promise } = $props(); + }: { + api: Api; + onCimdAccessSave: (update: ApiCimdAccessUpdate) => Promise; + } = $props(); let clientsTab = $state(); let tab = $state('clients'); diff --git a/frontend/src/routes/settings/admin/apis/[id]/api-cimd-access-tab.svelte b/frontend/src/routes/settings/admin/apis/[id]/api-cimd-access-tab.svelte index 096f4c9b..9d774a72 100644 --- a/frontend/src/routes/settings/admin/apis/[id]/api-cimd-access-tab.svelte +++ b/frontend/src/routes/settings/admin/apis/[id]/api-cimd-access-tab.svelte @@ -1,31 +1,28 @@ -
+
- form.setValue( + formStore.setValue( 'permissionIds', checked ? [...$inputs.permissionIds.value, permission.id] @@ -71,8 +68,4 @@ {/if}
{/if} - -
- -
-
+ diff --git a/frontend/src/routes/settings/admin/apis/api-form.svelte b/frontend/src/routes/settings/admin/apis/api-form.svelte index 94b5cfae..284d230f 100644 --- a/frontend/src/routes/settings/admin/apis/api-form.svelte +++ b/frontend/src/routes/settings/admin/apis/api-form.svelte @@ -3,8 +3,10 @@ import { Button } from '$lib/components/ui/button'; import { m } from '$lib/paraglide/messages'; import type { Api, ApiCreate } from '$lib/types/api.type'; + import { axiosErrorToast } from '$lib/utils/error-util'; import { preventDefault } from '$lib/utils/event-util'; import { createForm } from '$lib/utils/form-util'; + import { trackFormChanges } from '$lib/utils/unsaved-changes-util.svelte'; import { z } from 'zod/v4'; let { @@ -12,7 +14,7 @@ existingApi }: { existingApi?: Api; - callback: (api: ApiCreate) => Promise; + callback: (api: ApiCreate) => Promise; } = $props(); let isLoading = $state(false); @@ -35,17 +37,30 @@ }); type FormSchema = typeof formSchema; - const { inputs, ...form } = createForm(formSchema, api); + const formStore = createForm(formSchema, api); + const { inputs } = formStore; + async function saveApi(data: z.infer) { + await callback(data); + if (!existingApi) formStore.reset(); + } + + // Create mode has its own Save button rather than going through the unsaved-changes bar. async function onSubmit() { - const data = form.validate(); + const data = formStore.validate(); if (!data) return; isLoading = true; - const success = await callback(data); - if (success && !existingApi) { - form.reset(); + try { + await saveApi(data); + } catch (e) { + axiosErrorToast(e); + } finally { + isLoading = false; } - isLoading = false; + } + + if (isEdit) { + trackFormChanges(() => formStore, saveApi); } @@ -59,7 +74,9 @@ readonly={isEdit} /> -
- -
+ {#if !isEdit} +
+ +
+ {/if} diff --git a/frontend/src/routes/settings/admin/application-configuration/+page.svelte b/frontend/src/routes/settings/admin/application-configuration/+page.svelte index f21e7fb6..1411622a 100644 --- a/frontend/src/routes/settings/admin/application-configuration/+page.svelte +++ b/frontend/src/routes/settings/admin/application-configuration/+page.svelte @@ -6,9 +6,7 @@ import AppConfigService from '$lib/services/app-config-service'; import appConfigStore from '$lib/stores/application-configuration-store'; import type { AllAppConfig } from '$lib/types/application-configuration.type'; - import { axiosErrorToast } from '$lib/utils/error-util'; import { LucideInfo } from '@lucide/svelte'; - import { toast } from 'svelte-sonner'; import AppConfigDynamicClientsForm from './forms/app-config-dynamic-clients-form.svelte'; import AppConfigEmailForm from './forms/app-config-email-form.svelte'; import AppConfigGeneralForm from './forms/app-config-general-form.svelte'; @@ -19,20 +17,17 @@ let { data } = $props(); let appConfig = $state(data.appConfig); + let persistedAppConfig = { ...data.appConfig }; const appConfigService = new AppConfigService(); async function updateAppConfig(updatedAppConfig: Partial) { - appConfig = await appConfigService - .update({ - ...appConfig, - ...updatedAppConfig - }) - .catch((e) => { - axiosErrorToast(e); - throw e; - }); - await appConfigStore.reload(); + persistedAppConfig = await appConfigService.update({ + ...persistedAppConfig, + ...updatedAppConfig + }); + + appConfigStore.set({ ...$appConfigStore, ...persistedAppConfig }); } async function updateImages( @@ -84,9 +79,7 @@ defaultProfilePicturePromise, backgroundImagePromise, faviconPromise - ]) - .then(() => toast.success(m.images_updated_successfully())) - .catch(axiosErrorToast); + ]); } diff --git a/frontend/src/routes/settings/admin/application-configuration/forms/accent-color-picker.svelte b/frontend/src/routes/settings/admin/application-configuration/forms/accent-color-picker.svelte index a297ef75..07968e64 100644 --- a/frontend/src/routes/settings/admin/application-configuration/forms/accent-color-picker.svelte +++ b/frontend/src/routes/settings/admin/application-configuration/forms/accent-color-picker.svelte @@ -27,9 +27,14 @@ let isCustomColor = $derived(!accentColors.some((c) => c.color === selectedColor)); let isPreviousColorCustom = $derived(!accentColors.some((c) => c.color === previousColor)); + // Applies the selection as a live preview. Because it re-runs whenever the bound value + // changes, it also restores the saved colour when the selection is discarded. + $effect(() => { + applyAccentColor(selectedColor); + }); + function handleAccentColorChange(accentValue: string) { selectedColor = accentValue; - applyAccentColor(accentValue); } diff --git a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-dynamic-clients-form.svelte b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-dynamic-clients-form.svelte index a27b31f0..1fa4e1ce 100644 --- a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-dynamic-clients-form.svelte +++ b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-dynamic-clients-form.svelte @@ -1,13 +1,11 @@ {#snippet cimdUrlAllowlistDescription()} {/snippet} -
-
- - - - -
- -
-
-
+
+ + + +
diff --git a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-email-form.svelte b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-email-form.svelte index 29cf7093..35c4d532 100644 --- a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-email-form.svelte +++ b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-email-form.svelte @@ -9,8 +9,9 @@ import AppConfigService from '$lib/services/app-config-service'; import appConfigStore from '$lib/stores/application-configuration-store'; import type { AllAppConfig } from '$lib/types/application-configuration.type'; - import { preventDefault } from '$lib/utils/event-util'; - import { createForm } from '$lib/utils/form-util'; + import { axiosErrorToast } from '$lib/utils/error-util'; + import { createForm, pickSchemaValues } from '$lib/utils/form-util'; + import { trackFormChanges } from '$lib/utils/unsaved-changes-util.svelte'; import { toast } from 'svelte-sonner'; import { z } from 'zod/v4'; @@ -87,23 +88,32 @@ requireFieldsWhen(emailEnabled, m.smtp_field_required_when_email_enabled()); }); - let { inputs, ...form } = $derived(createForm(formSchema, appConfig)); + const formStore = createForm(formSchema, pickSchemaValues(formSchema, appConfig)); + const inputs = formStore.inputs; - async function onSubmit() { - const data = form.validate(); - if (!data) return false; + async function saveEmailConfig(data: z.infer) { await callback(data); - - // Update the app config to don't display the unsaved changes warning - Object.assign(appConfig, data); - - toast.success(m.email_configuration_updated_successfully()); - return true; } + + // Saving from the "send test email" prompt is outside the unsaved-changes bar, so it + // reports failures itself rather than letting them reach the bar. + async function saveForTestEmail() { + const data = formStore.validate(); + if (!data) return false; + try { + await saveEmailConfig(data); + formStore.commit(data); + return true; + } catch (e) { + axiosErrorToast(e); + return false; + } + } + + trackFormChanges(() => formStore, saveEmailConfig); + async function onTestEmail() { - const hasChanges = Object.entries($inputs).some( - ([key, input]) => input.value !== appConfig[key as keyof AllAppConfig] - ); + const hasChanges = formStore.isDirty(); if (hasChanges) { openConfirmDialog({ @@ -113,8 +123,7 @@ confirm: { label: m.save_and_send(), action: async () => { - const saved = await onSubmit(); - if (saved) { + if (await saveForTestEmail()) { sendTestEmail(); } } @@ -135,7 +144,7 @@ } -
+

{m.general()}

@@ -221,6 +230,5 @@ -
- +
diff --git a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-general-form.svelte b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-general-form.svelte index 891aa714..e59525d1 100644 --- a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-general-form.svelte +++ b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-general-form.svelte @@ -1,15 +1,13 @@ -
-
-
- - - - {m.app_config_home_page()} - - {m.app_config_home_page_description()} - - ($inputs.homePageUrl.value = v as string)} - > - - {homePageUrlOptions.find((option) => option.value === $inputs.homePageUrl.value) - ?.label ?? $inputs.homePageUrl.value} - - - {#each homePageUrlOptions as option (option.value)} - - {option.label} - - {/each} - - - - - +
+
+ + + + {m.app_config_home_page()} + + {m.app_config_home_page_description()} + + ($inputs.homePageUrl.value = v as string)} + > + + {homePageUrlOptions.find((option) => option.value === $inputs.homePageUrl.value)?.label ?? + $inputs.homePageUrl.value} + + + {#each homePageUrlOptions as option (option.value)} + + {option.label} + + {/each} + + + + + - -
- - {m.accent_color()} - - - {m.select_an_accent_color_to_customize_the_appearance_of_pocket_id()} - -
- - -
-
- -
-
- + +
+ + {m.accent_color()} + + + {m.select_an_accent_color_to_customize_the_appearance_of_pocket_id()} + +
+ + +
+
diff --git a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-ldap-form.svelte b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-ldap-form.svelte index bb7aebd2..f1e9c48e 100644 --- a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-ldap-form.svelte +++ b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-ldap-form.svelte @@ -7,8 +7,8 @@ import appConfigStore from '$lib/stores/application-configuration-store'; import type { AllAppConfig } from '$lib/types/application-configuration.type'; import { axiosErrorToast } from '$lib/utils/error-util'; - import { preventDefault } from '$lib/utils/event-util'; - import { createForm } from '$lib/utils/form-util'; + import { createForm, pickSchemaValues } from '$lib/utils/form-util'; + import { trackFormChanges } from '$lib/utils/unsaved-changes-util.svelte'; import { toast } from 'svelte-sonner'; import { z } from 'zod/v4'; @@ -47,19 +47,18 @@ ldapSoftDeleteUsers: z.boolean() }); - let { inputs, ...form } = $derived(createForm(formSchema, appConfig)); + const formStore = createForm(formSchema, pickSchemaValues(formSchema, appConfig)); + const inputs = formStore.inputs; - async function onSubmit() { - const data = form.validate(); - if (!data) return false; - await callback({ - ...data, - ldapEnabled: true - }); - toast.success(m.ldap_configuration_updated_successfully()); - return true; + async function saveLdap(data: z.infer, enable: boolean) { + await callback({ ...data, ldapEnabled: enable }); } + trackFormChanges( + () => formStore, + (data) => saveLdap(data, ldapEnabled) + ); + async function onDisable() { ldapEnabled = false; await callback({ ldapEnabled }); @@ -67,8 +66,15 @@ } async function onEnable() { - if (await onSubmit()) { + const data = formStore.validate(); + if (!data) return; + try { + await saveLdap(data, true); + formStore.commit(data); ldapEnabled = true; + toast.success(m.ldap_configuration_updated_successfully()); + } catch (e) { + axiosErrorToast(e); } } @@ -83,7 +89,7 @@ } -
+

{m.client_configuration()}

@@ -204,9 +210,8 @@ >{m.disable()} - {:else} {/if}
- +
diff --git a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-passkeys-form.svelte b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-passkeys-form.svelte index 35c68f92..903cede3 100644 --- a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-passkeys-form.svelte +++ b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-passkeys-form.svelte @@ -1,14 +1,12 @@ -
-
- - -
- {m.user_verification()} - {m.user_verification_description()} -
- - ($inputs.webauthnUserVerification.value = value as 'required' | 'preferred')} +
+ + +
+ {m.user_verification()} + {m.user_verification_description()} +
+ + ($inputs.webauthnUserVerification.value = value as 'required' | 'preferred')} + > + - - {userVerificationOptions[$inputs.webauthnUserVerification.value]?.label} - - - - {#each Object.entries(userVerificationOptions) as [value, option] (value)} - -
- {option.label} - {option.description} -
- - {/each} - - - - + {userVerificationOptions[$inputs.webauthnUserVerification.value]?.label} + + + + {#each Object.entries(userVerificationOptions) as [value, option] (value)} + +
+ {option.label} + {option.description} +
+ + {/each} + + + + - - -
- {m.allow_synced_passkeys()} - {m.allow_synced_passkeys_description()} -
- - - - - + +
- {m.allowed_authenticator_type()} - {m.allowed_authenticator_type_description()} + {m.allow_synced_passkeys()} + {m.allow_synced_passkeys_description()}
- - ($inputs.webauthnAuthenticatorAttachment.value = value as - 'any' | 'platform' | 'cross-platform')} - > - - {authenticatorAttachmentOptions[$inputs.webauthnAuthenticatorAttachment.value]?.label} - - - - {#each Object.entries(authenticatorAttachmentOptions) as [value, option] (value)} - -
- {option.label} - {option.description} -
- - {/each} - - - - + + + - - - - -
- + +
+ {m.allowed_authenticator_type()} + {m.allowed_authenticator_type_description()} +
+ + ($inputs.webauthnAuthenticatorAttachment.value = value as + 'any' | 'platform' | 'cross-platform')} + > + + {authenticatorAttachmentOptions[$inputs.webauthnAuthenticatorAttachment.value]?.label} + + + + {#each Object.entries(authenticatorAttachmentOptions) as [value, option] (value)} + +
+ {option.label} + {option.description} +
+ + {/each} + + + + + +
diff --git a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-signup-defaults-form.svelte b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-signup-defaults-form.svelte index 9411ae7a..8e78aa79 100644 --- a/frontend/src/routes/settings/admin/application-configuration/forms/app-config-signup-defaults-form.svelte +++ b/frontend/src/routes/settings/admin/application-configuration/forms/app-config-signup-defaults-form.svelte @@ -1,14 +1,12 @@ -
-
-
- -
- {m.enable_user_signups()} - - {m.enable_user_signups_description()} - -
- (allowUserSignups = v as typeof allowUserSignups)} +
+
+ +
+ {m.enable_user_signups()} + + {m.enable_user_signups_description()} + +
+ (allowUserSignups = v as typeof allowUserSignups)} + > + - - {signupOptions[allowUserSignups]?.label} - - - -
- {signupOptions.disabled.label} - - {signupOptions.disabled.description} - -
- - -
- {signupOptions.withToken.label} - - {signupOptions.withToken.description} - -
- - -
- {signupOptions.open.label} - - {signupOptions.open.description} - -
- - - - -
- - -
- {m.user_groups()} - - {m.user_creation_groups_description()} - -
- - - -
- {m.custom_claims()} - - {m.user_creation_claims_description()} - -
- + {signupOptions[allowUserSignups]?.label} + + + +
+ {signupOptions.disabled.label} + + {signupOptions.disabled.description} + +
+ + +
+ {signupOptions.withToken.label} + + {signupOptions.withToken.description} + +
+ + +
+ {signupOptions.open.label} + + {signupOptions.open.description} + +
+ + + +
-
- + +
+ {m.user_groups()} + + {m.user_creation_groups_description()} +
-
-
+ + + +
+ {m.custom_claims()} + + {m.user_creation_claims_description()} + +
+ + + diff --git a/frontend/src/routes/settings/admin/application-configuration/update-application-images.svelte b/frontend/src/routes/settings/admin/application-configuration/update-application-images.svelte index c0b2448d..eb79bf67 100644 --- a/frontend/src/routes/settings/admin/application-configuration/update-application-images.svelte +++ b/frontend/src/routes/settings/admin/application-configuration/update-application-images.svelte @@ -1,6 +1,5 @@ {#snippet lightLogoFallback()} @@ -44,71 +82,64 @@ {/snippet} -
- - - - - - -
-
- -
+{#key resetKey} +
+ + + + + + +
+{/key} diff --git a/frontend/src/routes/settings/admin/oidc-clients/+page.svelte b/frontend/src/routes/settings/admin/oidc-clients/+page.svelte index 31fefa17..25d0fe6e 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/+page.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/+page.svelte @@ -8,7 +8,6 @@ import clientSecretStore from '$lib/stores/client-secret-store'; import type { OidcClientCreateWithLogo } from '$lib/types/oidc.type'; import { encodeClientIdParam } from '$lib/utils/client-id-util'; - import { axiosErrorToast } from '$lib/utils/error-util'; import { LucideMinus, ShieldCheck, ShieldPlus } from '@lucide/svelte'; import { toast } from 'svelte-sonner'; import { slide } from 'svelte/transition'; @@ -20,26 +19,20 @@ const oidcService = new OIDCService(); async function createOIDCClient(client: OidcClientCreateWithLogo) { - try { - clientSecretStore.clear(); - const createdClient = await oidcService.createClient(client); + clientSecretStore.clear(); + const createdClient = await oidcService.createClient(client); - const logoPromise = client.logo - ? oidcService.updateClientLogo(createdClient, client.logo, true) - : Promise.resolve(); - const darkLogoPromise = client.darkLogo - ? oidcService.updateClientLogo(createdClient, client.darkLogo, false) - : Promise.resolve(); - await Promise.all([logoPromise, darkLogoPromise]); + const logoPromise = client.logo + ? oidcService.updateClientLogo(createdClient, client.logo, true) + : Promise.resolve(); + const darkLogoPromise = client.darkLogo + ? oidcService.updateClientLogo(createdClient, client.darkLogo, false) + : Promise.resolve(); + await Promise.all([logoPromise, darkLogoPromise]); - // A new client starts without any secret: the admin creates the ones they need from the credentials tab - goto(`/settings/admin/oidc-clients/${encodeClientIdParam(createdClient.id)}`); - toast.success(m.oidc_client_created_successfully()); - return true; - } catch (e) { - axiosErrorToast(e); - return false; - } + // A new client starts without any secret: the admin creates the ones they need from the credentials tab + goto(`/settings/admin/oidc-clients/${encodeClientIdParam(createdClient.id)}`); + toast.success(m.oidc_client_created_successfully()); } diff --git a/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte b/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte index d102032e..48f98a19 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/[id]/+page.svelte @@ -1,5 +1,4 @@ @@ -343,11 +312,6 @@
- -
{/if} diff --git a/frontend/src/routes/settings/admin/oidc-clients/[id]/oidc-client-federated-credentials-card.svelte b/frontend/src/routes/settings/admin/oidc-clients/[id]/oidc-client-federated-credentials-card.svelte index 4ae1d4ac..d1344868 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/[id]/oidc-client-federated-credentials-card.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/[id]/oidc-client-federated-credentials-card.svelte @@ -3,8 +3,8 @@ import * as Card from '$lib/components/ui/card'; import { m } from '$lib/paraglide/messages'; import type { OidcClient, OidcClientFederatedIdentity } from '$lib/types/oidc.type'; - import { preventDefault } from '$lib/utils/event-util'; import { createForm } from '$lib/utils/form-util'; + import { trackFormChanges } from '$lib/utils/unsaved-changes-util.svelte'; import { slide } from 'svelte/transition'; import { z } from 'zod/v4'; import FederatedIdentitiesInput from '../federated-identities-input.svelte'; @@ -14,10 +14,9 @@ callback }: { client: OidcClient; - callback: (federatedIdentities: OidcClientFederatedIdentity[]) => Promise; + callback: (federatedIdentities: OidcClientFederatedIdentity[]) => Promise; } = $props(); - let isLoading = $state(false); const isCIMDClient = $derived(client.clientType === 'cimd'); const formSchema = z.object({ @@ -34,12 +33,16 @@ ) }) }); - const { inputs, errors, ...form } = createForm(formSchema, { + const formStore = createForm(formSchema, { credentials: { federatedIdentities: - client.credentials?.federatedIdentities?.map((identity) => ({ ...identity })) ?? [] + client.credentials?.federatedIdentities?.map((identity) => ({ + ...identity, + publicKeys: identity.publicKeys?.length ? identity.publicKeys : undefined + })) ?? [] } }); + const { inputs, errors } = formStore; const hasFederatedIdentities = $derived($inputs.credentials.value.federatedIdentities.length > 0); @@ -61,63 +64,54 @@ subject: '', audience: '', jwks: '', - publicKeys: [], + publicKeys: undefined, replayProtection: true } ]; } - async function onSubmit() { - if (isCIMDClient) return; - - const data = form.validate(); - if (!data) return; - - isLoading = true; - await callback(data.credentials.federatedIdentities).finally(() => (isLoading = false)); + // Metadata document clients manage their own credentials, so there is nothing to save here. + if (!isCIMDClient) { + trackFormChanges( + () => formStore, + (data) => callback(data.credentials.federatedIdentities) + ); } -
- - -
-
- {m.federated_client_credentials()} - - {m.federated_client_credentials_description()} - - {m.docs()} - - -
- {#if !hasFederatedIdentities} - - {/if} + + +
+
+ {m.federated_client_credentials()} + + {m.federated_client_credentials_description()} + + {m.docs()} + +
- - {#if hasFederatedIdentities} -
- - - -
- {/if} - {#if !isCIMDClient && hasFederatedIdentities} - - - - {/if} - - + {#if !hasFederatedIdentities} + + {/if} +
+ + {#if hasFederatedIdentities} +
+ + + +
+ {/if} + diff --git a/frontend/src/routes/settings/admin/oidc-clients/[id]/oidc-client-token-lifetimes-card.svelte b/frontend/src/routes/settings/admin/oidc-clients/[id]/oidc-client-token-lifetimes-card.svelte index 99de2e06..a7031b6d 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/[id]/oidc-client-token-lifetimes-card.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/[id]/oidc-client-token-lifetimes-card.svelte @@ -1,11 +1,10 @@ -
- - - {m.token_lifetimes()} - {m.token_lifetimes_description()} - - -
- - -
- - - - - -
+ + + {m.token_lifetimes()} + {m.token_lifetimes_description()} + + +
+ + +
+ + diff --git a/frontend/src/routes/settings/admin/oidc-clients/[id]/scim-resource-provider-form.svelte b/frontend/src/routes/settings/admin/oidc-clients/[id]/scim-resource-provider-form.svelte index 2cda8e34..91f3a1b5 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/[id]/scim-resource-provider-form.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/[id]/scim-resource-provider-form.svelte @@ -5,8 +5,10 @@ import { m } from '$lib/paraglide/messages'; import ScimService from '$lib/services/scim-service'; import type { ScimServiceProvider, ScimServiceProviderCreate } from '$lib/types/scim.type'; + import { axiosErrorToast } from '$lib/utils/error-util'; import { preventDefault } from '$lib/utils/event-util'; import { createForm } from '$lib/utils/form-util'; + import { trackFormChanges } from '$lib/utils/unsaved-changes-util.svelte'; import { emptyToUndefined } from '$lib/utils/zod-util'; import { toast } from 'svelte-sonner'; import { z } from 'zod/v4'; @@ -17,7 +19,7 @@ oidcClientId }: { existingProvider?: ScimServiceProvider; - onSave: (provider: ScimServiceProviderCreate | null) => Promise; + onSave: (provider: ScimServiceProviderCreate | null) => Promise; oidcClientId: string; } = $props(); @@ -36,17 +38,39 @@ }); type FormSchema = typeof formSchema; - const { inputs, ...form } = createForm(formSchema, serviceProvider); + const formStore = createForm(formSchema, serviceProvider); + const { inputs } = formStore; - async function onSubmit() { - const data = form.validate(); - if (!data) return false; - return await onSave({ + async function saveProvider(data: z.infer) { + await onSave({ ...data, oidcClientId }); } + // Enable/disable/sync have their own buttons rather than going through the unsaved-changes + // bar, so they report their outcome themselves. + async function trySaveProvider() { + const data = formStore.validate(); + if (!data) return false; + try { + await saveProvider(data); + formStore.commit(data); + return true; + } catch (e) { + axiosErrorToast(e); + return false; + } + } + + async function onEnable() { + if (await trySaveProvider()) toast.success(m.scim_enabled_successfully()); + } + + // Tracked by the unsaved-changes bar once the provider exists; until then the Enable button + // saves it. + trackFormChanges(() => formStore, saveProvider, { enabled: () => !!existingProvider }); + async function onDisable() { openConfirmDialog({ title: m.disable_scim_provisioning(), @@ -58,9 +82,17 @@ label: m.disable(), destructive: true, action: async () => { - await onSave(null); - form.setValue('endpoint', ''); - form.setValue('token', ''); + try { + await onSave(null); + toast.success(m.scim_disabled_successfully()); + } catch (e) { + axiosErrorToast(e); + return; + } + formStore.setValue('endpoint', ''); + formStore.setValue('token', ''); + // The cleared fields are the saved state now, so they aren't unsaved changes. + formStore.commit({ endpoint: '', token: undefined }); } } }); @@ -78,8 +110,7 @@ confirm: { label: m.save_and_sync(), action: async () => { - const saved = await onSubmit(); - if (saved) { + if (await trySaveProvider()) { syncProvider(); } } @@ -106,7 +137,7 @@ } -
+
{m.disable()} + {:else} + {/if} -
diff --git a/frontend/src/routes/settings/admin/oidc-clients/federated-identities-input.svelte b/frontend/src/routes/settings/admin/oidc-clients/federated-identities-input.svelte index a68230c2..aae8f654 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/federated-identities-input.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/federated-identities-input.svelte @@ -30,11 +30,9 @@ children?: Snippet; } = $props(); - // The source can't be derived from the identity alone: it stays on "Public keys" while no key has been added yet - let keySources = $state([]); - - function keySourceFor(index: number, identity: OidcClientFederatedIdentity): KeySource { - return keySources[index] ?? (identity.publicKeys?.length ? 'publicKeys' : 'jwks'); + // An empty publicKeys array records that Public keys is selected before the first key is added + function keySourceFor(identity: OidcClientFederatedIdentity): KeySource { + return identity.publicKeys === undefined ? 'jwks' : 'publicKeys'; } function addFederatedIdentity() { @@ -45,7 +43,7 @@ subject: '', audience: '', jwks: '', - publicKeys: [], + publicKeys: undefined, replayProtection: true } ]; @@ -53,7 +51,6 @@ function removeFederatedIdentity(index: number) { federatedIdentities = federatedIdentities.filter((_, i) => i !== index); - keySources = keySources.filter((_, i) => i !== index); } function updateFederatedIdentity( @@ -69,15 +66,12 @@ // Only one of the two sources is ever submitted, so the one that is not selected is cleared function updateKeySource(index: number, source: KeySource) { - // The list is rebuilt in full so it stays aligned with the identities when one of them is removed - keySources = federatedIdentities.map((identity, i) => - i === index ? source : keySourceFor(i, identity) - ); - if (source === 'jwks') { - updateFederatedIdentity(index, 'publicKeys', []); - } else { - updateFederatedIdentity(index, 'jwks', ''); - } + const identity = federatedIdentities[index]; + federatedIdentities[index] = { + ...identity, + jwks: source === 'jwks' ? identity.jwks : '', + publicKeys: source === 'publicKeys' ? (identity.publicKeys ?? []) : undefined + }; } function getFieldError(index: number, field: keyof OidcClientFederatedIdentity): string | null { @@ -157,7 +151,7 @@ {m.signing_keys()} updateKeySource(i, value as KeySource)} {disabled} > @@ -173,7 +167,7 @@
- {#if keySourceFor(i, identity) === 'publicKeys'} + {#if keySourceFor(identity) === 'publicKeys'} (publicKeys); let pastedKey = $state(''); let error = $state(null); @@ -33,29 +30,27 @@ // A JWKS may repeat a key that was already added, and duplicate key IDs would make the key // to verify an assertion with ambiguous - const existingKeyIds = new Set(keys.map(getJwkKeyId)); + const existingKeyIds = new Set(publicKeys.map(getJwkKeyId)); const duplicate = result.keys.find((key) => existingKeyIds.has(getJwkKeyId(key))); if (duplicate) { error = m.public_key_already_added({ keyId: getJwkKeyId(duplicate) }); return; } - keys = [...keys, ...result.keys]; - onChange(keys); + onChange([...publicKeys, ...result.keys]); pastedKey = ''; error = null; } function removeKey(index: number) { - keys = keys.filter((_, i) => i !== index); - onChange(keys); + onChange(publicKeys.filter((_, i) => i !== index)); }
- {#if keys.length > 0} + {#if publicKeys.length > 0}
    - {#each keys as key, i (getJwkKeyId(key))} + {#each publicKeys as key, i (getJwkKeyId(key))}
  • Promise; + callback: (client: OidcClientCreateWithLogo | OidcClientUpdateWithLogo) => Promise; mode: 'create' | 'update'; } = $props(); let isLoading = $state(false); let showAdvancedOptions = $state(false); let logo = $state(); let darkLogo = $state(); - let logoDataURL: string | null = $state( - existingClient?.hasLogo ? cachedOidcClientLogo.getUrl(existingClient!.id) : null - ); - let darkLogoDataURL: string | null = $state( - existingClient?.hasDarkLogo ? cachedOidcClientLogo.getUrl(existingClient!.id, false) : null - ); + // What discarding restores the previews to; moves forward whenever a logo is saved. + let savedLogoDataURL = existingClient?.hasLogo + ? cachedOidcClientLogo.getUrl(existingClient!.id) + : null; + let savedDarkLogoDataURL = existingClient?.hasDarkLogo + ? cachedOidcClientLogo.getUrl(existingClient!.id, false) + : null; + let logoDataURL: string | null = $state(savedLogoDataURL); + let darkLogoDataURL: string | null = $state(savedDarkLogoDataURL); const isCIMDClient = $derived(existingClient?.clientType === 'cimd'); + // Defaults for new clients; existing clients keep the lifetimes edited in their own card. + const DEFAULT_ACCESS_TOKEN_DURATION_MINUTES = 60; + const DEFAULT_REFRESH_TOKEN_DURATION_MINUTES = 30 * 24 * 60; + const client = { id: '', name: existingClient?.name || '', @@ -57,9 +66,7 @@ launchURL: existingClient?.launchURL || '', logoUrl: '', darkLogoUrl: '', - pkceSupported: existingClient?.pkceSupported || false, - accessTokenDurationMinutes: existingClient?.accessTokenDurationMinutes ?? 60, - refreshTokenDurationMinutes: existingClient?.refreshTokenDurationMinutes ?? 30 * 24 * 60 + pkceSupported: existingClient?.pkceSupported || false }; const formSchema = z.object({ @@ -84,58 +91,83 @@ skipConsent: z.boolean(), launchURL: optionalUrl, logoUrl: optionalUrl, - darkLogoUrl: optionalUrl, - accessTokenDurationMinutes: z - .number() - .min(1) - .max(365 * 24 * 60) - .int(), - refreshTokenDurationMinutes: z - .number() - .min(1) - .max(365 * 24 * 60) - .int() + darkLogoUrl: optionalUrl }); type FormSchema = typeof formSchema; - const { inputs, ...form } = createForm(formSchema, client); + const formStore = createForm(formSchema, client); + const { inputs } = formStore; const pkcePromptNeeded = $derived(!$inputs.pkceEnabled.value && client.pkceSupported); - async function onSubmit() { - const data = form.validate(); - if (!data) return; - isLoading = true; - - const success = await callback({ + async function saveClient(data: z.infer) { + await callback({ ...data, credentials: existingClient?.credentials ?? { federatedIdentities: [], secrets: [] }, logo: $inputs.logoUrl?.value ? undefined : logo, logoUrl: $inputs.logoUrl?.value, darkLogo: $inputs.darkLogoUrl?.value ? undefined : darkLogo, darkLogoUrl: $inputs.darkLogoUrl?.value, - isGroupRestricted: existingClient?.isGroupRestricted ?? true + isGroupRestricted: existingClient?.isGroupRestricted ?? true, + // The token lifetimes are edited in their own card. The current values are sent along + // because the backend falls back to the defaults for missing ones. + accessTokenDurationMinutes: + existingClient?.accessTokenDurationMinutes ?? DEFAULT_ACCESS_TOKEN_DURATION_MINUTES, + refreshTokenDurationMinutes: + existingClient?.refreshTokenDurationMinutes ?? DEFAULT_REFRESH_TOKEN_DURATION_MINUTES }); const hasLogo = logo != null || !!$inputs.logoUrl?.value; const hasDarkLogo = darkLogo != null || !!$inputs.darkLogoUrl?.value; - if (success && existingClient) { + if (existingClient) { if (hasLogo) { logoDataURL = cachedOidcClientLogo.getUrl(existingClient.id); } if (hasDarkLogo) { darkLogoDataURL = cachedOidcClientLogo.getUrl(existingClient.id, false); } + savedLogoDataURL = logoDataURL; + savedDarkLogoDataURL = darkLogoDataURL; + // The uploaded file has been persisted, so it's no longer "pending" for dirty-tracking. + logo = undefined; + darkLogo = undefined; + } else { + formStore.reset(); } + } - if (success && !existingClient) form.reset(); - isLoading = false; + // Create mode has its own Save button rather than going through the unsaved-changes bar. + async function onSubmit() { + const data = formStore.validate(); + if (!data) return; + isLoading = true; + try { + await saveClient(data); + } catch (e) { + axiosErrorToast(e); + } finally { + isLoading = false; + } + } + + function discardLogoChanges() { + logo = undefined; + darkLogo = undefined; + logoDataURL = savedLogoDataURL; + darkLogoDataURL = savedDarkLogoDataURL; + } + + if (mode === 'update') { + trackFormChanges(() => formStore, saveClient, { + dirty: () => logo !== undefined || darkLogo !== undefined, + discard: discardLogoChanges + }); } function onLogoChange(input: File | string | null, light: boolean = true) { if (input == null) return; - const logoUrlInput = light ? $inputs.logoUrl : $inputs.darkLogoUrl; + const logoUrlKey = light ? 'logoUrl' : 'darkLogoUrl'; if (typeof input === 'string') { if (light) { @@ -145,7 +177,7 @@ darkLogo = null; darkLogoDataURL = input || null; } - logoUrlInput!.value = input; + formStore.setValue(logoUrlKey, input); } else { if (light) { logo = input; @@ -154,9 +186,7 @@ darkLogo = input; darkLogoDataURL = URL.createObjectURL(input); } - if (logoUrlInput) { - logoUrlInput.value = ''; - } + formStore.setValue(logoUrlKey, ''); } } @@ -342,6 +372,8 @@ )} /> - + {#if mode === 'create'} + + {/if}
diff --git a/frontend/src/routes/settings/admin/user-groups/+page.svelte b/frontend/src/routes/settings/admin/user-groups/+page.svelte index a7e0704f..c3570c20 100644 --- a/frontend/src/routes/settings/admin/user-groups/+page.svelte +++ b/frontend/src/routes/settings/admin/user-groups/+page.svelte @@ -5,7 +5,6 @@ import { m } from '$lib/paraglide/messages'; import UserGroupService from '$lib/services/user-group-service'; import type { UserGroupCreate } from '$lib/types/user-group.type'; - import { axiosErrorToast } from '$lib/utils/error-util'; import { LucideMinus, UserCog, UserPlus } from '@lucide/svelte'; import { toast } from 'svelte-sonner'; import { slide } from 'svelte/transition'; @@ -17,18 +16,9 @@ const userGroupService = new UserGroupService(); async function createUserGroup(userGroup: UserGroupCreate) { - let success = true; - await userGroupService - .create(userGroup) - .then((createdUserGroup) => { - toast.success(m.user_group_created_successfully()); - goto(`/settings/admin/user-groups/${createdUserGroup.id}`); - }) - .catch((e) => { - axiosErrorToast(e); - success = false; - }); - return success; + const createdUserGroup = await userGroupService.create(userGroup); + toast.success(m.user_group_created_successfully()); + goto(`/settings/admin/user-groups/${createdUserGroup.id}`); } diff --git a/frontend/src/routes/settings/admin/user-groups/[id]/+page.svelte b/frontend/src/routes/settings/admin/user-groups/[id]/+page.svelte index 66b53b17..b5cdff05 100644 --- a/frontend/src/routes/settings/admin/user-groups/[id]/+page.svelte +++ b/frontend/src/routes/settings/admin/user-groups/[id]/+page.svelte @@ -1,7 +1,6 @@ @@ -120,12 +105,6 @@ bind:selectedUserIds={userGroup.userIds} selectionDisabled={!!userGroup.ldapId && $appConfigStore.ldapEnabled} /> -
- -
@@ -141,11 +120,6 @@ bind:this={oidcClientSelectionRef} bind:selectedGroupIds={userGroup.allowedOidcClientIds} /> -
- -
@@ -160,9 +134,6 @@ -
- -
diff --git a/frontend/src/routes/settings/admin/user-groups/[id]/oidc-client-selection.svelte b/frontend/src/routes/settings/admin/user-groups/[id]/oidc-client-selection.svelte index a34daa7f..2f910a64 100644 --- a/frontend/src/routes/settings/admin/user-groups/[id]/oidc-client-selection.svelte +++ b/frontend/src/routes/settings/admin/user-groups/[id]/oidc-client-selection.svelte @@ -18,6 +18,15 @@ const oidcClientService = new OidcService(); let tableRef: AdvancedTable; + let unrestrictedClientIds = $state([]); + + function getTableSelectedIds() { + return [...new Set([...selectedGroupIds, ...unrestrictedClientIds])]; + } + + function setTableSelectedIds(clientIds: string[]) { + selectedGroupIds = clientIds.filter((id) => !unrestrictedClientIds.includes(id)); + } export function refresh() { return tableRef?.refresh(); @@ -56,8 +65,7 @@ async function fetchCallback(requestOptions: ListRequestOptions) { const clients = await oidcClientService.listClients(requestOptions); - const unrestrictedClientIds = clients.data.filter((c) => !c.isGroupRestricted).map((c) => c.id); - selectedGroupIds = [...new Set([...selectedGroupIds, ...unrestrictedClientIds])]; + unrestrictedClientIds = clients.data.filter((c) => !c.isGroupRestricted).map((c) => c.id); return clients; } @@ -82,7 +90,7 @@ id="oidc-client-selection" {fetchCallback} defaultSort={{ column: 'name', direction: 'asc' }} - bind:selectedIds={selectedGroupIds} + bind:selectedIds={getTableSelectedIds, setTableSelectedIds} rowSelectionDisabled={(item) => !item.isGroupRestricted} {columns} /> diff --git a/frontend/src/routes/settings/admin/user-groups/user-group-form.svelte b/frontend/src/routes/settings/admin/user-groups/user-group-form.svelte index 76f32951..4eada3aa 100644 --- a/frontend/src/routes/settings/admin/user-groups/user-group-form.svelte +++ b/frontend/src/routes/settings/admin/user-groups/user-group-form.svelte @@ -4,8 +4,10 @@ import { m } from '$lib/paraglide/messages'; import appConfigStore from '$lib/stores/application-configuration-store'; import type { UserGroupCreate } from '$lib/types/user-group.type'; + import { axiosErrorToast } from '$lib/utils/error-util'; import { preventDefault } from '$lib/utils/event-util'; import { createForm } from '$lib/utils/form-util'; + import { trackFormChanges } from '$lib/utils/unsaved-changes-util.svelte'; import { z } from 'zod/v4'; let { @@ -13,7 +15,7 @@ existingUserGroup }: { existingUserGroup?: UserGroupCreate; - callback: (userGroup: UserGroupCreate) => Promise; + callback: (userGroup: UserGroupCreate) => Promise; } = $props(); let isLoading = $state(false); @@ -31,7 +33,8 @@ }); type FormSchema = typeof formSchema; - const { inputs, ...form } = createForm(formSchema, userGroup); + const formStore = createForm(formSchema, userGroup); + const { inputs } = formStore; function onFriendlyNameInput(e: any) { if (!hasManualNameEdit) { @@ -43,17 +46,31 @@ hasManualNameEdit = true; } - async function onSubmit() { - const data = form.validate(); - if (!data) return; - isLoading = true; - const success = await callback(data); + async function saveUserGroup(data: z.infer) { + await callback(data); // Reset form if user group was successfully created - if (success && !existingUserGroup) { - form.reset(); + if (!existingUserGroup) { + formStore.reset(); hasManualNameEdit = false; } - isLoading = false; + } + + // Create mode has its own Save button rather than going through the unsaved-changes bar. + async function onSubmit() { + const data = formStore.validate(); + if (!data) return; + isLoading = true; + try { + await saveUserGroup(data); + } catch (e) { + axiosErrorToast(e); + } finally { + isLoading = false; + } + } + + if (existingUserGroup) { + trackFormChanges(() => formStore, saveUserGroup); } @@ -77,8 +94,10 @@ /> -
- -
+ {#if !existingUserGroup} +
+ +
+ {/if} diff --git a/frontend/src/routes/settings/admin/users/+page.svelte b/frontend/src/routes/settings/admin/users/+page.svelte index 3db413ea..c1e854d8 100644 --- a/frontend/src/routes/settings/admin/users/+page.svelte +++ b/frontend/src/routes/settings/admin/users/+page.svelte @@ -9,7 +9,6 @@ import UserService from '$lib/services/user-service'; import appConfigStore from '$lib/stores/application-configuration-store'; import type { UserCreate } from '$lib/types/user.type'; - import { axiosErrorToast } from '$lib/utils/error-util'; import { ChevronDown, LucideMinus, UserPen, UserPlus } from '@lucide/svelte'; import { toast } from 'svelte-sonner'; import { slide } from 'svelte/transition'; @@ -28,17 +27,12 @@ const userService = new UserService(); async function createUser(user: UserCreate) { - let success = true; - await userService - .create(user) - .then(() => toast.success(m.user_created_successfully())) - .catch((e) => { - axiosErrorToast(e); - success = false; - }); - - await userListRef.refresh(); - return success; + try { + await userService.create(user); + toast.success(m.user_created_successfully()); + } finally { + await userListRef.refresh(); + } } diff --git a/frontend/src/routes/settings/admin/users/[id]/+page.svelte b/frontend/src/routes/settings/admin/users/[id]/+page.svelte index bd8f206d..e440d1f5 100644 --- a/frontend/src/routes/settings/admin/users/[id]/+page.svelte +++ b/frontend/src/routes/settings/admin/users/[id]/+page.svelte @@ -2,7 +2,6 @@ import CustomClaimsInput from '$lib/components/form/custom-claims-input.svelte'; import ProfilePictureSettings from '$lib/components/form/profile-picture-settings.svelte'; import Badge from '$lib/components/ui/badge/badge.svelte'; - import { Button } from '$lib/components/ui/button'; import * as Card from '$lib/components/ui/card'; import * as Item from '$lib/components/ui/item/index.js'; import * as Tabs from '$lib/components/ui/tabs'; @@ -14,6 +13,7 @@ import type { Passkey } from '$lib/types/passkey.type'; import type { UserCreate } from '$lib/types/user.type'; import { axiosErrorToast } from '$lib/utils/error-util'; + import { trackUnsavedValue } from '$lib/utils/unsaved-changes-util.svelte'; import { KeyRound, LucideChevronLeft } from '@lucide/svelte'; import { toast } from 'svelte-sonner'; import { backNavigate } from '../navigate-back-util'; @@ -31,36 +31,25 @@ const customClaimService = new CustomClaimService(); const backNavigation = backNavigate('/settings/admin/users'); - async function updateUserGroups(userIds: string[]) { - await userService - .updateUserGroups(user.id, userIds) - .then(() => toast.success(m.user_groups_updated_successfully())) - .catch((e) => { - axiosErrorToast(e); - }); - } - async function updateUser(updatedUser: UserCreate) { - let success = true; - await userService - .update(user.id, updatedUser) - .then(() => toast.success(m.user_updated_successfully())) - .catch((e) => { - axiosErrorToast(e); - success = false; - }); - - return success; + await userService.update(user.id, updatedUser); } - async function updateCustomClaims() { - await customClaimService - .updateUserCustomClaims(user.id, user.customClaims) - .then(() => toast.success(m.custom_claims_updated_successfully())) - .catch((e) => { - axiosErrorToast(e); - }); - } + trackUnsavedValue( + () => user.userGroupIds, + (userGroupIds) => { + user.userGroupIds = userGroupIds; + }, + (userGroupIds) => userService.updateUserGroups(user.id, userGroupIds) + ); + + trackUnsavedValue( + () => user.customClaims, + (customClaims) => { + user.customClaims = customClaims; + }, + (customClaims) => customClaimService.updateUserCustomClaims(user.id, customClaims) + ); async function updateProfilePicture(image: File) { await userService @@ -137,13 +126,6 @@ bind:selectedGroupIds={user.userGroupIds} selectionDisabled={!!user.ldapId && $appConfigStore.ldapEnabled} /> -
- -
@@ -179,9 +161,6 @@ -
- -
diff --git a/frontend/src/routes/settings/admin/users/user-form.svelte b/frontend/src/routes/settings/admin/users/user-form.svelte index b68e7f97..7ec00845 100644 --- a/frontend/src/routes/settings/admin/users/user-form.svelte +++ b/frontend/src/routes/settings/admin/users/user-form.svelte @@ -8,8 +8,10 @@ import { m } from '$lib/paraglide/messages'; import appConfigStore from '$lib/stores/application-configuration-store'; import type { User, UserCreate } from '$lib/types/user.type'; + import { axiosErrorToast } from '$lib/utils/error-util'; import { preventDefault } from '$lib/utils/event-util'; import { createForm } from '$lib/utils/form-util'; + import { trackFormChanges } from '$lib/utils/unsaved-changes-util.svelte'; import { emptyToUndefined, usernameSchema } from '$lib/utils/zod-util'; import { LucideMailCheck, LucideMailWarning } from '@lucide/svelte'; import { get } from 'svelte/store'; @@ -22,7 +24,7 @@ }: { existingUser?: User; emailsVerifiedPerDefault?: boolean; - callback: (user: UserCreate) => Promise; + callback: (user: UserCreate) => Promise; } = $props(); let isLoading = $state(false); @@ -54,15 +56,27 @@ }); type FormSchema = typeof formSchema; - const { inputs, ...form } = createForm(formSchema, user); + const formStore = createForm(formSchema, user); + const { inputs } = formStore; + + async function saveUser(data: z.infer) { + await callback(data); + // Reset form if user was successfully created + if (!existingUser) formStore.reset(); + } + + // Create mode has its own Save button rather than going through the unsaved-changes bar. async function onSubmit() { - const data = form.validate(); + const data = formStore.validate(); if (!data) return; isLoading = true; - const success = await callback(data); - // Reset form if user was successfully created - if (success && !existingUser) form.reset(); - isLoading = false; + try { + await saveUser(data); + } catch (e) { + axiosErrorToast(e); + } finally { + isLoading = false; + } } function onNameInput() { if (!hasManualDisplayNameEdit) { @@ -71,6 +85,10 @@ }`.trim(); } } + + if (existingUser) { + trackFormChanges(() => formStore, saveUser); + }
@@ -130,8 +148,10 @@ bind:checked={$inputs.disabled.value} /> -
- -
+ {#if !existingUser} +
+ +
+ {/if}
diff --git a/tests/specs/account-settings.spec.ts b/tests/specs/account-settings.spec.ts index b0179d7a..02f89462 100644 --- a/tests/specs/account-settings.spec.ts +++ b/tests/specs/account-settings.spec.ts @@ -3,6 +3,7 @@ import { emailVerificationTokens, users } from '../data'; import authUtil from '../utils/auth.util'; import { cleanupBackend } from '../utils/cleanup.util'; import passkeyUtil from '../utils/passkey.util'; +import { saveUnsavedChanges } from '../utils/unsaved-changes.util'; test.beforeEach(async () => await cleanupBackend()); @@ -15,11 +16,38 @@ test('Update account details', async ({ page }) => { await page.getByLabel('Display Name').fill('Timothy Apple'); await page.getByLabel('Email').fill('timothy.apple@test.com'); await page.getByLabel('Username').fill('timothy'); - await page.getByRole('button', { name: 'Save' }).click(); + await saveUnsavedChanges(page); +}); - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Account details updated successfully' - ); +test('Failed account update remains dirty and can be retried', async ({ page }) => { + await page.goto('/settings/account'); + + let failedUpdates = 0; + await page.route('**/api/users/me', async (route) => { + if (route.request().method() !== 'PUT' || failedUpdates > 0) { + await route.fallback(); + return; + } + + failedUpdates++; + await route.fulfill({ + status: 500, + contentType: 'application/json', + body: JSON.stringify({ error: 'Temporary account update failure' }) + }); + }); + + const displayName = page.getByLabel('Display Name'); + await displayName.fill('Retryable Account'); + await page.getByRole('button', { name: 'Save', exact: true }).click(); + + await expect(page.getByText('Temporary account update failure', { exact: true })).toBeVisible(); + await expect(page.getByRole('button', { name: 'Save', exact: true })).toBeVisible(); + expect(failedUpdates).toBe(1); + + await saveUnsavedChanges(page); + await page.reload(); + await expect(page.getByLabel('Display Name')).toHaveValue('Retryable Account'); }); test('Update account details fails with already taken email', async ({ page }) => { @@ -27,9 +55,9 @@ test('Update account details fails with already taken email', async ({ page }) = await page.getByLabel('Email').fill(users.craig.email); - await page.getByRole('button', { name: 'Save' }).click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); - await expect(page.locator('[data-type="error"]')).toHaveText('Email is already in use'); + await expect(page.getByText('Email is already in use', { exact: true })).toBeVisible(); }); test('Update account details fails with already taken username', async ({ page }) => { @@ -37,9 +65,9 @@ test('Update account details fails with already taken username', async ({ page } await page.getByLabel('Username').fill(users.craig.username); - await page.getByRole('button', { name: 'Save' }).click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); - await expect(page.locator('[data-type="error"]')).toHaveText('Username is already in use'); + await expect(page.getByText('Username is already in use', { exact: true })).toBeVisible(); }); test('Update account details fails with already taken username in different casing', async ({ @@ -49,9 +77,9 @@ test('Update account details fails with already taken username in different casi await page.getByLabel('Username').fill(users.craig.username.toUpperCase()); - await page.getByRole('button', { name: 'Save' }).click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); - await expect(page.locator('[data-type="error"]')).toHaveText('Username is already in use'); + await expect(page.getByText('Username is already in use', { exact: true })).toBeVisible(); }); test('Change Locale', async ({ page }) => { diff --git a/tests/specs/api.spec.ts b/tests/specs/api.spec.ts index 3cff2a82..8f90f0a1 100644 --- a/tests/specs/api.spec.ts +++ b/tests/specs/api.spec.ts @@ -3,6 +3,7 @@ import * as jose from 'jose'; import { apis, oidcClients } from '../data'; import { cleanupBackend } from '../utils/cleanup.util'; import * as oidcUtil from '../utils/oidc.util'; +import { saveUnsavedChanges } from '../utils/unsaved-changes.util'; test.beforeEach(async () => await cleanupBackend()); @@ -69,9 +70,7 @@ test('Edit the name of an API', async ({ page }) => { await page.goto(`/settings/admin/apis/${apis.orders.id}`); await page.getByLabel('Name', { exact: true }).fill('Orders API renamed'); - await page.getByRole('button', { name: 'Save' }).nth(0).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText('API updated successfully'); + await saveUnsavedChanges(page); await page.reload(); await expect(page.getByLabel('Name', { exact: true })).toHaveValue('Orders API renamed'); @@ -84,11 +83,7 @@ test('Add a permission to an API', async ({ page }) => { await page.getByRole('button', { name: 'Add another' }).click(); await page.getByPlaceholder('Permission', { exact: true }).last().fill('ship:orders'); await page.getByPlaceholder('Name', { exact: true }).last().fill('Ship orders'); - await page.getByRole('button', { name: 'Save' }).nth(1).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Permissions updated successfully' - ); + await saveUnsavedChanges(page); await page.reload(); // The two seeded permissions plus the newly added one @@ -259,9 +254,7 @@ test('Allow all metadata document clients for an API', async ({ page }) => { await page.getByRole('tab', { name: 'Metadata document clients' }).click(); await page.getByLabel('Allow all metadata document clients').click(); await page.getByLabel(apis.orders.permissions.readOrders.name, { exact: true }).click(); - await page.getByRole('button', { name: 'Save' }).nth(2).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText('API access updated successfully'); + await saveUnsavedChanges(page); await page.reload(); await page.getByRole('tab', { name: 'Metadata document clients' }).click(); diff --git a/tests/specs/application-configuration.spec.ts b/tests/specs/application-configuration.spec.ts index 39a501d7..378a3c70 100644 --- a/tests/specs/application-configuration.spec.ts +++ b/tests/specs/application-configuration.spec.ts @@ -1,5 +1,6 @@ import { expect, test } from '@playwright/test'; import { cleanupBackend } from '../utils/cleanup.util'; +import { saveUnsavedChanges } from '../utils/unsaved-changes.util'; test.beforeEach(async ({ page }) => { await cleanupBackend(); @@ -13,11 +14,7 @@ test('Update general configuration', async ({ page }) => { await page.getByRole('button', { name: 'Home Page' }).click(); await page.getByRole('option', { name: 'My Apps' }).click(); - await page.getByRole('button', { name: 'Save' }).first().click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Application configuration updated successfully' - ); + await saveUnsavedChanges(page); await page.reload(); @@ -28,6 +25,95 @@ test('Update general configuration', async ({ page }) => { await page.waitForURL('/settings/apps'); }); +test('Save configuration from every editable tab together', async ({ page }) => { + await page.getByLabel('Application Name', { exact: true }).fill('Combined Settings'); + await page.getByRole('tab', { name: 'User Creation' }).click(); + await page.getByRole('button', { name: 'Enable User Signups' }).click(); + await page.getByRole('option', { name: 'Open Signup' }).click(); + await page.getByRole('tab', { name: 'Passkeys' }).click(); + await page.getByRole('button', { name: 'User verification' }).click(); + await page.getByRole('option', { name: 'Preferred' }).click(); + await page.getByRole('tab', { name: 'Email' }).click(); + await page.getByLabel('SMTP Host').fill('smtp.combined.test'); + await page.getByLabel('SMTP Port').fill('587'); + await page.getByLabel('SMTP From').fill('combined@example.com'); + await page.getByRole('tab', { name: 'OIDC' }).click(); + await page.getByRole('textbox').fill('https://combined.example.com/*'); + + await saveUnsavedChanges(page); + + await page.getByRole('tab', { name: 'General' }).click(); + await expect(page.getByLabel('Application Name', { exact: true })).toHaveValue( + 'Combined Settings' + ); + await page.getByRole('tab', { name: 'Email' }).click(); + await expect(page.getByLabel('SMTP Host')).toHaveValue('smtp.combined.test'); + await page.getByRole('tab', { name: 'Passkeys' }).click(); + await expect(page.getByRole('button', { name: 'User verification' })).toContainText('Preferred'); + await page.getByRole('tab', { name: 'OIDC' }).click(); + await expect(page.getByRole('textbox')).toHaveValue('https://combined.example.com/*'); + + await page.reload(); + await page.getByRole('tab', { name: 'General' }).click(); + await expect(page.getByLabel('Application Name', { exact: true })).toHaveValue( + 'Combined Settings' + ); + await page.getByRole('tab', { name: 'User Creation' }).click(); + await page.getByRole('button', { name: 'Enable User Signups' }).click(); + await expect(page.getByRole('option', { name: 'Open Signup' })).toHaveAttribute( + 'aria-selected', + 'true' + ); + await page.keyboard.press('Escape'); + await page.getByRole('tab', { name: 'Email' }).click(); + await expect(page.getByLabel('SMTP Host')).toHaveValue('smtp.combined.test'); + await page.getByRole('tab', { name: 'Passkeys' }).click(); + await expect(page.getByRole('button', { name: 'User verification' })).toContainText('Preferred'); + await page.getByRole('tab', { name: 'OIDC' }).click(); + await expect(page.getByRole('textbox')).toHaveValue('https://combined.example.com/*'); +}); + +test('Invalid hidden configuration prevents every section from being saved', async ({ page }) => { + let updateRequests = 0; + page.on('request', (request) => { + if ( + request.method() === 'PUT' && + new URL(request.url()).pathname === '/api/application-configuration' + ) { + updateRequests++; + } + }); + + await page.getByLabel('Application Name', { exact: true }).fill('Validated Together'); + await page.getByRole('tab', { name: 'Email' }).click(); + await page.getByLabel('SMTP Host').fill('smtp.validation.test'); + await page.getByLabel('SMTP From').fill('validation@example.com'); + await page.getByRole('tab', { name: 'General' }).click(); + + await page.getByRole('button', { name: 'Save', exact: true }).click(); + + const emailTab = page.getByRole('tab', { name: 'Email' }); + const smtpPort = page.getByLabel('SMTP Port'); + await expect(page.getByText('Please fix the highlighted errors before saving')).toBeVisible(); + await expect(emailTab).toHaveAttribute('aria-selected', 'true'); + await expect(smtpPort).toHaveAttribute('aria-invalid', 'true'); + await expect(smtpPort).toBeFocused(); + expect(updateRequests).toBe(0); + + await smtpPort.fill('587'); + await saveUnsavedChanges(page); + + await page.reload(); + await page.getByRole('tab', { name: 'General' }).click(); + await expect(page.getByLabel('Application Name', { exact: true })).toHaveValue( + 'Validated Together' + ); + await page.getByRole('tab', { name: 'Email' }).click(); + await expect(page.getByLabel('SMTP Host')).toHaveValue('smtp.validation.test'); + await expect(page.getByLabel('SMTP Port')).toHaveValue('587'); + await expect(page.getByLabel('SMTP From')).toHaveValue('validation@example.com'); +}); + test.describe('Update user creation configuration', () => { test.beforeEach(async ({ page }) => { await page.getByRole('tab', { name: 'User Creation' }).click(); @@ -37,11 +123,7 @@ test.describe('Update user creation configuration', () => { await page.getByRole('button', { name: 'Enable User Signups' }).click(); await page.getByRole('option', { name: 'Open Signup' }).click(); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]').last()).toHaveText( - 'User creation settings updated successfully.' - ); + await saveUnsavedChanges(page); await page.reload(); @@ -59,11 +141,7 @@ test.describe('Update user creation configuration', () => { await expect(designersOption).toBeChecked(); await page.keyboard.press('Escape'); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]').last()).toHaveText( - 'User creation settings updated successfully.' - ); + await saveUnsavedChanges(page); await page.reload(); @@ -81,11 +159,7 @@ test.describe('Update user creation configuration', () => { await page.getByPlaceholder('Key').nth(1).fill('another-claim'); await page.getByPlaceholder('Value').nth(1).fill('another-value'); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]').last()).toHaveText( - 'User creation settings updated successfully.' - ); + await saveUnsavedChanges(page); await page.reload(); @@ -114,10 +188,7 @@ test('Update passkey configuration', async ({ page }) => { await expect(allowSyncedPasskeys).toBeChecked(); await allowSyncedPasskeys.click(); - await page.getByRole('button', { name: 'Save', exact: true }).click(); - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Passkey configuration updated successfully' - ); + await saveUnsavedChanges(page); const registrationResponse = await page.request.get('/api/webauthn/register/start'); expect(registrationResponse.ok()).toBeTruthy(); @@ -157,11 +228,7 @@ test('Update email configuration', async ({ page }) => { await page.getByLabel('Email Login Code from Admin').click(); await page.getByLabel('API Key Expiration').click(); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Email configuration updated successfully' - ); + await saveUnsavedChanges(page); await page.reload(); @@ -176,7 +243,52 @@ test('Update email configuration', async ({ page }) => { await expect(page.getByLabel('API Key Expiration')).toBeChecked(); }); +test('Save LDAP configuration while LDAP remains disabled', async ({ page }) => { + await page.getByRole('tab', { name: 'LDAP' }).click(); + + const disableButton = page.getByRole('button', { name: 'Disable', exact: true }); + if (await disableButton.isVisible()) { + await disableButton.click(); + await expect(page.getByRole('button', { name: 'Enable', exact: true })).toBeVisible(); + } + + const softDeleteUsers = page.getByRole('switch', { name: 'Keep disabled users from LDAP' }); + const originalValue = await softDeleteUsers.isChecked(); + await softDeleteUsers.click(); + await expect(page.getByText('You have unsaved changes', { exact: true })).toBeVisible(); + + await page.getByRole('button', { name: 'Discard', exact: true }).click(); + await expect(softDeleteUsers).toBeChecked({ checked: originalValue }); + + await softDeleteUsers.click(); + await saveUnsavedChanges(page); + + await page.reload(); + await page.getByRole('tab', { name: 'LDAP' }).click(); + await expect(page.getByRole('button', { name: 'Enable', exact: true })).toBeVisible(); + await expect(page.getByRole('switch', { name: 'Keep disabled users from LDAP' })).toBeChecked({ + checked: !originalValue + }); +}); + test.describe('Update application images', () => { + test('should detect image resets as unsaved changes', async ({ page }) => { + await page + .getByLabel('Background Image', { exact: true }) + .setInputFiles('resources/images/clouds.jpg'); + await saveUnsavedChanges(page); + + await page + .getByRole('button', { name: 'Reset to default Background Image', exact: true }) + .click(); + await expect(page.getByText('You have unsaved changes', { exact: true })).toBeVisible(); + + await page.getByRole('button', { name: 'Discard', exact: true }).click(); + await expect( + page.getByRole('button', { name: 'Reset to default Background Image', exact: true }) + ).toBeVisible(); + }); + test('should upload images and reset custom logos', async ({ page }) => { await page .getByLabel('Favicon', { exact: true }) @@ -196,11 +308,7 @@ test.describe('Update application images', () => { await page .getByLabel('Background Image', { exact: true }) .setInputFiles('resources/images/clouds.jpg'); - await page.getByRole('button', { name: 'Save', exact: true }).nth(1).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Images updated successfully. It may take a few minutes to update.' - ); + await saveUnsavedChanges(page); await page.request .get('/api/application-images/favicon') @@ -234,15 +342,11 @@ test.describe('Update application images', () => { ); }) ); - await page.getByRole('button', { name: 'Save', exact: true }).nth(1).click(); + await saveUnsavedChanges(page); for (const response of await Promise.all(logoDeleteResponses)) { expect(response.status()).toBe(204); } - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Images updated successfully. It may take a few minutes to update.' - ); - // Without a custom logo the endpoint falls back to the logo bundled with Pocket ID await page.request .get('/api/application-images/logo?light=true') @@ -264,10 +368,8 @@ test.describe('Update application images', () => { const emailLogoInput = page.getByLabel('Email Logo', { exact: true }); await emailLogoInput.setInputFiles('resources/images/cloud-logo.svg'); - await page.getByRole('button', { name: 'Save', exact: true }).nth(1).click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); - await expect(page.locator('[data-type="error"]')).toHaveText( - 'File must be of type PNG or JPEG' - ); + await expect(page.getByText('File must be of type PNG or JPEG', { exact: true })).toBeVisible(); }); }); diff --git a/tests/specs/navigation.spec.ts b/tests/specs/navigation.spec.ts index b3b3c3df..5e1fdf6e 100644 --- a/tests/specs/navigation.spec.ts +++ b/tests/specs/navigation.spec.ts @@ -20,3 +20,24 @@ test('keyboard focus stays on sidebar link after navigating', async ({ page }) = await page.waitForURL('**/settings/audit-log'); await expect(auditLog).toBeFocused(); }); + +test('unsaved changes block navigation until they are discarded', async ({ page }) => { + await page.goto('/settings/account'); + + const displayName = page.getByLabel('Display Name'); + const originalDisplayName = await displayName.inputValue(); + await displayName.fill('Pending navigation'); + await expect(page.getByText('You have unsaved changes', { exact: true })).toBeVisible(); + + const myApps = page.getByRole('link', { name: 'My Apps' }); + await myApps.click(); + await expect(page).toHaveURL(/\/settings\/account$/); + + await page.getByRole('button', { name: 'Discard', exact: true }).click(); + await expect(displayName).toHaveValue(originalDisplayName); + await expect(page.getByText('You have unsaved changes', { exact: true })).toHaveCount(0); + + await myApps.click(); + await expect(page).toHaveURL(/\/settings\/apps$/); + await expect(page.getByText('You have unsaved changes', { exact: true })).toHaveCount(0); +}); diff --git a/tests/specs/oidc-client-settings.spec.ts b/tests/specs/oidc-client-settings.spec.ts index e0c3471c..7dd9b72c 100644 --- a/tests/specs/oidc-client-settings.spec.ts +++ b/tests/specs/oidc-client-settings.spec.ts @@ -3,6 +3,7 @@ import * as jose from 'jose'; import { oidcClients, userGroups } from '../data'; import { cleanupBackend } from '../utils/cleanup.util'; import * as oidcUtil from '../utils/oidc.util'; +import { saveUnsavedChanges } from '../utils/unsaved-changes.util'; test.beforeEach(async () => await cleanupBackend()); @@ -77,12 +78,7 @@ test('Edit OIDC client', async ({ page }) => { await page.locator('[role="tab"][data-value="dark-logo"]').first().click(); await page.setInputFiles('#oidc-client-logo-dark', 'resources/images/cloud-logo.png'); await page.getByLabel('Client Launch URL').fill(oidcClient.launchURL); - const clientForm = page.getByLabel('Name').locator('xpath=ancestor::form'); - await clientForm.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'OIDC client updated successfully' - ); + await saveUnsavedChanges(page); await expect(page.getByRole('img', { name: 'Nextcloud updated logo' }).first()).toBeVisible(); await page.request .get(`/api/oidc/clients/${oidcClient.id}/logo`) @@ -134,8 +130,7 @@ test('Update OIDC client token lifetimes', async ({ page }) => { await expect(refreshLifetime).toHaveValue('720'); await refreshLifetime.fill('336'); - await card.getByRole('button', { name: 'Save' }).click(); - await expect(page.getByText('OIDC client updated successfully', { exact: true })).toBeVisible(); + await saveUnsavedChanges(page); await page.reload(); await expect(card.getByLabel('Access token lifetime', { exact: true })).toHaveValue('90'); @@ -146,21 +141,44 @@ test('Update OIDC client token lifetimes', async ({ page }) => { await expect(card.getByLabel('Refresh token inactivity timeout unit')).toHaveText('Days'); await card.getByLabel('Access token lifetime', { exact: true }).fill('0'); - await card.getByRole('button', { name: 'Save' }).click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); await expect(card.getByText('Token lifetime must be at least 1 minute.')).toBeVisible(); await card.getByLabel('Access token lifetime', { exact: true }).fill('525601'); - await card.getByRole('button', { name: 'Save' }).click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); await expect(card.getByText('Token lifetime cannot exceed 365 days.')).toBeVisible(); await card.getByLabel('Access token lifetime', { exact: true }).fill('1.5'); - await card.getByRole('button', { name: 'Save' }).click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); await expect(card.getByText('Token lifetime must use whole-minute increments.')).toBeVisible(); await card.getByLabel('Access token lifetime', { exact: true }).fill('60'); await card.getByLabel('Refresh token inactivity timeout', { exact: true }).fill('30'); - await card.getByRole('button', { name: 'Save' }).click(); - await expect(page.getByText('OIDC client updated successfully', { exact: true })).toBeVisible(); + await saveUnsavedChanges(page); +}); + +test('Save OIDC client details and token lifetimes together', async ({ page }) => { + const client = oidcClients.nextcloud; + await page.goto(`/settings/admin/oidc-clients/${client.id}`); + + const name = page.getByLabel('Name'); + const accessLifetime = page + .getByTestId('token-lifetimes-card') + .getByLabel('Access token lifetime', { exact: true }); + await name.fill('Nextcloud with custom lifetime'); + await accessLifetime.fill('2'); + await page.getByRole('button', { name: 'Discard', exact: true }).click(); + await expect(name).toHaveValue(client.name); + await expect(accessLifetime).toHaveValue('1'); + + await name.fill('Nextcloud with custom lifetime'); + await accessLifetime.fill('2'); + + await saveUnsavedChanges(page); + + await page.reload(); + await expect(name).toHaveValue('Nextcloud with custom lifetime'); + await expect(accessLifetime).toHaveValue('2'); }); test('Update OIDC client federated credentials', async ({ page }) => { @@ -178,7 +196,7 @@ test('Update OIDC client federated credentials', async ({ page }) => { response.request().method() === 'PUT' && response.url().endsWith(`/api/oidc/clients/${client.id}`) ); - await card.getByRole('button', { name: 'Save' }).click(); + await saveUnsavedChanges(page); expect((await cardUpdate).ok()).toBeTruthy(); await page.reload(); @@ -186,17 +204,23 @@ test('Update OIDC client federated credentials', async ({ page }) => { await expect(card.getByLabel('Subject')).toHaveValue('workload-client'); await expect(card.getByLabel('Audience')).toHaveValue('https://pocket-id.example.com'); + await card.getByRole('radio', { name: 'Public keys' }).click(); + await card.getByRole('button', { name: 'Add another federated client credential' }).click(); + await expect(card.getByLabel('Issuer')).toHaveCount(2); + await page.getByRole('button', { name: 'Discard', exact: true }).click(); + await expect(card.getByLabel('Issuer')).toHaveCount(1); + await expect(card.getByRole('radio', { name: 'JWKS URL' })).toBeChecked(); + // Saving the main client form must preserve credentials managed by the separate card await page.locator('[role="tab"][data-value="general"]').click(); const description = page.getByLabel('Description'); await description.fill('Updated without replacing federated credentials'); - const clientForm = description.locator('xpath=ancestor::form'); const formUpdate = page.waitForResponse( (response) => response.request().method() === 'PUT' && response.url().endsWith(`/api/oidc/clients/${client.id}`) ); - await clientForm.getByRole('button', { name: 'Save' }).click(); + await saveUnsavedChanges(page); expect((await formUpdate).ok()).toBeTruthy(); await page.goto(`/settings/admin/oidc-clients/${client.id}#credentials`); @@ -228,7 +252,7 @@ test('Update OIDC client federated credentials with public keys', async ({ page const pasteInput = card.getByLabel('Public key', { exact: true }); const addKeyButton = card.getByRole('button', { name: 'Add public key' }); const publicKeys = card.getByTestId('federated-identity-public-key'); - const saveButton = card.getByRole('button', { name: 'Save' }); + const saveButton = page.getByRole('button', { name: 'Save', exact: true }); const waitForClientUpdate = () => page.waitForResponse( (response) => @@ -254,9 +278,7 @@ test('Update OIDC client federated credentials with public keys', async ({ page const privateKeyUpdate = waitForClientUpdate(); await saveButton.click(); expect((await privateKeyUpdate).status()).toBe(400); - await expect( - page.locator('[data-type="error"]').filter({ hasText: 'private key material' }) - ).toBeVisible(); + await expect(page.getByText(/private key material/)).toBeVisible(); await publicKeys.last().getByRole('button').click(); await expect(publicKeys).toHaveCount(3); @@ -268,9 +290,7 @@ test('Update OIDC client federated credentials with public keys', async ({ page const missingKeyIdUpdate = waitForClientUpdate(); await saveButton.click(); expect((await missingKeyIdUpdate).status()).toBe(400); - await expect( - page.locator('[data-type="error"]').filter({ hasText: 'missing the "kid" property' }) - ).toBeVisible(); + await expect(page.getByText(/missing the "kid" property/)).toBeVisible(); await publicKeys.last().getByRole('button').click(); await expect(publicKeys).toHaveCount(3); @@ -278,7 +298,7 @@ test('Update OIDC client federated credentials with public keys', async ({ page await expect(publicKeys).toHaveCount(2); const cardUpdate = waitForClientUpdate(); - await saveButton.click(); + await saveUnsavedChanges(page); expect((await cardUpdate).ok()).toBeTruthy(); await page.reload(); @@ -466,9 +486,7 @@ test('Update OIDC client allowed user groups', async ({ page }) => { await page.getByRole('row', { name: userGroups.designers.name }).getByRole('checkbox').click(); await page.getByRole('row', { name: userGroups.developers.name }).getByRole('checkbox').click(); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.getByText('Allowed user groups updated successfully')).toBeVisible(); + await saveUnsavedChanges(page); await page.reload(); diff --git a/tests/specs/oidc.spec.ts b/tests/specs/oidc.spec.ts index 9d5abdd7..e1d13223 100644 --- a/tests/specs/oidc.spec.ts +++ b/tests/specs/oidc.spec.ts @@ -4,6 +4,7 @@ import { cleanupBackend } from '../utils/cleanup.util'; import { generateIdToken } from '../utils/jwt.util'; import * as oidcUtil from '../utils/oidc.util'; import passkeyUtil from '../utils/passkey.util'; +import { saveUnsavedChanges } from '../utils/unsaved-changes.util'; test.beforeEach(async () => await cleanupBackend()); @@ -1626,8 +1627,7 @@ test.describe('Pushed Authorization Requests (PAR)', () => { await parToggle.click(); } - await page.getByRole('button', { name: 'Save', exact: true }).first().click(); - await expect(page.getByText('OIDC client updated successfully', { exact: true })).toBeVisible(); + await saveUnsavedChanges(page); await page.reload(); await page.getByRole('button', { name: 'Show Advanced Options' }).click(); @@ -1676,9 +1676,7 @@ test.describe('OIDC skip consent', () => { // Disabling it and saving must persist across a reload await toggle.click(); await expect(toggle).not.toBeChecked(); - const clientForm = toggle.locator('xpath=ancestor::form'); - await clientForm.getByRole('button', { name: 'Save', exact: true }).click(); - await expect(page.getByText('OIDC client updated successfully', { exact: true })).toBeVisible(); + await saveUnsavedChanges(page); await page.reload(); await expect(page.getByRole('switch', { name: 'Skip Consent Screen' })).not.toBeChecked(); diff --git a/tests/specs/scim.spec.ts b/tests/specs/scim.spec.ts index e487f771..f4abf751 100644 --- a/tests/specs/scim.spec.ts +++ b/tests/specs/scim.spec.ts @@ -1,6 +1,7 @@ import test, { expect, type Page } from '@playwright/test'; import { cleanupBackend, cleanupScimServiceProvider } from 'utils/cleanup.util'; import { oidcClients, userGroups, users } from '../data'; +import { saveUnsavedChanges } from '../utils/unsaved-changes.util'; async function configureOidcClient(page: Page) { await page.goto(`/settings/admin/oidc-clients/${oidcClients.scim.id}`); @@ -34,11 +35,17 @@ test.describe('SCIM Configuration', () => { await page.getByLabel('SCIM Endpoint').fill('http://scim.provider/api'); await page.getByLabel('SCIM Token').fill('supersecrettoken'); + await expect(page.getByText('You have unsaved changes', { exact: true })).toHaveCount(0); await page.getByRole('button', { name: 'Enable' }).click(); await expect(page.locator('[data-type="success"]')).toHaveText('SCIM enabled successfully.'); + await page.getByLabel('SCIM Endpoint').fill('http://edited.scim.provider/api'); + await expect(page.getByText('You have unsaved changes', { exact: true })).toBeVisible(); + await page.getByRole('button', { name: 'Discard', exact: true }).click(); + await expect(page.getByLabel('SCIM Endpoint')).toHaveValue('http://scim.provider/api'); + await page.reload(); await expect(page.getByLabel('SCIM Endpoint')).toHaveValue('http://scim.provider/api'); @@ -54,11 +61,7 @@ test.describe('SCIM Configuration', () => { await page.getByLabel('SCIM Endpoint').fill('http://new.scim.provider/api'); await page.getByLabel('SCIM Token').fill('evenmoresecrettoken'); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'SCIM configuration updated successfully.' - ); + await saveUnsavedChanges(page); await page.reload(); @@ -134,10 +137,7 @@ test.describe('SCIM Sync', () => { await developersCheckbox.click(); await expect(developersCheckbox).toHaveAttribute('data-state', 'unchecked'); - await page.getByRole('button', { name: 'Save' }).click(); - await expect( - page.getByText('Allowed user groups updated successfully', { exact: true }) - ).toBeVisible(); + await saveUnsavedChanges(page); await syncScimServiceProvider(page); diff --git a/tests/specs/user-group.spec.ts b/tests/specs/user-group.spec.ts index a58b08e5..daeb2847 100644 --- a/tests/specs/user-group.spec.ts +++ b/tests/specs/user-group.spec.ts @@ -1,6 +1,7 @@ import test, { expect } from '@playwright/test'; import { oidcClients, userGroups, users } from '../data'; import { cleanupBackend } from '../utils/cleanup.util'; +import { saveUnsavedChanges } from '../utils/unsaved-changes.util'; test.beforeEach(async () => await cleanupBackend()); @@ -34,9 +35,7 @@ test('Edit user group', async ({ page }) => { await page.getByLabel('Name', { exact: true }).fill('developers_updated'); - await page.getByRole('button', { name: 'Save' }).nth(0).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText('User group updated successfully'); + await saveUnsavedChanges(page); await expect(page.getByLabel('Friendly Name')).toHaveValue('Developers updated'); await expect(page.getByLabel('Name', { exact: true })).toHaveValue('developers_updated'); }); @@ -49,9 +48,7 @@ test('Update user group users', async ({ page }) => { await page.getByRole('row', { name: users.tim.username }).getByRole('checkbox').click(); await page.getByRole('row', { name: users.craig.username }).getByRole('checkbox').click(); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText('Users updated successfully'); + await saveUnsavedChanges(page); await page.reload(); @@ -90,11 +87,7 @@ test('Update user group custom claims', async ({ page }) => { await page.getByPlaceholder('Key').nth(1).fill('customClaim2'); await page.getByPlaceholder('Value').nth(1).fill('customClaim2_value'); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Custom claims updated successfully' - ); + await saveUnsavedChanges(page); await page.reload(); await page.waitForLoadState('networkidle'); @@ -107,11 +100,7 @@ test('Update user group custom claims', async ({ page }) => { // Remove one custom claim await page.getByLabel('Remove custom claim').first().click(); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Custom claims updated successfully' - ); + await saveUnsavedChanges(page); await page.reload(); await page.waitForLoadState('networkidle'); @@ -136,11 +125,7 @@ test('Update user group allowed user groups', async ({ page }) => { await page.getByRole('row', { name: oidcClients.tailscale.name }).getByRole('checkbox').click(); await page.getByRole('row', { name: oidcClients.immich.name }).getByRole('checkbox').click(); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Allowed OIDC clients updated successfully' - ); + await saveUnsavedChanges(page); await page.reload(); diff --git a/tests/specs/user-settings.spec.ts b/tests/specs/user-settings.spec.ts index 38ee694b..dbaab3f2 100644 --- a/tests/specs/user-settings.spec.ts +++ b/tests/specs/user-settings.spec.ts @@ -2,6 +2,7 @@ import test, { expect } from '@playwright/test'; import { userGroups, users } from '../data'; import authUtil from '../utils/auth.util'; import { cleanupBackend } from '../utils/cleanup.util'; +import { saveUnsavedChanges } from '../utils/unsaved-changes.util'; test.beforeEach(async () => await cleanupBackend()); @@ -128,9 +129,7 @@ test('Update user', async ({ page }) => { await page.getByLabel('Display Name').fill('Crack Apple'); await page.getByLabel('Email').fill('crack.apple@test.com'); await page.getByLabel('Username').fill('crack'); - await page.getByRole('button', { name: 'Save' }).first().click(); - - await expect(page.locator('[data-type="success"]')).toHaveText('User updated successfully'); + await saveUnsavedChanges(page); }); test('Update user fails with already taken email', async ({ page }) => { @@ -145,9 +144,9 @@ test('Update user fails with already taken email', async ({ page }) => { await page.getByRole('menuitem', { name: 'Edit' }).click(); await page.getByLabel('Email').fill(users.tim.email); - await page.getByRole('button', { name: 'Save' }).first().click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); - await expect(page.locator('[data-type="error"]')).toHaveText('Email is already in use'); + await expect(page.getByText('Email is already in use', { exact: true })).toBeVisible(); }); test('Update user fails with already taken username', async ({ page }) => { @@ -162,9 +161,9 @@ test('Update user fails with already taken username', async ({ page }) => { await page.getByRole('menuitem', { name: 'Edit' }).click(); await page.getByLabel('Username').fill(users.tim.username); - await page.getByRole('button', { name: 'Save' }).first().click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); - await expect(page.locator('[data-type="error"]')).toHaveText('Username is already in use'); + await expect(page.getByText('Username is already in use', { exact: true })).toBeVisible(); }); test('Update user fails with already taken username in different casing', async ({ page }) => { @@ -179,9 +178,9 @@ test('Update user fails with already taken username in different casing', async await page.getByRole('menuitem', { name: 'Edit' }).click(); await page.getByLabel('Username').fill(users.tim.username.toUpperCase()); - await page.getByRole('button', { name: 'Save' }).first().click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); - await expect(page.locator('[data-type="error"]')).toHaveText('Username is already in use'); + await expect(page.getByText('Username is already in use', { exact: true })).toBeVisible(); }); test('Update user custom claims', async ({ page }) => { @@ -199,11 +198,7 @@ test('Update user custom claims', async ({ page }) => { await page.getByPlaceholder('Key').nth(1).fill('customClaim2'); await page.getByPlaceholder('Value').nth(1).fill('customClaim2_value'); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Custom claims updated successfully' - ); + await saveUnsavedChanges(page); await page.reload(); @@ -215,11 +210,7 @@ test('Update user custom claims', async ({ page }) => { // Remove one custom claim await page.getByLabel('Remove custom claim').first().click(); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'Custom claims updated successfully' - ); + await saveUnsavedChanges(page); await page.reload(); @@ -237,11 +228,7 @@ test('Update user group assignments', async ({ page }) => { await page.getByRole('row', { name: userGroups.developers.name }).getByRole('checkbox').click(); await page.getByRole('row', { name: userGroups.designers.name }).getByRole('checkbox').click(); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]')).toHaveText( - 'User groups updated successfully' - ); + await saveUnsavedChanges(page); await page.reload(); diff --git a/tests/specs/user-signup.spec.ts b/tests/specs/user-signup.spec.ts index db84d62b..1442b94d 100644 --- a/tests/specs/user-signup.spec.ts +++ b/tests/specs/user-signup.spec.ts @@ -2,6 +2,7 @@ import test, { expect, type Page } from '@playwright/test'; import { signupTokens, userGroups, users } from '../data'; import { cleanupBackend } from '../utils/cleanup.util'; import passkeyUtil from '../utils/passkey.util'; +import { saveUnsavedChanges } from '../utils/unsaved-changes.util'; async function setSignupMode( page: Page, @@ -11,13 +12,12 @@ async function setSignupMode( await page.goto('/settings/admin/application-configuration'); await page.getByRole('tab', { name: 'User Creation' }).click(); - await page.getByRole('button', { name: 'Enable User Signups' }).click(); - await page.getByRole('option', { name: mode }).click(); - await page.getByRole('button', { name: 'Save' }).click(); - - await expect(page.locator('[data-type="success"]').last()).toHaveText( - 'User creation settings updated successfully.' - ); + const signupMode = page.getByRole('button', { name: 'Enable User Signups' }); + if (!(await signupMode.textContent())?.includes(mode)) { + await signupMode.click(); + await page.getByRole('option', { name: mode }).click(); + await saveUnsavedChanges(page); + } if (signout) { await page.context().clearCookies(); diff --git a/tests/utils/unsaved-changes.util.ts b/tests/utils/unsaved-changes.util.ts new file mode 100644 index 00000000..1d5eff4f --- /dev/null +++ b/tests/utils/unsaved-changes.util.ts @@ -0,0 +1,6 @@ +import { expect, type Page } from '@playwright/test'; + +export async function saveUnsavedChanges(page: Page) { + await page.getByRole('button', { name: 'Save', exact: true }).click(); + await expect(page.getByText('Changes saved successfully', { exact: true })).toBeVisible(); +}