From 4df03fc28e164ef8daeb18936741dc953e0069b7 Mon Sep 17 00:00:00 2001 From: Simon Lehericey Date: Wed, 29 Mar 2017 13:37:07 +0200 Subject: [PATCH 1/6] ChampsService: add tests and refactor --- .../private_formulaires_controller.rb | 2 +- .../users/description_controller.rb | 6 ++- app/models/champ.rb | 4 ++ app/services/champs_service.rb | 54 ++++++++++++------- spec/models/champ_shared_example.rb | 27 ++++++++++ spec/services/champs_service_spec.rb | 49 +++++++++++++++++ 6 files changed, 119 insertions(+), 23 deletions(-) create mode 100644 spec/services/champs_service_spec.rb diff --git a/app/controllers/backoffice/private_formulaires_controller.rb b/app/controllers/backoffice/private_formulaires_controller.rb index 8a9283959..81a728085 100644 --- a/app/controllers/backoffice/private_formulaires_controller.rb +++ b/app/controllers/backoffice/private_formulaires_controller.rb @@ -5,7 +5,7 @@ class Backoffice::PrivateFormulairesController < ApplicationController dossier = current_gestionnaire.dossiers.find(params[:dossier_id]) unless params[:champs].nil? - champs_service_errors = ChampsService.save_formulaire dossier.champs_private, params + champs_service_errors = ChampsService.save_champs dossier.champs_private, params 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 71bc8aa29..5dcfb162e 100644 --- a/app/controllers/users/description_controller.rb +++ b/app/controllers/users/description_controller.rb @@ -44,8 +44,10 @@ class Users::DescriptionController < UsersController return redirect_to users_dossier_description_path(dossier_id: @dossier.id) end - unless params[:champs].nil? - champs_service_errors = ChampsService.save_formulaire @dossier.champs, params, mandatory + if params[:champs] + champs_service_errors = ChampsService.save_champs @dossier.champs, + params, + mandatory unless champs_service_errors.empty? flash.alert = (champs_service_errors.inject('') { |acc, error| acc+= error[:message]+'
' }).html_safe diff --git a/app/models/champ.rb b/app/models/champ.rb index e4a6e820e..ce12e32a3 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -28,6 +28,10 @@ class Champ < ActiveRecord::Base same_date? num, '%M' end + def mandatory_and_blank? + mandatory? && value.blank? + end + def same_date? num, compare if type_champ == 'datetime' && !value.nil? if value.to_datetime.strftime(compare) == num diff --git a/app/services/champs_service.rb b/app/services/champs_service.rb index 04fdf9602..31b53973f 100644 --- a/app/services/champs_service.rb +++ b/app/services/champs_service.rb @@ -1,27 +1,41 @@ class ChampsService - def self.save_formulaire champs, params, check_mandatory=true - errors = Array.new + class << self + def save_champs(champs, params, check_mandatory = true) + fill_champs(champs, params) - champs.each do |champ| - champ.value = params[:champs]["'#{champ.id}'"] + champs.select(&:changed?).each(&:save) - if champ.type_champ == 'datetime' - champ.value = params[:champs]["'#{champ.id}'"]+ - ' ' + - params[:time_hour]["'#{champ.id}'"] + - ':' + - params[:time_minute]["'#{champ.id}'"] - end - - if check_mandatory - if champ.mandatory? && (champ.value.nil? || champ.value.blank?) - errors.push({message: "Le champ #{champ.libelle} doit être rempli."}) - end - end - - champ.save if champ.changed? + check_mandatory ? build_error_messages(champs) : [] end - errors + private + + def fill_champs(champs, h) + datetimes, not_datetimes = champs.partition { |c| c.type_champ == 'datetime' } + + not_datetimes.each { |c| c.value = h[:champs]["'#{c.id}'"] } + datetimes.each { |c| c.value = parse_datetime(c.id, h) } + end + + def parse_datetime(champ_id, h) + "#{h[:champs]["'#{champ_id}'"]} #{extract_hour(champ_id, h)}:#{extract_minute(champ_id, h)}" + end + + def extract_hour(champ_id, h) + h[:time_hour]["'#{champ_id}'"] + end + + def extract_minute(champ_id, h) + h[:time_minute]["'#{champ_id}'"] + end + + def build_error_messages(champs) + champs.select(&:mandatory_and_blank?) + .map { |c| build_champ_error_message(c) } + end + + def build_champ_error_message(champ) + { message: "Le champ #{champ.libelle} doit être rempli." } + end end end diff --git a/spec/models/champ_shared_example.rb b/spec/models/champ_shared_example.rb index 9922fd38d..b404154df 100644 --- a/spec/models/champ_shared_example.rb +++ b/spec/models/champ_shared_example.rb @@ -14,6 +14,33 @@ shared_examples 'champ_spec' do it { is_expected.to delegate_method(:order_place).to(:type_de_champ) } end + describe 'mandatory_and_blank?' do + let(:type_de_champ) { TypeDeChamp.new(mandatory: mandatory) } + let(:champ) { Champ.new(type_de_champ: type_de_champ, value: value) } + let(:value) { '' } + let(:mandatory) { true } + + context 'when mandatory and blank' do + it { expect(champ.mandatory_and_blank?).to be(true) } + end + + context 'when not blank' do + let(:value) { 'yop' } + it { expect(champ.mandatory_and_blank?).to be(false) } + end + + context 'when not mandatory' do + let(:mandatory) { false } + it { expect(champ.mandatory_and_blank?).to be(false) } + end + + context 'when not mandatory or blank' do + let(:value) { 'u' } + let(:mandatory) { false } + it { expect(champ.mandatory_and_blank?).to be(false) } + end + end + describe 'data_provide' do let(:champ) { create :champ } diff --git a/spec/services/champs_service_spec.rb b/spec/services/champs_service_spec.rb new file mode 100644 index 000000000..4e8ce53a1 --- /dev/null +++ b/spec/services/champs_service_spec.rb @@ -0,0 +1,49 @@ +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] } + + before :each do + params_hash = { + champs: { + "'#{champ.id}'" => 'yop', + "'#{champ_datetime.id}'" => 'd' + }, + time_hour: { "'#{champ_datetime.id}'" => '12' }, + time_minute: { "'#{champ_datetime.id}'" => '24' } + } + @errors = ChampsService.save_champs(champs, params_hash, check_mandatory) + 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([{ message: 'Le champ mandatory doit être rempli.' }]) + end + end + + context 'check_mandatory is false' do + let(:check_mandatory) { false } + + it 'does not add errors' do + expect(@errors).to match([]) + end + end + end +end From c4e128a506bb70e453be6d8e2756ad99fe59d46e Mon Sep 17 00:00:00 2001 From: Simon Lehericey Date: Wed, 29 Mar 2017 14:35:50 +0200 Subject: [PATCH 2/6] DescriptionController: rename variable --- app/controllers/users/description_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/users/description_controller.rb b/app/controllers/users/description_controller.rb index 5dcfb162e..97df42657 100644 --- a/app/controllers/users/description_controller.rb +++ b/app/controllers/users/description_controller.rb @@ -34,8 +34,8 @@ class Users::DescriptionController < UsersController @champs = @dossier.ordered_champs - mandatory = true - mandatory = !(params[:submit].keys.first == 'brouillon') unless params[:submit].nil? + check_mandatory_fields = true + check_mandatory_fields = !(params[:submit].keys.first == 'brouillon') unless params[:submit].nil? unless @dossier.update_attributes(create_params) @dossier = @dossier.decorate @@ -47,7 +47,7 @@ class Users::DescriptionController < UsersController if params[:champs] champs_service_errors = ChampsService.save_champs @dossier.champs, params, - mandatory + check_mandatory_fields unless champs_service_errors.empty? flash.alert = (champs_service_errors.inject('') { |acc, error| acc+= error[:message]+'
' }).html_safe @@ -71,7 +71,7 @@ class Users::DescriptionController < UsersController end - if mandatory + if check_mandatory_fields if @dossier.draft? @dossier.initiated! NotificationMailer.send_notification(@dossier, @dossier.procedure.initiated_mail).deliver_now! From 3a9065e4c116b29946b053e4071af0ff159ad29a Mon Sep 17 00:00:00 2001 From: Simon Lehericey Date: Wed, 29 Mar 2017 15:35:55 +0200 Subject: [PATCH 3/6] DescriptionController: remove useless code --- app/controllers/users/description_controller.rb | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/app/controllers/users/description_controller.rb b/app/controllers/users/description_controller.rb index 97df42657..c4944775f 100644 --- a/app/controllers/users/description_controller.rb +++ b/app/controllers/users/description_controller.rb @@ -37,13 +37,6 @@ class Users::DescriptionController < UsersController check_mandatory_fields = true check_mandatory_fields = !(params[:submit].keys.first == 'brouillon') unless params[:submit].nil? - unless @dossier.update_attributes(create_params) - @dossier = @dossier.decorate - - flash.alert = @dossier.errors.full_messages.join('
').html_safe - return redirect_to users_dossier_description_path(dossier_id: @dossier.id) - end - if params[:champs] champs_service_errors = ChampsService.save_champs @dossier.champs, params, @@ -139,9 +132,4 @@ class Users::DescriptionController < UsersController redirect_to url_for(users_dossier_path(@dossier.id)) end end - - def create_params - params.permit() - end - end From 191b29aff5c34bf235ea5a942252a5157889df78 Mon Sep 17 00:00:00 2001 From: Simon Lehericey Date: Mon, 10 Apr 2017 11:45:16 +0200 Subject: [PATCH 4/6] DescriptionController: simplify update logic --- .../users/description_controller.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/app/controllers/users/description_controller.rb b/app/controllers/users/description_controller.rb index c4944775f..9798ea3cf 100644 --- a/app/controllers/users/description_controller.rb +++ b/app/controllers/users/description_controller.rb @@ -35,7 +35,7 @@ class Users::DescriptionController < UsersController @champs = @dossier.ordered_champs check_mandatory_fields = true - check_mandatory_fields = !(params[:submit].keys.first == 'brouillon') unless params[:submit].nil? + check_mandatory_fields = !(params[:submit].keys.first == 'brouillon') if params[:submit] if params[:champs] champs_service_errors = ChampsService.save_champs @dossier.champs, @@ -48,22 +48,20 @@ class Users::DescriptionController < UsersController end end - if @procedure.cerfa_flag? - unless params[:cerfa_pdf].nil? - cerfa = Cerfa.new(content: params[:cerfa_pdf], dossier: @dossier, user: current_user) - unless cerfa.save - flash.alert = cerfa.errors.full_messages.join('
').html_safe - return redirect_to users_dossier_description_path(dossier_id: @dossier.id) - end + if @procedure.cerfa_flag? && params[:cerfa_pdf] + cerfa = Cerfa.new(content: params[:cerfa_pdf], dossier: @dossier, user: current_user) + unless cerfa.save + flash.alert = cerfa.errors.full_messages.join('
').html_safe + return redirect_to users_dossier_description_path(dossier_id: @dossier.id) end end - unless (errors_upload = PiecesJustificativesService.upload!(@dossier, current_user, params)).empty? + errors_upload = PiecesJustificativesService.upload!(@dossier, current_user, params) + unless errors_upload.empty? flash.alert = errors_upload.html_safe return redirect_to users_dossier_description_path(dossier_id: @dossier.id) end - if check_mandatory_fields if @dossier.draft? @dossier.initiated! From d14a75c24a1fdb7211eebf414e47bf303e35c2aa Mon Sep 17 00:00:00 2001 From: Simon Lehericey Date: Wed, 29 Mar 2017 23:18:37 +0200 Subject: [PATCH 5/6] DescriptionController: refactor check_mandatory_fields --- app/controllers/users/description_controller.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/controllers/users/description_controller.rb b/app/controllers/users/description_controller.rb index 9798ea3cf..c792777ae 100644 --- a/app/controllers/users/description_controller.rb +++ b/app/controllers/users/description_controller.rb @@ -34,8 +34,7 @@ class Users::DescriptionController < UsersController @champs = @dossier.ordered_champs - check_mandatory_fields = true - check_mandatory_fields = !(params[:submit].keys.first == 'brouillon') if params[:submit] + check_mandatory_fields = !draft_submission? if params[:champs] champs_service_errors = ChampsService.save_champs @dossier.champs, @@ -116,6 +115,10 @@ class Users::DescriptionController < UsersController private + def draft_submission? + params[:submit] && params[:submit].keys.first == 'brouillon' + end + def check_autorisation_donnees @dossier = current_user_dossier From ccf956ec0162303d9d21cec4d137b7337e4681ef Mon Sep 17 00:00:00 2001 From: Simon Lehericey Date: Thu, 30 Mar 2017 09:31:46 +0200 Subject: [PATCH 6/6] DescriptionController: refactor redirection for draft --- app/controllers/users/description_controller.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/controllers/users/description_controller.rb b/app/controllers/users/description_controller.rb index c792777ae..e8d83cd20 100644 --- a/app/controllers/users/description_controller.rb +++ b/app/controllers/users/description_controller.rb @@ -61,17 +61,16 @@ class Users::DescriptionController < UsersController return redirect_to users_dossier_description_path(dossier_id: @dossier.id) end - if check_mandatory_fields + if draft_submission? + flash.notice = 'Votre brouillon a bien été sauvegardé.' + redirect_to url_for(controller: :dossiers, action: :index, liste: :brouillon) + else if @dossier.draft? @dossier.initiated! NotificationMailer.send_notification(@dossier, @dossier.procedure.initiated_mail).deliver_now! end - flash.notice = 'Félicitations, votre demande a bien été enregistrée.' redirect_to url_for(controller: :recapitulatif, action: :show, dossier_id: @dossier.id) - else - flash.notice = 'Votre brouillon a bien été sauvegardé.' - redirect_to url_for(controller: :dossiers, action: :index, liste: :brouillon) end end