From 9ca026a630a5ef4315c8749bd9347ae36283d9b0 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Nov 2019 14:49:53 +0100 Subject: [PATCH 1/7] Use a select2 box for looking to instructeurs --- .../new_design/add_instructeur.scss | 38 ++++++++++++++++ .../groupe_instructeurs_controller.rb | 2 +- .../groupe_instructeurs_controller.rb | 45 ++++++++++++++----- app/javascript/new_design/select2.js | 23 ++++++++++ app/mailers/groupe_instructeur_mailer.rb | 6 +-- .../add_instructeur.html.haml | 11 ----- .../add_instructeurs.html.haml | 11 +++++ .../groupe_instructeurs/show.html.haml | 9 ++-- .../groupe_instructeurs/fr.yml | 10 +++++ .../groupe_instructeurs_controller_spec.rb | 16 ++++--- 10 files changed, 136 insertions(+), 35 deletions(-) create mode 100644 app/assets/stylesheets/new_design/add_instructeur.scss delete mode 100644 app/views/groupe_instructeur_mailer/add_instructeur.html.haml create mode 100644 app/views/groupe_instructeur_mailer/add_instructeurs.html.haml create mode 100644 config/locales/views/new_administrateur/groupe_instructeurs/fr.yml diff --git a/app/assets/stylesheets/new_design/add_instructeur.scss b/app/assets/stylesheets/new_design/add_instructeur.scss new file mode 100644 index 000000000..8e446e281 --- /dev/null +++ b/app/assets/stylesheets/new_design/add_instructeur.scss @@ -0,0 +1,38 @@ +@import "constants"; +@import "colors"; + +.instructeur-wrapper { + .select-instructeurs { + width: 100%; + } + + .select2-container--default { + .select2-selection--multiple { + border: solid 1px $border-grey; + + .select2-selection__choice, // scss-lint:disable SelectorFormat + .select2-search--inline { + padding: $default-spacer; + } + } + + &.select2-container--focus { + .select2-selection--multiple { + border: 1px solid $blue; + box-shadow: 0px 0px 2px 1px $blue; + } + } + + .select2-results__option { // scss-lint:disable SelectorFormat + padding: $default-spacer; + } + + .custom-select2-option { + .icon { + margin-right: $default-spacer; + } + } + } +} + + diff --git a/app/controllers/instructeurs/groupe_instructeurs_controller.rb b/app/controllers/instructeurs/groupe_instructeurs_controller.rb index e55d7bb79..42b73866e 100644 --- a/app/controllers/instructeurs/groupe_instructeurs_controller.rb +++ b/app/controllers/instructeurs/groupe_instructeurs_controller.rb @@ -24,7 +24,7 @@ module Instructeurs groupe_instructeur.instructeurs << @instructeur flash[:notice] = "L’instructeur « #{instructeur_email} » a été affecté au groupe." GroupeInstructeurMailer - .add_instructeur(groupe_instructeur, @instructeur, current_user.email) + .add_instructeurs(groupe_instructeur, [@instructeur], current_user.email) .deliver_later end diff --git a/app/controllers/new_administrateur/groupe_instructeurs_controller.rb b/app/controllers/new_administrateur/groupe_instructeurs_controller.rb index 4bacdb64d..06cdf829c 100644 --- a/app/controllers/new_administrateur/groupe_instructeurs_controller.rb +++ b/app/controllers/new_administrateur/groupe_instructeurs_controller.rb @@ -12,6 +12,7 @@ module NewAdministrateur @procedure = procedure @groupe_instructeur = groupe_instructeur @instructeurs = paginated_instructeurs + @available_instructeur_emails = available_instructeur_emails end def create @@ -40,6 +41,7 @@ module NewAdministrateur else @procedure = procedure @instructeurs = paginated_instructeurs + @available_instructeur_emails = available_instructeur_emails flash[:alert] = "le nom « #{label} » est déjà pris par un autre groupe." render :show @@ -47,18 +49,35 @@ module NewAdministrateur end def add_instructeur - @instructeur = Instructeur.by_email(instructeur_email) || - create_instructeur(instructeur_email) + emails = params['emails'].map(&:strip).map(&:downcase) - if groupe_instructeur.instructeurs.include?(@instructeur) - flash[:alert] = "L’instructeur « #{instructeur_email} » est déjà dans le groupe." + correct_emails, bad_emails = emails + .partition { |email| URI::MailTo::EMAIL_REGEXP.match?(email) } + + if bad_emails.present? + flash[:alert] = t('.wrong_address', + count: bad_emails.count, + value: bad_emails.join(', ')) + end + + email_to_adds = correct_emails - groupe_instructeur.instructeurs.pluck(:email) + + if email_to_adds.present? + instructeurs = email_to_adds.map do |instructeur_email| + Instructeur.by_email(instructeur_email) || + create_instructeur(instructeur_email) + end - else - groupe_instructeur.instructeurs << @instructeur - flash[:notice] = "L’instructeur « #{instructeur_email} » a été affecté au groupe." GroupeInstructeurMailer - .add_instructeur(groupe_instructeur, @instructeur, current_user.email) + .add_instructeurs(groupe_instructeur, instructeurs, current_user.email) .deliver_later + + groupe_instructeur.instructeurs << instructeurs + + flash[:notice] = t('.assignment', + count: email_to_adds.count, + value: email_to_adds.join(', '), + groupe: groupe_instructeur.label) end redirect_to procedure_groupe_instructeur_path(procedure, groupe_instructeur) @@ -110,10 +129,6 @@ module NewAdministrateur procedure.groupe_instructeurs.find(params[:id]) end - def instructeur_email - params[:instructeur][:email].strip.downcase - end - def instructeur_id params[:instructeur][:id] end @@ -141,5 +156,11 @@ module NewAdministrateur def routing_criteria_name params[:procedure][:routing_criteria_name] end + + def available_instructeur_emails + all = current_administrateur.instructeurs.pluck(:email) + assigned = groupe_instructeur.instructeurs.pluck(:email) + (all - assigned).sort + end end end diff --git a/app/javascript/new_design/select2.js b/app/javascript/new_design/select2.js index bbc8cdb86..a9fa8506c 100644 --- a/app/javascript/new_design/select2.js +++ b/app/javascript/new_design/select2.js @@ -1,6 +1,13 @@ import $ from 'jquery'; import 'select2'; +const optionTemplate = email => + $( + '' + + email.text + + '' + ); + addEventListener('ds:page:update', () => { $('select.select2').select2({ language: 'fr', @@ -20,4 +27,20 @@ addEventListener('ds:page:update', () => { maximumSelectionLength: '30', width: '300px' }); + + $('select.select2-limited.select-instructeurs').select2({ + language: 'fr', + dropdownParent: $('.instructeur-wrapper'), + placeholder: 'Saisir l’adresse email de l’instructeur', + tags: true, + tokenSeparators: [',', ' '], + templateResult: optionTemplate, + templateSelection: function(email) { + return $( + '' + + email.text + + '' + ); + } + }); }); diff --git a/app/mailers/groupe_instructeur_mailer.rb b/app/mailers/groupe_instructeur_mailer.rb index 612ac2fd5..dbb122c87 100644 --- a/app/mailers/groupe_instructeur_mailer.rb +++ b/app/mailers/groupe_instructeur_mailer.rb @@ -1,9 +1,9 @@ class GroupeInstructeurMailer < ApplicationMailer layout 'mailers/layout' - def add_instructeur(group, instructeur, current_instructeur_email) - @email = instructeur.email - @group = group + def add_instructeurs(group, instructeurs, current_instructeur_email) + @new_instructeur_emails = instructeurs.map(&:email) + @group = Procedure.last.defaut_groupe_instructeur @current_instructeur_email = current_instructeur_email subject = "Ajout d’un instructeur dans le groupe \"#{group.label}\"" diff --git a/app/views/groupe_instructeur_mailer/add_instructeur.html.haml b/app/views/groupe_instructeur_mailer/add_instructeur.html.haml deleted file mode 100644 index 578312cb7..000000000 --- a/app/views/groupe_instructeur_mailer/add_instructeur.html.haml +++ /dev/null @@ -1,11 +0,0 @@ -%p - Bonjour, - -%p - L’instructeur « #{@email} » a été affecté au groupe « #{@group.label} » par « #{@current_instructeur_email} », en charge de la démarche « #{@group.procedure.libelle} ». - -%p - Cliquez sur le lien ci-dessous pour voir la liste des instructeurs de ce groupe : - = link_to(@group.label, procedure_groupe_instructeur_url(@group.procedure, @group)) - -= render partial: "layouts/mailers/signature" diff --git a/app/views/groupe_instructeur_mailer/add_instructeurs.html.haml b/app/views/groupe_instructeur_mailer/add_instructeurs.html.haml new file mode 100644 index 000000000..b04110592 --- /dev/null +++ b/app/views/groupe_instructeur_mailer/add_instructeurs.html.haml @@ -0,0 +1,11 @@ +%p + Bonjour, + +%p + #{t('new_administrateur.groupe_instructeurs.add_instructeur.assignment', count: @new_instructeur_emails.count, value: @new_instructeur_emails.join(', '), groupe: @group.label).chomp('.')} par « #{@current_instructeur_email} », en charge de la démarche « #{@group.procedure.libelle} ». + +%p + Cliquez sur le lien ci-dessous pour voir la liste des instructeurs de ce groupe : + = link_to(@group.label, procedure_groupe_instructeur_url(@group.procedure, @group)) + += render partial: "layouts/mailers/signature" diff --git a/app/views/new_administrateur/groupe_instructeurs/show.html.haml b/app/views/new_administrateur/groupe_instructeurs/show.html.haml index 393726bd0..4bb5ffe04 100644 --- a/app/views/new_administrateur/groupe_instructeurs/show.html.haml +++ b/app/views/new_administrateur/groupe_instructeurs/show.html.haml @@ -22,9 +22,12 @@ url: { action: :add_instructeur }, html: { class: 'form' } do |f| - = f.label :email do - Affecter un nouvel instructeur - = f.email_field :email, placeholder: 'marie.dupont@exemple.fr', required: true + .instructeur-wrapper + = select_tag :emails, + options_for_select(@available_instructeur_emails), + multiple: true, + class: 'select-instructeurs select2-limited' + = f.submit 'Affecter', class: 'button primary send' %table.table.mt-2 diff --git a/config/locales/views/new_administrateur/groupe_instructeurs/fr.yml b/config/locales/views/new_administrateur/groupe_instructeurs/fr.yml new file mode 100644 index 000000000..98b9c6c34 --- /dev/null +++ b/config/locales/views/new_administrateur/groupe_instructeurs/fr.yml @@ -0,0 +1,10 @@ +fr: + new_administrateur: + groupe_instructeurs: + add_instructeur: + wrong_address: + one: "%{value} n'est pas une adresse email valide" + other: "%{value} ne sont pas des adresses emails valides" + assignment: + one: "L’instructeur %{value} a été affecté au groupe « %{groupe} »." + other: "Les instructeurs %{value} ont été affectés au groupe « %{groupe} »." diff --git a/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb b/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb index d821de26a..2e128a487 100644 --- a/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb @@ -93,20 +93,26 @@ describe NewAdministrateur::GroupeInstructeursController, type: :controller do params: { procedure_id: procedure.id, id: gi_1_1.id, - instructeur: { email: new_instructeur_email } + emails: new_instructeur_emails } end - context 'of a new instructeur' do - let(:new_instructeur_email) { 'new_instructeur@mail.com' } + context 'of a news instructeurs' do + let(:new_instructeur_emails) { ['new_i1@mail.com', 'new_i2@mail.com'] } - it { expect(gi_1_1.instructeurs.map(&:email)).to include(new_instructeur_email) } + it { expect(gi_1_1.instructeurs.pluck(:email)).to include(*new_instructeur_emails) } it { expect(flash.notice).to be_present } it { expect(response).to redirect_to(procedure_groupe_instructeur_path(procedure, gi_1_1)) } end context 'of an instructeur already in the group' do - let(:new_instructeur_email) { instructeur.email } + let(:new_instructeur_emails) { [instructeur.email] } + + it { expect(response).to redirect_to(procedure_groupe_instructeur_path(procedure, procedure.defaut_groupe_instructeur)) } + end + + context 'of badly formed email' do + let(:new_instructeur_emails) { ['badly_formed_email'] } it { expect(flash.alert).to be_present } it { expect(response).to redirect_to(procedure_groupe_instructeur_path(procedure, procedure.defaut_groupe_instructeur)) } From 67495e9662c946b13d50c0dbe078b3b4f01124f2 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 13 Nov 2019 18:48:18 +0100 Subject: [PATCH 2/7] Add person icon to table --- .../new_administrateur/groupe_instructeurs/show.html.haml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/new_administrateur/groupe_instructeurs/show.html.haml b/app/views/new_administrateur/groupe_instructeurs/show.html.haml index 4bb5ffe04..76bf71f6f 100644 --- a/app/views/new_administrateur/groupe_instructeurs/show.html.haml +++ b/app/views/new_administrateur/groupe_instructeurs/show.html.haml @@ -37,7 +37,9 @@ %tbody - @instructeurs.each do |instructeur| %tr - %td= instructeur.email + %td + %span.icon.person + #{instructeur.email} %td.actions= button_to 'retirer', { action: :remove_instructeur }, { method: :delete, From 874439580ba357757e802140bcd9cc2b7ed0de42 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 13 Nov 2019 18:48:53 +0100 Subject: [PATCH 3/7] Pluralize some texts --- .../groupe_instructeurs/index.html.haml | 2 +- .../new_administrateur/groupe_instructeurs/show.html.haml | 2 +- .../views/new_administrateur/groupe_instructeurs/fr.yml | 8 ++++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/views/new_administrateur/groupe_instructeurs/index.html.haml b/app/views/new_administrateur/groupe_instructeurs/index.html.haml index 6f9661a11..c7882b258 100644 --- a/app/views/new_administrateur/groupe_instructeurs/index.html.haml +++ b/app/views/new_administrateur/groupe_instructeurs/index.html.haml @@ -28,7 +28,7 @@ %table.table.mt-2 %thead %tr - %th{ colspan: 2 } Liste des groupes + %th{ colspan: 2 }= t(".existing_groupe", count: @groupes_instructeurs.count) %tbody - @groupes_instructeurs.each do |group| %tr diff --git a/app/views/new_administrateur/groupe_instructeurs/show.html.haml b/app/views/new_administrateur/groupe_instructeurs/show.html.haml index 76bf71f6f..c73e2af53 100644 --- a/app/views/new_administrateur/groupe_instructeurs/show.html.haml +++ b/app/views/new_administrateur/groupe_instructeurs/show.html.haml @@ -33,7 +33,7 @@ %table.table.mt-2 %thead %tr - %th{ colspan: 2 } Instructeurs affectés + %th{ colspan: 2 }= t('.assigned_instructeur', count: @instructeurs.count) %tbody - @instructeurs.each do |instructeur| %tr diff --git a/config/locales/views/new_administrateur/groupe_instructeurs/fr.yml b/config/locales/views/new_administrateur/groupe_instructeurs/fr.yml index 98b9c6c34..4e0ace44d 100644 --- a/config/locales/views/new_administrateur/groupe_instructeurs/fr.yml +++ b/config/locales/views/new_administrateur/groupe_instructeurs/fr.yml @@ -1,6 +1,14 @@ fr: new_administrateur: groupe_instructeurs: + index: + existing_groupe: + one: "%{count} groupe existe" + other: "%{count} groupes existent" + show: + assigned_instructeur: + one: "%{count} instructeur est affecté" + other: "%{count} instructeurs sont affectés" add_instructeur: wrong_address: one: "%{value} n'est pas une adresse email valide" From 3217f18a0b4584231347bf5263492a77b67aa42c Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 13 Nov 2019 19:41:30 +0100 Subject: [PATCH 4/7] Simple wording --- .../new_administrateur/groupe_instructeurs/index.html.haml | 4 ++-- .../new_administrateur/groupe_instructeurs/show.html.haml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/new_administrateur/groupe_instructeurs/index.html.haml b/app/views/new_administrateur/groupe_instructeurs/index.html.haml index c7882b258..9adc759d7 100644 --- a/app/views/new_administrateur/groupe_instructeurs/index.html.haml +++ b/app/views/new_administrateur/groupe_instructeurs/index.html.haml @@ -12,7 +12,7 @@ = f.label :routing_criteria_name do Libellé du routage %span.notice Ce texte apparaitra sur le formulaire usager comme le libellé d'une liste - = f.text_field :routing_criteria_name, placeholder: 'Votre ville', required: true + = f.text_field :routing_criteria_name, placeholder: 'ex. Votre ville', required: true = f.submit 'Renommer', class: 'button primary send' .card @@ -22,7 +22,7 @@ = f.label :label do Ajouter un groupe %span.notice Ce groupe sera un choix de la liste « #{@procedure.routing_criteria_name} » . - = f.text_field :label, placeholder: 'Ville de Bordeaux', required: true + = f.text_field :label, placeholder: 'ex. Ville de Bordeaux', required: true = f.submit 'Ajouter le groupe', class: 'button primary send' %table.table.mt-2 diff --git a/app/views/new_administrateur/groupe_instructeurs/show.html.haml b/app/views/new_administrateur/groupe_instructeurs/show.html.haml index c73e2af53..b6cb80182 100644 --- a/app/views/new_administrateur/groupe_instructeurs/show.html.haml +++ b/app/views/new_administrateur/groupe_instructeurs/show.html.haml @@ -17,7 +17,7 @@ = f.submit 'Renommer', class: 'button primary send' .card - .card-title Gestion des instructeurs + .card-title Affectation des instructeurs = form_for :instructeur, url: { action: :add_instructeur }, html: { class: 'form' } do |f| From 8a42b9f97a8da9b88e45120e005f294e1e4e7964 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 14 Nov 2019 13:06:42 +0100 Subject: [PATCH 5/7] Add groupe instructeur mailer preview --- .../mailers/previews/groupe_instructeur_mailer_preview.rb | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 spec/mailers/previews/groupe_instructeur_mailer_preview.rb diff --git a/spec/mailers/previews/groupe_instructeur_mailer_preview.rb b/spec/mailers/previews/groupe_instructeur_mailer_preview.rb new file mode 100644 index 000000000..3ba0527c4 --- /dev/null +++ b/spec/mailers/previews/groupe_instructeur_mailer_preview.rb @@ -0,0 +1,8 @@ +class GroupeInstructeurMailerPreview < ActionMailer::Preview + def add_instructeurs + groupe = GroupeInstructeur.new(label: 'Val-De-Marne') + current_instructeur_email = 'admin@dgfip.com' + instructeurs = Instructeur.limit(2) + GroupeInstructeurMailer.add_instructeurs(groupe, instructeurs, current_instructeur_email) + end +end From 361e57355a2d264371b04cfb9b36dfecc6c01bf7 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 14 Nov 2019 15:12:25 +0100 Subject: [PATCH 6/7] Spec: full_scenario use js --- spec/features/routing/full_scenario_spec.rb | 41 +++++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/spec/features/routing/full_scenario_spec.rb b/spec/features/routing/full_scenario_spec.rb index 3eeb141dc..9162b14bb 100644 --- a/spec/features/routing/full_scenario_spec.rb +++ b/spec/features/routing/full_scenario_spec.rb @@ -1,13 +1,16 @@ require 'spec_helper' -feature 'The routing' do +feature 'The routing', js: true do let(:password) { 'a very complicated password' } let(:procedure) { create(:procedure, :with_type_de_champ, :with_service, :for_individual) } let(:administrateur) { create(:administrateur, procedures: [procedure]) } let(:scientifique_user) { create(:user, password: password) } let(:litteraire_user) { create(:user, password: password) } - before { Flipper.enable_actor(:administrateur_routage, administrateur.user) } + before do + procedure.defaut_groupe_instructeur.instructeurs << administrateur.instructeur + Flipper.enable_actor(:administrateur_routage, administrateur.user) + end scenario 'works' do login_as administrateur.user, scope: :user @@ -22,11 +25,15 @@ feature 'The routing' do # rename defaut groupe to littéraire click_on 'voir' - fill_in 'groupe_instructeur_label', with: 'littéraire' + expect(page).to have_css('#groupe_instructeur_label') + 2.times { find(:css, "#groupe_instructeur_label").set("littéraire") } click_on 'Renommer' + expect(procedure.defaut_groupe_instructeur.reload.label).to eq('littéraire') + # add victor to littéraire groupe - fill_in 'instructeur_email', with: 'victor@inst.com' + try_twice { find('input.select2-search__field').send_keys('victor@inst.com', :enter) } + perform_enqueued_jobs { click_on 'Affecter' } victor = User.find_by(email: 'victor@inst.com').instructeur @@ -37,13 +44,13 @@ feature 'The routing' do click_on 'Ajouter le groupe' # add marie to scientifique groupe - fill_in 'instructeur_email', with: 'marie@inst.com' + try_twice { find('input.select2-search__field').send_keys('marie@inst.com', :enter) } perform_enqueued_jobs { click_on 'Affecter' } marie = User.find_by(email: 'marie@inst.com').instructeur # publish publish_procedure(procedure) - log_out + log_out(old_layout: true) # 2 users fill a dossier in each group user_send_dossier(scientifique_user, 'scientifique') @@ -165,7 +172,25 @@ feature 'The routing' do expect(page).to have_content 'Mot de passe enregistré' end - def log_out - click_on 'Se déconnecter' + def log_out(old_layout: false) + if old_layout + expect(page).to have_content('Se déconnecter') + click_on 'Se déconnecter' + else + try_twice do + expect(page).to have_css('[title="Mon compte"]') + find('[title="Mon compte"]').click + expect(page).to have_content('Se déconnecter') + click_on 'Se déconnecter' + end + end + end + + def try_twice + begin + yield + rescue Selenium::WebDriver::Error::ElementNotInteractableError, Capybara::ElementNotFound + yield + end end end From 4b62c9267a7681423dcbe2f8a6598384a2b8ca62 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 18 Nov 2019 17:26:28 +0100 Subject: [PATCH 7/7] fix flaky test --- spec/models/user_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a24949fbc..54eb0dcf3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -149,7 +149,7 @@ describe User, type: :model do it 'keeps the existing instructeurs and adds administrateur' do user = subject expect(user.instructeur).to eq(instructeur) - expect(user.instructeur.administrateurs).to eq(old_admins + admins) + expect(user.instructeur.administrateurs).to match_array(old_admins + admins) end end end