Skip to content

Commit

Permalink
Fix the columns used in the partition selector
Browse files Browse the repository at this point in the history
- Was referencing the columns from the original table instead of the actual table
- Was causing a mismatch when fetching the viewport, where it would just stall instead of throwing an error
  • Loading branch information
mofojed committed Jan 16, 2024
1 parent f4cba3a commit 02542dd
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 24 deletions.
34 changes: 22 additions & 12 deletions packages/iris-grid/src/IrisGridPartitionSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,12 @@ class IrisGridPartitionSelector extends Component<
)
);

this.setState({ isLoading: false, keysTable, partitionTables }, () => {
this.updatePartitionFilters();
const partitionFilters = this.getPartitionFilters(partitionTables);
this.setState({
isLoading: false,
keysTable,
partitionFilters,
partitionTables,
});
} catch (e) {
if (!PromiseUtils.isCanceled(e)) {
Expand Down Expand Up @@ -167,7 +171,7 @@ class IrisGridPartitionSelector extends Component<
const partitionFilters = newPartitions
.slice(0, index + 1)
.map((partition, i) => {
const partitionColumn = model.partitionColumns[i];
const partitionColumn = t.columns[i];
return partitionColumn
.filter()
.eq(
Expand All @@ -180,14 +184,11 @@ class IrisGridPartitionSelector extends Component<
t.applyFilter(partitionFilters);
t.setViewport(0, 0, t.columns);
const data = await this.pending.add(t.getViewportData());
t.close();

const newConfig: PartitionConfig = {
partitions: model.partitionColumns.map(column =>
data.rows[0].get(column)
),
partitions: t.columns.map(column => data.rows[0].get(column)),
mode: 'partition',
};
t.close();
this.sendUpdate(newConfig);
} catch (e) {
if (!PromiseUtils.isCanceled(e)) {
Expand Down Expand Up @@ -230,15 +231,24 @@ class IrisGridPartitionSelector extends Component<
const { partitionTables } = this.state;
assertNotNull(partitionTables);

const { model, partitionConfig } = this.props;
const { mode, partitions } = partitionConfig;
const { partitionConfig } = this.props;
const { mode } = partitionConfig;
log.debug('updatePartitionFilters', partitionConfig);
if (mode !== 'partition') {
// We only need to update the filters if the mode is `partitions`
// In the other modes, we disable the dropdowns anyway
return;
}

const partitionFilters = this.getPartitionFilters(partitionTables);
this.setState({ partitionFilters });
}

getPartitionFilters(partitionTables: Table[]): FilterCondition[][] {
const { model, partitionConfig } = this.props;
const { partitions } = partitionConfig;
log.debug('getPartitionFilters', partitionConfig);

if (partitions.length !== partitionTables.length) {
throw new Error(
`Invalid partition config set. Expected ${partitionTables.length} partitions, but got ${partitions.length}`
Expand Down Expand Up @@ -269,7 +279,7 @@ class IrisGridPartitionSelector extends Component<
partitionFilters.push(partitionFilter);
}
}
this.setState({ partitionFilters });
return partitionFilters;
}

getCachedChangeCallback = memoizee(
Expand All @@ -293,7 +303,7 @@ class IrisGridPartitionSelector extends Component<
<TableDropdown
className="custom-select-sm"
table={partitionTables?.[index]}
column={column}
column={partitionTables?.[index]?.columns[index]}
filter={partitionFilters?.[index]}
onChange={this.getCachedChangeCallback(index)}
selectedValue={mode === 'partition' ? partitions[index] : undefined}
Expand Down
31 changes: 23 additions & 8 deletions packages/iris-grid/src/IrisGridUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ import IrisGridModel from './IrisGridModel';
import type AdvancedSettingsType from './sidebar/AdvancedSettingsType';
import AdvancedSettings from './sidebar/AdvancedSettings';
import ColumnHeaderGroup from './ColumnHeaderGroup';
import { PartitionConfig } from './PartitionedGridModel';
import {
isPartitionedGridModelProvider,
PartitionConfig,
} from './PartitionedGridModel';

const log = Log.module('IrisGridUtils');

Expand Down Expand Up @@ -1177,6 +1180,10 @@ class IrisGridUtils {
assertNotNull(metrics);
const { userColumnWidths, userRowHeights } = metrics;
const { columns } = model;
const partitionColumns = isPartitionedGridModelProvider(model)
? model.partitionColumns
: [];

// Return value will be serialized, should not contain undefined
return {
advancedFilters: this.dehydrateAdvancedFilters(columns, advancedFilters),
Expand Down Expand Up @@ -1210,7 +1217,10 @@ class IrisGridUtils {
children: item.children,
color: item.color,
})),
partitionConfig: this.dehydratePartitionConfig(columns, partitionConfig),
partitionConfig: this.dehydratePartitionConfig(
partitionColumns,
partitionConfig
),
};
}

Expand Down Expand Up @@ -1250,7 +1260,9 @@ class IrisGridUtils {
partitionConfig,
} = irisGridState;
const { columns, formatter } = model;

const partitionColumns = isPartitionedGridModelProvider(model)
? model.partitionColumns
: [];
return {
advancedFilters: this.hydrateAdvancedFilters(
columns,
Expand Down Expand Up @@ -1306,7 +1318,10 @@ class IrisGridUtils {
model,
columnHeaderGroups ?? model.layoutHints?.columnGroups ?? []
).groups,
partitionConfig: this.hydratePartitionConfig(columns, partitionConfig),
partitionConfig: this.hydratePartitionConfig(
partitionColumns,
partitionConfig
),
};
}

Expand Down Expand Up @@ -1501,7 +1516,7 @@ class IrisGridUtils {
}

dehydratePartitionConfig(
columns: readonly Column[],
partitionColumns: readonly Column[],
partitionConfig: PartitionConfig | undefined
): PartitionConfig | undefined {
if (partitionConfig == null) {
Expand All @@ -1511,13 +1526,13 @@ class IrisGridUtils {
return {
...partitionConfig,
partitions: partitionConfig.partitions.map((partition, index) =>
this.dehydrateValue(partition, columns[index].type)
this.dehydrateValue(partition, partitionColumns[index].type)
),
};
}

hydratePartitionConfig(
columns: readonly Column[],
partitionColumns: readonly Column[],
partitionConfig: PartitionConfig | undefined
): PartitionConfig | undefined {
if (partitionConfig == null) {
Expand All @@ -1527,7 +1542,7 @@ class IrisGridUtils {
return {
...partitionConfig,
partitions: partitionConfig.partitions.map((partition, index) =>
this.hydrateValue(partition, columns[index].type)
this.hydrateValue(partition, partitionColumns[index].type)
),
};
}
Expand Down
9 changes: 5 additions & 4 deletions packages/jsapi-components/src/TableDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export type TableDropdownProps = {
/** Table to use as the source of data. Does not own the table, does not close it on unmount. */
table?: Table;

/** Column to read data from the table */
column: Column;
/** Column to read data from the table. Defaults to the first column in the table if it's not provided. */
column?: Column;

/** Triggered when the dropdown selection has changed */
onChange: (value: unknown) => void;
Expand Down Expand Up @@ -78,15 +78,16 @@ export function TableDropdown({
return undefined;
}

const tableColumn = column ?? table.columns[0];
// Need to set a viewport on the table and start listening to get the values to populate the dropdown
table.applyFilter(filter as FilterCondition[]);
const subscription = table.setViewport(0, maxSize, [column]);
const subscription = table.setViewport(0, maxSize, [tableColumn]);

subscription.addEventListener(
dh.Table.EVENT_UPDATED,
(event: CustomEvent<ViewportData>) => {
const { detail } = event;
const newValues = detail.rows.map(row => row.get(column));
const newValues = detail.rows.map(row => row.get(tableColumn));
setValues(newValues);
}
);
Expand Down

0 comments on commit 02542dd

Please sign in to comment.