bito-code-review[bot] commented on code in PR #41208:
URL: https://github.com/apache/superset/pull/41208#discussion_r3446543068
##########
superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.test.tsx:
##########
@@ -124,6 +124,48 @@ test('accepts an edited metric from an
AdhocMetricEditPopover', async () => {
]);
});
+test('only edits the targeted metric when two metrics share an optionName',
async () => {
+ // A saved chart can carry two adhoc metrics with the same optionName (e.g.
+ // born from a duplicated metric). Editing one must not overwrite the other.
+ // Saved charts store metrics as plain dictionaries in form_data, so mirror
+ // that shape (not AdhocMetric instances) to exercise the real load path.
+ const sharedOptionName = 'metric_shared_option';
+ const { onChange } = setup({
+ value: [
+ {
+ expressionType: EXPRESSION_TYPES.SIMPLE,
+ column: valueColumn,
+ aggregate: AGGREGATES.SUM,
+ label: 'SUM(value)',
+ optionName: sharedOptionName,
+ },
+ {
+ expressionType: EXPRESSION_TYPES.SIMPLE,
+ column: valueColumn,
+ aggregate: AGGREGATES.AVG,
+ label: 'AVG(value)',
+ optionName: sharedOptionName,
+ },
+ ],
+ });
+
+ userEvent.click(screen.getByText('SUM(value)'));
+ await screen.findByText('aggregate');
+ await selectOption('MAX', 'Select aggregate options');
+ await screen.findByText('MAX(value)');
+ userEvent.click(screen.getByRole('button', { name: /save/i }));
+
+ // The untouched AVG(value) metric must still be present and unchanged.
+ expect(onChange).toHaveBeenCalledWith(
+ expect.arrayContaining([
+ expect.objectContaining({
+ aggregate: AGGREGATES.AVG,
+ label: 'AVG(value)',
+ }),
+ ]),
+ );
Review Comment:
<!-- Bito Reply -->
The reviewer's suggestion is appropriate. The test currently only verifies
that the untouched metric is preserved, but it fails to confirm that the
intended edit (SUM(value) to MAX(value)) was successfully applied.
Strengthening the assertion to check for the presence of the updated metric
ensures the test validates the full behavior of the save operation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
