Skip to content

Add SignIn Mutation#72

Merged
AlessioRocco merged 4 commits intomasterfrom
login-mutation
Feb 14, 2020
Merged

Add SignIn Mutation#72
AlessioRocco merged 4 commits intomasterfrom
login-mutation

Conversation

@AlessioRocco
Copy link
Copy Markdown
Contributor

@AlessioRocco AlessioRocco commented Jan 31, 2020

#43

Add the mutation to SignIn a User by using Relay Classic conventions.

⚠️ We have a schema breaking change here that can't be fixed because we have changed the mutation root.

@AlessioRocco AlessioRocco force-pushed the login-mutation branch 2 times, most recently from a2c300e to 7abe8ac Compare January 31, 2020 14:03
@AlessioRocco AlessioRocco changed the base branch from master to add-mutation-spec-helpers January 31, 2020 14:03
@AlessioRocco AlessioRocco force-pushed the add-mutation-spec-helpers branch 2 times, most recently from bf7858f to 44944b4 Compare January 31, 2020 14:25
@AlessioRocco AlessioRocco force-pushed the login-mutation branch 2 times, most recently from b603af1 to 057e090 Compare February 7, 2020 11:06
@AlessioRocco AlessioRocco force-pushed the add-mutation-spec-helpers branch 5 times, most recently from 679cef6 to 74b16f2 Compare February 7, 2020 14:20
@AlessioRocco AlessioRocco self-assigned this Feb 7, 2020
@AlessioRocco AlessioRocco changed the base branch from add-mutation-spec-helpers to master February 7, 2020 14:40
@AlessioRocco AlessioRocco force-pushed the login-mutation branch 3 times, most recently from bd9dd70 to 4f3d3a7 Compare February 7, 2020 15:30
@AlessioRocco AlessioRocco changed the title Login Mutation Add SignIn Mutation Feb 14, 2020
@AlessioRocco AlessioRocco marked this pull request as ready for review February 14, 2020 07:41
@AlessioRocco AlessioRocco added the enhancement New feature or request label Feb 14, 2020
Copy link
Copy Markdown

@mdesantis mdesantis left a comment

Choose a reason for hiding this comment

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

A major concern about the method used in order to find the user to be authenticated, and some other minor concerns

Comment thread lib/solidus_graphql_api/services/user/sign_in.rb Outdated
Comment thread spec/integration/queries/current_user_spec.rb
Comment thread lib/solidus_graphql_api/mutations/user/sign_in.rb
@AlessioRocco AlessioRocco force-pushed the login-mutation branch 2 times, most recently from 224ecdf to 31c55eb Compare February 14, 2020 10:14
@mdesantis mdesantis self-requested a review February 14, 2020 11:22
mdesantis
mdesantis previously approved these changes Feb 14, 2020
Copy link
Copy Markdown

@mdesantis mdesantis left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

It's needed after the signIn/signUp to authenticate the requests.
We use "GraphQL::Schema::RelayClassicMutation" as a base class to
support Relay Classic mutation specification.
Copy link
Copy Markdown
Contributor

@ChristianRimondi ChristianRimondi left a comment

Choose a reason for hiding this comment

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

🎉

@AlessioRocco AlessioRocco merged commit 9328b17 into master Feb 14, 2020
@AlessioRocco AlessioRocco deleted the login-mutation branch July 3, 2020 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants