diff --git a/app/graphql/mutations/base_mutation.rb b/app/graphql/mutations/base_mutation.rb index 38dde893..f93356e5 100644 --- a/app/graphql/mutations/base_mutation.rb +++ b/app/graphql/mutations/base_mutation.rb @@ -18,12 +18,16 @@ def self.require_one_of(arguments, context) end end - field :errors, [GraphQL::Types::String], + field :errors, [Types::Errors::ErrorType], null: false, description: 'Errors encountered during execution of the mutation.' def current_user context[:current_user] end + + def create_message_error(message) + Sagittarius::Graphql::ErrorMessageContainer.new(message: message) + end end end diff --git a/app/graphql/mutations/echo.rb b/app/graphql/mutations/echo.rb index 8f1aa1cf..5b9c04d6 100644 --- a/app/graphql/mutations/echo.rb +++ b/app/graphql/mutations/echo.rb @@ -9,11 +9,6 @@ class Echo < BaseMutation that a user has mutation access. DOC - argument :errors, - type: [::GraphQL::Types::String], - required: false, - description: 'Errors to return to the user.' - argument :message, type: ::GraphQL::Types::String, required: false, diff --git a/app/graphql/mutations/users/logout.rb b/app/graphql/mutations/users/logout.rb index 8594b6c3..666f9a1e 100644 --- a/app/graphql/mutations/users/logout.rb +++ b/app/graphql/mutations/users/logout.rb @@ -13,7 +13,7 @@ class Logout < BaseMutation def resolve(user_session_id:) user_session = SagittariusSchema.object_from_id(user_session_id) - return { user_session: nil, errors: ['Invalid user session'] } if user_session.nil? + return { user_session: nil, errors: [create_message_error('Invalid user session')] } if user_session.nil? UserLogoutService.new(current_user, user_session).execute.to_mutation_response(success_key: :user_session) end diff --git a/app/graphql/types/errors/active_model_error_type.rb b/app/graphql/types/errors/active_model_error_type.rb new file mode 100644 index 00000000..38b11e3a --- /dev/null +++ b/app/graphql/types/errors/active_model_error_type.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Types + module Errors + class ActiveModelErrorType < Types::BaseObject + description 'Represents an active model error' + + field :attribute, String, null: false, description: 'The affected attribute on the model' + field :type, String, null: false, description: 'The validation type that failed for the attribute' + + def type + object.details[:error] + end + end + end +end diff --git a/app/graphql/types/errors/error_type.rb b/app/graphql/types/errors/error_type.rb new file mode 100644 index 00000000..937c6818 --- /dev/null +++ b/app/graphql/types/errors/error_type.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Types + module Errors + class ErrorType < BaseUnion + description 'Objects that can present an error' + possible_types Errors::ActiveModelErrorType, Errors::MessageErrorType + + def self.resolve_type(object, _ctx) + case object + when ActiveModel::Error + Errors::ActiveModelErrorType + when Sagittarius::Graphql::ErrorMessageContainer + Errors::MessageErrorType + else + raise 'Unsupported ErrorType' + end + end + end + end +end diff --git a/app/graphql/types/errors/message_error_type.rb b/app/graphql/types/errors/message_error_type.rb new file mode 100644 index 00000000..c215bccd --- /dev/null +++ b/app/graphql/types/errors/message_error_type.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Types + module Errors + class MessageErrorType < Types::BaseObject + description 'Represents an error message' + + field :message, String, null: false, description: 'The message provided from the error' + end + end +end diff --git a/app/services/service_response.rb b/app/services/service_response.rb index 9e4faff7..2f96e445 100644 --- a/app/services/service_response.rb +++ b/app/services/service_response.rb @@ -38,9 +38,13 @@ def to_mutation_response(success_key: :object) return { success_key => payload, errors: [] } if success? if payload.is_a?(ActiveModel::Errors) - { success_key => nil, errors: payload.full_messages } + { success_key => nil, errors: payload.errors } else - { success_key => nil, errors: Array.wrap(payload) } + errors = Array.wrap(payload).map do |message| + Sagittarius::Graphql::ErrorMessageContainer.new(message: message) + end + + { success_key => nil, errors: errors } end end end diff --git a/app/services/user_login_service.rb b/app/services/user_login_service.rb index 69838c24..43c8a7a9 100644 --- a/app/services/user_login_service.rb +++ b/app/services/user_login_service.rb @@ -13,7 +13,7 @@ def execute user = User.authenticate_by(args) if user.nil? logger.info(message: 'Failed login', username: args[:username], email: args[:email]) - return ServiceResponse.error(message: 'Invalid login data', payload: ['Invalid login data']) + return ServiceResponse.error(message: 'Invalid login data', payload: :invalid_login_data) end user_session = UserSession.create(user: user) diff --git a/app/services/user_logout_service.rb b/app/services/user_logout_service.rb index febe2138..f9797556 100644 --- a/app/services/user_logout_service.rb +++ b/app/services/user_logout_service.rb @@ -10,7 +10,7 @@ def initialize(current_user, user_session) def execute unless Ability.allowed?(@current_user, :logout_session, @user_session) - return ServiceResponse.error(payload: "You can't log out this session") + return ServiceResponse.error(payload: :missing_permission) end @user_session.active = false @@ -20,7 +20,7 @@ def execute ServiceResponse.success(message: 'Logged out session', payload: @user_session) else logger.warn(message: 'Failed to log out session', session_id: @user_session.id, user_id: @user_session.user_id) - ServiceResponse.error(payload: 'Failed to log out session') + ServiceResponse.error(payload: @user_session.errors) end end end diff --git a/docs/content/graphql/mutation/echo.md b/docs/content/graphql/mutation/echo.md index 1fd4037a..fec7679c 100644 --- a/docs/content/graphql/mutation/echo.md +++ b/docs/content/graphql/mutation/echo.md @@ -13,7 +13,6 @@ that a user has mutation access. | Name | Type | Description | |------|------|-------------| | `clientMutationId` | [`String`](../scalar/string.md) | A unique identifier for the client performing the mutation. | -| `errors` | [`[String!]`](../scalar/string.md) | Errors to return to the user. | | `message` | [`String`](../scalar/string.md) | Message to return to the user. | ## Fields @@ -21,5 +20,5 @@ that a user has mutation access. | Name | Type | Description | |------|------|-------------| | `clientMutationId` | [`String`](../scalar/string.md) | A unique identifier for the client performing the mutation. | -| `errors` | [`[String!]!`](../scalar/string.md) | Errors encountered during execution of the mutation. | +| `errors` | [`[Error!]!`](../union/error.md) | Errors encountered during execution of the mutation. | | `message` | [`String`](../scalar/string.md) | Message returned to the user. | diff --git a/docs/content/graphql/mutation/userslogin.md b/docs/content/graphql/mutation/userslogin.md index 7bad7e95..192e4812 100644 --- a/docs/content/graphql/mutation/userslogin.md +++ b/docs/content/graphql/mutation/userslogin.md @@ -18,5 +18,5 @@ Login to an existing user | Name | Type | Description | |------|------|-------------| | `clientMutationId` | [`String`](../scalar/string.md) | A unique identifier for the client performing the mutation. | -| `errors` | [`[String!]!`](../scalar/string.md) | Errors encountered during execution of the mutation. | +| `errors` | [`[Error!]!`](../union/error.md) | Errors encountered during execution of the mutation. | | `userSession` | [`UserSession`](../object/usersession.md) | The created user session | diff --git a/docs/content/graphql/mutation/userslogout.md b/docs/content/graphql/mutation/userslogout.md index 32e85171..5a13ed37 100644 --- a/docs/content/graphql/mutation/userslogout.md +++ b/docs/content/graphql/mutation/userslogout.md @@ -16,5 +16,5 @@ Logout an existing user session | Name | Type | Description | |------|------|-------------| | `clientMutationId` | [`String`](../scalar/string.md) | A unique identifier for the client performing the mutation. | -| `errors` | [`[String!]!`](../scalar/string.md) | Errors encountered during execution of the mutation. | +| `errors` | [`[Error!]!`](../union/error.md) | Errors encountered during execution of the mutation. | | `userSession` | [`UserSession`](../object/usersession.md) | The logged out user session | diff --git a/docs/content/graphql/mutation/usersregister.md b/docs/content/graphql/mutation/usersregister.md index 2f76b26e..e248546c 100644 --- a/docs/content/graphql/mutation/usersregister.md +++ b/docs/content/graphql/mutation/usersregister.md @@ -18,5 +18,5 @@ Register a new user | Name | Type | Description | |------|------|-------------| | `clientMutationId` | [`String`](../scalar/string.md) | A unique identifier for the client performing the mutation. | -| `errors` | [`[String!]!`](../scalar/string.md) | Errors encountered during execution of the mutation. | +| `errors` | [`[Error!]!`](../union/error.md) | Errors encountered during execution of the mutation. | | `user` | [`User`](../object/user.md) | The created user | diff --git a/docs/content/graphql/object/activemodelerror.md b/docs/content/graphql/object/activemodelerror.md new file mode 100644 index 00000000..f4178f42 --- /dev/null +++ b/docs/content/graphql/object/activemodelerror.md @@ -0,0 +1,13 @@ +--- +title: ActiveModelError +--- + +Represents an active model error + +## Fields without arguments + +| Name | Type | Description | +|------|------|-------------| +| `attribute` | [`String!`](../scalar/string.md) | The affected attribute on the model | +| `type` | [`String!`](../scalar/string.md) | The validation type that failed for the attribute | + diff --git a/docs/content/graphql/object/messageerror.md b/docs/content/graphql/object/messageerror.md new file mode 100644 index 00000000..74df969d --- /dev/null +++ b/docs/content/graphql/object/messageerror.md @@ -0,0 +1,12 @@ +--- +title: MessageError +--- + +Represents an error message + +## Fields without arguments + +| Name | Type | Description | +|------|------|-------------| +| `message` | [`String!`](../scalar/string.md) | The message provided from the error | + diff --git a/docs/content/graphql/union/error.md b/docs/content/graphql/union/error.md new file mode 100644 index 00000000..f2e33dea --- /dev/null +++ b/docs/content/graphql/union/error.md @@ -0,0 +1,10 @@ +--- +title: Error +--- + +Objects that can present an error + +## Possible types + +- [`ActiveModelError`](../object/activemodelerror.md) +- [`MessageError`](../object/messageerror.md) diff --git a/lib/sagittarius/graphql/error_message_container.rb b/lib/sagittarius/graphql/error_message_container.rb new file mode 100644 index 00000000..34a8d03f --- /dev/null +++ b/lib/sagittarius/graphql/error_message_container.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Sagittarius + module Graphql + ErrorMessageContainer = Struct.new(:message) + end +end diff --git a/spec/graphql/types/errors/active_model_error_type_spec.rb b/spec/graphql/types/errors/active_model_error_type_spec.rb new file mode 100644 index 00000000..24275296 --- /dev/null +++ b/spec/graphql/types/errors/active_model_error_type_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SagittariusSchema.types['ActiveModelError'] do + let(:fields) do + %w[ + attribute + type + ] + end + + it { expect(described_class.graphql_name).to eq('ActiveModelError') } + it { expect(described_class).to have_graphql_fields(fields) } +end diff --git a/spec/graphql/types/errors/error_type_spec.rb b/spec/graphql/types/errors/error_type_spec.rb new file mode 100644 index 00000000..964d8910 --- /dev/null +++ b/spec/graphql/types/errors/error_type_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SagittariusSchema.types['Error'] do + it 'returns possible types' do + expect(described_class.possible_types).to include( + Types::Errors::ActiveModelErrorType, + Types::Errors::MessageErrorType + ) + end + + describe '.resolve_type' do + it 'resolves active model errors' do + expect( + described_class.resolve_type(ActiveModel::Error.new(nil, :test, :invalid), {}) + ).to eq(Types::Errors::ActiveModelErrorType) + end + + it 'resolves message errors' do + expect( + described_class.resolve_type(Sagittarius::Graphql::ErrorMessageContainer.new(message: 'message'), {}) + ).to eq(Types::Errors::MessageErrorType) + end + + it 'raises an error for invalid types' do + expect { described_class.resolve_type(build(:user), {}) }.to raise_error 'Unsupported ErrorType' + end + end +end diff --git a/spec/graphql/types/errors/message_error_type_spec.rb b/spec/graphql/types/errors/message_error_type_spec.rb new file mode 100644 index 00000000..69aa3578 --- /dev/null +++ b/spec/graphql/types/errors/message_error_type_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SagittariusSchema.types['MessageError'] do + let(:fields) do + %w[ + message + ] + end + + it { expect(described_class.graphql_name).to eq('MessageError') } + it { expect(described_class).to have_graphql_fields(fields) } +end diff --git a/spec/requests/graphql/mutation/users/login_spec.rb b/spec/requests/graphql/mutation/users/login_spec.rb index 2a3ae364..63a1b666 100644 --- a/spec/requests/graphql/mutation/users/login_spec.rb +++ b/spec/requests/graphql/mutation/users/login_spec.rb @@ -9,7 +9,7 @@ <<~QUERY mutation($input: UsersLoginInput!) { usersLogin(input: $input) { - errors + #{error_query} userSession { id token @@ -100,7 +100,7 @@ it 'returns errors', :aggregate_failures do expect(graphql_data_at(:users_login, :user_session)).not_to be_present - expect(graphql_data_at(:users_login, :errors)).to include('Invalid login data') + expect(graphql_data_at(:users_login, :errors, :message)).to include('invalid_login_data') end end @@ -115,7 +115,7 @@ it 'returns errors', :aggregate_failures do expect(graphql_data_at(:users_login, :user_session)).not_to be_present - expect(graphql_data_at(:users_login, :errors)).to include('Invalid login data') + expect(graphql_data_at(:users_login, :errors, :message)).to include('invalid_login_data') end end @@ -130,7 +130,7 @@ it 'returns errors', :aggregate_failures do expect(graphql_data_at(:users_login, :user_session)).not_to be_present - expect(graphql_data_at(:users_login, :errors)).to include('Invalid login data') + expect(graphql_data_at(:users_login, :errors, :message)).to include('invalid_login_data') end end end diff --git a/spec/requests/graphql/mutation/users/logout_spec.rb b/spec/requests/graphql/mutation/users/logout_spec.rb index 77a32103..a2aed47a 100644 --- a/spec/requests/graphql/mutation/users/logout_spec.rb +++ b/spec/requests/graphql/mutation/users/logout_spec.rb @@ -9,7 +9,7 @@ <<~QUERY mutation($input: UsersLogoutInput!) { usersLogout(input: $input) { - errors + #{error_query} userSession { id active @@ -42,7 +42,7 @@ let(:current_user) { create(:user) } it 'does not log out the session', :aggregate_failures do - expect(graphql_data_at(:users_logout, :errors)).to include("You can't log out this session") + expect(graphql_data_at(:users_logout, :errors, :message)).to include('missing_permission') expect(graphql_data_at(:users_logout, :user_session)).to be_nil end end @@ -67,7 +67,7 @@ let(:user_session_id) { 'gid://sagittarius/UserSession/0' } it 'raises validation error' do - expect(graphql_data_at(:users_logout, :errors)).to include('Invalid user session') + expect(graphql_data_at(:users_logout, :errors, :message)).to include('Invalid user session') end end end diff --git a/spec/requests/graphql/mutation/users/register_spec.rb b/spec/requests/graphql/mutation/users/register_spec.rb index 48c65187..7b971ce8 100644 --- a/spec/requests/graphql/mutation/users/register_spec.rb +++ b/spec/requests/graphql/mutation/users/register_spec.rb @@ -9,7 +9,7 @@ <<~QUERY mutation($input: UsersRegisterInput!) { usersRegister(input: $input) { - errors + #{error_query} user { id } @@ -53,9 +53,12 @@ expect(graphql_data_at(:users_register, :user)).not_to be_present expect(graphql_data_at(:users_register, :errors)).to include( - "Username can't be blank", - "Email can't be blank", - "Password can't be blank" + { 'attribute' => 'password', 'type' => 'blank' }, + { 'attribute' => 'username', 'type' => 'too_short' }, + { 'attribute' => 'username', 'type' => 'blank' }, + { 'attribute' => 'email', 'type' => 'too_short' }, + { 'attribute' => 'email', 'type' => 'invalid' }, + { 'attribute' => 'email', 'type' => 'blank' } ) end end diff --git a/spec/requests/graphql_spec.rb b/spec/requests/graphql_spec.rb index 438ee797..4d183052 100644 --- a/spec/requests/graphql_spec.rb +++ b/spec/requests/graphql_spec.rb @@ -52,7 +52,7 @@ email: "" password: "" }) { - errors + #{error_query} } } QUERY @@ -62,7 +62,7 @@ expect(response).to have_http_status(:ok) expect_graphql_errors_to_be_empty - expect(graphql_data_at(:users_login, :errors)).to be_present + expect(graphql_data_at(:users_login, :errors, :message)).to be_present end context 'when aliasing the mutation' do @@ -73,7 +73,7 @@ email: "" password: "" }) { - errors + #{error_query} } } QUERY @@ -83,7 +83,7 @@ expect(response).to have_http_status(:ok) expect_graphql_errors_to_be_empty - expect(graphql_data_at(:login, :errors)).to be_present + expect(graphql_data_at(:login, :errors, :message)).to be_present end end @@ -95,14 +95,14 @@ email: "" password: "" }) { - errors + #{error_query} } usersRegister(input: { username: "" email: "" password: "" }) { - errors + #{error_query} } } QUERY diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index ccb78374..48d138eb 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -74,4 +74,18 @@ def expect_graphql_errors_to_be_empty expect(graphql_errors).to be_empty end + + def error_query + %( + errors { + ...on ActiveModelError { + attribute + type + } + ...on MessageError { + message + } + } + ) + end end