Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
119 changes: 115 additions & 4 deletions frontend/src/__tests__/settings-accounts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,15 @@ function buildAccountsDOM(): void {
overrideModal.appendChild(overrideForm);
overrideModal.appendChild(btn('close-override-modal-btn'));
document.body.appendChild(overrideModal);

// ── Per-account overrides modal (issue #122) ───────────────
const accountOverridesModal = div('account-overrides-modal', 'modal hidden');
const accountOverridesTitle = el('h2', {}, 'account-overrides-modal-title');
accountOverridesTitle.textContent = 'Service overrides';
accountOverridesModal.appendChild(accountOverridesTitle);
accountOverridesModal.appendChild(div('account-overrides-modal-body'));
accountOverridesModal.appendChild(btn('close-account-overrides-modal-btn'));
document.body.appendChild(accountOverridesModal);
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -516,7 +525,9 @@ describe('Overrides panel — AWS payment selector', () => {
overridesBtn!.click();
// loadOverridesPanel is async; let microtasks flush.
await new Promise(r => setTimeout(r, 0));
const panel = document.querySelector('.account-overrides-panel') as HTMLElement | null;
// Issue #122: the inline expandable panel was replaced by a per-account
// modal. The body element is what loadOverridesPanel renders into.
const panel = document.getElementById('account-overrides-modal-body') as HTMLElement | null;
expect(panel).not.toBeNull();
return panel!;
}
Expand Down Expand Up @@ -605,7 +616,8 @@ describe('Create-override modal', () => {
expect(overridesBtn).not.toBeNull();
overridesBtn!.click();
await new Promise(r => setTimeout(r, 0));
const panel = document.querySelector('.account-overrides-panel') as HTMLElement | null;
// Issue #122: panel is now inside the per-account overrides modal.
const panel = document.getElementById('account-overrides-modal-body') as HTMLElement | null;
expect(panel).not.toBeNull();
const modal = document.getElementById('override-modal') as HTMLElement | null;
expect(modal).not.toBeNull();
Expand Down Expand Up @@ -773,10 +785,10 @@ describe('Create-override modal', () => {
overridesBtn.click();
await new Promise(r => setTimeout(r, 0));

const panel = document.querySelector('.account-overrides-panel') as HTMLElement;
const panel = document.getElementById('account-overrides-modal-body') as HTMLElement;
const modal = document.getElementById('override-modal') as HTMLElement;

// Modal must NOT have auto-opened for a non-AWS account.
// The inner create modal must NOT have auto-opened for a non-AWS account.
expect(modal.classList.contains('hidden')).toBe(true);

// No Add override button on non-AWS for now (issue #104 follow-up).
Expand Down Expand Up @@ -804,3 +816,102 @@ describe('Create-override modal', () => {
expect(submitBtn.disabled).toBe(true);
});
});

// ---------------------------------------------------------------------------
// Per-account overrides modal — issue #122
// ---------------------------------------------------------------------------

describe('Account overrides modal', () => {
beforeEach(() => {
buildAccountsDOM();
setupSettingsHandlers();
jest.clearAllMocks();
});

test('Overrides button opens an account-scoped modal whose title binds to the account', async () => {
(api.listAccounts as jest.Mock).mockResolvedValue([
{ id: 'acc-1', name: 'AWS Prod', provider: 'aws', external_id: '540659244915', enabled: true },
]);
(api.listAccountServiceOverrides as jest.Mock).mockResolvedValue([
{ id: 'o1', account_id: 'acc-1', provider: 'aws', service: 'ec2', payment: 'all-upfront' },
]);

await loadAccountsForProvider('aws');
const btn = document.querySelector(
`button[aria-label="Service overrides for AWS Prod (540659244915)"]`,
) as HTMLButtonElement;
btn.click();
await new Promise(r => setTimeout(r, 0));

const modal = document.getElementById('account-overrides-modal') as HTMLElement;
expect(modal.classList.contains('hidden')).toBe(false);
const title = document.getElementById('account-overrides-modal-title') as HTMLElement;
expect(title.textContent).toBe('Service overrides for AWS Prod (540659244915)');
});

test('switching accounts swaps the modal title; only one account context is active at a time', async () => {
(api.listAccounts as jest.Mock).mockResolvedValue([
{ id: 'acc-a', name: 'AWS Prod', provider: 'aws', external_id: '540659244915', enabled: true },
{ id: 'acc-b', name: 'CUDly host', provider: 'aws', external_id: '909626172446', enabled: true },
]);
(api.listAccountServiceOverrides as jest.Mock).mockResolvedValue([]);

await loadAccountsForProvider('aws');
const titleEl = document.getElementById('account-overrides-modal-title') as HTMLElement;

// Click Overrides on Account A.
(document.querySelector(
`button[aria-label="Service overrides for AWS Prod (540659244915)"]`,
) as HTMLButtonElement).click();
await new Promise(r => setTimeout(r, 0));
expect(titleEl.textContent).toContain('AWS Prod');

// Click Overrides on Account B without closing the modal first.
(document.querySelector(
`button[aria-label="Service overrides for CUDly host (909626172446)"]`,
) as HTMLButtonElement).click();
await new Promise(r => setTimeout(r, 0));

// Title now reflects Account B; only ONE modal exists in the DOM.
expect(titleEl.textContent).toContain('CUDly host');
expect(titleEl.textContent).not.toContain('AWS Prod');
expect(document.querySelectorAll('#account-overrides-modal').length).toBe(1);
});

test('Close button hides the modal and clears the body to avoid stale flash on next open', async () => {
(api.listAccounts as jest.Mock).mockResolvedValue([
{ id: 'acc-1', name: 'Prod', provider: 'aws', external_id: '111', enabled: true },
]);
(api.listAccountServiceOverrides as jest.Mock).mockResolvedValue([
{ id: 'o1', account_id: 'acc-1', provider: 'aws', service: 'ec2', payment: 'all-upfront' },
]);

await loadAccountsForProvider('aws');
(document.querySelector(
`button[aria-label="Service overrides for Prod (111)"]`,
) as HTMLButtonElement).click();
await new Promise(r => setTimeout(r, 0));

const modal = document.getElementById('account-overrides-modal') as HTMLElement;
const body = document.getElementById('account-overrides-modal-body') as HTMLElement;
expect(modal.classList.contains('hidden')).toBe(false);
expect(body.querySelector('table.overrides-table')).not.toBeNull();

(document.getElementById('close-account-overrides-modal-btn') as HTMLButtonElement).click();
expect(modal.classList.contains('hidden')).toBe(true);
// Body cleared so the next open doesn't flash stale rows for ~1ms.
expect(body.children.length).toBe(0);
});

test('no inline panel renders below the accounts table (issue #122 regression guard)', async () => {
(api.listAccounts as jest.Mock).mockResolvedValue([
{ id: 'acc-1', name: 'Prod', provider: 'aws', external_id: '111', enabled: true },
]);

await loadAccountsForProvider('aws');

// The inline expandable panel was the cause of the panel-stacking bug.
// Verify it is gone — the only override container is the modal body.
expect(document.querySelector('.account-overrides-panel')).toBeNull();
});
});
16 changes: 16 additions & 0 deletions frontend/src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,22 @@ <h2 id="group-duplicate-modal-title">Duplicate Group</h2>
</div>
</div>

<!-- Per-account overrides modal (issue #122) — replaces the inline
expandable panel that stacked all accounts' panels below the
accounts table without per-row attachment. Title binds the
modal to one specific account; body holds the overrides table
rendered dynamically. -->
<div id="account-overrides-modal" class="modal hidden" role="dialog" aria-modal="true" aria-labelledby="account-overrides-modal-title">
<div class="modal-content modal-wide">
<h2 id="account-overrides-modal-title">Service overrides</h2>
<p class="help-text">Per-account overrides take precedence over the global service defaults configured in Settings → Purchasing.</p>
<div id="account-overrides-modal-body"></div>
<div class="modal-buttons">
<button type="button" id="close-account-overrides-modal-btn">Close</button>
</div>
</div>
</div>

<!-- Cloud Account Modal -->
<div id="override-modal" class="modal hidden" role="dialog" aria-modal="true" aria-labelledby="override-modal-title">
<div class="modal-content">
Expand Down
66 changes: 55 additions & 11 deletions frontend/src/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@ function renderAccountsList(
table.appendChild(thead);

const tbody = document.createElement('tbody');
const panels: HTMLDivElement[] = [];

visible.forEach((account) => {
const accountLabel = `${account.name} (${account.external_id})`;
Expand Down Expand Up @@ -393,12 +392,7 @@ function renderAccountsList(
overridesBtn.className = 'btn btn-small';
overridesBtn.textContent = 'Overrides';
overridesBtn.setAttribute('aria-label', `Service overrides for ${accountLabel}`);
const overridesPanel = document.createElement('div');
overridesPanel.className = 'account-overrides-panel hidden';
overridesBtn.addEventListener('click', () => {
const hidden = overridesPanel.classList.toggle('hidden');
if (!hidden) void loadOverridesPanel(account.id, overridesPanel, account.provider);
});
overridesBtn.addEventListener('click', () => openAccountOverridesModal(account));
actionsTd.appendChild(overridesBtn);

// Destructive Delete lives in its own subgroup to isolate it from the
Expand All @@ -419,13 +413,13 @@ function renderAccountsList(

tr.appendChild(actionsTd);
tbody.appendChild(tr);
panels.push(overridesPanel);
});
table.appendChild(tbody);
container.appendChild(table);
// Append overrides panels after the table so "show overrides" expands
// below the row rather than inside it.
panels.forEach((p) => container.appendChild(p));
// Per-account overrides used to render in inline panels appended below
// the table; they now live in the account-overrides-modal (issue #122)
// so the accounts table stays compact and per-account scoping is
// unambiguous.
}

// AWS payment-option choices, kept in sync with the per-service Purchasing
Expand Down Expand Up @@ -561,6 +555,45 @@ async function handlePaymentOverrideChange(
* empty-state text since their per-product term/payment semantics differ
* (issue #104 follow-up tracks Azure/GCP modal support).
*/
/**
* Open the per-account overrides modal (issue #122). Replaces the inline
* expandable panel that used to render below the accounts table — the
* panels stacked without per-row attachment, so two open panels gave the
* user no way to tell which override row belonged to which account.
*
* The modal title binds explicitly to the account; the body uses the same
* `loadOverridesPanel` rendering function as before, so all CRUD wiring
* (Reset button, inline payment select, Add override button, empty-state
* auto-open of the create modal) carries forward unchanged.
*/
function openAccountOverridesModal(account: api.CloudAccount): void {
const modal = document.getElementById('account-overrides-modal');
if (!modal) return;
const titleEl = document.getElementById('account-overrides-modal-title');
if (titleEl) {
titleEl.textContent = `Service overrides for ${account.name} (${account.external_id})`;
}
const body = document.getElementById('account-overrides-modal-body');
if (!body) return;
modal.classList.remove('hidden');
void loadOverridesPanel(account.id, body, account.provider);
}

function closeAccountOverridesModal(): void {
const modal = document.getElementById('account-overrides-modal');
modal?.classList.add('hidden');
// Drop the body content so the next open doesn't briefly flash stale
// data from a previous account before loadOverridesPanel populates it.
const body = document.getElementById('account-overrides-modal-body');
if (body) body.replaceChildren();
// Close the inner create modal too if it's still open. The user can't
// visually reach the outer's close affordances while the inner is up
// (inner backdrop covers them), but a programmatic close — or a future
// ESC-to-close from issue #115 — should leave a clean slate, not an
// orphaned inner modal whose Save would target a hidden parent.
closeOverrideModal();
}

async function loadOverridesPanel(accountId: string, panel: HTMLElement, provider: AccountProvider): Promise<void> {
panel.textContent = 'Loading\u2026';
try {
Expand Down Expand Up @@ -1380,6 +1413,17 @@ export function setupSettingsHandlers(signal?: AbortSignal): void {
overrideModal?.addEventListener('click', (e) => {
if (e.target === overrideModal) closeOverrideModal();
}, { signal });

// Per-account overrides modal — close + click-outside-to-dismiss (issue #122).
document.getElementById('close-account-overrides-modal-btn')?.addEventListener(
'click',
closeAccountOverridesModal,
{ signal },
);
const accountOverridesModal = document.getElementById('account-overrides-modal');
accountOverridesModal?.addEventListener('click', (e) => {
if (e.target === accountOverridesModal) closeAccountOverridesModal();
}, { signal });
}

/**
Expand Down