From 16984dcdc7dff305778df4c139a8a2b951e485b9 Mon Sep 17 00:00:00 2001 From: Mario Celi Date: Sun, 1 Sep 2019 21:39:10 -0500 Subject: [PATCH] Return errors using new GraphQL specification --- app/graphql/graphql_devise/mutations/base.rb | 8 ++-- app/graphql/graphql_devise/mutations/login.rb | 11 ++--- .../graphql_devise/mutations/logout.rb | 7 +-- lib/graphql_devise.rb | 2 + lib/graphql_devise/user_error.rb | 7 +++ spec/requests/mutations/login_spec.rb | 45 +++++++++---------- spec/requests/mutations/logout_spec.rb | 12 ++--- 7 files changed, 44 insertions(+), 48 deletions(-) create mode 100644 lib/graphql_devise/user_error.rb diff --git a/app/graphql/graphql_devise/mutations/base.rb b/app/graphql/graphql_devise/mutations/base.rb index 34f78f89..93dce923 100644 --- a/app/graphql/graphql_devise/mutations/base.rb +++ b/app/graphql/graphql_devise/mutations/base.rb @@ -5,16 +5,16 @@ module Mutations class Base < GraphQL::Schema::Mutation private + def raise_user_error(message) + raise GraphqlDevise::UserError, message + end + def remove_resource controller.resource = nil controller.client_id = nil controller.token = nil end - def single_error_object(error) - { success: false, errors: [error] } - end - def request controller.request end diff --git a/app/graphql/graphql_devise/mutations/login.rb b/app/graphql/graphql_devise/mutations/login.rb index 75e43fa5..669d5526 100644 --- a/app/graphql/graphql_devise/mutations/login.rb +++ b/app/graphql/graphql_devise/mutations/login.rb @@ -4,15 +4,12 @@ class Login < Base argument :email, String, required: true argument :password, String, required: true - field :success, Boolean, null: false - field :errors, [String], null: false - def resolve(email:, password:) resource = resource_class.find_by(email: email) if resource && active_for_authentication?(resource) if invalid_for_authentication?(resource, password) - return single_error_object(I18n.t('graphql_devise.sessions.bad_credentials')) + raise_user_error(I18n.t('graphql_devise.sessions.bad_credentials')) end set_auth_headers(resource) @@ -23,12 +20,12 @@ def resolve(email:, password:) { success: true, authenticable: resource, errors: [] } elsif resource && !active_for_authentication?(resource) if locked?(resource) - single_error_object(I18n.t('graphql_devise.mailer.unlock_instructions.account_lock_msg')) + raise_user_error(I18n.t('graphql_devise.mailer.unlock_instructions.account_lock_msg')) else - single_error_object(I18n.t('devise_token_auth.sessions.not_confirmed', email: resource.email)) + raise_user_error(I18n.t('devise_token_auth.sessions.not_confirmed', email: resource.email)) end else - single_error_object(I18n.t('graphql_devise.sessions.bad_credentials')) + raise_user_error(I18n.t('graphql_devise.sessions.bad_credentials')) end end diff --git a/app/graphql/graphql_devise/mutations/logout.rb b/app/graphql/graphql_devise/mutations/logout.rb index 2a213607..9fae8b22 100644 --- a/app/graphql/graphql_devise/mutations/logout.rb +++ b/app/graphql/graphql_devise/mutations/logout.rb @@ -1,9 +1,6 @@ module GraphqlDevise module Mutations class Logout < Base - field :success, Boolean, null: false - field :errors, [String], null: false - def resolve if current_resource && client && current_resource.tokens[client] current_resource.tokens.delete(client) @@ -13,9 +10,9 @@ def resolve yield current_resource if block_given? - { success: true, errors: [], authenticable: current_resource } + { authenticable: current_resource } else - { success: false, errors: [I18n.t('graphql_devise.user_not_found')] } + raise_user_error(I18n.t('graphql_devise.user_not_found')) end end end diff --git a/lib/graphql_devise.rb b/lib/graphql_devise.rb index 6e2ccf2f..d306d2ae 100644 --- a/lib/graphql_devise.rb +++ b/lib/graphql_devise.rb @@ -7,3 +7,5 @@ module GraphqlDevise class Error < StandardError; end end + +require 'graphql_devise/user_error' diff --git a/lib/graphql_devise/user_error.rb b/lib/graphql_devise/user_error.rb new file mode 100644 index 00000000..3a4b0acc --- /dev/null +++ b/lib/graphql_devise/user_error.rb @@ -0,0 +1,7 @@ +module GraphqlDevise + class UserError < GraphQL::ExecutionError + def to_h + super.merge(extensions: { code: 'USER_ERROR' }) + end + end +end diff --git a/spec/requests/mutations/login_spec.rb b/spec/requests/mutations/login_spec.rb index 24ecb16b..8e1c1de1 100644 --- a/spec/requests/mutations/login_spec.rb +++ b/spec/requests/mutations/login_spec.rb @@ -13,8 +13,6 @@ password: "#{password}" ) { user { email name signInCount } - success - errors } } GRAPHQL @@ -28,10 +26,9 @@ expect(response).to include_auth_headers expect(user.reload.tokens.keys).to include(response.headers['client']) expect(json_response[:data][:userLogin]).to match( - success: true, - errors: [], - user: { email: user.email, name: user.name, signInCount: 1 } + user: { email: user.email, name: user.name, signInCount: 1 } ) + expect(json_response[:errors]).to be_nil end end @@ -40,10 +37,9 @@ it 'returns bad credentials error' do expect(response).not_to include_auth_headers - expect(json_response[:data][:userLogin]).to match( - success: false, - errors: ['Invalid login credentials. Please try again.'], - user: nil + expect(json_response[:data][:userLogin]).to be_nil + expect(json_response[:errors]).to contain_exactly( + hash_including(message: 'Invalid login credentials. Please try again.', extensions: { code: 'USER_ERROR' }) ) end end @@ -54,13 +50,13 @@ it 'returns a must confirm account message' do expect(response).not_to include_auth_headers - expect(json_response[:data][:userLogin]).to match( - success: false, - errors: [ - "A confirmation email was sent to your account at '#{user.email}'. You must follow the instructions in the " \ - "email before your account can be activated" - ], - user: nil + expect(json_response[:data][:userLogin]).to be_nil + expect(json_response[:errors]).to contain_exactly( + hash_including( + message: "A confirmation email was sent to your account at '#{user.email}'. You must follow the " \ + "instructions in the email before your account can be activated", + extensions: { code: 'USER_ERROR' } + ) ) end end @@ -70,10 +66,12 @@ it 'returns a must confirm account message' do expect(response).not_to include_auth_headers - expect(json_response[:data][:userLogin]).to match( - success: false, - errors: ['Your account has been locked due to an excessive number of unsuccessful sign in attempts.'], - user: nil + expect(json_response[:data][:userLogin]).to be_nil + expect(json_response[:errors]).to contain_exactly( + hash_including( + message: 'Your account has been locked due to an excessive number of unsuccessful sign in attempts.', + extensions: { code: 'USER_ERROR' } + ) ) end end @@ -83,10 +81,9 @@ it 'returns a must confirm account message' do expect(response).not_to include_auth_headers - expect(json_response[:data][:userLogin]).to match( - success: false, - errors: ['Invalid login credentials. Please try again.'], - user: nil + expect(json_response[:data][:userLogin]).to be_nil + expect(json_response[:errors]).to contain_exactly( + hash_including(message: 'Invalid login credentials. Please try again.', extensions: { code: 'USER_ERROR' }) ) end end diff --git a/spec/requests/mutations/logout_spec.rb b/spec/requests/mutations/logout_spec.rb index 4c04834d..c8c6363c 100644 --- a/spec/requests/mutations/logout_spec.rb +++ b/spec/requests/mutations/logout_spec.rb @@ -9,8 +9,6 @@ mutation { userLogout{ authenticable { email } - success - errors } } GRAPHQL @@ -25,10 +23,9 @@ expect(response).not_to include_auth_headers expect(user.reload.tokens.keys).to be_empty expect(json_response[:data][:userLogout]).to match( - success: true, - errors: [], authenticable: { email: user.email } ) + expect(json_response[:errors]).to be_nil end end @@ -36,10 +33,9 @@ it 'returns an error' do expect(response).not_to include_auth_headers expect(user.reload.tokens.keys).to be_empty - expect(json_response[:data][:userLogout]).to match( - success: false, - errors: ['User was not found or was not logged in.'], - authenticable: nil + expect(json_response[:data][:userLogout]).to be_nil + expect(json_response[:errors]).to contain_exactly( + hash_including(message: 'User was not found or was not logged in.', extensions: { code: 'USER_ERROR' }) ) end end