Table: Column order not preserved in URL (#978)

* fix: update url state when order changes

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<LogsTableSceneState> {
 
     // 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 });
       }
     };

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);
+  });
+});

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);
+};

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();