bito-code-review[bot] commented on code in PR #41208:
URL: https://github.com/apache/superset/pull/41208#discussion_r3445065807
##########
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:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete test assertion</b></div>
<div id="fix">
The assertion only verifies the untouched AVG(value) metric is preserved. It
does not verify the edited SUM(value)→MAX(value) metric was actually saved. If
the save operation fails to propagate the edit, this test passes incorrectly.
Per BITO.md rule [6262], tests must validate actual behavior, not just partial
outcomes.
</div>
</div>
<small><i>Code Review Run #168057</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
