Merge pull request #8396 from E-L-T/import-instructeurs-in-procedures-non-routees

feat(groupe_instructeurs): import instructeurs in unrouted procedures with a proper CSV

app/controllers/administrateurs/groupe_instructeurs_controller.rb

@@ -120,8 +120,6 @@ module Administrateurs
       end
 
       if instructeurs.present?
-        instructeurs.each { groupe_instructeur.add(_1) }
-
         flash[:notice] = if procedure.routing_enabled?
           t('.assignment',
             count: instructeurs.size,
@@ -200,15 +198,17 @@ module Administrateurs
 
     def import
       if procedure.publiee_or_close?
-        if !CSV_ACCEPTED_CONTENT_TYPES.include?(group_csv_file.content_type) && !CSV_ACCEPTED_CONTENT_TYPES.include?(marcel_content_type)
+        if !CSV_ACCEPTED_CONTENT_TYPES.include?(csv_file.content_type) && !CSV_ACCEPTED_CONTENT_TYPES.include?(marcel_content_type)
           flash[:alert] = "Importation impossible : veuillez importer un fichier CSV"
 
-        elsif group_csv_file.size > CSV_MAX_SIZE
+        elsif csv_file.size > CSV_MAX_SIZE
           flash[:alert] = "Importation impossible : le poids du fichier est supérieur à #{number_to_human_size(CSV_MAX_SIZE)}"
 
         else
-          file = group_csv_file.read
+          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') }
 
@@ -217,18 +217,27 @@ module Administrateurs
             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
-            add_instructeurs_and_get_errors = InstructeursImportService.import(procedure, groupes_emails)
+              result = InstructeursImportService.import_groupes(procedure, groupes_emails)
-
+              flash_message_for_import(result)
-            if add_instructeurs_and_get_errors.empty?
-              flash[:notice] = "La liste des instructeurs a été importée avec succès"
-            else
-              flash[:alert] = "Import terminé. Cependant les emails suivants ne sont pas pris en compte: #{add_instructeurs_and_get_errors.join(', ')}"
             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
+              result = InstructeursImportService.import_instructeurs(procedure, instructors_emails)
+              flash_message_for_import(result)
             end
           end
           redirect_to admin_procedure_groupe_instructeurs_path(procedure)
         end
       end
+    end
 
     def export_groupe_instructeurs
       groupe_instructeurs = procedure.groupe_instructeurs
@@ -298,12 +307,12 @@ module Administrateurs
       (all - assigned).sort
     end
 
-    def group_csv_file
+    def csv_file
-      params[:group_csv_file]
+      params[:group_csv_file] || params[:instructeurs_csv_file]
     end
 
     def marcel_content_type
-      Marcel::MimeType.for(group_csv_file.read, name: group_csv_file.original_filename, declared_type: group_csv_file.content_type)
+      Marcel::MimeType.for(csv_file.read, name: csv_file.original_filename, declared_type: csv_file.content_type)
     end
 
     def instructeurs_self_management_enabled_params
@@ -313,5 +322,13 @@ module Administrateurs
     def routing_enabled_params
       { routing_enabled: params.require(:routing) == 'enable' }
     end
+
+    def flash_message_for_import(result)
+      if result.blank?
+        flash[:notice] = "La liste des instructeurs a été importée avec succès"
+      else
+        flash[:alert] = "Import terminé. Cependant les emails suivants ne sont pas pris en compte: #{result.join(', ')}"
+      end
+    end
   end
 end

app/graphql/mutations/groupe_instructeur_ajouter_instructeurs.rb

@@ -11,9 +11,8 @@ module Mutations
 
     def resolve(groupe_instructeur:, instructeurs:)
       ids, emails = partition_instructeurs_by(instructeurs)
-      instructeurs, invalid_emails = groupe_instructeur.add_instructeurs(ids:, emails:)
+      _, invalid_emails = groupe_instructeur.add_instructeurs(ids:, emails:)
 
-      instructeurs.each { groupe_instructeur.add(_1) }
       groupe_instructeur.reload
 
       result = { groupe_instructeur: }

app/graphql/mutations/groupe_instructeur_creer.rb

@@ -29,9 +29,8 @@ module Mutations
         result = { groupe_instructeur: }
 
         if emails.present? || ids.present?
-          instructeurs, invalid_emails = groupe_instructeur.add_instructeurs(ids:, emails:)
+          _, invalid_emails = groupe_instructeur.add_instructeurs(ids:, emails:)
 
-          instructeurs.each { groupe_instructeur.add(_1) }
           groupe_instructeur.reload
 
           if invalid_emails.present?

app/models/groupe_instructeur.rb

@@ -66,6 +66,7 @@ class GroupeInstructeur < ApplicationRecord
 
     # We dont't want to assign a user to a groupe_instructeur if they are already assigned to it
     instructeurs_to_add -= instructeurs
+    instructeurs_to_add.each { add(_1) }
 
     [instructeurs_to_add, invalid_emails]
   end

app/services/instructeurs_import_service.rb

@@ -1,54 +1,52 @@
 class InstructeursImportService
-  def self.import(procedure, groupes_emails)
+  def self.import_groupes(procedure, groupes_emails)
-    created_at = Time.zone.now
+    groupes_emails, error_groupe_emails = groupes_emails.partition { _1['groupe'].present? }
-    updated_at = Time.zone.now
 
-    admins = procedure.administrateurs
+    groupes_emails = groupes_emails.map do
+      {
+        groupe: _1['groupe'].strip,
+        email: _1['email'].present? ? EmailSanitizableConcern::EmailSanitizer.sanitize(_1['email']) : nil
+      }
+    end
+    errors = error_groupe_emails.map { _1['email'] }.uniq
+    target_labels = groupes_emails.map { _1[:groupe] }.uniq
 
-    groupes_emails, error_groupe_emails = groupes_emails
-      .map { |groupe_email| { "groupe" => groupe_email["groupe"].present? ? groupe_email["groupe"].strip : nil, "email" => groupe_email["email"].present? ? groupe_email["email"].gsub(/[[:space:]]/, '').downcase : nil } }
-      .partition { |groupe_email| Devise.email_regexp.match?(groupe_email['email']) && groupe_email['groupe'].present? }
-
-    errors = error_groupe_emails.map { |group_email| group_email['email'] }
-
-    target_labels = groupes_emails.map { |groupe_email| groupe_email['groupe'] }.uniq
     missing_labels = target_labels - procedure.groupe_instructeurs.pluck(:label)
 
     if missing_labels.present?
-      GroupeInstructeur.insert_all(missing_labels.map { |label| { label: label, procedure_id: procedure.id, created_at: created_at, updated_at: updated_at } })
+      created_at = Time.zone.now
+      GroupeInstructeur.insert_all(missing_labels.map { |label| { procedure_id: procedure.id, label:, created_at:, updated_at: created_at } })
     end
 
-    target_groupes = procedure.reload.groupe_instructeurs
+    emails_in_groupe = groupes_emails
+      .group_by { _1[:groupe] }
+      .transform_values { |groupes_emails| groupes_emails.map { _1[:email] }.uniq }
+    emails_in_groupe.default = []
 
-    target_emails = groupes_emails.map { |groupe_email| groupe_email["email"] }.uniq
+    target_groupes = procedure
+      .groupe_instructeurs
+      .where(label: target_labels)
+      .map { [_1, emails_in_groupe[_1.label]] }
+      .to_h
 
-    existing_emails = Instructeur.where(user: { email: target_emails }).pluck(:email)
+    target_groupes.each do |groupe_instructeur, emails|
-    missing_emails = target_emails - existing_emails
+      _, invalid_emails = groupe_instructeur.add_instructeurs(emails:)
-    missing_emails.each { |email| create_instructeur(admins, email) }
+      errors << invalid_emails
-
-    target_instructeurs = User.where(email: target_emails).map(&:instructeur)
-
-    groupes_emails.each do |groupe_email|
-      gi = target_groupes.find { |g| g.label == groupe_email['groupe'] }
-      instructeur = target_instructeurs.find { |i| i.email == groupe_email['email'] }
-
-      if !gi.instructeurs.include?(instructeur)
-        gi.instructeurs << instructeur
-      end
     end
 
-    errors
+    errors.flatten
   end
 
-  private
+  def self.import_instructeurs(procedure, emails)
+    instructeurs_emails = emails
+      .map { _1["email"] }
+      .compact
+      .map { EmailSanitizableConcern::EmailSanitizer.sanitize(_1) }
 
-  def self.create_instructeur(administrateurs, email)
+    groupe_instructeur = procedure.defaut_groupe_instructeur
-    user = User.create_or_promote_to_instructeur(
+
-      email,
+    _, invalid_emails = groupe_instructeur.add_instructeurs(emails: instructeurs_emails)
-      SecureRandom.hex,
+
-      administrateurs: administrateurs
+    invalid_emails
-    )
-    user.invite!
-    user.instructeur
   end
 end

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

@@ -28,11 +28,13 @@
     = 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
-        = t('.csv_import.notice_1')
+        = procedure.routing_enabled? ? t('.csv_import.routing_enabled.notice_1') : t('.csv_import.routing_disabled.notice_1')
       %p.notice
         = t('.csv_import.notice_2', csv_max_size: number_to_human_size(csv_max_size))
-      %p.mt-2.mb-2= link_to t('.csv_import.download_exemple'), "/csv/#{I18n.locale}/import-groupe-test.csv"
+      - sample_file_path = procedure.routing_enabled? ? "/csv/#{I18n.locale}/import-groupe-test.csv" : "/csv/import-instructeurs-test.csv"
-      = file_field_tag :group_csv_file, required: true, accept: 'text/csv', size: "1"
+      %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"
       = submit_tag t('.csv_import.import_file'), class: 'button primary send', data: { disable_with: "Envoi..." }
   - else
     %p.mt-4.form.font-weight-bold.mb-2.text-lg

config/initializers/acsv.rb

@@ -4,7 +4,7 @@ require 'csv'
 module ACSV
   class CSV < ::CSV
     def self.new_for_ruby3(data, options = {})
-      options[:col_sep] ||= ACSV::Detect.separator(data)
+      options[:col_sep] ||= ACSV::Detect.separator(data) || :auto
       # because of the Separation of positional and keyword arguments in Ruby 3.0
       # (https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/)
       # instead of

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

@@ -41,7 +41,10 @@ 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_2: The size of the file must be less than %{csv_max_size}.
           download_exemple: Download sample CSV file
           import_file: Import file

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

@@ -47,8 +47,11 @@ fr:
           notice: Ce groupe sera un choix de la liste "%{routing_criteria_name}"
         csv_import:
           title: Importer par CSV
-          notice_1: Le fichier csv doit comporter 2 colonnes (Groupe, Email) et être séparé par des virgules. Si vous n'avez pas créé de groupe, entrez « défaut » dans la colonne Groupe pour chaque instructeur. 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 ».
+          routing_enabled:
-          notice_2: Le poids du fichier doit être inférieur à %{csv_max_size}
+            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
           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

public/csv/import-instructeurs-test.csv

@@ -0,0 +1,5 @@
+Email
+camilia@gouv.fr
+kara@gouv.fr
+simon@gouv.fr
+pauline@gouv.fr

spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb

@@ -405,6 +405,17 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
       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") }
 
@@ -465,6 +476,65 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do
     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 { 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") }
+    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') }
+
+      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
+      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
+  end
+
   describe '#export_groupe_instructeurs' do
     let(:procedure) { create(:procedure, :published) }
     let(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 1 2') }

spec/fixtures/files/instructeurs-file.csv

@@ -0,0 +1,5 @@
+Email
+kara@beta-gouv.fr
+philippe@mail.com
+lisa@gouv.fr
+eric

spec/fixtures/files/valid-instructeurs-file.csv

@@ -0,0 +1,4 @@
+Email
+kara@beta-gouv.fr
+philippe@mail.com
+lisa@gouv.fr

spec/services/instructeurs_import_service_spec.rb

@@ -1,5 +1,5 @@
 describe InstructeursImportService do
-  describe '#import' do
+  describe '#import_groupes' do
     let(:procedure) { create(:procedure) }
 
     let(:procedure_groupes) do
@@ -9,7 +9,7 @@ describe InstructeursImportService do
         .to_h
     end
 
-    subject { described_class.import(procedure, lines) }
+    subject { described_class.import_groupes(procedure, lines) }
 
     context 'nominal case' do
       let(:lines) do
@@ -20,7 +20,7 @@ describe InstructeursImportService do
         ]
       end
 
-      it 'imports' do
+      it 'imports groupes' do
         errors = subject
 
         expect(procedure_groupes.keys).to contain_exactly("Auvergne Rhone-Alpes", "Occitanie", "défaut")
@@ -126,4 +126,21 @@ describe InstructeursImportService do
       end
     end
   end
+
+  describe '#import_instructeurs' do
+    let(:procedure_non_routee) { create(:procedure) }
+
+    subject { described_class.import_instructeurs(procedure_non_routee, emails) }
+
+    context 'nominal case' do
+      let(:emails) { [{ "email" => "john@lennon.fr" }, { "email" => "paul@mccartney.uk" }, { "email" => "ringo@starr.uk" }] }
+
+      it 'imports instructeurs' do
+        errors = subject
+        expect(procedure_non_routee.defaut_groupe_instructeur.instructeurs.pluck(:email)).to contain_exactly("john@lennon.fr", "paul@mccartney.uk", "ringo@starr.uk")
+
+        expect(errors).to match_array([])
+      end
+    end
+  end
 end