Merge pull request #8733 from E-L-T/improve-instructeurs-import

feat(groupe instructeur): allow import of many groups when procedure is not routed yet

app/controllers/administrateurs/groupe_instructeurs_controller.rb

@@ -204,43 +204,34 @@ module Administrateurs
           file = csv_file.read
           base_encoding = CharlockHolmes::EncodingDetector.detect(file)
 
-          if params[:group_csv_file]
-            groupes_emails = ACSV::CSV.new_for_ruby3(file.encode("UTF-8", base_encoding[:encoding], invalid: :replace, replace: ""), headers: true, header_converters: :downcase)
-              .map { |r| r.to_h.slice('groupe', 'email') }
+          csv_content = ACSV::CSV.new_for_ruby3(file.encode("UTF-8", base_encoding[:encoding], invalid: :replace, replace: ""), headers: true, header_converters: :downcase).map(&:to_h)
 
-            groupes_emails_has_keys = groupes_emails.first.has_key?("groupe") && groupes_emails.first.has_key?("email")
+          if csv_content.first.has_key?("groupe") && csv_content.first.has_key?("email")
+            groupes_emails = csv_content.map { |r| r.to_h.slice('groupe', 'email') }
 
-            if groupes_emails_has_keys.blank?
-              flash[:alert] = "Importation impossible, veuillez importer un csv #{view_context.link_to('suivant ce modèle', "/csv/#{I18n.locale}/import-groupe-test.csv")}"
-            else
-              added_instructeurs_by_group, invalid_emails = InstructeursImportService.import_groupes(procedure, groupes_emails)
+            added_instructeurs_by_group, invalid_emails = InstructeursImportService.import_groupes(procedure, groupes_emails)
 
-              added_instructeurs_by_group.each do |groupe, added_instructeurs|
+            added_instructeurs_by_group.each do |groupe, added_instructeurs|
+              if added_instructeurs.present?
                 GroupeInstructeurMailer
                   .notify_added_instructeurs(groupe, added_instructeurs, current_administrateur.email)
                   .deliver_later
               end
-
               flash_message_for_import(invalid_emails)
             end
 
-          elsif params[:instructeurs_csv_file]
-            instructors_emails = ACSV::CSV.new_for_ruby3(file.encode("UTF-8", base_encoding[:encoding], invalid: :replace, replace: ""), headers: true, header_converters: :downcase)
-              .map(&:to_h)
-
-            instructors_emails_has_key = instructors_emails.first.has_key?("email") && !instructors_emails.first.keys.many?
-
-            if instructors_emails_has_key.blank?
-              flash[:alert] = "Importation impossible, veuillez importer un csv #{view_context.link_to('suivant ce modèle', "/csv/import-instructeurs-test.csv")}"
-            else
-              added_instructeurs, invalid_emails = InstructeursImportService.import_instructeurs(procedure, instructors_emails)
+          elsif csv_content.first.has_key?("email") && !csv_content.map(&:to_h).first.keys.many? && procedure.groupe_instructeurs.one?
+            instructors_emails = csv_content.map(&:to_h)
 
+            added_instructeurs, invalid_emails = InstructeursImportService.import_instructeurs(procedure, instructors_emails)
+            if added_instructeurs.present?
               GroupeInstructeurMailer
                 .notify_added_instructeurs(groupe_instructeur, added_instructeurs, current_administrateur.email)
                 .deliver_later
-
-              flash_message_for_import(invalid_emails)
             end
+            flash_message_for_import(invalid_emails)
+          else
+            flash_message_for_invalid_csv
           end
           redirect_to admin_procedure_groupe_instructeurs_path(procedure)
         end
@@ -316,7 +307,7 @@ module Administrateurs
     end
 
     def csv_file
-      params[:group_csv_file] || params[:instructeurs_csv_file]
+      params[:csv_file]
     end
 
     def marcel_content_type
@@ -338,5 +329,9 @@ module Administrateurs
         flash[:alert] = "Import terminé. Cependant les emails suivants ne sont pas pris en compte: #{result.join(', ')}"
       end
     end
+
+    def flash_message_for_invalid_csv
+      flash[:alert] = "Importation impossible, veuillez importer un csv suivant #{view_context.link_to('ce modèle', "/csv/import-instructeurs-test.csv")} pour une procédure sans routage ou #{view_context.link_to('celui-ci', "/csv/#{I18n.locale}/import-groupe-test.csv")} pour une procédure routée"
+    end
   end
 end

app/services/instructeurs_import_service.rb

@@ -15,7 +15,7 @@ class InstructeursImportService
 
     if missing_labels.present?
       created_at = Time.zone.now
-      GroupeInstructeur.insert_all(missing_labels.map { |label| { procedure_id: procedure.id, label:, created_at:, updated_at: created_at } })
+      GroupeInstructeur.create!(missing_labels.map { |label| { procedure_id: procedure.id, label:, created_at:, updated_at: created_at } })
     end
 
     emails_in_groupe = groupes_emails

app/views/administrateurs/groupe_instructeurs/_edit.html.haml

@@ -28,13 +28,10 @@
     = form_tag import_admin_procedure_groupe_instructeurs_path(procedure), method: :post, multipart: true, class: "mt-4 form" do
       = label_tag t('.csv_import.title')
       %p.notice
-        = procedure.routing_enabled? ? t('.csv_import.routing_enabled.notice_1') : t('.csv_import.routing_disabled.notice_1')
+        = t('.csv_import.notice_1_html')
       %p.notice
         = t('.csv_import.notice_2', csv_max_size: number_to_human_size(csv_max_size))
-      - sample_file_path = procedure.routing_enabled? ? "/csv/#{I18n.locale}/import-groupe-test.csv" : "/csv/import-instructeurs-test.csv"
-      %p.mt-2.mb-2= link_to t('.csv_import.download_exemple'), sample_file_path
-      - csv_params = procedure.routing_enabled? ? :group_csv_file : :instructeurs_csv_file
-      = file_field_tag csv_params, required: true, accept: 'text/csv', size: "1"
+      = file_field_tag :csv_file, required: true, accept: 'text/csv', size: "1"
       = submit_tag t('.csv_import.import_file'), class: 'button primary send', data: { disable_with: "Envoi...", confirm: t('.csv_import.import_file_alert') }
   - else
     %p.mt-4.form.font-weight-bold.mb-2.text-lg

config/locales/views/administrateurs/groupe_instructeurs/en.yml

@@ -41,12 +41,8 @@ en:
           notice: This group will be a choice from the list "%{routing_criteria_name}"
         csv_import:
           title: CSV Import
-          routing_enabled:
-            notice_1: The csv file must have 2 columns (Group, Email) and be separated by commas. The import does not overwrite existing groups and instructors.
-          routing_disabled:
-            notice_1: The csv file must have 1 column with instructors emails.
+          notice_1_html: The csv file must have 1 column with instructors emails. For routed procedures, the csv file must have 2 columns (Group, Email) and be separated by commas. The import does not overwrite existing groups and instructors.
           notice_2: The size of the file must be less than %{csv_max_size}.
-          download_exemple: Download sample CSV file
           import_file: Import file
           import_file_procedure_not_published: The import of instructors by CSV file is available once the process has been published
           import_file_alert: Instructors added to the procedure will receive an email. Are you sure you want to continue ?"

config/locales/views/administrateurs/groupe_instructeurs/fr.yml

@@ -47,12 +47,10 @@ fr:
           notice: Ce groupe sera un choix de la liste "%{routing_criteria_name}"
         csv_import:
           title: Importer par CSV
-          routing_enabled:
-            notice_1: Le fichier csv doit comporter 2 colonnes (Groupe, Email) et être séparé par des virgules.
-          routing_disabled:
-            notice_1: Le fichier csv doit comporter 1 seule colonne (Email) avec une adresse email d'instructeur par ligne.
-          notice_2: L’import n’écrase pas les groupes et les instructeurs existants. La modification du fichier csv ne s’opère que pour l’ajout de nouveaux instructeurs. La suppression d’un instructeur s’opère manuellement en cliquant sur le bouton « retirer ». Le poids du fichier doit être inférieur à %{csv_max_size}.
-          download_exemple: Télécharger l’exemple de fichier CSV
+          notice_1_html: |
+            Pour une procédure standard, le fichier csv doit comporter 1 seule colonne (Email) avec une adresse email d'instructeur par ligne (<a href=\"/csv/import-instructeurs-test.csv\">exemple de fichier</a>).
+            Pour une procédure routée, le fichier doit comporter 2 colonnes (Groupe, Email) et être séparé par des virgules (<a href=\"/csv/fr/import-groupe-test.csv\">exemple de fichier</a>).
+          notice_2: L’import n’écrase pas les instructeurs existants. Il permet uniquement d'en ajouter. Pour supprimer un instructeur, cliquez sur le bouton « retirer ». Le poids du fichier doit être inférieur %{csv_max_size}.
           import_file: Importer le fichier
           import_file_procedure_not_published: L’import d’instructeurs par fichier CSV est disponible une fois la démarche publiée
           import_file_alert: Tous les instructeurs ajoutés à la procédure vont être notifiés par email. Voulez-vous continuer ?

spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb

@@ -2,8 +2,9 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
   render_views
 
   let(:admin) { create(:administrateur) }
-  let(:procedure) { create(:procedure, :published, :for_individual, administrateurs: [admin], routing_enabled: true) }
+  let(:procedure) { create(:procedure, :published, :for_individual, administrateurs: [admin]) }
   let!(:gi_1_1) { procedure.defaut_groupe_instructeur }
+  let!(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 2') }
 
   let(:procedure2) { create(:procedure, :published) }
   let!(:gi_2_2) { procedure2.groupe_instructeurs.create(label: 'groupe instructeur 2 2') }
@@ -12,8 +13,6 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
 
   describe '#index' do
     context 'of a procedure I own' do
-      let!(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 2') }
-
       before { get :index, params: { procedure_id: procedure.id } }
 
       context 'when a procedure has multiple groups' do
@@ -63,14 +62,14 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
 
       it { expect(flash.notice).to be_present }
       it { expect(response).to redirect_to(admin_procedure_groupe_instructeur_path(procedure, procedure.groupe_instructeurs.last)) }
-      it { expect(procedure.groupe_instructeurs.count).to eq(2) }
+      it { expect(procedure.groupe_instructeurs.count).to eq(3) }
     end
 
     context 'with an invalid group name' do
       let(:label) { gi_1_1.label }
 
       it { expect(response).to render_template(:index) }
-      it { expect(procedure.groupe_instructeurs.count).to eq(1) }
+      it { expect(procedure.groupe_instructeurs.count).to eq(2) }
       it { expect(flash.alert).to be_present }
     end
   end
@@ -85,7 +84,10 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
     end
 
     context 'with only one group' do
-      before { delete_group gi_1_1 }
+      before do
+        delete_group gi_1_1
+        delete_group gi_1_2
+      end
 
       it { expect(flash.alert).to be_present }
       it { expect(response).to redirect_to(admin_procedure_groupe_instructeurs_path(procedure)) }
@@ -93,8 +95,6 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
     end
 
     context 'with many groups' do
-      let!(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 2') }
-
       context 'of a group that can be deleted' do
         before { delete_group gi_1_2 }
         it { expect(flash.notice).to eq "le groupe « groupe instructeur 2 » a été supprimé et le routage a été désactivé." }
@@ -115,7 +115,6 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
   end
 
   describe '#reaffecter_dossiers' do
-    let!(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 2') }
     let!(:gi_1_3) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 3') }
 
     before do
@@ -137,7 +136,6 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
   end
 
   describe '#reaffecter' do
-    let!(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 2') }
     let!(:gi_1_3) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 3') }
     let!(:dossier12) { create(:dossier, :en_construction, :with_individual, procedure: procedure, groupe_instructeur: gi_1_1) }
     let!(:instructeur) { create(:instructeur) }
@@ -184,18 +182,20 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
   describe '#update' do
     let(:new_name) { 'nouveau nom du groupe' }
     let(:closed_value) { false }
+    let!(:procedure_non_routee) { create(:procedure, :published, :for_individual, administrateurs: [admin]) }
+    let!(:gi_1_1) { procedure_non_routee.defaut_groupe_instructeur }
 
     before do
       patch :update,
         params: {
-          procedure_id: procedure.id,
+          procedure_id: procedure_non_routee.id,
           id: gi_1_1.id,
           groupe_instructeur: { label: new_name, closed: closed_value }
         }
       gi_1_1.reload
     end
 
-    it { expect(response).to redirect_to(admin_procedure_groupe_instructeur_path(procedure, gi_1_1)) }
+    it { expect(response).to redirect_to(admin_procedure_groupe_instructeur_path(procedure_non_routee, gi_1_1)) }
     it { expect(gi_1_1.label).to eq(new_name) }
     it { expect(gi_1_1.closed).to eq(false) }
     it { expect(flash.notice).to be_present }
@@ -210,7 +210,7 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
     end
 
     context 'when the name is already taken' do
-      let!(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 2') }
+      let!(:gi_1_2) { procedure_non_routee.groupe_instructeurs.create(label: 'groupe instructeur 2') }
       let(:new_name) { gi_1_2.label }
 
       it { expect(gi_1_1.label).not_to eq(new_name) }
@@ -219,17 +219,18 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
   end
 
   describe '#add_instructeur_procedure_non_routee' do
-    let(:procedure) { create :procedure }
-    let!(:groupe_instructeur) { create(:administrateurs_procedure, procedure: procedure, administrateur: admin, manager: manager) }
+    # faire la meme chose sur une procedure non routee
+    let(:procedure_non_routee) { create :procedure }
+    let!(:groupe_instructeur) { create(:administrateurs_procedure, procedure: procedure_non_routee, administrateur: admin, manager: manager) }
     let(:emails) { ['instructeur_3@ministere_a.gouv.fr', 'instructeur_4@ministere_b.gouv.fr'].to_json }
-    subject { post :add_instructeur, params: { emails: emails, procedure_id: procedure.id, id: gi_1_1.id } }
+    subject { post :add_instructeur, params: { emails: emails, procedure_id: procedure_non_routee.id, id: procedure_non_routee.defaut_groupe_instructeur.id } }
     let(:manager) { false }
     context 'when all emails are valid' do
       let(:emails) { ['test@b.gouv.fr', 'test2@b.gouv.fr'].to_json }
       it { expect(response.status).to eq(200) }
       it { expect(subject.request.flash[:alert]).to be_nil }
       it { expect(subject.request.flash[:notice]).to be_present }
-      it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure) }
+      it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure_non_routee) }
     end
 
     context 'when there is at least one bad email' do
@@ -237,13 +238,13 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
       it { expect(response.status).to eq(200) }
       it { expect(subject.request.flash[:alert]).to be_present }
       it { expect(subject.request.flash[:notice]).to be_present }
-      it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure) }
+      it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure_non_routee) }
     end
 
     context 'when the admin wants to assign an instructor who is already assigned on this procedure' do
       let(:emails) { ['instructeur_1@ministere_a.gouv.fr'].to_json }
       it { expect(subject.request.flash[:alert]).to be_present }
-      it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure) }
+      it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure_non_routee) }
     end
 
     context 'when signed in admin comes from manager' do
@@ -254,7 +255,6 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
 
   describe '#add_instructeur' do
     let!(:instructeur) { create(:instructeur) }
-    let(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 2') }
     let(:do_request) do
       post :add_instructeur,
         params: {
@@ -363,197 +363,225 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
   end
 
   describe '#remove_instructeur_procedure_non_routee' do
-    let(:procedure) { create :procedure, administrateur: admin, instructeurs: [instructeur_assigned_1, instructeur_assigned_2] }
+    let(:procedure_non_routee) { create :procedure, administrateur: admin, instructeurs: [instructeur_assigned_1, instructeur_assigned_2] }
     let!(:instructeur_assigned_1) { create :instructeur, email: 'instructeur_1@ministere-a.gouv.fr', administrateurs: [admin] }
     let!(:instructeur_assigned_2) { create :instructeur, email: 'instructeur_2@ministere-b.gouv.fr', administrateurs: [admin] }
     let!(:instructeur_assigned_3) { create :instructeur, email: 'instructeur_3@ministere-a.gouv.fr', administrateurs: [admin] }
-    subject! { get :show, params: { procedure_id: procedure.id, id: gi_1_1.id } }
+    subject! { get :show, params: { procedure_id: procedure_non_routee.id, id: procedure_non_routee.defaut_groupe_instructeur.id } }
     it 'sets the assigned instructeurs' do
       expect(assigns(:instructeurs)).to match_array([instructeur_assigned_1, instructeur_assigned_2])
     end
 
     context 'when the instructor is assigned to the procedure' do
-      subject { delete :remove_instructeur, params: { instructeur: { id: instructeur_assigned_1.id }, procedure_id: procedure.id, id: gi_1_1.id } }
+      subject { delete :remove_instructeur, params: { instructeur: { id: instructeur_assigned_1.id }, procedure_id: procedure_non_routee.id, id: procedure_non_routee.defaut_groupe_instructeur.id } }
       it { expect(subject.request.flash[:notice]).to be_present }
       it { expect(subject.request.flash[:alert]).to be_nil }
       it { expect(response.status).to eq(302) }
-      it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure) }
+      it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure_non_routee) }
     end
 
     context 'when the instructor is not assigned to the procedure' do
-      subject { delete :remove_instructeur, params: { instructeur: { id: instructeur_assigned_3.id }, procedure_id: procedure.id, id: gi_1_1.id } }
+      subject { delete :remove_instructeur, params: { instructeur: { id: instructeur_assigned_3.id }, procedure_id: procedure_non_routee.id, id: procedure_non_routee.defaut_groupe_instructeur.id } }
       it { expect(subject.request.flash[:alert]).to be_present }
       it { expect(subject.request.flash[:notice]).to be_nil }
       it { expect(response.status).to eq(302) }
-      it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure) }
+      it { expect(subject).to redirect_to admin_procedure_groupe_instructeurs_path(procedure_non_routee) }
     end
   end
 
-  describe '#add_groupe_instructeurs_via_csv_file' do
+  describe '#import' do
     subject do
-      post :import, params: { procedure_id: procedure.id, group_csv_file: csv_file }
+      post :import, params: { procedure_id: procedure.id, csv_file: csv_file }
     end
 
-    context 'when the csv file is less than 1 mo and content type text/csv' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur.csv', 'text/csv') }
+    context 'routed procedures' do
+      context 'when the csv file is less than 1 mo and content type text/csv' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur.csv', 'text/csv') }
 
-      before { subject }
+        before { subject }
 
-      it { expect(response.status).to eq(302) }
-      it { expect(procedure.groupe_instructeurs.first.label).to eq("Afrique") }
-      it { expect(flash.alert).to be_present }
-      it { expect(flash.alert).to eq("Import terminé. Cependant les emails suivants ne sont pas pris en compte: kara") }
-    end
-
-    context 'when the csv file has only one column' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/valid-instructeurs-file.csv', 'text/csv') }
-
-      before { subject }
-
-      it { expect { subject }.not_to raise_error }
-      it { expect(response.status).to eq(302) }
-      it { expect(flash.alert).to be_present }
-      it { expect(flash.alert).to eq("Importation impossible, veuillez importer un csv <a href=\"/csv/#{I18n.locale}/import-groupe-test.csv\">suivant ce modèle</a>") }
-    end
-
-    context 'when the file content type is application/vnd.ms-excel' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe_avec_caracteres_speciaux.csv', "application/vnd.ms-excel") }
-
-      before { subject }
-
-      it { expect(flash.notice).to be_present }
-      it { expect(flash.notice).to eq("La liste des instructeurs a été importée avec succès") }
-    end
-
-    context 'when the content of csv contains special characters' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe_avec_caracteres_speciaux.csv', 'text/csv') }
-
-      before do
-        allow(GroupeInstructeurMailer).to receive(:notify_added_instructeurs)
-          .and_return(double(deliver_later: true))
-        subject
+        it { expect(response.status).to eq(302) }
+        it { expect(procedure.groupe_instructeurs.first.label).to eq("Afrique") }
+        it { expect(flash.alert).to be_present }
+        it { expect(flash.alert).to eq("Import terminé. Cependant les emails suivants ne sont pas pris en compte: kara") }
       end
 
-      it { expect(procedure.groupe_instructeurs.pluck(:label)).to match_array(["Auvergne-Rhône-Alpes", "Vendée", "défaut"]) }
-      it { expect(flash.notice).to be_present }
-      it { expect(flash.notice).to eq("La liste des instructeurs a été importée avec succès") }
-      it { expect(GroupeInstructeurMailer).to have_received(:notify_added_instructeurs).twice }
-    end
+      context 'when the csv file has only one column' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/valid-instructeurs-file.csv', 'text/csv') }
 
-    context 'when the csv file length is more than 1 mo' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur.csv', 'text/csv') }
+        before { subject }
 
-      before do
-        allow_any_instance_of(ActionDispatch::Http::UploadedFile).to receive(:size).and_return(3.megabytes)
-        subject
+        it { expect { subject }.not_to raise_error }
+        it { expect(response.status).to eq(302) }
+        it { expect(flash.alert).to be_present }
+        it { expect(flash.alert).to eq("Importation impossible, veuillez importer un csv suivant <a href=\"/csv/import-instructeurs-test.csv\">ce modèle</a> pour une procédure sans routage ou <a href=\"/csv/fr/import-groupe-test.csv\">celui-ci</a> pour une procédure routée") }
       end
 
-      it { expect(flash.alert).to be_present }
-      it { expect(flash.alert).to eq("Importation impossible : le poids du fichier est supérieur à 1 Mo") }
-    end
+      context 'when the file content type is application/vnd.ms-excel' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe_avec_caracteres_speciaux.csv', "application/vnd.ms-excel") }
 
-    context 'when the file content type is not accepted' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/french-flag.gif', 'image/gif') }
+        before { subject }
 
-      before { subject }
-
-      it { expect(flash.alert).to be_present }
-      it { expect(flash.alert).to eq("Importation impossible : veuillez importer un fichier CSV") }
-    end
-
-    context 'when the headers are wrong' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/invalid-group-file.csv', 'text/csv') }
-
-      before { subject }
-
-      it { expect(flash.alert).to be_present }
-      it { expect(flash.alert).to eq("Importation impossible, veuillez importer un csv <a href=\"/csv/#{I18n.locale}/import-groupe-test.csv\">suivant ce modèle</a>") }
-    end
-
-    context 'when procedure is closed' do
-      let(:procedure) { create(:procedure, :closed, administrateurs: [admin]) }
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur.csv', 'text/csv') }
-
-      before { subject }
-
-      it { expect(procedure.groupe_instructeurs.first.label).to eq("Afrique") }
-      it { expect(flash.alert).to eq("Import terminé. Cependant les emails suivants ne sont pas pris en compte: kara") }
-    end
-  end
-
-  describe '#add_instructeurs_via_csv_file' do
-    let(:procedure_non_routee) { create(:procedure, :published, :for_individual, administrateurs: [admin]) }
-
-    subject do
-      post :import, params: { procedure_id: procedure_non_routee.id, instructeurs_csv_file: csv_file }
-    end
-
-    context 'when the csv file is less than 1 mo and content type text/csv' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/instructeurs-file.csv', 'text/csv') }
-
-      before do
-        allow(GroupeInstructeurMailer).to receive(:notify_added_instructeurs)
-          .and_return(double(deliver_later: true))
-        subject
+        it { expect(flash.notice).to be_present }
+        it { expect(flash.notice).to eq("La liste des instructeurs a été importée avec succès") }
       end
 
-      it { expect(response.status).to eq(302) }
-      it { expect(procedure_non_routee.instructeurs.pluck(:email)).to match_array(["kara@beta-gouv.fr", "philippe@mail.com", "lisa@gouv.fr"]) }
-      it { expect(flash.alert).to be_present }
-      it { expect(flash.alert).to eq("Import terminé. Cependant les emails suivants ne sont pas pris en compte: eric") }
-      it "calls GroupeInstructeurMailer" do
-        expect(GroupeInstructeurMailer).to have_received(:notify_added_instructeurs).with(
-          procedure_non_routee.defaut_groupe_instructeur,
-          any_args,
-          admin.email
-        )
+      context 'when the content of csv contains special characters' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe_avec_caracteres_speciaux.csv', 'text/csv') }
+
+        before do
+          allow(GroupeInstructeurMailer).to receive(:notify_added_instructeurs)
+            .and_return(double(deliver_later: true))
+          subject
+        end
+
+        it { expect(procedure.groupe_instructeurs.pluck(:label)).to match_array(["Auvergne-Rhône-Alpes", "Vendée", "défaut", "groupe instructeur 2"]) }
+        it { expect(flash.notice).to be_present }
+        it { expect(flash.notice).to eq("La liste des instructeurs a été importée avec succès") }
+        it { expect(GroupeInstructeurMailer).to have_received(:notify_added_instructeurs).twice }
+      end
+
+      context 'when the csv file length is more than 1 mo' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur.csv', 'text/csv') }
+
+        before do
+          allow_any_instance_of(ActionDispatch::Http::UploadedFile).to receive(:size).and_return(3.megabytes)
+          subject
+        end
+
+        it { expect(flash.alert).to be_present }
+        it { expect(flash.alert).to eq("Importation impossible : le poids du fichier est supérieur à 1 Mo") }
+      end
+
+      context 'when the file content type is not accepted' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/french-flag.gif', 'image/gif') }
+
+        before { subject }
+
+        it { expect(flash.alert).to be_present }
+        it { expect(flash.alert).to eq("Importation impossible : veuillez importer un fichier CSV") }
+      end
+
+      context 'when the headers are wrong' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/invalid-group-file.csv', 'text/csv') }
+
+        before { subject }
+
+        it { expect(flash.alert).to be_present }
+        it { expect(flash.alert).to eq("Importation impossible, veuillez importer un csv suivant <a href=\"/csv/import-instructeurs-test.csv\">ce modèle</a> pour une procédure sans routage ou <a href=\"/csv/fr/import-groupe-test.csv\">celui-ci</a> pour une procédure routée") }
+      end
+
+      context 'when procedure is closed' do
+        let(:procedure) { create(:procedure, :closed, administrateurs: [admin]) }
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur.csv', 'text/csv') }
+
+        before { subject }
+
+        it { expect(procedure.groupe_instructeurs.first.label).to eq("Afrique") }
+        it { expect(flash.alert).to eq("Import terminé. Cependant les emails suivants ne sont pas pris en compte: kara") }
+      end
+
+      context 'when emails are invalid' do
+        let(:procedure) { create(:procedure, :closed, administrateurs: [admin]) }
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur-emails-invalides.csv', 'text/csv') }
+
+        before do
+          allow(GroupeInstructeurMailer).to receive(:notify_added_instructeurs)
+            .and_return(double(deliver_later: true))
+          subject
+        end
+
+        it { expect(flash.alert).to include("Import terminé. Cependant les emails suivants ne sont pas pris en compte:") }
+        it { expect(GroupeInstructeurMailer).not_to have_received(:notify_added_instructeurs) }
       end
     end
 
-    context 'when the csv file has more than one column' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur.csv', 'text/csv') }
+    context 'unrouted procedures' do
+      let(:procedure_non_routee) { create(:procedure, :published, :for_individual, administrateurs: [admin]) }
 
-      before { subject }
-
-      it { expect(response.status).to eq(302) }
-      it { expect(flash.alert).to be_present }
-      it { expect(flash.alert).to eq("Importation impossible, veuillez importer un csv <a href=\"/csv/import-instructeurs-test.csv\">suivant ce modèle</a>") }
-    end
-
-    context 'when the file content type is application/vnd.ms-excel' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/valid-instructeurs-file.csv', "application/vnd.ms-excel") }
-
-      before { subject }
-      it { expect(procedure_non_routee.instructeurs.pluck(:email)).to match_array(["kara@beta-gouv.fr", "philippe@mail.com", "lisa@gouv.fr"]) }
-      it { expect(flash.notice).to be_present }
-      it { expect(flash.notice).to eq("La liste des instructeurs a été importée avec succès") }
-    end
-
-    context 'when the csv file length is more than 1 mo' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur.csv', 'text/csv') }
-
-      before do
-        allow_any_instance_of(ActionDispatch::Http::UploadedFile).to receive(:size).and_return(3.megabytes)
-        subject
+      subject do
+        post :import, params: { procedure_id: procedure_non_routee.id, csv_file: csv_file }
       end
 
-      it { expect(flash.alert).to be_present }
-      it { expect(flash.alert).to eq("Importation impossible : le poids du fichier est supérieur à 1 Mo") }
-    end
+      context 'when the csv file is less than 1 mo and content type text/csv' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/instructeurs-file.csv', 'text/csv') }
 
-    context 'when the file content type is not accepted' do
-      let(:csv_file) { fixture_file_upload('spec/fixtures/files/french-flag.gif', 'image/gif') }
+        before do
+          allow(GroupeInstructeurMailer).to receive(:notify_added_instructeurs)
+            .and_return(double(deliver_later: true))
+          subject
+        end
 
-      before { subject }
+        it { expect(response.status).to eq(302) }
+        it { expect(procedure_non_routee.instructeurs.pluck(:email)).to match_array(["kara@beta-gouv.fr", "philippe@mail.com", "lisa@gouv.fr"]) }
+        it { expect(flash.alert).to be_present }
+        it { expect(flash.alert).to eq("Import terminé. Cependant les emails suivants ne sont pas pris en compte: eric") }
+        it "calls GroupeInstructeurMailer" do
+          expect(GroupeInstructeurMailer).to have_received(:notify_added_instructeurs).with(
+            procedure_non_routee.defaut_groupe_instructeur,
+            any_args,
+            admin.email
+          )
+        end
+      end
 
-      it { expect(flash.alert).to be_present }
-      it { expect(flash.alert).to eq("Importation impossible : veuillez importer un fichier CSV") }
+      context 'when the csv file has more than one column' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur.csv', 'text/csv') }
+
+        before { subject }
+
+        it { expect(response.status).to eq(302) }
+        it { expect(flash.alert).to be_present }
+        it { expect(flash.alert).to eq("Import terminé. Cependant les emails suivants ne sont pas pris en compte: kara") }
+        it { expect(procedure_non_routee.reload.routing_enabled?).to be_truthy }
+      end
+
+      context 'when the file content type is application/vnd.ms-excel' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/valid-instructeurs-file.csv', "application/vnd.ms-excel") }
+
+        before { subject }
+        it { expect(procedure_non_routee.instructeurs.pluck(:email)).to match_array(["kara@beta-gouv.fr", "philippe@mail.com", "lisa@gouv.fr"]) }
+        it { expect(flash.notice).to be_present }
+        it { expect(flash.notice).to eq("La liste des instructeurs a été importée avec succès") }
+      end
+
+      context 'when the csv file length is more than 1 mo' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/groupe-instructeur.csv', 'text/csv') }
+
+        before do
+          allow_any_instance_of(ActionDispatch::Http::UploadedFile).to receive(:size).and_return(3.megabytes)
+          subject
+        end
+
+        it { expect(flash.alert).to be_present }
+        it { expect(flash.alert).to eq("Importation impossible : le poids du fichier est supérieur à 1 Mo") }
+      end
+
+      context 'when the file content type is not accepted' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/french-flag.gif', 'image/gif') }
+
+        before { subject }
+
+        it { expect(flash.alert).to be_present }
+        it { expect(flash.alert).to eq("Importation impossible : veuillez importer un fichier CSV") }
+      end
+
+      context 'when emails are invalid' do
+        let(:csv_file) { fixture_file_upload('spec/fixtures/files/instructeurs-emails-invalides.csv', 'text/csv') }
+
+        before do
+          allow(GroupeInstructeurMailer).to receive(:notify_added_instructeurs)
+            .and_return(double(deliver_later: true))
+          subject
+        end
+
+        it { expect(flash.alert).to include("Import terminé. Cependant les emails suivants ne sont pas pris en compte:") }
+        it { expect(GroupeInstructeurMailer).not_to have_received(:notify_added_instructeurs) }
+      end
     end
   end
 
   describe '#export_groupe_instructeurs' do
-    let(:procedure) { create(:procedure, :published) }
-    let(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 1 2') }
     let(:instructeur_assigned_1) { create :instructeur, email: 'instructeur_1@ministere-a.gouv.fr', administrateurs: [admin] }
     let(:instructeur_assigned_2) { create :instructeur, email: 'instructeur_2@ministere-b.gouv.fr', administrateurs: [admin] }
 
@@ -562,14 +590,13 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
     end
 
     before do
-      procedure.administrateurs << admin
       gi_1_2.instructeurs << [instructeur_assigned_1, instructeur_assigned_2]
     end
 
     it 'generates a CSV file containing the instructeurs and groups' do
       expect(subject.status).to eq(200)
       expect(subject.stream.body.split("\n").size).to eq(3)
-      expect(subject.stream.body).to include("groupe instructeur 1 2")
+      expect(subject.stream.body).to include("groupe instructeur 2")
       expect(subject.stream.body).to include(instructeur_assigned_1.email)
       expect(subject.stream.body).to include(instructeur_assigned_2.email)
       expect(subject.header["Content-Disposition"]).to include("#{procedure.id}-groupe-instructeurs-#{Date.today}.csv")

spec/fixtures/files/groupe-instructeur-emails-invalides.csv

@@ -0,0 +1,4 @@
+Email,Groupe
+instructeur1, Paris
+instructeur2,Paris
+instructeur3,Marseille

spec/fixtures/files/instructeurs-emails-invalides.csv

@@ -0,0 +1,4 @@
+Email
+instructeur1.test
+instructeur2
+instructeur3