Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Table: Column order not preserved in URL #978

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Components/ServiceScene/LogsTableScene.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 });
}
};
Expand Down
112 changes: 111 additions & 1 deletion src/services/comparison.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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);
});
});
8 changes: 8 additions & 0 deletions src/services/comparison.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
21 changes: 21 additions & 0 deletions tests/exploreServicesBreakDown.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading