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

Support _contains operation for CSV type #22002

Merged
merged 11 commits into from
Apr 1, 2024

Conversation

DanielBiegler
Copy link
Contributor

@DanielBiegler DanielBiegler commented Mar 27, 2024

Scope

What's changed:

This makes the client side applying of conditions align with the behavior of the API when dealing with an array (i.e. csv type)

Screencast.from.2024-03-29.14-37-28.webm
  • Added some tests to confirm this behavior
  • Removed relevant tests from generate-joi.test.ts because its actually just testing implementation instead of behavior. Lets improve the tests inside of validate-payload.test.ts so we nail down the actual behaviour without relying on hardcoding the schemas. Please note though that in the long run we want to overhaul the behavior differences between all occurences anyway, see Implement case insensitive (and other) filters consistently #20832 and a couple other issues 🙏

Potential Risks / Drawbacks

  • Accidently validating something incorrectly that I didnt check for
  • Fixed a bug where applying insensitive contains condition is not actually insensitive! That did the same thing as plain contains.

Review Notes / Questions

  • Before checking out the branch you can see the difference between api and app when filtering via the search input. There it works and correctly looks for the substring inside the array - but if you now the same thing when applying a condition it gets ignored
  • After checking out this branch that should behave the same!

Fixes #11309

_ncontains doesnt work yet - Joi doesnt let me do it like I want it to be
Copy link

changeset-bot bot commented Mar 27, 2024

🦋 Changeset detected

Latest commit: e2a61cd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages
Name Type
@directus/utils Patch
@directus/api Patch
@directus/composables Patch
@directus/env Patch
@directus/extensions-registry Patch
@directus/extensions-sdk Patch
@directus/extensions Patch
@directus/memory Patch
@directus/pressure Patch
@directus/storage-driver-azure Patch
@directus/storage-driver-cloudinary Patch
@directus/storage-driver-gcs Patch
@directus/storage-driver-s3 Patch
@directus/storage-driver-supabase Patch
@directus/themes Patch
@directus/validation Patch
directus Patch
create-directus-extension Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@DanielBiegler DanielBiegler marked this pull request as ready for review March 29, 2024 19:48
@paescuj paescuj changed the title Fix #11309 Support _contains operation for CSV type Support _contains operation for CSV type Mar 31, 2024
@paescuj paescuj self-assigned this Apr 1, 2024
Copy link
Member

@paescuj paescuj left a comment

Choose a reason for hiding this comment

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

lgtm.mp4

@paescuj paescuj merged commit cf70e1a into main Apr 1, 2024
4 checks passed
@paescuj paescuj deleted the fix-11309-conditions-for-csv-type branch April 1, 2024 15:17
@github-actions github-actions bot added this to the Next Release milestone Apr 1, 2024
paescuj added a commit that referenced this pull request Apr 7, 2024
Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

CSV field based multi select dropdown as a filter on a condition isn't working in the admin app
2 participants