michael-s-molina commented on PR #26439:
URL: https://github.com/apache/superset/pull/26439#issuecomment-1887017231

   > @sivasathyaseeelan I think you've stumbled on a very critical bug here. I 
believe checking for hasGenericChartAxes should be removed from 
legacyRegularTime, i.e. those controls should always be present, irrespective 
of whether GENERIC_CHART_AXES is enabled.
   
   I'm not sure about this. The only difference between the `legacyRegularTime` 
and `genericTime` is the `time_grain_sqla` control. It seems these controls 
were created for the case where the plugin is able to handle both states of the 
feature flag. One example is the line chart that displays the legacy time 
section when the feature flag is off and uses the x-axis control when the 
feature flag is on. In that case the logic is correct because you don't want to 
display the legacy time when the feature is off. I'm not sure if legacy plugins 
have this ability, we would need to check if `legacyRegularTime` will behave 
differently than `genericTime` or if we need to go plugin by plugin and see if 
it has the ability to handle both feature flag states.
   
   > FYI @michael-s-molina I think this may be important to get fixed in both 
the 3.0 and 3.1 RCs that are being voted on right now..
   
   Considering that this problem already exists in 3.0.2 and that is affects 
mostly legacy plugins, I believe we could cherry-pick the fix in the next patch.


-- 
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]

Reply via email to