Skip to content

Conversation

@alexandrebodin
Copy link
Member

Signed-off-by: Alexandre Bodin bodin.alex@gmail.com

Description of what you did:

Fixes #8322

@alexandrebodin alexandrebodin requested review from a team and Convly October 14, 2020 13:13
@alexandrebodin alexandrebodin added this to the 3.2.4 milestone Oct 14, 2020
@alexandrebodin alexandrebodin added source: plugin:graphql Source is plugin/graphql package issue: bug Issue reporting a bug labels Oct 14, 2020
@Convly Convly self-assigned this Oct 14, 2020
Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

Works well, nice!
Could you just add a test in graphqlCrud.test.e2e > List posts with where clause %o with an or clause?

@Convly Convly changed the title SUpport query operators _or _where in graphql with deep nesting Support query operators _or & _where in graphql with deep nesting Oct 14, 2020
@derrickmehaffy
Copy link
Member

confirmed issue though with this: #8331 not sure if this will hold back this PR or not

@alexandrebodin
Copy link
Member Author

@derrickmehaffy not related as this only fixed the querying in graphql not passing the parameters correctly

Convly
Convly previously approved these changes Oct 15, 2020
Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

LGTM, nice addition!

@codecov
Copy link

codecov bot commented Oct 15, 2020

Codecov Report

Merging #8332 into master will increase coverage by 0.00%.
The diff coverage is 91.42%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8332   +/-   ##
=======================================
  Coverage   33.20%   33.20%           
=======================================
  Files        1220     1220           
  Lines       13610    13614    +4     
  Branches     1355     1357    +2     
=======================================
+ Hits         4519     4521    +2     
  Misses       8208     8208           
- Partials      883      885    +2     
Flag Coverage Δ
#front 24.72% <ø> (ø)
#unit 54.48% <91.42%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/strapi-plugin-graphql/services/utils.js 25.00% <25.00%> (+0.30%) ⬆️
packages/strapi-admin/services/user.js 83.89% <100.00%> (ø)
...ages/strapi-utils/lib/convert-rest-query-params.js 94.59% <100.00%> (+0.07%) ⬆️
packages/strapi-utils/lib/index.js 100.00% <100.00%> (ø)
packages/strapi/lib/commands/admin-reset.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 435b294...e253f7d. Read the comment docs.

Signed-off-by: Alexandre Bodin <bodin.alex@gmail.com>
Signed-off-by: Alexandre Bodin <bodin.alex@gmail.com>
Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

LGTM

@Convly Convly merged commit e53b54c into master Oct 15, 2020
@Convly Convly deleted the fix/deep-filter-or branch October 15, 2020 11:22
@lauriejim
Copy link
Contributor

This pull request has been mentioned on Strapi Community. There might be relevant details there:

https://forum.strapi.io/t/new-release-strapi-v3-2-4-security-fix/509/1

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

Labels

issue: bug Issue reporting a bug source: plugin:graphql Source is plugin/graphql package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GraphQL complex query + deep filtering issue

5 participants