diff --git a/frontend/src/__tests__/ladder.test.ts b/frontend/src/__tests__/ladder.test.ts index e9534438..32809282 100644 --- a/frontend/src/__tests__/ladder.test.ts +++ b/frontend/src/__tests__/ladder.test.ts @@ -12,6 +12,7 @@ import type { LadderConfig } from '../api'; import { waitFor } from '@testing-library/dom'; import { listAccountsMinimal, type AccountSummary } from '../api/accounts'; import { canAccess } from '../permissions'; +import { confirmDialog } from '../confirmDialog'; jest.mock('../api/accounts', () => ({ listAccountsMinimal: jest.fn() })); jest.mock('../ladder-timeline', () => ({ initLadderTimeline: jest.fn() })); @@ -23,6 +24,8 @@ jest.mock('../api', () => ({ updateConfig: jest.fn(), })); +jest.mock('../confirmDialog', () => ({ confirmDialog: jest.fn() })); + jest.mock('../permissions', () => ({ canAccess: jest.fn(() => true), })); @@ -67,6 +70,7 @@ describe('ladder.ts', () => { jest.clearAllMocks(); mockShowToast.mockReset(); (canAccess as jest.Mock).mockReturnValue(true); + (confirmDialog as jest.Mock).mockResolvedValue(true); (listAccountsMinimal as jest.Mock).mockResolvedValue([ { id: 'acct-1', name: 'AWS account', external_id: 'aws-external', provider: 'aws' }, { id: 'azure-id', name: 'Azure account', external_id: 'azure-external', provider: 'azure' }, @@ -97,6 +101,7 @@ describe('ladder.ts', () => { test('lookback input advertises the 10-day baseline minimum', async () => { await renderSection([baseConfig({ id: 'config-1' })]); expect(document.getElementById('ladder-cfg-lookback-days')).toHaveAttribute('min', '10'); + expect(document.getElementById('ladder-cfg-lookback-days')).toHaveAccessibleDescription('Minimum 10'); }); test.each([ @@ -346,8 +351,12 @@ describe('ladder.ts', () => { const status = (): HTMLElement => document.getElementById('ladder-cfg-account-status')!; const saveButton = (): HTMLElement => document.getElementById('ladder-config-save-btn')!; const changeProvider = (value: string): void => { provider().value = value; provider().dispatchEvent(new Event('change')); }; - async function open(): Promise { + // pendingAccounts lets a test hold the modal's account request open. It is + // installed after the section renders because the config table also loads + // account names during init. + async function open(pendingAccounts?: (mock: jest.Mock) => void): Promise { await renderSection(); + pendingAccounts?.(listAccountsMinimal as jest.Mock); document.getElementById('ladder-add-config-btn')!.click(); } async function ready(): Promise { await waitFor(() => expect(account()).toBeEnabled()); } @@ -395,8 +404,7 @@ describe('ladder.ts', () => { test('blocks save while loading and filters the provider selected before response', async () => { let resolve!: (accounts: AccountSummary[]) => void; - (listAccountsMinimal as jest.Mock).mockReturnValue(new Promise(done => { resolve = done; })); - await open(); + await open(mock => mock.mockReturnValue(new Promise(done => { resolve = done; }))); expect(account()).toBeDisabled(); expect(saveButton()).toBeDisabled(); expect(status()).toHaveTextContent(/loading/i); @@ -410,8 +418,7 @@ describe('ladder.ts', () => { test('a response from a closed modal cannot replace a reopened form', async () => { let resolve!: (accounts: AccountSummary[]) => void; - (listAccountsMinimal as jest.Mock).mockReturnValueOnce(new Promise(done => { resolve = done; })); - await open(); + await open(mock => mock.mockReturnValueOnce(new Promise(done => { resolve = done; }))); document.getElementById('ladder-modal-cancel-btn')!.click(); document.getElementById('ladder-add-config-btn')!.click(); await ready(); @@ -459,9 +466,85 @@ describe('ladder.ts', () => { (canAccess as jest.Mock).mockReturnValue(false); await saveLadderConfig(); expect(api.upsertLadderConfig).not.toHaveBeenCalled(); + (listAccountsMinimal as jest.Mock).mockClear(); await renderSection(); expect(document.getElementById('ladder-add-config-btn')).toBeNull(); + // Only the table's name lookup runs; no account selector is opened. expect(listAccountsMinimal).toHaveBeenCalledTimes(1); }); }); + + // Issue #742: readable account labels, auto-approve impact, server errors. + describe('account labels and auto-approve safeguards', () => { + const mode = (): HTMLSelectElement => document.getElementById('ladder-cfg-mode') as HTMLSelectElement; + const warning = (): HTMLElement => document.getElementById('ladder-cfg-mode-warning')!; + const setMode = (value: string): void => { mode().value = value; mode().dispatchEvent(new Event('change')); }; + async function openNew(): Promise { + await renderSection(); + document.getElementById('ladder-add-config-btn')!.click(); + await waitFor(() => expect(document.getElementById('ladder-cfg-account')).toBeEnabled()); + (document.getElementById('ladder-cfg-account') as HTMLSelectElement).value = 'acct-1'; + (api.upsertLadderConfig as jest.Mock).mockImplementation(async (cfg: LadderConfig) => cfg); + } + + test('table shows the registered account name and keeps the ID as the title', async () => { + await renderSection([baseConfig({ cloud_account_id: 'acct-1' })]); + const cell = document.querySelector('#ladder-configs-table-container tbody td')!; + expect(cell).toHaveTextContent('AWS account (aws-external)'); + expect(cell).toHaveAttribute('title', 'acct-1'); + expect(document.querySelector('#ladder-configs-table-container th')).toHaveTextContent('Account'); + }); + + test('table falls back to the stored ID when the account list fails', async () => { + (listAccountsMinimal as jest.Mock).mockRejectedValue(new Error('Forbidden')); + await renderSection([baseConfig({ cloud_account_id: 'acct-1' })]); + expect(document.querySelector('#ladder-configs-table-container tbody td')).toHaveTextContent('acct-1'); + }); + + test('the warning follows the Approval Mode selection', async () => { + await openNew(); + expect(warning()).not.toBeVisible(); + setMode('auto_approve'); + expect(warning()).toBeVisible(); + expect(warning()).toHaveTextContent(/skips human approval/i); + setMode('email_approval'); + expect(warning()).not.toBeVisible(); + }); + + test('editing an Auto Approve config shows the warning immediately', async () => { + await renderSection([baseConfig({ id: 'c', cloud_account_id: 'acct-1', mode: 'auto_approve' })]); + document.querySelector('.ladder-edit-btn')!.click(); + expect(warning()).toBeVisible(); + }); + + test.each([[true, 1], [false, 0]])('turning on Auto Approve asks first (confirmed=%s)', async (confirmed, saves) => { + (confirmDialog as jest.Mock).mockResolvedValue(confirmed); + await openNew(); + setMode('auto_approve'); + (document.getElementById('ladder-cfg-enabled') as HTMLInputElement).checked = true; + await saveLadderConfig(); + expect(confirmDialog).toHaveBeenCalledWith(expect.objectContaining({ title: 'Enable Auto Approve?', destructive: true })); + expect(api.upsertLadderConfig).toHaveBeenCalledTimes(saves); + expect(document.getElementById('ladder-config-modal')!.classList.contains('hidden')).toBe(confirmed); + expect(document.getElementById('ladder-config-save-btn')).not.toBeDisabled(); + }); + + test('no confirmation when Auto Approve is already active or the config stays disabled', async () => { + await openNew(); + setMode('auto_approve'); + await saveLadderConfig(); + await renderSection([baseConfig({ id: 'c', cloud_account_id: 'acct-1', mode: 'auto_approve' })]); + document.querySelector('.ladder-edit-btn')!.click(); + await saveLadderConfig(); + expect(confirmDialog).not.toHaveBeenCalled(); + expect(api.upsertLadderConfig).toHaveBeenCalledTimes(2); + }); + + test('a failed save shows the server error text', async () => { + await openNew(); + (api.upsertLadderConfig as jest.Mock).mockRejectedValue(new Error('ramp_schedule fractions must sum to 1.0')); + await saveLadderConfig(); + expect(mockShowToast).toHaveBeenCalledWith({ message: 'Failed to save ladder config: ramp_schedule fractions must sum to 1.0', kind: 'error' }); + }); + }); }); diff --git a/frontend/src/__tests__/toast.test.ts b/frontend/src/__tests__/toast.test.ts index a69374b2..0acee6f2 100644 --- a/frontend/src/__tests__/toast.test.ts +++ b/frontend/src/__tests__/toast.test.ts @@ -2,7 +2,7 @@ * Tests for the shared toast notification system. */ -import { showToast } from '../toast'; +import { showToast, layoutToastStack } from '../toast'; describe('showToast', () => { beforeEach(() => { @@ -106,3 +106,243 @@ describe('showToast', () => { expect(document.querySelector('.toast--success')?.getAttribute('role')).toBe('status'); }); }); + +// Issue #742: a toast must not sit on top of an open dialog's Save button, +// and must not move onto a dialog it wasn't covering. jsdom has no layout, +// so the dialog, button-row and toast-stack geometry is stubbed. +describe('toast stack and open dialogs', () => { + interface Box { left: number; top: number; right: number; bottom: number } + const rect = (left: number, top: number, width: number, height: number): DOMRect => + ({ left, top, width, height, right: left + width, bottom: top + height, x: left, y: top, toJSON: () => ({}) }) as DOMRect; + const NONE = rect(0, 0, 0, 0); + const overlaps = (a: Box, b: Box): boolean => + a.left < b.right && b.left < a.right && a.top < b.bottom && b.top < a.bottom; + + let vw = 0; + let vh = 0; + // The stack's size at its default corner (right 16px, bottom 16px). + let stackW = 0; + let stackH = 0; + + function setViewport(width: number, height: number): void { + vw = width; + vh = height; + jest.spyOn(document.documentElement, 'clientWidth', 'get').mockImplementation(() => vw); + jest.spyOn(document.documentElement, 'clientHeight', 'get').mockImplementation(() => vh); + } + + // Pre-create the container showToast() reuses, so its rect can be stubbed. + function setStack(width: number, height: number): void { + stackW = width; + stackH = height; + let el = document.getElementById('toast-container'); + if (!el) { + el = document.createElement('div'); + el.id = 'toast-container'; + document.body.appendChild(el); + } + jest.spyOn(el, 'getBoundingClientRect').mockImplementation(() => + rect(vw - 16 - stackW, vh - 16 - stackH, stackW, stackH)); + } + + function openDialog(box: DOMRect, footers: DOMRect[], cls = 'modal-content'): HTMLElement { + const modal = document.createElement('div'); + modal.className = 'modal'; + const rows = footers.map(() => '
').join(''); + modal.innerHTML = `
${rows}
`; + document.body.appendChild(modal); + jest.spyOn(modal.querySelector(`.${cls}`)!, 'getBoundingClientRect').mockReturnValue(box); + modal.querySelectorAll('.modal-buttons').forEach((row, i) => + jest.spyOn(row, 'getBoundingClientRect').mockReturnValue(footers[i]!)); + return modal; + } + + function openConfirm(box: DOMRect, actions: DOMRect): HTMLElement { + const backdrop = document.createElement('div'); + backdrop.className = 'modal-confirm-backdrop'; + backdrop.innerHTML = '

Enable Auto Approve?

' + + '
Ladder purchases will run without approval.
' + + '
'; + document.body.appendChild(backdrop); + jest.spyOn(backdrop.querySelector('.modal-confirm')!, 'getBoundingClientRect').mockReturnValue(box); + jest.spyOn(backdrop.querySelector('.modal-confirm-actions')!, 'getBoundingClientRect').mockReturnValue(actions); + return backdrop; + } + + const container = (): HTMLElement => document.getElementById('toast-container')!; + const prop = (name: string): string => container().style.getPropertyValue(name); + // Where the stack ends up, from the properties layoutToastStack() set. + function stackBox(): Box { + const bottom = parseInt(prop('--toast-bottom') || '16', 10); + const width = Math.min(stackW, parseInt(prop('--toast-max-width') || '400', 10)); + return { left: vw - 16 - width, right: vw - 16, top: vh - bottom - stackH, bottom: vh - bottom }; + } + const nextFrame = (): Promise => new Promise(resolve => requestAnimationFrame(() => resolve())); + + afterEach(() => { + jest.restoreAllMocks(); + document.body.replaceChildren(); + }); + + it('keeps the default corner when no dialog is open', () => { + setViewport(1280, 720); + setStack(400, 60); + showToast({ message: 'saved' }); + expect(prop('--toast-bottom')).toBe(''); + expect(prop('--toast-max-width')).toBe(''); + }); + + it('fits the stack beside a dialog when its corner would cover the button row', () => { + setViewport(1280, 720); + setStack(400, 60); + openDialog(rect(300, 36, 680, 648), [rect(300, 620, 680, 64)]); + showToast({ message: 'Buffer fraction is required.', kind: 'error' }); + // 1280 - 980 - 2 * 16 = 268px beside the dialog; the bottom stays put. + expect(prop('--toast-max-width')).toBe('268px'); + expect(prop('--toast-bottom')).toBe(''); + }); + + it('never widens the stack past its default 400px beside a dialog', () => { + // A 480px confirm at 1920 wide leaves a 688px gap. + setViewport(1920, 1080); + setStack(400, 60); + openConfirm(rect(720, 400, 480, 280), rect(744, 616, 432, 40)); + showToast({ message: 'A long error message that would stretch across the whole gap', kind: 'error' }); + expect(prop('--toast-max-width')).toBe(''); + }); + + it("lifts the stack above the dialog's button row when there is no room beside or above it", () => { + setViewport(390, 844); + setStack(358, 70); + openDialog(rect(10, 40, 370, 760), [rect(10, 736, 370, 64)]); + showToast({ message: 'Failed to save', kind: 'error' }); + // 844 - 736 + 8 = 116px from the bottom: above the footer, not on it. + expect(prop('--toast-bottom')).toBe('116px'); + expect(prop('--toast-max-width')).toBe(''); + }); + + it('lifts the stack above a short bottom-sheet dialog instead of onto its body', () => { + setViewport(390, 844); + setStack(358, 70); + openConfirm(rect(0, 600, 390, 244), rect(24, 780, 342, 40)); + showToast({ message: 'Failed', kind: 'error' }); + // 844 - 600 + 8: the whole sheet stays clear. + expect(prop('--toast-bottom')).toBe('252px'); + expect(overlaps(stackBox(), rect(0, 600, 390, 244))).toBe(false); + }); + + it('keeps the default corner under a centred short dialog with room below it', () => { + // A .modal-wide dialog (800px) at 1280x720 leaves a 208px gap, but its + // buttons sit mid-screen, far above the toast's corner. + setViewport(1280, 720); + setStack(400, 60); + const dialog = rect(240, 170, 800, 380); + openDialog(dialog, [rect(272, 480, 736, 40)]); + showToast({ message: 'Failed to save overrides', kind: 'error' }); + expect(prop('--toast-bottom')).toBe(''); + expect(prop('--toast-max-width')).toBe(''); + expect(overlaps(stackBox(), dialog)).toBe(false); + }); + + it('keeps the default corner when the stack is too narrow to reach the button row', () => { + setViewport(1280, 720); + setStack(200, 60); + // The footer ends at x=980; a 200px stack starts at 1280 - 16 - 200 = 1064. + openDialog(rect(300, 36, 680, 648), [rect(300, 620, 680, 64)]); + showToast({ message: 'Saved', kind: 'success' }); + expect(prop('--toast-bottom')).toBe(''); + expect(prop('--toast-max-width')).toBe(''); + }); + + it('follows only the topmost dialog: a confirm over the ladder modal keeps the stack off its body', () => { + setViewport(1100, 720); + setStack(400, 60); + // The ladder modal's footer is under the toast's corner, but a confirm + // (and its backdrop) now covers that modal. + openDialog(rect(210, 36, 680, 648), [rect(210, 620, 680, 64)]); + const confirmBox = rect(310, 250, 480, 220); + openConfirm(confirmBox, rect(334, 410, 432, 40)); + showToast({ message: 'Max actions per run must be between 1 and 50', kind: 'error' }); + expect(prop('--toast-bottom')).toBe(''); + expect(overlaps(stackBox(), confirmBox)).toBe(false); + }); + + it('moves the stack beside a tall confirm whose buttons it would cover, never onto the confirm', () => { + setViewport(1100, 720); + setStack(400, 60); + openDialog(rect(210, 36, 680, 648), [rect(210, 620, 680, 64)]); + const confirmBox = rect(310, 60, 480, 600); + openConfirm(confirmBox, rect(334, 620, 432, 40)); + showToast({ message: 'x', kind: 'error' }); + expect(prop('--toast-max-width')).toBe('278px'); + expect(overlaps(stackBox(), confirmBox)).toBe(false); + }); + + it('uses the visible button row, not the last one in the DOM', () => { + // Profile modal shape: a Save Changes row, then a hidden MFA row. + setViewport(390, 844); + setStack(358, 70); + openDialog(rect(10, 40, 370, 760), [rect(10, 736, 370, 64), NONE]); + showToast({ message: 'Failed to save profile', kind: 'error' }); + expect(prop('--toast-bottom')).toBe('116px'); + }); + + it('re-lays out when a button row scrolls into view under the stack', async () => { + setViewport(1100, 720); + setStack(400, 60); + const modal = openDialog(rect(210, 36, 680, 648), [rect(210, 760, 680, 64)]); + showToast({ message: 'Failed', kind: 'error' }); + // The row is below the fold of the scrolling dialog: nothing to avoid yet. + await nextFrame(); + expect(prop('--toast-bottom')).toBe(''); + + jest.spyOn(modal.querySelector('.modal-buttons')!, 'getBoundingClientRect').mockReturnValue(rect(210, 600, 680, 64)); + modal.querySelector('.modal-body')!.dispatchEvent(new Event('scroll')); + await nextFrame(); + expect(prop('--toast-bottom')).toBe(`${720 - 600 + 8}px`); + }); + + it('re-lays out when a dialog opens or closes under a showing toast', async () => { + setViewport(390, 844); + setStack(358, 70); + showToast({ message: 'saved', kind: 'success' }); + expect(prop('--toast-bottom')).toBe(''); + + const modal = openDialog(rect(10, 40, 370, 760), [rect(10, 736, 370, 64)]); + await nextFrame(); + expect(prop('--toast-bottom')).toBe('116px'); + + jest.spyOn(modal.querySelector('.modal-content')!, 'getBoundingClientRect').mockReturnValue(NONE); + modal.classList.add('hidden'); + await nextFrame(); + expect(prop('--toast-bottom')).toBe(''); + }); + + it('ignores dialogs that are not rendered', () => { + setViewport(390, 844); + setStack(358, 70); + openDialog(NONE, [NONE]); + showToast({ message: 'saved' }); + layoutToastStack(); + expect(prop('--toast-bottom')).toBe(''); + }); + + it('stops watching for dialogs and scrolling once the last toast is gone', () => { + jest.useFakeTimers(); + try { + const disconnect = jest.spyOn(MutationObserver.prototype, 'disconnect'); + const removed = jest.spyOn(document, 'removeEventListener'); + const a = showToast({ message: 'a', timeout: null }); + const b = showToast({ message: 'b', timeout: null }); + a.dismiss(); + jest.advanceTimersByTime(200); + expect(disconnect).not.toHaveBeenCalled(); + b.dismiss(); + jest.advanceTimersByTime(200); + expect(disconnect).toHaveBeenCalled(); + expect(removed).toHaveBeenCalledWith('scroll', expect.any(Function), { capture: true }); + } finally { + jest.useRealTimers(); + } + }); +}); diff --git a/frontend/src/ladder-config-modal.ts b/frontend/src/ladder-config-modal.ts new file mode 100644 index 00000000..e7baea5c --- /dev/null +++ b/frontend/src/ladder-config-modal.ts @@ -0,0 +1,149 @@ +/** + * Markup for the per-account Commitment Laddering config modal (Add/Edit + * Account Config). Kept apart from ladder.ts so that module stays focused on + * loading, wiring and saving. Field IDs are the contract with ladder.ts and + * ladder-account-selector.ts. + * + * Layout (styles/ladder-form.css): labels sit above their inputs, the short + * fields pair up in a two-column grid that collapses to one column on narrow + * screens, and range/unit qualifiers live in a muted hint under each input. + * The header stays put while the body scrolls on short viewports. + */ + +import { iconSvg } from './icons'; + +export function renderLadderConfigModal(): string { + return ` + + +`; +} diff --git a/frontend/src/ladder-config-table.ts b/frontend/src/ladder-config-table.ts new file mode 100644 index 00000000..6068b3cb --- /dev/null +++ b/frontend/src/ladder-config-table.ts @@ -0,0 +1,77 @@ +/** + * Per-account ladder configuration table (issue #742). + * + * Shows each configuration's registered account name and external ID instead + * of the internal account UUID. The UUID stays available as the cell's title + * so support can still match it against API payloads. When the account list + * is unavailable (still loading, forbidden, or the account was removed) the + * cell falls back to the stored ID so the row is never blank. + */ + +import type { AccountSummary } from './api/accounts'; +import type { LadderConfig } from './api/ladder'; +import { escapeHtml, formatDate } from './utils'; + +export interface LadderConfigTableOptions { + accounts: readonly AccountSummary[]; + canEdit: boolean; + onEdit: (config: LadderConfig) => void; +} + +export function ladderAccountLabel(config: LadderConfig, accounts: readonly AccountSummary[]): string { + const account = accounts.find(item => item.id === config.cloud_account_id && item.provider === config.provider); + return account ? `${account.name} (${account.external_id})` : config.cloud_account_id; +} + +export function renderLadderConfigTable(container: HTMLElement, configs: readonly LadderConfig[], options: LadderConfigTableOptions): void { + if (configs.length === 0) { + container.innerHTML = '

No per-account configurations yet. Click Add Account Config to create one.

'; + return; + } + + const rows = configs.map(cfg => ` + + ${escapeHtml(ladderAccountLabel(cfg, options.accounts))} + ${escapeHtml(cfg.provider.toUpperCase())} + ${cfg.enabled ? 'Yes' : 'No'} + ${escapeHtml(cfg.mode === 'email_approval' ? 'Email Approval' : 'Auto Approve')} + ${escapeHtml(cfg.cadence === 'daily' ? 'Daily' : 'Weekly')} + ${typeof cfg.target_coverage === 'number' ? `${cfg.target_coverage.toFixed(1)}%` : 'N/A'} + ${cfg.updated_at ? escapeHtml(formatDate(cfg.updated_at)) : 'N/A'} + ${options.canEdit + ? `` + : ''} + + `).join(''); + + container.innerHTML = ` + + + + + + + + + + + + + + ${rows} +
AccountProviderEnabledModeCadenceTarget CoverageUpdated
+ `; + + // Wire edit buttons via querySelectorAll so no user data is ever injected + // into a JS string context (data-* attributes are HTML-attribute-context only). + container.querySelectorAll('.ladder-edit-btn').forEach(btn => { + btn.addEventListener('click', () => { + const accountID = btn.dataset['accountId'] ?? ''; + const provider = btn.dataset['provider'] ?? ''; + const cfg = configs.find(c => c.cloud_account_id === accountID && c.provider === provider); + if (cfg) options.onEdit(cfg); + }); + }); +} diff --git a/frontend/src/ladder.ts b/frontend/src/ladder.ts index 51f5e632..f765c701 100644 --- a/frontend/src/ladder.ts +++ b/frontend/src/ladder.ts @@ -14,13 +14,15 @@ */ import * as api from './api'; -import { escapeHtml, formatDate } from './utils'; import { showToast } from './toast'; -import { iconSvg } from './icons'; +import { confirmDialog } from './confirmDialog'; import { openModal, closeModal } from './modal'; import { canAccess } from './permissions'; import { initLadderTimeline } from './ladder-timeline'; import { initLadderAccountSelector } from './ladder-account-selector'; +import { renderLadderConfigTable } from './ladder-config-table'; +import { renderLadderConfigModal } from './ladder-config-modal'; +import { listAccountsMinimal, type AccountSummary } from './api/accounts'; import { readLadderNumbers } from './ladder-form-numbers'; // ========================================== @@ -35,6 +37,9 @@ let configModalSaveInFlight = false; // Cached configs from the last successful load. let cachedConfigs: api.LadderConfig[] = []; +// Registered accounts used to label table rows; empty when the list could not +// be loaded, in which case rows fall back to the stored account ID. +let tableAccounts: AccountSummary[] = []; let selectedAccount: ReturnType | undefined; // ========================================== @@ -62,7 +67,12 @@ export async function initLadderingSettings(globalEnabled: boolean): Promise []), + ]); + cachedConfigs = configs; + tableAccounts = accounts; renderConfigTable(cachedConfigs); const timeline = document.getElementById('ladder-purchase-timeline'); if (timeline) await initLadderTimeline(timeline, cachedConfigs, globalEnabled); @@ -131,122 +141,7 @@ function renderLadderingSection(globalEnabled: boolean): string {
- - +${renderLadderConfigModal()} `; } @@ -255,57 +150,10 @@ function renderLadderingSection(globalEnabled: boolean): string { export function renderConfigTable(configs: api.LadderConfig[]): void { const container = document.getElementById('ladder-configs-table-container'); if (!container) return; - - if (configs.length === 0) { - container.innerHTML = '

No per-account configurations yet. Click Add Account Config to create one.

'; - return; - } - - const canEdit = canAccess('update', 'config'); - const rows = configs.map(cfg => ` - - ${escapeHtml(cfg.cloud_account_id)} - ${escapeHtml(cfg.provider.toUpperCase())} - ${cfg.enabled ? 'Yes' : 'No'} - ${escapeHtml(cfg.mode === 'email_approval' ? 'Email Approval' : 'Auto Approve')} - ${escapeHtml(cfg.cadence === 'daily' ? 'Daily' : 'Weekly')} - ${typeof cfg.target_coverage === 'number' ? `${cfg.target_coverage.toFixed(1)}%` : 'N/A'} - ${cfg.updated_at ? escapeHtml(formatDate(cfg.updated_at)) : 'N/A'} - ${canEdit - ? `` - : ''} - - `).join(''); - - container.innerHTML = ` - - - - - - - - - - - - - - ${rows} -
Account IDProviderEnabledModeCadenceTarget CoverageUpdated
- `; - - // Wire edit buttons via querySelectorAll so no user data is ever injected - // into a JS string context (data-* attributes are HTML-attribute-context only). - container.querySelectorAll('.ladder-edit-btn').forEach(btn => { - btn.addEventListener('click', () => { - const accountID = btn.dataset['accountId'] ?? ''; - const provider = btn.dataset['provider'] ?? ''; - const cfg = cachedConfigs.find(c => c.cloud_account_id === accountID && c.provider === provider); - if (cfg) openLadderConfigModal(cfg); - }); + renderLadderConfigTable(container, configs, { + accounts: tableAccounts, + canEdit: canAccess('update', 'config'), + onEdit: cfg => openLadderConfigModal(cfg), }); } @@ -383,6 +231,7 @@ function openLadderConfigModal(existing?: api.LadderConfig): void { // Cancel is inside the cloned form; the outside Close button keeps its listener. newForm.querySelector('#ladder-modal-cancel-btn') ?.addEventListener('click', closeLadderModal); + newForm.querySelector('#ladder-cfg-mode')?.addEventListener('change', syncAutoApproveWarning); } // Populate modal fields. @@ -390,6 +239,7 @@ function openLadderConfigModal(existing?: api.LadderConfig): void { setSelectValue('ladder-cfg-provider', existing?.provider ?? 'aws'); setChecked('ladder-cfg-enabled', existing?.enabled ?? false); setSelectValue('ladder-cfg-mode', existing?.mode ?? 'email_approval'); + syncAutoApproveWarning(); setSelectValue('ladder-cfg-cadence', existing?.cadence ?? 'daily'); setValue('ladder-cfg-target-coverage', String(existing?.target_coverage ?? 100)); setValue('ladder-cfg-buffer-fraction', String(existing?.buffer_fraction ?? 0.10)); @@ -458,6 +308,7 @@ export async function saveLadderConfig(): Promise { if (saveBtn) saveBtn.disabled = true; try { + if (activatesAutoApprove(cfg) && !(await confirmAutoApprove())) return; const saved = await api.upsertLadderConfig(cfg); // Update the in-memory cache and re-render the table. @@ -474,7 +325,10 @@ export async function saveLadderConfig(): Promise { closeLadderModal(); showToast({ message: 'Ladder config saved.', kind: 'success' }); } catch (err) { - showToast({ message: 'Failed to save ladder config. Check inputs and try again.', kind: 'error' }); + // Surface the server's validation text (apiRequest copies the response's + // `error` field into err.message) instead of a generic failure. + const detail = err instanceof Error && err.message ? err.message : 'Check inputs and try again.'; + showToast({ message: `Failed to save ladder config: ${detail}`, kind: 'error' }); console.error('saveLadderConfig:', err); } finally { configModalSaveInFlight = false; @@ -482,6 +336,33 @@ export async function saveLadderConfig(): Promise { } } +// ========================================== +// AUTO-APPROVE SAFEGUARDS +// ========================================== + +function syncAutoApproveWarning(): void { + const mode = document.getElementById('ladder-cfg-mode') as HTMLSelectElement | null; + const warning = document.getElementById('ladder-cfg-mode-warning'); + if (warning) warning.hidden = mode?.value !== 'auto_approve'; +} + +// True when this save turns on unattended purchasing for the account: the +// saved config would be enabled with Auto Approve and the stored one was not. +function activatesAutoApprove(cfg: api.LadderConfig): boolean { + const unattended = (c?: api.LadderConfig): boolean => !!c && c.enabled && c.mode === 'auto_approve'; + const previous = cachedConfigs.find(c => c.cloud_account_id === cfg.cloud_account_id && c.provider === cfg.provider); + return unattended(cfg) && !unattended(previous); +} + +function confirmAutoApprove(): Promise { + return confirmDialog({ + title: 'Enable Auto Approve?', + body: 'Ladder purchases for this account will run without human approval. Commitments are long-term and usually cannot be cancelled once bought. Check the target coverage and per-run caps before continuing.', + confirmLabel: 'Enable Auto Approve', + destructive: true, + }); +} + // ========================================== // DOM HELPERS // ========================================== diff --git a/frontend/src/styles/components.css b/frontend/src/styles/components.css index 8e6ebee7..4915027f 100644 --- a/frontend/src/styles/components.css +++ b/frontend/src/styles/components.css @@ -347,18 +347,25 @@ button.secondary:hover { /* Toast notifications (bottom-right stack, AutoSpotting-style). * Rendered by toast.ts. Default timeout 30s. Kind classes set the - * left-border + icon colour. + * left-border + icon colour. When the default corner would cover a + * visible button row of the topmost open dialog, toast.ts overrides + * --toast-max-width (the stack fits beside the dialog) or --toast-bottom + * (the stack is lifted above the dialog or that row) on the element, and + * re-checks on open/close, scroll and resize (issue #742). Otherwise these + * defaults apply. */ .toast-container { + --toast-bottom: 1rem; + --toast-max-width: 400px; position: fixed; - bottom: 1rem; + bottom: var(--toast-bottom); right: 1rem; display: flex; flex-direction: column; gap: 0.5rem; z-index: 2100; pointer-events: none; - max-width: 400px; + max-width: var(--toast-max-width); } .toast { diff --git a/frontend/src/styles/index.css b/frontend/src/styles/index.css index 601e4de8..a5356659 100644 --- a/frontend/src/styles/index.css +++ b/frontend/src/styles/index.css @@ -14,4 +14,5 @@ @import './icons.css'; @import './plans.css'; @import './settings.css'; +@import './ladder-form.css'; @import './responsive.css'; diff --git a/frontend/src/styles/ladder-form.css b/frontend/src/styles/ladder-form.css new file mode 100644 index 00000000..f968eaa6 --- /dev/null +++ b/frontend/src/styles/ladder-form.css @@ -0,0 +1,149 @@ +/* Commitment Laddering per-account config modal (issue #742 review). + * Labels stack above their inputs, the short fields pair up in a two-column + * grid, and qualifiers such as ranges live in a muted hint under the input. + * The header stays fixed and the body scrolls on short viewports, with the + * Cancel / Save footer pinned to the bottom of the scroll area. */ + +.ladder-config-dialog { + width: 680px; + max-width: 95vw; + max-height: 90vh; + padding: 0; + display: flex; + flex-direction: column; + overflow: hidden; +} + +.ladder-config-dialog .modal-header { + flex: none; + display: flex; + align-items: center; + justify-content: space-between; + gap: var(--cudly-sp-4); + padding: var(--cudly-sp-4) var(--cudly-sp-5); + border-bottom: 1px solid var(--cudly-border); +} + +.ladder-config-dialog .modal-header h3 { + margin: 0; + font-size: var(--cudly-fs-lg); + color: var(--cudly-text); +} + +.ladder-config-dialog .modal-body { + flex: 1 1 auto; + min-height: 0; + overflow-y: auto; + padding: var(--cudly-sp-5) var(--cudly-sp-5) 0; +} + +.ladder-form-grid { + display: grid; + grid-template-columns: repeat(2, minmax(0, 1fr)); + gap: 0.875rem var(--cudly-sp-5); + align-items: start; +} + +.ladder-field { + display: flex; + flex-direction: column; + gap: 0.3rem; + min-width: 0; +} + +.ladder-field-full { + grid-column: 1 / -1; +} + +.ladder-config-dialog .ladder-field > label { + margin: 0; + font-size: var(--cudly-fs-sm); + font-weight: var(--cudly-fw-medium); +} + +.ladder-config-dialog .ladder-field input:not([type="checkbox"]), +.ladder-config-dialog .ladder-field select, +.ladder-config-dialog .ladder-field textarea { + width: 100%; + min-height: 2.25rem; + margin: 0; +} + +.ladder-config-dialog .ladder-field textarea { + min-height: 6rem; + font-family: 'Monaco', 'Menlo', 'Ubuntu Mono', monospace; + font-size: 0.85rem; +} + +.ladder-config-dialog .form-hint { + font-size: var(--cudly-fs-xs); + line-height: 1.4; + color: var(--cudly-text-muted); +} + +.ladder-config-dialog .form-hint:empty { + display: none; +} + +.ladder-config-dialog .form-hint code { + font-size: 0.7rem; + word-break: break-all; +} + +/* Sits beside Approval Mode, centred on the select's line. */ +.ladder-field-check { + align-self: end; + min-height: 2.25rem; + justify-content: center; +} + +.ladder-config-dialog .form-checkbox-label { + display: flex; + align-items: center; + gap: var(--cudly-sp-2); + margin: 0; + font-size: var(--cudly-fs-sm); + cursor: pointer; +} + +.ladder-config-dialog .warning-message { + margin: 0; + padding: var(--cudly-sp-3) var(--cudly-sp-4); + font-size: var(--cudly-fs-sm); +} + +.ladder-config-dialog .ladder-form-section { + grid-column: 1 / -1; + margin: var(--cudly-sp-1) 0 0; + padding-top: var(--cudly-sp-4); + border-top: 1px solid var(--cudly-border); + font-size: var(--cudly-fs-xs); + font-weight: var(--cudly-fw-semibold); + letter-spacing: 0.04em; + text-transform: uppercase; + color: var(--cudly-text-muted); +} + +.ladder-config-dialog .modal-footer { + position: sticky; + bottom: 0; + z-index: 1; + display: flex; + justify-content: flex-end; + gap: var(--cudly-sp-3); + margin: 0 calc(-1 * var(--cudly-sp-5)); + padding: var(--cudly-sp-4) var(--cudly-sp-5); + background: var(--cudly-surface); + border-top: 1px solid var(--cudly-border); +} + +@media (max-width: 640px) { + .ladder-form-grid { + grid-template-columns: minmax(0, 1fr); + } + + .ladder-field-check { + align-self: auto; + min-height: 0; + } +} diff --git a/frontend/src/toast.ts b/frontend/src/toast.ts index 0469f934..eeb6e3ff 100644 --- a/frontend/src/toast.ts +++ b/frontend/src/toast.ts @@ -17,6 +17,16 @@ * focus. The dismiss() return value lets callers clear a toast * programmatically (e.g. tie an "uploading..." toast to the completion * of the upload). + * + * Open dialogs (issue #742): a toast stays up for 30s, and the default + * bottom-right corner is where a dialog's Cancel / Save row sits on a + * laptop-sized viewport, so an error toast used to swallow the click on + * Save until it was dismissed. layoutToastStack() moves the stack only when + * its default corner would cover a visible button row of the topmost open + * dialog: beside the dialog when the gap to its right is wide enough for a + * readable toast, otherwise above the dialog or its button row. The + * positions are CSS custom properties set through the CSSOM (CSP forbids + * style attributes in markup). */ import { createIcon, type IconName } from './icons'; @@ -50,6 +60,154 @@ const KIND_ICONS: Record = { error: 'x-circle', }; +// Dialog boxes the stack must not cover, and the button rows inside them +// that must stay clickable. +const DIALOG_BOX_SELECTOR = '.modal-content, .modal-confirm'; +const DIALOG_FOOTER_SELECTOR = '.modal-footer, .modal-buttons, .modal-confirm-actions'; +// Matches the container's 1rem inset and 400px max-width in components.css. +const EDGE_PX = 16; +const DEFAULT_MAX_WIDTH_PX = 400; +const FOOTER_GAP_PX = 8; +// Narrower than this, a toast beside the dialog is too cramped to read, so +// the stack is lifted above the dialog (or its button row) instead. +const MIN_SIDE_WIDTH_PX = 240; + +interface Box { left: number; top: number; right: number; bottom: number } + +function setVar(el: HTMLElement, name: string, value: string | null): void { + if (value === null) el.style.removeProperty(name); + else if (el.style.getPropertyValue(name) !== value) el.style.setProperty(name, value); +} + +function visibleRect(el: Element): DOMRect | null { + const rect = el.getBoundingClientRect(); + return rect.width > 0 && rect.height > 0 ? rect : null; +} + +function intersect(a: Box, b: Box): Box | null { + const box = { + left: Math.max(a.left, b.left), + top: Math.max(a.top, b.top), + right: Math.min(a.right, b.right), + bottom: Math.min(a.bottom, b.bottom), + }; + return box.right > box.left && box.bottom > box.top ? box : null; +} + +/** + * The dialog the user is looking at: a confirm dialog (z-index 2000) sits + * over any .modal (1000), and among equals the later one in the DOM paints + * on top. Dialogs underneath are behind a backdrop and can't be clicked. + */ +function topmostDialog(): { el: Element; rect: DOMRect } | null { + let top: { el: Element; rect: DOMRect; rank: number } | null = null; + for (const el of Array.from(document.querySelectorAll(DIALOG_BOX_SELECTOR))) { + const rect = visibleRect(el); + const rank = el.classList.contains('modal-confirm') ? 1 : 0; + if (rect && (!top || rank >= top.rank)) top = { el, rect, rank }; + } + return top; +} + +/** + * Keep the toast stack off the topmost open dialog's button rows. The stack + * stays in its default corner unless that corner overlaps a button row that + * is actually on screen. Then it moves, in order of preference: beside the + * dialog (when the gap is at least 240px), above the whole dialog (when + * there is room), or just above the button row it would cover. The last + * case can put the stack over the lower part of a dialog that fills the + * screen; the buttons stay clickable and the toast can be dismissed. + * Exported for tests; runs on every new toast and, while toasts are + * showing, whenever a dialog opens or closes, something scrolls, or the + * viewport is resized. + */ +export function layoutToastStack(): void { + const container = document.getElementById(CONTAINER_ID); + if (!container) return; + // Measure the stack where it sits by default; the rect is its real size. + setVar(container, '--toast-max-width', null); + setVar(container, '--toast-bottom', null); + const stack = visibleRect(container); + const dialog = topmostDialog(); + if (!stack || !dialog) return; + + const vw = document.documentElement.clientWidth || window.innerWidth; + const vh = document.documentElement.clientHeight || window.innerHeight; + const onScreen = intersect(dialog.rect, { left: 0, top: 0, right: vw, bottom: vh }); + const rows: Box[] = []; + dialog.el.querySelectorAll(DIALOG_FOOTER_SELECTOR).forEach((el) => { + const rect = visibleRect(el); + const shown = rect && onScreen ? intersect(rect, onScreen) : null; + if (shown) rows.push(shown); + }); + const width = Math.min(stack.width, DEFAULT_MAX_WIDTH_PX); + const boxAt = (bottom: number): Box => + ({ left: vw - EDGE_PX - width, right: vw - EDGE_PX, top: vh - bottom - stack.height, bottom: vh - bottom }); + const covered = (bottom: number): Box | undefined => + rows.find(row => intersect(boxAt(bottom), row) !== null); + + let row = covered(EDGE_PX); + if (!row) return; + + const side = Math.floor(vw - dialog.rect.right - 2 * EDGE_PX); + if (side >= MIN_SIDE_WIDTH_PX) { + setVar(container, '--toast-max-width', `${Math.min(side, DEFAULT_MAX_WIDTH_PX)}px`); + return; + } + const aboveDialog = Math.ceil(vh - dialog.rect.top + FOOTER_GAP_PX); + if (dialog.rect.top - FOOTER_GAP_PX - stack.height >= EDGE_PX) { + setVar(container, '--toast-bottom', `${aboveDialog}px`); + return; + } + // Step up past each button row the stack would cover, lowest first. + let bottom = EDGE_PX; + for (let i = 0; row && i <= rows.length; i++) { + bottom = Math.ceil(vh - row.top + FOOTER_GAP_PX); + row = covered(bottom); + } + setVar(container, '--toast-bottom', `${bottom}px`); +} + +// While toasts are showing, re-run the layout when a dialog opens or closes +// (class / hidden toggles, or a confirm dialog added to ), anything +// scrolls (a dialog body brings its buttons into view), or the viewport +// changes. Nothing is watched once the last toast is gone. +let watcher: MutationObserver | null = null; +let layoutQueued = false; + +function queueLayout(): void { + if (layoutQueued) return; + layoutQueued = true; + requestAnimationFrame(() => { + layoutQueued = false; + const container = document.getElementById(CONTAINER_ID); + if (container?.querySelector('.toast')) layoutToastStack(); + else stopWatchingIfEmpty(container); + }); +} + +function startWatching(): void { + if (watcher) return; + watcher = new MutationObserver(queueLayout); + watcher.observe(document.body, { + subtree: true, + childList: true, + attributes: true, + attributeFilter: ['class', 'hidden'], + }); + window.addEventListener('resize', queueLayout); + // scroll doesn't bubble; capture sees scrolling inside any element. + document.addEventListener('scroll', queueLayout, { capture: true, passive: true }); +} + +function stopWatchingIfEmpty(container: HTMLElement | null): void { + if (container?.querySelector('.toast')) return; + watcher?.disconnect(); + watcher = null; + window.removeEventListener('resize', queueLayout); + document.removeEventListener('scroll', queueLayout, { capture: true }); +} + function ensureContainer(): HTMLElement { let container = document.getElementById(CONTAINER_ID); if (!container) { @@ -132,7 +290,10 @@ export function showToast(opts: ToastOptions): ToastHandle { // Allow CSS transition to play out; fall back to immediate remove // if the element has no transition (tests run without computed // transitions). - const remove = (): void => toast.remove(); + const remove = (): void => { + toast.remove(); + stopWatchingIfEmpty(container); + }; toast.addEventListener('transitionend', remove, { once: true }); setTimeout(remove, 200); }, @@ -147,6 +308,8 @@ export function showToast(opts: ToastOptions): ToastHandle { toast.addEventListener('keydown', onKey); container.appendChild(toast); + layoutToastStack(); + startWatching(); if (timeout !== null) { timeoutId = setTimeout(() => handle.dismiss(), timeout); diff --git a/frontend/tests-e2e/ladder-account-selector.spec.ts b/frontend/tests-e2e/ladder-account-selector.spec.ts index 83d6e434..59ab35dc 100644 --- a/frontend/tests-e2e/ladder-account-selector.spec.ts +++ b/frontend/tests-e2e/ladder-account-selector.spec.ts @@ -1,4 +1,4 @@ -import { test, expect, type Page, type Route } from '@playwright/test'; +import { test, expect, type Locator, type Page, type Route } from '@playwright/test'; import { mockApi, seedAuth } from './fixtures/recs'; import type { LadderConfig } from '../src/api/ladder'; @@ -15,6 +15,18 @@ const existing: LadderConfig = { ramp_schedule: { steps: [{ after_days: 0, fraction: 1 }] }, }; +type Box = { x: number; y: number; width: number; height: number }; +const overlaps = (t: Box, s: Box): boolean => + t.x < s.x + s.width && s.x < t.x + t.width && t.y < s.y + s.height && s.y < t.y + t.height; + +// Fails when the toast's box overlaps Save's; Playwright's click() would +// otherwise only time out on the intercepted pointer event. +async function expectSaveClear(page: Page, toast: Locator): Promise { + const t = (await toast.boundingBox())!; + const s = (await page.locator('#ladder-config-save-btn').boundingBox())!; + expect(overlaps(t, s), `toast ${JSON.stringify(t)} covers Save ${JSON.stringify(s)}`).toBe(false); +} + async function setup(page: Page): Promise<{ saves: LadderConfig[]; writes: string[]; errors: string[]; failures: string[] }> { const saves: LadderConfig[] = []; const writes: string[] = []; @@ -132,6 +144,78 @@ test('provider changes during loading and stale reopened responses keep the curr expect(failures).toEqual([]); }); +test('table names the account, Auto Approve warns and confirms, and save errors keep the server text', async ({ page }) => { + const { saves, errors } = await setup(page); + await page.route('**/api/accounts/list', route => route.fulfill({ json: [...accounts, { id: 'unavailable-saved-account', name: 'Azure saved', external_id: 'sub-saved', provider: 'azure' }] })); + await page.reload(); + const firstCell = page.locator('#ladder-configs-table-container tbody td').first(); + await expect(firstCell).toHaveText('Azure saved (sub-saved)'); + await expect(firstCell).toHaveAttribute('title', 'unavailable-saved-account'); + + await page.locator('#ladder-add-config-btn').click(); + const warning = page.locator('#ladder-cfg-mode-warning'); + await expect(warning).toBeHidden(); + await page.locator('#ladder-cfg-account').selectOption('registered-aws'); + await page.locator('#ladder-cfg-mode').selectOption('auto_approve'); + await expect(warning).toBeVisible(); + await expect(warning).toContainText('skips human approval'); + await page.locator('#ladder-cfg-enabled').check(); + await page.locator('#ladder-config-save-btn').click(); + const confirm = page.getByRole('dialog', { name: 'Enable Auto Approve?' }); + await expect(confirm).toBeVisible(); + await confirm.getByRole('button', { name: 'Cancel' }).click(); + await expect(confirm).toBeHidden(); + await expect(page.locator('#ladder-config-modal')).toBeVisible(); + expect(saves).toEqual([]); + + await page.route('**/api/ladder/configs', route => route.fulfill({ status: 400, json: { error: 'max_actions_per_run must be between 1 and 50' } })); + await page.locator('#ladder-config-save-btn').click(); + await confirm.getByRole('button', { name: 'Enable Auto Approve' }).click(); + await expect(page.getByText('Failed to save ladder config: max_actions_per_run must be between 1 and 50')).toBeVisible(); + await expect(page.locator('#ladder-config-modal')).toBeVisible(); + // The rejected PUT logs its 400 and the handler's console.error; nothing else may fail. + expect(errors.filter(error => !error.includes('400') && !error.startsWith('saveLadderConfig:'))).toEqual([]); +}); + +test('config form uses compact aligned inputs and scrolls inside the modal on short viewports', async ({ page }) => { + await page.setViewportSize({ width: 1440, height: 900 }); + await setup(page); + await page.locator('#ladder-add-config-btn').click(); + const box = async (id: string) => (await page.locator(`#ladder-cfg-${id}`).boundingBox())!; + const coverage = await box('target-coverage'); + const fraction = await box('buffer-fraction'); + for (const id of ['target-coverage', 'buffer-fraction', 'lookback-days', 'max-hourly', 'max-actions', 'cadence']) { + expect((await box(id)).height).toBeLessThan(44); + } + // Numeric fields pair up in two aligned columns. + expect(Math.abs(fraction.y - coverage.y)).toBeLessThan(1); + expect(fraction.x).toBeGreaterThan(coverage.x + coverage.width); + for (const id of ['baseline-percentile', 'max-hourly']) expect((await box(id)).x).toBeCloseTo(coverage.x, 0); + for (const id of ['buf-util-threshold', 'max-actions']) expect((await box(id)).x).toBeCloseTo(fraction.x, 0); + // The ramp textarea spans both columns, with its help text underneath. + const ramp = await box('ramp-schedule'); + expect(ramp.x).toBeCloseTo(coverage.x, 0); + expect(ramp.x + ramp.width).toBeCloseTo(fraction.x + fraction.width, 0); + const rampHint = (await page.locator('#ladder-cfg-ramp-schedule-hint').boundingBox())!; + expect(rampHint.y).toBeGreaterThanOrEqual(ramp.y + ramp.height); + await expect(page.locator('#ladder-cfg-lookback-days')).toHaveAccessibleDescription('Minimum 10'); + + // Short viewport: header and Save stay on screen while the body scrolls. + await page.setViewportSize({ width: 1280, height: 720 }); + const body = page.locator('#ladder-config-modal .modal-body'); + expect(await body.evaluate(el => el.scrollHeight > el.clientHeight)).toBe(true); + await page.locator('#ladder-cfg-ramp-schedule').scrollIntoViewIfNeeded(); + for (const selector of ['#ladder-modal-title', '#ladder-config-save-btn']) { + const rect = (await page.locator(selector).boundingBox())!; + expect(rect.y).toBeGreaterThanOrEqual(0); + expect(rect.y + rect.height).toBeLessThanOrEqual(720); + } + + // Narrow viewport: one column. + await page.setViewportSize({ width: 390, height: 844 }); + expect((await box('buffer-fraction')).x).toBeCloseTo((await box('target-coverage')).x, 0); +}); + test('a cleared Buffer Fraction blocks the save instead of sending a zero buffer (#134)', async ({ page }) => { const { saves, writes, errors } = await setup(page); await page.locator('#ladder-add-config-btn').click(); @@ -144,6 +228,11 @@ test('a cleared Buffer Fraction blocks the save instead of sending a zero buffer await expect(page.locator('#ladder-cfg-buffer-fraction')).toBeFocused(); expect(saves).toEqual([]); expect(writes).toEqual([]); + // The error toast stays up for 30s; it must sit beside the modal, not on + // Save, so the corrected form saves without dismissing it first (#742). + const toast = page.getByRole('alert').filter({ hasText: 'Buffer fraction is required.' }); + await expect(toast).toBeVisible(); + await expectSaveClear(page, toast); await page.locator('#ladder-cfg-buffer-fraction').fill('0'); await page.locator('#ladder-config-save-btn').click(); @@ -152,3 +241,62 @@ test('a cleared Buffer Fraction blocks the save instead of sending a zero buffer expect(saves[0]!.buffer_fraction).toBe(0); expect(errors).toEqual([]); }); + +for (const [width, height] of [[1280, 720], [390, 844]]) { + test(`Save stays clickable while a save-error toast is showing at ${width}x${height} (#742)`, async ({ page }) => { + await page.setViewportSize({ width, height }); + const { errors } = await setup(page); + let puts = 0; + await page.route('**/api/ladder/configs', route => { + puts += 1; + return route.fulfill({ status: 400, json: { error: 'max_actions_per_run must be between 1 and 50' } }); + }); + await page.locator('#ladder-add-config-btn').click(); + await expect(page.locator('#ladder-cfg-account')).toBeEnabled(); + await page.locator('#ladder-cfg-account').selectOption('registered-aws'); + await page.locator('#ladder-config-save-btn').click(); + const toast = page.getByRole('alert').filter({ hasText: 'Failed to save ladder config: max_actions_per_run must be between 1 and 50' }); + await expect(toast).toBeVisible(); + await expect.poll(() => puts).toBe(1); + + await expectSaveClear(page, toast); + // Retry with the toast still up: the click reaches Save and sends a second PUT. + await page.locator('#ladder-config-save-btn').click({ timeout: 5000 }); + await expect.poll(() => puts).toBe(2); + await expect(toast.first()).toBeVisible(); + await expect(page.locator('#ladder-config-modal')).toBeVisible(); + expect(errors.filter(error => !error.includes('400') && !error.startsWith('saveLadderConfig:'))).toEqual([]); + }); +} + +// Only the topmost dialog counts: when the Auto Approve confirm opens over the +// ladder modal while a validation toast is still up, the toast must stay off +// the confirm, including the warning text in its body (#742). +test('a toast stays off the Auto Approve confirm stacked over the ladder modal at 1100x720', async ({ page }) => { + await page.setViewportSize({ width: 1100, height: 720 }); + const { saves, errors } = await setup(page); + await page.locator('#ladder-add-config-btn').click(); + await expect(page.locator('#ladder-cfg-account')).toBeEnabled(); + await page.locator('#ladder-cfg-account').selectOption('registered-aws'); + await page.locator('#ladder-cfg-buffer-fraction').fill(''); + await page.locator('#ladder-config-save-btn').click(); + const toast = page.getByRole('alert').filter({ hasText: 'Buffer fraction is required.' }); + await expect(toast).toBeVisible(); + + await page.locator('#ladder-cfg-buffer-fraction').fill('0.1'); + await page.locator('#ladder-cfg-mode').selectOption('auto_approve'); + await page.locator('#ladder-cfg-enabled').check(); + await page.locator('#ladder-config-save-btn').click(); + const confirm = page.getByRole('dialog', { name: 'Enable Auto Approve?' }); + await expect(confirm).toBeVisible(); + await expect(toast).toBeVisible(); + // The layout re-runs on the next frame after the confirm is added. + await expect.poll(async () => overlaps((await toast.boundingBox())!, (await confirm.locator('.modal-confirm').boundingBox())!)).toBe(false); + expect(overlaps((await toast.boundingBox())!, (await confirm.locator('.modal-confirm-body').boundingBox())!)).toBe(false); + + await confirm.getByRole('button', { name: 'Cancel' }).click(); + await expect(confirm).toHaveCount(0); + await expectSaveClear(page, toast); + expect(saves).toEqual([]); + expect(errors).toEqual([]); +}); diff --git a/frontend/tests-e2e/toast-dialog-placement.spec.ts b/frontend/tests-e2e/toast-dialog-placement.spec.ts new file mode 100644 index 00000000..2b2bc014 --- /dev/null +++ b/frontend/tests-e2e/toast-dialog-placement.spec.ts @@ -0,0 +1,39 @@ +import { test, expect } from '@playwright/test'; +import { mockApi, seedAuth } from './fixtures/recs'; + +// Issue #742: the toast stack moves only when its default bottom-right corner +// would cover an open dialog's button row. A short .modal-wide dialog is +// centred with its buttons mid-screen, so an error toast stays in the corner +// instead of moving onto the dialog's content. +const OVERRIDES = [ + { id: 'o1', account_id: 'acct-001', provider: 'aws', service: 'ec2', enabled: true, term: 3, payment: 'no-upfront', coverage: 70 }, +]; + +test('an error toast stays off a short centred .modal-wide dialog at 1280x720', async ({ page }) => { + await page.setViewportSize({ width: 1280, height: 720 }); + await seedAuth(page); await mockApi(page); + await page.route('**/api/accounts/*/service-overrides', route => route.fulfill({ json: OVERRIDES })); + await page.route('**/api/accounts/*/service-overrides/*/*', route => route.request().method() === 'DELETE' + ? route.fulfill({ status: 500, json: { error: 'database unavailable' } }) + : route.fallback()); + await page.goto('/admin/accounts'); + await page.locator('#aws-accounts-list button', { hasText: 'Overrides' }).first().click(); + const dialog = page.locator('#account-overrides-modal .modal-content'); + await expect(dialog.locator('table.overrides-table tbody tr')).toHaveCount(1); + + await dialog.getByRole('button', { name: 'Delete' }).click(); + await page.getByRole('dialog', { name: 'Delete override?' }).getByRole('button', { name: 'Delete override' }).click(); + const toast = page.getByRole('alert').filter({ hasText: 'Failed to delete override' }); + await expect(toast).toBeVisible(); + await expect(page.getByRole('dialog', { name: 'Delete override?' })).toHaveCount(0); + + // Let the layout pass queued by the confirm closing run. + await page.evaluate(() => new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve)))); + const t = (await toast.boundingBox())!; + const d = (await dialog.boundingBox())!; + const overlaps = t.x < d.x + d.width && d.x < t.x + t.width && t.y < d.y + d.height && d.y < t.y + t.height; + expect(overlaps, `toast ${JSON.stringify(t)} covers the dialog ${JSON.stringify(d)}`).toBe(false); + // The dialog's own button still works with the toast up. + await page.locator('#close-account-overrides-modal-btn').click({ timeout: 5000 }); + await expect(page.locator('#account-overrides-modal')).toBeHidden(); +});