Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EES-5902 Update migration endpoint to set auto select for group Total… #5637

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

mmyoungman
Copy link
Collaborator

… item Total filters

When the auto select work was initially deployed, we only set an auto select item for a filter if it only had one item labelled "Total". But this was incorrect - we also want to set the auto select item for filters where there are multiple totals, providing one of them belongs to the group "Total".

This change to the migration endpoint allows us to set these filter AutoSelectFilterItemIds to restore the previous behaviour from before the new auto select work was deployed.

@benoutram benoutram self-requested a review February 28, 2025 13:00
Comment on lines +30 to +34
// one group with label "Total" that contains one item with label "Total"
&& f.FilterGroups.Count(group => group.Label == "Total"
&& group.FilterItems.Count(item => item.Label == "Total")
== 1)
== 1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we also want to set the auto select item for filters where there is a single filter item with label 'Total', regardless of what its group label is?

Or maybe this code assumes the previous version of the code has already done that, and that it's been successfully run in all environments and we don't need that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe this code assumes the previous version of the code has already done that, and that it's been successfully run in all environments and we don't need that.

Yeah that's right. The previous migration already set the AutoSelectFilterItemId for Filters where there is only one "Total" FilterItem associated with a Filter. That was done in all environments in https://dfedigital.atlassian.net/browse/EES-5795

Just to be crystal clear about it, the original auto select work would only set an AutoSelectFilterItemId on a Filter if there was a sole "Total" FilterItem. This was changed in EES-5860 such that if there were mutliple "Total" FilterItems under a Filter, it would still set an AutoSelectFilterItem if a single one of those "Total" FilterItems had a "Total" FilterGroup. But that was only changed for newly imported data sets. This PR sets the auto select filter items of data sets with multiple "Total"s imported prior to these all changes.

@benoutram benoutram self-requested a review February 28, 2025 13:40
@mmyoungman mmyoungman merged commit 425ce0d into dev Feb 28, 2025
11 checks passed
@mmyoungman mmyoungman deleted the EES-5902 branch February 28, 2025 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants