Skip to content

Commit c225374

Browse files
authored
feat: add median agg for totals table (#2389)
https://deephaven.atlassian.net/browse/DH-18856 Cherry pick back to Grizzly.
1 parent fe8b874 commit c225374

6 files changed

Lines changed: 133 additions & 26 deletions

File tree

packages/iris-grid/src/IrisGrid.tsx

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3397,15 +3397,32 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
33973397
/**
33983398
* User added, removed, or changed the order of aggregations, or position
33993399
* @param aggregationSettings The new aggregation settings
3400+
* @param added The aggregations that were added
3401+
* @param removed The aggregations that were removed
34003402
*/
3401-
handleAggregationsChange(aggregationSettings: AggregationSettings): void {
3402-
log.debug('handleAggregationsChange', aggregationSettings);
3403-
3404-
this.startLoading(
3405-
`Aggregating ${aggregationSettings.aggregations
3406-
.map(a => a.operation)
3407-
.join(', ')}...`
3403+
handleAggregationsChange(
3404+
aggregationSettings: AggregationSettings,
3405+
added: AggregationOperation[] = [],
3406+
removed: AggregationOperation[] = []
3407+
): void {
3408+
log.debug('handleAggregationsChange', aggregationSettings, added, removed);
3409+
const { rollupConfig } = this.state;
3410+
const isRollup = (rollupConfig?.columns?.length ?? 0) > 0;
3411+
// Do not start loading if this is rollup and added / removed aggregations are prohibited for rollups
3412+
const changes = [...added, ...removed];
3413+
const shouldStartLoading = !(
3414+
isRollup &&
3415+
changes.length > 0 &&
3416+
changes.every(op => AggregationUtils.isRollupProhibited(op))
34083417
);
3418+
3419+
if (shouldStartLoading) {
3420+
this.startLoading(
3421+
`Aggregating ${aggregationSettings.aggregations
3422+
.map(a => a.operation)
3423+
.join(', ')}...`
3424+
);
3425+
}
34093426
this.setState({ aggregationSettings });
34103427
}
34113428

@@ -3415,8 +3432,16 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
34153432
*/
34163433
handleAggregationChange(aggregation: Aggregation): void {
34173434
log.debug('handleAggregationChange', aggregation);
3435+
const { rollupConfig } = this.state;
3436+
const isRollup = (rollupConfig?.columns?.length ?? 0) > 0;
3437+
// Do not start loading if this is rollup and the aggregation is prohibited for rollups
3438+
const shouldStartLoading = !(
3439+
isRollup && AggregationUtils.isRollupProhibited(aggregation.operation)
3440+
);
34183441

3419-
this.startLoading(`Aggregating ${aggregation.operation}...`);
3442+
if (shouldStartLoading) {
3443+
this.startLoading(`Aggregating ${aggregation.operation}...`);
3444+
}
34203445

34213446
this.setState(({ aggregationSettings }) => ({
34223447
selectedAggregation: aggregation,
@@ -4656,6 +4681,7 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
46564681
isRollup={isRollup}
46574682
onChange={this.handleAggregationsChange}
46584683
onEdit={this.handleAggregationEdit}
4684+
dh={model.dh}
46594685
/>
46604686
);
46614687
case OptionType.AGGREGATION_EDIT:

packages/iris-grid/src/IrisGridUtils.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,10 @@ class IrisGridUtils {
822822
includeDescriptions = true,
823823
} = config ?? {};
824824
const { aggregations = [] } = aggregationSettings ?? {};
825-
const aggregationColumns = aggregations.map(
825+
const filteredAggregations = aggregations.filter(
826+
({ operation }) => !AggregationUtils.isRollupProhibited(operation)
827+
);
828+
const aggregationColumns = filteredAggregations.map(
826829
({ operation, selected, invert }) =>
827830
AggregationUtils.isRollupOperation(operation)
828831
? []
@@ -836,8 +839,8 @@ class IrisGridUtils {
836839

837840
const aggregationMap = {} as Record<AggregationOperation, string[]>;
838841
// Aggregation columns should show first, add them first
839-
for (let i = 0; i < aggregations.length; i += 1) {
840-
aggregationMap[aggregations[i].operation] = aggregationColumns[i];
842+
for (let i = 0; i < filteredAggregations.length; i += 1) {
843+
aggregationMap[filteredAggregations[i].operation] = aggregationColumns[i];
841844
}
842845

843846
if (showNonAggregatedColumns) {

packages/iris-grid/src/sidebar/aggregations/AggregationOperation.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ enum AggregationOperation {
66
ABS_SUM = 'AbsSum',
77
VAR = 'Var',
88
AVG = 'Avg',
9+
MEDIAN = 'Median',
910
STD = 'Std',
1011
FIRST = 'First',
1112
LAST = 'Last',

packages/iris-grid/src/sidebar/aggregations/AggregationUtils.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export const SELECTABLE_OPTIONS = [
99
AggregationOperation.MAX,
1010
AggregationOperation.VAR,
1111
AggregationOperation.AVG,
12+
AggregationOperation.MEDIAN,
1213
AggregationOperation.STD,
1314
AggregationOperation.FIRST,
1415
AggregationOperation.LAST,
@@ -31,6 +32,20 @@ export const isRollupOperation = (type: AggregationOperation): boolean => {
3132
}
3233
};
3334

35+
/**
36+
* Check if an operation is not supported for a rollup/table grouping
37+
* @param type The operation to check
38+
* @returns True if this operation is not supported for a rollup/table grouping
39+
*/
40+
export const isRollupProhibited = (type: AggregationOperation): boolean => {
41+
switch (type) {
42+
case AggregationOperation.MEDIAN:
43+
return true;
44+
default:
45+
return false;
46+
}
47+
};
48+
3449
/**
3550
* Check if an operation is valid against the given column type
3651
* @param operationType The operation to check
@@ -48,6 +63,7 @@ export const isValidOperation = (
4863
case AggregationOperation.DISTINCT:
4964
case AggregationOperation.UNIQUE:
5065
return true;
66+
case AggregationOperation.MEDIAN:
5167
case AggregationOperation.MIN:
5268
case AggregationOperation.MAX:
5369
return (
@@ -87,6 +103,7 @@ export const getOperationColumnNames = (
87103

88104
export default {
89105
isRollupOperation,
106+
isRollupProhibited,
90107
filterValidColumns,
91108
getOperationColumnNames,
92109
};

packages/iris-grid/src/sidebar/aggregations/Aggregations.test.tsx

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import React from 'react';
22
import { render, RenderResult, screen } from '@testing-library/react';
33
import userEvent from '@testing-library/user-event';
4+
import type { dh as DhType } from '@deephaven/jsapi-types';
45
import Aggregations, { Aggregation } from './Aggregations';
56
import AggregationOperation from './AggregationOperation';
67
import { SELECTABLE_OPTIONS } from './AggregationUtils';
@@ -13,6 +14,25 @@ function makeAggregation({
1314
return { operation, selected, invert };
1415
}
1516

17+
const MOCK_DH = {
18+
AggregationOperation: {
19+
SUM: 'SUM',
20+
ABS_SUM: 'ABS_SUM',
21+
MIN: 'MIN',
22+
MAX: 'MAX',
23+
VAR: 'VAR',
24+
AVG: 'AVG',
25+
MEDIAN: 'MEDIAN',
26+
STD: 'STD',
27+
FIRST: 'FIRST',
28+
LAST: 'LAST',
29+
COUNT_DISTINCT: 'COUNT_DISTINCT',
30+
DISTINCT: 'DISTINCT',
31+
COUNT: 'COUNT',
32+
UNIQUE: 'UNIQUE',
33+
},
34+
} as unknown as typeof DhType;
35+
1636
function mountAggregations({
1737
settings = { aggregations: [] as Aggregation[], showOnTop: false },
1838
onChange = jest.fn(),
@@ -25,6 +45,7 @@ function mountAggregations({
2545
onChange={onChange}
2646
onEdit={onEdit}
2747
isRollup={isRollup}
48+
dh={MOCK_DH}
2849
/>
2950
);
3051
}
@@ -37,6 +58,7 @@ it('shows all operations in select when no aggregations selected yet', () => {
3758
expect(screen.getByText('Min')).toBeInTheDocument();
3859
expect(screen.getByText('Max')).toBeInTheDocument();
3960
expect(screen.getByText('Avg')).toBeInTheDocument();
61+
expect(screen.getByText('Median')).toBeInTheDocument();
4062
expect(screen.getByText('Std')).toBeInTheDocument();
4163
expect(screen.getByText('First')).toBeInTheDocument();
4264
expect(screen.getByText('Last')).toBeInTheDocument();
@@ -61,7 +83,9 @@ it('adds an aggregation when clicking the add button', async () => {
6183
aggregations: expect.arrayContaining([
6284
expect.objectContaining({ operation: SELECTABLE_OPTIONS[0] }),
6385
]),
64-
})
86+
}),
87+
expect.arrayContaining([SELECTABLE_OPTIONS[0]]),
88+
expect.arrayContaining([])
6589
);
6690
});
6791

@@ -80,7 +104,9 @@ it('deletes an aggregation when clicking the trash can', async () => {
80104
await user.click(buttons[buttons.length - 1]);
81105

82106
expect(onChange).toHaveBeenCalledWith(
83-
expect.objectContaining({ aggregations: [], showOnTop: false })
107+
expect.objectContaining({ aggregations: [], showOnTop: false }),
108+
expect.arrayContaining([]),
109+
expect.arrayContaining([AggregationOperation.SUM])
84110
);
85111
});
86112

packages/iris-grid/src/sidebar/aggregations/Aggregations.tsx

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
} from '@deephaven/components';
2121
import type { DraggableRenderItemProps, Range } from '@deephaven/components';
2222
import { ModelIndex } from '@deephaven/grid';
23+
import { dh as DhType } from '@deephaven/jsapi-types';
2324
import AggregationOperation from './AggregationOperation';
2425
import AggregationUtils, { SELECTABLE_OPTIONS } from './AggregationUtils';
2526
import './Aggregations.scss';
@@ -40,36 +41,48 @@ export type AggregationSettings = {
4041
export type AggregationsProps = {
4142
isRollup: boolean;
4243
settings: AggregationSettings;
43-
onChange: (settings: AggregationSettings) => void;
44+
onChange: (
45+
settings: AggregationSettings,
46+
added: AggregationOperation[],
47+
removed: AggregationOperation[]
48+
) => void;
4449
onEdit: (aggregation: Aggregation) => void;
50+
dh: typeof DhType;
4551
};
4652

4753
function Aggregations({
4854
isRollup,
4955
settings,
5056
onChange,
5157
onEdit,
58+
dh,
5259
}: AggregationsProps): JSX.Element {
5360
const { aggregations, showOnTop } = settings;
5461
const options = useMemo(
5562
() =>
5663
SELECTABLE_OPTIONS.filter(
5764
option =>
58-
!aggregations.some(aggregation => aggregation.operation === option)
65+
!aggregations.some(aggregation => aggregation.operation === option) &&
66+
!(
67+
option === AggregationOperation.MEDIAN &&
68+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
69+
// @ts-ignore - MEDIAN is not defined in older version of Core
70+
dh.AggregationOperation.MEDIAN === undefined
71+
)
5972
),
60-
[aggregations]
73+
[aggregations, dh]
6174
);
6275
const [selectedOperation, setSelectedOperation] = useState(options[0]);
6376
const [selectedRanges, setSelectedRanges] = useState<Range[]>([]);
6477
const changeSettings = useCallback(
65-
changedSettings => {
66-
onChange({ ...settings, ...changedSettings });
78+
(changedSettings, added = [], removed = []) => {
79+
onChange({ ...settings, ...changedSettings }, added, removed);
6780
},
6881
[onChange, settings]
6982
);
7083
const changeAggregations = useCallback(
71-
newAggregations => {
72-
changeSettings({ aggregations: newAggregations });
84+
(newAggregations, added = [], removed = []) => {
85+
changeSettings({ aggregations: newAggregations }, added, removed);
7386
},
7487
[changeSettings]
7588
);
@@ -130,17 +143,29 @@ function Aggregations({
130143
);
131144

132145
const handleAdd = useCallback(() => {
133-
changeAggregations([
134-
...aggregations,
135-
{ operation: selectedOperation, selected: [], invert: true },
136-
]);
146+
changeAggregations(
147+
[
148+
...aggregations,
149+
{ operation: selectedOperation, selected: [], invert: true },
150+
],
151+
[selectedOperation]
152+
);
137153
}, [aggregations, selectedOperation, changeAggregations]);
138154

139155
const handleDeleteClicked = useCallback(
140156
(itemIndex: ModelIndex) => {
141-
changeAggregations(
142-
aggregations.filter((aggregation, index) => index !== itemIndex)
157+
const [keep, remove] = aggregations.reduce(
158+
([keepSoFar, removeSoFar], aggregation, index) => {
159+
if (index === itemIndex) {
160+
removeSoFar.push(aggregation.operation);
161+
} else {
162+
keepSoFar.push(aggregation);
163+
}
164+
return [keepSoFar, removeSoFar];
165+
},
166+
[[], []] as [Aggregation[], AggregationOperation[]]
143167
);
168+
changeAggregations(keep, [], remove);
144169
},
145170
[aggregations, changeAggregations]
146171
);
@@ -191,6 +216,9 @@ function Aggregations({
191216
const isRollupOperation = AggregationUtils.isRollupOperation(
192217
item.operation
193218
);
219+
const isRollupProhibited = AggregationUtils.isRollupProhibited(
220+
item.operation
221+
);
194222
const isEditable = !isClone && !isRollupOperation;
195223
return (
196224
<>
@@ -208,6 +236,12 @@ function Aggregations({
208236
<FontAwesomeIcon icon={dhWarningFilled} /> Requires rollup
209237
</span>
210238
)}
239+
{isRollup && isRollupProhibited && (
240+
<span className="small text-warning">
241+
<FontAwesomeIcon icon={dhWarningFilled} /> Not available on
242+
rollups
243+
</span>
244+
)}
211245
</span>
212246
{DraggableItemList.renderBadge({ text: badgeText })}
213247
{DraggableItemList.renderHandle()}

0 commit comments

Comments
 (0)