Skip to content

feat(settings): per-product Coverage and Enabled controls on SP cards (closes #136) #580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 2 commits into from
May 22, 2026
Merged

feat(settings): per-product Coverage and Enabled controls on SP cards (closes #136) #580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
851bd5c
feat(settings): per-product Coverage and Enabled controls on SP cards…
cristim May 20, 2026
822a4aa
fix(settings): apply SP coverage/enabled fallback when service row ab…
cristim May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions frontend/src/__tests__/settings.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ describe('Settings Module', () => {
<select id="aws-savings-plans-ec2instance-term"><option value="1">1</option><option value="3">3</option></select>
<select id="aws-savings-plans-sagemaker-term"><option value="1">1</option><option value="3">3</option></select>
<select id="aws-savings-plans-database-term"><option value="1">1</option><option value="3">3</option></select>
<!-- Issue #136: per-product SP coverage and enabled controls -->
<input type="number" id="aws-savings-plans-compute-coverage" min="0" max="100" value="80">
<input type="checkbox" id="aws-savings-plans-compute-enabled" checked>
<input type="number" id="aws-savings-plans-ec2instance-coverage" min="0" max="100" value="80">
<input type="checkbox" id="aws-savings-plans-ec2instance-enabled" checked>
<input type="number" id="aws-savings-plans-sagemaker-coverage" min="0" max="100" value="80">
<input type="checkbox" id="aws-savings-plans-sagemaker-enabled" checked>
<input type="number" id="aws-savings-plans-database-coverage" min="0" max="100" value="80">
<input type="checkbox" id="aws-savings-plans-database-enabled" checked>
<select id="aws-ec2-payment"><option value="no-upfront">No</option><option value="partial-upfront">Partial</option><option value="all-upfront">All</option></select>
<select id="aws-rds-payment"><option value="no-upfront">No</option><option value="partial-upfront">Partial</option><option value="all-upfront">All</option></select>
<select id="aws-elasticache-payment"><option value="no-upfront">No</option><option value="partial-upfront">Partial</option><option value="all-upfront">All</option></select>
Expand Down Expand Up @@ -590,6 +599,9 @@ describe('Settings Module', () => {
for (const planType of ['compute', 'ec2instance', 'sagemaker', 'database']) {
expect(html).toMatch(new RegExp(`id="aws-savings-plans-${planType}-term"`));
expect(html).toMatch(new RegExp(`id="aws-savings-plans-${planType}-payment"`));
// Issue #136: coverage and enabled controls must be present on each card.
expect(html).toMatch(new RegExp(`id="aws-savings-plans-${planType}-coverage"`));
expect(html).toMatch(new RegExp(`id="aws-savings-plans-${planType}-enabled"`));
}
});

Expand Down Expand Up @@ -626,6 +638,86 @@ describe('Settings Module', () => {
expect(cfg.payment).toBe('no-upfront');
});

test('saveGlobalSettings sends per-card coverage from SP coverage input (issue #136)', async () => {
(api.updateConfig as jest.Mock).mockResolvedValue({});
(api.updateServiceConfig as jest.Mock).mockClear().mockResolvedValue(undefined);
window.alert = jest.fn();

// Pin compute SP to 60% coverage, leave others at default 80%.
(document.getElementById('aws-savings-plans-compute-coverage') as HTMLInputElement).value = '60';

const event = { preventDefault: jest.fn() } as unknown as Event;
await saveGlobalSettings(event);

const computeCall = (api.updateServiceConfig as jest.Mock).mock.calls.find(
([provider, service]) => provider === 'aws' && service === 'savings-plans-compute',
);
expect(computeCall).toBeDefined();
expect(computeCall![2].coverage).toBe(60);

// Other SP cards keep their default value.
const ec2Call = (api.updateServiceConfig as jest.Mock).mock.calls.find(
([provider, service]) => provider === 'aws' && service === 'savings-plans-ec2instance',
);
expect(ec2Call).toBeDefined();
expect(ec2Call![2].coverage).toBe(80);
});

test('saveGlobalSettings sends per-card enabled=false when SP enabled checkbox unchecked (issue #136)', async () => {
(api.updateConfig as jest.Mock).mockResolvedValue({});
(api.updateServiceConfig as jest.Mock).mockClear().mockResolvedValue(undefined);
window.alert = jest.fn();

// Disable the database SP card.
(document.getElementById('aws-savings-plans-database-enabled') as HTMLInputElement).checked = false;

const event = { preventDefault: jest.fn() } as unknown as Event;
await saveGlobalSettings(event);

const dbCall = (api.updateServiceConfig as jest.Mock).mock.calls.find(
([provider, service]) => provider === 'aws' && service === 'savings-plans-database',
);
expect(dbCall).toBeDefined();
expect(dbCall![2].enabled).toBe(false);

// Other SP cards remain enabled.
const computeCall = (api.updateServiceConfig as jest.Mock).mock.calls.find(
([provider, service]) => provider === 'aws' && service === 'savings-plans-compute',
);
expect(computeCall).toBeDefined();
expect(computeCall![2].enabled).toBe(true);
});

test('loadGlobalSettings populates SP coverage and enabled from service config (issue #136)', async () => {
// Seed a non-default initial DOM state on the fallback card so the test
// fails if the fallback assignment is skipped (it would otherwise pass on
// the HTML defaults, which happen to equal the asserted values).
(document.getElementById('aws-savings-plans-ec2instance-coverage') as HTMLInputElement).value = '12';
(document.getElementById('aws-savings-plans-ec2instance-enabled') as HTMLInputElement).checked = false;

(api.getConfig as jest.Mock).mockResolvedValue({
// Non-80 global default so the fallback writes an observably different value.
global: { enabled_providers: ['aws'], default_term: 3, default_payment: 'all-upfront', default_coverage: 67 },
services: [
{ provider: 'aws', service: 'savings-plans-compute', term: 1, payment: 'no-upfront', coverage: 65, enabled: false },
],
});
setupSettingsHandlers();
await loadGlobalSettings();

const coverageEl = document.getElementById('aws-savings-plans-compute-coverage') as HTMLInputElement;
const enabledEl = document.getElementById('aws-savings-plans-compute-enabled') as HTMLInputElement;
expect(coverageEl.value).toBe('65');
expect(enabledEl.checked).toBe(false);

// Cards without an explicit service row fall back to the global default
// (67, not the HTML default 80) and re-enable from the seeded false state.
const ec2Coverage = document.getElementById('aws-savings-plans-ec2instance-coverage') as HTMLInputElement;
const ec2Enabled = document.getElementById('aws-savings-plans-ec2instance-enabled') as HTMLInputElement;
expect(ec2Coverage.value).toBe('67');
expect(ec2Enabled.checked).toBe(true);
});

test('calls updateServiceConfig once per service field (18 calls)', async () => {
(api.updateConfig as jest.Mock).mockResolvedValue({});
(api.updateServiceConfig as jest.Mock).mockResolvedValue(undefined);
Expand Down
8 changes: 8 additions & 0 deletions frontend/src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,24 +506,32 @@ <h5>Compute Savings Plans</h5>
<p class="service-default-hint">EC2, Fargate, Lambda — most flexible</p>
<label>Term: <select id="aws-savings-plans-compute-term"><option value="1">1 Year</option><option value="3" selected>3 Years</option></select></label>
<label>Payment: <select id="aws-savings-plans-compute-payment"><option value="no-upfront">No Upfront</option><option value="partial-upfront">Partial</option><option value="all-upfront" selected>All Upfront</option></select></label>
<label>Coverage %: <input type="number" id="aws-savings-plans-compute-coverage" min="0" max="100" value="80"></label>
<label class="toggle-label"><input type="checkbox" id="aws-savings-plans-compute-enabled" checked> Enabled</label>
</div>
<div class="service-default-card">
<h5>EC2 Instance Savings Plans</h5>
<p class="service-default-hint">EC2 only, region-locked — deepest discount</p>
<label>Term: <select id="aws-savings-plans-ec2instance-term"><option value="1">1 Year</option><option value="3" selected>3 Years</option></select></label>
<label>Payment: <select id="aws-savings-plans-ec2instance-payment"><option value="no-upfront">No Upfront</option><option value="partial-upfront">Partial</option><option value="all-upfront" selected>All Upfront</option></select></label>
<label>Coverage %: <input type="number" id="aws-savings-plans-ec2instance-coverage" min="0" max="100" value="80"></label>
<label class="toggle-label"><input type="checkbox" id="aws-savings-plans-ec2instance-enabled" checked> Enabled</label>
</div>
<div class="service-default-card">
<h5>SageMaker Savings Plans</h5>
<p class="service-default-hint">SageMaker training/inference</p>
<label>Term: <select id="aws-savings-plans-sagemaker-term"><option value="1">1 Year</option><option value="3" selected>3 Years</option></select></label>
<label>Payment: <select id="aws-savings-plans-sagemaker-payment"><option value="no-upfront">No Upfront</option><option value="partial-upfront">Partial</option><option value="all-upfront" selected>All Upfront</option></select></label>
<label>Coverage %: <input type="number" id="aws-savings-plans-sagemaker-coverage" min="0" max="100" value="80"></label>
<label class="toggle-label"><input type="checkbox" id="aws-savings-plans-sagemaker-enabled" checked> Enabled</label>
</div>
<div class="service-default-card">
<h5>Database Savings Plans</h5>
<p class="service-default-hint">Reserved for future AWS Database Savings Plans (currently not GA; defaults stored for forward-compatibility)</p>
<label>Term: <select id="aws-savings-plans-database-term"><option value="1">1 Year</option><option value="3" selected>3 Years</option></select></label>
<label>Payment: <select id="aws-savings-plans-database-payment"><option value="no-upfront">No Upfront</option><option value="partial-upfront">Partial</option><option value="all-upfront" selected>All Upfront</option></select></label>
<label>Coverage %: <input type="number" id="aws-savings-plans-database-coverage" min="0" max="100" value="80"></label>
<label class="toggle-label"><input type="checkbox" id="aws-savings-plans-database-enabled" checked> Enabled</label>
</div>
</div>
</fieldset>
Expand Down
72 changes: 57 additions & 15 deletions frontend/src/settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,12 @@ const SERVICE_FIELDS = [
// (when present) overrides the SageMaker slot from PR #71's sagemaker
// row. Lambda is intentionally NOT a separate card — Lambda has no
// standalone SP product; its commitments roll up into Compute SP.
{ provider: 'aws', service: 'savings-plans-compute', termId: 'aws-savings-plans-compute-term', paymentId: 'aws-savings-plans-compute-payment' },
{ provider: 'aws', service: 'savings-plans-ec2instance', termId: 'aws-savings-plans-ec2instance-term', paymentId: 'aws-savings-plans-ec2instance-payment' },
{ provider: 'aws', service: 'savings-plans-sagemaker', termId: 'aws-savings-plans-sagemaker-term', paymentId: 'aws-savings-plans-sagemaker-payment' },
{ provider: 'aws', service: 'savings-plans-database', termId: 'aws-savings-plans-database-term', paymentId: 'aws-savings-plans-database-payment' },
// Issue #136: SP cards carry per-card coverageId and enabledId so users
// can set divergent coverage and toggle each plan type independently.
{ provider: 'aws', service: 'savings-plans-compute', termId: 'aws-savings-plans-compute-term', paymentId: 'aws-savings-plans-compute-payment', coverageId: 'aws-savings-plans-compute-coverage', enabledId: 'aws-savings-plans-compute-enabled' },
{ provider: 'aws', service: 'savings-plans-ec2instance', termId: 'aws-savings-plans-ec2instance-term', paymentId: 'aws-savings-plans-ec2instance-payment', coverageId: 'aws-savings-plans-ec2instance-coverage', enabledId: 'aws-savings-plans-ec2instance-enabled' },
{ provider: 'aws', service: 'savings-plans-sagemaker', termId: 'aws-savings-plans-sagemaker-term', paymentId: 'aws-savings-plans-sagemaker-payment', coverageId: 'aws-savings-plans-sagemaker-coverage', enabledId: 'aws-savings-plans-sagemaker-enabled' },
{ provider: 'aws', service: 'savings-plans-database', termId: 'aws-savings-plans-database-term', paymentId: 'aws-savings-plans-database-payment', coverageId: 'aws-savings-plans-database-coverage', enabledId: 'aws-savings-plans-database-enabled' },
{ provider: 'azure', service: 'vm', termId: 'azure-vm-term', paymentId: 'azure-vm-payment' },
{ provider: 'azure', service: 'sql', termId: 'azure-sql-term', paymentId: 'azure-sql-payment' },
{ provider: 'azure', service: 'cosmosdb', termId: 'azure-cosmosdb-term', paymentId: 'azure-cosmosdb-payment' },
Expand All @@ -120,6 +122,9 @@ const TRACKED_FIELDS = [
// Per-service fields
...SERVICE_FIELDS.map(f => f.termId),
...SERVICE_FIELDS.filter(f => f.paymentId !== null).map(f => f.paymentId as string),
// Per-product SP fields (issue #136)
...SERVICE_FIELDS.filter(f => 'coverageId' in f).map(f => (f as { coverageId: string }).coverageId),
...SERVICE_FIELDS.filter(f => 'enabledId' in f).map(f => (f as { enabledId: string }).enabledId),
];

// Snapshot of field values at last save (or initial load).
Expand Down Expand Up @@ -2666,14 +2671,33 @@ export async function loadGlobalSettings(): Promise<void> {
updateCollectionScheduleVisibility();
}

if (data.services) {
loadedServiceConfigs = data.services;
for (const svc of data.services) {
const key = `${svc.provider}-${svc.service}`;
const termEl = document.getElementById(`${key}-term`) as HTMLSelectElement | null;
if (termEl) termEl.value = String(svc.term);
const paymentEl = document.getElementById(`${key}-payment`) as HTMLSelectElement | null;
if (paymentEl) paymentEl.value = svc.payment;
const services = data.services ?? [];
loadedServiceConfigs = services;
for (const svc of services) {
const key = `${svc.provider}-${svc.service}`;
const termEl = document.getElementById(`${key}-term`) as HTMLSelectElement | null;
if (termEl) termEl.value = String(svc.term);
const paymentEl = document.getElementById(`${key}-payment`) as HTMLSelectElement | null;
if (paymentEl) paymentEl.value = svc.payment;
}

// Issue #136: populate per-product SP coverage and enabled fields for every
// SP card the DOM exposes, not just those with a service row. A card whose
// service row is absent from the response must still fall back to
// global.default_coverage (and enabled=true); leaving it at the HTML default
// (80) would persist an incorrect value on the user's next save. Iterate
// SERVICE_FIELDS (the source of truth for card IDs) and overlay any matching
// service-config values keyed by `${provider}-${service}`.
const svcByKey = new Map(services.map(s => [`${s.provider}-${s.service}`, s] as const));
for (const field of SERVICE_FIELDS) {
const svc = svcByKey.get(`${field.provider}-${field.service}`);
if ('coverageId' in field && field.coverageId) {
const el = byId<HTMLInputElement>(field.coverageId);
if (el) el.value = String(svc?.coverage ?? data.global?.default_coverage ?? 80);
}
if ('enabledId' in field && field.enabledId) {
const el = byId<HTMLInputElement>(field.enabledId);
if (el) el.checked = svc?.enabled !== false;
}
}

Expand Down Expand Up @@ -2903,7 +2927,8 @@ export async function saveGlobalSettings(e: Event): Promise<void> {
try {
await api.updateConfig(settings);

const serviceSaves = SERVICE_FIELDS.map(({ provider, service, termId, paymentId }) => {
const serviceSaves = SERVICE_FIELDS.map((field) => {
const { provider, service, termId, paymentId } = field;
const term = parseInt(byId<HTMLSelectElement>(termId)?.value || '3', 10);
const payment = paymentId
? (byId<HTMLSelectElement>(paymentId)?.value || 'all-upfront')
Expand All @@ -2913,14 +2938,31 @@ export async function saveGlobalSettings(e: Event): Promise<void> {
// include_engines, etc. can be set out-of-band (API, future UI, migration)
// and a full UPSERT that only honoured the four term/payment/enabled/coverage
// fields would silently wipe them every time the user clicked Save.

// Issue #136: per-product SP cards expose their own coverage and enabled
// controls. Read from the DOM when the card has the controls; fall back
// to the base row value (or the global default) otherwise so RI and
// Azure/GCP cards continue to inherit the global settings.
let coverage = base?.coverage ?? settings.default_coverage;
let enabled = base?.enabled ?? true;
if ('coverageId' in field && field.coverageId) {
const rawCov = byId<HTMLInputElement>(field.coverageId)?.value ?? '';
const parsed = Number(rawCov);
if (rawCov !== '' && Number.isFinite(parsed)) coverage = parsed;
}
if ('enabledId' in field && field.enabledId) {
const el = byId<HTMLInputElement>(field.enabledId);
if (el) enabled = el.checked;
}

const cfg: api.ServiceConfig = {
...(base ?? {}),
provider,
service,
enabled: base?.enabled ?? true,
enabled,
term,
payment,
coverage: base?.coverage ?? settings.default_coverage,
coverage,
};
return api.updateServiceConfig(provider, service, cfg);
});
Expand Down
Loading