Create sign up mutation by 00dav00 · Pull Request #7 · graphql-devise/graphql_devise

00dav00 · 2019-08-31T19:42:23Z

Add signup mutation
Add url helper to create inline mutation

Add confirmation query as url params in email

mcelicalderon

Looks good, left some comments

mcelicalderon · 2019-09-04T13:37:43Z

 /*.sqlite3
 /spec/dummy/db/development.sqlite3
 /spec/dummy/db/test.sqlite3
+/graphql_devise-0.1.0.gem


Tus will change with every version use *.gem instead

mcelicalderon · 2019-09-04T13:41:34Z

+      argument :config_name,           String, required: false
+
+      def resolve(email:, **attrs)
+        redirect_url = attrs.delete(:confirm_success_url)


I would add kargs that are required directly in the resolve params so you don't have to do something like this

mcelicalderon · 2019-09-04T13:42:48Z

+  module Mutations
+    class SignUp < Base
+      argument :email,                 String, required: true
+      argument :name,                  String, required: false


Name is an attribute only on the dummy app, we need a way to configure that, or at least remove this one for now. You can use a custom mutation in the dummy app that extends from this one

mcelicalderon · 2019-09-04T13:45:11Z

+        resource.respond_to?(:confirmed_at)
+      end
+
+      def active_for_authentication?(resource)


This one seems redundant when you can call that directly on resource

mcelicalderon · 2019-09-04T13:46:48Z

+        :email
+      end
+
+      # NOTE: Devise controller method, find a way to re use it


You could put it on the base mutation if some other mutation uses it

mcelicalderon · 2019-09-04T13:47:11Z

@@ -0,0 +1,19 @@
+module GraphqlDevise
+  module MailerHelper
+    extend ActiveSupport::Concern


Not necessary, right?

mcelicalderon · 2019-09-04T13:47:45Z

+  module MailerHelper
+    extend ActiveSupport::Concern
+
+    protected


I think helpers are usually just public

mcelicalderon · 2019-09-04T13:50:27Z

      default_mutations = {
        login:           GraphqlDevise::Mutations::Login,
        logout:          GraphqlDevise::Mutations::Logout,
+        signUp:          GraphqlDevise::Mutations::SignUp,


This should be snake_case

mcelicalderon · 2019-09-04T13:51:53Z

        end

-        GraphqlDevise::Types::MutationType.field("#{mapping_name}_#{action}", mutation: used_mutation)
+        GraphqlDevise::Types::MutationType.field("#{mapping_name}_#{action.to_s.underscore}", mutation: used_mutation)


Use snake case everywhere until it is required otherwise

Naming here uses snake case, note "#{mapping_name}_

Yes, calling underscore here is not necessary as you changed line 22

mcelicalderon · 2019-09-04T13:52:32Z

+        GraphqlDevise::Types::MutationType.field("#{mapping_name}_#{action.to_s.underscore}", mutation: used_mutation)
      end

+      Devise.mailer.send(:add_template_helper, GraphqlDevise::MailerHelper)


There might be an engine way not to need this, but is the method not public?

Let me take a look, but it is private doc

00dav00 force-pushed the create-sign-up-mutation branch 2 times, most recently from f163526 to af626ac Compare September 4, 2019 12:13

Add sign up mutation

3002a6f

Add confirmation query as url params in email

00dav00 force-pushed the create-sign-up-mutation branch from af626ac to 3002a6f Compare September 4, 2019 12:15

David Revelo added 2 commits September 4, 2019 08:10

Add not change matcher

b825358

Use errors extension in sign up mutation

319a18a

mcelicalderon requested changes Sep 4, 2019

View reviewed changes

00dav00 force-pushed the create-sign-up-mutation branch from f013fc2 to 284f501 Compare September 5, 2019 13:32

mcelicalderon approved these changes Sep 5, 2019

View reviewed changes

Apply code review suggestions

aa9c806

00dav00 force-pushed the create-sign-up-mutation branch from 284f501 to aa9c806 Compare September 5, 2019 14:48

00dav00 merged commit ce61b29 into master Sep 5, 2019

00dav00 deleted the create-sign-up-mutation branch September 5, 2019 16:24

mcelicalderon added the enhancement New feature or request label Oct 30, 2019

Conversation

00dav00 commented Aug 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcelicalderon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

00dav00 commented Aug 31, 2019 •

edited

Loading