Skip to content

feat(settings): Azure and GCP create-override modal (closes #109) #588

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 27, 2026
Merged

feat(settings): Azure and GCP create-override modal (closes #109) #588

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
eb5641b
feat(settings): Azure and GCP create-override modal (closes #109)
cristim May 20, 2026
95d1384
docs(frontend/settings): update stale Enabled-cell comment to reflect…
cristim May 27, 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
59 changes: 40 additions & 19 deletions frontend/src/__tests__/settings-accounts.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,14 +650,22 @@ describe('Overrides panel — AWS payment selector', () => {
expect(api.saveAccountServiceOverride).not.toHaveBeenCalled();
});

test('non-AWS rows render the existing read-only payment cell, no <select>', async () => {
test('Azure rows render an editable payment select with provider-appropriate options (issue #109)', async () => {
// Issue #109: Azure override rows now get inline payment editing.
// Azure uses upfront/monthly (not the AWS no-upfront/partial/all-upfront set).
(api.listAccountServiceOverrides as jest.Mock).mockResolvedValue([
{ id: 'o1', account_id: 'acc-1', provider: 'azure', service: 'vm', payment: 'all-upfront' },
{ id: 'o1', account_id: 'acc-1', provider: 'azure', service: 'vm', term: 1, payment: 'upfront' },
]);

const panel = await openOverridesPanel('acc-1');
expect(panel.querySelector('select.override-payment-select')).toBeNull();
expect(panel.textContent).toContain('all-upfront');
const sel = panel.querySelector('select.override-payment-select') as HTMLSelectElement | null;
expect(sel).not.toBeNull();
const options = Array.from(sel!.options).map(o => o.value);
// Azure has upfront + monthly (not the AWS no-upfront/partial/all-upfront set).
expect(options).toContain('upfront');
expect(options).toContain('monthly');
expect(options).not.toContain('no-upfront');
expect(options).not.toContain('all-upfront');
});
});

Expand Down Expand Up @@ -937,21 +945,25 @@ describe('Overrides panel: inline Term/Coverage/Enabled (issue #110)', () => {

// ----- Non-AWS rows -----

test('Non-AWS rows: term/coverage render as text, enabled checkbox is disabled with tooltip', async () => {
test('Azure rows: term/coverage/enabled are all editable (issue #109)', async () => {
// Issue #109: Azure override rows now have inline editing on all fields,
// same as AWS rows after issue #110.
(api.listAccountServiceOverrides as jest.Mock).mockResolvedValue([
{ id: 'o1', account_id: 'acc-1', provider: 'azure', service: 'vm', term: 1, coverage: 80 },
]);

const panel = await openOverridesPanel('acc-1');
expect(panel.querySelector('select.override-term-select')).toBeNull();
expect(panel.querySelector('input.override-coverage-input')).toBeNull();
// Term select is present and editable.
const termSel = panel.querySelector('select.override-term-select') as HTMLSelectElement | null;
expect(termSel).not.toBeNull();
expect(termSel!.disabled).toBe(false);
// Coverage input is present.
const covInput = panel.querySelector('input.override-coverage-input') as HTMLInputElement | null;
expect(covInput).not.toBeNull();
// Enabled toggle is present and not disabled.
const cb = panel.querySelector('input.override-enabled-toggle') as HTMLInputElement;
expect(cb).not.toBeNull();
expect(cb.disabled).toBe(true);
expect(cb.title).toContain('AWS-only');
// Read-only fallbacks for term and coverage on non-AWS rows.
expect(panel.textContent).toContain('1yr');
expect(panel.textContent).toContain('80%');
expect(cb.disabled).toBe(false);
});
});

Expand Down Expand Up @@ -1137,7 +1149,10 @@ describe('Create-override modal', () => {
expect(api.saveAccountServiceOverride).not.toHaveBeenCalled();
});

test('non-AWS account: empty state shows passive text, no modal auto-open, no Add button', async () => {
test('Azure account: empty state auto-opens the create modal with Azure service list (issue #109)', async () => {
// Issue #109: Azure accounts now get the same "Add override" flow as AWS.
// The empty-state auto-opens the create modal and the service dropdown
// is populated with Azure services (vm, sql, cosmosdb, redis, search).
(api.listAccounts as jest.Mock).mockResolvedValue([
{ id: 'az-1', name: 'AzureProd', provider: 'azure', external_id: 'sub-x', enabled: true },
]);
Expand All @@ -1153,15 +1168,21 @@ describe('Create-override modal', () => {
const panel = document.getElementById('account-overrides-modal-body') as HTMLElement;
const modal = document.getElementById('override-modal') as HTMLElement;

// The inner create modal must NOT have auto-opened for a non-AWS account.
expect(modal.classList.contains('hidden')).toBe(true);
// The inner create modal auto-opens (same as AWS empty state).
expect(modal.classList.contains('hidden')).toBe(false);

// No Add override button on non-AWS for now (issue #104 follow-up).
// Add override button is present.
const addBtn = Array.from(panel.querySelectorAll('button')).find(b => b.textContent === 'Add override');
expect(addBtn).toBeUndefined();
expect(addBtn).toBeDefined();

// Passive empty-state copy is what they see.
expect(panel.textContent).toContain('No service overrides set');
// Service dropdown lists Azure services, not AWS services.
const svcSel = document.getElementById('override-service') as HTMLSelectElement;
const svcValues = Array.from(svcSel.options).map(o => o.value);
expect(svcValues).toContain('vm');
expect(svcValues).toContain('sql');
expect(svcValues).toContain('cosmosdb');
expect(svcValues).not.toContain('ec2');
expect(svcValues).not.toContain('rds');
});

test('all services already overridden disables submit', async () => {
Expand Down
120 changes: 58 additions & 62 deletions frontend/src/settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { initFederationPanel } from './federation';
import { confirmDialog } from './confirmDialog';
import { reflectDirtyState } from './settings-subnav';
import { showToast } from './toast';
import { isValidCombination, getValidPaymentOptions, getValidTermOptions, getPaymentLabel } from './commitmentOptions';
import { isValidCombination, getValidPaymentOptions, getValidTermOptions, getPaymentLabel, getCommitmentConfig } from './commitmentOptions';
import { openModal, closeModal } from './modal';
import { loadRecommendations } from './recommendations';
import { canAccess } from './permissions';
Expand Down Expand Up @@ -569,17 +569,10 @@ function renderAccountsList(
// unambiguous.
}

// AWS payment-option choices, kept in sync with the per-service Purchasing
// selectors in frontend/src/index.html (e.g. #aws-ec2-payment). Centralised
// here so the override editor and the global selectors can't drift.
const AWS_PAYMENT_OPTIONS: ReadonlyArray<{ value: string; label: string }> = [
{ value: 'no-upfront', label: 'No Upfront' },
{ value: 'partial-upfront', label: 'Partial Upfront' },
{ value: 'all-upfront', label: 'All Upfront' },
];

// AWS services that can carry a per-account override. Mirrors the AWS-only
// entries in SERVICE_FIELDS \u2014 Azure/GCP override creation is a follow-up
// entries in SERVICE_FIELDS. Issue #109 adds AZURE_OVERRIDE_SERVICES and
// GCP_OVERRIDE_SERVICES below; getOverrideServicesForProvider dispatches to
// the right list so the create modal is provider-aware.
// (issue #104) since the backend payment/term semantics differ per provider.
const AWS_OVERRIDE_SERVICES: ReadonlyArray<{ value: string; label: string }> = [
{ value: 'ec2', label: 'EC2 (Reserved Instances)' },
Expand All @@ -598,6 +591,29 @@ const AWS_OVERRIDE_SERVICES: ReadonlyArray<{ value: string; label: string }> = [
{ value: 'savings-plans-database', label: 'Database Savings Plans' },
];

// Human-readable labels for Azure and GCP services used in the override
// service dropdown. AWS labels are carried by AWS_OVERRIDE_SERVICES above.
const AZURE_OVERRIDE_SERVICES: ReadonlyArray<{ value: string; label: string }> = [
{ value: 'vm', label: 'Virtual Machines' },
{ value: 'sql', label: 'SQL Database' },
{ value: 'cosmosdb', label: 'Cosmos DB' },
{ value: 'redis', label: 'Cache for Redis' },
{ value: 'search', label: 'AI Search' },
];

const GCP_OVERRIDE_SERVICES: ReadonlyArray<{ value: string; label: string }> = [
{ value: 'compute', label: 'Compute Engine' },
{ value: 'sql', label: 'Cloud SQL' },
{ value: 'memorystore', label: 'Memorystore' },
{ value: 'storage', label: 'Cloud Storage' },
];

/** Return the service list for the given provider's override modal. */
function getOverrideServicesForProvider(provider: string): ReadonlyArray<{ value: string; label: string }> {
if (provider === 'azure') return AZURE_OVERRIDE_SERVICES;
if (provider === 'gcp') return GCP_OVERRIDE_SERVICES;
return AWS_OVERRIDE_SERVICES;
}

/**
* Build the per-row Payment <select> for the overrides panel. Issue #23.
Expand Down Expand Up @@ -629,15 +645,16 @@ function buildPaymentOverrideSelect(
select.appendChild(inheritOpt);

// Filter the payment dropdown to only the (term, payment) combinations
// AWS actually supports for THIS service. e.g. RDS rejects 3yr no-upfront,
// so an RDS override with term=3 must not list no-upfront. Per #107.
// Falls back to the full AWS list when term is missing (override row was
// saved without a term) — the global default's term will resolve at
// the provider/service actually supports. e.g. RDS rejects 3yr no-upfront;
// Azure has upfront/monthly only; GCP has monthly only. Per #107.
// Falls back to the full provider payment list when term is missing (override
// row was saved without a term) — the global default's term will resolve at
// recommendation time, and we can't pre-validate without it.
const overrideTerm = override.term ?? 0;
const providerPayments = getCommitmentConfig(override.provider).payments;
const validPayments = overrideTerm > 0
? getValidPaymentOptions(override.provider, override.service, overrideTerm)
: AWS_PAYMENT_OPTIONS.map(p => ({ value: p.value, label: p.label }));
: providerPayments.map(p => ({ value: p.value, label: p.label }));

for (const { value, label } of validPayments) {
const opt = document.createElement('option');
Expand Down Expand Up @@ -705,9 +722,6 @@ async function handlePaymentOverrideChange(
message: `Payment override updated for ${override.provider}/${override.service}.`,
kind: 'success',
});
// The inline payment selector only renders for AWS rows (per the
// o.provider === 'aws' guard in loadOverridesPanel's row loop), so
// override.provider is always 'aws' at this call site — cast is safe.
await loadOverridesPanel(accountId, panel, override.provider as AccountProvider);
await refreshRecommendationsAfterOverrideChange();
} catch (err) {
Expand Down Expand Up @@ -969,12 +983,6 @@ function buildEnabledOverrideCheckbox(
`Enable ${override.provider}/${override.service} override`,
);

if (override.provider !== 'aws') {
input.disabled = true;
input.title = 'Per-provider override editing is AWS-only — Azure/GCP coming with the provider-aware modal';
return input;
}

input.addEventListener('change', () => {
void handleEnabledOverrideChange(accountId, override, input, panel, row);
});
Expand Down Expand Up @@ -1031,9 +1039,10 @@ async function handleEnabledOverrideChange(
* Populated state: render the existing table + an "Add override" button at
* the top so users can add another override for a different service.
*
* The Add Override flow is AWS-only for now; Azure/GCP keep the read-only
* empty-state text since their per-product term/payment semantics differ
* (issue #104 follow-up tracks Azure/GCP modal support).
* Issue #109: Azure and GCP accounts now also get the "Add override" button
* and full inline editing (term, payment, coverage, enabled) on existing rows.
* Provider-specific payment options are derived via getCommitmentConfig so
* GCP shows "Monthly only" and Azure shows "Upfront / Monthly".
*/
/**
* Open the per-account overrides modal (issue #122). Replaces the inline
Expand Down Expand Up @@ -1080,14 +1089,13 @@ export async function loadOverridesPanel(accountId: string, panel: HTMLElement,
const overrides = await api.listAccountServiceOverrides(accountId);
panel.textContent = '';

const canCreate = provider === 'aws';
// Issue #109: all providers can now create overrides.
const canCreate = true;

if (!overrides || overrides.length === 0) {
const msg = document.createElement('p');
msg.className = 'help-text';
msg.textContent = canCreate
? 'No service overrides yet for this account.'
: 'No service overrides set. All services use global defaults.';
msg.textContent = 'No service overrides yet for this account.';
panel.appendChild(msg);
if (canCreate) {
// No overrides yet \u2014 auto-open the modal so the user lands directly
Expand Down Expand Up @@ -1143,37 +1151,24 @@ export async function loadOverridesPanel(accountId: string, panel: HTMLElement,
const serviceTd = tr.insertCell();
serviceTd.textContent = `${o.provider}/${o.service}`;

// Term cell: editable <select> for AWS (issue #110); read-only text
// for Azure/GCP until the provider-aware modal lands.
// Term cell: editable <select> for all providers (issue #109 extends
// the AWS-only #110 inline editing to Azure/GCP).
const termTd = tr.insertCell();
if (o.provider === 'aws') {
termTd.appendChild(buildTermOverrideSelect(accountId, o, panel));
} else {
termTd.textContent = o.term !== undefined ? `${o.term}yr` : '\u2014';
}
termTd.appendChild(buildTermOverrideSelect(accountId, o, panel));

// Payment cell: editable <select> for AWS (the only provider whose
// reservations support distinct payment options); read-only text for
// Azure/GCP. Issue #23.
// Payment cell: editable <select> for all providers. GCP's payment
// dropdown only shows "Monthly"; Azure shows upfront/monthly. The
// underlying getValidPaymentOptions already handles per-provider
// filtering, so the same builder works for all three clouds.
const paymentTd = tr.insertCell();
if (o.provider === 'aws') {
paymentTd.appendChild(buildPaymentOverrideSelect(accountId, o, panel));
} else {
paymentTd.textContent = o.payment ?? '\u2014';
}
paymentTd.appendChild(buildPaymentOverrideSelect(accountId, o, panel));

// Coverage cell: editable numeric <input> for AWS (issue #110);
// read-only text for Azure/GCP.
// Coverage cell: editable numeric <input> for all providers (issue #109).
const coverageTd = tr.insertCell();
if (o.provider === 'aws') {
coverageTd.appendChild(buildCoverageOverrideInput(accountId, o, panel));
} else {
coverageTd.textContent = o.coverage !== undefined ? `${o.coverage}%` : '\u2014';
}
coverageTd.appendChild(buildCoverageOverrideInput(accountId, o, panel));

// Enabled cell: per-row checkbox toggling account_service_overrides.enabled.
// AWS-editable, disabled (with tooltip) for non-AWS to keep the column
// alignment consistent across provider types. Issue #110.
// Editable for all providers. Issue `#110`, extended to Azure/GCP in `#109`.
const enabledTd = tr.insertCell();
enabledTd.appendChild(buildEnabledOverrideCheckbox(accountId, o, panel, tr));

Expand Down Expand Up @@ -1311,9 +1306,9 @@ export function openOverrideModal(
const errEl = document.getElementById('override-form-error');
if (errEl) errEl.textContent = '';

// Populate the service dropdown, excluding services this account already
// has an override for. AWS-only for now (issue #104 follow-up tracks
// Azure/GCP).
// Populate the service dropdown with provider-specific services, excluding
// any that already have an override for this account. Issue #109 extends
// this to Azure and GCP (previously AWS-only per issue #104 follow-up).
const used = new Set(
existingOverrides
.filter(o => o.provider === provider)
Expand All @@ -1322,7 +1317,7 @@ export function openOverrideModal(
const select = document.getElementById('override-service') as HTMLSelectElement | null;
if (select) {
select.replaceChildren();
const available = AWS_OVERRIDE_SERVICES.filter(s => !used.has(s.value));
const available = getOverrideServicesForProvider(provider).filter(s => !used.has(s.value));
if (available.length === 0) {
const opt = document.createElement('option');
opt.value = '';
Expand Down Expand Up @@ -1382,11 +1377,12 @@ function syncOverridePaymentOptions(provider: string): void {
paymentSel.appendChild(inheritOpt);

// When service or term is unset, can't run the validity check yet —
// show the full AWS payment list so the user can choose; the
// show the full provider payment list so the user can choose; the
// submit-side guard will catch any invalid combo on Save.
const provPayments = getCommitmentConfig(provider).payments;
const candidates = (service && Number.isFinite(term) && term > 0)
? getValidPaymentOptions(provider, service, term).map(p => ({ value: p.value, label: p.label }))
: AWS_PAYMENT_OPTIONS.map(p => ({ value: p.value, label: p.label }));
: provPayments.map(p => ({ value: p.value, label: p.label }));

for (const { value, label } of candidates) {
const opt = document.createElement('option');
Expand Down
Loading