From 068471003d35cad2d828d4b39e50c8e30112226f Mon Sep 17 00:00:00 2001
From: Paul Chavard <git@paul.chavard.net>
Date: Thu, 30 Jan 2020 13:55:03 +0100
Subject: [PATCH 01/21] Add a timeout to select2 queries

---
 app/javascript/new_design/select2.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/javascript/new_design/select2.js b/app/javascript/new_design/select2.js
index ca55d1dd9..1b3cf05a5 100644
--- a/app/javascript/new_design/select2.js
+++ b/app/javascript/new_design/select2.js
@@ -52,6 +52,7 @@ const baseOptions = {
 
 const baseAjaxOptions = {
   delay: 250,
+  timeout: 10 * 1000, // 10 sec
   cache: true,
   data({ term: nom }) {
     return {

From 57a0973c9978aa6f46fea98fbad01d83c00dbe5b Mon Sep 17 00:00:00 2001
From: clemkeirua <clement@keiruaprod.fr>
Date: Thu, 30 Jan 2020 17:12:36 +0100
Subject: [PATCH 02/21] added a test to show #4658 does not exist

---
 .../concern/tags_substitution_concern_spec.rb | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/spec/models/concern/tags_substitution_concern_spec.rb b/spec/models/concern/tags_substitution_concern_spec.rb
index f80832d3f..288c0075a 100644
--- a/spec/models/concern/tags_substitution_concern_spec.rb
+++ b/spec/models/concern/tags_substitution_concern_spec.rb
@@ -114,6 +114,29 @@ describe TagsSubstitutionConcern, type: :model do
       end
     end
 
+    context 'when the procedure has a type de champ with apostrophes' do
+      let(:types_de_champ) do
+        [
+          create(:type_de_champ, libelle: "Intitulé de l'‘«\"évènement\"»’")
+        ]
+      end
+
+      context 'and they are used in the template' do
+        let(:template) { "--Intitulé de l'‘«\"évènement\"»’--" }
+
+        context 'and their value in the dossier are not nil' do
+          before do
+            dossier.champs
+              .filter { |champ| champ.libelle == "Intitulé de l'‘«\"évènement\"»’" }
+              .first
+              .update(value: 'ceci est mon évènement')
+          end
+
+          it { is_expected.to eq('ceci est mon évènement') }
+        end
+      end
+    end
+
     context 'when the procedure has a type de champ repetition' do
       let(:template) { '--Répétition--' }
       let(:types_de_champ) do

From 5afe158c89d9bb39937e4ea869223829f9384e9c Mon Sep 17 00:00:00 2001
From: Paul Chavard <git@paul.chavard.net>
Date: Thu, 30 Jan 2020 11:24:16 +0100
Subject: [PATCH 03/21] Tweak a bit timeout values for export polling

---
 app/javascript/shared/remote-poller.js                   | 4 ++--
 app/views/instructeurs/procedures/download_export.js.erb | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/javascript/shared/remote-poller.js b/app/javascript/shared/remote-poller.js
index a04607212..6aca55745 100644
--- a/app/javascript/shared/remote-poller.js
+++ b/app/javascript/shared/remote-poller.js
@@ -86,5 +86,5 @@ class RemotePoller {
   }
 }
 
-const attachementPoller = new RemotePoller({ interval: 2000, maxChecks: 5 });
-const exportPoller = new RemotePoller({ interval: 4000, maxChecks: 10 });
+const attachementPoller = new RemotePoller({ interval: 3000, maxChecks: 5 });
+const exportPoller = new RemotePoller({ interval: 6000, maxChecks: 10 });
diff --git a/app/views/instructeurs/procedures/download_export.js.erb b/app/views/instructeurs/procedures/download_export.js.erb
index fe8b67bb9..d25101ffc 100644
--- a/app/views/instructeurs/procedures/download_export.js.erb
+++ b/app/views/instructeurs/procedures/download_export.js.erb
@@ -7,4 +7,4 @@
   <% end %>
 <% end %>
 
-<%= render_flash(timeout: 7000) %>
+<%= render_flash(timeout: 20000) %>

From cee4c5b8fb4ba73bd139d5d969957f45dd8141fe Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Thu, 30 Jan 2020 10:48:28 +0100
Subject: [PATCH 04/21] Revert "Revert "4127 fix superadmin supprime compte
 usager""

This reverts commit 751f24f7bb79c7d9d0af4ef1b7afe7aeabb4cdbc.
---
 .../manager/administrateurs_controller.rb     |  6 +--
 .../manager/instructeurs_controller.rb        | 14 ++++++
 app/controllers/manager/users_controller.rb   |  2 +-
 app/models/administrateur.rb                  | 13 ++++++
 app/models/instructeur.rb                     |  8 +++-
 app/models/user.rb                            | 11 +++--
 app/views/manager/instructeurs/show.html.erb  |  3 ++
 app/views/manager/users/show.html.erb         |  2 +-
 config/routes.rb                              |  1 +
 .../manager/instructeurs_controller_spec.rb   | 17 +++++++
 spec/models/administrateur_spec.rb            | 15 +++++++
 spec/models/instructeur_spec.rb               | 35 ++++++++++++++-
 spec/models/user_spec.rb                      | 45 ++++++++++++++++---
 13 files changed, 150 insertions(+), 22 deletions(-)
 create mode 100644 spec/controllers/manager/instructeurs_controller_spec.rb

diff --git a/app/controllers/manager/administrateurs_controller.rb b/app/controllers/manager/administrateurs_controller.rb
index f976f3cf6..10dec19d5 100644
--- a/app/controllers/manager/administrateurs_controller.rb
+++ b/app/controllers/manager/administrateurs_controller.rb
@@ -22,11 +22,7 @@ module Manager
     def delete
       administrateur = Administrateur.find(params[:id])
 
-      if !administrateur.can_be_deleted?
-        fail "Impossible de supprimer cet administrateur car il a des dossiers ou des procédures"
-      end
-      administrateur.dossiers.each(&:delete_and_keep_track)
-      administrateur.destroy
+      administrateur.delete_and_transfer_services
 
       logger.info("L'administrateur #{administrateur.id} est supprimé par #{current_administration.id}")
       flash[:notice] = "L'administrateur #{administrateur.id} est supprimé"
diff --git a/app/controllers/manager/instructeurs_controller.rb b/app/controllers/manager/instructeurs_controller.rb
index 82b1e11eb..ed2107c45 100644
--- a/app/controllers/manager/instructeurs_controller.rb
+++ b/app/controllers/manager/instructeurs_controller.rb
@@ -6,5 +6,19 @@ module Manager
       flash[:notice] = "Instructeur réinvité."
       redirect_to manager_instructeur_path(instructeur)
     end
+
+    def delete
+      instructeur = Instructeur.find(params[:id])
+
+      if !instructeur.can_be_deleted?
+        fail "Impossible de supprimer cet instructeur car il est administrateur ou il est le seul instructeur sur une démarche"
+      end
+      instructeur.destroy!
+
+      logger.info("L'instructeur #{instructeur.id} est supprimé par #{current_administration.id}")
+      flash[:notice] = "L'instructeur #{instructeur.id} est supprimé"
+
+      redirect_to manager_instructeurs_path
+    end
   end
 end
diff --git a/app/controllers/manager/users_controller.rb b/app/controllers/manager/users_controller.rb
index 9bdbb0449..d9e33fcb9 100644
--- a/app/controllers/manager/users_controller.rb
+++ b/app/controllers/manager/users_controller.rb
@@ -37,7 +37,7 @@ module Manager
     def delete
       user = User.find(params[:id])
       if !user.can_be_deleted?
-        fail "Impossible de supprimer cet utilisateur car il a des dossiers en instruction"
+        fail "Impossible de supprimer cet utilisateur. Il a des dossiers en instruction ou il est administrateur."
       end
       user.delete_and_keep_track_dossiers(current_administration)
 
diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb
index 4d4b84d4a..1174d4b60 100644
--- a/app/models/administrateur.rb
+++ b/app/models/administrateur.rb
@@ -70,4 +70,17 @@ class Administrateur < ApplicationRecord
   def can_be_deleted?
     dossiers.state_instruction_commencee.none? && procedures.all? { |p| p.administrateurs.count > 1 }
   end
+
+  def delete_and_transfer_services
+    if !can_be_deleted?
+      fail "Impossible de supprimer cet administrateur car il a des dossiers ou des procédures"
+    end
+    dossiers.each(&:delete_and_keep_track)
+
+    procedures.each do |procedure|
+      next_administrateur = procedure.administrateurs.where.not(id: self.id).first
+      procedure.service.update(administrateur: next_administrateur)
+    end
+    destroy
+  end
 end
diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb
index 4c142b870..97b2b0a49 100644
--- a/app/models/instructeur.rb
+++ b/app/models/instructeur.rb
@@ -17,9 +17,9 @@ class Instructeur < ApplicationRecord
   has_many :previously_followed_dossiers, -> { distinct }, through: :previous_follows, source: :dossier
   has_many :avis
   has_many :dossiers_from_avis, through: :avis, source: :dossier
-  has_many :trusted_device_tokens
+  has_many :trusted_device_tokens, dependent: :destroy
 
-  has_one :user
+  has_one :user, dependent: :nullify
 
   default_scope { eager_load(:user) }
 
@@ -176,6 +176,10 @@ class Instructeur < ApplicationRecord
     trusted_device_token&.token_young?
   end
 
+  def can_be_deleted?
+    user.administrateur.nil? && procedures.all? { |p| p.defaut_groupe_instructeur.instructeurs.count > 1 }
+  end
+
   private
 
   def annotations_hash(demande, annotations_privees, avis, messagerie)
diff --git a/app/models/user.rb b/app/models/user.rb
index 4838da20d..35c0edd5e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -97,7 +97,7 @@ class User < ApplicationRecord
   end
 
   def can_be_deleted?
-    dossiers.state_instruction_commencee.empty?
+    administrateur.nil? && instructeur.nil? && dossiers.state_instruction_commencee.empty?
   end
 
   def delete_and_keep_track_dossiers(administration)
@@ -105,12 +105,11 @@ class User < ApplicationRecord
       raise "Cannot delete this user because instruction has started for some dossiers"
     end
 
-    if can_be_deleted?
-      dossiers.each do |dossier|
-        dossier.delete_and_keep_track(administration)
-      end
-      destroy
+    dossiers.each do |dossier|
+      dossier.delete_and_keep_track(administration)
     end
+    dossiers.unscoped.destroy_all
+    destroy!
   end
 
   private
diff --git a/app/views/manager/instructeurs/show.html.erb b/app/views/manager/instructeurs/show.html.erb
index e423414a9..983e171e9 100644
--- a/app/views/manager/instructeurs/show.html.erb
+++ b/app/views/manager/instructeurs/show.html.erb
@@ -34,7 +34,10 @@ as well as a link to its edit page.
 
   <div>
       <%= link_to 'Réinviter', reinvite_manager_instructeur_path(instructeur), method: :post, class: 'button' %>
+  </div>
   <div>
+    <%= button_to "Supprimer", delete_manager_instructeur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'instructeur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet instructeur est administrateur ou a des démarches dont il est le seul instructeur et ne peut être supprimé" %>
+  </div>
 </header>
 
 <section class="main-content__body">
diff --git a/app/views/manager/users/show.html.erb b/app/views/manager/users/show.html.erb
index bf49d4076..a14acc228 100644
--- a/app/views/manager/users/show.html.erb
+++ b/app/views/manager/users/show.html.erb
@@ -28,7 +28,7 @@ as well as a link to its edit page.
     <%= button_to "modifier", edit_manager_user_path(page.resource), method: :get, class: "button" %>
   </div>
   <div>
-    <%= button_to "supprimer", delete_manager_user_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'utilisateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet utilisateur a des dossiers dont l'instruction a commencé et ne peut être supprimé" %>
+    <%= button_to "supprimer", delete_manager_user_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'utilisateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet utilisateur ne peut être supprimé. Il a des dossiers dont l'instruction a commencé ou il est administrateur ou instructeur" %>
   </div>
   <div>
     <% if !user.confirmed? %>
diff --git a/config/routes.rb b/config/routes.rb
index 7feeaadb4..30d4a3402 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -32,6 +32,7 @@ Rails.application.routes.draw do
 
     resources :instructeurs, only: [:index, :show] do
       post 'reinvite', on: :member
+      delete 'delete', on: :member
     end
 
     resources :dossiers, only: [:show]
diff --git a/spec/controllers/manager/instructeurs_controller_spec.rb b/spec/controllers/manager/instructeurs_controller_spec.rb
new file mode 100644
index 000000000..3a0164d4b
--- /dev/null
+++ b/spec/controllers/manager/instructeurs_controller_spec.rb
@@ -0,0 +1,17 @@
+describe Manager::InstructeursController, type: :controller do
+  let(:administration) { create(:administration) }
+
+  describe '#delete' do
+    let!(:instructeur) { create(:instructeur) }
+
+    before { sign_in administration }
+
+    subject { delete :delete, params: { id: instructeur.id } }
+
+    it 'deletes the instructeur' do
+      subject
+
+      expect(Instructeur.find_by(id: instructeur.id)).to be_nil
+    end
+  end
+end
diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb
index 9f0c30923..3e6864466 100644
--- a/spec/models/administrateur_spec.rb
+++ b/spec/models/administrateur_spec.rb
@@ -46,6 +46,21 @@ describe Administrateur, type: :model do
     end
   end
 
+  describe '#delete_and_transfer_services' do
+    let!(:administrateur) { create(:administrateur) }
+    let!(:autre_administrateur) { create(:administrateur) }
+    let!(:procedure) { create(:procedure, :with_service, administrateurs: [administrateur, autre_administrateur]) }
+    let(:service) { procedure.service }
+
+    it "delete and transfer services to other admin" do
+      service.update(administrateur: administrateur)
+      administrateur.delete_and_transfer_services
+
+      expect(Administrateur.find_by(id: administrateur.id)).to be_nil
+      expect(service.reload.administrateur).to eq(autre_administrateur)
+    end
+  end
+
   # describe '#password_complexity' do
   #   let(:email) { 'mail@beta.gouv.fr' }
   #   let(:passwords) { ['pass', '12pass23', 'démarches ', 'démarches-simple', 'démarches-simplifiées-pwd'] }
diff --git a/spec/models/instructeur_spec.rb b/spec/models/instructeur_spec.rb
index 37917bf18..78737bf89 100644
--- a/spec/models/instructeur_spec.rb
+++ b/spec/models/instructeur_spec.rb
@@ -424,9 +424,40 @@ describe Instructeur, type: :model do
     it { expect(instructeur_a.procedures.all.to_ary).to eq([procedure_a]) }
   end
 
+  describe "#can_be_deleted?" do
+    subject { instructeur.can_be_deleted? }
+
+    context 'when the instructeur is an administrateur' do
+      let!(:administrateur) { create(:administrateur) }
+      let(:instructeur) { administrateur.instructeur }
+
+      it { is_expected.to be false }
+    end
+
+    context "when the instructeur's procedures have other instructeurs" do
+      let(:instructeur_not_admin) { create(:instructeur) }
+      let(:autre_instructeur) { create(:instructeur) }
+
+      it "can be deleted" do
+        assign(procedure, instructeur_assigne: instructeur_not_admin)
+        assign(procedure, instructeur_assigne: autre_instructeur)
+        expect(autre_instructeur.can_be_deleted?).to be_truthy
+      end
+    end
+
+    context "when the instructeur's procedures is the only one" do
+      let(:instructeur_not_admin) { create :instructeur }
+      let(:autre_procedure) { create :procedure }
+      it "can be deleted" do
+        assign(autre_procedure, instructeur_assigne: instructeur_not_admin)
+        expect(instructeur_not_admin.can_be_deleted?).to be_falsy
+      end
+    end
+  end
+
   private
 
-  def assign(procedure_to_assign)
-    create :assign_to, instructeur: instructeur, procedure: procedure_to_assign, groupe_instructeur: procedure_to_assign.defaut_groupe_instructeur
+  def assign(procedure_to_assign, instructeur_assigne: instructeur)
+    create :assign_to, instructeur: instructeur_assigne, procedure: procedure_to_assign, groupe_instructeur: procedure_to_assign.defaut_groupe_instructeur
   end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index b127d35c6..19b317f40 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -224,6 +224,24 @@ describe User, type: :model do
     context 'when the user has no dossier in instruction' do
       it { is_expected.to be true }
     end
+
+    context 'when the user is an administrateur' do
+      it 'cannot be deleted' do
+        administrateur = create(:administrateur)
+        user = administrateur.user
+
+        expect(user.can_be_deleted?).to be_falsy
+      end
+    end
+
+    context 'when the user is an instructeur' do
+      it 'cannot be deleted' do
+        instructeur = create(:instructeur)
+        user = instructeur.user
+
+        expect(user.can_be_deleted?).to be_falsy
+      end
+    end
   end
 
   describe '#delete_and_keep_track_dossiers' do
@@ -241,12 +259,29 @@ describe User, type: :model do
       let!(:dossier_en_construction) { create(:dossier, :en_construction, user: user) }
       let!(:dossier_brouillon) { create(:dossier, user: user) }
 
-      it "keep track of dossiers and delete user" do
-        user.delete_and_keep_track_dossiers(administration)
+      context 'without a hidden dossier' do
+        it "keep track of dossiers and delete user" do
+          user.delete_and_keep_track_dossiers(administration)
 
-        expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present
-        expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present
-        expect(User.find_by(id: user.id)).to be_nil
+          expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present
+          expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present
+          expect(User.find_by(id: user.id)).to be_nil
+        end
+      end
+
+      context 'with a hidden dossier' do
+        let!(:dossier_cache) do
+          create(:dossier, :en_construction, user: user)
+        end
+
+        it "keep track of dossiers and delete user" do
+          dossier_cache.delete_and_keep_track(administration)
+          user.delete_and_keep_track_dossiers(administration)
+
+          expect(DeletedDossier.find_by(dossier_id: dossier_en_construction)).to be_present
+          expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present
+          expect(User.find_by(id: user.id)).to be_nil
+        end
       end
     end
   end

From 6fc8a27bd74c5952b1552895254a3326444cfa76 Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Thu, 30 Jan 2020 11:18:01 +0100
Subject: [PATCH 05/21] destroys not all dossiers

but only dossiers for a specific user
---
 app/models/user.rb       |  2 +-
 spec/models/user_spec.rb | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 35c0edd5e..6d2c6db70 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -108,7 +108,7 @@ class User < ApplicationRecord
     dossiers.each do |dossier|
       dossier.delete_and_keep_track(administration)
     end
-    dossiers.unscoped.destroy_all
+    dossiers.with_hidden.destroy_all
     destroy!
   end
 
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 19b317f40..2de7cfccd 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -273,6 +273,9 @@ describe User, type: :model do
         let!(:dossier_cache) do
           create(:dossier, :en_construction, user: user)
         end
+        let!(:dossier_from_another_user) do
+          create(:dossier, :en_construction, user: create(:user))
+        end
 
         it "keep track of dossiers and delete user" do
           dossier_cache.delete_and_keep_track(administration)
@@ -282,6 +285,13 @@ describe User, type: :model do
           expect(DeletedDossier.find_by(dossier_id: dossier_brouillon)).to be_present
           expect(User.find_by(id: user.id)).to be_nil
         end
+
+        it "doesn't destroy dossiers of another user" do
+          dossier_cache.delete_and_keep_track(administration)
+          user.delete_and_keep_track_dossiers(administration)
+
+          expect(Dossier.find_by(id: dossier_from_another_user.id)).not_to be_nil
+        end
       end
     end
   end

From 4bf020cb9602dc330bf2ee5978d66e849995cc4f Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 16:14:35 +0100
Subject: [PATCH 06/21] =?UTF-8?q?rend=20le=20test=20plus=20=C3=A9l=C3=A9ga?=
 =?UTF-8?q?nt?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 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 2de7cfccd..1af6ae573 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -290,7 +290,7 @@ describe User, type: :model do
           dossier_cache.delete_and_keep_track(administration)
           user.delete_and_keep_track_dossiers(administration)
 
-          expect(Dossier.find_by(id: dossier_from_another_user.id)).not_to be_nil
+          expect(Dossier.find_by(id: dossier_from_another_user.id)).to be_present
         end
       end
     end

From 5a46effcbc8d714a31825a9dbb896dfb3c69cec6 Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 16:33:47 +0100
Subject: [PATCH 07/21] remove useless condition to admin that can be deleted

administrateur can be deleted only if he/she has a procedure where
he/she is the only admin
---
 app/models/administrateur.rb                    |  5 ++---
 app/views/manager/administrateurs/show.html.erb |  2 +-
 spec/models/administrateur_spec.rb              | 14 +++++++-------
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb
index 1174d4b60..195352b6f 100644
--- a/app/models/administrateur.rb
+++ b/app/models/administrateur.rb
@@ -68,14 +68,13 @@ class Administrateur < ApplicationRecord
   end
 
   def can_be_deleted?
-    dossiers.state_instruction_commencee.none? && procedures.all? { |p| p.administrateurs.count > 1 }
+    procedures.all? { |p| p.administrateurs.count > 1 }
   end
 
   def delete_and_transfer_services
     if !can_be_deleted?
-      fail "Impossible de supprimer cet administrateur car il a des dossiers ou des procédures"
+      fail "Impossible de supprimer cet administrateur car il a des procédures où il est le seul administrateur"
     end
-    dossiers.each(&:delete_and_keep_track)
 
     procedures.each do |procedure|
       next_administrateur = procedure.administrateurs.where.not(id: self.id).first
diff --git a/app/views/manager/administrateurs/show.html.erb b/app/views/manager/administrateurs/show.html.erb
index 73019d37f..650f6ab60 100644
--- a/app/views/manager/administrateurs/show.html.erb
+++ b/app/views/manager/administrateurs/show.html.erb
@@ -36,7 +36,7 @@ as well as a link to its edit page.
     <% if page.resource.invitation_expired? %>
       <%= link_to "renvoyer l'invitation", reinvite_manager_administrateur_path(page.resource), method: :post, class: "button" %>
     <% end %>
-    <%= button_to "supprimer", delete_manager_administrateur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'administrateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet administrateur a des dossiers ou des procédures dont il est le seul admin et ne peut être supprimé" %>
+    <%= button_to "supprimer", delete_manager_administrateur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'administrateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet administrateur a des procédures dont il est le seul admin et ne peut être supprimé" %>
   </div>
 
 </header>
diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb
index 3e6864466..b252dc42b 100644
--- a/spec/models/administrateur_spec.rb
+++ b/spec/models/administrateur_spec.rb
@@ -24,13 +24,6 @@ describe Administrateur, type: :model do
   describe "#can_be_deleted?" do
     subject { administrateur.can_be_deleted? }
 
-    context 'when the administrateur has a dossier in instruction' do
-      let!(:dossier) { create(:dossier, :en_instruction) }
-      let(:administrateur) { dossier.procedure.administrateurs.first }
-
-      it { is_expected.to be false }
-    end
-
     context "when the administrateur's procedures have other administrateurs" do
       let!(:administrateur) { create(:administrateur) }
       let!(:autre_administrateur) { create(:administrateur) }
@@ -39,6 +32,13 @@ describe Administrateur, type: :model do
       it { is_expected.to be true }
     end
 
+    context "when the administrateur has a procedure where he/she is the only admin" do
+      let!(:administrateur) { create(:administrateur) }
+      let!(:procedure) { create(:procedure, administrateurs: [administrateur]) }
+
+      it { is_expected.to be false }
+    end
+
     context "when the administrateur has no procedure" do
       let!(:administrateur) { create(:administrateur) }
 

From deb11f281f415e36a3165ff17a2fcbd7f066c30c Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 16:37:18 +0100
Subject: [PATCH 08/21] remove useless relation between administrateur and
 dossiers

---
 app/models/administrateur.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb
index 195352b6f..04323eb97 100644
--- a/app/models/administrateur.rb
+++ b/app/models/administrateur.rb
@@ -7,7 +7,6 @@ class Administrateur < ApplicationRecord
   has_many :administrateurs_procedures
   has_many :procedures, through: :administrateurs_procedures
   has_many :services
-  has_many :dossiers, -> { state_not_brouillon }, through: :procedures
 
   has_one :user, dependent: :nullify
 

From c137917396c8e42cd0f53f23561ee598e3515bc1 Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 17:46:24 +0100
Subject: [PATCH 09/21] fix typo

---
 app/models/administrateur.rb                    | 2 +-
 app/views/manager/administrateurs/show.html.erb | 2 +-
 spec/models/administrateur_spec.rb              | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb
index 04323eb97..90bf8e8f7 100644
--- a/app/models/administrateur.rb
+++ b/app/models/administrateur.rb
@@ -72,7 +72,7 @@ class Administrateur < ApplicationRecord
 
   def delete_and_transfer_services
     if !can_be_deleted?
-      fail "Impossible de supprimer cet administrateur car il a des procédures où il est le seul administrateur"
+      fail "Impossible de supprimer cet administrateur car il a des démarches où il est le seul administrateur"
     end
 
     procedures.each do |procedure|
diff --git a/app/views/manager/administrateurs/show.html.erb b/app/views/manager/administrateurs/show.html.erb
index 650f6ab60..b33304288 100644
--- a/app/views/manager/administrateurs/show.html.erb
+++ b/app/views/manager/administrateurs/show.html.erb
@@ -36,7 +36,7 @@ as well as a link to its edit page.
     <% if page.resource.invitation_expired? %>
       <%= link_to "renvoyer l'invitation", reinvite_manager_administrateur_path(page.resource), method: :post, class: "button" %>
     <% end %>
-    <%= button_to "supprimer", delete_manager_administrateur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'administrateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet administrateur a des procédures dont il est le seul admin et ne peut être supprimé" %>
+    <%= button_to "supprimer", delete_manager_administrateur_path(page.resource), method: :delete, disabled: !page.resource.can_be_deleted?, class: "button", data: { confirm: "Confirmez-vous la suppression de l'administrateur ?" }, title: page.resource.can_be_deleted? ? "Supprimer" : "Cet administrateur a des démarches dont il est le seul admin et ne peut être supprimé" %>
   </div>
 
 </header>
diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb
index b252dc42b..3b1a023cd 100644
--- a/spec/models/administrateur_spec.rb
+++ b/spec/models/administrateur_spec.rb
@@ -32,7 +32,7 @@ describe Administrateur, type: :model do
       it { is_expected.to be true }
     end
 
-    context "when the administrateur has a procedure where he/she is the only admin" do
+    context "when the administrateur has a procedure where they is the only admin" do
       let!(:administrateur) { create(:administrateur) }
       let!(:procedure) { create(:procedure, administrateurs: [administrateur]) }
 

From 055918aabc0574c25bec20cde6572abf9fa6e5e8 Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Thu, 30 Jan 2020 17:08:09 +0100
Subject: [PATCH 10/21] admin: replace calls to `Administrateur.find_by(email:
 ...)`

---
 app/controllers/admin/procedures_controller.rb                | 2 +-
 app/controllers/manager/demandes_controller.rb                | 4 ++--
 app/controllers/manager/procedures_controller.rb              | 2 +-
 .../procedure_administrateurs_controller.rb                   | 2 +-
 app/controllers/users/passwords_controller.rb                 | 4 ++--
 app/models/administrateur.rb                                  | 3 +++
 spec/features/admin/admin_creation_spec.rb                    | 2 +-
 7 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb
index 47a86c99c..cd6259131 100644
--- a/app/controllers/admin/procedures_controller.rb
+++ b/app/controllers/admin/procedures_controller.rb
@@ -79,7 +79,7 @@ class Admin::ProceduresController < AdminController
   end
 
   def transfer
-    admin = Administrateur.find_by(email: params[:email_admin].downcase)
+    admin = Administrateur.by_email(params[:email_admin].downcase)
 
     if admin.nil?
       render '/admin/procedures/transfer', formats: 'js', status: 404
diff --git a/app/controllers/manager/demandes_controller.rb b/app/controllers/manager/demandes_controller.rb
index ffdac506c..60c01efb4 100644
--- a/app/controllers/manager/demandes_controller.rb
+++ b/app/controllers/manager/demandes_controller.rb
@@ -48,8 +48,8 @@ module Manager
     end
 
     def pending_demandes
-      already_approved_emails = Administrateur
-        .where(email: demandes.map { |d| d[:email] })
+      already_approved_emails = Administrateur.eager(:user)
+        .where(users: { email: demandes.map { |d| d[:email] } })
         .pluck(:email)
 
       demandes.reject { |demande| already_approved_emails.include?(demande[:email]) }
diff --git a/app/controllers/manager/procedures_controller.rb b/app/controllers/manager/procedures_controller.rb
index 3e1026f78..bd20a3061 100644
--- a/app/controllers/manager/procedures_controller.rb
+++ b/app/controllers/manager/procedures_controller.rb
@@ -29,7 +29,7 @@ module Manager
     end
 
     def add_administrateur
-      administrateur = Administrateur.find_by(email: params[:email])
+      administrateur = Administrateur.by_email(params[:email])
       if administrateur
         procedure.administrateurs << administrateur
         flash[:notice] = "L'administrateur \"#{params[:email]}\" est ajouté à la démarche."
diff --git a/app/controllers/new_administrateur/procedure_administrateurs_controller.rb b/app/controllers/new_administrateur/procedure_administrateurs_controller.rb
index f01a83f97..a35b980a6 100644
--- a/app/controllers/new_administrateur/procedure_administrateurs_controller.rb
+++ b/app/controllers/new_administrateur/procedure_administrateurs_controller.rb
@@ -9,7 +9,7 @@ module NewAdministrateur
       email = params.require(:administrateur)[:email]&.strip&.downcase
 
       # Find the admin
-      administrateur = Administrateur.find_by(email: email)
+      administrateur = Administrateur.by_email(email)
       if administrateur.nil?
         flash.alert = "L’administrateur « #{email} » n’existe pas. Invitez-le à demander un compte administrateur à l’addresse <a href=#{new_demande_url}>#{new_demande_url}</a>."
         return
diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb
index 97708654f..47d1c75e0 100644
--- a/app/controllers/users/passwords_controller.rb
+++ b/app/controllers/users/passwords_controller.rb
@@ -11,7 +11,7 @@ class Users::PasswordsController < Devise::PasswordsController
   def create
     # Check the credentials associated to the mail to generate a correct reset link
     email = params[:user][:email]
-    if Administrateur.find_by(email: email)
+    if Administrateur.by_email(email)
       @devise_mapping = Devise.mappings[:administrateur]
       params[:administrateur] = params[:user]
       # uncomment to check password complexity for Instructeur
@@ -56,7 +56,7 @@ class Users::PasswordsController < Devise::PasswordsController
 
   def try_to_authenticate_administrateur
     if user_signed_in?
-      administrateur = Administrateur.find_by(email: current_user.email)
+      administrateur = Administrateur.by_email(current_user.email)
 
       if administrateur
         sign_in(administrateur.user)
diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb
index 90bf8e8f7..e3016bd4b 100644
--- a/app/models/administrateur.rb
+++ b/app/models/administrateur.rb
@@ -15,6 +15,9 @@ class Administrateur < ApplicationRecord
   scope :inactive, -> { joins(:user).where(users: { last_sign_in_at: nil }) }
   scope :with_publiees_ou_closes, -> { joins(:procedures).where(procedures: { aasm_state: [:publiee, :close, :depubliee] }) }
 
+  def self.by_email(email)
+    Administrateur.eager_load(:user).find_by(users: { email: email })
+  end
   # validate :password_complexity, if: Proc.new { |a| Devise.password_length.include?(a.password.try(:size)) }
 
   def password_complexity
diff --git a/spec/features/admin/admin_creation_spec.rb b/spec/features/admin/admin_creation_spec.rb
index de719edc9..a5ce312dc 100644
--- a/spec/features/admin/admin_creation_spec.rb
+++ b/spec/features/admin/admin_creation_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 feature 'As an administrateur', js: true do
   let(:administration) { create(:administration) }
   let(:admin_email) { 'new_admin@gouv.fr' }
-  let(:new_admin) { Administrateur.find_by(email: admin_email) }
+  let(:new_admin) { Administrateur.by_email(admin_email) }
 
   before do
     perform_enqueued_jobs do

From 7bb298722f3e1daf8b4c4a0d3cb42eaad010553a Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 11:02:57 +0100
Subject: [PATCH 11/21] remove unique constraint on admin emails

---
 ...ve_unique_constraint_on_administrateur_emails.rb | 13 +++++++++++++
 db/schema.rb                                        |  4 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)
 create mode 100644 db/migrate/20200130165328_remove_unique_constraint_on_administrateur_emails.rb

diff --git a/db/migrate/20200130165328_remove_unique_constraint_on_administrateur_emails.rb b/db/migrate/20200130165328_remove_unique_constraint_on_administrateur_emails.rb
new file mode 100644
index 000000000..daa071220
--- /dev/null
+++ b/db/migrate/20200130165328_remove_unique_constraint_on_administrateur_emails.rb
@@ -0,0 +1,13 @@
+class RemoveUniqueConstraintOnAdministrateurEmails < ActiveRecord::Migration[5.2]
+  def up
+    # Drop the index entirely
+    remove_index :administrateurs, :email
+    # Add the index again, without the unicity constraint
+    add_index :administrateurs, :email
+  end
+
+  def down
+    remove_index :administrateurs, :email
+    add_index :administrateurs, :email, unique: true
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index e998f6be4..8bf635fc8 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: 2020_01_14_113700) do
+ActiveRecord::Schema.define(version: 2020_01_30_165328) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -53,7 +53,7 @@ ActiveRecord::Schema.define(version: 2020_01_14_113700) do
     t.datetime "updated_at"
     t.boolean "active", default: false
     t.string "encrypted_token"
-    t.index ["email"], name: "index_administrateurs_on_email", unique: true
+    t.index ["email"], name: "index_administrateurs_on_email"
   end
 
   create_table "administrateurs_instructeurs", id: false, force: :cascade do |t|

From 2bb161c3cb40b4432373997093e46892c15e39d4 Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 11:07:53 +0100
Subject: [PATCH 12/21] admin: alias `Administrateur.email`

---
 app/models/administrateur.rb                           | 8 +++++---
 app/models/dossier.rb                                  | 2 +-
 app/views/admin/procedures/new_from_existing.html.haml | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb
index e3016bd4b..0013dcbf5 100644
--- a/app/models/administrateur.rb
+++ b/app/models/administrateur.rb
@@ -1,6 +1,5 @@
 class Administrateur < 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
   include ActiveRecord::SecureToken
 
   has_and_belongs_to_many :instructeurs
@@ -10,14 +9,17 @@ class Administrateur < ApplicationRecord
 
   has_one :user, dependent: :nullify
 
-  before_validation -> { sanitize_email(:email) }
-
   scope :inactive, -> { joins(:user).where(users: { last_sign_in_at: nil }) }
   scope :with_publiees_ou_closes, -> { joins(:procedures).where(procedures: { aasm_state: [:publiee, :close, :depubliee] }) }
 
   def self.by_email(email)
     Administrateur.eager_load(:user).find_by(users: { email: email })
   end
+
+  def email
+    user.email
+  end
+
   # validate :password_complexity, if: Proc.new { |a| Devise.password_length.include?(a.password.try(:size)) }
 
   def password_complexity
diff --git a/app/models/dossier.rb b/app/models/dossier.rb
index 29ea1344b..8a9b41352 100644
--- a/app/models/dossier.rb
+++ b/app/models/dossier.rb
@@ -381,7 +381,7 @@ class Dossier < ApplicationRecord
     update(hidden_at: deleted_dossier.deleted_at)
 
     if en_construction?
-      administration_emails = followers_instructeurs.present? ? followers_instructeurs.map(&:email) : procedure.administrateurs.pluck(:email)
+      administration_emails = followers_instructeurs.present? ? followers_instructeurs.map(&:email) : procedure.administrateurs.map(&:email)
       administration_emails.each do |email|
         DossierMailer.notify_deletion_to_administration(deleted_dossier, email).deliver_later
       end
diff --git a/app/views/admin/procedures/new_from_existing.html.haml b/app/views/admin/procedures/new_from_existing.html.haml
index 49b7b424a..e171bd860 100644
--- a/app/views/admin/procedures/new_from_existing.html.haml
+++ b/app/views/admin/procedures/new_from_existing.html.haml
@@ -56,4 +56,4 @@
               %td.flex
                 = link_to('Consulter', apercu_procedure_path(id: procedure.id), target: "_blank", rel: "noopener", class: 'button small')
                 = link_to('Cloner', admin_procedure_clone_path(procedure.id, from_new_from_existing: true), 'data-method' => :put, class: 'button small primary')
-                = link_to('Contacter', "mailto:#{procedure.administrateurs.pluck(:email) * ","}", class: 'button small')
+                = link_to('Contacter', "mailto:#{procedure.administrateurs.map(&:email) * ","}", class: 'button small')

From ed970d3f3c0cc72db58864a7e6b7e60154c21236 Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 11:09:54 +0100
Subject: [PATCH 13/21] admin: create without providing email

---
 app/models/user.rb                                     | 2 +-
 spec/factories/administrateur.rb                       | 3 +--
 spec/mailers/previews/administration_mailer_preview.rb | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 6d2c6db70..1191236c4 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -82,7 +82,7 @@ class User < ApplicationRecord
     user = User.create_or_promote_to_instructeur(email, password)
 
     if user.valid? && user.administrateur_id.nil?
-      user.create_administrateur!(email: email)
+      user.create_administrateur!
     end
 
     user
diff --git a/spec/factories/administrateur.rb b/spec/factories/administrateur.rb
index 277cfd415..c82404e2c 100644
--- a/spec/factories/administrateur.rb
+++ b/spec/factories/administrateur.rb
@@ -1,9 +1,8 @@
 FactoryBot.define do
   sequence(:administrateur_email) { |n| "admin#{n}@admin.com" }
   factory :administrateur do
-    email { generate(:administrateur_email) }
-
     transient do
+      email { generate(:administrateur_email) }
       password { 'mon chien aime les bananes' }
     end
 
diff --git a/spec/mailers/previews/administration_mailer_preview.rb b/spec/mailers/previews/administration_mailer_preview.rb
index 7527f9632..189650445 100644
--- a/spec/mailers/previews/administration_mailer_preview.rb
+++ b/spec/mailers/previews/administration_mailer_preview.rb
@@ -41,6 +41,6 @@ class AdministrationMailerPreview < ActionMailer::Preview
   end
 
   def administrateur
-    Administrateur.new(id: 111, email: "chef.de.service@administration.gouv.fr")
+    Administrateur.new(id: 111, user: User.new(email: "chef.de.service@administration.gouv.fr"))
   end
 end

From 4a1980e95a0848944f84dba9f403bbacf99475d4 Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 13:37:10 +0100
Subject: [PATCH 14/21] admin: disable the email column

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

diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb
index 0013dcbf5..1dadad894 100644
--- a/app/models/administrateur.rb
+++ b/app/models/administrateur.rb
@@ -1,5 +1,5 @@
 class Administrateur < 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']
   include ActiveRecord::SecureToken
 
   has_and_belongs_to_many :instructeurs

From 91f172208806b32142cfc6bddee1a8845d7c527f Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 13:44:12 +0100
Subject: [PATCH 15/21] admin: always eager load the user relationship

Now that `Administrateur.email` is merely an alias to
`administrateur.user.email`,
and we changed every occurence of `administrateurs.pluck(:email)` to
`administrateurs.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
  Administrateur:

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

  Of course fetching an administrateur 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/administrateur.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb
index 1dadad894..a353901f5 100644
--- a/app/models/administrateur.rb
+++ b/app/models/administrateur.rb
@@ -9,11 +9,13 @@ class Administrateur < ApplicationRecord
 
   has_one :user, dependent: :nullify
 
+  default_scope { eager_load(:user) }
+
   scope :inactive, -> { joins(:user).where(users: { last_sign_in_at: nil }) }
   scope :with_publiees_ou_closes, -> { joins(:procedures).where(procedures: { aasm_state: [:publiee, :close, :depubliee] }) }
 
   def self.by_email(email)
-    Administrateur.eager_load(:user).find_by(users: { email: email })
+    Administrateur.find_by(users: { email: email })
   end
 
   def email

From f8309c45a68d2b6fee4a43e67ab7ce52d3bd2c2f Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 15:10:56 +0100
Subject: [PATCH 16/21] admin: order by user email

---
 .../procedure_administrateurs/index.html.haml                   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/new_administrateur/procedure_administrateurs/index.html.haml b/app/views/new_administrateur/procedure_administrateurs/index.html.haml
index d270dad37..8383f5e78 100644
--- a/app/views/new_administrateur/procedure_administrateurs/index.html.haml
+++ b/app/views/new_administrateur/procedure_administrateurs/index.html.haml
@@ -11,7 +11,7 @@
       %th= 'Enregistré le'
       %th= 'État'
     %tbody{ id: "procedure-#{@procedure.id}-administrateurs" }
-      = render partial: 'administrateur', collection: @procedure.administrateurs.order(:email)
+      = render partial: 'administrateur', collection: @procedure.administrateurs.order('users.email')
     %tfoot
       %tr
         %th{ colspan: 4 }

From cef776ff9eb011faf6c6b28a621fe42175e8e093 Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 15:11:37 +0100
Subject: [PATCH 17/21] admin: fix add administrateur to a procedure

---
 .../_add_admin_form.html.haml                      |  2 +-
 spec/features/admin/procedure_update_spec.rb       | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/app/views/new_administrateur/procedure_administrateurs/_add_admin_form.html.haml b/app/views/new_administrateur/procedure_administrateurs/_add_admin_form.html.haml
index 67c79d745..14692df28 100644
--- a/app/views/new_administrateur/procedure_administrateurs/_add_admin_form.html.haml
+++ b/app/views/new_administrateur/procedure_administrateurs/_add_admin_form.html.haml
@@ -1,4 +1,4 @@
-= form_for procedure.administrateurs.new,
+= form_for procedure.administrateurs.new(user: User.new),
   url: { controller: 'procedure_administrateurs' },
   html: { class: 'form', id:  "procedure-#{procedure.id}-new_administrateur" } ,
   remote: true do |f|
diff --git a/spec/features/admin/procedure_update_spec.rb b/spec/features/admin/procedure_update_spec.rb
index f2d09565a..7fc165ab3 100644
--- a/spec/features/admin/procedure_update_spec.rb
+++ b/spec/features/admin/procedure_update_spec.rb
@@ -59,4 +59,18 @@ feature 'Administrateurs can edit procedures', js: true do
       expect(page).to have_field('procedure_libelle', with: 'Ma petite démarche')
     end
   end
+
+  scenario 'the administrator can add another administrator' do
+    another_administrateur = create(:administrateur)
+    visit admin_procedure_path(procedure)
+    click_on 'Administrateurs'
+
+    fill_in('administrateur_email', with: another_administrateur.email)
+
+    click_on 'Ajouter comme administrateur'
+
+    within('.alert-success') do
+      expect(page).to have_content(another_administrateur.email)
+    end
+  end
 end

From 53f420c7482c0d206125f1c34a6e196cea3acf47 Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 15:50:42 +0100
Subject: [PATCH 18/21] admin: fix sql column ambiguity

administrateurs_instructeurs and users (loaded by
Administrateur.default_scope) have administrateur_id column with same
name. This commit indicates which column to use for GROUP query.
---
 app/services/administrateur_usage_statistics_service.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/administrateur_usage_statistics_service.rb b/app/services/administrateur_usage_statistics_service.rb
index 1cb9d5b57..16d220efb 100644
--- a/app/services/administrateur_usage_statistics_service.rb
+++ b/app/services/administrateur_usage_statistics_service.rb
@@ -119,7 +119,7 @@ class AdministrateurUsageStatisticsService
   end
 
   def nb_instructeurs_by_administrateur_id
-    @nb_instructeurs_by_administrateur_id ||= with_default(0, Administrateur.joins(:instructeurs).group(:administrateur_id).count)
+    @nb_instructeurs_by_administrateur_id ||= with_default(0, Administrateur.joins(:instructeurs).group('administrateurs.id').count)
   end
 
   def nb_dossiers_by_administrateur_id_and_procedure_id_and_synthetic_state

From 50fcd24b46f992f682738416056d52e36fcb1f76 Mon Sep 17 00:00:00 2001
From: Christophe Robillard <geek@robiweb.net>
Date: Mon, 3 Feb 2020 16:54:31 +0100
Subject: [PATCH 19/21] admin: fix pending demandes

fetch email with map
---
 app/controllers/manager/demandes_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/manager/demandes_controller.rb b/app/controllers/manager/demandes_controller.rb
index 60c01efb4..fc9aa6fb2 100644
--- a/app/controllers/manager/demandes_controller.rb
+++ b/app/controllers/manager/demandes_controller.rb
@@ -50,7 +50,7 @@ module Manager
     def pending_demandes
       already_approved_emails = Administrateur.eager(:user)
         .where(users: { email: demandes.map { |d| d[:email] } })
-        .pluck(:email)
+        .map(&:email)
 
       demandes.reject { |demande| already_approved_emails.include?(demande[:email]) }
     end

From c16409819b508117cb2f91b7d31877e1d599d6d5 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Mon, 3 Feb 2020 17:16:50 +0100
Subject: [PATCH 20/21] mailers: fix some missing unbreakable space in emails

---
 app/views/administration_mailer/invite_admin.html.haml      | 4 ++--
 app/views/devise_mailer/confirmation_instructions.html.haml | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/views/administration_mailer/invite_admin.html.haml b/app/views/administration_mailer/invite_admin.html.haml
index b048e8b30..ee56b73a0 100644
--- a/app/views/administration_mailer/invite_admin.html.haml
+++ b/app/views/administration_mailer/invite_admin.html.haml
@@ -12,11 +12,11 @@
 - if @reset_password_token.present?
   %p
     %b
-      Pour l’activer, cliquez sur le lien suivant :
+      Pour l’activer, cliquez sur le lien suivant :
       = link_to(admin_activate_url(token: @reset_password_token), admin_activate_url(token: @reset_password_token))
 - else
   %p
-    Pour vous connecter, cliquez sur le lien suivant :
+    Pour vous connecter, cliquez sur le lien suivant :
     = link_to(new_user_session_url, new_user_session_url)
 
 %p
diff --git a/app/views/devise_mailer/confirmation_instructions.html.haml b/app/views/devise_mailer/confirmation_instructions.html.haml
index a63b05978..08ed08de9 100644
--- a/app/views/devise_mailer/confirmation_instructions.html.haml
+++ b/app/views/devise_mailer/confirmation_instructions.html.haml
@@ -6,7 +6,7 @@
     Bonjour,
 
   %p
-    Pour activer votre compte sur demarches-simplifiees.fr, veuillez cliquer sur le lien suivant :
+    Pour activer votre compte sur demarches-simplifiees.fr, veuillez cliquer sur le lien suivant :
     = link_to(confirmation_url(@user, confirmation_token: @token), confirmation_url(@user, confirmation_token: @token))
 
 - else
@@ -16,7 +16,7 @@
     Bonjour,
 
   %p
-    Pour confirmer votre changement d'adresse email, veuillez cliquer sur le lien suivant :
+    Pour confirmer votre changement d'adresse email, veuillez cliquer sur le lien suivant :
     = link_to(confirmation_url(@user, confirmation_token: @token), confirmation_url(@user, confirmation_token: @token))
 
 = render partial: "layouts/mailers/signature"

From 415cc2c2f1d4a7d037bf59da65239a10f1e82581 Mon Sep 17 00:00:00 2001
From: clemkeirua <clement@keiruaprod.fr>
Date: Wed, 5 Feb 2020 10:32:20 +0100
Subject: [PATCH 21/21] openly fail when not delivering mail

---
 app/mailers/application_mailer.rb       |  5 -----
 app/mailers/devise_user_mailer.rb       |  5 -----
 spec/mailers/application_mailer_spec.rb | 10 ----------
 3 files changed, 20 deletions(-)

diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb
index b444548f4..3504f1a7a 100644
--- a/app/mailers/application_mailer.rb
+++ b/app/mailers/application_mailer.rb
@@ -3,11 +3,6 @@ class ApplicationMailer < ActionMailer::Base
   default from: "demarches-simplifiees.fr <#{CONTACT_EMAIL}>"
   layout 'mailer'
 
-  # Don’t retry to send a message if the server rejects the recipient address
-  rescue_from Net::SMTPSyntaxError do |_error|
-    message.perform_deliveries = false
-  end
-
   # Attach the procedure logo to the email (if any).
   # Returns the attachment url.
   def attach_logo(procedure)
diff --git a/app/mailers/devise_user_mailer.rb b/app/mailers/devise_user_mailer.rb
index de07cc1b7..a19c04768 100644
--- a/app/mailers/devise_user_mailer.rb
+++ b/app/mailers/devise_user_mailer.rb
@@ -4,11 +4,6 @@ class DeviseUserMailer < Devise::Mailer
   include Devise::Controllers::UrlHelpers # Optional. eg. `confirmation_url`
   layout 'mailers/layout'
 
-  # Don’t retry to send a message if the server rejects the recipient address
-  rescue_from Net::SMTPSyntaxError do |_error|
-    message.perform_deliveries = false
-  end
-
   def template_paths
     ['devise_mailer']
   end
diff --git a/spec/mailers/application_mailer_spec.rb b/spec/mailers/application_mailer_spec.rb
index 9cfa295f9..f4464b7ba 100644
--- a/spec/mailers/application_mailer_spec.rb
+++ b/spec/mailers/application_mailer_spec.rb
@@ -3,16 +3,6 @@ RSpec.describe ApplicationMailer, type: :mailer do
     let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) }
     subject { DossierMailer.notify_new_draft(dossier) }
 
-    describe 'invalid emails are not sent' do
-      before do
-        allow_any_instance_of(DossierMailer)
-          .to receive(:notify_new_draft)
-          .and_raise(Net::SMTPSyntaxError)
-      end
-
-      it { expect(subject.message).to be_an_instance_of(ActionMailer::Base::NullMail) }
-    end
-
     describe 'valid emails are sent' do
       it { expect(subject.message).not_to be_an_instance_of(ActionMailer::Base::NullMail) }
     end