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

Update view handling of filters #14614

Merged
merged 2 commits into from
Sep 20, 2024
Merged

Update view handling of filters #14614

merged 2 commits into from
Sep 20, 2024

Conversation

aptkingston
Copy link
Member

Description

This PR updates how views handle filters and when they're attached to requests. This fixes some pre-existing issues and some newly introduced ones, that resulted in things like double queries and attaching view filters in the search queries (which is unnecessary as they are saved into the view definition).

@aptkingston aptkingston marked this pull request as ready for review September 20, 2024 13:01
}
// Join them together if both
allFilters.groups = [...allFilters.groups, ...$filter.groups]
return allFilters
Copy link
Member Author

Choose a reason for hiding this comment

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

Just rewrote this to be more explicit and construct the minimal complexity of query possible.

@@ -365,7 +364,9 @@ export default class DataFetch {
let refresh = false
const entries = Object.entries(newOptions || {})
for (let [key, value] of entries) {
if (JSON.stringify(value) !== JSON.stringify(this.options[key])) {
const oldVal = this.options[key] == null ? null : this.options[key]
const newVal = value == null ? null : value
Copy link
Member Author

Choose a reason for hiding this comment

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

Added this in as undefined and null were treating as being different and therefore causing a fetch to refresh all data.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch!


if (!parsed?.groups?.length && definition.query?.groups?.length) {
this.options.filter = definition.query
}
Copy link
Member Author

Choose a reason for hiding this comment

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

We never want to set the filter to the query definition, as that is redundant and causes the filters to be sent up in the search request.

Copy link
Contributor

@deanhannigan deanhannigan left a comment

Choose a reason for hiding this comment

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

LGTM!

@aptkingston aptkingston merged commit 2a7686a into v3-ui Sep 20, 2024
10 of 11 checks passed
@aptkingston aptkingston deleted the view-filters-update branch September 20, 2024 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants