Merge pull request #2938 from betagouv/frederic/fix_835-multiple_avis

Ask multiple experts for an avis in one click

app/controllers/concerns/create_avis_concern.rb

@@ -0,0 +1,50 @@
+module CreateAvisConcern
+  extend ActiveSupport::Concern
+
+  private
+
+  def create_avis_from_params(dossier, confidentiel = false)
+    confidentiel ||= create_avis_params[:confidentiel]
+
+    # Because of a limitation of the email_field rails helper,
+    # the :emails parameter is a 1-element array.
+    # Hence the call to first
+    # https://github.com/rails/rails/issues/17225
+    emails = create_avis_params[:emails].first.split(',').map(&:strip)
+
+    create_results = Avis.create(
+      emails.map do |email|
+        {
+          email: email,
+          introduction: create_avis_params[:introduction],
+          claimant: current_gestionnaire,
+          dossier: dossier,
+          confidentiel: confidentiel
+        }
+      end
+    )
+
+    persisted, failed = create_results.partition(&:persisted?)
+
+    if persisted.any?
+      sent_emails_addresses = persisted.map(&:email_to_display).join(", ")
+      flash.notice = "Une demande d'avis a été envoyée à #{sent_emails_addresses}"
+    end
+
+    if failed.any?
+      flash.now.alert = failed
+        .select { |avis| avis.errors.present? }
+        .map { |avis| "#{avis.email} : #{avis.errors.full_messages.join(', ')}" }
+
+      # When an error occurs, return the avis back to the controller
+      # to give the user a chance to correct and resubmit
+      Avis.new(create_avis_params.merge(emails: [failed.map(&:email).join(", ")]))
+    else
+      nil
+    end
+  end
+
+  def create_avis_params
+    params.require(:avis).permit(:introduction, :confidentiel, emails: [])
+  end
+end

app/controllers/new_gestionnaire/avis_controller.rb

@@ -1,5 +1,7 @@
 module NewGestionnaire
   class AvisController < GestionnaireController
+    include CreateAvisConcern
+
     before_action :authenticate_gestionnaire!, except: [:sign_up, :create_gestionnaire]
     before_action :redirect_if_no_sign_up_needed, only: [:sign_up]
     before_action :check_avis_exists_and_email_belongs_to_avis, only: [:sign_up, :create_gestionnaire]
@@ -60,14 +62,11 @@ module NewGestionnaire
     end
 
     def create_avis
-      confidentiel = avis.confidentiel || params[:avis][:confidentiel]
-      @new_avis = Avis.new(create_avis_params.merge(claimant: current_gestionnaire, dossier: avis.dossier, confidentiel: confidentiel))
+      @new_avis = create_avis_from_params(avis.dossier, avis.confidentiel)
 
-      if @new_avis.save
-        flash.notice = "Une demande d'avis a été envoyée à #{@new_avis.email_to_display}"
+      if @new_avis.nil?
         redirect_to instruction_gestionnaire_avis_path(avis)
       else
-        flash.now.alert = @new_avis.errors.full_messages
         set_avis_and_dossier
         render :instruction
       end
@@ -141,9 +140,5 @@ module NewGestionnaire
     def commentaire_params
       params.require(:commentaire).permit(:body, :file)
     end
-
-    def create_avis_params
-      params.require(:avis).permit(:email, :introduction)
-    end
   end
 end

app/controllers/new_gestionnaire/dossiers_controller.rb

@@ -2,6 +2,7 @@ module NewGestionnaire
   class DossiersController < ProceduresController
     include ActionView::Helpers::NumberHelper
     include ActionView::Helpers::TextHelper
+    include CreateAvisConcern
 
     after_action :mark_demande_as_read, only: :show
     after_action :mark_messagerie_as_read, only: [:messagerie, :create_commentaire]
@@ -130,12 +131,11 @@ module NewGestionnaire
     end
 
     def create_avis
-      @avis = Avis.new(avis_params.merge(claimant: current_gestionnaire, dossier: dossier))
-      if @avis.save
-        flash.notice = "Une demande d'avis a été envoyée à #{@avis.email_to_display}"
+      @avis = create_avis_from_params(dossier)
+
+      if @avis.nil?
         redirect_to avis_gestionnaire_dossier_path(procedure, dossier)
       else
-        flash.now.alert = @avis.errors.full_messages
         @avis_seen_at = current_gestionnaire.follows.find_by(dossier: dossier)&.avis_seen_at
         render :avis
       end
@@ -163,10 +163,6 @@ module NewGestionnaire
       params.require(:commentaire).permit(:body, :file)
     end
 
-    def avis_params
-      params.require(:avis).permit(:email, :introduction, :confidentiel)
-    end
-
     def champs_private_params
       params.require(:dossier).permit(champs_private_attributes: [
         :id, :primary_value, :secondary_value, :piece_justificative_file, :value, value: [],

app/models/avis.rb

@@ -6,6 +6,7 @@ class Avis < ApplicationRecord
   belongs_to :claimant, class_name: 'Gestionnaire'
 
   validates :email, format: { with: Devise.email_regexp, message: "n'est pas valide" }, allow_nil: true
+  validates :claimant, presence: true
 
   before_validation -> { sanitize_email(:email) }
   before_create :try_to_assign_gestionnaire
@@ -18,6 +19,10 @@ class Avis < ApplicationRecord
   scope :by_latest, -> { order(updated_at: :desc) }
   scope :updated_since?, -> (date) { where('avis.updated_at > ?', date) }
 
+  # The form allows subtmitting avis requests to several emails at once,
+  # hence this virtual attribute.
+  attr_accessor :emails
+
   def email_to_display
     gestionnaire&.email || email
   end

app/views/new_gestionnaire/shared/avis/_form.html.haml

@@ -1,9 +1,9 @@
 %section.ask-avis
-  %h1.tab-title Inviter une personne à donner son avis
-  %p.avis-notice L'invité pourra consulter, donner un avis sur le dossier et contribuer au fil de messagerie, mais il ne pourra le modifier.
+  %h1.tab-title Inviter des personnes à donner leur avis
+  %p.avis-notice Les invités pourront consulter, donner un avis sur le dossier et contribuer au fil de messagerie, mais ils ne pourront pas le modifier.
 
   = form_for avis, url: url, html: { class: 'form' } do |f|
-    = f.email_field :email, placeholder: 'Adresse email', required: true
+    = f.email_field :emails, placeholder: 'Adresses email, séparées par des virgules', required: true, multiple: true
     = f.text_area :introduction, rows: 3, value: avis.introduction || 'Bonjour, merci de me donner votre avis sur ce dossier.', required: true
     .flex.justify-between.align-baseline
       - if must_be_confidentiel

config/application.rb

@@ -24,7 +24,9 @@ module TPS
     config.i18n.load_path += Dir[Rails.root.join('config', 'locales', '**', '*.{rb,yml}')]
     config.i18n.available_locales = [:fr]
 
-    config.autoload_paths += ["#{config.root}/lib", "#{config.root}/app/validators", "#{config.root}/app/facades"]
+    config.paths.add "#{config.root}/lib", eager_load: true
+    config.paths.add "#{config.root}/app/controllers/concerns", eager_load: true
+
     config.assets.paths << Rails.root.join('app', 'assets', 'javascript')
     config.assets.paths << Rails.root.join('app', 'assets', 'fonts')
     config.assets.precompile += ['.woff']

spec/controllers/new_gestionnaire/avis_controller_spec.rb

@@ -100,24 +100,37 @@ describe NewGestionnaire::AvisController, type: :controller do
 
     describe '#create_avis' do
       let!(:previous_avis) { Avis.create(dossier: dossier, claimant: claimant, gestionnaire: gestionnaire, confidentiel: previous_avis_confidentiel) }
-      let(:email) { 'a@b.com' }
+      let(:emails) { ['a@b.com'] }
       let(:intro) { 'introduction' }
       let(:created_avis) { Avis.last }
+      let!(:old_avis_count) { Avis.count }
 
       before do
-        post :create_avis, params: { id: previous_avis.id, avis: { email: email, introduction: intro, confidentiel: asked_confidentiel } }
+        post :create_avis, params: { id: previous_avis.id, avis: { emails: emails, introduction: intro, confidentiel: asked_confidentiel } }
       end
 
       context 'when an invalid email' do
         let(:previous_avis_confidentiel) { false }
         let(:asked_confidentiel) { false }
-        let(:email) { "toto.fr" }
+        let(:emails) { ["toto.fr"] }
 
         it { expect(response).to render_template :instruction }
-        it { expect(flash.alert).to eq(["Email n'est pas valide"]) }
+        it { expect(flash.alert).to eq(["toto.fr : Email n'est pas valide"]) }
         it { expect(Avis.last).to eq(previous_avis) }
       end
 
+      context 'with multiple emails' do
+        let(:asked_confidentiel) { false }
+        let(:previous_avis_confidentiel) { false }
+        let(:emails) { ["toto.fr,titi@titimail.com"] }
+
+        it { expect(response).to render_template :instruction }
+        it { expect(flash.alert).to eq(["toto.fr : Email n'est pas valide"]) }
+        it { expect(flash.notice).to eq("Une demande d'avis a été envoyée à titi@titimail.com") }
+        it { expect(Avis.count).to eq(old_avis_count + 1) }
+        it { expect(created_avis.email).to eq("titi@titimail.com") }
+      end
+
       context 'when the previous avis is public' do
         let(:previous_avis_confidentiel) { false }
 
@@ -125,7 +138,7 @@ describe NewGestionnaire::AvisController, type: :controller do
           let(:asked_confidentiel) { false }
 
           it { expect(created_avis.confidentiel).to be(false) }
-          it { expect(created_avis.email).to eq(email) }
+          it { expect(created_avis.email).to eq(emails.last) }
           it { expect(created_avis.introduction).to eq(intro) }
           it { expect(created_avis.dossier).to eq(previous_avis.dossier) }
           it { expect(created_avis.claimant).to eq(gestionnaire) }

spec/controllers/new_gestionnaire/dossiers_controller_spec.rb

@@ -308,12 +308,13 @@ describe NewGestionnaire::DossiersController, type: :controller do
 
   describe "#create_avis" do
     let(:saved_avis) { dossier.avis.first }
+    let!(:old_avis_count) { Avis.count }
 
     subject do
       post :create_avis, params: {
         procedure_id: procedure.id,
         dossier_id: dossier.id,
-        avis: { email: email, introduction: 'intro', confidentiel: true }
+        avis: { emails: emails, introduction: 'intro', confidentiel: true }
       }
     end
 
@@ -321,7 +322,7 @@ describe NewGestionnaire::DossiersController, type: :controller do
       subject
     end
 
-    let(:email) { 'email@a.com' }
+    let(:emails) { ['email@a.com'] }
 
     it { expect(saved_avis.email).to eq('email@a.com') }
     it { expect(saved_avis.introduction).to eq('intro') }
@@ -331,12 +332,22 @@ describe NewGestionnaire::DossiersController, type: :controller do
     it { expect(response).to redirect_to(avis_gestionnaire_dossier_path(dossier.procedure, dossier)) }
 
     context "with an invalid email" do
-      let(:email) { 'emaila.com' }
+      let(:emails) { ['emaila.com'] }
 
       it { expect(response).to render_template :avis }
-      it { expect(flash.alert).to eq(["Email n'est pas valide"]) }
+      it { expect(flash.alert).to eq(["emaila.com : Email n'est pas valide"]) }
       it { expect { subject }.not_to change(Avis, :count) }
     end
+
+    context 'with multiple emails' do
+      let(:emails) { ["toto.fr,titi@titimail.com"] }
+
+      it { expect(response).to render_template :avis }
+      it { expect(flash.alert).to eq(["toto.fr : Email n'est pas valide"]) }
+      it { expect(flash.notice).to eq("Une demande d'avis a été envoyée à titi@titimail.com") }
+      it { expect(Avis.count).to eq(old_avis_count + 1) }
+      it { expect(saved_avis.email).to eq("titi@titimail.com") }
+    end
   end
 
   describe "#update_annotations" do

spec/features/new_gestionnaire/gestionnaire_spec.rb

@@ -192,7 +192,7 @@ feature 'The gestionnaire part' do
   end
 
   def ask_confidential_avis(to, introduction)
-    fill_in 'avis_email', with: to
+    fill_in 'avis_emails', with: to
     fill_in 'avis_introduction', with: introduction
     select 'confidentiel', from: 'avis_confidentiel'
     click_on 'Demander un avis'