From 8be0f564b4f91dfdff8a3a350ac3ec8ac0752294 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 27 Apr 2026 19:37:01 +0200 Subject: [PATCH] =?UTF-8?q?feat(settings):=20per-account=20overrides=20mod?= =?UTF-8?q?al=20=E2=80=94=20fixes=20panel-stacking=20bug=20(closes=20#122,?= =?UTF-8?q?=20#116)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Overrides UX from #72 (inline payment edit) and #106 (create modal) rendered each account's overrides into an inline expandable panel appended below the entire accounts table. Two open panels stacked without per-row attachment, so an override row created for one account appeared visually adjacent to a different account's empty-state. From the user's screenshot: an aws/ec2 override created for AWS 540... was displayed under CUDly host (909...), with two unscoped "Add override" buttons and a confusing "No service overrides yet for this account." text in between. Rework: every Overrides operation now lives inside a per-account modal keyed by an explicit title. Clicking the row's Overrides button opens account-overrides-modal whose title reads "Service overrides for {account.name} ({external_id})". The modal body uses the same loadOverridesPanel rendering function as before, so all four CRUD operations carry forward unchanged: - LIST: the existing overrides-table with Service / Term / Payment / Coverage / Reset, including #72's inline payment as #72, now scoped inside the modal body so its loadOverridesPanel reload target is unambiguous. - DELETE: Reset button + confirmDialog, identical to before. renderAccountsList no longer creates an inline panel per row, no longer collects them in a panels[] array, and no longer appends them after the table — that whole inline-panel scaffolding is gone. Empty-state UX is preserved: an AWS account with zero overrides still auto-opens the inner create modal so users land directly on the form in 1 click instead of 2. Defensive: closeAccountOverridesModal also closes the inner create modal so a programmatic close (or future ESC-to-close from #115) doesn't leave an orphan modal whose Save would target a hidden parent. Tests: existing #72 + #106 test helpers updated to look for #account-overrides-modal-body instead of .account-overrides-panel. Four new tests in a "Account overrides modal" describe block lock in the behaviour: title-binds-to-account, switching-accounts-swaps-title, close-clears-body, and a regression guard that .account-overrides-panel no longer renders. Full settings-accounts test count: 40/40 (was 36 + 4 new). Full frontend suite: 1273/1273 green; typecheck + production build clean. --- .../src/__tests__/settings-accounts.test.ts | 119 +++++++++++++++++- frontend/src/index.html | 16 +++ frontend/src/settings.ts | 66 ++++++++-- 3 files changed, 186 insertions(+), 15 deletions(-) diff --git a/frontend/src/__tests__/settings-accounts.test.ts b/frontend/src/__tests__/settings-accounts.test.ts index ab05cb50..bd83774a 100644 --- a/frontend/src/__tests__/settings-accounts.test.ts +++ b/frontend/src/__tests__/settings-accounts.test.ts @@ -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); } // --------------------------------------------------------------------------- @@ -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!; } @@ -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(); @@ -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). @@ -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(); + }); +}); diff --git a/frontend/src/index.html b/frontend/src/index.html index 05c783c4..c71387c8 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -990,6 +990,22 @@

Duplicate Group

+ + +