fix: WhereBuilder component does not accept all valid Where queries by AlessioGr · Pull Request #3087 · payloadcms/payload

AlessioGr · 2023-07-26T20:24:29Z

Description

Fixes #3082. This update brings both enhanced validation and additional flexibility to Where queries passed into the WhereBuilder from the URL search query params (the filter in the admin panel), while also adding valuable jsDocs for clarity.

Improvements:

The validateWhereQuery function has been reinforced to catch and reject any malformed or nonsensical queries that previously could cause errors.
Simplified Where queries are now supported. Queries such as where[text][equals]=example%20post, which do not fit the [or][and] format, were previously causing errors. The newly introduced transformWhereQuery function elegantly handles such queries, transforming them to fit the [or][and] constraints required by the component.
I have read and understand the CONTRIBUTING.md document in this repository

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist:

I have added tests that prove my fix is effective or that my feature works
Existing test suite passes locally with my changes
I have made corresponding changes to the documentation

…improve their validation

…ting old ones got transformed and are not valid, as that would cause duplicates

AlessioGr · 2023-07-26T21:11:49Z

One thing we could also do now is to make sure that the query is always simplified in the URL, by using a simplifyWhereQuery function. This will keep URLs simple and easier to understand.

The more complex, forced or[]and[] format can then only be used internally.

I don't know if that's a good idea, though. I also do not understand the way the current search query params are updated here: https://github.com/payloadcms/payload/blob/fix/3082/src/admin/components/elements/WhereBuilder/index.tsx#L106

Why do we care about the existing where params in the URL, which are merged with the new ones in newWhereQuery?
And what's the deal with paramsToKeep?

@jmikrut maybe you can chime in and give some input regarding that (& if that's a good thing to do in the first place)

DanRibbens · 2023-07-27T13:57:03Z

Does this also address #2905? After reading the code, it seems like it would.

AlessioGr · 2023-07-27T14:04:25Z

Does this also address #2905? After reading the code, it seems like it would.

Yep! This would also be addressed with this PR.

jacobsfletch · 2023-07-31T14:29:57Z

src/admin/components/elements/WhereBuilder/index.tsx

-      return whereFromSearch.or;
-    }
+    if (modifySearchQuery && whereFromSearch) {
+      if (validateWhereQuery(whereFromSearch)) {


Validating the where query here seems unnecessary, couldn't you just simply transform it? The transformer function will ensure it's returned in the correct format no matter what. No extra validation needed.

@jacobsfletch So, I have tried experimenting with removing that validate and ALWAYS transforming those where queries.

The problem here is, that for a valid where query, it would result in additional map operations in the transform step. With that additional validate step, we can skip the unnecessary transform for valid where queries, which will improve the performance. And valid where queries will happen a lot more common than invalid ones anyways.

I'd thus vote for keeping it this way

src/admin/components/elements/WhereBuilder/index.tsx

Co-authored-by: Jacob Fletcher <jacobsfletch@gmail.com> Co-authored-by: Alessio Gravili <alessio@gravili.de> Co-authored-by: PatrikKozak <patrik@trbl.design> Co-authored-by: Lucas Blancas <lablancas@gmail.com> Co-authored-by: Stef Gootzen <37367280+stefgootzen@users.noreply.github.com> Co-authored-by: Jarrod Flesch <30633324+JarrodMFlesch@users.noreply.github.com> Co-authored-by: Jessica Chowdhury <67977755+JessChowdhury@users.noreply.github.com> Co-authored-by: PatrikKozak <35232443+PatrikKozak@users.noreply.github.com> Co-authored-by: Greg Willard <Wickett06@gmail.com> Co-authored-by: James Mikrut <james@payloadcms.com> Co-authored-by: Dan Ribbens <dan.ribbens@gmail.com> Co-authored-by: Elliot DeNolf <denolfe@gmail.com> fix: WhereBuilder component does not accept all valid Where queries (#3087) fix: passes in height to resizeOptions upload option to allow height resize (#3171)

BREAKING CHANGE: - Unhandled Errors are now omitted by default. This can be breaking if people depend on those error messages. Now, it will just say "Something went wrong.". * chore: slightly improved testing of registration via graphql Signed-off-by: Vsevolod Volkov <st.lyn4@gmail.com> * chore: hiding details of internal errors from responses Signed-off-by: Vsevolod Volkov <st.lyn4@gmail.com> * feat: ability to remove authorization tokens from response bodies Signed-off-by: Vsevolod Volkov <st.lyn4@gmail.com> * chore: add section for design contributions in contributing.md * feat: add afterOperation hook (#2697) * feat: add afterOperation hook for Find operation * docs: change #afterOperation to #afteroperation * chore: extract afterOperation in function * chore: implement afterChange in operations * docs: use proper CollectionAfterOperationHook * chore: remove outdated info * chore: types afterOperation hook * chore: improves afterOperation tests * docs: updates description of afterOperation hook * chore: improve typings * chore: improve types * chore: rename index.tsx => index.ts --------- Co-authored-by: Jacob Fletcher <jacobsfletch@gmail.com> Co-authored-by: Alessio Gravili <alessio@gravili.de> * chore: remove swc version pin (#3179) * fix: WhereBuilder component does not accept all valid Where queries (#3087) * chore: add jsDocs for ListControls * chore: add jsDocs for ListView * chore: add jsDocs for WhereBuilder * chore: add comment * chore: remove unnecessary console log * chore: improve operator type * fix: transform where queries which aren't necessarily incorrect, and improve their validation * chore: add type to import * fix: do not merge existing old query params with new ones if the existing old ones got transformed and are not valid, as that would cause duplicates * chore: sort imports and remove extra validation * fix: transformWhereQuery logic * chore: add back extra validation * chore: add e2e tests * chore(test): adds test to ensure relationship returns over 10 docs (#3181) * chore(test): adds test to ensure relationship returns over 10 docs * chore: remove unnecessary movieDocs variable * fix: passes in height to resizeOptions upload option to allow height resize (#3171) * docs: fixes syntax error in rich-text.mdx that was breaking build * docs: removes auto-formatting from rich-text.mdx (#3188) * feat: Improve admin dashboard accessibility (#3053) Co-authored-by: Alessio Gravili <alessio@gravili.de> * feat: improve field ops (#3172) Co-authored-by: PatrikKozak <patrik@trbl.design> * chore: file cleanup (#3190) * chore(release): v1.14.0 * chore: improve ts typing in sanitization functions (#3194) * chore(templates): default port on website * chore(templates): safely handles bad network requests * chore(templates): implements draft preview and on-demand revalidation * chore(templates): renders static cart page fallback * chore(examples): updates draft-preview next-app example to use revalidateTag (#3199) * feat: query support for geo within and intersects + dynamic GraphQL operator types (#3183) Co-authored-by: Lucas Blancas <lablancas@gmail.com> * chore: improve checkboxes (#3191) * chore: improve filtering for hasMany number field (#3193) * chore: improve fiiltering for hasMany number field * chore: add translation for 'items' and replace rows with items * chore: new exceededLimit key * Revert "chore: add translation for 'items' and replace rows with items" This reverts commit 3a91dab. * chore: undo adding items key in translation schema * chore: new limitReached key * chore: remove unnecessary exceededLimit key * chore: spelling improvements * chore: update test build config import --------- Signed-off-by: Vsevolod Volkov <st.lyn4@gmail.com> Co-authored-by: Vsevolod Volkov <st.lyn4@gmail.com> Co-authored-by: Jarrod Flesch <30633324+JarrodMFlesch@users.noreply.github.com>

…ayloadcms#3087) * chore: add jsDocs for ListControls * chore: add jsDocs for ListView * chore: add jsDocs for WhereBuilder * chore: add comment * chore: remove unnecessary console log * chore: improve operator type * fix: transform where queries which aren't necessarily incorrect, and improve their validation * chore: add type to import * fix: do not merge existing old query params with new ones if the existing old ones got transformed and are not valid, as that would cause duplicates * chore: sort imports and remove extra validation * fix: transformWhereQuery logic * chore: add back extra validation * chore: add e2e tests

AlessioGr added 7 commits July 26, 2023 21:20

chore: add jsDocs for ListControls

36fee0f

chore: add jsDocs for ListView

b240a14

chore: add jsDocs for WhereBuilder

86ecdff

chore: add comment

7bff657

chore: remove unnecessary console log

0d4dad5

chore: improve operator type

d215d2f

fix: transform where queries which aren't necessarily incorrect, and …

6c38648

…improve their validation

AlessioGr added the bug label Jul 26, 2023

AlessioGr mentioned this pull request Jul 26, 2023

Filter state out of sync when loading a collection URL with search params #3082

Closed

AlessioGr added 2 commits July 26, 2023 22:33

chore: add type to import

9eaa940

fix: do not merge existing old query params with new ones if the exis…

d7c26ac

…ting old ones got transformed and are not valid, as that would cause duplicates

jacobsfletch reviewed Jul 31, 2023

View reviewed changes

src/admin/components/elements/WhereBuilder/index.tsx Outdated Show resolved Hide resolved

AlessioGr added 3 commits August 9, 2023 11:53

chore: sort imports and remove extra validation

8968c49

fix: transformWhereQuery logic

243bf1a

chore: add back extra validation

2facc43

AlessioGr assigned jacobsfletch Aug 9, 2023

AlessioGr requested a review from jacobsfletch August 9, 2023 12:49

AlessioGr added the merge-priority-1 label Aug 9, 2023

AlessioGr added 3 commits August 15, 2023 14:41

Merge remote-tracking branch 'origin/master' into fix/3082

b54c727

chore: add e2e tests

532a45b

chore: add explanatory comment in e2e test

e791542

AlessioGr merged commit fdfdfc8 into master Aug 15, 2023

AlessioGr deleted the fix/3082 branch August 15, 2023 17:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: WhereBuilder component does not accept all valid Where queries#3087

fix: WhereBuilder component does not accept all valid Where queries#3087
AlessioGr merged 15 commits intomasterfrom
fix/3082

AlessioGr commented Jul 26, 2023

Uh oh!

AlessioGr commented Jul 26, 2023 •

edited

Loading

Uh oh!

DanRibbens commented Jul 27, 2023

Uh oh!

AlessioGr commented Jul 27, 2023

Uh oh!

jacobsfletch Jul 31, 2023

Uh oh!

AlessioGr Aug 9, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

AlessioGr commented Jul 26, 2023

Description

Type of change

Checklist:

Uh oh!

AlessioGr commented Jul 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DanRibbens commented Jul 27, 2023

Uh oh!

AlessioGr commented Jul 27, 2023

Uh oh!

jacobsfletch Jul 31, 2023

Choose a reason for hiding this comment

Uh oh!

AlessioGr Aug 9, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AlessioGr commented Jul 26, 2023 •

edited

Loading