Skip to content

Commit e9ab8ba

Browse files
cristimcristim
authored
ux(settings): add SageMaker and Lambda purchasing-defaults cards (#71)
* ux(settings): add SageMaker and Lambda purchasing-defaults cards Settings → Purchasing previously offered per-service term/payment cards for EC2, RDS, ElastiCache, OpenSearch, Redshift, and the umbrella Savings Plans, but had no cards for SageMaker or Lambda. Both services are significant cost-optimisation targets covered by Savings Plans (SageMaker SP and Compute SP respectively), so the omission forced users to fall back to the global default term/payment for any SageMaker/Lambda recommendation instead of pinning a per-workload preference. Add `aws/sagemaker` and `aws/lambda` cards mirroring the existing AWS card shape (1y/3y term, NoUpfront / Partial / AllUpfront payment). Both services use AWS_PAYMENTS via commitmentOptions.ts's `_default` fallback — no per-service constraints. The new entries plug into the existing SERVICE_FIELDS array so save / load / dirty-tracking / sticky save bar wiring picks them up automatically. The umbrella Savings Plans hint copy is tweaked to clarify the new per-service cards override it. Tests cover render presence (grep index.html for heading + select IDs) and save round-trip (per-service config endpoint receives the edited term and payment) for each service, plus constraint-suite regression tests confirming the 3yr / no-upfront combination stays selectable (SageMaker / Lambda have no service-level restriction). Closes #22 * fix(settings): drop invalid Lambda card — no standalone Lambda SP Lambda has no dedicated Savings Plan product — Lambda usage is covered exclusively by Compute Savings Plans (alongside EC2 + Fargate), already represented by the umbrella "Savings Plans" card. A separate "Lambda Savings Plans" card invents a non-existent SKU: picking term/payment on it has no purchasable instrument behind it. SageMaker's card stays — SageMaker Savings Plans is a real, distinct SP type (separate product since 2020). - frontend/src/index.html: drop the Lambda service-default-card; rephrase the umbrella SP hint copy so SageMaker is the only call-out (Lambda is implicitly part of "Compute (EC2, Fargate, Lambda)"). - frontend/src/settings.ts: remove the aws/lambda entry from SERVICE_FIELDS, with a comment explaining the omission so a future copy-paste doesn't re-add it. - frontend/src/__tests__/settings.test.ts: drop the three Lambda-specific tests (presence, save round-trip, no-upfront visibility), drop Lambda IDs from the test fixture, and add a negative-guard test asserting no Lambda card or selects ever land in index.html. updateServiceConfig call-count expectation: 17 → 16 (7 AWS + 5 Azure + 4 GCP).
1 parent 2e33c88 commit e9ab8ba

3 files changed

Lines changed: 86 additions & 6 deletions

File tree

‎frontend/src/__tests__/settings.test.ts‎

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,16 @@ describe('Settings Module', () => {
8282
<select id="aws-opensearch-term"><option value="1">1</option><option value="3">3</option></select>
8383
<select id="aws-redshift-term"><option value="1">1</option><option value="3">3</option></select>
8484
<select id="aws-savingsplans-term"><option value="1">1</option><option value="3">3</option></select>
85+
<!-- Issue #22: SageMaker per-service card (its own SP type). Lambda
86+
intentionally omitted — it has no standalone SP, only Compute SP. -->
87+
<select id="aws-sagemaker-term"><option value="1">1</option><option value="3">3</option></select>
8588
<select id="aws-ec2-payment"><option value="no-upfront">No</option><option value="partial-upfront">Partial</option><option value="all-upfront">All</option></select>
8689
<select id="aws-rds-payment"><option value="no-upfront">No</option><option value="partial-upfront">Partial</option><option value="all-upfront">All</option></select>
8790
<select id="aws-elasticache-payment"><option value="no-upfront">No</option><option value="partial-upfront">Partial</option><option value="all-upfront">All</option></select>
8891
<select id="aws-opensearch-payment"><option value="no-upfront">No</option><option value="partial-upfront">Partial</option><option value="all-upfront">All</option></select>
8992
<select id="aws-redshift-payment"><option value="no-upfront">No</option><option value="partial-upfront">Partial</option><option value="all-upfront">All</option></select>
9093
<select id="aws-savingsplans-payment"><option value="no-upfront">No</option><option value="partial-upfront">Partial</option><option value="all-upfront">All</option></select>
94+
<select id="aws-sagemaker-payment"><option value="no-upfront">No</option><option value="partial-upfront">Partial</option><option value="all-upfront">All</option></select>
9195
<!-- Azure term selects -->
9296
<select id="azure-vm-term"><option value="1">1</option><option value="3">3</option></select>
9397
<select id="azure-sql-term"><option value="1">1</option><option value="3">3</option></select>
@@ -108,11 +112,11 @@ describe('Settings Module', () => {
108112
(document.getElementById('setting-default-payment') as HTMLSelectElement).value = 'all-upfront';
109113
// Mirror the same baseline onto every per-service select so the cascade
110114
// diff only reports genuinely-changed rows.
111-
['aws-ec2-term','aws-rds-term','aws-elasticache-term','aws-opensearch-term','aws-redshift-term','aws-savingsplans-term','azure-vm-term','azure-sql-term','azure-cosmos-term','gcp-compute-term','gcp-sql-term'].forEach(id => {
115+
['aws-ec2-term','aws-rds-term','aws-elasticache-term','aws-opensearch-term','aws-redshift-term','aws-savingsplans-term','aws-sagemaker-term','azure-vm-term','azure-sql-term','azure-cosmos-term','gcp-compute-term','gcp-sql-term'].forEach(id => {
112116
const el = document.getElementById(id) as HTMLSelectElement | null;
113117
if (el) el.value = '3';
114118
});
115-
['aws-ec2-payment','aws-rds-payment','aws-elasticache-payment','aws-opensearch-payment','aws-redshift-payment','aws-savingsplans-payment'].forEach(id => {
119+
['aws-ec2-payment','aws-rds-payment','aws-elasticache-payment','aws-opensearch-payment','aws-redshift-payment','aws-savingsplans-payment','aws-sagemaker-payment'].forEach(id => {
116120
const el = document.getElementById(id) as HTMLSelectElement | null;
117121
if (el) el.value = 'all-upfront';
118122
});
@@ -506,16 +510,64 @@ describe('Settings Module', () => {
506510
expect(api.updateConfig).toHaveBeenCalled();
507511
});
508512

509-
test('calls updateServiceConfig once per service field (15 calls)', async () => {
513+
// Issue #22: SageMaker has its own SP type, so it gets a dedicated
514+
// purchasing-defaults card. Verify the card exists in the canonical
515+
// index.html (so the deployed UI actually renders it) and that the
516+
// save flow round-trips edits to the per-service config endpoint.
517+
// Lambda is intentionally absent — it has no standalone SP product.
518+
test('SageMaker card present in index.html with term and payment selects (issue #22)', () => {
519+
const fs = require('fs') as typeof import('fs');
520+
const path = require('path') as typeof import('path');
521+
const html = fs.readFileSync(path.join(__dirname, '..', 'index.html'), 'utf-8');
522+
expect(html).toMatch(/<h5>\s*SageMaker Savings Plans\s*<\/h5>/);
523+
expect(html).toMatch(/id="aws-sagemaker-term"/);
524+
expect(html).toMatch(/id="aws-sagemaker-payment"/);
525+
});
526+
527+
test('no Lambda card in index.html — Lambda has no standalone SP, only Compute SP (issue #22)', () => {
528+
const fs = require('fs') as typeof import('fs');
529+
const path = require('path') as typeof import('path');
530+
const html = fs.readFileSync(path.join(__dirname, '..', 'index.html'), 'utf-8');
531+
// Guard against accidental re-introduction of a Lambda-specific card.
532+
expect(html).not.toMatch(/<h5>\s*Lambda Savings Plans\s*<\/h5>/);
533+
expect(html).not.toMatch(/id="aws-lambda-term"/);
534+
expect(html).not.toMatch(/id="aws-lambda-payment"/);
535+
});
536+
537+
test('saveGlobalSettings sends per-service SageMaker term and payment (issue #22)', async () => {
538+
(api.updateConfig as jest.Mock).mockResolvedValue({});
539+
(api.updateServiceConfig as jest.Mock).mockClear().mockResolvedValue(undefined);
540+
window.alert = jest.fn();
541+
542+
// User pins SageMaker to 1yr / no-upfront while leaving the global
543+
// default at 3yr / all-upfront.
544+
(document.getElementById('aws-sagemaker-term') as HTMLSelectElement).value = '1';
545+
(document.getElementById('aws-sagemaker-payment') as HTMLSelectElement).value = 'no-upfront';
546+
547+
const event = { preventDefault: jest.fn() } as unknown as Event;
548+
await saveGlobalSettings(event);
549+
550+
const call = (api.updateServiceConfig as jest.Mock).mock.calls.find(
551+
([provider, service]) => provider === 'aws' && service === 'sagemaker',
552+
);
553+
expect(call).toBeDefined();
554+
const cfg = call![2];
555+
expect(cfg.term).toBe(1);
556+
expect(cfg.payment).toBe('no-upfront');
557+
});
558+
559+
test('calls updateServiceConfig once per service field (16 calls)', async () => {
510560
(api.updateConfig as jest.Mock).mockResolvedValue({});
511561
(api.updateServiceConfig as jest.Mock).mockResolvedValue(undefined);
512562
window.alert = jest.fn();
513563

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

517-
// 6 AWS + 5 Azure (vm, sql, cosmosdb, redis, search) + 4 GCP.
518-
expect(api.updateServiceConfig).toHaveBeenCalledTimes(15);
567+
// 7 AWS (ec2, rds, elasticache, opensearch, redshift, savingsplans,
568+
// sagemaker — last one added per issue #22) + 5 Azure
569+
// (vm, sql, cosmosdb, redis, search) + 4 GCP.
570+
expect(api.updateServiceConfig).toHaveBeenCalledTimes(16);
519571
});
520572
}); // end saveGlobalSettings
521573

@@ -835,6 +887,21 @@ describe('Settings Module', () => {
835887
},
836888
);
837889

890+
test('SageMaker 3yr keeps "no-upfront" visible (issue #22 — no service-level restriction)', async () => {
891+
(api.getConfig as jest.Mock).mockResolvedValue({
892+
global: { enabled_providers: ['aws'], default_term: 3, default_payment: 'no-upfront', default_coverage: 80 },
893+
services: [{ provider: 'aws', service: 'sagemaker', term: 3, payment: 'no-upfront' }],
894+
});
895+
setupSettingsHandlers();
896+
await loadGlobalSettings();
897+
898+
const payment = document.getElementById('aws-sagemaker-payment') as HTMLSelectElement;
899+
expect(optVisible(payment, 'no-upfront')).toBe(true);
900+
expect(optVisible(payment, 'partial-upfront')).toBe(true);
901+
expect(optVisible(payment, 'all-upfront')).toBe(true);
902+
expect(payment.value).toBe('no-upfront');
903+
});
904+
838905
test('propagating global "no-upfront" to all services while term=3 clamps restricted services', async () => {
839906
(api.getConfig as jest.Mock).mockResolvedValue({
840907
global: { enabled_providers: ['aws'], default_term: 3, default_payment: 'all-upfront', default_coverage: 80 },

‎frontend/src/index.html‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,10 +485,16 @@ <h5>Redshift Reserved Nodes</h5>
485485
</div>
486486
<div class="service-default-card">
487487
<h5>Savings Plans</h5>
488-
<p class="service-default-hint">Compute (EC2, Fargate, Lambda), EC2 Instance, SageMaker, and Database (RDS) plans share these defaults.</p>
488+
<p class="service-default-hint">Compute (EC2, Fargate, Lambda), EC2 Instance, and Database (RDS) plans share these defaults. SageMaker is its own SP type — see card below.</p>
489489
<label>Term: <select id="aws-savingsplans-term"><option value="1">1 Year</option><option value="3" selected>3 Years</option></select></label>
490490
<label>Payment: <select id="aws-savingsplans-payment"><option value="no-upfront">No Upfront</option><option value="partial-upfront">Partial</option><option value="all-upfront" selected>All Upfront</option></select></label>
491491
</div>
492+
<div class="service-default-card">
493+
<h5>SageMaker Savings Plans</h5>
494+
<p class="service-default-hint">SageMaker Savings Plans are a distinct SP product (separate from Compute SP). These defaults narrow SageMaker recommendations independently of the umbrella Savings Plans card.</p>
495+
<label>Term: <select id="aws-sagemaker-term"><option value="1">1 Year</option><option value="3" selected>3 Years</option></select></label>
496+
<label>Payment: <select id="aws-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>
497+
</div>
492498
</div>
493499
</fieldset>
494500

‎frontend/src/settings.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ const SERVICE_FIELDS = [
3232
{ provider: 'aws', service: 'opensearch', termId: 'aws-opensearch-term', paymentId: 'aws-opensearch-payment' },
3333
{ provider: 'aws', service: 'redshift', termId: 'aws-redshift-term', paymentId: 'aws-redshift-payment' },
3434
{ provider: 'aws', service: 'savingsplans', termId: 'aws-savingsplans-term', paymentId: 'aws-savingsplans-payment' },
35+
// Issue #22: SageMaker has its own SP type (SageMaker Savings Plans),
36+
// so it gets a dedicated card and users can pin term/payment per workload
37+
// instead of sharing the umbrella Savings Plans defaults. Lambda is
38+
// intentionally NOT here — Lambda has no standalone SP product; its
39+
// commitments roll up into Compute Savings Plans, already covered by
40+
// the umbrella card above.
41+
{ provider: 'aws', service: 'sagemaker', termId: 'aws-sagemaker-term', paymentId: 'aws-sagemaker-payment' },
3542
{ provider: 'azure', service: 'vm', termId: 'azure-vm-term', paymentId: 'azure-vm-payment' },
3643
{ provider: 'azure', service: 'sql', termId: 'azure-sql-term', paymentId: 'azure-sql-payment' },
3744
{ provider: 'azure', service: 'cosmosdb', termId: 'azure-cosmosdb-term', paymentId: 'azure-cosmosdb-payment' },

0 commit comments

Comments
 (0)