fix: propagate ad hoc displayAsLabel migrated attr filter immediately #5988
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -780,6 +780,8 @@ function useCallbacks( | |||
| } | |||
| }, [handler, supportsKeepingDependentFiltersSelection, supportsShowingFilteredElements]); | |||
|
|
|||
| useReportMigratedFilter(handler, onApply); | |||
There was a problem hiding this comment.
I would expect this to reexecute the dashboard insights on init. Which might be perf/resource problem.
Did not test it though.
If not, can you please explain me why not?
There was a problem hiding this comment.
Yes, it will cause double execution. This is the cost to get the features that are affected working.
The code is triggered only on old dashboards with filters that use alt labels. You can edit the dashboard, change something in the filter, change it back, save the dashboard, and the code will stop triggering as it is no longer needed.
I'm afraid we will have to do metadata migration of all the affected dashboards down the road which will fix this permanently.
xMort
force-pushed
the
pdo-lx-824-migrate-attr-filter-label
branch
2 times, most recently
from
15b5555 to
01585e3
Compare
If a filter was created before the introduction of the ‘support duplicated label values’ feature, it is migrated to the new configuration ad hoc during initialization. Previously, the new configuration was only propagated to the filter’s parent application when the user opened the filter, made changes, and applied them. After the change, the migration is now detected during filter initialization and reported immediately. There are some consequences: - If dashboard component starts in edit mode, Save button is enabled if filter was migrated. This does not happen in view mode. - Filters can be stored in filter view in the old format when user save view quicker than all filters load values and mark themselves as "initialized: success" - Dashboard is double executed in the case when invalid filters are migrated JIRA: LX-824 risk: low
xMort
force-pushed
the
pdo-lx-824-migrate-attr-filter-label
branch
from
01585e3 to
097936b
Compare
ivan-nejezchleb
approved these changes
Feb 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a filter was created before the introduction of the ‘support duplicated label values’ feature, it is migrated to the new configuration ad hoc during initialization. Previously, the new configuration was only propagated to the filter’s parent application when the user opened the filter, made changes, and applied them.
After the change, the migration is now detected during filter initialization and reported immediately.
There are some consequences:
JIRA: LX-824
risk: low
Important
Please, don't forget to run
rush changefor the commits that introduce new features or significant changes 🙏 This information is used to generate the change log.Run extended test by pull request comment
Commands can be triggered by posting a comment with specific text on the pull request. It is possible to trigger multiple commands simultaneously.
Explanation
--backstopThe command to run screen tests.--integratedThe command to run integrated tests against the live backend.--isolatedThe command to run isolated tests against recordings.--recordThe command to create new recordings for isolated tests.--filter(Optional) A comma-separated list of test files to run. This parameter is valid only for the--integrated,--isolated, and--recordcommands.Examples