diff --git a/frontend/src/__tests__/settings-service-payment.test.ts b/frontend/src/__tests__/settings-service-payment.test.ts index 1f2b8fe37..0b6d22df4 100644 --- a/frontend/src/__tests__/settings-service-payment.test.ts +++ b/frontend/src/__tests__/settings-service-payment.test.ts @@ -170,6 +170,63 @@ describe('Settings Save payment tokens (issue #545)', () => { expect(awsPayment.classList.contains('dirty')).toBe(false); consoleError.mockRestore(); }); + // Issue #617: after #614 a scoped admin's service PUTs all 403. With no + // global change and no service saved, the toast must not claim "Settings + // saved" or repeat the same denial 17 times. + function denied(): Error { + return Object.assign(new Error('permission denied: global configuration requires unrestricted account access'), { status: 403 }); + } + + it('scoped admin: every service PUT denied shows one permission-denied failure toast', async () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + (api.updateServiceConfig as jest.Mock).mockRejectedValue(denied()); + await loadGlobalSettings(); + const awsPayment = document.getElementById('aws-ec2-payment') as HTMLSelectElement; + awsPayment.value = 'no-upfront'; + + await saveGlobalSettings(new Event('submit')); + + expect(api.updateConfig).not.toHaveBeenCalled(); + expect(mockShowToast).toHaveBeenCalledTimes(1); + expect(mockShowToast).toHaveBeenCalledWith({ + message: 'Failed to save settings: permission denied: global configuration requires unrestricted account access', + kind: 'error', + }); + // Nothing saved, so the edited card stays dirty for a retry. + expect(awsPayment.classList.contains('dirty')).toBe(true); + consoleError.mockRestore(); + }); + + it('every service PUT failing for other reasons says the save failed, listing each service', async () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + (api.updateServiceConfig as jest.Mock).mockImplementation(async (provider: string, service: string) => { + throw provider === 'aws' && service === 'ec2' ? new Error('boom') : denied(); + }); + await loadGlobalSettings(); + + await saveGlobalSettings(new Event('submit')); + + const { message } = mockShowToast.mock.calls[0]![0] as { message: string }; + expect(message).toMatch(/^Failed to save settings: 17 services failed: /); + expect(message).toContain('aws/ec2: boom'); + consoleError.mockRestore(); + }); + + it('a saved global change still reports "Settings saved, but" when every service PUT fails', async () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + (api.updateServiceConfig as jest.Mock).mockRejectedValue(new Error('unavailable')); + await loadGlobalSettings(); + const notify = document.getElementById('setting-notification-days') as HTMLInputElement; + notify.value = '9'; + + await saveGlobalSettings(new Event('submit')); + + expect(api.updateConfig).toHaveBeenCalledTimes(1); + const { message } = mockShowToast.mock.calls[0]![0] as { message: string }; + expect(message).toMatch(/^Settings saved, but 17 services failed: /); + consoleError.mockRestore(); + }); + it.each([ ['no-upfront', 'monthly'], ['partial-upfront', 'monthly'], diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index efbed45af..7725a70d9 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -3558,7 +3558,8 @@ export async function saveGlobalSettings(e: Event): Promise { for (const [key, ids] of GLOBAL_CONFIG_FIELDS) { if (ids.every(id => getFieldValue(id) === savedSnapshot[id])) delete changedGlobal[key]; } - if (Object.keys(changedGlobal).length > 0) { + const globalSent = Object.keys(changedGlobal).length > 0; + if (globalSent) { await api.updateConfig(changedGlobal); // Persisted: advance the snapshot now, so a later validation failure or // service-save error does not leave these fields looking unsaved. @@ -3678,10 +3679,7 @@ export async function saveGlobalSettings(e: Event): Promise { if (failures.length > 0) { console.error('Failed to save service configs:', failures); - showToast({ - message: `Settings saved, but ${failures.length} service${failures.length === 1 ? '' : 's'} failed: ${failures.join('; ')}`, - kind: 'error', - }); + showToast({ message: serviceSaveFailureMessage(results, failures, globalSent), kind: 'error' }); } else { showToast({ message: 'Settings saved successfully', kind: 'success', timeout: 5_000 }); } @@ -3695,6 +3693,31 @@ export async function saveGlobalSettings(e: Event): Promise { } } +/** + * Toast text for a Save where at least one service PUT failed. Only claim + * "Settings saved" when something was: the global PUT went out or a service + * PUT succeeded. When nothing saved and every service PUT was denied (a + * scoped admin after #614 gated service config on unrestricted account + * scope), show the single backend reason, matching the global PUT's + * "Failed to save settings: permission denied: ..." toast instead of + * repeating it once per service (#617). + */ +function serviceSaveFailureMessage( + results: PromiseSettledResult[], + failures: string[], + globalSent: boolean, +): string { + const savedSomething = globalSent || results.some(r => r.status === 'fulfilled'); + if (savedSomething) { + return `Settings saved, but ${failures.length} service${failures.length === 1 ? '' : 's'} failed: ${failures.join('; ')}`; + } + const reasons = results.map(r => (r.status === 'rejected' ? r.reason : null)); + if (reasons.every(isPermissionDeniedError)) { + return `Failed to save settings: ${(reasons[0] as Error).message}`; + } + return `Failed to save settings: ${failures.length} service${failures.length === 1 ? '' : 's'} failed: ${failures.join('; ')}`; +} + /** * Reset settings to defaults */