Skip to content

Commit

Permalink
fix: Invalid migration of legacy partitions (#1892)
Browse files Browse the repository at this point in the history
- We didn't check if the partition was `null` before adding it to an
array
- Was setting a partition of `[null]` for all migrated tables
- Fix up unit tests to actually check the results instead of just
checking against the object passed in
  • Loading branch information
mofojed authored Mar 27, 2024
1 parent 78c1af1 commit 96298f6
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 12 deletions.
23 changes: 14 additions & 9 deletions packages/iris-grid/src/IrisGridUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import IrisGridTestUtils from './IrisGridTestUtils';
import IrisGridUtils, {
DehydratedSort,
LegacyDehydratedSort,
isPanelStateV1,
} from './IrisGridUtils';

const irisGridUtils = new IrisGridUtils(dh);
Expand Down Expand Up @@ -682,6 +681,7 @@ describe('hydration methods', () => {
partition: null,
advancedSettings: [],
},
[],
],
[
'hydrateIrisGridPanelStateV1 one selected partition',
Expand All @@ -690,6 +690,7 @@ describe('hydration methods', () => {
partition: 'a',
advancedSettings: [],
},
['a'],
],
[
'hydrateIrisGridPanelStateV2 no partition columns',
Expand All @@ -698,6 +699,7 @@ describe('hydration methods', () => {
partitions: [],
advancedSettings: [],
},
[],
],
[
'hydrateIrisGridPanelStateV2 two unselected columns',
Expand All @@ -706,6 +708,7 @@ describe('hydration methods', () => {
partitions: [null, null],
advancedSettings: [],
},
[null, null],
],
[
'hydrateIrisGridPanelStateV2 two selected columns',
Expand All @@ -714,6 +717,7 @@ describe('hydration methods', () => {
partitions: ['a', 'b'],
advancedSettings: [],
},
['a', 'b'],
],
[
'hydrateIrisGridPanelStateV2 mixed selection columns',
Expand All @@ -722,6 +726,7 @@ describe('hydration methods', () => {
partitions: [null, 'b', null],
advancedSettings: [],
},
[null, 'b', null],
],
[
'hydrateIrisGridPanelStateV2 mixed selection columns',
Expand All @@ -730,14 +735,14 @@ describe('hydration methods', () => {
partitions: ['a', null, 'b'],
advancedSettings: [],
},
['a', null, 'b'],
],
])('%s partitions and columns match', (_label, panelState) => {
const result = IrisGridUtils.hydrateIrisGridPanelState(model, panelState);
expect(result.isSelectingPartition).toBe(panelState.isSelectingPartition);
if (isPanelStateV1(panelState)) {
expect(result.partitions).toEqual([panelState.partition]);
} else {
expect(result.partitions).toEqual(panelState.partitions);
])(
'%s partitions and columns match',
(_label, panelState, expectedPartitions) => {
const result = IrisGridUtils.hydrateIrisGridPanelState(model, panelState);
expect(result.isSelectingPartition).toBe(panelState.isSelectingPartition);
expect(result.partitions).toEqual(expectedPartitions);
}
});
);
});
9 changes: 6 additions & 3 deletions packages/iris-grid/src/IrisGridUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,12 @@ class IrisGridUtils {
} {
const { isSelectingPartition, advancedSettings } = irisGridPanelState;

const partitions = isPanelStateV2(irisGridPanelState)
? irisGridPanelState.partitions
: [irisGridPanelState.partition];
let partitions: (string | null)[] = [];
if (isPanelStateV2(irisGridPanelState)) {
partitions = irisGridPanelState.partitions;
} else if (irisGridPanelState.partition != null) {
partitions = [irisGridPanelState.partition];
}

return {
isSelectingPartition,
Expand Down

0 comments on commit 96298f6

Please sign in to comment.