Skip to content

Conversation

@pimsomeday
Copy link
Contributor

What does it do?

Fixes disabling queries and mutations other than the ones created by the ShadowCRUD. Example: register, forgotPassword, resetPassword, emailConfirmation etc. can be disabled now.

Why is it needed?

We want to hide them in the generated graphql-schema.

Related issue(s)/PR(s)

#8754
#8845

@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #9131 (c43be39) into master (9fd22a9) will increase coverage by 20.23%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #9131       +/-   ##
===========================================
+ Coverage   34.64%   54.87%   +20.23%     
===========================================
  Files        1308      133     -1175     
  Lines       14431     4304    -10127     
  Branches     1432      871      -561     
===========================================
- Hits         5000     2362     -2638     
+ Misses       8517     1526     -6991     
+ Partials      914      416      -498     
Flag Coverage Δ
front ?
unit 54.87% <ø> (ø)

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

Impacted Files Coverage Δ
...missions/admin/src/containers/Providers/reducer.js
...lder/admin/src/components/ComponentCard/Wrapper.js
...Roles/ConditionsSelect/IndicatorSeparator/index.js
...in/src/components/LeftMenu/LeftMenuFooter/index.js
...nt-manager/admin/src/components/PopupForm/index.js
...uilder/admin/src/components/ComponentCard/Close.js
...ers/SettingsPage/utils/findFirstAllowedEndpoint.js
...pload/admin/src/components/InputFileModal/Label.js
...pi-plugin-upload/admin/src/icons/Download/index.js
...nt-manager/admin/src/utils/formatFiltersToQuery.js
... and 1164 more

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 42836bf...5c5560f. Read the comment docs.

return methodString.match(regex)[0];
};

const removeDisabledResolvers = (mutationOrQueryDef, disabledResolvers) => {
Copy link
Member

Choose a reason for hiding this comment

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

I feel like on that string processing could be avoided by using a schema transform on the makeExecutableSchema instead :) this would make it more powerfull too :) inspiration https://www.advancedgraphql.com/content/schema-transformation.
What do you think ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes good one! i'll update the code

Copy link
Member

Choose a reason for hiding this comment

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

FYI you should use @graphql-tools for this :) they have a lot of utilities to do that

@alexandrebodin alexandrebodin added source: plugin:graphql Source is plugin/graphql package issue: enhancement Issue suggesting an enhancement to an existing feature labels Jan 18, 2021
@alexandrebodin alexandrebodin added this to the 3.4.5 milestone Jan 22, 2021
Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for this improvement

@alexandrebodin alexandrebodin merged commit 5d5ed08 into strapi:master Jan 22, 2021
@abett
Copy link

abett commented Jan 22, 2021

Thanks for implementing that fix, guys! 💚

@derrickmehaffy
Copy link
Member

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

https://forum.strapi.io/t/new-release-strapi-v3-4-5/2446/1

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

Labels

issue: enhancement Issue suggesting an enhancement to an existing feature source: plugin:graphql Source is plugin/graphql package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants