diff --git a/src/Components/ServiceScene/LogsTableScene.tsx b/src/Components/ServiceScene/LogsTableScene.tsx index 554dd578..ed21e7a9 100644 --- a/src/Components/ServiceScene/LogsTableScene.tsx +++ b/src/Components/ServiceScene/LogsTableScene.tsx @@ -7,7 +7,7 @@ import { PanelChrome, useStyles2 } from '@grafana/ui'; import { LogsPanelHeaderActions } from '../Table/LogsHeaderActions'; import { css } from '@emotion/css'; import { addAdHocFilter } from './Breakdowns/AddToFiltersButton'; -import { areArraysEqual } from '../../services/comparison'; +import { areArraysStrictlyEqual } from '../../services/comparison'; import { getLogsPanelFrame } from './ServiceScene'; import { getVariableForLabel } from '../../services/fields'; import { PanelMenu } from '../Panels/PanelMenu'; @@ -53,7 +53,7 @@ export class LogsTableScene extends SceneObjectBase { // Define callback function to update url columns in react const setUrlColumns = (urlColumns: string[]) => { - if (!areArraysEqual(urlColumns, parentModel.state.urlColumns)) { + if (!areArraysStrictlyEqual(urlColumns, parentModel.state.urlColumns)) { parentModel.setState({ urlColumns }); } }; diff --git a/src/services/comparison.test.ts b/src/services/comparison.test.ts index 7fe9bf82..04482764 100644 --- a/src/services/comparison.test.ts +++ b/src/services/comparison.test.ts @@ -1,4 +1,4 @@ -import { areArraysEqual } from './comparison'; +import { areArraysEqual, areArraysStrictlyEqual } from './comparison'; describe('areArraysEqual', () => { it('should return false when one array is undefined and the other is empty', () => { @@ -110,3 +110,113 @@ describe('areArraysEqual', () => { ).toBe(false); }); }); +describe('areArraysStrictlyEqual', () => { + it('should return false when one array is undefined and the other is empty', () => { + expect(areArraysStrictlyEqual([], undefined)).toBe(false); + }); + it('should return false when one array contains empty object', () => { + expect(areArraysStrictlyEqual([''], [{}])).toBe(false); + }); + it('should return true when both arrays are empty', () => { + expect(areArraysStrictlyEqual([], [])).toBe(true); + }); + it('should return true when both arrays are undefined', () => { + expect(areArraysStrictlyEqual(undefined, undefined)).toBe(true); + }); + it('should return false when arrays are same, but in different order', () => { + expect(areArraysStrictlyEqual(['a', 'b', 'c'], ['b', 'c', 'a'])).toBe(false); + }); + it('should return false when nested objects have different orders', () => { + expect( + areArraysStrictlyEqual( + [ + { d: 'd', c: 'c' }, + { b: 'b', a: 'a' }, + ], + [ + { a: 'a', b: 'b' }, + { c: 'c', d: 'd' }, + ] + ) + ).toBe(false); + }); + + it('should return true when nested objects are same', () => { + expect( + areArraysEqual( + [ + { + a: { b: 'b', a: 'a' }, + }, + { b: 'b', a: 'a' }, + ], + [ + { + a: { a: 'a', b: 'b' }, + }, + { a: 'a', b: 'b' }, + ] + ) + ).toBe(true); + }); + + it('should return false when nested objects are different', () => { + expect( + areArraysEqual( + [ + { + a: { b: 'b', a: 'a' }, + }, + { + b: 'b', + a: 'a', + }, + ], + [ + { + a: { + c: 'b', + a: 'a', + }, + }, + { + c: 'c', + d: 'd', + }, + ] + ) + ).toBe(false); + }); + + it('should return false when nested objects contain subsets', () => { + expect( + areArraysEqual( + [ + { + a: { + b: 'b', + a: 'a', + }, + }, + { + b: 'b', + a: 'a', + }, + ], + [ + { + a: { + c: 'b', + a: 'a', + e: 'e', + }, + }, + { + c: 'c', + d: 'd', + }, + ] + ) + ).toBe(false); + }); +}); diff --git a/src/services/comparison.ts b/src/services/comparison.ts index ccad6dba..ab6f7b1d 100644 --- a/src/services/comparison.ts +++ b/src/services/comparison.ts @@ -21,3 +21,11 @@ export const areArraysEqual = (arr1: any[] | undefined, arr2: any[] | undefined) return _.isEqual(set1, set2); }; + +export const areArraysStrictlyEqual = (arr1: any[] | undefined, arr2: any[] | undefined) => { + // If one array is undefined, and the other is empty, they will cast to the same set. + if (typeof arr1 !== typeof arr2) { + return false; + } + return _.isEqual(arr1, arr2); +}; diff --git a/tests/exploreServicesBreakDown.spec.ts b/tests/exploreServicesBreakDown.spec.ts index fc26226f..d09b9b2b 100644 --- a/tests/exploreServicesBreakDown.spec.ts +++ b/tests/exploreServicesBreakDown.spec.ts @@ -111,6 +111,27 @@ test.describe('explore services breakdown page', () => { await expect(page.getByText(`drop __error__, __error_details__`)).toBeVisible(); }); + test(`should persist column ordering`, async ({ page }) => { + const table = page.getByTestId(testIds.table.wrapper); + await explorePage.goToLogsTab(); + // Switch to table view + await explorePage.getTableToggleLocator().click(); + + // Assert table column order + await expect(table.getByRole('columnheader').nth(0)).toContainText('timestamp'); + await expect(table.getByRole('columnheader').nth(0)).not.toContainText('body'); + await expect(table.getByRole('columnheader').nth(1)).toContainText('body'); + + // Open the menu for "Line" + await page.getByRole('button', { name: 'Show menu' }).nth(1).click(); + await page.getByText('Move left').click(); + await expect(table.getByRole('columnheader').nth(0)).toContainText('body'); + + // Refresh the page to see if the columns were saved in the url state + await page.reload(); + await expect(table.getByRole('columnheader').nth(0)).toContainText('body'); + }); + test(`should add ${levelName} filter on table click`, async ({ page }) => { // Switch to table view await explorePage.getTableToggleLocator().click();