feat: Adds the Default Temporal Column

[michael-s-molina]

SUMMARY

This PR is a follow-up #21767 and introduces the Default Temporal Column control to non-legacy charts. Previously, the default temporal column used by time-based filters such as Time Range was defined in the Filters control of Explore. When creating a new chart with a dataset containing a temporal column, the application would automatically add that column as a filter in the Filters control and use it as the source for time-based filters. That strategy resulted in some problems with our clients when evaluating the Generic X-axis feature:

• It was not clear that the automatically added filter was used by time-based filters such as Time Range
• Some users deleted the filter without knowing the consequences and then complained that Time Range was not working
• It was not clear that a Time Range filter would be applied to ALL temporal columns present in the Filters control

After internal discussion, we concluded that the best place to make these configurations would be in the native filters module, where a user, when creating a Time Range filter, could potentially select all the temporal columns affected by the filter, even if they come from different datasets. This would give the users the flexibility to create different filter configurations and also clearly show which columns are being affected by the filter. We would like to prototype and validate the solution with our users before changing the native filters module. Meanwhile, we want to provide a solution to the problems stated above, and this PR proposes the following changes:

• Create a new control called Default Temporal Column below the Filters control for non-legacy charts
• This control only appears when GENERIC_CHART_AXES is enabled and the dataset has at least one temporal column
• The control should allow the selection of temporal columns or a custom SQL and cannot have its value removed
• When creating a new chart, the control will be automatically filled with the default dataset temporal column
• If the user, drags a temporal column to the X-axis, and it is a different column than the one in the Default Temporal Column, we'll present a modal asking the user if they wish to use the value in the X-axis as the Default Temporal Column
• We're also going to display a tooltip in the control to explain its use with time-based filters
• We'll remove the previous behavior where a filter was automatically added to the Filters control
• If a user defines a temporal filter in Explore (they don't have to), and the column involved in the filter is the same as the one defined in Default Temporal Column then we should replace its value. If the column used in the filter is different than the one in Default Temporal Column, then the filter will not be affected by a Time Range.
• We'll create a migration script to update the Default Temporal Column of charts with the first temporal column in the Filters control or dataset if such a temporal column exists.

Here's a preview of how the feature will look like:

The PR description will be updated after the changes.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TODO - It will be updated after the changes

TESTING INSTRUCTIONS

TODO - It will be updated after the changes

@codyml @kasiazjc @lauderbaugh

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[michael-s-molina]

@zhaoyongjie @villebro I'm tagging you to help with the review of the proposed changes and also to make sure we don't forget to change specific parts of the code. At this point, I'm only mapping the parts of the code that will be affected before starting the implementation.

[zhaoyongjie]

@michael-s-molina Thanks for mentioning these potential issues. I've some thoughts on the current design. I'd like to know what your thoughts are.

1. I'd like to say that the X-Axis Control and Time Filter(whether the Date Picker is in the Adhoc Filter, or in the Native Filter) are not related. So, the column in the X-Axis and the columns in the Adhoc Filters might be used differently.

2. the current Time Range is used in "replaced mode" to accept new Time Filter Values if a chart in a dashboard with a Native filter. It means that a validated Time Filter should be placed in the Adhoc Filters in advance, then Time Range in the native filter will replace its value. I don't know what Default Temporal Colulmn to do.

[zhaoyongjie]

If we can consider some breaking changes on the Superset 3.0, I think just removing the default temporal column in the Adhoc filters might be a simple and easy approach.

[michael-s-molina]

Hi @zhaoyongjie. Thanks for the review.

1. I'd like to say that the X-Axis Control and Time Filter(whether the Date Picker is in the Adhoc Filter, or in the Native Filter) are not related. So, the column in the X-Axis and the columns in the Adhoc Filters might be used differently.

We're aligned here. That's why we want to preserve the changes made by your original PR that improved the separation of these concepts. We just want to make it clearer to the user what column is being used by a Time Range.

2. the current Time Range is used in "replaced mode" to accept new Time Filter Values if a chart in a dashboard with a Native filter. It means that a validated Time Filter should be placed in the Adhoc Filters in advance, then Time Range in the native filter will replace its value. I don't know what Default Temporal Column to do.

That's an important observation. In our proposal, the Default Temporal Column defines which column will be used when applying a Time Range. We're intentionally supporting only one column affected by the Time Range at this stage. If a user defines a temporal filter in Explore (they don't have to), and the column involved in the filter is the same as the one defined in Default Temporal Column then we should replace its value as you pointed out. If the column used in the filter is different than the one in Default Temporal Column, then the filter will not be affected by a Time Range. I added these rules to the PR description. Thank you for the observation.

If we can consider some breaking changes on the Superset 3.0, I think just removing the default temporal column in the Adhoc filters might be a simple and easy approach.

In the future, our idea is to move this configuration to the native filters and increase its flexibility. At that stage, we won't need a Default Temporal Column anymore.

[zhaoyongjie]

@michael-s-molina, Thanks for the reply.

In our proposal, the Default Temporal Column defines which column will be used when applying a Time Range.

If I understand correctly, the Default Temporal Column will define a column on the Explore, and it would receive Time Range from Native Filter, and then the Default Temporal Column and the Time Range will be added to where clause.

This design should quickly solve the customer's request, but it will also have some side effects. For example:

1. It introduces a new concept (DTC) except filters, which makes maintaining filters more complex.
2. After the DTC, Time Range cannot be applied to multiple time filters in the AD-Hoc Filter, but previously the Time Range can do that.
3. I don't know what the effect of the time offset(time shift in Advanced Analytics) is.

[michael-s-molina]

If I understand correctly, the Default Temporal Column will define a column on the Explore, and it would receive Time Range from Native Filter, and then the Default Temporal Column and the Time Range will be added to where clause.

The Time Range will use the column defined by DTC to generate the the where clause 👍

1. It introduces a new concept (DTC) except filters, which makes maintaining filters more complex.

We believe the new concept will improve the user experience. Especially because we can clearly describe the purpose for the control.

2. After the DTC, Time Range cannot be applied to multiple time filters in the AD-Hoc Filter, but previously the Time Range can do that.

You're absolutely right about this. We discussed internally, and Product decided that it was OK no not support that requirement at this stage.

3. I don't know what the effect of the time offset(time shift in Advanced Analytics) is.

Good point. We need to consider it in the implementation.

Thank you again 🙂

[michael-s-molina]

Closing this in favor of #22707