From cd478b489eaa0076ae792b34dfadffa82934575c Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Tue, 15 Oct 2019 17:44:59 +0200
Subject: [PATCH 01/10] =?UTF-8?q?instructeurs:=20replace=20calls=20to=20`I?=
 =?UTF-8?q?nstructeur.find=5Fby(email:=20=E2=80=A6)`?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 app/controllers/admin/instructeurs_controller.rb       | 2 +-
 app/controllers/users/passwords_controller.rb          | 4 ++--
 app/controllers/webhook_controller.rb                  | 4 ++--
 app/models/administrateur.rb                           | 2 +-
 app/models/avis.rb                                     | 2 +-
 app/models/instructeur.rb                              | 4 ++++
 spec/controllers/admin/instructeurs_controller_spec.rb | 4 ++--
 spec/controllers/instructeurs/avis_controller_spec.rb  | 2 +-
 spec/models/administration_spec.rb                     | 2 +-
 9 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/app/controllers/admin/instructeurs_controller.rb b/app/controllers/admin/instructeurs_controller.rb
index 0cacc9bc2..abf61e00c 100644
--- a/app/controllers/admin/instructeurs_controller.rb
+++ b/app/controllers/admin/instructeurs_controller.rb
@@ -13,7 +13,7 @@ class Admin::InstructeursController < AdminController
 
   def create
     email = params[:instructeur][:email].downcase
-    @instructeur = Instructeur.find_by(email: email)
+    @instructeur = Instructeur.by_email(email)
     procedure_id = params[:procedure_id]
 
     if @instructeur.nil?
diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb
index b3f174452..97708654f 100644
--- a/app/controllers/users/passwords_controller.rb
+++ b/app/controllers/users/passwords_controller.rb
@@ -15,7 +15,7 @@ class Users::PasswordsController < Devise::PasswordsController
       @devise_mapping = Devise.mappings[:administrateur]
       params[:administrateur] = params[:user]
       # uncomment to check password complexity for Instructeur
-      # elsif Instructeur.find_by(email: email)
+      # elsif Instructeur.by_email(email)
       #   @devise_mapping = Devise.mappings[:instructeur]
       #   params[:instructeur] = params[:user]
     end
@@ -46,7 +46,7 @@ class Users::PasswordsController < Devise::PasswordsController
 
   def try_to_authenticate_instructeur
     if user_signed_in?
-      instructeur = Instructeur.find_by(email: current_user.email)
+      instructeur = Instructeur.by_email(current_user.email)
 
       if instructeur
         sign_in(instructeur.user)
diff --git a/app/controllers/webhook_controller.rb b/app/controllers/webhook_controller.rb
index 5fc66982f..970b07f27 100644
--- a/app/controllers/webhook_controller.rb
+++ b/app/controllers/webhook_controller.rb
@@ -4,8 +4,8 @@ class WebhookController < ActionController::Base
   def helpscout
     email = params[:customer][:email].downcase
     user = User.find_by(email: email)
-    instructeur = Instructeur.find_by(email: email)
-    administrateur = Administrateur.find_by(email: email)
+    instructeur = user.instructeur
+    administrateur = user.administrateur
     html = []
 
     if user
diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb
index 70bf196f0..a874fbe27 100644
--- a/app/models/administrateur.rb
+++ b/app/models/administrateur.rb
@@ -74,7 +74,7 @@ class Administrateur < ApplicationRecord
   end
 
   def instructeur
-    Instructeur.find_by(email: email)
+    user.instructeur
   end
 
   def can_be_deleted?
diff --git a/app/models/avis.rb b/app/models/avis.rb
index f30cec28b..2c1051a65 100644
--- a/app/models/avis.rb
+++ b/app/models/avis.rb
@@ -50,7 +50,7 @@ class Avis < ApplicationRecord
   private
 
   def try_to_assign_instructeur
-    instructeur = Instructeur.find_by(email: email)
+    instructeur = Instructeur.by_email(email)
     if instructeur
       self.instructeur = instructeur
       self.email = nil
diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb
index 21d00068e..4c0511849 100644
--- a/app/models/instructeur.rb
+++ b/app/models/instructeur.rb
@@ -24,6 +24,10 @@ class Instructeur < ApplicationRecord
 
   has_one :user
 
+  def self.by_email(email)
+    Instructeur.eager_load(:user).find_by(users: { email: email })
+  end
+
   def follow(dossier)
     begin
       followed_dossiers << dossier
diff --git a/spec/controllers/admin/instructeurs_controller_spec.rb b/spec/controllers/admin/instructeurs_controller_spec.rb
index 61cdf62c1..fd92a4757 100644
--- a/spec/controllers/admin/instructeurs_controller_spec.rb
+++ b/spec/controllers/admin/instructeurs_controller_spec.rb
@@ -112,7 +112,7 @@ describe Admin::InstructeursController, type: :controller do
     end
 
     context 'when an other admin will add the same email' do
-      let(:instructeur) { Instructeur.find_by(email: email) }
+      let(:instructeur) { Instructeur.by_email(email) }
 
       before do
         create :instructeur, email: email, administrateurs: [admin]
@@ -133,7 +133,7 @@ describe Admin::InstructeursController, type: :controller do
 
     context 'when an other admin will add the same email with some uppercase in it' do
       let(:email) { 'Test@Plop.com' }
-      let(:instructeur) { Instructeur.find_by(email: email.downcase) }
+      let(:instructeur) { Instructeur.by_email(email.downcase) }
 
       before do
         create :instructeur, email: email, administrateurs: [admin]
diff --git a/spec/controllers/instructeurs/avis_controller_spec.rb b/spec/controllers/instructeurs/avis_controller_spec.rb
index 86e7a5bec..5c0d60966 100644
--- a/spec/controllers/instructeurs/avis_controller_spec.rb
+++ b/spec/controllers/instructeurs/avis_controller_spec.rb
@@ -284,7 +284,7 @@ describe Instructeurs::AvisController, type: :controller do
       let!(:avis) { create(:avis, email: invited_email, dossier: dossier) }
       let(:avis_id) { avis.id }
       let(:password) { 'démarches-simplifiées-pwd' }
-      let(:created_instructeur) { Instructeur.find_by(email: invited_email) }
+      let(:created_instructeur) { Instructeur.by_email(invited_email) }
       let(:invitations_email) { true }
 
       before do
diff --git a/spec/models/administration_spec.rb b/spec/models/administration_spec.rb
index 374b60c16..6a8f1c35c 100644
--- a/spec/models/administration_spec.rb
+++ b/spec/models/administration_spec.rb
@@ -24,7 +24,7 @@ describe Administration, type: :model do
 
     it 'creates a corresponding instructeur account for the email' do
       subject
-      instructeur = Instructeur.find_by(email: valid_email)
+      instructeur = Instructeur.by_email(valid_email)
       expect(instructeur).to be_present
     end
 

From a462edb9bcefda33212c29b4097a04e551fc7c3d Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Tue, 15 Oct 2019 16:32:18 +0000
Subject: [PATCH 02/10] instructeurs: alias `instructeur.email`

This also means we need to replace instances of `pluck` on the email
column.
---
 app/controllers/instructeurs/dossiers_controller.rb           | 4 ++--
 app/mailers/groupe_instructeur_mailer.rb                      | 4 ++--
 app/models/dossier.rb                                         | 2 +-
 app/models/instructeur.rb                                     | 4 ++++
 app/serializers/dossier_serializer.rb                         | 2 +-
 .../instructeurs/groupe_instructeurs_controller_spec.rb       | 2 +-
 .../new_administrateur/groupe_instructeurs_controller_spec.rb | 2 +-
 spec/models/procedure_spec.rb                                 | 2 +-
 spec/views/admin/assigns/show.html.haml_spec.rb               | 2 +-
 spec/views/admin/instructeurs/index.html.haml_spec.rb         | 2 +-
 10 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb
index 551c4cb83..69bc65b34 100644
--- a/app/controllers/instructeurs/dossiers_controller.rb
+++ b/app/controllers/instructeurs/dossiers_controller.rb
@@ -44,9 +44,9 @@ module Instructeurs
     end
 
     def personnes_impliquees
-      @following_instructeurs_emails = dossier.followers_instructeurs.pluck(:email)
+      @following_instructeurs_emails = dossier.followers_instructeurs.map(&:email)
       previous_followers = dossier.previous_followers_instructeurs - dossier.followers_instructeurs
-      @previous_following_instructeurs_emails = previous_followers.pluck(:email)
+      @previous_following_instructeurs_emails = previous_followers.map(&:email)
       @avis_emails = dossier.avis.includes(:instructeur).map(&:email_to_display)
       @invites_emails = dossier.invites.map(&:email)
       @potential_recipients = dossier.groupe_instructeur.instructeurs.reject { |g| g == current_instructeur }
diff --git a/app/mailers/groupe_instructeur_mailer.rb b/app/mailers/groupe_instructeur_mailer.rb
index f5af9dd3c..612ac2fd5 100644
--- a/app/mailers/groupe_instructeur_mailer.rb
+++ b/app/mailers/groupe_instructeur_mailer.rb
@@ -8,7 +8,7 @@ class GroupeInstructeurMailer < ApplicationMailer
 
     subject = "Ajout d’un instructeur dans le groupe \"#{group.label}\""
 
-    emails = @group.instructeurs.pluck(:email)
+    emails = @group.instructeurs.map(&:email)
     mail(bcc: emails, subject: subject)
   end
 
@@ -19,7 +19,7 @@ class GroupeInstructeurMailer < ApplicationMailer
 
     subject = "Suppression d’un instructeur dans le groupe \"#{group.label}\""
 
-    emails = @group.instructeurs.pluck(:email)
+    emails = @group.instructeurs.map(&:email)
     mail(bcc: emails, subject: subject)
   end
 end
diff --git a/app/models/dossier.rb b/app/models/dossier.rb
index 1b5c2a4f9..d7d25ad7f 100644
--- a/app/models/dossier.rb
+++ b/app/models/dossier.rb
@@ -365,7 +365,7 @@ class Dossier < ApplicationRecord
     update(hidden_at: deleted_dossier.deleted_at)
 
     if en_construction?
-      administration_emails = followers_instructeurs.present? ? followers_instructeurs.pluck(:email) : procedure.administrateurs.pluck(:email)
+      administration_emails = followers_instructeurs.present? ? followers_instructeurs.map(&:email) : procedure.administrateurs.pluck(:email)
       administration_emails.each do |email|
         DossierMailer.notify_deletion_to_administration(deleted_dossier, email).deliver_later
       end
diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb
index 4c0511849..704818008 100644
--- a/app/models/instructeur.rb
+++ b/app/models/instructeur.rb
@@ -28,6 +28,10 @@ class Instructeur < ApplicationRecord
     Instructeur.eager_load(:user).find_by(users: { email: email })
   end
 
+  def email
+    user.email
+  end
+
   def follow(dossier)
     begin
       followed_dossiers << dossier
diff --git a/app/serializers/dossier_serializer.rb b/app/serializers/dossier_serializer.rb
index 5e32472ba..ba829d3e6 100644
--- a/app/serializers/dossier_serializer.rb
+++ b/app/serializers/dossier_serializer.rb
@@ -94,7 +94,7 @@ class DossierSerializer < ActiveModel::Serializer
   end
 
   def instructeurs
-    object.followers_instructeurs.pluck(:email)
+    object.followers_instructeurs.map(&:email)
   end
 
   def created_at
diff --git a/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb
index c10912227..c9855e498 100644
--- a/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb
+++ b/spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb
@@ -50,7 +50,7 @@ describe Instructeurs::GroupeInstructeursController, type: :controller do
     context 'of a new instructeur' do
       let(:new_instructeur_email) { 'new_instructeur@mail.com' }
 
-      it { expect(gi_1_2.instructeurs.pluck(:email)).to include(new_instructeur_email) }
+      it { expect(gi_1_2.instructeurs.map(&:email)).to include(new_instructeur_email) }
       it { expect(flash.notice).to be_present }
       it { expect(response).to redirect_to(instructeur_groupe_path(procedure, gi_1_2)) }
     end
diff --git a/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb b/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb
index d13e3d8b4..d821de26a 100644
--- a/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb
+++ b/spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb
@@ -100,7 +100,7 @@ describe NewAdministrateur::GroupeInstructeursController, type: :controller do
     context 'of a new instructeur' do
       let(:new_instructeur_email) { 'new_instructeur@mail.com' }
 
-      it { expect(gi_1_1.instructeurs.pluck(:email)).to include(new_instructeur_email) }
+      it { expect(gi_1_1.instructeurs.map(&:email)).to include(new_instructeur_email) }
       it { expect(flash.notice).to be_present }
       it { expect(response).to redirect_to(procedure_groupe_instructeur_path(procedure, gi_1_1)) }
     end
diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb
index d63ff1670..a21fac1f7 100644
--- a/spec/models/procedure_spec.rb
+++ b/spec/models/procedure_spec.rb
@@ -401,7 +401,7 @@ describe Procedure do
     subject { @procedure }
 
     it { expect(subject.parent_procedure).to eq(procedure) }
-    it { expect(subject.defaut_groupe_instructeur.instructeurs.pluck(:email)).to eq([administrateur.email]) }
+    it { expect(subject.defaut_groupe_instructeur.instructeurs.map(&:email)).to eq([administrateur.email]) }
 
     it 'should duplicate specific objects with different id' do
       expect(subject.id).not_to eq(procedure.id)
diff --git a/spec/views/admin/assigns/show.html.haml_spec.rb b/spec/views/admin/assigns/show.html.haml_spec.rb
index 6c0342922..7ce691f77 100644
--- a/spec/views/admin/assigns/show.html.haml_spec.rb
+++ b/spec/views/admin/assigns/show.html.haml_spec.rb
@@ -9,7 +9,7 @@ describe 'admin/assigns/show.html.haml', type: :view do
 
   before do
     assign(:procedure, procedure)
-    assign(:instructeur, Instructeur.new)
+    assign(:instructeur, create(:instructeur))
 
     assign(:instructeurs_assign, (smart_listing_create :instructeurs_assign,
       assign_instructeurs,
diff --git a/spec/views/admin/instructeurs/index.html.haml_spec.rb b/spec/views/admin/instructeurs/index.html.haml_spec.rb
index 60e351dbf..f40405451 100644
--- a/spec/views/admin/instructeurs/index.html.haml_spec.rb
+++ b/spec/views/admin/instructeurs/index.html.haml_spec.rb
@@ -8,7 +8,7 @@ describe 'admin/instructeurs/index.html.haml', type: :view do
       admin.instructeurs,
       partial: "admin/instructeurs/list",
       array: true))
-    assign(:instructeur, Instructeur.new())
+    assign(:instructeur, create(:instructeur))
   end
 
   context 'Aucun instructeur' do

From 8b8213c30190132769f4e361f2ad661a1fa22727 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Thu, 31 Oct 2019 11:49:22 +0100
Subject: [PATCH 03/10] instructeurs: always eager load the user relationship

Now that `Instructeur.email` is merely an alias to `instructeur.user.email`,
and we changed every occurence of `instructeurs.pluck(:email)` to
`instructeurs.map(&:email)`, the new version using `map` may cause N+1 queries
if the users have not been preloaded.

It makes sense to always preload the user when fetching an Instructeur:

- Instructeur and User have a strongly coupled relationship
- It avoids N+1 queries everywhere in the app

Of course fetching an instructeur without needing its user will now do an
unecessary fetch of the associated user. But it seems better than leaving
a risk of N+1 queries in many places.
---
 app/models/instructeur.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb
index 704818008..b54ea68b8 100644
--- a/app/models/instructeur.rb
+++ b/app/models/instructeur.rb
@@ -24,6 +24,8 @@ class Instructeur < ApplicationRecord
 
   has_one :user
 
+  default_scope { eager_load(:user) }
+
   def self.by_email(email)
     Instructeur.eager_load(:user).find_by(users: { email: email })
   end

From 8e6930d257e26c5c48bb4d46f6f2ffa4a92482a9 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 16 Oct 2019 13:02:42 +0200
Subject: [PATCH 04/10] instructeurs: fix ProcedurePresentation to use
 instructeur.user.email

The `joins` are declared explicitely in order to associate a predictable
name to the joined table.

Otherwise, when the query is joined with `:users`, ActiveRecord will
alias the join automatically  to solve the conflict. Unfortunately, the
automatic resolution means that the table name becomes unpredictable,
and thus unsuitable to perform queries on.
---
 app/models/procedure_presentation.rb       | 30 +++++++++++++++++++---
 config/brakeman.ignore                     | 30 ++++++++++++++++++----
 spec/models/procedure_presentation_spec.rb | 22 +++++++++++++++-
 3 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb
index 4c29d3a03..75872f175 100644
--- a/app/models/procedure_presentation.rb
+++ b/app/models/procedure_presentation.rb
@@ -94,7 +94,15 @@ class ProcedurePresentation < ApplicationRecord
           .where("champs.type_de_champ_id = #{column.to_i}")
           .order("champs.value #{order}")
           .pluck(:id)
-    when 'self', 'user', 'individual', 'etablissement', 'followers_instructeurs', 'groupe_instructeur'
+    when 'followers_instructeurs'
+      assert_supported_column(table, column)
+      # LEFT OUTER JOIN allows to keep dossiers without assignated instructeurs yet
+      return dossiers
+          .includes(:followers_instructeurs)
+          .joins('LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id')
+          .order("instructeurs_users.email #{order}")
+          .pluck(:id)
+    when 'self', 'user', 'individual', 'etablissement', 'groupe_instructeur'
       return (table == 'self' ? dossiers : dossiers.includes(table))
           .order("#{self.class.sanitized_column(table, column)} #{order}")
           .pluck(:id)
@@ -130,7 +138,13 @@ class ProcedurePresentation < ApplicationRecord
             .includes(table)
             .filter_ilike(table, column, values)
         end
-      when 'user', 'individual', 'followers_instructeurs'
+      when 'followers_instructeurs'
+        assert_supported_column(table, column)
+        dossiers
+          .includes(:followers_instructeurs)
+          .joins('INNER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id')
+          .filter_ilike('instructeurs_users', :email, values)
+      when 'user', 'individual'
         dossiers
           .includes(table)
           .filter_ilike(table, column, values)
@@ -243,8 +257,12 @@ class ProcedurePresentation < ApplicationRecord
   def self.sanitized_column(association, column)
     table = if association == 'self'
       Dossier.table_name
+    elsif (association_reflection = Dossier.reflect_on_association(association))
+      association_reflection.klass.table_name
     else
-      Dossier.reflect_on_association(association).klass.table_name
+      # Allow filtering on a joined table alias (which doesn’t exist
+      # in the ActiveRecord domain).
+      association
     end
 
     [table, column]
@@ -255,4 +273,10 @@ class ProcedurePresentation < ApplicationRecord
   def dossier_field_service
     @dossier_field_service ||= DossierFieldService.new
   end
+
+  def assert_supported_column(table, column)
+    if table == 'followers_instructeurs' && column != 'email'
+      raise ArgumentError, 'Table `followers_instructeurs` only supports the `email` column.'
+    end
+  end
 end
diff --git a/config/brakeman.ignore b/config/brakeman.ignore
index 79864585e..e6b718775 100644
--- a/config/brakeman.ignore
+++ b/config/brakeman.ignore
@@ -7,10 +7,10 @@
       "check_name": "CrossSiteScripting",
       "message": "Unescaped model attribute",
       "file": "app/views/users/dossiers/merci.html.haml",
-      "line": 26,
+      "line": 28,
       "link": "https://brakemanscanner.org/docs/warning_types/cross_site_scripting",
       "code": "current_user.dossiers.includes(:procedure).find(params[:id]).procedure.monavis_embed",
-      "render_path": [{"type":"controller","class":"Users::DossiersController","method":"merci","line":178,"file":"app/controllers/users/dossiers_controller.rb"}],
+      "render_path": [{"type":"controller","class":"Users::DossiersController","method":"merci","line":177,"file":"app/controllers/users/dossiers_controller.rb"}],
       "location": {
         "type": "template",
         "template": "users/dossiers/merci"
@@ -46,7 +46,7 @@
       "check_name": "SQL",
       "message": "Possible SQL injection",
       "file": "app/models/procedure_presentation.rb",
-      "line": 98,
+      "line": 106,
       "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
       "code": "((\"self\" == \"self\") ? (dossiers) : (dossiers.includes(\"self\"))).order(\"#{self.class.sanitized_column(\"self\", column)} #{order}\")",
       "render_path": null,
@@ -66,7 +66,7 @@
       "check_name": "SQL",
       "message": "Possible SQL injection",
       "file": "app/models/procedure_presentation.rb",
-      "line": 94,
+      "line": 95,
       "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
       "code": "dossiers.includes(((\"type_de_champ\" == \"type_de_champ\") ? (:champs) : (:champs_private))).where(\"champs.type_de_champ_id = #{column.to_i}\").order(\"champs.value #{order}\")",
       "render_path": null,
@@ -78,8 +78,28 @@
       "user_input": "order",
       "confidence": "Weak",
       "note": "`column` and `order` come from the model, which is validated to prevent injection attacks. Furthermore, the sql injection attack on `column` would need to survive the `to_i`"
+    },
+    {
+      "warning_type": "SQL Injection",
+      "warning_code": 0,
+      "fingerprint": "fd9d738975ccb93c8915833fceb3f43ac35410d653b8c64a1c92c1afc36d2177",
+      "check_name": "SQL",
+      "message": "Possible SQL injection",
+      "file": "app/models/procedure_presentation.rb",
+      "line": 102,
+      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
+      "code": "dossiers.includes(:followers_instructeurs).joins(\"LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id\").order(\"instructeurs_users.email #{order}\")",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "ProcedurePresentation",
+        "method": "sorted_ids"
+      },
+      "user_input": "order",
+      "confidence": "Weak",
+      "note": ""
     }
   ],
-  "updated": "2019-07-02 11:23:21 -1000",
+  "updated": "2019-10-16 16:19:43 +0200",
   "brakeman_version": "4.3.1"
 }
diff --git a/spec/models/procedure_presentation_spec.rb b/spec/models/procedure_presentation_spec.rb
index 6460e7bc5..ce6a06349 100644
--- a/spec/models/procedure_presentation_spec.rb
+++ b/spec/models/procedure_presentation_spec.rb
@@ -379,6 +379,26 @@ describe ProcedurePresentation do
       end
     end
 
+    context 'for followers_instructeurs table' do
+      let(:table) { 'followers_instructeurs' }
+      let(:order) { 'asc' } # Desc works the same, no extra test required
+
+      let!(:dossier_a) { create(:dossier, :en_construction, procedure: procedure) }
+      let!(:dossier_z) { create(:dossier, :en_construction, procedure: procedure) }
+      let!(:dossier_without_instructeur) { create(:dossier, :en_construction, procedure: procedure) }
+
+      before do
+        create(:follow, dossier: dossier_a, instructeur: create(:instructeur, email: 'abaca@exemple.fr'))
+        create(:follow, dossier: dossier_z, instructeur: create(:instructeur, email: 'zythum@exemple.fr'))
+      end
+
+      context 'for email column' do
+        let(:column) { 'email' }
+
+        it { is_expected.to eq([dossier_a, dossier_z, dossier_without_instructeur].map(&:id)) }
+      end
+    end
+
     context 'for other tables' do
       # All other columns and tables work the same so it’s ok to test only one
       let(:table) { 'etablissement' }
@@ -395,7 +415,7 @@ describe ProcedurePresentation do
   describe '#filtered_ids' do
     let(:procedure_presentation) { create(:procedure_presentation, assign_to: create(:assign_to, procedure: procedure), filters: { "suivis" => filter }) }
 
-    subject { procedure_presentation.filtered_ids(procedure.dossiers, 'suivis') }
+    subject { procedure_presentation.filtered_ids(procedure.dossiers.joins(:user), 'suivis') }
 
     context 'for self table' do
       context 'for created_at column' do

From 38f994a1516d2fff98ff0e5035a27604a9e831c4 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 16 Oct 2019 16:39:19 +0200
Subject: [PATCH 05/10] instructeurs: fix mailer previews not to use
 Instructeur.email

Mailers previews now create mock Instructeurs without directly accessing
the email.
---
 spec/mailers/previews/avis_mailer_preview.rb        |  2 +-
 spec/mailers/previews/instructeur_mailer_preview.rb | 10 ++++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/spec/mailers/previews/avis_mailer_preview.rb b/spec/mailers/previews/avis_mailer_preview.rb
index 0c61cef52..f9abd2c85 100644
--- a/spec/mailers/previews/avis_mailer_preview.rb
+++ b/spec/mailers/previews/avis_mailer_preview.rb
@@ -1,7 +1,7 @@
 # Preview all emails at http://localhost:3000/rails/mailers/avis_mailer
 class AvisMailerPreview < ActionMailer::Preview
   def avis_invitation
-    gestionaire = Instructeur.new(id: 1, email: 'jeanmichel.de-chauvigny@exemple.fr')
+    gestionaire = Instructeur.new(id: 1, user: User.new(email: 'jeanmichel.de-chauvigny@exemple.fr'))
     avis = Avis.new(id: 1, email: 'test@exemple.fr', claimant: gestionaire)
     avis.dossier = Dossier.new(id: 1)
     avis.dossier.procedure = Procedure.new(libelle: 'Démarche pour faire des marches')
diff --git a/spec/mailers/previews/instructeur_mailer_preview.rb b/spec/mailers/previews/instructeur_mailer_preview.rb
index 0932eab6a..0a906a7dc 100644
--- a/spec/mailers/previews/instructeur_mailer_preview.rb
+++ b/spec/mailers/previews/instructeur_mailer_preview.rb
@@ -44,11 +44,17 @@ class InstructeurMailerPreview < ActionMailer::Preview
   private
 
   def instructeur
-    Instructeur.new(id: 10, email: 'instructeur@administration.gouv.fr')
+    Instructeur.new(
+      id: 10,
+      user: User.new(email: 'instructeur@administration.gouv.fr')
+    )
   end
 
   def target_instructeur
-    Instructeur.new(id: 12, email: 'collegue@administration.gouv.fr')
+    Instructeur.new(
+      id: 12,
+      user: User.new(email: 'collegue@administration.gouv.fr')
+    )
   end
 
   def procedure

From d78d20654441aeb400f2ba3115bcdfad0a484369 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Mon, 21 Oct 2019 09:24:43 +0000
Subject: [PATCH 06/10] instructeurs: fix expert creation not to use
 Instructeur.email

It kind of worked until now, because the email field is disabled, and
thus never accessed.

But better make it clean, by accessing an object (User) where the email
field actually exists.
---
 app/controllers/instructeurs/avis_controller.rb       | 2 +-
 app/views/instructeurs/avis/sign_up.html.haml         | 2 +-
 spec/controllers/instructeurs/avis_controller_spec.rb | 2 +-
 spec/features/instructeurs/instruction_spec.rb        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/controllers/instructeurs/avis_controller.rb b/app/controllers/instructeurs/avis_controller.rb
index 2cff98e59..5e2e0c8a8 100644
--- a/app/controllers/instructeurs/avis_controller.rb
+++ b/app/controllers/instructeurs/avis_controller.rb
@@ -81,7 +81,7 @@ module Instructeurs
 
     def create_instructeur
       email = params[:email]
-      password = params['instructeur']['password']
+      password = params[:user][:password]
 
       # Not perfect because the password will not be changed if the user already exists
       user = User.create_or_promote_to_instructeur(email, password)
diff --git a/app/views/instructeurs/avis/sign_up.html.haml b/app/views/instructeurs/avis/sign_up.html.haml
index 29049546e..44e0ab237 100644
--- a/app/views/instructeurs/avis/sign_up.html.haml
+++ b/app/views/instructeurs/avis/sign_up.html.haml
@@ -4,7 +4,7 @@
       %p.description= @dossier.procedure.libelle
       %p.dossier Dossier nº #{@dossier.id}
     .column
-      = form_for(Instructeur.new, url: { controller: "instructeurs/avis", action: :create_instructeur }, method: :post, html: { class: "form" }) do |f|
+      = form_for(User.new, url: { controller: "instructeurs/avis", action: :create_instructeur }, method: :post, html: { class: "form" }) do |f|
         %h1 Créez-vous un compte
 
         = f.label :email, "Email"
diff --git a/spec/controllers/instructeurs/avis_controller_spec.rb b/spec/controllers/instructeurs/avis_controller_spec.rb
index 5c0d60966..1c7c9d007 100644
--- a/spec/controllers/instructeurs/avis_controller_spec.rb
+++ b/spec/controllers/instructeurs/avis_controller_spec.rb
@@ -296,7 +296,7 @@ describe Instructeurs::AvisController, type: :controller do
         post :create_instructeur, params: {
           id: avis_id,
           email: invited_email,
-          instructeur: {
+          user: {
             password: password
           }
         }
diff --git a/spec/features/instructeurs/instruction_spec.rb b/spec/features/instructeurs/instruction_spec.rb
index 52140f952..884b73631 100644
--- a/spec/features/instructeurs/instruction_spec.rb
+++ b/spec/features/instructeurs/instruction_spec.rb
@@ -181,7 +181,7 @@ feature 'Instructing a dossier:' do
 
   def avis_sign_up(avis, email)
     visit sign_up_instructeur_avis_path(avis, email)
-    fill_in 'instructeur_password', with: 'démarches-simplifiées-pwd'
+    fill_in 'user_password', with: 'démarches-simplifiées-pwd'
     click_on 'Créer un compte'
     expect(page).to have_current_path(instructeur_avis_index_path)
   end

From f131dbb80ddb3ad635dd36dc50a2ebc38d9d6e16 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Thu, 17 Oct 2019 14:07:12 +0000
Subject: [PATCH 07/10] instructeurs: make the create form not using email
 directly
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before the form attempted to read an email value from the Instructeur
model, and failed (because the empty Instructeur had no user yet).

We could let `Instructeur#email` return `nil` if there is no User –
but as a created Instructeur is always supposed to have a User, this
seems like a nice safeguard to keep.

So instead this commit rewrites the create form, which now doesn’t
depend on an Instructeur model. Seems easy enough for now.
---
 app/controllers/admin/assigns_controller.rb             | 2 --
 app/controllers/admin/instructeurs_controller.rb        | 2 --
 app/views/admin/assigns/show.html.haml                  | 6 +++---
 app/views/admin/instructeurs/_informations.html.haml    | 9 ++++-----
 app/views/admin/instructeurs/index.html.haml            | 6 +++---
 spec/features/instructeurs/instructeur_creation_spec.rb | 2 +-
 6 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/app/controllers/admin/assigns_controller.rb b/app/controllers/admin/assigns_controller.rb
index fca3aabc6..5a59330c4 100644
--- a/app/controllers/admin/assigns_controller.rb
+++ b/app/controllers/admin/assigns_controller.rb
@@ -25,8 +25,6 @@ class Admin::AssignsController < AdminController
       not_assign_scope,
       partial: "admin/assigns/list_not_assign",
       array: true
-
-    @instructeur ||= Instructeur.new
   end
 
   def update
diff --git a/app/controllers/admin/instructeurs_controller.rb b/app/controllers/admin/instructeurs_controller.rb
index abf61e00c..fd35b15b5 100644
--- a/app/controllers/admin/instructeurs_controller.rb
+++ b/app/controllers/admin/instructeurs_controller.rb
@@ -7,8 +7,6 @@ class Admin::InstructeursController < AdminController
       current_administrateur.instructeurs,
       partial: "admin/instructeurs/list",
       array: true
-
-    @instructeur ||= Instructeur.new
   end
 
   def create
diff --git a/app/views/admin/assigns/show.html.haml b/app/views/admin/assigns/show.html.haml
index 6fba3a9ca..dba32fd32 100644
--- a/app/views/admin/assigns/show.html.haml
+++ b/app/views/admin/assigns/show.html.haml
@@ -9,15 +9,15 @@
         %h3
           = t('dynamics.admin.procedure.onglet_instructeurs.add.title')
         #procedure_new.section.section-label
-          = form_for @instructeur, url: { controller: 'admin/instructeurs', action: :create } do |f|
+          = form_with url: { controller: 'admin/instructeurs', action: :create } do
             .row
               .col-xs-5
                 = hidden_field_tag :procedure_id, params[:procedure_id]
-                = render partial: 'admin/instructeurs/informations', locals: { f: f }
+                = render partial: 'admin/instructeurs/informations'
               .col-xs-2
                 %br
                 %br
-                = f.submit 'Valider', class: 'btn btn-info', style: 'float: left;', id: 'add-instructeur-email'
+                = submit_tag 'Ajouter', class: 'btn btn-info', style: 'float: left;', id: 'add-instructeur-email'
       .col-xs-6
         %h3.text-success Affectés
         = smart_listing_render :instructeurs_assign
diff --git a/app/views/admin/instructeurs/_informations.html.haml b/app/views/admin/instructeurs/_informations.html.haml
index 29b25928d..6a50cb3ab 100644
--- a/app/views/admin/instructeurs/_informations.html.haml
+++ b/app/views/admin/instructeurs/_informations.html.haml
@@ -1,5 +1,4 @@
-- { email: 'Email*' }.each do |key, value|
-  .form-group
-    %h4
-      = value
-    = f.text_field key, class: 'form-control', placeholder: value
+.form-group
+  %h4
+    = 'Email *'
+  = text_field_tag 'instructeur[email]', nil, class: 'form-control', placeholder: 'laura.azema@exemple.gouv.fr'
diff --git a/app/views/admin/instructeurs/index.html.haml b/app/views/admin/instructeurs/index.html.haml
index 09773adba..abb6ddbc1 100644
--- a/app/views/admin/instructeurs/index.html.haml
+++ b/app/views/admin/instructeurs/index.html.haml
@@ -18,11 +18,11 @@
   .col-xs-6
     %h3 Ajouter un instructeur
     #procedure_new.section.section-label
-      = form_for @instructeur, url: { controller: 'admin/instructeurs', action: :create } do |f|
+      = form_with url: { controller: 'admin/instructeurs', action: :create } do
         .row
           .col-xs-5
-            = render partial: 'informations', locals: { f: f }
+            = render partial: 'admin/instructeurs/informations'
           .col-xs-2
             %br
             %br
-            = f.submit 'Valider', class: 'btn btn-info', style: 'float: left;'
+            = submit_tag 'Ajouter', class: 'btn btn-info', style: 'float: left;'
diff --git a/spec/features/instructeurs/instructeur_creation_spec.rb b/spec/features/instructeurs/instructeur_creation_spec.rb
index 50d284e96..189144cb6 100644
--- a/spec/features/instructeurs/instructeur_creation_spec.rb
+++ b/spec/features/instructeurs/instructeur_creation_spec.rb
@@ -12,7 +12,7 @@ feature 'As an instructeur', js: true do
     fill_in :instructeur_email, with: instructeur_email
 
     perform_enqueued_jobs do
-      click_button 'Valider'
+      click_button 'Ajouter'
     end
   end
 

From f8358b3ae9836be8084f4fa5b8ba585dba998062 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 16 Oct 2019 12:15:47 +0000
Subject: [PATCH 08/10] instructeurs: create without providing the email

---
 app/models/instructeur.rb                           |  3 ---
 app/models/user.rb                                  |  2 +-
 ...emove_unique_constraint_on_instructeur_emails.rb | 13 +++++++++++++
 db/schema.rb                                        |  4 ++--
 db/seeds.rb                                         | 10 ++++------
 spec/factories/instructeur.rb                       |  3 +--
 spec/models/user_spec.rb                            |  2 +-
 7 files changed, 22 insertions(+), 15 deletions(-)
 create mode 100644 db/migrate/20191024150452_remove_unique_constraint_on_instructeur_emails.rb

diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb
index b54ea68b8..4f9a871ba 100644
--- a/app/models/instructeur.rb
+++ b/app/models/instructeur.rb
@@ -1,11 +1,8 @@
 class Instructeur < ApplicationRecord
   self.ignored_columns = ['features', 'encrypted_password', 'reset_password_token', 'reset_password_sent_at', 'remember_created_at', 'sign_in_count', 'current_sign_in_at', 'last_sign_in_at', 'current_sign_in_ip', 'last_sign_in_ip', 'failed_attempts', 'unlock_token', 'locked_at']
-  include EmailSanitizableConcern
 
   has_and_belongs_to_many :administrateurs
 
-  before_validation -> { sanitize_email(:email) }
-
   has_many :assign_to, dependent: :destroy
   has_many :groupe_instructeurs, through: :assign_to
   has_many :procedures, -> { distinct }, through: :groupe_instructeurs
diff --git a/app/models/user.rb b/app/models/user.rb
index c49cd2718..a94d9212d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -70,7 +70,7 @@ class User < ApplicationRecord
 
     if user.valid?
       if user.instructeur_id.nil?
-        user.create_instructeur!(email: email)
+        user.create_instructeur!
       end
 
       user.instructeur.administrateurs << administrateurs
diff --git a/db/migrate/20191024150452_remove_unique_constraint_on_instructeur_emails.rb b/db/migrate/20191024150452_remove_unique_constraint_on_instructeur_emails.rb
new file mode 100644
index 000000000..c0bbdf1d7
--- /dev/null
+++ b/db/migrate/20191024150452_remove_unique_constraint_on_instructeur_emails.rb
@@ -0,0 +1,13 @@
+class RemoveUniqueConstraintOnInstructeurEmails < ActiveRecord::Migration[5.2]
+  def up
+    # Drop the index entirely
+    remove_index :instructeurs, :email
+    # Add the index again, without the unicity constraint
+    add_index :instructeurs, :email
+  end
+
+  def down
+    remove_index :instructeurs, :email
+    add_index :instructeurs, :email, unique: true
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index b4c4e4823..2fb2e05ac 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_10_23_183120) do
+ActiveRecord::Schema.define(version: 2019_10_24_150452) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -419,7 +419,7 @@ ActiveRecord::Schema.define(version: 2019_10_23_183120) do
     t.datetime "updated_at"
     t.text "encrypted_login_token"
     t.datetime "login_token_created_at"
-    t.index ["email"], name: "index_instructeurs_on_email", unique: true
+    t.index ["email"], name: "index_instructeurs_on_email"
   end
 
   create_table "invites", id: :serial, force: :cascade do |t|
diff --git a/db/seeds.rb b/db/seeds.rb
index 9598aebcf..6849cda9a 100644
--- a/db/seeds.rb
+++ b/db/seeds.rb
@@ -10,12 +10,10 @@ default_password = "this is a very complicated password !"
 
 puts "Create test user '#{default_user}'"
 Administration.create!(email: default_user, password: default_password)
-administrateur = Administrateur.create!(email: default_user)
-instructeur = Instructeur.create!(email: default_user)
-User.create!(
+user = User.create!(
   email: default_user,
   password: default_password,
-  confirmed_at: Time.zone.now,
-  administrateur: administrateur,
-  instructeur: instructeur
+  confirmed_at: Time.zone.now
 )
+user.create_instructeur!
+user.create_administrateur!(email: user.email)
diff --git a/spec/factories/instructeur.rb b/spec/factories/instructeur.rb
index eb51ed6fa..7b73a5f9c 100644
--- a/spec/factories/instructeur.rb
+++ b/spec/factories/instructeur.rb
@@ -2,9 +2,8 @@ FactoryBot.define do
   sequence(:instructeur_email) { |n| "inst#{n}@inst.com" }
 
   factory :instructeur do
-    email { generate(:instructeur_email) }
-
     transient do
+      email { generate(:instructeur_email) }
       password { 'somethingverycomplated!' }
     end
 
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 1f9f8eff6..430e0b1d4 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -138,7 +138,7 @@ describe User, type: :model do
       context 'with an existing instructeur' do
         let(:old_admins) { [create(:administrateur)] }
         let(:admins) { [create(:administrateur)] }
-        let!(:instructeur) { Instructeur.create(email: 'i@mail.com', administrateurs: old_admins) }
+        let!(:instructeur) { create(:instructeur, email: 'i@mail.com', administrateurs: old_admins) }
 
         before do
           User

From 2e8d365b93df04de4dad95bf70c8d16f87b4403d Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 16 Oct 2019 14:17:34 +0200
Subject: [PATCH 09/10] instructeurs: disable the email column

---
 app/models/instructeur.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb
index 4f9a871ba..793e45606 100644
--- a/app/models/instructeur.rb
+++ b/app/models/instructeur.rb
@@ -1,5 +1,5 @@
 class Instructeur < ApplicationRecord
-  self.ignored_columns = ['features', 'encrypted_password', 'reset_password_token', 'reset_password_sent_at', 'remember_created_at', 'sign_in_count', 'current_sign_in_at', 'last_sign_in_at', 'current_sign_in_ip', 'last_sign_in_ip', 'failed_attempts', 'unlock_token', 'locked_at']
+  self.ignored_columns = ['email', 'features', 'encrypted_password', 'reset_password_token', 'reset_password_sent_at', 'remember_created_at', 'sign_in_count', 'current_sign_in_at', 'last_sign_in_at', 'current_sign_in_ip', 'last_sign_in_ip', 'failed_attempts', 'unlock_token', 'locked_at']
 
   has_and_belongs_to_many :administrateurs
 

From a458693a19abba62fe4a8fb4d3550209918528d2 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Mon, 4 Nov 2019 10:56:15 +0100
Subject: [PATCH 10/10] specs: fix #usual_traitement_time tests when near the
 DST date

---
 spec/models/procedure_spec.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb
index a21fac1f7..0117b43df 100644
--- a/spec/models/procedure_spec.rb
+++ b/spec/models/procedure_spec.rb
@@ -814,11 +814,15 @@ describe Procedure do
     end
 
     before do
+      Timecop.freeze(Time.utc(2019, 6, 1, 12, 0))
+
       delays.each do |delay|
         create_dossier(construction_date: 1.week.ago - delay, instruction_date: 1.week.ago - delay + 12.hours, processed_date: 1.week.ago)
       end
     end
 
+    after { Timecop.return }
+
     context 'when there are several processed dossiers' do
       let(:delays) { [1.day, 2.days, 2.days, 2.days, 2.days, 3.days, 3.days, 3.days, 3.days, 12.days] }