PiecesJustificativesService: mandatory pj check and add test

app/services/pieces_justificatives_service.rb

@@ -1,25 +1,21 @@
 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><br>' }
 
-          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) }
+
+    errors += missing_pj_error_messages(dossier)
+
+    errors.join
   end
 
   def self.upload_one! dossier, user, params
@@ -37,4 +33,21 @@ class PiecesJustificativesService
 
     piece_justificative
   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é<br>"
+  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.<br>" }
+  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 eq("le fichier not mandatory n'a pas pu être sauvegardé<br>") }
+      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 eq('bad_file: <b>Virus détecté !!</b><br>') }
+      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 eq('La pièce jointe justificatif doit être fournie.<br>') }
+      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 eq('') }
+      end
+    end
+  end
+end