diff --git a/frontend/src/__tests__/settings-service-payment.test.ts b/frontend/src/__tests__/settings-service-payment.test.ts index ca8361e45..55d6afba6 100644 --- a/frontend/src/__tests__/settings-service-payment.test.ts +++ b/frontend/src/__tests__/settings-service-payment.test.ts @@ -276,6 +276,31 @@ describe('Settings Save payment tokens (issue #545)', () => { ]); }); + it.each([ + ['all-upfront', { provider: 'gcp', service: 'compute', term: 1, payment: 'monthly' }], + ['monthly', { provider: 'gcp', service: 'compute', term: 1 }], + ])('a GCP row holding payment %p: a term edit PUTs %j', async (stored, body) => { + (api.getConfig as jest.Mock).mockResolvedValue({ + global: { enabled_providers: ['gcp'], default_term: 3, default_payment: 'all-upfront', default_coverage: 80 }, + services: [{ provider: 'gcp', service: 'compute', term: 3, payment: stored, enabled: true, coverage: 80 }], + }); + await loadGlobalSettings(); + (document.getElementById('gcp-compute-term') as HTMLSelectElement).value = '1'; + await saveGlobalSettings(new Event('submit')); + + expect((api.updateServiceConfig as jest.Mock).mock.calls).toEqual([['gcp', 'compute', body]]); + }); + + it('an emptied Savings Plan coverage card shows an inline error', async () => { + await loadGlobalSettings(); + setupSettingsHandlers(); + const card = document.getElementById('aws-savings-plans-compute-coverage') as HTMLInputElement; + card.value = ''; + card.dispatchEvent(new Event('blur')); + + expect(card.getAttribute('aria-invalid')).toBe('true'); + }); + it('never falls back to upfront when a payment select has no matching option at Save', async () => { await loadGlobalSettings(); (document.getElementById('azure-vm-payment') as HTMLSelectElement).value = 'junk'; diff --git a/frontend/src/__tests__/settings.test.ts b/frontend/src/__tests__/settings.test.ts index b44cac070..36710802b 100644 --- a/frontend/src/__tests__/settings.test.ts +++ b/frontend/src/__tests__/settings.test.ts @@ -931,6 +931,41 @@ describe('Settings Module', () => { expect(api.updateServiceConfig).toHaveBeenCalledWith('aws', 'savings-plans-compute', expect.objectContaining({ coverage: 65 })); }); + test('an unedited SP card is never flagged, whatever its stored value', async () => { + await loadStaleTab(); + (api.getConfig as jest.Mock).mockResolvedValue({ + global: { enabled_providers: ['aws'], default_term: 1, default_payment: 'no-upfront', default_coverage: 50, notification_days_before: 3 }, + services: [{ provider: 'aws', service: 'savings-plans-compute', enabled: true, term: 1, payment: 'no-upfront', coverage: 150 }], + }); + await loadGlobalSettings(); + (document.getElementById('setting-notification-email') as HTMLInputElement).value = 'new@test.com'; + mockShowToast.mockClear(); + + await save(); + + expect(api.updateConfig).toHaveBeenCalledWith({ notification_email: 'new@test.com' }); + expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'success' })); + }); + + test('an emptied cache stale threshold is refused, an explicit 0 is saved', async () => { + await loadStaleTab(); + const hours = document.getElementById('setting-recs-stale-hours') as HTMLInputElement; + hours.value = ''; + mockShowToast.mockClear(); + + await save(); + + expect(api.updateConfig).not.toHaveBeenCalled(); + expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ + message: expect.stringContaining('Cache stale threshold must be a whole number'), + kind: 'error', + })); + + hours.value = '0'; + await save(); + expect(api.updateConfig).toHaveBeenCalledWith({ recommendations_cache_stale_hours: 0 }); + }); + test('a second save after a service PUT succeeded resends nothing', async () => { await loadStaleTab(); (document.getElementById('aws-savings-plans-compute-coverage') as HTMLInputElement).value = '65'; diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index 68f928607..0796fdb30 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -2621,7 +2621,7 @@ export function setupSettingsHandlers(signal?: AbortSignal): void { // save-time-only toast contract) — so the inline-error UX is uniform // across Settings. wireInlineRangeValidation('setting-notification-days', { signal, requireInteger: true }); - wireInlineRangeValidation('setting-recs-stale-hours', { signal, requireInteger: true }); + wireInlineRangeValidation('setting-recs-stale-hours', { signal, requireInteger: true, required: true }); wireInlineRangeValidation('setting-default-coverage', { signal, requireInteger: true, required: true }); wireInlineRangeValidation('setting-grace-aws', { signal, requireInteger: true }); wireInlineRangeValidation('setting-grace-azure', { signal, requireInteger: true }); @@ -2634,7 +2634,7 @@ export function setupSettingsHandlers(signal?: AbortSignal): void { .forEach(f => wireInlineRangeValidation( (f as { coverageId: string }).coverageId, - { signal, requireInteger: true }, + { signal, requireInteger: true, required: true }, ) ); @@ -3420,10 +3420,12 @@ function changedServiceFields( if (filters.min_count !== (base?.min_count ?? 0)) changes.min_count = filters.min_count; } if (Object.keys(changes).length === 0) return null; - if (!changes.payment && field.paymentId && base) { + if (!changes.payment && base) { // A stored payment the card normalises on load (#545) goes out with any // PUT of the row, or the backend validates the legacy token and rejects it. - const shown = byId(field.paymentId)?.value; + // GCP cards have no payment control and GCP only accepts monthly, so a row + // holding another token (an AWS default copied onto it) is repaired too. + const shown = field.paymentId ? byId(field.paymentId)?.value : field.provider === 'gcp' ? 'monthly' : undefined; if (shown && shown !== base.payment) changes.payment = shown; } return changes; @@ -3541,8 +3543,11 @@ export async function saveGlobalSettings(e: Event): Promise { saveInFlight = false; return; } - const rawStaleHours = Number(byId('setting-recs-stale-hours')?.value ?? '24'); - if (!Number.isFinite(rawStaleHours) || !Number.isInteger(rawStaleHours) || rawStaleHours < 0 || rawStaleHours > 8760) { + const staleHoursText = byId('setting-recs-stale-hours')?.value ?? '24'; + const rawStaleHours = Number(staleHoursText); + // 0 is a legal value (disables auto-refresh) but Number('') is also 0, so an + // emptied field must be rejected explicitly rather than saved as "disable". + if (staleHoursText.trim() === '' || !Number.isFinite(rawStaleHours) || !Number.isInteger(rawStaleHours) || rawStaleHours < 0 || rawStaleHours > 8760) { showToast({ message: 'Cache stale threshold must be a whole number between 0 and 8760 hours (0 = disable)', kind: 'error' }); if (saveBtn) saveBtn.disabled = false; saveInFlight = false; diff --git a/internal/config/store_postgres_payment_propagation_integration_test.go b/internal/config/store_postgres_payment_propagation_integration_test.go new file mode 100644 index 000000000..9689c87e3 --- /dev/null +++ b/internal/config/store_postgres_payment_propagation_integration_test.go @@ -0,0 +1,36 @@ +//go:build integration +// +build integration + +package config + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +// A default payment valid globally but not for AWS ("monthly") must not be +// copied onto AWS rows (#539). +func TestIntegration_PropagateDefaultPayment_SkipsValueAWSRejects(t *testing.T) { + ctx := context.Background() + store := setupRampStepStore(ctx, t) + + global, err := store.GetGlobalConfig(ctx) + require.NoError(t, err) + global.DefaultPayment = "no-upfront" + require.NoError(t, store.SaveGlobalConfig(ctx, global)) + require.NoError(t, store.SaveServiceConfig(ctx, &ServiceConfig{ + Provider: "aws", Service: "ec2", Enabled: true, Term: 1, Payment: "no-upfront", Coverage: 80, RampSchedule: RampImmediate, + })) + + _, err = store.UpdateGlobalConfigAtomic(ctx, func(c *GlobalConfig) error { + c.DefaultPayment = "monthly" + return nil + }) + require.NoError(t, err) + + svc, err := store.GetServiceConfig(ctx, "aws", "ec2") + require.NoError(t, err) + require.Equal(t, "no-upfront", svc.Payment) +}