Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error models on api responses #12

Merged
merged 1 commit into from
Dec 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/graphql/mutations/base_mutation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 0 additions & 5 deletions app/graphql/mutations/echo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion app/graphql/mutations/users/logout.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions app/graphql/types/errors/active_model_error_type.rb
Original file line number Diff line number Diff line change
@@ -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
21 changes: 21 additions & 0 deletions app/graphql/types/errors/error_type.rb
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions app/graphql/types/errors/message_error_type.rb
Original file line number Diff line number Diff line change
@@ -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
8 changes: 6 additions & 2 deletions app/services/service_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion app/services/user_login_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions app/services/user_logout_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
3 changes: 1 addition & 2 deletions docs/content/graphql/mutation/echo.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ 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

| 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. |
2 changes: 1 addition & 1 deletion docs/content/graphql/mutation/userslogin.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
2 changes: 1 addition & 1 deletion docs/content/graphql/mutation/userslogout.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
2 changes: 1 addition & 1 deletion docs/content/graphql/mutation/usersregister.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
13 changes: 13 additions & 0 deletions docs/content/graphql/object/activemodelerror.md
Original file line number Diff line number Diff line change
@@ -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 |

12 changes: 12 additions & 0 deletions docs/content/graphql/object/messageerror.md
Original file line number Diff line number Diff line change
@@ -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 |

10 changes: 10 additions & 0 deletions docs/content/graphql/union/error.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
title: Error
---

Objects that can present an error

## Possible types

- [`ActiveModelError`](../object/activemodelerror.md)
- [`MessageError`](../object/messageerror.md)
7 changes: 7 additions & 0 deletions lib/sagittarius/graphql/error_message_container.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

module Sagittarius
module Graphql
ErrorMessageContainer = Struct.new(:message)
end
end
15 changes: 15 additions & 0 deletions spec/graphql/types/errors/active_model_error_type_spec.rb
Original file line number Diff line number Diff line change
@@ -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
30 changes: 30 additions & 0 deletions spec/graphql/types/errors/error_type_spec.rb
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions spec/graphql/types/errors/message_error_type_spec.rb
Original file line number Diff line number Diff line change
@@ -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
8 changes: 4 additions & 4 deletions spec/requests/graphql/mutation/users/login_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<<~QUERY
mutation($input: UsersLoginInput!) {
usersLogin(input: $input) {
errors
#{error_query}
userSession {
id
token
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions spec/requests/graphql/mutation/users/logout_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<<~QUERY
mutation($input: UsersLogoutInput!) {
usersLogout(input: $input) {
errors
#{error_query}
userSession {
id
active
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
11 changes: 7 additions & 4 deletions spec/requests/graphql/mutation/users/register_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<<~QUERY
mutation($input: UsersRegisterInput!) {
usersRegister(input: $input) {
errors
#{error_query}
user {
id
}
Expand Down Expand Up @@ -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
Expand Down
Loading