From 2b969ef575240578b0154b58a57fa679a78716f8 Mon Sep 17 00:00:00 2001 From: Simon Lehericey Date: Thu, 30 Mar 2017 16:12:01 +0200 Subject: [PATCH 1/2] MandatoryAttachment: group missing errors logic together and show errors only if not draft --- .../private_formulaires_controller.rb | 3 +- .../users/description_controller.rb | 14 +- app/services/champs_service.rb | 12 +- app/services/pieces_justificatives_service.rb | 2 - spec/services/champs_service_spec.rb | 45 +++--- .../pieces_justificatives_service_spec.rb | 129 ++++++++++-------- 6 files changed, 103 insertions(+), 102 deletions(-) diff --git a/app/controllers/backoffice/private_formulaires_controller.rb b/app/controllers/backoffice/private_formulaires_controller.rb index 3c62ad362..0353b02f3 100644 --- a/app/controllers/backoffice/private_formulaires_controller.rb +++ b/app/controllers/backoffice/private_formulaires_controller.rb @@ -5,7 +5,8 @@ class Backoffice::PrivateFormulairesController < ApplicationController dossier = current_gestionnaire.dossiers.find(params[:dossier_id]) unless params[:champs].nil? - champs_service_errors = ChampsService.save_champs dossier.champs_private, params + ChampsService.save_champs dossier.champs_private, params + champs_service_errors = ChampsService.build_error_messages(dossier.champs_private) if champs_service_errors.empty? flash[:notice] = "Formulaire enregistré" diff --git a/app/controllers/users/description_controller.rb b/app/controllers/users/description_controller.rb index 69e17b4a8..bdd38cce9 100644 --- a/app/controllers/users/description_controller.rb +++ b/app/controllers/users/description_controller.rb @@ -28,13 +28,8 @@ class Users::DescriptionController < UsersController procedure = dossier.procedure return head :forbidden unless dossier.can_be_initiated? - check_mandatory_fields = !draft_submission? - if params[:champs] - champs_service_errors = - ChampsService.save_champs(dossier.champs, params, check_mandatory_fields) - return redirect_to_description_with_errors(dossier, champs_service_errors) if champs_service_errors.any? - end + ChampsService.save_champs(dossier.champs, params) if params[:champs] if procedure.cerfa_flag? && params[:cerfa_pdf] cerfa = Cerfa.new(content: params[:cerfa_pdf], dossier: dossier, user: current_user) @@ -44,6 +39,13 @@ class Users::DescriptionController < UsersController errors_upload = PiecesJustificativesService.upload!(dossier, current_user, params) return redirect_to_description_with_errors(dossier, errors_upload) if errors_upload.any? + if params[:champs] && !draft_submission? + errors = + ChampsService.build_error_messages(dossier.champs) + + PiecesJustificativesService.missing_pj_error_messages(dossier) + return redirect_to_description_with_errors(dossier, errors) if errors.any? + end + if draft_submission? flash.notice = 'Votre brouillon a bien été sauvegardé.' redirect_to url_for(controller: :dossiers, action: :index, liste: :brouillon) diff --git a/app/services/champs_service.rb b/app/services/champs_service.rb index b35ec4d7b..40c46a993 100644 --- a/app/services/champs_service.rb +++ b/app/services/champs_service.rb @@ -1,11 +1,14 @@ class ChampsService class << self - def save_champs(champs, params, check_mandatory = true) + def save_champs(champs, params) fill_champs(champs, params) champs.select(&:changed?).each(&:save) + end - check_mandatory ? build_error_messages(champs) : [] + def build_error_messages(champs) + champs.select(&:mandatory_and_blank?) + .map { |c| "Le champ #{c.libelle} doit être rempli." } end private @@ -28,10 +31,5 @@ class ChampsService def extract_minute(champ_id, h) h[:time_minute]["'#{champ_id}'"] end - - def build_error_messages(champs) - champs.select(&:mandatory_and_blank?) - .map { |c| "Le champ #{c.libelle} doit être rempli." } - end end end diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index 1298897e5..8003e3c92 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -13,8 +13,6 @@ class PiecesJustificativesService errors += without_virus .map { |tpj, content| save_pj(content, dossier, tpj, user) } .compact() - - errors += missing_pj_error_messages(dossier) end def self.upload_one! dossier, user, params diff --git a/spec/services/champs_service_spec.rb b/spec/services/champs_service_spec.rb index ee0047ea2..67f4c04d1 100644 --- a/spec/services/champs_service_spec.rb +++ b/spec/services/champs_service_spec.rb @@ -1,15 +1,15 @@ require 'spec_helper' describe ChampsService do - describe 'save_champs' do - let!(:champ) { Champ.create(value: 'toto', type_de_champ: TypeDeChamp.new) } - let!(:champ_mandatory_empty) { Champ.create(type_de_champ: TypeDeChamp.new(libelle: 'mandatory', mandatory: true)) } - let!(:champ_datetime) do - champ_datetime = TypeDeChamp.new(type_champ: 'datetime') - Champ.create(type_de_champ: champ_datetime) - end - let!(:champs) { [champ, champ_mandatory_empty, champ_datetime] } + let!(:champ) { Champ.create(value: 'toto', type_de_champ: TypeDeChamp.new) } + let!(:champ_mandatory_empty) { Champ.create(type_de_champ: TypeDeChamp.new(libelle: 'mandatory', mandatory: true)) } + let!(:champ_datetime) do + champ_datetime = TypeDeChamp.new(type_champ: 'datetime') + Champ.create(type_de_champ: champ_datetime) + end + let!(:champs) { [champ, champ_mandatory_empty, champ_datetime] } + describe 'save_champs' do before :each do params_hash = { champs: { @@ -19,31 +19,22 @@ describe ChampsService do time_hour: { "'#{champ_datetime.id}'" => '12' }, time_minute: { "'#{champ_datetime.id}'" => '24' } } - @errors = ChampsService.save_champs(champs, params_hash, check_mandatory) + ChampsService.save_champs(champs, params_hash) champs.each(&:reload) end - context 'check_mandatory is true' do - let(:check_mandatory) { true } - it 'saves the changed champ' do - expect(champ.value).to eq('yop') - end - - it 'parses and save the date' do - expect(champ_datetime.value).to eq('d 12:24') - end - - it 'adds error for the missing mandatory champ' do - expect(@errors).to match(['Le champ mandatory doit être rempli.']) - end + it 'saves the changed champ' do + expect(champ.value).to eq('yop') end - context 'check_mandatory is false' do - let(:check_mandatory) { false } + it 'parses and save the date' do + expect(champ_datetime.value).to eq('d 12:24') + end + end - it 'does not add errors' do - expect(@errors).to match([]) - end + describe 'build_error_message' do + it 'adds error for the missing mandatory champ' do + expect(ChampsService.build_error_messages(champs)).to match(['Le champ mandatory doit être rempli.']) end end end diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb index d6ac03c87..0e6704515 100644 --- a/spec/services/pieces_justificatives_service_spec.rb +++ b/spec/services/pieces_justificatives_service_spec.rb @@ -8,74 +8,85 @@ describe PiecesJustificativesService do allow(ClamavService).to receive(:safe_file?).and_return(safe_file) end + let(:hash) { {} } + let!(:tpj_not_mandatory) do + TypeDePieceJustificative.create(libelle: 'not mandatory', mandatory: false) + end + let!(:tpj_mandatory) do + TypeDePieceJustificative.create(libelle: 'justificatif', mandatory: true) + end + let(:procedure) { Procedure.create(types_de_piece_justificative: tpjs) } + let(:dossier) { Dossier.create(procedure: procedure) } + let(:errors) { PiecesJustificativesService.upload!(dossier, user, hash) } + let(:tpjs) { [tpj_not_mandatory] } + describe 'self.upload!' do - let(:hash) { {} } - let!(:tpj_not_mandatory) do - TypeDePieceJustificative.create(libelle: 'not mandatory', mandatory: false) - end - let!(:tpj_mandatory) do - TypeDePieceJustificative.create(libelle: 'justificatif', mandatory: true) - end - let(:procedure) { Procedure.create(types_de_piece_justificative: tpjs) } - let(:dossier) { Dossier.create(procedure: procedure) } - let(:errors) { PiecesJustificativesService.upload!(dossier, user, hash) } - - context 'when no piece justificative is required' do - let(:tpjs) { [tpj_not_mandatory] } - - context 'when no params are given' do - it { expect(errors).to eq([]) } - end - - context 'when sometihing wrong with file save' do - let(:hash) do - { - "piece_justificative_#{tpj_not_mandatory.id}" => - double(path: '', original_filename: 'filename') - } - end - - it { expect(errors).to match(["le fichier filename (not mandatory) n'a pas pu être sauvegardé"]) } - end - - context 'when a virus is provided' do - let(:safe_file) { false } - let(:hash) do - { - "piece_justificative_#{tpj_not_mandatory.id}" => - double(path: '', original_filename: 'bad_file') - } - end - - it { expect(errors).to match(['bad_file : virus détecté']) } - end + context 'when no params are given' do + it { expect(errors).to eq([]) } end - context 'when a piece justificative is required' do - let(:tpjs) { [tpj_mandatory] } - - context 'when no params are given' do - it { expect(errors).to match(['La pièce jointe justificatif doit être fournie.']) } + context 'when there is something wrong with file save' do + let(:hash) do + { + "piece_justificative_#{tpj_not_mandatory.id}" => + double(path: '', original_filename: 'filename') + } end - context 'when the piece justificative is provided' do - before :each do - # we are messing around piece_justificative - # because directly doubling carrierwave params seems complicated + it { expect(errors).to match(["le fichier filename (not mandatory) n'a pas pu être sauvegardé"]) } + end - allow(PiecesJustificativesService).to receive(:save_pj).and_return(nil) - piece_justificative_double = double(type_de_piece_justificative: tpj_mandatory) - expect(dossier).to receive(:pieces_justificatives).and_return([piece_justificative_double]) - end + context 'when a virus is provided' do + let(:safe_file) { false } + let(:hash) do + { + "piece_justificative_#{tpj_not_mandatory.id}" => + double(path: '', original_filename: 'bad_file') + } + end - let(:hash) do - { - "piece_justificative_#{tpj_mandatory.id}" => double(path: '') - } - end + it { expect(errors).to match(['bad_file : virus détecté']) } + end - it { expect(errors).to match([]) } + context 'when a regular file is provided' do + let(:content) { double(path: '', original_filename: 'filename') } + let(:hash) do + { + "piece_justificative_#{tpj_not_mandatory.id}" => + content + } + end + + before :each do + expect(PiecesJustificativesService).to receive(:save_pj) + .with(content, dossier, tpj_not_mandatory, user) + .and_return(nil) + end + + it 'is saved' do + expect(errors).to match([]) end end end + + describe 'missing_pj_error_messages' do + let(:errors) { PiecesJustificativesService.missing_pj_error_messages(dossier) } + let(:tpjs) { [tpj_mandatory] } + + context 'when no params are given' do + it { expect(errors).to match(['La pièce jointe justificatif doit être fournie.']) } + end + + context 'when the piece justificative is provided' do + before :each do + # we are messing around piece_justificative + # because directly doubling carrierwave params seems complicated + + piece_justificative_double = double(type_de_piece_justificative: tpj_mandatory) + expect(dossier).to receive(:pieces_justificatives).and_return([piece_justificative_double]) + end + + it { expect(errors).to match([]) } + end + end end From ce4a23ec7ff97495db7fed8edcd90935e459841b Mon Sep 17 00:00:00 2001 From: Simon Lehericey Date: Thu, 30 Mar 2017 16:12:26 +0200 Subject: [PATCH 2/2] MandatoryAttachment: show * near mandatory attachment --- .../_pieces_justificatives.html.haml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/views/users/description/_pieces_justificatives.html.haml b/app/views/users/description/_pieces_justificatives.html.haml index e3a109ba9..bb6604478 100644 --- a/app/views/users/description/_pieces_justificatives.html.haml +++ b/app/views/users/description/_pieces_justificatives.html.haml @@ -18,24 +18,24 @@ - else %input{ type: 'file', name: 'cerfa_pdf', id: 'cerfa_pdf', accept: PieceJustificative.accept_format, :max_file_size => 6.megabytes } - - dossier.types_de_piece_justificative.order('order_place ASC').each do |type_de_piece_justificative| + - dossier.types_de_piece_justificative.order('order_place ASC').each do |tpj| %tr %th.piece-libelle - = type_de_piece_justificative.libelle + = tpj.mandatory ? tpj.libelle + ' *' : tpj.libelle %td - - unless type_de_piece_justificative.lien_demarche.blank? + - unless tpj.lien_demarche.blank? %em Récupérer le formulaire vierge pour mon dossier : - = link_to "Télécharger", type_de_piece_justificative.lien_demarche, target: :blank + = link_to "Télécharger", tpj.lien_demarche, target: :blank %td - - if type_de_piece_justificative.api_entreprise - %span.text-success{ id: "piece_justificative_#{type_de_piece_justificative.id}" } Nous l'avons récupéré pour vous. + - if tpj.api_entreprise + %span.text-success{ id: "piece_justificative_#{tpj.id}" } Nous l'avons récupéré pour vous. - else - - if dossier.retrieve_last_piece_justificative_by_type(type_de_piece_justificative.id).nil? - = file_field_tag "piece_justificative_#{type_de_piece_justificative.id}", accept: PieceJustificative.accept_format, :max_file_size => 6.megabytes + - if dossier.retrieve_last_piece_justificative_by_type(tpj.id).nil? + = file_field_tag "piece_justificative_#{tpj.id}", accept: PieceJustificative.accept_format, :max_file_size => 6.megabytes - else %span.btn.btn-sm.btn-file.btn-success Modifier - = file_field_tag "piece_justificative_#{type_de_piece_justificative.id}", accept: PieceJustificative.accept_format, :max_file_size => 6.megabytes + = file_field_tag "piece_justificative_#{tpj.id}", accept: PieceJustificative.accept_format, :max_file_size => 6.megabytes