Merge pull request #4527 from betagouv/better_add_instructeur_ux

Ajoute un champ autocomplété pour l'ajout des gestionnaires

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;
+      }
+    }
+  }
+}
+
+

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
 

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

app/javascript/new_design/select2.js

@@ -1,6 +1,13 @@
 import $ from 'jquery';
 import 'select2';
 
+const optionTemplate = email =>
+  $(
+    '<span class="custom-select2-option"><span class="icon person"></span>' +
+      email.text +
+      '</span>'
+  );
+
 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 $(
+        '<span class="custom-select2-option"><span class="icon person"></span>' +
+          email.text +
+          '</span>'
+      );
+    }
+  });
 });

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}\""

app/views/groupe_instructeur_mailer/add_instructeur.html.haml

@@ -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"

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"

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,13 +22,13 @@
       = 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
       %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

app/views/new_administrateur/groupe_instructeurs/show.html.haml

@@ -17,24 +17,29 @@
       = 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|
 
-      = 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
       %thead
         %tr
-          %th{ colspan: 2 } Instructeurs affectés
+          %th{ colspan: 2 }= t('.assigned_instructeur', count: @instructeurs.count)
       %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,

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

@@ -0,0 +1,18 @@
+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"
+          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} »."

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)) }

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

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

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