fix: Remove loading flashes from data tables (#39874)

Co-authored-by: n8n-cat-bot[bot] <n8n-cat-bot[bot]@users.noreply.github.com>
Co-authored-by: Arvin Ansari <[email protected]>
This commit is contained in:
n8n-cat-bot[bot]
2026-10-07 14:31:09 +00:00
committed by GitHub
co-authored by n8n-cat-bot[bot] Arvin Ansari
parent 3816c4afec
commit 2f4a3868fb
16 changed files with 1007 additions and 121 deletions
@@ -0,0 +1,491 @@
import { createComponentRenderer } from '@/__tests__/render';
import { useDataTableStore } from '@/features/core/dataTable/dataTable.store';
import type { DataTable, DataTableRow } from '@/features/core/dataTable/dataTable.types';
import { useSourceControlStore } from '@/features/integrations/sourceControl.ee/sourceControl.store';
import { createDeferredPromise } from '@n8n/utils/promise/deferred-promise';
import { createTestingPinia } from '@pinia/testing';
import { fireEvent, waitFor } from '@testing-library/vue';
import userEvent from '@testing-library/user-event';
import { flushPromises } from '@vue/test-utils';
import { ref } from 'vue';
import { getGridApi } from 'ag-grid-community';
import InstanceAiDataTablePreview from '../components/InstanceAiDataTablePreview.vue';
const isAgentWorking = ref(false);
vi.mock('../composables/useIsAgentWorking', () => ({
useIsAgentWorking: () => isAgentWorking,
}));
const firstTable: DataTable = {
id: 'table-1',
name: 'Products',
projectId: 'project-1',
columns: [{ id: 'name-column', name: 'name', type: 'string', index: 0 }],
createdAt: '2026-01-01T00:00:00Z',
updatedAt: '2026-01-01T00:00:00Z',
sizeBytes: 0,
};
const secondTable: DataTable = { ...firstTable, id: 'table-2', name: 'Inventory' };
const firstRows = { data: [{ id: 1, name: 'Notebook' }], count: 1 };
type Rows = { data: DataTableRow[]; count: number };
const renderComponent = createComponentRenderer(InstanceAiDataTablePreview, {
props: { dataTableId: firstTable.id, projectId: firstTable.projectId },
});
describe('InstanceAiDataTablePreview', () => {
beforeEach(() => {
isAgentWorking.value = false;
});
afterEach(() => {
vi.useRealTimers();
});
function setup() {
const pinia = createTestingPinia();
const store = useDataTableStore();
vi.mocked(store.fetchDataTableDetails).mockResolvedValue(firstTable);
vi.mocked(store.fetchDataTableContent).mockResolvedValue(firstRows);
return { pinia, store };
}
it('shows the slow switch indicator after one second across schema and row requests', async () => {
const { pinia, store } = setup();
const { getByTestId, queryByTestId, rerender } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
const displayedGrid = getByTestId('instance-ai-data-table-grid');
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] });
const details = createDeferredPromise<DataTable>();
const rows = createDeferredPromise<Rows>();
vi.mocked(store.fetchDataTableDetails).mockReturnValue(details.promise);
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
await rerender({ dataTableId: secondTable.id });
await vi.advanceTimersByTimeAsync(600);
details.resolve(secondTable);
await flushPromises();
await vi.advanceTimersByTimeAsync(399);
expect(store.fetchDataTableContent).toHaveBeenCalledTimes(2);
expect(queryByTestId('data-table-loading')).not.toBeInTheDocument();
await vi.advanceTimersByTimeAsync(1);
expect(getByTestId('data-table-loading')).toBeVisible();
expect(displayedGrid).toHaveTextContent('Notebook');
expect(displayedGrid).toHaveAttribute('inert');
rows.resolve({ data: [{ id: 2, name: 'Pencil' }], count: 1 });
await flushPromises();
await vi.advanceTimersByTimeAsync(100);
expect(queryByTestId('data-table-loading')).not.toBeInTheDocument();
expect(displayedGrid).not.toBeInTheDocument();
expect(getByTestId('instance-ai-data-table-grid')).toHaveTextContent('Pencil');
});
it('restarts the delay for another tab and clears the indicator after an error', async () => {
const { pinia, store } = setup();
const { getByTestId, queryByTestId, rerender } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] });
const earlierDetails = createDeferredPromise<DataTable>();
const currentDetails = createDeferredPromise<DataTable>();
vi.mocked(store.fetchDataTableDetails)
.mockReturnValueOnce(earlierDetails.promise)
.mockReturnValueOnce(currentDetails.promise);
await rerender({ dataTableId: secondTable.id });
await vi.advanceTimersByTimeAsync(999);
await rerender({ dataTableId: 'table-3' });
await vi.advanceTimersByTimeAsync(999);
expect(queryByTestId('data-table-loading')).not.toBeInTheDocument();
await vi.advanceTimersByTimeAsync(1);
expect(getByTestId('data-table-loading')).toBeVisible();
earlierDetails.resolve(secondTable);
await flushPromises();
expect(getByTestId('data-table-loading')).toBeVisible();
currentDetails.reject(new Error('Request failed'));
await flushPromises();
expect(queryByTestId('data-table-loading')).not.toBeInTheDocument();
expect(queryByTestId('instance-ai-data-table-grid')).not.toBeInTheDocument();
});
it('does not show a delayed indicator after a fast switch finishes', async () => {
const { pinia, store } = setup();
const { getByTestId, queryByTestId, rerender } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] });
vi.mocked(store.fetchDataTableDetails).mockResolvedValue(secondTable);
await rerender({ dataTableId: secondTable.id });
await vi.advanceTimersByTimeAsync(100);
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute(
'data-table-id',
secondTable.id,
);
expect(queryByTestId('data-table-loading')).not.toBeInTheDocument();
await vi.advanceTimersByTimeAsync(1000);
expect(queryByTestId('data-table-loading')).not.toBeInTheDocument();
});
it.each([firstRows, { data: [], count: 0 }])(
'keeps the initial spinner until the grid is ready for $count rows',
async (response) => {
const { pinia, store } = setup();
const rows = createDeferredPromise<Rows>();
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
const { getByTestId, queryByTestId } = renderComponent({ pinia });
await waitFor(() => expect(store.fetchDataTableContent).toHaveBeenCalled());
expect(getByTestId('instance-ai-data-table-loading')).toBeInTheDocument();
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'true');
rows.resolve(response);
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
expect(queryByTestId('instance-ai-data-table-loading')).not.toBeInTheDocument();
expect(
getByTestId('instance-ai-data-table-grid').querySelector('.ag-header-cell'),
).toBeInTheDocument();
},
);
it.each(['tab switch', 'refresh'])(
'keeps the displayed grid until its replacement is ready: %s',
async (change) => {
const { pinia, store } = setup();
const { getAllByTestId, getByTestId, queryByTestId, rerender } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
const displayedGrid = getByTestId('instance-ai-data-table-grid');
const details = createDeferredPromise<DataTable>();
const rows = createDeferredPromise<Rows>();
vi.mocked(store.fetchDataTableDetails).mockReturnValue(details.promise);
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
if (change === 'tab switch') {
await rerender({ dataTableId: secondTable.id });
} else {
await rerender({ refreshKey: 1 });
}
expect(displayedGrid).toBeInTheDocument();
expect(displayedGrid).toHaveAttribute('aria-hidden', 'false');
expect(displayedGrid).toHaveAttribute('inert');
expect(displayedGrid).toHaveTextContent('Notebook');
expect(queryByTestId('instance-ai-data-table-loading')).not.toBeInTheDocument();
details.resolve(change === 'tab switch' ? secondTable : firstTable);
await waitFor(() => expect(store.fetchDataTableContent).toHaveBeenCalledTimes(2));
const pendingGrid = getAllByTestId('instance-ai-data-table-grid')[1];
expect(pendingGrid).toHaveAttribute('aria-hidden', 'true');
expect(displayedGrid).toHaveAttribute('aria-hidden', 'false');
rows.resolve({ data: [{ id: 2, name: 'Pencil' }], count: 1 });
await waitFor(() => expect(pendingGrid).toHaveAttribute('aria-hidden', 'false'));
expect(pendingGrid).toHaveTextContent('Pencil');
expect(pendingGrid).not.toHaveAttribute('inert');
expect(displayedGrid).not.toBeInTheDocument();
expect(store.fetchDataTableContent).toHaveBeenCalledTimes(2);
},
);
it.each(['agent', 'branch'])('keeps the grid and page when the %s lock changes', async (lock) => {
const { pinia, store } = setup();
const sourceControlStore = useSourceControlStore();
vi.mocked(store.fetchDataTableContent).mockResolvedValue({ ...firstRows, count: 40 });
const { getByTestId, queryByTestId } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
const displayedGrid = getByTestId('instance-ai-data-table-grid');
const gridApi = getGridApi(displayedGrid.querySelector('[grid-id]')?.parentElement)!;
gridApi.applyColumnState({ state: [{ colId: 'name-column', width: 300 }] });
const columnState = gridApi.getColumnState();
vi.mocked(store.fetchDataTableContent).mockResolvedValue({
data: [{ id: 21, name: 'Pencil' }],
count: 40,
});
await fireEvent.click(getByTestId('pagination-next'));
await waitFor(() => expect(displayedGrid).toHaveTextContent('Pencil'));
for (const locked of [true, false]) {
if (lock === 'agent') isAgentWorking.value = locked;
else sourceControlStore.preferences.branchReadOnly = locked;
await flushPromises();
expect(getByTestId('instance-ai-data-table-grid')).toBe(displayedGrid);
expect(gridApi.isDestroyed()).toBe(false);
expect(gridApi.getColumnState()).toEqual(columnState);
expect(displayedGrid).toHaveTextContent('Pencil');
expect(getByTestId('pagination-next')).toBeDisabled();
expect(queryByTestId('instance-ai-data-table-loading')).not.toBeInTheDocument();
expect(store.fetchDataTableDetails).toHaveBeenCalledOnce();
expect(store.fetchDataTableContent).toHaveBeenCalledTimes(2);
}
});
it.each([false, true])(
'updates editing controls in place from readOnly=%s',
async (initialLock) => {
const { pinia, store } = setup();
vi.mocked(store.fetchDataTableDetails).mockResolvedValue({
...firstTable,
columns: [
...firstTable.columns,
{ id: 'stock-column', name: 'inStock', type: 'boolean', index: 1 },
],
});
vi.mocked(store.fetchDataTableContent).mockResolvedValue({
data: [{ id: 1, name: 'Notebook', inStock: true }],
count: 1,
});
isAgentWorking.value = initialLock;
const { getByTestId, getByRole } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
const displayedGrid = getByTestId('instance-ai-data-table-grid');
const gridApi = getGridApi(displayedGrid.querySelector('[grid-id]')?.parentElement)!;
const header = displayedGrid.querySelector('.ag-header-cell[col-id="name-column"]')!;
const row = gridApi.getRowNode('1')!;
for (const locked of [initialLock, !initialLock, initialLock]) {
isAgentWorking.value = locked;
await waitFor(() => expect(row.selectable).toBe(!locked));
expect(gridApi.getGridOption('suppressMovableColumns')).toBe(locked);
expect(getByTestId('instance-ai-data-table-grid')).toBe(displayedGrid);
await fireEvent.mouseEnter(
header.querySelector('[data-test-id="data-table-column-header"]')!,
);
const addRow = getByRole('button', { name: 'Add Row' });
const addColumn = getByTestId('data-table-add-column-trigger-button');
const menu = header.querySelector('button[aria-haspopup="menu"]');
const checkbox = displayedGrid.querySelector(
'.ag-row[row-id="1"] .ag-cell[col-id="stock-column"] input[type="checkbox"]',
);
if (locked) {
expect(addRow).toBeDisabled();
expect(addColumn).toBeDisabled();
expect(checkbox).toBeDisabled();
expect(menu).not.toBeVisible();
expect(row.isSelected()).toBe(false);
} else {
expect(addRow).toBeEnabled();
expect(addColumn).toBeEnabled();
expect(checkbox).toBeEnabled();
expect(menu).toBeVisible();
expect(gridApi.getColumn('name-column')!.getColDef().suppressMovable).not.toBe(true);
row.setSelected(true);
expect(row.isSelected()).toBe(true);
}
}
expect(row.data.name).toBe('Notebook');
expect(store.updateRow).not.toHaveBeenCalled();
expect(store.fetchDataTableDetails).toHaveBeenCalledOnce();
expect(store.fetchDataTableContent).toHaveBeenCalledOnce();
},
);
it('cancels an active cell edit on lock and permits editing after unlock', async () => {
const { pinia, store } = setup();
const { getByTestId } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
const displayedGrid = getByTestId('instance-ai-data-table-grid');
const gridApi = getGridApi(displayedGrid.querySelector('[grid-id]')?.parentElement)!;
gridApi.startEditingCell({ rowIndex: 0, colKey: 'name-column' });
expect(gridApi.getEditingCells()).toHaveLength(1);
await fireEvent.update(displayedGrid.querySelector('textarea')!, 'Changed value');
isAgentWorking.value = true;
await waitFor(() => expect(gridApi.getEditingCells()).toHaveLength(0));
expect(gridApi.getRowNode('1')!.data.name).toBe('Notebook');
expect(store.updateRow).not.toHaveBeenCalled();
gridApi.startEditingCell({ rowIndex: 0, colKey: 'name-column' });
expect(gridApi.getEditingCells()).toHaveLength(0);
isAgentWorking.value = false;
await flushPromises();
gridApi.startEditingCell({ rowIndex: 0, colKey: 'name-column' });
expect(gridApi.getEditingCells()).toHaveLength(1);
expect(store.fetchDataTableDetails).toHaveBeenCalledOnce();
expect(store.fetchDataTableContent).toHaveBeenCalledOnce();
});
it('uses the current lock when a pending table becomes ready without restarting its requests', async () => {
const { pinia, store } = setup();
const details = createDeferredPromise<DataTable>();
const rows = createDeferredPromise<Rows>();
vi.mocked(store.fetchDataTableDetails).mockReturnValue(details.promise);
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
const { getByTestId, getByRole } = renderComponent({ pinia });
isAgentWorking.value = true;
await flushPromises();
details.resolve(firstTable);
await waitFor(() => expect(store.fetchDataTableContent).toHaveBeenCalledOnce());
isAgentWorking.value = false;
await flushPromises();
rows.resolve(firstRows);
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
expect(getByRole('button', { name: 'Add Row' })).toBeEnabled();
expect(store.fetchDataTableDetails).toHaveBeenCalledOnce();
expect(store.fetchDataTableContent).toHaveBeenCalledOnce();
});
it('closes the open column popover when the agent locks the displayed grid', async () => {
const { pinia, store } = setup();
const user = userEvent.setup();
const { getByTestId, queryByTestId } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
await user.click(getByTestId('data-table-add-column-trigger-button'));
await user.type(getByTestId('add-column-name-input'), 'description');
const submitButton = getByTestId('data-table-add-column-submit-button');
isAgentWorking.value = true;
await flushPromises();
await fireEvent.click(submitButton);
expect(store.addDataTableColumn).not.toHaveBeenCalled();
expect(queryByTestId('add-column-popover-content')).not.toBeInTheDocument();
expect(getByTestId('instance-ai-data-table-grid')).toHaveTextContent('Notebook');
});
it('closes the open column menu when the agent locks the displayed grid', async () => {
const { pinia, store } = setup();
const user = userEvent.setup();
const { getByTestId, getByRole, queryByRole } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
const header = getByTestId('instance-ai-data-table-grid').querySelector(
'.ag-header-cell[col-id="name-column"]',
)!;
await fireEvent.mouseEnter(header.querySelector('[data-test-id="data-table-column-header"]')!);
await user.click(header.querySelector('button[aria-haspopup="menu"]')!);
expect(getByRole('menu')).toBeInTheDocument();
isAgentWorking.value = true;
await waitFor(() => expect(queryByRole('menu')).not.toBeInTheDocument());
expect(store.deleteDataTableColumn).not.toHaveBeenCalled();
expect(getByTestId('instance-ai-data-table-grid')).toHaveTextContent('Notebook');
});
it('ignores an earlier details response after another tab is selected', async () => {
const { pinia, store } = setup();
const earlierDetails = createDeferredPromise<DataTable>();
vi.mocked(store.fetchDataTableDetails).mockReturnValueOnce(earlierDetails.promise);
const { getByTestId, rerender } = renderComponent({ pinia });
vi.mocked(store.fetchDataTableDetails).mockResolvedValue(secondTable);
await rerender({ dataTableId: secondTable.id });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
earlierDetails.resolve(firstTable);
await flushPromises();
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute(
'data-table-id',
secondTable.id,
);
expect(store.fetchDataTableContent).toHaveBeenCalledTimes(1);
});
it.each(['success', 'failure'])(
'keeps the selected table when an earlier rows request finishes later: %s',
async (result) => {
const { pinia, store } = setup();
const { getAllByTestId, getByTestId, rerender } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
const earlierRows = createDeferredPromise<Rows>();
vi.mocked(store.fetchDataTableDetails).mockResolvedValueOnce(secondTable);
vi.mocked(store.fetchDataTableContent).mockReturnValueOnce(earlierRows.promise);
await rerender({ dataTableId: secondTable.id });
await waitFor(() => expect(store.fetchDataTableContent).toHaveBeenCalledTimes(2));
expect(getAllByTestId('instance-ai-data-table-grid')).toHaveLength(2);
await rerender({ dataTableId: firstTable.id });
await waitFor(() => expect(store.fetchDataTableContent).toHaveBeenCalledTimes(3));
await waitFor(() => expect(getAllByTestId('instance-ai-data-table-grid')).toHaveLength(1));
if (result === 'success') {
earlierRows.resolve({ data: [{ id: 2, name: 'Pencil' }], count: 1 });
} else {
earlierRows.reject(new Error('Request failed'));
}
await flushPromises();
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute(
'data-table-id',
firstTable.id,
);
expect(getByTestId('instance-ai-data-table-grid')).toHaveTextContent('Notebook');
},
);
it.each(['initial load', 'tab switch'])(
'shows an error when the rows request fails during %s',
async (change) => {
const { pinia, store } = setup();
const rows = createDeferredPromise<Rows>();
if (change === 'initial load') {
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
}
const { getByTestId, queryByTestId, findByText, rerender } = renderComponent({ pinia });
if (change === 'tab switch') {
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute(
'aria-hidden',
'false',
);
});
vi.mocked(store.fetchDataTableDetails).mockResolvedValue(secondTable);
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
await rerender({ dataTableId: secondTable.id });
}
await waitFor(() => {
expect(store.fetchDataTableContent).toHaveBeenCalledTimes(change === 'tab switch' ? 2 : 1);
});
rows.reject(new Error('Request failed'));
expect(await findByText('Could not load data table')).toBeInTheDocument();
expect(queryByTestId('instance-ai-data-table-grid')).not.toBeInTheDocument();
expect(queryByTestId('instance-ai-data-table-loading')).not.toBeInTheDocument();
},
);
it('shows an error when the selected table cannot be loaded', async () => {
const { pinia, store } = setup();
const { getByTestId, queryByTestId, findByText, rerender } = renderComponent({ pinia });
await waitFor(() => {
expect(getByTestId('instance-ai-data-table-grid')).toHaveAttribute('aria-hidden', 'false');
});
vi.mocked(store.fetchDataTableDetails).mockRejectedValue(new Error('Request failed'));
await rerender({ dataTableId: secondTable.id });
expect(await findByText('Could not load data table')).toBeInTheDocument();
expect(queryByTestId('instance-ai-data-table-grid')).not.toBeInTheDocument();
expect(queryByTestId('instance-ai-data-table-loading')).not.toBeInTheDocument();
});
});
@@ -1,8 +1,9 @@
<script lang="ts" setup>
import { computed, ref, watch } from 'vue';
import { computed, ref, shallowRef, watch } from 'vue';
import { N8nIcon, N8nText } from '@n8n/design-system';
import { useI18n } from '@n8n/i18n';
import DataTableTable from '@/features/core/dataTable/components/dataGrid/DataTableTable.vue';
import DataTableLoadingIndicator from '@/features/core/dataTable/components/dataGrid/DataTableLoadingIndicator.vue';
import { useDataTableStore } from '@/features/core/dataTable/dataTable.store';
import type { DataTable } from '@/features/core/dataTable/dataTable.types';
import { useSourceControlStore } from '@/features/integrations/sourceControl.ee/sourceControl.store';
@@ -22,9 +23,21 @@ const i18n = useI18n();
const dataTableStore = useDataTableStore();
const sourceControlStore = useSourceControlStore();
const dataTable = ref<DataTable | null>(null);
type TablePreview = {
key: number;
dataTable: DataTable;
};
const tableSlots = shallowRef<{ displayed: TablePreview | null; pending: TablePreview | null }>({
displayed: null,
pending: null,
});
const tables = computed(() =>
[tableSlots.value.displayed, tableSlots.value.pending].filter((table) => table !== null),
);
const isLoading = ref(false);
const fetchError = ref<string | null>(null);
const requestKey = ref(0);
// === Editing lock ===
// The grid is editable only while the AI is not running, so user edits can't
@@ -38,41 +51,50 @@ const isReadOnly = computed(
() => isAgentWorking.value || sourceControlStore.preferences.branchReadOnly,
);
async function fetchDataTable(id: string, projectId: string) {
const isRefresh = dataTable.value?.id === id;
function showTable(key: number) {
const { pending } = tableSlots.value;
if (pending?.key !== key) return;
tableSlots.value = { displayed: pending, pending: null };
isLoading.value = false;
}
fetchError.value = null;
if (!isRefresh) {
isLoading.value = true;
dataTable.value = null;
}
function onLoadError(key: number) {
if (tableSlots.value.pending?.key !== key) return;
try {
// Always fetch fresh details (never the store cache): the grid is
// editable, so stale columns would let the user edit against a schema
// the agent has since changed.
const result = await dataTableStore.fetchDataTableDetails(id, projectId);
dataTable.value = result ?? null;
if (!result) {
fetchError.value = i18n.baseText('instanceAi.dataTablePreview.fetchError');
}
} catch {
dataTable.value = null;
fetchError.value = i18n.baseText('instanceAi.dataTablePreview.fetchError');
} finally {
isLoading.value = false;
}
tableSlots.value = { displayed: null, pending: null };
isLoading.value = false;
fetchError.value = i18n.baseText('instanceAi.dataTablePreview.fetchError');
}
// Re-fetch when dataTableId changes OR when refreshKey increments (same table modified).
watch(
() => [props.dataTableId, props.refreshKey] as const,
async ([id]) => {
if (id && props.projectId) {
await fetchDataTable(id, props.projectId);
() => [props.dataTableId, props.projectId, props.refreshKey] as const,
async ([id, projectId], _previous, onCleanup) => {
let cancelled = false;
onCleanup(() => {
cancelled = true;
});
tableSlots.value = { displayed: tableSlots.value.displayed, pending: null };
fetchError.value = null;
isLoading.value = !!id && !!projectId;
if (!id || !projectId) {
tableSlots.value = { displayed: null, pending: null };
return;
}
const key = ++requestKey.value;
// Fetch the current schema before the replacement grid permits edits.
const result = await dataTableStore.fetchDataTableDetails(id, projectId).catch(() => null);
if (cancelled) return;
if (result) {
tableSlots.value = {
displayed: tableSlots.value.displayed,
pending: { key, dataTable: result },
};
} else {
dataTable.value = null;
fetchError.value = null;
tableSlots.value = { displayed: null, pending: null };
isLoading.value = false;
fetchError.value = i18n.baseText('instanceAi.dataTablePreview.fetchError');
}
},
{ immediate: true },
@@ -80,23 +102,36 @@ watch(
</script>
<template>
<div :class="$style.content">
<!-- Error (only when no data table to show) -->
<div v-if="fetchError && !dataTable" :class="$style.centerState">
<div :class="$style.content" :aria-busy="isLoading">
<div v-if="fetchError" :class="$style.centerState">
<N8nText color="text-light">{{ fetchError }}</N8nText>
</div>
<!-- Data table grid. readOnly is part of the key because the grid bakes it
into its column defs at grid-ready, so flipping it requires a remount. -->
<DataTableTable
v-if="dataTable"
:key="`${props.refreshKey}-${isReadOnly}`"
:data-table="dataTable"
:read-only="isReadOnly"
/>
<!-- Keep the previous grid visible until the replacement has rendered its rows. -->
<div
v-for="table in tables"
:key="table.key"
:class="{ [$style.pendingTable]: table.key !== tableSlots.displayed?.key }"
:aria-hidden="table.key !== tableSlots.displayed?.key"
:inert="isLoading || undefined"
:data-table-id="table.dataTable.id"
data-test-id="instance-ai-data-table-grid"
>
<DataTableTable
:data-table="table.dataTable"
:read-only="isReadOnly || (isLoading && table.key === tableSlots.displayed?.key)"
@ready="showTable(table.key)"
@load-error="onLoadError(table.key)"
/>
</div>
<!-- Loading overlay (shown during initial load or when no data table yet) -->
<div v-if="isLoading && !dataTable" :class="$style.centerState">
<DataTableLoadingIndicator v-if="isLoading && tableSlots.displayed" :key="requestKey" />
<div
v-if="isLoading && !tableSlots.displayed"
:class="$style.centerState"
data-test-id="instance-ai-data-table-loading"
>
<N8nIcon icon="loader-circle" :size="80" spin />
</div>
</div>
@@ -110,6 +145,12 @@ watch(
height: 100%;
}
.pendingTable {
position: absolute;
inset: 0;
visibility: hidden;
}
.centerState {
display: flex;
flex-direction: column;
@@ -83,6 +83,18 @@ describe('DataTableDetailsView', () => {
});
describe('Loading states', () => {
it('shows the loading layout on the first render when opening a data table', () => {
const pinia = createTestingPinia({ stubActions: false });
const dataTableStore = useDataTableStore();
vi.spyOn(dataTableStore, 'fetchOrFindDataTable').mockImplementation(
async () => await new Promise(() => {}),
);
const { getByTestId } = renderComponent({ pinia });
expect(getByTestId('data-table-details-loading')).toBeInTheDocument();
});
it('should show loading state initially', async () => {
const pinia = createTestingPinia({ stubActions: false });
const dataTableStore = useDataTableStore();
@@ -50,7 +50,7 @@ const readOnlyEnv = computed(() => sourceControlStore.preferences.branchReadOnly
const dataTableHasDependents = computed(() => hasDependencies(props.id, 'dataTable'));
const loading = ref(false);
const loading = ref(true);
const saving = ref(false);
const dataTable = ref<DataTable | null>(null);
const dataTableTableRef = ref<InstanceType<typeof DataTableTable>>();
@@ -1,5 +1,5 @@
<script setup lang="ts">
import { computed, nextTick, ref } from 'vue';
import { computed, nextTick, ref, toValue, watch } from 'vue';
import type {
AddColumnResponse,
DataTableColumnCreatePayload,
@@ -33,7 +33,7 @@ const props = withDefaults(
// the params key is needed so that we can pass this directly to ag-grid as column
params: {
onAddColumn: (column: DataTableColumnCreatePayload) => Promise<AddColumnResponse>;
disabled?: boolean;
disabled?: boolean | (() => boolean);
};
popoverId?: string;
useTextTrigger?: boolean;
@@ -45,7 +45,7 @@ const props = withDefaults(
);
// Use disabled from params if available (when used as AG Grid header), otherwise use prop
const isDisabled = computed(() => props.params?.disabled ?? props.disabled);
const isDisabled = computed(() => toValue(props.params?.disabled ?? props.disabled));
const i18n = useI18n();
const { getIconForType } = useDataTableTypes();
@@ -64,6 +64,13 @@ const error = ref<FormError | null>(null);
const popoverOpen = ref(false);
const isSelectOpen = ref(false);
watch(isDisabled, (disabled) => {
if (disabled) {
isSelectOpen.value = false;
popoverOpen.value = false;
}
});
const popoverId = computed(() => props.popoverId ?? 'add-column-popover');
const columnTypeOptions = computed(() => {
@@ -76,6 +83,7 @@ const columnTypeOptions = computed(() => {
});
const onAddButtonClicked = async () => {
if (isDisabled.value) return;
validateName();
if (!columnName.value || !columnType.value || error.value) {
return;
@@ -0,0 +1,29 @@
import { createComponentRenderer } from '@/__tests__/render';
import { waitFor } from '@testing-library/vue';
import userEvent from '@testing-library/user-event';
import { ref } from 'vue';
import AddRowButton from './AddRowButton.vue';
describe('AddRowButton', () => {
it('updates the button when the disabled callback changes', async () => {
const locked = ref(true);
const onClick = vi.fn();
const { getByRole } = createComponentRenderer(AddRowButton, {
props: { params: { onClick, disabled: () => locked.value } },
})();
const button = getByRole('button');
const user = userEvent.setup();
expect(button).toBeDisabled();
await user.click(button);
expect(onClick).not.toHaveBeenCalled();
locked.value = false;
await waitFor(() => expect(button).toBeEnabled());
await user.click(button);
expect(onClick).toHaveBeenCalledOnce();
locked.value = true;
await waitFor(() => expect(button).toBeDisabled());
});
});
@@ -1,4 +1,5 @@
<script setup lang="ts">
import { computed, toValue } from 'vue';
import { useI18n } from '@n8n/i18n';
import { N8nIconButton, N8nTooltip } from '@n8n/design-system';
@@ -7,9 +8,11 @@ const i18n = useI18n();
const props = defineProps<{
params: {
onClick: () => void;
disabled?: boolean;
disabled?: boolean | (() => boolean);
};
}>();
const isDisabled = computed(() => toValue(props.params.disabled) ?? false);
</script>
<template>
@@ -18,7 +21,7 @@ const props = defineProps<{
variant="ghost"
icon="plus"
:aria-label="i18n.baseText('dataTable.addRow.label')"
:disabled="props.params.disabled"
:disabled="isDisabled"
@click="props.params.onClick"
/>
</N8nTooltip>
@@ -1,7 +1,7 @@
<script setup lang="ts">
import type { IHeaderParams, SortDirection } from 'ag-grid-community';
import { useDataTableTypes } from '@/features/core/dataTable/composables/useDataTableTypes';
import { ref, computed, onMounted, onUnmounted, useTemplateRef } from 'vue';
import { ref, computed, onMounted, onUnmounted, useTemplateRef, toValue, watch } from 'vue';
import { useI18n } from '@n8n/i18n';
import { isAGGridCellType } from '@/features/core/dataTable/typeGuards';
import { N8nActionDropdown, N8nIcon, N8nIconButton, N8nInlineTextEdit } from '@n8n/design-system';
@@ -12,7 +12,7 @@ export type HeaderParamsWithDelete = IHeaderParams & {
onRename?: (columnId: string, newName: string) => void;
allowMenuActions: boolean;
showTypeIcon?: boolean;
readOnly: boolean;
readOnly: boolean | (() => boolean);
};
const props = defineProps<{
@@ -23,11 +23,21 @@ const { getIconForType, mapToDataTableColumnType } = useDataTableTypes();
const i18n = useI18n();
const renameInput = useTemplateRef<InstanceType<typeof N8nInlineTextEdit>>('renameInput');
const actionDropdown = useTemplateRef<{ close: () => void }>('actionDropdown');
const isReadOnly = computed(() => toValue(props.params.readOnly));
const isHovered = ref(false);
const isDropdownOpen = ref(false);
const isFilterOpen = ref(false);
const hasActiveFilter = ref(false);
const currentSort = ref<SortDirection>(null);
watch(isReadOnly, (readOnly) => {
if (readOnly) {
actionDropdown.value?.close();
renameInput.value?.forceCancel();
}
});
const shouldShowTypeIcon = computed(() => props.params.showTypeIcon !== false);
const isFilterable = computed(() => props.params.column.getColDef().filter !== false);
@@ -37,6 +47,7 @@ const enum ItemAction {
}
const onNameSubmit = (newName: string) => {
if (isReadOnly.value) return;
const trimmed = newName.trim();
if (!trimmed || trimmed === props.params.displayName) {
renameInput.value?.forceCancel();
@@ -47,12 +58,14 @@ const onNameSubmit = (newName: string) => {
const onNameToggle = (e?: Event) => {
e?.stopPropagation();
if (isReadOnly.value) return;
if (renameInput.value?.forceFocus && !isSystemColumn.value) {
renameInput.value.forceFocus();
}
};
const onItemClick = (action: string) => {
if (isReadOnly.value) return;
const actionEnum = action as ItemAction;
if (actionEnum === ItemAction.Delete) {
props.params.onDelete?.(props.params.column.getColId());
@@ -89,6 +102,7 @@ const checkSortStatus = () => {
const isMenuButtonVisible = computed(() => {
return (
!isReadOnly.value &&
props.params.allowMenuActions &&
(isHovered.value || isDropdownOpen.value || isFilterOpen.value || hasActiveFilter.value)
);
@@ -214,8 +228,8 @@ onUnmounted(() => {
ref="renameInput"
:model-value="props.params.displayName"
:max-width="columnWidth"
:read-only="props.params.readOnly"
:disabled="props.params.readOnly"
:read-only="isReadOnly"
:disabled="isReadOnly"
class="ag-header-cell-text"
data-test-id="data-table-column-header-text"
@update:model-value="onNameSubmit"
@@ -243,6 +257,7 @@ onUnmounted(() => {
/>
<N8nActionDropdown
ref="actionDropdown"
v-show="isMenuButtonVisible"
data-test-id="data-table-column-header-actions"
:items="columnActionItems"
@@ -0,0 +1,38 @@
<script setup lang="ts">
import { ref } from 'vue';
import { useTimeoutFn } from '@vueuse/core';
import { N8nSpinner, N8nText } from '@n8n/design-system';
import { useI18n } from '@n8n/i18n';
import { TIME } from '@/app/constants/durations';
const i18n = useI18n();
const isVisible = ref(false);
// Mount only during loading. Unmounting cancels the delay for fast requests.
useTimeoutFn(() => {
isVisible.value = true;
}, TIME.SECOND);
</script>
<template>
<div v-if="isVisible" :class="$style.indicator" role="status" data-test-id="data-table-loading">
<N8nSpinner size="small" />
<N8nText size="small">{{ i18n.baseText('generic.loadingEllipsis') }}</N8nText>
</div>
</template>
<style module lang="scss">
.indicator {
position: absolute;
top: var(--spacing--xs);
right: var(--spacing--xs);
// Keep the indicator above AG Grid's loading overlay.
z-index: 3;
display: flex;
align-items: center;
gap: var(--spacing--2xs);
padding: var(--spacing--2xs) var(--spacing--xs);
background: var(--background--surface);
pointer-events: none;
}
</style>
@@ -0,0 +1,143 @@
import { createComponentRenderer } from '@/__tests__/render';
import { useDataTableStore } from '@/features/core/dataTable/dataTable.store';
import type { DataTable, DataTableRow } from '@/features/core/dataTable/dataTable.types';
import { createDeferredPromise } from '@n8n/utils/promise/deferred-promise';
import { createTestingPinia } from '@pinia/testing';
import { fireEvent, waitFor } from '@testing-library/vue';
import { flushPromises } from '@vue/test-utils';
import DataTableTable from './DataTableTable.vue';
const dataTable: DataTable = {
id: 'table-1',
name: 'Products',
projectId: 'project-1',
columns: [{ id: 'name-column', name: 'name', type: 'string', index: 0 }],
createdAt: '2026-01-01T00:00:00Z',
updatedAt: '2026-01-01T00:00:00Z',
sizeBytes: 0,
};
const renderComponent = createComponentRenderer(DataTableTable, {
props: { dataTable },
});
describe('DataTableTable loading', () => {
afterEach(() => {
vi.useRealTimers();
});
it('shows an indicator after one second while the first rows are pending', async () => {
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] });
const pinia = createTestingPinia();
const store = useDataTableStore();
const rows = createDeferredPromise<{ data: DataTableRow[]; count: number }>();
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
const { getByTestId, queryByTestId } = renderComponent({ pinia });
await vi.advanceTimersByTimeAsync(999);
expect(store.fetchDataTableContent).toHaveBeenCalledOnce();
expect(queryByTestId('data-table-loading')).not.toBeInTheDocument();
expect(queryByTestId('data-table-no-rows-overlay')).not.toBeInTheDocument();
await vi.advanceTimersByTimeAsync(1);
expect(getByTestId('data-table-loading')).toHaveTextContent('Loading...');
expect(getByTestId('data-table-grid')).toHaveAttribute('aria-busy', 'true');
rows.resolve({ data: [], count: 0 });
await flushPromises();
expect(queryByTestId('data-table-loading')).not.toBeInTheDocument();
expect(getByTestId('data-table-grid')).toHaveAttribute('aria-busy', 'false');
});
it.each(['success', 'failure'])(
'clears the slow pagination indicator after %s',
async (result) => {
const pinia = createTestingPinia();
const store = useDataTableStore();
vi.mocked(store.fetchDataTableContent).mockResolvedValue({
data: [{ id: 1, name: 'Notebook' }],
count: 40,
});
const { getByTestId, queryByTestId } = renderComponent({ pinia });
await waitFor(() => expect(getByTestId('data-table-grid')).toHaveTextContent('Notebook'));
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] });
const rows = createDeferredPromise<{ data: DataTableRow[]; count: number }>();
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
await fireEvent.click(getByTestId('pagination-next'));
await vi.advanceTimersByTimeAsync(999);
expect(store.fetchDataTableContent).toHaveBeenCalledTimes(2);
expect(queryByTestId('data-table-loading')).not.toBeInTheDocument();
expect(getByTestId('data-table-grid')).toHaveTextContent('Notebook');
await vi.advanceTimersByTimeAsync(1);
expect(getByTestId('data-table-loading')).toBeVisible();
if (result === 'success') {
rows.resolve({ data: [{ id: 21, name: 'Pencil' }], count: 40 });
} else {
rows.reject(new Error('Request failed'));
}
await flushPromises();
expect(queryByTestId('data-table-loading')).not.toBeInTheDocument();
expect(getByTestId('data-table-grid')).toHaveAttribute('aria-busy', 'false');
},
);
it('does not show an empty state or loading pill while the first rows are pending', async () => {
const pinia = createTestingPinia();
const store = useDataTableStore();
const rows = createDeferredPromise<{ data: DataTableRow[]; count: number }>();
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
const { container, queryByTestId } = renderComponent({ pinia });
expect(queryByTestId('data-table-no-rows-overlay')).not.toBeInTheDocument();
await waitFor(() => expect(store.fetchDataTableContent).toHaveBeenCalled());
await flushPromises();
expect(container.querySelector('.ag-header-cell[col-id="name-column"]')).toBeInTheDocument();
expect(queryByTestId('data-table-no-rows-overlay')).not.toBeInTheDocument();
expect(container.querySelector('.ag-overlay-loading-center')).not.toBeInTheDocument();
rows.resolve({ data: [{ id: 1, name: 'Notebook' }], count: 1 });
await waitFor(() => {
expect(container.querySelector('.ag-row[row-id="1"]')).toHaveTextContent('Notebook');
});
expect(queryByTestId('data-table-no-rows-overlay')).not.toBeInTheDocument();
});
it('shows the empty state only after the first row request finishes', async () => {
const pinia = createTestingPinia();
const store = useDataTableStore();
const rows = createDeferredPromise<{ data: DataTableRow[]; count: number }>();
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
const { container, queryByTestId, findByTestId, emitted } = renderComponent({ pinia });
await waitFor(() => expect(store.fetchDataTableContent).toHaveBeenCalled());
await flushPromises();
expect(queryByTestId('data-table-no-rows-overlay')).not.toBeInTheDocument();
rows.resolve({ data: [], count: 0 });
expect(await findByTestId('data-table-no-rows-overlay')).toBeVisible();
expect(container.querySelector('.ag-header-cell[col-id="name-column"]')).toBeInTheDocument();
expect(container.querySelector('.ag-overlay-loading-center')).not.toBeInTheDocument();
expect(emitted('ready')).toHaveLength(1);
});
it('does not signal readiness when the initial rows request fails', async () => {
const pinia = createTestingPinia();
const store = useDataTableStore();
const rows = createDeferredPromise<{ data: DataTableRow[]; count: number }>();
vi.mocked(store.fetchDataTableContent).mockReturnValue(rows.promise);
const { emitted } = renderComponent({ pinia });
await waitFor(() => expect(store.fetchDataTableContent).toHaveBeenCalled());
rows.reject(new Error('Request failed'));
await flushPromises();
expect(emitted('ready')).toBeUndefined();
expect(emitted('loadError')).toHaveLength(1);
});
});
@@ -1,5 +1,5 @@
<script setup lang="ts">
import { computed, ref, useTemplateRef, watch } from 'vue';
import { computed, nextTick, ref, useTemplateRef, watch } from 'vue';
import type {
DataTable,
DataTableColumnCreatePayload,
@@ -27,6 +27,7 @@ import { useDataTableColumnFilters } from '@/features/core/dataTable/composables
import { useI18n } from '@n8n/i18n';
import { GRID_FILTER_CONFIG } from '@/features/core/dataTable/utils/filterMappings';
import { useDebounce } from '@n8n/composables/useDebounce';
import DataTableLoadingIndicator from './DataTableLoadingIndicator.vue';
import { N8nPagination, N8nSelectedItemsInfo } from '@n8n/design-system';
registerAgGridModulesOnce();
@@ -41,6 +42,8 @@ const props = defineProps<Props>();
const emit = defineEmits<{
toggleSave: [value: boolean];
ready: [];
loadError: [];
}>();
const gridContainerRef = useTemplateRef<HTMLDivElement>('gridContainerRef');
@@ -49,6 +52,7 @@ const i18n = useI18n();
const { debounce } = useDebounce();
const rowData = ref<DataTableRow[]>([]);
const hasRecords = computed(() => rowData.value.length > 0);
let isGridReady = false;
const defaultColDef = computed(() => ({
...GRID_FILTER_CONFIG.defaultColDef,
@@ -107,7 +111,7 @@ const dataTableOperations = useDataTableOperations({
setTotalItems,
ensureItemOnPage,
focusFirstEditableCell: agGrid.focusFirstEditableCell,
toggleSave: emit.bind(null, 'toggleSave'),
toggleSave: (value) => emit('toggleSave', value),
currentPage,
pageSize,
currentSortBy: agGrid.currentSortBy,
@@ -117,6 +121,7 @@ const dataTableOperations = useDataTableOperations({
handleCopyFocusedCell: agGrid.handleCopyFocusedCell,
currentFilterJSON,
searchQuery: computed(() => props.search),
readOnly: computed(() => props.readOnly ?? false),
});
async function onDeleteColumnFunction(columnId: string) {
@@ -141,13 +146,40 @@ async function fetchDataTableRowsFunction() {
const initialize = async (params: GridReadyEvent) => {
agGrid.onGridReady(params);
isGridReady = true;
dataTableColumns.loadColumns(props.dataTable.columns);
agGrid.setGridData({ colDefs: dataTableColumns.colDefs.value });
await dataTableOperations.fetchDataTableRows();
const loaded = await dataTableOperations.fetchDataTableRows();
if (!loaded) {
emit('loadError');
return;
}
// Empty tables do not emit AG Grid's first-data-rendered event.
if (!hasRecords.value) {
await nextTick();
emit('ready');
}
};
const customNoRowsOverlay = `<div class="no-rows-overlay ag-overlay-no-rows-center" data-test-id="data-table-no-rows-overlay">${i18n.baseText('dataTable.noRows')}</div>`;
// Keep the loading state to suppress "No rows", without showing a loading pill.
const customLoadingOverlay = '<span aria-hidden="true"></span>';
watch(
() => props.readOnly,
(readOnly) => {
if (!isGridReady) return;
if (readOnly) {
agGrid.gridApi.value.stopEditing(true);
selection.handleClearSelection();
}
// Checkbox renderers cache whether their cells are editable.
agGrid.gridApi.value.refreshCells({ force: true });
},
);
watch([agGrid.currentSortBy, agGrid.currentSortOrder], async () => {
await setCurrentPage(1);
});
@@ -180,6 +212,7 @@ defineExpose({
<div
ref="gridContainerRef"
:class="[$style['grid-container'], { [$style['has-records']]: hasRecords }]"
:aria-busy="dataTableOperations.contentLoading.value"
data-test-id="data-table-grid"
>
<AgGridVue
@@ -190,14 +223,17 @@ defineExpose({
:animate-rows="false"
:theme="n8nTheme"
:suppress-drag-leave-hides-columns="true"
:suppress-movable-columns="props.readOnly"
:loading="dataTableOperations.contentLoading.value"
:row-selection="selection.rowSelection"
:row-selection="selection.rowSelection.value"
:get-row-id="(params: GetRowIdParams) => String(params.data.id)"
:stop-editing-when-cells-lose-focus="true"
:undo-redo-cell-editing="true"
:suppress-multi-sort="true"
:overlay-loading-template="customLoadingOverlay"
:overlay-no-rows-template="customNoRowsOverlay"
@grid-ready="initialize"
@first-data-rendered="emit('ready')"
@cell-value-changed="dataTableOperations.onCellValueChanged"
@column-moved="dataTableOperations.onColumnMoved"
@cell-clicked="agGrid.onCellClicked"
@@ -211,6 +247,7 @@ defineExpose({
@cell-key-down="dataTableOperations.onCellKeyDown"
@filter-changed="onFilterChanged"
/>
<DataTableLoadingIndicator v-if="dataTableOperations.contentLoading.value" />
<div :class="$style.footer">
<N8nPagination
:page="currentPage"
@@ -166,30 +166,6 @@ describe('useDataTableColumns', () => {
expect(colDef.editable).toBe(false);
expect(colDef.width).toBe(100);
});
it('should suppress column move only in read-only mode', () => {
const column: DataTableColumn = { id: 'col1', name: 'Column 1', type: 'string', index: 0 };
const editable = useDataTableColumns({
onDeleteColumn: mockOnDeleteColumn,
onRenameColumn: mockOnRenameColumn,
onAddRowClick: mockOnAddRowClick,
onAddColumn: mockOnAddColumn,
isTextEditorOpen,
readOnly: ref(false),
});
expect(editable.createColumnDef(column).suppressMovable).toBe(false);
const readOnly = useDataTableColumns({
onDeleteColumn: mockOnDeleteColumn,
onRenameColumn: mockOnRenameColumn,
onAddRowClick: mockOnAddRowClick,
onAddColumn: mockOnAddColumn,
isTextEditorOpen,
readOnly: ref(true),
});
expect(readOnly.createColumnDef(column).suppressMovable).toBe(true);
});
});
describe('loadColumns', () => {
@@ -462,7 +438,7 @@ describe('useDataTableColumns', () => {
}
});
it('should disable column menu actions when readOnly is true', () => {
it('should pass the current lock to column menus', () => {
const readOnly = ref(true);
const { createColumnDef } = useDataTableColumns({
onDeleteColumn: mockOnDeleteColumn,
@@ -483,8 +459,11 @@ describe('useDataTableColumns', () => {
const colDef = createColumnDef(column);
expect(colDef.headerComponentParams).toMatchObject({
allowMenuActions: false,
allowMenuActions: true,
});
expect(colDef.headerComponentParams.readOnly()).toBe(true);
readOnly.value = false;
expect(colDef.headerComponentParams.readOnly()).toBe(false);
});
it('should enable column menu actions when readOnly is false', () => {
@@ -532,17 +511,12 @@ describe('useDataTableColumns', () => {
const idColumn = colDefs.value.find((col) => col.colId === 'id');
expect(idColumn).toBeDefined();
// Check cellRendererSelector for AddRowButton params
// The cellRendererSelector should return the AddRowButton with disabled: true
// when the row value is the ADD_ROW_ROW_ID constant
if (idColumn?.cellRendererSelector) {
const addRowParams = { value: ADD_ROW_ROW_ID } as ICellRendererParams;
const result = idColumn.cellRendererSelector(addRowParams);
expect(result).toBeDefined();
if (result && 'params' in result) {
expect(result.params).toMatchObject({
disabled: true,
});
expect(result.params.disabled()).toBe(true);
}
}
});
@@ -567,17 +541,12 @@ describe('useDataTableColumns', () => {
const idColumn = colDefs.value.find((col) => col.colId === 'id');
expect(idColumn).toBeDefined();
// Check cellRendererSelector for AddRowButton params
// The cellRendererSelector should return the AddRowButton with disabled: false
// when the row value is the ADD_ROW_ROW_ID constant
if (idColumn?.cellRendererSelector) {
const addRowParams = { value: ADD_ROW_ROW_ID } as ICellRendererParams;
const result = idColumn.cellRendererSelector(addRowParams);
expect(result).toBeDefined();
if (result && 'params' in result) {
expect(result.params).toMatchObject({
disabled: false,
});
expect(result.params.disabled()).toBe(false);
}
}
});
@@ -601,9 +570,7 @@ describe('useDataTableColumns', () => {
const addColumnCol = colDefs.value.find((col) => col.colId === 'add-column');
expect(addColumnCol).toBeDefined();
expect(addColumnCol?.headerComponentParams).toMatchObject({
disabled: true,
});
expect(addColumnCol?.headerComponentParams.disabled()).toBe(true);
});
it('should enable AddColumn button when readOnly is false', () => {
@@ -625,9 +592,7 @@ describe('useDataTableColumns', () => {
const addColumnCol = colDefs.value.find((col) => col.colId === 'add-column');
expect(addColumnCol).toBeDefined();
expect(addColumnCol?.headerComponentParams).toMatchObject({
disabled: false,
});
expect(addColumnCol?.headerComponentParams.disabled()).toBe(false);
});
it('should work without readOnly parameter (defaults to false)', () => {
@@ -69,15 +69,12 @@ export const useDataTableColumns = ({
!isOversizedValue(params.data?.[col.name]),
resizable: true,
lockPinned: true,
// Dragging a column triggers a server-side move, so block it in
// read-only mode.
suppressMovable: readOnly.value,
headerComponent: ColumnHeader,
headerComponentParams: {
onDelete: onDeleteColumn,
onRename: onRenameColumn,
allowMenuActions: !readOnly.value,
readOnly: readOnly.value,
allowMenuActions: true,
readOnly: () => readOnly.value,
},
cellEditorPopup: false,
cellDataType: mapToAGCellType(col.type),
@@ -165,7 +162,7 @@ export const useDataTableColumns = ({
if (params.value === ADD_ROW_ROW_ID) {
return {
component: AddRowButton,
params: { onClick: onAddRowClick, disabled: readOnly.value },
params: { onClick: onAddRowClick, disabled: () => readOnly.value },
};
}
return undefined;
@@ -208,7 +205,7 @@ export const useDataTableColumns = ({
minWidth: MIN_ADD_COLUMN_BUTTON_COLUMN_WIDTH,
flex: 1,
headerComponent: AddColumnButton,
headerComponentParams: { onAddColumn, disabled: readOnly.value },
headerComponentParams: { onAddColumn, disabled: () => readOnly.value },
},
),
];
@@ -17,6 +17,7 @@ import { useMessage } from '@/app/composables/useMessage';
import { useToast } from '@n8n/composables/useToast';
import { useTelemetry } from '@n8n/composables/useTelemetry';
import { MODAL_CONFIRM } from '@/app/constants';
import { createDeferredPromise } from '@n8n/utils/promise/deferred-promise';
import type { DataTableRow } from '@/features/core/dataTable/dataTable.types';
vi.mock('@/features/core/dataTable/dataTable.store', () => ({
@@ -129,6 +130,44 @@ describe('useDataTableOperations', () => {
vi.clearAllMocks();
});
describe('column editing lock', () => {
it('blocks column mutations after editing is locked', async () => {
const readOnly = ref(false);
params.colDefs.value = [{ colId: 'name-column', field: 'name', headerName: 'name' }];
const operations = useDataTableOperations({ ...params, readOnly });
readOnly.value = true;
expect(await operations.onAddColumn({ name: 'description', type: 'string' })).toEqual({
success: false,
});
await operations.onRenameColumn('name-column', 'title');
await operations.onDeleteColumn('name-column');
expect(dataTableStore.addDataTableColumn).not.toHaveBeenCalled();
expect(dataTableStore.renameDataTableColumn).not.toHaveBeenCalled();
expect(dataTableStore.deleteDataTableColumn).not.toHaveBeenCalled();
expect(confirmMock).not.toHaveBeenCalled();
expect(params.setGridData).not.toHaveBeenCalled();
});
it('blocks column deletion when editing is locked during confirmation', async () => {
const readOnly = ref(false);
params.colDefs.value = [{ colId: 'name-column', field: 'name', headerName: 'name' }];
const confirmation = createDeferredPromise<typeof MODAL_CONFIRM>();
confirmMock.mockReturnValue(confirmation.promise);
const operations = useDataTableOperations({ ...params, readOnly });
const deletion = operations.onDeleteColumn('name-column');
expect(confirmMock).toHaveBeenCalled();
readOnly.value = true;
confirmation.resolve(MODAL_CONFIRM);
await deletion;
expect(dataTableStore.deleteDataTableColumn).not.toHaveBeenCalled();
expect(params.deleteGridColumn).not.toHaveBeenCalled();
});
});
describe('onAddColumn', () => {
it('should add column when column is added successfully', async () => {
const returnedColumn = { name: 'test', type: 'string' } as const;
@@ -300,6 +339,62 @@ describe('useDataTableOperations', () => {
getColId: () => colId,
});
it('ignores queued move events after editing is locked', async () => {
const readOnly = ref(false);
const { onColumnMoved } = useDataTableOperations({ ...params, readOnly });
readOnly.value = true;
await onColumnMoved({
finished: true,
source: 'uiColumnMoved',
toIndex: 4,
column: createMockColumn('col1'),
} as unknown as ColumnMovedEvent);
expect(dataTableStore.moveDataTableColumn).not.toHaveBeenCalled();
expect(params.moveGridColumn).not.toHaveBeenCalled();
});
it.each(['success', 'failure'])(
'reconciles a move started before editing was locked: %s',
async (result) => {
const readOnly = ref(false);
const response = createDeferredPromise<boolean>();
vi.mocked(dataTableStore.moveDataTableColumn).mockReturnValue(response.promise);
const moveColumnByIndex = vi.fn();
params.gridApi.value = { moveColumnByIndex } as unknown as GridApi;
params.colDefs.value = [{ colId: 'col1', field: 'name' }];
const { onColumnMoved } = useDataTableOperations({ ...params, readOnly });
const move = onColumnMoved({
finished: true,
source: 'uiColumnMoved',
toIndex: 4,
column: createMockColumn('col1'),
} as unknown as ColumnMovedEvent);
expect(dataTableStore.moveDataTableColumn).toHaveBeenCalled();
readOnly.value = true;
if (result === 'success') {
response.resolve(true);
} else {
response.reject(new Error('Move failed'));
}
await move;
if (result === 'success') {
expect(params.moveGridColumn).toHaveBeenCalledWith(0, 2);
expect(moveColumnByIndex).not.toHaveBeenCalled();
} else {
expect(params.moveGridColumn).not.toHaveBeenCalled();
expect(moveColumnByIndex).toHaveBeenCalledWith(4, 1);
expect(showErrorMock).toHaveBeenCalledWith(
expect.any(Error),
'dataTable.moveColumn.error',
);
}
},
);
it('should return early when event is not finished', async () => {
const { onColumnMoved } = useDataTableOperations(params);
const moveEvent = {
@@ -711,7 +806,7 @@ describe('useDataTableOperations', () => {
currentFilterJSON,
});
await fetchDataTableRows();
expect(await fetchDataTableRows()).toBe(true);
expect(fetchDataTableContentMock).toHaveBeenCalledWith(
'test',
@@ -778,7 +873,7 @@ describe('useDataTableOperations', () => {
const { fetchDataTableRows } = useDataTableOperations({ ...params, rowData });
await fetchDataTableRows();
expect(await fetchDataTableRows()).toBe(false);
expect(showErrorMock).toHaveBeenCalledWith(fetchError, 'dataTable.fetchContent.error');
expect(rowData.value).toEqual([{ id: 1 }]);
@@ -50,6 +50,7 @@ export type UseDataTableOperationsParams = {
currentSortOrder: Ref<string | null>;
currentFilterJSON?: Ref<string | undefined>;
searchQuery?: Ref<string | undefined>;
readOnly?: Ref<boolean>;
handleClearSelection: () => void;
selectedRowIds: Ref<Set<number>>;
handleCopyFocusedCell: (params: CellKeyDownEvent<DataTableRow>) => Promise<void>;
@@ -77,6 +78,7 @@ export const useDataTableOperations = ({
currentSortOrder,
currentFilterJSON,
searchQuery,
readOnly = ref(false),
handleClearSelection,
selectedRowIds,
handleCopyFocusedCell,
@@ -85,7 +87,7 @@ export const useDataTableOperations = ({
const toast = useToast();
const message = useMessage();
const dataTableStore = useDataTableStore();
const contentLoading = ref(false);
const contentLoading = ref(true);
const telemetry = useTelemetry();
const dataTableTypes = useDataTableTypes();
@@ -112,6 +114,7 @@ export const useDataTableOperations = ({
};
async function onDeleteColumn(columnId: string) {
if (readOnly.value) return;
const columnToDelete = colDefs.value.find((col) => col.colId === columnId);
if (!columnToDelete) return;
@@ -126,7 +129,7 @@ export const useDataTableOperations = ({
},
);
if (promptResponse !== MODAL_CONFIRM) {
if (promptResponse !== MODAL_CONFIRM || readOnly.value) {
return;
}
@@ -154,6 +157,7 @@ export const useDataTableOperations = ({
}
async function onRenameColumn(columnId: string, newName: string): Promise<void> {
if (readOnly.value) return;
const columnToRename = colDefs.value.find((col) => col.colId === columnId);
if (!columnToRename) return;
@@ -203,6 +207,7 @@ export const useDataTableOperations = ({
}
async function onAddColumn(column: DataTableColumnCreatePayload): Promise<AddColumnResponse> {
if (readOnly.value) return { success: false };
try {
const newColumn = await dataTableStore.addDataTableColumn(dataTableId, projectId, column);
addGridColumn(newColumn);
@@ -228,6 +233,7 @@ export const useDataTableOperations = ({
const onColumnMoved = async (moveEvent: ColumnMovedEvent) => {
if (
readOnly.value ||
!moveEvent.finished ||
moveEvent.source !== 'uiColumnMoved' ||
moveEvent.toIndex === undefined ||
@@ -328,7 +334,7 @@ export const useDataTableOperations = ({
}
};
async function fetchDataTableRows() {
async function fetchDataTableRows(): Promise<boolean> {
try {
contentLoading.value = true;
@@ -345,8 +351,10 @@ export const useDataTableOperations = ({
setTotalItems(fetchedRows.count);
setGridData({ rowData: rowData.value });
handleClearSelection();
return true;
} catch (error) {
toast.showError(error, i18n.baseText('dataTable.fetchContent.error'));
return false;
} finally {
contentLoading.value = false;
}
@@ -12,13 +12,17 @@ export const useDataTableSelection = ({
const selectedRowIds = ref<Set<number>>(new Set());
const selectedCount = computed(() => selectedRowIds.value.size);
const rowSelection: RowSelectionOptions | 'single' | 'multiple' = {
mode: 'multiRow',
enableClickSelection: false,
checkboxes: (params) => params.data?.id !== ADD_ROW_ROW_ID,
isRowSelectable: (params) => !readOnly.value && params.data?.id !== ADD_ROW_ROW_ID,
hideDisabledCheckboxes: false,
};
const rowSelection = computed<RowSelectionOptions>(() => {
// AG Grid updates row selectability when the callback changes.
const locked = readOnly.value;
return {
mode: 'multiRow',
enableClickSelection: false,
checkboxes: (params) => params.data?.id !== ADD_ROW_ROW_ID,
isRowSelectable: (params) => !locked && params.data?.id !== ADD_ROW_ROW_ID,
hideDisabledCheckboxes: false,
};
});
const onSelectionChanged = () => {
const selectedNodes = gridApi.value.getSelectedNodes();