From 25df48f2553cc23e84f3148ced51d7ed7af92a33 Mon Sep 17 00:00:00 2001 From: Eric Leroy-Terquem Date: Tue, 10 Jan 2023 14:36:56 +0100 Subject: [PATCH] feat(groupe_instructeurs): import instructeurs in unrouted procedure with a proper CSV --- .../groupe_instructeurs_controller.rb | 55 +++++++++++------ app/services/instructeurs_import_service.rb | 51 ++++++++++++---- .../groupe_instructeurs/_edit.html.haml | 8 ++- .../groupe_instructeurs/en.yml | 5 +- .../groupe_instructeurs/fr.yml | 7 ++- public/csv/import-instructeurs-test.csv | 5 ++ .../groupe_instructeurs_controller_spec.rb | 59 +++++++++++++++++++ spec/fixtures/files/instructeurs-file.csv | 5 ++ .../files/valid-instructeurs-file.csv | 4 ++ .../instructeurs_import_service_spec.rb | 23 +++++++- 10 files changed, 184 insertions(+), 38 deletions(-) create mode 100644 public/csv/import-instructeurs-test.csv create mode 100644 spec/fixtures/files/instructeurs-file.csv create mode 100644 spec/fixtures/files/valid-instructeurs-file.csv diff --git a/app/controllers/administrateurs/groupe_instructeurs_controller.rb b/app/controllers/administrateurs/groupe_instructeurs_controller.rb index 199b53b19..0e4392224 100644 --- a/app/controllers/administrateurs/groupe_instructeurs_controller.rb +++ b/app/controllers/administrateurs/groupe_instructeurs_controller.rb @@ -200,33 +200,54 @@ 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) - 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') } - groupes_emails_has_keys = groupes_emails.first.has_key?("groupe") && groupes_emails.first.has_key?("email") + 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') } - 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) + groupes_emails_has_keys = groupes_emails.first.has_key?("groupe") && groupes_emails.first.has_key?("email") - if add_instructeurs_and_get_errors.empty? - flash[:notice] = "La liste des instructeurs a été importée avec succès" + 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 - flash[:alert] = "Import terminé. Cependant les emails suivants ne sont pas pris en compte: #{add_instructeurs_and_get_errors.join(', ')}" + add_instructeurs_and_get_errors = InstructeursImportService.import_groupes(procedure, groupes_emails) + + if add_instructeurs_and_get_errors.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: #{add_instructeurs_and_get_errors.join(', ')}" + end + 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 + add_instructeurs_and_get_errors = InstructeursImportService.import_instructeurs(procedure, instructors_emails) + + if add_instructeurs_and_get_errors.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: #{add_instructeurs_and_get_errors.join(', ')}" + end end end + redirect_to admin_procedure_groupe_instructeurs_path(procedure) end - redirect_to admin_procedure_groupe_instructeurs_path(procedure) end end @@ -298,12 +319,12 @@ module Administrateurs (all - assigned).sort end - def group_csv_file - params[:group_csv_file] + def 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 diff --git a/app/services/instructeurs_import_service.rb b/app/services/instructeurs_import_service.rb index fad1e5922..5638c4cdc 100644 --- a/app/services/instructeurs_import_service.rb +++ b/app/services/instructeurs_import_service.rb @@ -1,5 +1,5 @@ class InstructeursImportService - def self.import(procedure, groupes_emails) + def self.import_groupes(procedure, groupes_emails) created_at = Time.zone.now updated_at = Time.zone.now @@ -20,21 +20,30 @@ class InstructeursImportService target_groupes = procedure.reload.groupe_instructeurs - target_emails = groupes_emails.map { |groupe_email| groupe_email["email"] }.uniq - - existing_emails = Instructeur.where(user: { email: target_emails }).pluck(:email) - missing_emails = target_emails - existing_emails - missing_emails.each { |email| create_instructeur(admins, email) } - - target_instructeurs = User.where(email: target_emails).map(&:instructeur) + target_instructeurs = find_or_create_instructeurs(admins, groupes_emails) 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'] } + add_instructeur_to_groupe(target_instructeurs, groupe_email, gi) + end - if !gi.instructeurs.include?(instructeur) - gi.instructeurs << instructeur - end + errors + end + + def self.import_instructeurs(procedure, emails) + admins = procedure.administrateurs + + instructeurs_emails, error_instructeurs_emails = emails + .map { |instructeur_email| { "email" => instructeur_email["email"].present? ? instructeur_email["email"].gsub(/[[:space:]]/, '').downcase : nil } } + .partition { |instructeur_email| Devise.email_regexp.match?(instructeur_email['email']) } + + errors = error_instructeurs_emails.map { |instructeur_email| instructeur_email['email'] } + + target_instructeurs = find_or_create_instructeurs(admins, instructeurs_emails) + + instructeurs_emails.each do |instructeur_email| + gi = procedure.defaut_groupe_instructeur + add_instructeur_to_groupe(target_instructeurs, instructeur_email, gi) end errors @@ -42,6 +51,16 @@ class InstructeursImportService private + def self.find_or_create_instructeurs(administrateurs, instructeurs_emails) + target_emails = instructeurs_emails.map { |instructeur_email| instructeur_email["email"] }.uniq + + existing_emails = Instructeur.where(user: { email: target_emails }).pluck(:email) + missing_emails = target_emails - existing_emails + missing_emails.each { |email| create_instructeur(administrateurs, email) } + + User.where(email: target_emails).map(&:instructeur) + end + def self.create_instructeur(administrateurs, email) user = User.create_or_promote_to_instructeur( email, @@ -51,4 +70,12 @@ class InstructeursImportService user.invite! user.instructeur end + + def self.add_instructeur_to_groupe(target_instructeurs, instructeur_email, gi) + instructeur = target_instructeurs.find { |i| i.email == instructeur_email['email'] } + + if !gi.instructeurs.include?(instructeur) + gi.instructeurs << instructeur + end + end end diff --git a/app/views/administrateurs/groupe_instructeurs/_edit.html.haml b/app/views/administrateurs/groupe_instructeurs/_edit.html.haml index 7a5543d3b..8e19f57d3 100644 --- a/app/views/administrateurs/groupe_instructeurs/_edit.html.haml +++ b/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" - = file_field_tag :group_csv_file, required: true, accept: 'text/csv', size: "1" + - 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" = 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 diff --git a/config/locales/views/administrateurs/groupe_instructeurs/en.yml b/config/locales/views/administrateurs/groupe_instructeurs/en.yml index 18a10f864..e39c7b6f8 100644 --- a/config/locales/views/administrateurs/groupe_instructeurs/en.yml +++ b/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 - 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_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 diff --git a/config/locales/views/administrateurs/groupe_instructeurs/fr.yml b/config/locales/views/administrateurs/groupe_instructeurs/fr.yml index 3d1f3a256..6f94ef85f 100644 --- a/config/locales/views/administrateurs/groupe_instructeurs/fr.yml +++ b/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 ». - notice_2: Le poids du fichier doit être inférieur à %{csv_max_size} + 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 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 diff --git a/public/csv/import-instructeurs-test.csv b/public/csv/import-instructeurs-test.csv new file mode 100644 index 000000000..df7c8ecff --- /dev/null +++ b/public/csv/import-instructeurs-test.csv @@ -0,0 +1,5 @@ +Email +camilia@gouv.fr +kara@gouv.fr +simon@gouv.fr +pauline@gouv.fr diff --git a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb index 7c1693b88..b785ce4c7 100644 --- a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb @@ -476,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 suivant ce modèle") } + 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') } diff --git a/spec/fixtures/files/instructeurs-file.csv b/spec/fixtures/files/instructeurs-file.csv new file mode 100644 index 000000000..6e3f06311 --- /dev/null +++ b/spec/fixtures/files/instructeurs-file.csv @@ -0,0 +1,5 @@ +Email +kara@beta-gouv.fr +philippe@mail.com +lisa@gouv.fr +eric diff --git a/spec/fixtures/files/valid-instructeurs-file.csv b/spec/fixtures/files/valid-instructeurs-file.csv new file mode 100644 index 000000000..fb5f0d056 --- /dev/null +++ b/spec/fixtures/files/valid-instructeurs-file.csv @@ -0,0 +1,4 @@ +Email +kara@beta-gouv.fr +philippe@mail.com +lisa@gouv.fr diff --git a/spec/services/instructeurs_import_service_spec.rb b/spec/services/instructeurs_import_service_spec.rb index 285d3dc98..88f9b616c 100644 --- a/spec/services/instructeurs_import_service_spec.rb +++ b/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