villebro commented on code in PR #23274:
URL: https://github.com/apache/superset/pull/23274#discussion_r1125990482
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts:
##########
@@ -62,22 +63,26 @@ export default function buildQuery(formData: QueryFormData)
{
2015-03-01 318.0 0.0
*/
+ // only add series limit metric if it's explicitly needed e.g. for sorting
+ const extra_metrics = extractExtraMetrics(formData);
Review Comment:
I'm kind of split on this one. I had this going into `queryObject`
originally, but then decided against it, as it would make it ever more
difficult to follow which form fields result in which query object fields. I
think going forward (v2 chart data API) it would be better to keep the
`baseQueryObject` as slim as possible, and be more explicit about which form
data fields should be mapped to which query object fields.
Anyway, I'm not against it, so I'm happy to move this into core later if
needed, but for now I think it's simpler to just keep this in chart controls
and use it explicitly where it's needed.
--
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]
