fix: Applying Dashboard Time Range Filters to Overwritten Charts #25156
Conversation
There was a problem hiding this comment.
Thanks for the PR @lilykuang. This logic was changed so many times before and I would like to make sure this change does not impact previous fixes in #24405 and #24876.
@sadpandajoe @jinghua-qa can we get your help here to test the multiple flows before merging this PR?
| // If a filter is of type TEMPORAL_RANGE and isExtra, it sets its comparator to | ||
| // 'No filter' and adds the modified filter to the adhocFilters array. This ensures that all | ||
| // TEMPORAL_RANGE filters are converted to 'No filter' when saving a chart. | ||
| formDataWithNativeFilters?.adhoc_filters?.forEach(filter => { |
There was a problem hiding this comment.
Previously, formDataWithNativeFilters.adhoc_filters were only used if adhocFilters.adhoc_filters and formDataFromSlice were empty. Why were these checks necessary? Don't we need them anymore?
There was a problem hiding this comment.
In the past, we used those checks for adhocFilters.adhoc_filters and formDataFromSlice being empty because we noticed that the TEMPORAL_RANGE filter would disappear when those conditions were met. But as we dug deeper, we found out that this wasn't just happening when both of them were empty. It was also happening when adhocFilters.adhoc_filters had another filter. This PR is attempting to address both scenarios, ensuring that the TEMPORAL_RANGE filter is retained properly.
There was a problem hiding this comment.
Thank you for the additional context @lilykuang. Do you know if the fixes in #24405 and #24876 are not affected by this change?
There was a problem hiding this comment.
they shouldn't be affected
There was a problem hiding this comment.
This change is okay for my hotfix #24876
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
|
/testenv up |
|
@michael-s-molina Ephemeral environment spinning up at http://54.188.121.210:8080. Credentials are |
There was a problem hiding this comment.
Thank you for the fix @lilykuang and for addressing the comments!
michael-s-molina
added
the
v3.0
|
Ephemeral environment shutdown and build artifacts deleted. |
…che#25156) Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> (cherry picked from commit f2523b2)
|
🏷️ preset:2023.35 |
) Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> (cherry picked from commit f2523b2)
…che#25156) Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
…che#25156) Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
mistercrunch
added
🍒 3.0.0
🍒 3.0.1
🍒 3.0.2
🍒 3.0.3
🍒 3.0.4
🏷️ bot
SUMMARY
When a chart was accessed from a dashboard, the dashboard configuration, including color palette and temporal filter, was carried over to the Chart Builder. If a temporal filter was carried over and the chart was subsequently overwritten, the temporal filter was not functioning correctly with the dashboard temporal filter.
How to Reproduce the Bug:
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION