Apply code review suggestions

Author: David Revelo

config/locales/en.yml

@@ -14,6 +14,7 @@ en:
       password_not_required: "This account does not require a password. Sign in using your '%{provider}' account instead."
       reset_token_not_found: "No user found for the specified reset token."
       reset_token_expired: "Reset password token is no longer valid."
+      send_instructions: "You will receive an email with instructions on how to reset your password in a few minutes."
     sessions:
       bad_credentials: "Invalid login credentials. Please try again."
       not_confirmed: "A confirmation email was sent to your account at '%{email}'. You must follow the instructions in the email before your account can be activated"

lib/graphql_devise/default_operations/mutations.rb

@@ -9,13 +9,12 @@
 module GraphqlDevise
   module DefaultOperations
     MUTATIONS = {
-      login:               { klass: GraphqlDevise::Mutations::Login, authenticable: true },
-      logout:              { klass: GraphqlDevise::Mutations::Logout, authenticable: true },
-      sign_up:             { klass: GraphqlDevise::Mutations::SignUp, authenticable: true },
-      update_password:     { klass: GraphqlDevise::Mutations::UpdatePassword, authenticable: true },
-      send_password_reset: { klass: GraphqlDevise::Mutations::SendPasswordReset, authenticable: true },
-      resend_confirmation: { klass: GraphqlDevise::Mutations::ResendConfirmation, authenticable: true }
-
+      login:               { klass: GraphqlDevise::Mutations::Login, authenticatable: true },
+      logout:              { klass: GraphqlDevise::Mutations::Logout, authenticatable: true },
+      sign_up:             { klass: GraphqlDevise::Mutations::SignUp, authenticatable: true },
+      update_password:     { klass: GraphqlDevise::Mutations::UpdatePassword, authenticatable: true },
+      send_password_reset: { klass: GraphqlDevise::Mutations::SendPasswordReset, authenticatable: false },
+      resend_confirmation: { klass: GraphqlDevise::Mutations::ResendConfirmation, authenticatable: false }
     }.freeze
   end
 end

lib/graphql_devise/mount_method/operation_preparers/mutation_field_setter.rb

@@ -6,8 +6,8 @@ def initialize(authenticatable_type)
           @authenticatable_type = authenticatable_type
         end
 
-        def call(mutation, authenticable: true)
-          return unless authenticable
+        def call(mutation, authenticatable: true)
+          return mutation unless authenticatable
 
           mutation.field(:authenticatable, @authenticatable_type, null: false)
           mutation

lib/graphql_devise/mutations/resend_confirmation.rb

@@ -22,10 +22,7 @@ def resolve(email:, redirect_url:)
             template_path: ['graphql_devise/mailer']
           )
 
-          {
-            authenticatable: resource,
-            message:       I18n.t('graphql_devise.confirmations.send_instructions', email: email)
-          }
+          { message: I18n.t('graphql_devise.confirmations.send_instructions', email: email) }
         else
           raise_user_error(I18n.t('graphql_devise.confirmations.user_not_found', email: email))
         end

lib/graphql_devise/mutations/send_password_reset.rb

@@ -20,7 +20,7 @@ def resolve(email:, redirect_url:)
           )
 
           if resource.errors.empty?
-            { message: I18n.t('devise.passwords.send_instructions') }
+            { message: I18n.t('graphql_devise.passwords.send_instructions') }
           else
             raise_user_error_list(I18n.t('graphql_devise.invalid_resource'), errors: resource.errors.full_messages)
           end

spec/requests/mutations/resend_confirmation_spec.rb

@@ -15,10 +15,6 @@
           redirectUrl:"#{redirect}"
         ) {
           message
-          authenticatable {
-            id
-            email
-          }
         }
       }
     GRAPHQL
@@ -28,11 +24,7 @@
     it 'sends an email to the user with confirmation url and returns a success message' do
       expect { post_request }.to change(ActionMailer::Base.deliveries, :count).by(1)
       expect(json_response[:data][:userResendConfirmation]).to include(
-        message:         'You will receive an email with instructions for how to confirm your email address in a few minutes.',
-        authenticatable: {
-          id:    id,
-          email: email
-        }
+        message: 'You will receive an email with instructions for how to confirm your email address in a few minutes.'
       )
 
       email = Nokogiri::HTML(ActionMailer::Base.deliveries.last.body.encoded)
@@ -56,11 +48,7 @@
       it 'honors devise configuration for case insensitive fields' do
         expect { post_request }.to change(ActionMailer::Base.deliveries, :count).by(1)
         expect(json_response[:data][:userResendConfirmation]).to include(
-          message:         'You will receive an email with instructions for how to confirm your email address in a few minutes.',
-          authenticatable: {
-            id:    id,
-            email: user.email
-          }
+          message: 'You will receive an email with instructions for how to confirm your email address in a few minutes.'
         )
       end
     end

spec/requests/mutations/send_password_reset_spec.rb

@@ -23,7 +23,7 @@
     it 'sends password reset  email' do
       expect { post_request }.to change(ActionMailer::Base.deliveries, :count).by(1)
 
-      expect(json_response.dig(:data, :userSendPasswordReset)).to include(
+      expect(json_response[:data][:userSendPasswordReset]).to include(
         message: 'You will receive an email with instructions on how to reset your password in a few minutes.'
       )
 
@@ -43,7 +43,7 @@
 
     it 'honors devise configuration for case insensitive fields' do
       expect { post_request }.to change(ActionMailer::Base.deliveries, :count).by(1)
-      expect(json_response.dig(:data, :userSendPasswordReset)).to include(
+      expect(json_response[:data][:userSendPasswordReset]).to include(
         message: 'You will receive an email with instructions on how to reset your password in a few minutes.'
       )
     end

spec/services/mount_method/operation_preparers/default_operation_preparer_spec.rb

@@ -14,10 +14,10 @@
     let(:custom_keys)       { [:login, :logout] }
     let(:operations) do
       {
-        confirm: { klass: confirm_operation, authenticable: false },
-        sign_up: { klass: sign_up_operation, authenticable: true },
-        login:   { klass: login_operation, authenticable: true },
-        logout:  { klass: logout_operation, authenticable: true }
+        confirm: { klass: confirm_operation, authenticatable: false },
+        sign_up: { klass: sign_up_operation, authenticatable: true },
+        login:   { klass: login_operation, authenticatable: true },
+        logout:  { klass: logout_operation, authenticatable: true }
       }
     end
 
@@ -26,10 +26,10 @@
       allow(default_preparer).to receive(:child_class).with(sign_up_operation).and_return(sign_up_operation)
       allow(default_preparer).to receive(:child_class).with(login_operation).and_return(login_operation)
       allow(default_preparer).to receive(:child_class).with(logout_operation).and_return(logout_operation)
-      allow(preparer).to receive(:call).with(confirm_operation, authenticable: false).and_return(confirm_operation)
-      allow(preparer).to receive(:call).with(sign_up_operation, authenticable: true).and_return(sign_up_operation)
-      allow(preparer).to receive(:call).with(login_operation, authenticable: true).and_return(login_operation)
-      allow(preparer).to receive(:call).with(logout_operation, authenticable: true).and_return(logout_operation)
+      allow(preparer).to receive(:call).with(confirm_operation, authenticatable: false).and_return(confirm_operation)
+      allow(preparer).to receive(:call).with(sign_up_operation, authenticatable: true).and_return(sign_up_operation)
+      allow(preparer).to receive(:call).with(login_operation, authenticatable: true).and_return(login_operation)
+      allow(preparer).to receive(:call).with(logout_operation, authenticatable: true).and_return(logout_operation)
     end
 
     it 'returns only those operations with no custom operation provided' do
@@ -39,8 +39,8 @@
     it 'prepares default operations' do
       expect(confirm_operation).to receive(:graphql_name).with('UserConfirm')
       expect(sign_up_operation).to receive(:graphql_name).with('UserSignUp')
-      expect(preparer).to receive(:call).with(confirm_operation, authenticable: false)
-      expect(preparer).to receive(:call).with(sign_up_operation, authenticable: true)
+      expect(preparer).to receive(:call).with(confirm_operation, authenticatable: false)
+      expect(preparer).to receive(:call).with(sign_up_operation, authenticatable: true)
 
       prepared
 

spec/services/mount_method/operation_preparers/mutation_field_setter_spec.rb

@@ -2,13 +2,13 @@
 
 RSpec.describe GraphqlDevise::MountMethod::OperationPreparers::MutationFieldSetter do
   describe '#call' do
-    subject(:prepared_operation) { described_class.new(field_type).call(operation, authenticable: authenticable) }
+    subject(:prepared_operation) { described_class.new(field_type).call(operation, authenticatable: authenticatable) }
 
     let(:operation)  { double(:operation) }
     let(:field_type) { double(:type) }
 
     context 'when resource is authtenticable' do
-      let(:authenticable) { true }
+      let(:authenticatable) { true }
 
       it 'sets a field for the mutation' do
         expect(operation).to receive(:field).with(:authenticatable, field_type, null: false)
@@ -18,7 +18,7 @@
     end
 
     context 'when resource is *NOT* authtenticable' do
-      let(:authenticable) { false }
+      let(:authenticatable) { false }
 
       it 'does *NOT* set a field for the mutation' do
         expect(operation).not_to receive(:field)