From 216d8e7151e3b321b7e244a093e03d867b27ced3 Mon Sep 17 00:00:00 2001 From: sharan-bruno Date: Tue, 9 Jun 2026 18:51:24 +0530 Subject: [PATCH] fix(ui): prevent empty header row from persisting state and crashing CLI (#8167) * fix: 3228 Empty header row persists in state, file, and crashes CLI * fix: refactor test steps for auto-append empty header row functionality * fix: update key column identification to use isKeyField property * fix: prevent duplicate empty rows in EditableTable and improve empty row detection * fix: update addMultipartFileToLastRow to target the last row correctly * addressed review comment --- .../src/components/EditableTable/index.js | 80 ++++++------------- .../auto-append-empty-header-row.spec.ts | 79 ++++++++++++++++++ tests/utils/page/actions.ts | 16 ++-- 3 files changed, 115 insertions(+), 60 deletions(-) create mode 100644 tests/request/headers/auto-append-empty-header-row.spec.ts diff --git a/packages/bruno-app/src/components/EditableTable/index.js b/packages/bruno-app/src/components/EditableTable/index.js index 5dd24803f..bbf929d6e 100644 --- a/packages/bruno-app/src/components/EditableTable/index.js +++ b/packages/bruno-app/src/components/EditableTable/index.js @@ -168,6 +168,17 @@ const EditableTable = ({ }; }, [defaultRow, checkboxKey]); + const hasAnyValue = useCallback((row) => { + for (const col of columns) { + const val = col.getValue ? col.getValue(row) : row[col.key]; + const defaultVal = defaultRow[col.key]; + if (val && val !== defaultVal && (typeof val !== 'string' || val.trim() !== '')) { + return true; + } + } + return false; + }, [columns, defaultRow]); + const rowsWithEmpty = useMemo(() => { if (!showAddRow) { return rows; @@ -177,16 +188,11 @@ const EditableTable = ({ return [createEmptyRow()]; } - const lastRow = rows[rows.length - 1]; - const keyColumn = columns.find((col) => col.isKeyField); - - if (keyColumn) { - const lastRowKeyValue = keyColumn.getValue ? keyColumn.getValue(lastRow) : lastRow[keyColumn.key]; - const isLastRowEmpty = !lastRowKeyValue || (typeof lastRowKeyValue === 'string' && lastRowKeyValue.trim() === ''); - - if (isLastRowEmpty) { - return rows; - } + // If the last row is already empty (e.g. a stray empty row loaded from a + // pre-existing file), don't append another one — otherwise the table would + // render two empty rows at the bottom on the initial render. + if (!hasAnyValue(rows[rows.length - 1])) { + return rows; } if (!emptyRowUidRef.current || rows.some((r) => r.uid === emptyRowUidRef.current)) { @@ -198,15 +204,11 @@ const EditableTable = ({ [checkboxKey]: true, ...defaultRow }]; - }, [rows, columns, defaultRow, checkboxKey, createEmptyRow, showAddRow]); + }, [rows, columns, defaultRow, checkboxKey, createEmptyRow, hasAnyValue, showAddRow]); - const isEmptyRow = useCallback((row) => { - const keyColumn = columns.find((col) => col.isKeyField); - if (!keyColumn) return false; - - const value = keyColumn.getValue ? keyColumn.getValue(row) : row[keyColumn.key]; - return !value || (typeof value === 'string' && value.trim() === ''); - }, [columns]); + // A row is empty when none of its columns hold a value — the single source of + // truth used everywhere (memo guard, persistence filter, last-row rendering). + const isEmptyRow = useCallback((row) => !hasAnyValue(row), [hasAnyValue]); const isLastEmptyRow = useCallback((row, index) => { if (!showAddRow) return false; @@ -227,50 +229,20 @@ const EditableTable = ({ const rowIndex = rowsWithEmpty.findIndex((r) => r.uid === rowUid); if (rowIndex === -1) return; - const currentRow = rowsWithEmpty[rowIndex]; - const isLast = rowIndex === rowsWithEmpty.length - 1; - const wasEmpty = isEmptyRow(currentRow); - - const keyColumn = columns.find((col) => col.isKeyField); - const isKeyFieldChange = keyColumn && keyColumn.key === key; - - let updatedRows = rowsWithEmpty.map((row) => { + const updatedRows = rowsWithEmpty.map((row) => { if (row.uid === rowUid) { return { ...row, [key]: value }; } return row; }); - // Only add a new empty row when the key field is filled - if (showAddRow && isLast && wasEmpty && isKeyFieldChange && value && value.trim() !== '') { - emptyRowUidRef.current = uuid(); - updatedRows.push({ - uid: emptyRowUidRef.current, - [checkboxKey]: true, - ...defaultRow - }); - } - - const hasAnyValue = (row) => { - for (const col of columns) { - const val = col.getValue ? col.getValue(row) : row[col.key]; - const defaultVal = defaultRow[col.key]; - if (val && val !== defaultVal && (typeof val !== 'string' || val.trim() !== '')) { - return true; - } - } - return false; - }; - - const result = updatedRows.filter((row, i) => { - if (showAddRow && i === updatedRows.length - 1) { - return hasAnyValue(row); - } - return true; - }); + // Remove any fully-empty rows from the persisted data. The trailing empty + // "add row" is re-added by the rowsWithEmpty memo, so there's always + // exactly one empty row at the bottom and never a stray empty row above it. + const result = showAddRow ? updatedRows.filter(hasAnyValue) : updatedRows; onChange(result); - }, [rowsWithEmpty, columns, onChange, checkboxKey, defaultRow, isEmptyRow, showAddRow]); + }, [rowsWithEmpty, hasAnyValue, onChange, showAddRow]); const handleCheckboxChange = useCallback((rowUid, checked) => { handleValueChange(rowUid, checkboxKey, checked); diff --git a/tests/request/headers/auto-append-empty-header-row.spec.ts b/tests/request/headers/auto-append-empty-header-row.spec.ts new file mode 100644 index 000000000..eb71d1737 --- /dev/null +++ b/tests/request/headers/auto-append-empty-header-row.spec.ts @@ -0,0 +1,79 @@ +import { test, expect } from '../../../playwright'; +import { + buildCommonLocators, + closeAllCollections, + createCollection, + createFolder, + createRequest, + selectRequestPaneTab +} from '../../utils/page'; + +test.afterEach(async ({ page }) => { + await closeAllCollections(page); +}); + +test('Auto-appends an empty header row when either Name or Value is filled', async ({ page, createTmpDir }) => { + const collectionName = 'auto-append-empty-header-row'; + const locators = buildCommonLocators(page); + + await test.step('Create a collection', async () => { + await createCollection(page, collectionName, await createTmpDir()); + }); + + await test.step('Create folder-1 inside the collection', async () => { + await createFolder(page, 'folder-1', collectionName, true); + await locators.sidebar.folder('folder-1').dblclick(); + }); + + await test.step('Create an HTTP request inside folder-1', async () => { + await createRequest(page, 'request-1', 'folder-1', { url: 'https://example.com/api', inFolder: true }); + await selectRequestPaneTab(page, 'Headers'); + }); + + const headersTable = page.locator('table').first(); + const rows = headersTable.locator('tbody tr'); + + await test.step('Starts with a single empty header row', async () => { + await expect(rows).toHaveCount(1); + }); + + await test.step('Typing into the Name field appends a new empty row', async () => { + const nameEditor = rows.first().getByTestId('column-name').locator('.CodeMirror'); + await nameEditor.click(); + await page.keyboard.type('Content-Type'); + + await expect(rows).toHaveCount(2); + }); + + await test.step('Typing only into the Value field (Name left empty) also appends a new empty row', async () => { + const valueEditor = rows.nth(1).getByTestId('column-value').locator('.CodeMirror'); + await valueEditor.click(); + await page.keyboard.type('application/json'); + + await expect(rows).toHaveCount(3); + }); + + const selectAll = process.platform === 'darwin' ? 'Meta+a' : 'Control+a'; + + await test.step('Clearing the Name field empties that row and removes it', async () => { + // Row 0 has only a Name ("Content-Type"); clearing it makes the row fully + // empty, so it is dropped and the trailing empty row remains. + const nameEditor = rows.first().getByTestId('column-name').locator('.CodeMirror'); + await nameEditor.click(); + await page.keyboard.press(selectAll); + await page.keyboard.press('Backspace'); + + await expect(rows).toHaveCount(2); + }); + + await test.step('Clearing the Value field empties that row, leaving only the empty add row', async () => { + // The Value-only row is now first; clearing its Value makes it fully empty, + // so it is dropped and only the single trailing empty row is left. + const valueEditor = rows.first().getByTestId('column-value').locator('.CodeMirror'); + await valueEditor.click(); + await page.keyboard.press(selectAll); + await page.keyboard.press('Backspace'); + + await expect(rows).toHaveCount(1); + }); +}); diff --git a/tests/utils/page/actions.ts b/tests/utils/page/actions.ts index ea958fa04..b01065d44 100644 --- a/tests/utils/page/actions.ts +++ b/tests/utils/page/actions.ts @@ -1120,13 +1120,17 @@ const addMultipartFileToLastRow = async (page: Page, electronApp: ElectronApplic await mockBrowseFiles(electronApp, [filePath]); const table = buildCommonLocators(page).table('editable-table'); - const lastRow = table.allRows().last(); + // The last row is the empty "add" row. Capture its index now, because once + // we set a file the table appends a new empty row — so `.last()` would jump + // to that new row instead of staying on the one we just filled. + const rowIndex = (await table.allRows().count()) - 1; + const targetRow = table.allRows().nth(rowIndex); - await expect(lastRow.locator('.upload-btn')).toBeVisible(); - await lastRow.locator('.upload-btn').click(); - await expect(lastRow.locator('.file-value-cell')).toBeVisible(); - const inlineChip = lastRow.getByTestId('multipart-file-chip').filter({ hasText: path.basename(filePath) }); - const summary = lastRow.getByTestId('multipart-file-summary'); + await expect(targetRow.locator('.upload-btn')).toBeVisible(); + await targetRow.locator('.upload-btn').click(); + await expect(targetRow.locator('.file-value-cell')).toBeVisible(); + const inlineChip = targetRow.getByTestId('multipart-file-chip').filter({ hasText: path.basename(filePath) }); + const summary = targetRow.getByTestId('multipart-file-summary'); await expect(inlineChip.or(summary)).toBeVisible(); }); };