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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supporting `IN` and `NOT IN` as a filter type #2736

Merged
merged 13 commits into from Feb 10, 2019

Conversation

@sajjad-shirazy
Copy link
Contributor

sajjad-shirazy commented Feb 2, 2019

My PR is a:

  • 馃挜 Breaking change
  • 馃悰 Bug fix #issueNumber
  • 馃拝 Enhancement
  • 馃殌 New feature

Main update on the:

  • Admin
  • Documentation
  • Framework
  • Plugin

Manual testing done on the following databases:

  • Not applicable
  • MongoDB
  • MySQL
  • Postgres

Description:
Supporting IN and NOT IN as a filter, so we can have such queries:

{
  books(limit: 10, where: {_id_nin: ["5c4dad1a8f3845222ca88a56", "5c4dad1a8f3845222ca88a57"]}) {
    _id,
    title
  }
}

sajjad-shirazy added some commits Feb 2, 2019

Merge pull request #2 from sajjad-shirazy/patch-4
Supporting `NOT IN` as a filter
@derrickmehaffy

This comment has been minimized.

Copy link
Contributor

derrickmehaffy commented Feb 2, 2019

@sajjad-shirazy please add documentation and also please ensure you test with all 3 database types

@sajjad-shirazy

This comment has been minimized.

Copy link
Contributor Author

sajjad-shirazy commented Feb 3, 2019

ok

@sajjad-shirazy

This comment has been minimized.

Copy link
Contributor Author

sajjad-shirazy commented Feb 3, 2019

@sajjad-shirazy sajjad-shirazy changed the title Supporting `NOT IN` as a filter Supporting `IN` and `NOT IN` as a filter type Feb 3, 2019

@@ -1107,7 +1107,14 @@ module.exports = function(strapi) {
result.key = `where.${key}`;
result.value = {
symbol: 'IN',
value,
value: _.isArray(value) ? value : [value],

This comment has been minimized.

@kamalbennani

kamalbennani Feb 3, 2019

Contributor

you can use _.castArray instead, it'll only cast the value to an array if it's not the case already

This comment has been minimized.

@sajjad-shirazy

sajjad-shirazy Feb 4, 2019

Author Contributor

Thanks, @kamalbennani I didn't know it.

@kamalbennani

This comment has been minimized.

Copy link
Contributor

kamalbennani commented Feb 3, 2019

I've already implemented this operator in this PR: https://github.com/strapi/strapi/pull/2452/files#diff-6bd63d5cf1dec74aafa2dd897ecfe932R23 but it may take some time to be merged/released

@@ -567,6 +567,10 @@ module.exports = function (strapi) {
result.key = `where.${key}.$in`;
result.value = value;
break;
case '_nin':
result.key = `where.${key}.$nin`;
result.value = value;

This comment has been minimized.

@lauriejim

lauriejim Feb 5, 2019

Member

You use _.castArray in bookshelf and you don't use it in mongoose, there is a reason ?

This comment has been minimized.

@sajjad-shirazy

sajjad-shirazy Feb 6, 2019

Author Contributor

You are right, I missed this case for mongoose.

@lauriejim lauriejim added this to 馃暟Waiting for classification in 馃Cooking via automation Feb 5, 2019

@lauriejim lauriejim self-assigned this Feb 5, 2019

@lauriejim

This comment has been minimized.

Copy link
Member

lauriejim commented Feb 5, 2019

Thank you for this PR @sajjad-shirazy !
That's interesting. Need some test and it will be merged :-)

@lauriejim lauriejim moved this from 馃暟Waiting for classification to 馃惪For next release in 馃Cooking Feb 7, 2019

@lauriejim
Copy link
Member

lauriejim left a comment

LGTM!

Thank you for this new option.

@lauriejim lauriejim added this to the 3.0.0-alpha.24 milestone Feb 10, 2019

@lauriejim lauriejim merged commit ba47114 into strapi:master Feb 10, 2019

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.