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.

Sign up for GitHub

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

Jump to bottom

Fix checkBadrequest useless logic in user-permissions' graphql config #5625

Merged
merged 2 commits into from Mar 27, 2020
Merged

Fix checkBadrequest useless logic in user-permissions' graphql config #5625

merged 2 commits into from Mar 27, 2020

Conversation

Convly
Copy link
Member

@Convly Convly commented Mar 27, 2020

fix #5558

@Convly
Update path to contextBody attributes in schema.graphql.js@checkBadRe…
2d8e558 
…quest()

Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
@Convly Convly added issue: bug Issue reporting a bug source: plugin:users-permissions Source is plugin/users-permissions package labels Mar 27, 2020
@Convly Convly added this to the 3.0.0-beta.19.4 milestone Mar 27, 2020
@codecov
Copy link

codecov bot commented Mar 27, 2020 •
edited

Codecov Report

Merging #5625 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5625   +/-   ##
=======================================
  Coverage   17.51%   17.51%           
=======================================
  Files         700      700           
  Lines       10322    10322           
  Branches     1686     1686           
=======================================
  Hits         1808     1808           
  Misses       7083     7083           
  Partials     1431     1431
Flag Coverage Δ
#front 12.88% <ø> (ø)
#unit 37.40% <ø> (ø)

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 4a8de29...2d8e558. Read the comment docs.

@derrickmehaffy
Copy link
Member

This looks more like a temporary patch. @alexandrebodin wasn't there something in the works to fix logging in general?

alexandrebodin
alexandrebodin approved these changes Mar 27, 2020
View reviewed changes
Copy link
Member

@alexandrebodin alexandrebodin left a comment •
edited

Choose a reason for hiding this comment

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

We can do more refactoring outside of the fix :)

packages/strapi-plugin-users-permissions/config/schema.graphql.js Outdated Show resolved Hide resolved
packages/strapi-plugin-users-permissions/config/schema.graphql.js Outdated Show resolved Hide resolved
alexandrebodin
alexandrebodin requested changes Mar 27, 2020
View reviewed changes
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.

A few comments :)

@alexandrebodin
Copy link
Member

This looks more like a temporary patch. @alexandrebodin wasn't there something in the works to fix logging in general?

that's not about logging here

@Convly
Change checkBadRequest's exception shape
f171029 
Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
alexandrebodin
alexandrebodin approved these changes Mar 27, 2020
View reviewed changes
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

@alexandrebodin alexandrebodin merged commit 8ae55fd into master Mar 27, 2020
@alexandrebodin alexandrebodin deleted the fix/#5558/fixUselessCheckBadRequestLogic branch March 27, 2020 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexandrebodin alexandrebodin alexandrebodin approved these changes

Assignees
No one assigned
Labels
issue: bug Issue reporting a bug source: plugin:users-permissions Source is plugin/users-permissions package
Projects
None yet
Milestone
3.0.0-beta.19.4
Development

Successfully merging this pull request may close these issues.

GraphQL login mutation doesn't throw detailed error message
3 participants
@Convly @derrickmehaffy @alexandrebodin