Merge pull request #12 from code0-tech/improve_error_models

Improve error models on api responses

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

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,

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

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

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

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

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

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)

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

docs/content/graphql/mutation/echo.md

@@ -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. |

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 |

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 |

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 |

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 |
+

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 |
+

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)

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

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

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

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

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

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

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