aminghadersohi commented on code in PR #45571:
URL: https://github.com/apache/superset/pull/45571#discussion_r4354861020
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/controlPanel.tsx:
##########
@@ -39,6 +42,59 @@ const config: ControlPanelConfig = {
expanded: true,
controlSetRows: [
['columns'],
+ [
+ {
+ name: 'time_grain_sqla',
+ config: {
+ ...sharedControls.time_grain_sqla,
+ // Keep the saved grain (or none). Unlike the shared control, a
+ // new chart starts without a grain, so temporal levels keep
+ // their raw values until one is chosen.
+ initialValue: (control: ControlState) => control.value,
+ default: null,
+ description: t(
+ 'Time grain applied to the temporal levels of the hierarchy.',
+ ),
+ visibility: ({ controls }: ControlPanelsContainerProps) => {
+ const dttmLookup = Object.fromEntries(
+ ensureIsArray(controls?.columns?.options).map(option => [
+ option.column_name,
+ option.is_dttm,
+ ]),
+ );
+ // buildQuery grains only physical temporal levels, so an adhoc
+ // level alone does not show a grain that would not apply.
+ return ensureIsArray(controls?.columns?.value).some(
+ selection =>
+ isPhysicalColumn(selection) && !!dttmLookup[selection],
+ );
+ },
+ },
+ },
+ 'temporal_columns_lookup',
+ {
+ name: 'granularity_sqla',
Review Comment:
Follow-up to the landing plan: I merged master in
0463cee28aed7f86763036ed9a9c28f209358994 after
https://github.com/apache/superset/pull/45598 landed first. The
duplicate-helper disagreement is resolved: Sunburst/utils.ts imports the
lookup-based timeGrain.ts helper for cross-filters and drills, and
transformProps uses the same helper for labels. Both lookup-marked temporal
levels retain their grain, including with a Time Column outside the hierarchy.
Cross-filters retain bucket ranges; Drill to detail and Drill by retain raw
equality plus grain, and the inverted Hours-offset implementation is preserved.
Regression test: `grains every lookup-marked temporal hierarchy level` in
`superset-frontend/plugins/plugin-chart-echarts/test/Sunburst/utils.test.ts`,
`cross-filter and drills share lookup grains on both temporal levels with Time
Column %s` in the Sunburst component suite, and `formats both lookup-grained
temporal levels with Time Column %s` in the transformProps suite. Substituting
master's previous helper into timeGrain.ts produced 6 failures; restoring the
merged helper produced 6 passes. The complete
Sunburst/Timeseries/xAxisDrillByFilter/DrillBy and adapter run passed 755 tests
across 43 suites.
Siblings checked:
- superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/utils.ts —
fixed in 0463cee28aed7f86763036ed9a9c28f209358994
-
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts —
fixed in 0463cee28aed7f86763036ed9a9c28f209358994
- superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/buildQuery.ts
— not affected: already uses isGrainedLevel from timeGrain.ts; override tests
pass
- superset/common/form_data_query_context.py — not affected: backend mirror
already uses lookup OR legacy subject; backend tests pass
- superset/common/query_context_factory.py — not affected: six Sunburst
different-column cases return four bucket rows, while all twelve Timeseries
stale-filter cases pass; the factory source mutation produced six failures and
twelve passes
- superset/models/helpers.py — not affected: master's inverted offset
implementation is retained; Hours-offset suites pass
Remaining: an MCP-saved hierarchy without a lookup can grain only its legacy
Time Column, while Explore can grain both temporal levels. Clicked-level-only
Drill by ancestor behavior is unchanged.
--
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]
