You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "lilykuang (via GitHub)" <gi...@apache.org> on 2023/09/01 02:34:02 UTC
[GitHub] [superset] lilykuang opened a new pull request, #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
lilykuang opened a new pull request, #25156:
URL: https://github.com/apache/superset/pull/25156
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### 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:**
1. Create a chart and add it to a dashboard.
2. Add a time range filter to the dashboard and apply a value.
3. Click on the chart title to access it in the Chart Builder menu.
4. Add a new filter to the chart.
5. Overwrite the chart and return to the dashboard.
6. Apply any time range filter.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] sadpandajoe commented on pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on PR #25156:
URL: https://github.com/apache/superset/pull/25156#issuecomment-1703368318
🏷️ preset:2023.35
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lilykuang commented on a diff in pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "lilykuang (via GitHub)" <gi...@apache.org>.
lilykuang commented on code in PR #25156:
URL: https://github.com/apache/superset/pull/25156#discussion_r1313317333
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -79,19 +79,15 @@ export const getSlicePayload = (
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
}
- if (
- isEmpty(adhocFilters?.adhoc_filters) &&
- isEmpty(formDataFromSlice) &&
- formDataWithNativeFilters?.adhoc_filters?.[0]?.operator ===
- Operators.TEMPORAL_RANGE
- ) {
- adhocFilters.adhoc_filters = [
- {
- ...formDataWithNativeFilters.adhoc_filters[0],
- comparator: 'No filter',
- },
- ];
- }
+ // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
+ // 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 => {
Review Comment:
they should be affected
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #25156:
URL: https://github.com/apache/superset/pull/25156#discussion_r1313045722
##########
superset-frontend/src/explore/actions/saveModalActions.test.js:
##########
@@ -383,6 +394,7 @@ describe('getSlicePayload', () => {
'viz_type',
formDataWithAdhocFiltersWithExtra.viz_type,
);
+ console.log(result.params.adhoc_filters);
Review Comment:
```suggestion
```
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -79,19 +79,15 @@ export const getSlicePayload = (
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
}
- if (
- isEmpty(adhocFilters?.adhoc_filters) &&
- isEmpty(formDataFromSlice) &&
- formDataWithNativeFilters?.adhoc_filters?.[0]?.operator ===
- Operators.TEMPORAL_RANGE
- ) {
- adhocFilters.adhoc_filters = [
- {
- ...formDataWithNativeFilters.adhoc_filters[0],
- comparator: 'No filter',
- },
- ];
- }
+ // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
+ // 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 => {
Review 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?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] justinpark commented on a diff in pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "justinpark (via GitHub)" <gi...@apache.org>.
justinpark commented on code in PR #25156:
URL: https://github.com/apache/superset/pull/25156#discussion_r1313341221
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -79,19 +79,15 @@ export const getSlicePayload = (
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
}
- if (
- isEmpty(adhocFilters?.adhoc_filters) &&
- isEmpty(formDataFromSlice) &&
- formDataWithNativeFilters?.adhoc_filters?.[0]?.operator ===
- Operators.TEMPORAL_RANGE
- ) {
- adhocFilters.adhoc_filters = [
- {
- ...formDataWithNativeFilters.adhoc_filters[0],
- comparator: 'No filter',
- },
- ];
- }
+ // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
+ // 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 => {
Review Comment:
This change is okay for my hotfix #24876
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #25156:
URL: https://github.com/apache/superset/pull/25156#discussion_r1313333769
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -79,19 +79,15 @@ export const getSlicePayload = (
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
}
- if (
- isEmpty(adhocFilters?.adhoc_filters) &&
- isEmpty(formDataFromSlice) &&
- formDataWithNativeFilters?.adhoc_filters?.[0]?.operator ===
- Operators.TEMPORAL_RANGE
- ) {
- adhocFilters.adhoc_filters = [
- {
- ...formDataWithNativeFilters.adhoc_filters[0],
- comparator: 'No filter',
- },
- ];
- }
+ // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
+ // 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 => {
Review Comment:
Sorry, I'm not sure if I was clear. What I mean is, does your change also preserves the fixes that were made in those PRs?
- Creating a new chart with dashboard time filters but filter can't apply again
- Invalid "No Filter" applied
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #25156:
URL: https://github.com/apache/superset/pull/25156#discussion_r1313045494
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -79,19 +79,15 @@ export const getSlicePayload = (
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
}
- if (
- isEmpty(adhocFilters?.adhoc_filters) &&
- isEmpty(formDataFromSlice) &&
- formDataWithNativeFilters?.adhoc_filters?.[0]?.operator ===
- Operators.TEMPORAL_RANGE
- ) {
- adhocFilters.adhoc_filters = [
- {
- ...formDataWithNativeFilters.adhoc_filters[0],
- comparator: 'No filter',
- },
- ];
- }
+ // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
+ // 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 => {
Review 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?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lilykuang commented on a diff in pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "lilykuang (via GitHub)" <gi...@apache.org>.
lilykuang commented on code in PR #25156:
URL: https://github.com/apache/superset/pull/25156#discussion_r1313257360
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -79,19 +79,15 @@ export const getSlicePayload = (
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
}
- if (
- isEmpty(adhocFilters?.adhoc_filters) &&
- isEmpty(formDataFromSlice) &&
- formDataWithNativeFilters?.adhoc_filters?.[0]?.operator ===
- Operators.TEMPORAL_RANGE
- ) {
- adhocFilters.adhoc_filters = [
- {
- ...formDataWithNativeFilters.adhoc_filters[0],
- comparator: 'No filter',
- },
- ];
- }
+ // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
+ // 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 => {
Review 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.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #25156:
URL: https://github.com/apache/superset/pull/25156#discussion_r1313304626
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -79,19 +79,15 @@ export const getSlicePayload = (
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
}
- if (
- isEmpty(adhocFilters?.adhoc_filters) &&
- isEmpty(formDataFromSlice) &&
- formDataWithNativeFilters?.adhoc_filters?.[0]?.operator ===
- Operators.TEMPORAL_RANGE
- ) {
- adhocFilters.adhoc_filters = [
- {
- ...formDataWithNativeFilters.adhoc_filters[0],
- comparator: 'No filter',
- },
- ];
- }
+ // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
+ // 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 => {
Review Comment:
Thank you for the additional context @lilykuang. Do you know if the fixes in https://github.com/apache/superset/pull/24405 and https://github.com/apache/superset/pull/24876 are not affected by this change?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #25156:
URL: https://github.com/apache/superset/pull/25156#issuecomment-1703116045
/testenv up
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25156:
URL: https://github.com/apache/superset/pull/25156#issuecomment-1703338997
Ephemeral environment shutdown and build artifacts deleted.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lilykuang merged pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "lilykuang (via GitHub)" <gi...@apache.org>.
lilykuang merged PR #25156:
URL: https://github.com/apache/superset/pull/25156
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #25156:
URL: https://github.com/apache/superset/pull/25156#discussion_r1313333769
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -79,19 +79,15 @@ export const getSlicePayload = (
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
}
- if (
- isEmpty(adhocFilters?.adhoc_filters) &&
- isEmpty(formDataFromSlice) &&
- formDataWithNativeFilters?.adhoc_filters?.[0]?.operator ===
- Operators.TEMPORAL_RANGE
- ) {
- adhocFilters.adhoc_filters = [
- {
- ...formDataWithNativeFilters.adhoc_filters[0],
- comparator: 'No filter',
- },
- ];
- }
+ // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
+ // 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 => {
Review Comment:
Sorry, I'm not sure if I was clear. What I mean is, does your change also preserves the fixes that were made in those PRs?
- Creating a new chart with dashboard time filters but filter can't apply again
- Invalid "No Filter" applied
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lilykuang commented on a diff in pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "lilykuang (via GitHub)" <gi...@apache.org>.
lilykuang commented on code in PR #25156:
URL: https://github.com/apache/superset/pull/25156#discussion_r1313317333
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -79,19 +79,15 @@ export const getSlicePayload = (
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
}
- if (
- isEmpty(adhocFilters?.adhoc_filters) &&
- isEmpty(formDataFromSlice) &&
- formDataWithNativeFilters?.adhoc_filters?.[0]?.operator ===
- Operators.TEMPORAL_RANGE
- ) {
- adhocFilters.adhoc_filters = [
- {
- ...formDataWithNativeFilters.adhoc_filters[0],
- comparator: 'No filter',
- },
- ];
- }
+ // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
+ // 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 => {
Review Comment:
they shouldn't be affected
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lilykuang commented on a diff in pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "lilykuang (via GitHub)" <gi...@apache.org>.
lilykuang commented on code in PR #25156:
URL: https://github.com/apache/superset/pull/25156#discussion_r1313257360
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -79,19 +79,15 @@ export const getSlicePayload = (
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
}
- if (
- isEmpty(adhocFilters?.adhoc_filters) &&
- isEmpty(formDataFromSlice) &&
- formDataWithNativeFilters?.adhoc_filters?.[0]?.operator ===
- Operators.TEMPORAL_RANGE
- ) {
- adhocFilters.adhoc_filters = [
- {
- ...formDataWithNativeFilters.adhoc_filters[0],
- comparator: 'No filter',
- },
- ];
- }
+ // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
+ // 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 => {
Review 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.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #25156: fix: Applying Dashboard Time Range Filters to Overwritten Charts
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25156:
URL: https://github.com/apache/superset/pull/25156#issuecomment-1703119068
@michael-s-molina Ephemeral environment spinning up at http://54.188.121.210:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org