From 650a598698e0bb38f5963496afd5e89c09f10bbc Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 5 Jan 2021 10:03:35 +0100 Subject: [PATCH 1/3] ensure a buggy procedure does not block the others from being closed --- app/jobs/cron/auto_archive_procedure_job.rb | 24 ++++++++++++++----- .../cron/auto_archive_procedure_job_spec.rb | 20 +++++++++++++++- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/app/jobs/cron/auto_archive_procedure_job.rb b/app/jobs/cron/auto_archive_procedure_job.rb index 8887fda3c..c3a1ceb39 100644 --- a/app/jobs/cron/auto_archive_procedure_job.rb +++ b/app/jobs/cron/auto_archive_procedure_job.rb @@ -2,13 +2,25 @@ class Cron::AutoArchiveProcedureJob < Cron::CronJob self.schedule_expression = "every 1 minute" def perform(*args) - Procedure.publiees.where("auto_archive_on <= ?", Time.zone.today).each do |procedure| - procedure - .dossiers - .state_en_construction - .find_each(&:passer_automatiquement_en_instruction!) + procedures_to_close.each do |procedure| + # A buggy procedure should NEVER prevent the closing of another procedure + # we therefore exceptionally add a `begin resue` block. + begin + procedure + .dossiers + .state_en_construction + .find_each(&:passer_automatiquement_en_instruction!) - procedure.close! + procedure.close! + rescue StandardError => e + Raven.capture_exception(e, extra: { procedure_id: procedure.id }) + end end end + + def procedures_to_close + Procedure + .publiees + .where("auto_archive_on <= ?", Time.zone.today) + end end diff --git a/spec/jobs/cron/auto_archive_procedure_job_spec.rb b/spec/jobs/cron/auto_archive_procedure_job_spec.rb index fd414482a..5d00582f4 100644 --- a/spec/jobs/cron/auto_archive_procedure_job_spec.rb +++ b/spec/jobs/cron/auto_archive_procedure_job_spec.rb @@ -3,8 +3,9 @@ RSpec.describe Cron::AutoArchiveProcedureJob, type: :job do let!(:procedure_hier) { create(:procedure, :published, :with_instructeur, auto_archive_on: 1.day.ago.to_date) } let!(:procedure_aujourdhui) { create(:procedure, :published, :with_instructeur, auto_archive_on: Time.zone.today) } let!(:procedure_demain) { create(:procedure, :published, :with_instructeur, auto_archive_on: 1.day.from_now.to_date) } + let!(:job) { Cron::AutoArchiveProcedureJob.new } - subject { Cron::AutoArchiveProcedureJob.new.perform } + subject { job.perform } context "when procedures have no auto_archive_on" do before do @@ -63,4 +64,21 @@ RSpec.describe Cron::AutoArchiveProcedureJob, type: :job do it { expect(procedure_demain.close?).to eq false } end + + context 'when an error occurs' do + let!(:buggy_procedure) { create(:procedure, :published, :with_instructeur, auto_archive_on: 1.day.ago.to_date) } + + before do + error = StandardError.new('nop') + expect(buggy_procedure).to receive(:close!).and_raise(error) + expect(job).to receive(:procedures_to_close).and_return([buggy_procedure, procedure_hier]) + expect(Raven).to receive(:capture_exception).with(error, extra: { procedure_id: buggy_procedure.id }) + + subject + end + + it "should close all the procedure" do + expect(procedure_hier.reload.close?).to eq true + end + end end From 792b53beebad8a70175f1ec5ae0a18d3e2ef9f04 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Wed, 20 Jan 2021 11:07:44 +0100 Subject: [PATCH 2/3] import several instructeurs for a routee procedure Co-authored-by: simon lehericey --- app/services/instructeurs_import_service.rb | 39 +++++ .../instructeurs_import_service_spec.rb | 138 ++++++++++++++++++ 2 files changed, 177 insertions(+) create mode 100644 app/services/instructeurs_import_service.rb create mode 100644 spec/services/instructeurs_import_service_spec.rb diff --git a/app/services/instructeurs_import_service.rb b/app/services/instructeurs_import_service.rb new file mode 100644 index 000000000..fa10a00de --- /dev/null +++ b/app/services/instructeurs_import_service.rb @@ -0,0 +1,39 @@ +class InstructeursImportService + def import(procedure, groupes_emails) + admins = procedure.administrateurs + + errors = [] + + groupes_emails.each do |groupe_emails| + groupe = groupe_emails["groupe"].strip + instructeur_email = groupe_emails["email"].strip.downcase + + if groupe.present? && Devise.email_regexp.match?(instructeur_email) + gi = procedure.groupe_instructeurs.find_or_create_by!(label: groupe) + + instructeur = Instructeur.by_email(instructeur_email) || create_instructeur(admins, instructeur_email) + + if !gi.instructeurs.include?(instructeur) + gi.instructeurs << instructeur + + end + else + errors << instructeur_email + end + end + + errors + end + + private + + def create_instructeur(administrateurs, email) + user = User.create_or_promote_to_instructeur( + email, + SecureRandom.hex, + administrateurs: administrateurs + ) + user.invite! + user.instructeur + end +end diff --git a/spec/services/instructeurs_import_service_spec.rb b/spec/services/instructeurs_import_service_spec.rb new file mode 100644 index 000000000..7f50a9373 --- /dev/null +++ b/spec/services/instructeurs_import_service_spec.rb @@ -0,0 +1,138 @@ +describe InstructeursImportService do + describe '#import' do + let(:service) { InstructeursImportService.new } + let(:procedure) { create(:procedure) } + + let(:procedure_groupes) do + procedure + .groupe_instructeurs + .map { |gi| [gi.label, gi.instructeurs.map(&:email)] } + end + + subject { service.import(procedure, lines) } + + context 'nominal case' do + let(:lines) do + [ + { "groupe" => "Auvergne Rhone-Alpes", "email" => "john@lennon.fr" }, + { "groupe" => " Occitanie ", "email" => "paul@mccartney.uk" }, + { "groupe" => "Occitanie", "email" => "ringo@starr.uk" } + ] + end + + it 'imports' do + errors = subject + + expect(procedure_groupes).to match_array([ + ["Auvergne Rhone-Alpes", ["john@lennon.fr"]], + ["Occitanie", ["paul@mccartney.uk", "ringo@starr.uk"]], + ["défaut", []] + ]) + + expect(errors).to match_array([]) + end + end + + context 'when group already exists' do + let!(:gi) { create(:groupe_instructeur, label: 'Occitanie', procedure: procedure) } + let(:lines) do + [ + { "groupe" => "Occitanie", "email" => "ringo@starr.uk" } + ] + end + + before do + gi.instructeurs << create(:instructeur, email: 'george@harisson.uk') + end + + it 'adds instructeur to existing groupe' do + subject + + expect(procedure_groupes).to match_array([ + ["Occitanie", ["george@harisson.uk", "ringo@starr.uk"]], + ["défaut", []] + ]) + end + end + + context 'when an email is malformed' do + let(:lines) do + [ + { "groupe" => "Occitanie", "email" => "paul" }, + { "groupe" => "Occitanie", "email" => "  Paul@mccartney.uk " }, + { "groupe" => "Occitanie", "email" => "ringo@starr.uk" } + ] + end + + it 'ignores or corrects' do + errors = subject + + expect(procedure_groupes).to match_array([ + ["Occitanie", ["paul@mccartney.uk", "ringo@starr.uk"]], + ["défaut", []] + ]) + + expect(errors).to match_array(['paul']) + end + end + + context 'when an instructeur already exists' do + let!(:instructeur) { create(:instructeur) } + let(:lines) do + [ + { "groupe" => "Occitanie", "email" => instructeur.email }, + { "groupe" => "Occitanie", "email" => "ringo@starr.uk" } + ] + end + + it 'reuses instructeur' do + subject + + expect(procedure_groupes).to match_array([ + ["Occitanie", [instructeur.email, "ringo@starr.uk"]], + ["défaut", []] + ]) + end + end + + context 'when there are 2 emails of same instructeur to be imported' do + let(:lines) do + [ + { "groupe" => "Occitanie", "email" => "ringo@starr.uk" }, + { "groupe" => "Occitanie", "email" => "ringo@starr.uk" } + ] + end + + it 'ignores duplicated instructeur' do + subject + + expect(procedure_groupes).to match_array([ + ["Occitanie", ["ringo@starr.uk"]], + ["défaut", []] + ]) + end + end + + context 'when label of group is empty' do + let(:lines) do + [ + { "groupe" => "", "email" => "ringo@starr.uk" }, + { "groupe" => " ", "email" => "paul@starr.uk" } + ] + end + + it 'ignores instructeur' do + errors = subject + + expect(procedure_groupes).to match_array([ + ["défaut", []] + ]) + + expect(errors).to match_array([ + 'ringo@starr.uk', + 'paul@starr.uk' + ]) + end + end + end +end From b4256f9560c68689fccccd86675407ca7b5df3a6 Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Wed, 20 Jan 2021 14:35:26 +0100 Subject: [PATCH 3/3] add rake task for importing instructeurs from csv Co-authored-by: sim --- lib/tasks/instructeurs.rake | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 lib/tasks/instructeurs.rake diff --git a/lib/tasks/instructeurs.rake b/lib/tasks/instructeurs.rake new file mode 100644 index 000000000..75f49dff4 --- /dev/null +++ b/lib/tasks/instructeurs.rake @@ -0,0 +1,25 @@ +require Rails.root.join("lib", "tasks", "task_helper") + +namespace :instructeurs do + desc <<~EOD + Import several instructeurs for a procedure + rails instructeurs:import\[procedure_id,csv_path\] + EOD + task :import, [:procedure_id, :csv] => :environment do |_t, args| + procedure_id = args[:procedure_id] + csv = args[:csv] + lines = CSV.readlines(csv, headers: true) + + rake_puts "Import en cours..." + + errors = + InstructeursImportService.new.import(Procedure.find(procedure_id), lines) + + if errors.present? + rake_puts "Ces instructeurs n'ont pas pu être importés :" + rake_puts errors + end + + rake_puts "Import terminé" + end +end