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

fix(ui): filter out fields with disableListFilter in whereBuilder prior to adding conditions #10112

Conversation

akhrarovsaid
Copy link
Contributor

@akhrarovsaid akhrarovsaid commented Dec 20, 2024

What?

This PR fixes an issue with the WhereBuilder where if the first field in a collection had disableListFilter enabled, the select in that fields Condition would be rendered disabled, making it impossible to query docs in list view.

Why?

To allow users to query their documents while still being able to set disableListFilter on fields regardless of where they are in the collection hierarchy.

How?

By filtering out unfilterable fields earlier in WhereBuilder and Condition components.

Fixes #10110

Before:
Dashboard-wherebuilder-before--Payload.webm

After:
Dashboard-wherebuilder-after--Payload.webm

Copy link
Contributor

@DanRibbens DanRibbens left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

@DanRibbens DanRibbens enabled auto-merge (squash) December 21, 2024 11:54
@DanRibbens DanRibbens self-assigned this Dec 21, 2024
@DanRibbens
Copy link
Contributor

DanRibbens commented Dec 30, 2024

I'm not sure what is happening in CI, the e2e that is failing is running for me locally. Good job.

EDIT: I commented too soon. This one is failing for me locally test/fields/collections/Text/e2e.spec.ts should disable field when admin.disableListFilter is true but still exists in the query

@DanRibbens DanRibbens disabled auto-merge December 30, 2024 19:35
Copy link
Contributor

@DanRibbens DanRibbens left a comment

Choose a reason for hiding this comment

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

I couldn't get this change to work exactly with existing functionality when the filter is set from a query param. I opened a new PR instead: #10267

@akhrarovsaid
Copy link
Contributor Author

@DanRibbens Yeah, it seems that with this PR the last element in the WhereBuilder condition remains enabled instead of disabled as that test was expecting. That particular test was also moved to admin/e2e/list-view/e2e.spec.ts it seems.

Closing in favor of yours. Thanks for working with me here though, appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

All filters are disabled if the first field in the select has disableListFilter=true
2 participants