From 65d650eba60be57712a11b4135ce199ab475dacd Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 10 Dec 2020 16:23:24 +0100 Subject: [PATCH] Better graphql mutation error messages --- app/graphql/mutations/base_mutation.rb | 28 ++++++++++++------- app/graphql/mutations/dossier_accepter.rb | 24 +++++++++------- app/graphql/mutations/dossier_archiver.rb | 14 +++++----- .../dossier_changer_groupe_instructeur.rb | 17 ++++++----- .../mutations/dossier_classer_sans_suite.rb | 23 ++++++++------- .../mutations/dossier_envoyer_message.rb | 16 +++++++---- .../dossier_passer_en_instruction.rb | 13 ++++----- app/graphql/mutations/dossier_refuser.rb | 23 ++++++++------- .../api/v2/graphql_controller_spec.rb | 17 +++++++++-- 9 files changed, 106 insertions(+), 69 deletions(-) diff --git a/app/graphql/mutations/base_mutation.rb b/app/graphql/mutations/base_mutation.rb index ec677ef18..c42fb56f8 100644 --- a/app/graphql/mutations/base_mutation.rb +++ b/app/graphql/mutations/base_mutation.rb @@ -1,16 +1,24 @@ module Mutations class BaseMutation < GraphQL::Schema::RelayClassicMutation + private + def validate_blob(blob_id) - if blob_id.present? - begin - blob = ActiveStorage::Blob.find_signed(blob_id) - blob.identify - nil - rescue ActiveStorage::FileNotFoundError - return { errors: ['Le fichier n’a pas été correctement téléversé sur le serveur de stockage'] } - rescue ActiveSupport::MessageVerifier::InvalidSignature - return { errors: ['L’identifiant du fichier téléversé est invalide'] } - end + begin + blob = ActiveStorage::Blob.find_signed(blob_id) + blob.identify + true + rescue ActiveStorage::FileNotFoundError + return false, { errors: ['Le fichier n’a pas été correctement téléversé sur le serveur de stockage'] } + rescue ActiveSupport::MessageVerifier::InvalidSignature + return false, { errors: ['Le hash du fichier téléversé est invalide'] } + end + end + + def dossier_authorized_for?(dossier, instructeur) + if instructeur.is_a?(Instructeur) && instructeur.dossiers.exists?(id: dossier.id) + true + else + return false, { errors: ['L’instructeur n’a pas les droits d’accès à ce dossier'] } end end end diff --git a/app/graphql/mutations/dossier_accepter.rb b/app/graphql/mutations/dossier_accepter.rb index f90af1756..bbb5bd9f2 100644 --- a/app/graphql/mutations/dossier_accepter.rb +++ b/app/graphql/mutations/dossier_accepter.rb @@ -13,21 +13,25 @@ module Mutations field :errors, [Types::ValidationErrorType], null: true def resolve(dossier:, instructeur:, motivation: nil, justificatif: nil) - if dossier.en_instruction? - errors = validate_blob(justificatif) - if errors - return errors - end - dossier.accepter!(instructeur, motivation, justificatif) + dossier.accepter!(instructeur, motivation, justificatif) - { dossier: dossier } + { dossier: dossier } + end + + def ready?(justificatif: nil, **args) + if justificatif.present? + validate_blob(justificatif) else - { errors: ["Le dossier est déjà #{dossier_display_state(dossier, lower: true)}"] } + true end end - def authorized?(dossier:, instructeur:, motivation: nil, justificatif: nil) - instructeur.is_a?(Instructeur) && instructeur.dossiers.exists?(id: dossier.id) + def authorized?(dossier:, instructeur:, **args) + if !dossier.en_instruction? + return false, { errors: ["Le dossier est déjà #{dossier_display_state(dossier, lower: true)}"] } + end + + dossier_authorized_for?(dossier, instructeur) end end end diff --git a/app/graphql/mutations/dossier_archiver.rb b/app/graphql/mutations/dossier_archiver.rb index aad9e79e0..26d4019a7 100644 --- a/app/graphql/mutations/dossier_archiver.rb +++ b/app/graphql/mutations/dossier_archiver.rb @@ -9,17 +9,17 @@ module Mutations field :errors, [Types::ValidationErrorType], null: true def resolve(dossier:, instructeur:) - if dossier.termine? - dossier.archiver!(instructeur) + dossier.archiver!(instructeur) - { dossier: dossier } - else - { errors: ["Un dossier ne peut être archivé qu’une fois le traitement terminé"] } - end + { dossier: dossier } end def authorized?(dossier:, instructeur:) - instructeur.is_a?(Instructeur) && instructeur.dossiers.exists?(id: dossier.id) + if !dossier.termine? + return false, { errors: ["Un dossier ne peut être archivé qu’une fois le traitement terminé"] } + end + + dossier_authorized_for?(dossier, instructeur) end end end diff --git a/app/graphql/mutations/dossier_changer_groupe_instructeur.rb b/app/graphql/mutations/dossier_changer_groupe_instructeur.rb index ccb2ca906..0109d00aa 100644 --- a/app/graphql/mutations/dossier_changer_groupe_instructeur.rb +++ b/app/graphql/mutations/dossier_changer_groupe_instructeur.rb @@ -11,16 +11,19 @@ module Mutations field :errors, [Types::ValidationErrorType], null: true def resolve(dossier:, groupe_instructeur:) - if dossier.groupe_instructeur == groupe_instructeur - { errors: ["Le dossier est déjà avec le grope instructeur: '#{groupe_instructeur.label}'"] } - else - dossier.update!(groupe_instructeur: groupe_instructeur) - { dossier: dossier } - end + dossier.update!(groupe_instructeur: groupe_instructeur) + + { dossier: dossier } end def authorized?(dossier:, groupe_instructeur:) - dossier.groupe_instructeur.procedure == groupe_instructeur.procedure + if dossier.groupe_instructeur == groupe_instructeur + return false, { errors: ["Le dossier est déjà avec le grope instructeur: '#{groupe_instructeur.label}'"] } + elsif dossier.groupe_instructeur.procedure != groupe_instructeur.procedure + return false, { errors: ["Le groupe instructeur '#{groupe_instructeur.label}' n’appartient pas à la même démarche que le dossier"] } + else + true + end end end end diff --git a/app/graphql/mutations/dossier_classer_sans_suite.rb b/app/graphql/mutations/dossier_classer_sans_suite.rb index e858d8688..bf66add79 100644 --- a/app/graphql/mutations/dossier_classer_sans_suite.rb +++ b/app/graphql/mutations/dossier_classer_sans_suite.rb @@ -13,21 +13,24 @@ module Mutations field :errors, [Types::ValidationErrorType], null: true def resolve(dossier:, instructeur:, motivation:, justificatif: nil) - if dossier.en_instruction? - errors = validate_blob(justificatif) - if errors - return errors - end - dossier.classer_sans_suite!(instructeur, motivation, justificatif) + dossier.classer_sans_suite!(instructeur, motivation, justificatif) - { dossier: dossier } + { dossier: dossier } + end + + def ready?(justificatif: nil, **args) + if justificatif.present? + validate_blob(justificatif) else - { errors: ["Le dossier est déjà #{dossier_display_state(dossier, lower: true)}"] } + true end end - def authorized?(dossier:, instructeur:, motivation:, justificatif: nil) - instructeur.is_a?(Instructeur) && instructeur.dossiers.exists?(id: dossier.id) + def authorized?(dossier:, instructeur:, **args) + if !dossier.en_instruction? + return false, { errors: ["Le dossier est déjà #{dossier_display_state(dossier, lower: true)}"] } + end + dossier_authorized_for?(dossier, instructeur) end end end diff --git a/app/graphql/mutations/dossier_envoyer_message.rb b/app/graphql/mutations/dossier_envoyer_message.rb index 603be72f4..c271c43b3 100644 --- a/app/graphql/mutations/dossier_envoyer_message.rb +++ b/app/graphql/mutations/dossier_envoyer_message.rb @@ -11,10 +11,6 @@ module Mutations field :errors, [Types::ValidationErrorType], null: true def resolve(dossier:, instructeur:, body:, attachment: nil) - errors = validate_blob(attachment) - if errors - return errors - end message = CommentaireService.build(instructeur, dossier, body: body, piece_jointe: attachment) if message.save @@ -24,8 +20,16 @@ module Mutations end end - def authorized?(dossier:, instructeur:, body:, attachment: nil) - instructeur.is_a?(Instructeur) && instructeur.dossiers.exists?(id: dossier.id) + def ready?(attachment: nil, **args) + if attachment.present? + validate_blob(attachment) + else + true + end + end + + def authorized?(dossier:, instructeur:, **args) + dossier_authorized_for?(dossier, instructeur) end end end diff --git a/app/graphql/mutations/dossier_passer_en_instruction.rb b/app/graphql/mutations/dossier_passer_en_instruction.rb index 5ddd118ad..fa34ead70 100644 --- a/app/graphql/mutations/dossier_passer_en_instruction.rb +++ b/app/graphql/mutations/dossier_passer_en_instruction.rb @@ -11,17 +11,16 @@ module Mutations field :errors, [Types::ValidationErrorType], null: true def resolve(dossier:, instructeur:) - if dossier.en_construction? - dossier.passer_en_instruction!(instructeur) + dossier.passer_en_instruction!(instructeur) - { dossier: dossier } - else - { errors: ["Le dossier est déjà #{dossier_display_state(dossier, lower: true)}"] } - end + { dossier: dossier } end def authorized?(dossier:, instructeur:) - instructeur.is_a?(Instructeur) && instructeur.dossiers.exists?(id: dossier.id) + if !dossier.en_construction? + return false, { errors: ["Le dossier est déjà #{dossier_display_state(dossier, lower: true)}"] } + end + dossier_authorized_for?(dossier, instructeur) end end end diff --git a/app/graphql/mutations/dossier_refuser.rb b/app/graphql/mutations/dossier_refuser.rb index 028d62d4e..18abc2a7a 100644 --- a/app/graphql/mutations/dossier_refuser.rb +++ b/app/graphql/mutations/dossier_refuser.rb @@ -13,21 +13,24 @@ module Mutations field :errors, [Types::ValidationErrorType], null: true def resolve(dossier:, instructeur:, motivation:, justificatif: nil) - if dossier.en_instruction? - errors = validate_blob(justificatif) - if errors - return errors - end - dossier.refuser!(instructeur, motivation, justificatif) + dossier.refuser!(instructeur, motivation, justificatif) - { dossier: dossier } + { dossier: dossier } + end + + def ready?(justificatif: nil, **args) + if justificatif.present? + validate_blob(justificatif) else - { errors: ["Le dossier est déjà #{dossier_display_state(dossier, lower: true)}"] } + true end end - def authorized?(dossier:, instructeur:, motivation:, justificatif: nil) - instructeur.is_a?(Instructeur) && instructeur.dossiers.exists?(id: dossier.id) + def authorized?(dossier:, instructeur:, **args) + if !dossier.en_instruction? + return false, { errors: ["Le dossier est déjà #{dossier_display_state(dossier, lower: true)}"] } + end + dossier_authorized_for?(dossier, instructeur) end end end diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb index d30329d22..fa07ab6a0 100644 --- a/spec/controllers/api/v2/graphql_controller_spec.rb +++ b/spec/controllers/api/v2/graphql_controller_spec.rb @@ -627,7 +627,7 @@ describe API::V2::GraphqlController do it "should fail" do expect(gql_errors).to eq(nil) expect(gql_data).to eq(dossierEnvoyerMessage: { - errors: [{ message: "L’identifiant du fichier téléversé est invalide" }], + errors: [{ message: "Le hash du fichier téléversé est invalide" }], message: nil }) end @@ -636,11 +636,12 @@ describe API::V2::GraphqlController do describe 'dossierPasserEnInstruction' do let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure: procedure) } + let(:instructeur_id) { instructeur.to_typed_id } let(:query) do "mutation { dossierPasserEnInstruction(input: { dossierId: \"#{dossier.to_typed_id}\", - instructeurId: \"#{instructeur.to_typed_id}\" + instructeurId: \"#{instructeur_id}\" }) { dossier { id @@ -680,6 +681,18 @@ describe API::V2::GraphqlController do }) end end + + context 'instructeur error' do + let(:instructeur_id) { create(:instructeur).to_typed_id } + + it "should fail" do + expect(gql_errors).to eq(nil) + expect(gql_data).to eq(dossierPasserEnInstruction: { + errors: [{ message: 'L’instructeur n’a pas les droits d’accès à ce dossier' }], + dossier: nil + }) + end + end end describe 'dossierClasserSansSuite' do