Merge pull request #82 from sgmap/pieces_justificatives_refactor

Pieces justificatives refactor

app/assets/stylesheets/pieces_justificatives_fields.scss

@@ -0,0 +1,3 @@
+.pieces_justificatives_fields {
+  .form-inline > .form-group { vertical-align: top; }
+}

app/controllers/admin/pieces_justificatives_controller.rb

@@ -25,7 +25,7 @@ class Admin::PiecesJustificativesController < AdminController
   def update_params
     params
       .require(:procedure)
-      .permit(types_de_piece_justificative_attributes: [:libelle, :description, :id, :order_place, :lien_demarche])
+      .permit(types_de_piece_justificative_attributes: [:libelle, :description, :id, :order_place, :mandatory, :lien_demarche])
   end
 
   def move_up

app/controllers/backoffice/private_formulaires_controller.rb

@@ -10,7 +10,7 @@ class Backoffice::PrivateFormulairesController < ApplicationController
       if champs_service_errors.empty?
         flash[:notice] = "Formulaire enregistré"
       else
-        flash[:alert] = (champs_service_errors.inject('') { |acc, error| acc+= error[:message]+'<br>' }).html_safe
+        flash[:alert] = champs_service_errors.join('<br>').html_safe
       end
     end
 

app/controllers/users/description_controller.rb

@@ -38,25 +38,16 @@ class Users::DescriptionController < UsersController
                                                         params,
                                                         check_mandatory_fields
 
-      unless champs_service_errors.empty?
-        flash.alert = (champs_service_errors.inject('') { |acc, error| acc+= error[:message]+'<br>' }).html_safe
-        return redirect_to users_dossier_description_path(dossier_id: @dossier.id)
-      end
+      return redirect_to_description_with_errors(@dossier, champs_service_errors) if champs_service_errors.any?
     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('<br />').html_safe
-        return redirect_to users_dossier_description_path(dossier_id: @dossier.id)
-      end
+      return redirect_to_description_with_errors(@dossier, cerfa.errors.full_messages) unless cerfa.save
     end
 
     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
+    return redirect_to_description_with_errors(@dossier, errors_upload) if errors_upload.any?
 
     if draft_submission?
       flash.notice = 'Votre brouillon a bien été sauvegardé.'
@@ -88,9 +79,9 @@ class Users::DescriptionController < UsersController
 
     if !((errors_upload = PiecesJustificativesService.upload!(@dossier, current_user, params)).empty?)
       if flash.alert.nil?
-        flash.alert = errors_upload.html_safe
+        flash.alert = errors_upload.join('<br>').html_safe
       else
-        flash.alert = (flash.alert + '<br />' + errors_upload.html_safe).html_safe
+        flash.alert = (flash.alert + '<br />' + errors_upload.join('<br>').html_safe).html_safe
       end
 
     else
@@ -111,6 +102,11 @@ class Users::DescriptionController < UsersController
 
   private
 
+  def redirect_to_description_with_errors(dossier, errors)
+    flash.alert = errors.join('<br>').html_safe
+    redirect_to users_dossier_description_path(dossier_id: dossier.id)
+  end
+
   def draft_submission?
     params[:submit] && params[:submit].keys.first == 'brouillon'
   end

app/services/champs_service.rb

@@ -31,11 +31,7 @@ class ChampsService
 
     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." }
+            .map { |c| "Le champ #{c.libelle} doit être rempli." }
     end
   end
 end

app/services/pieces_justificatives_service.rb

@@ -1,40 +1,53 @@
 class PiecesJustificativesService
-  def self.upload! dossier, user, params
-    errors = ''
+  def self.upload!(dossier, user, params)
+    tpj_contents = dossier.types_de_piece_justificative
+                          .map { |tpj| [tpj, params["piece_justificative_#{tpj.id}"]] }
+                          .select { |_, content| content }
 
-    dossier.types_de_piece_justificative.each do |type_de_pieces_justificatives|
-      unless params["piece_justificative_#{type_de_pieces_justificatives.id}"].nil?
+    without_virus, with_virus = tpj_contents
+                                .partition { |_, content| ClamavService.safe_file?(content.path) }
 
-        if ClamavService.safe_file? params["piece_justificative_#{type_de_pieces_justificatives.id}"].path
-          piece_justificative = PieceJustificative.new(content: params["piece_justificative_#{type_de_pieces_justificatives.id}"],
-                                                       dossier: dossier,
-                                                       type_de_piece_justificative: type_de_pieces_justificatives,
-                                                       user: user)
+    errors = with_virus
+             .map { |_, content| content.original_filename + ': <b>Virus détecté !!</b>' }
 
-          unless piece_justificative.save
-            errors << piece_justificative.errors.messages[:content][0]+" (#{piece_justificative.libelle})"+"<br>"
-          end
-        else
-          errors << params["piece_justificative_#{type_de_pieces_justificatives.id}"].original_filename+": <b>Virus détecté !!</b>"+"<br>"
-        end
-      end
-    end
-    errors
+    errors += without_virus
+              .map { |tpj, content| save_pj(content, dossier, tpj, user) }
+              .reject(&:empty?)
+
+    errors += missing_pj_error_messages(dossier)
   end
 
   def self.upload_one! dossier, user, params
-    if ClamavService.safe_file? params[:piece_justificative][:content].path
-      piece_justificative = PieceJustificative.new(content: params[:piece_justificative][:content],
-                                                   dossier: dossier,
-                                                   type_de_piece_justificative: nil,
-                                                   user: user)
+    content = params[:piece_justificative][:content]
+    if ClamavService.safe_file? content.path
+      pj = PieceJustificative.new(content: content,
+                                  dossier: dossier,
+                                  type_de_piece_justificative: nil,
+                                  user: user)
 
-      piece_justificative.save
+      pj.save
     else
-      piece_justificative = PieceJustificative.new
-      piece_justificative.errors.add(:content, params[:piece_justificative][:content].original_filename+": <b>Virus détecté !!</b>")
+      pj = PieceJustificative.new
+      pj.errors.add(:content, content.original_filename + ': <b>Virus détecté !!</b>')
     end
 
-    piece_justificative
+    pj
+  end
+
+  def self.save_pj(content, dossier, tpj, user)
+    pj = PieceJustificative.new(content: content,
+                                dossier: dossier,
+                                type_de_piece_justificative: tpj,
+                                user: user)
+
+    pj.save ? '' : "le fichier #{pj.libelle} n'a pas pu être sauvegardé"
+  end
+
+  def self.missing_pj_error_messages(dossier)
+    mandatory_pjs = dossier.types_de_piece_justificative.select(&:mandatory)
+    present_pjs = dossier.pieces_justificatives.map(&:type_de_piece_justificative)
+    missing_pjs = mandatory_pjs - present_pjs
+
+    missing_pjs.map { |pj| "La pièce jointe #{pj.libelle} doit être fournie." }
   end
 end

app/views/admin/pieces_justificatives/_fields.html.haml

@@ -1,31 +1,36 @@
-= f.fields_for :types_de_piece_justificative, types_de_piece_justificative, remote: true do |ff|
-  .form-inline
-    .form-group
-      %h4 Libellé
-      = ff.text_field :libelle, class: 'form-control libelle', placeholder: 'Libellé'
-    .form-group
-      %h4 Description
-      = ff.text_area :description, class: 'form-control description', placeholder: 'Description'
-    .form-group
-      %h4
-        Lien du formulaire vierge
-        %small
-          (optionel)
-      = ff.url_field :lien_demarche, class: 'form-control', placeholder: 'Lien du document vierge'
+.pieces_justificatives_fields
+  = f.fields_for :types_de_piece_justificative, types_de_piece_justificative, remote: true do |ff|
+    .form-inline
+      .form-group
+        %h4 Libellé
+        = ff.text_field :libelle, class: 'form-control libelle', placeholder: 'Libellé'
+      .form-group
+        %h4 Description
+        = ff.text_area :description, class: 'form-control description', placeholder: 'Description'
+      .form-group
+        %h4
+          Lien du formulaire vierge
+          %small
+            (optionel)
+        = ff.url_field :lien_demarche, class: 'form-control', placeholder: 'Lien du document vierge'
+
+      .form-group
+        = ff.hidden_field :order_place, value: ff.index
+        = ff.hidden_field :id
+      - unless ff.object.id.nil?
+        .form-group
+          %br  
+          = ff.object.button_up(index: ff.index, url: move_up_admin_procedure_pieces_justificatives_path(@procedure, ff.index))
+          = ff.object.button_down(index: ff.index, url: move_down_admin_procedure_pieces_justificatives_path(@procedure, ff.index))
+
+      .form-group
+        %h4 Obligatoire ?
+        .center
+          = ff.check_box :mandatory
 
-    .form-group
-      = ff.hidden_field :order_place, value: ff.index
-      = ff.hidden_field :id
-    - unless ff.object.id.nil?
       .form-group
         %br  
-        = ff.object.button_up(index: ff.index, url: move_up_admin_procedure_pieces_justificatives_path(@procedure, ff.index))
-        = ff.object.button_down(index: ff.index, url: move_down_admin_procedure_pieces_justificatives_path(@procedure, ff.index))
-
-
-    .form-group
-      %br  
-      - if ff.object.id.nil?
-        = f.submit('Ajouter la pièce', class: 'btn btn-success', id: 'add_piece_justificative')
-      - else
-        = link_to("", admin_procedure_piece_justificative_path(@procedure, ff.object.id), method: :delete, remote: true, id: "delete_type_de_piece_justificative_#{ff.object.id}", class: %w(form-control btn btn-danger fa fa-trash-o) )
+        - if ff.object.id.nil?
+          = f.submit('Ajouter la pièce', class: 'btn btn-success', id: 'add_piece_justificative')
+        - else
+          = link_to("", admin_procedure_piece_justificative_path(@procedure, ff.object.id), method: :delete, remote: true, id: "delete_type_de_piece_justificative_#{ff.object.id}", class: %w(form-control btn btn-danger fa fa-trash-o) )

db/migrate/20170328142700_add_mandatory_column_to_types_de_piece_justificative.rb

@@ -0,0 +1,5 @@
+class AddMandatoryColumnToTypesDePieceJustificative < ActiveRecord::Migration[5.0]
+  def change
+    add_column :types_de_piece_justificative, :mandatory, :boolean, default: false
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170313140834) do
+ActiveRecord::Schema.define(version: 20170328142700) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -403,6 +403,7 @@ ActiveRecord::Schema.define(version: 20170313140834) do
     t.integer  "procedure_id"
     t.integer  "order_place"
     t.string   "lien_demarche"
+    t.boolean  "mandatory",      default: false
   end
 
   create_table "users", force: :cascade do |t|

spec/services/champs_service_spec.rb

@@ -34,7 +34,7 @@ describe ChampsService do
       end
 
       it 'adds error for the missing mandatory champ' do
-        expect(@errors).to match([{ message: 'Le champ mandatory doit être rempli.' }])
+        expect(@errors).to match(['Le champ mandatory doit être rempli.'])
       end
     end
 

spec/services/pieces_justificatives_service_spec.rb

@@ -0,0 +1,81 @@
+require 'spec_helper'
+
+describe PiecesJustificativesService do
+  let(:user) { create(:user) }
+  let(:safe_file) { true }
+
+  before :each do
+    allow(ClamavService).to receive(:safe_file?).and_return(safe_file)
+  end
+
+  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: 'file')
+          }
+        end
+
+        it { expect(errors).to match(["le fichier 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: <b>Virus détecté !!</b>']) }
+      end
+    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.']) }
+      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
+
+          allow(PiecesJustificativesService).to receive(:save_pj).and_return('')
+          piece_justificative_double = double(type_de_piece_justificative: tpj_mandatory)
+          expect(dossier).to receive(:pieces_justificatives).and_return([piece_justificative_double])
+        end
+
+        let(:hash) do
+          {
+            "piece_justificative_#{tpj_mandatory.id}" => double(path: '')
+          }
+        end
+
+        it { expect(errors).to match([]) }
+      end
+    end
+  end
+end