From dd80cc280a135e13a1a448f4628befc2a087415a Mon Sep 17 00:00:00 2001
From: Frederic Merizen <frederic.merizen@gmail.com>
Date: Tue, 23 Oct 2018 10:55:59 +0200
Subject: [PATCH 1/9] [#2604] Extract method in ProceduresController

---
 app/controllers/new_gestionnaire/procedures_controller.rb | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/controllers/new_gestionnaire/procedures_controller.rb b/app/controllers/new_gestionnaire/procedures_controller.rb
index 5fe1f2425..71ba18093 100644
--- a/app/controllers/new_gestionnaire/procedures_controller.rb
+++ b/app/controllers/new_gestionnaire/procedures_controller.rb
@@ -208,7 +208,11 @@ module NewGestionnaire
     end
 
     def procedure_presentation
-      @procedure_presentation ||= current_gestionnaire.procedure_presentation_for_procedure_id(params[:procedure_id])
+      @procedure_presentation ||= get_procedure_presentation
+    end
+
+    def get_procedure_presentation
+      current_gestionnaire.procedure_presentation_for_procedure_id(params[:procedure_id])
     end
 
     def displayed_fields_values

From 86f01a91205eae7541c5ec949ca821f71d42f859 Mon Sep 17 00:00:00 2001
From: Frederic Merizen <frederic.merizen@gmail.com>
Date: Tue, 23 Oct 2018 11:03:43 +0200
Subject: [PATCH 2/9] [#2604] Factor AssignTo spec

---
 spec/models/assign_to_spec.rb | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/spec/models/assign_to_spec.rb b/spec/models/assign_to_spec.rb
index b0d62ded4..8cec3dfdd 100644
--- a/spec/models/assign_to_spec.rb
+++ b/spec/models/assign_to_spec.rb
@@ -1,17 +1,18 @@
 describe AssignTo, type: :model do
   describe '#procedure_presentation_or_default' do
-    context "without a procedure_presentation" do
-      let!(:assign_to) { AssignTo.create }
+    let(:procedure) { create(:procedure) }
+    let(:assign_to) { AssignTo.create(procedure: procedure) }
 
-      it { expect(assign_to.procedure_presentation_or_default.persisted?).to be_falsey }
+    let(:procedure_presentation_or_default) { assign_to.procedure_presentation_or_default }
+
+    context "without a procedure_presentation" do
+      it { expect(procedure_presentation_or_default.persisted?).to be_falsey }
     end
 
     context "with a procedure_presentation" do
-      let(:procedure) { create(:procedure) }
-      let!(:assign_to) { AssignTo.create(procedure: procedure) }
       let!(:procedure_presentation) { ProcedurePresentation.create(assign_to: assign_to) }
 
-      it { expect(assign_to.procedure_presentation_or_default).to eq(procedure_presentation) }
+      it { expect(procedure_presentation_or_default).to eq(procedure_presentation) }
     end
   end
 end

From 91cfa69220de09d3132dded485e798c3ed3e20d5 Mon Sep 17 00:00:00 2001
From: Frederic Merizen <frederic.merizen@gmail.com>
Date: Tue, 23 Oct 2018 11:04:21 +0200
Subject: [PATCH 3/9] [#2604] Clean up AssignTo spec

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

diff --git a/spec/models/assign_to_spec.rb b/spec/models/assign_to_spec.rb
index 8cec3dfdd..9b520c188 100644
--- a/spec/models/assign_to_spec.rb
+++ b/spec/models/assign_to_spec.rb
@@ -6,7 +6,7 @@ describe AssignTo, type: :model do
     let(:procedure_presentation_or_default) { assign_to.procedure_presentation_or_default }
 
     context "without a procedure_presentation" do
-      it { expect(procedure_presentation_or_default.persisted?).to be_falsey }
+      it { expect(procedure_presentation_or_default).not_to be_persisted }
     end
 
     context "with a procedure_presentation" do

From eab8da43fcd638331f2b2347cae1390d6720ae55 Mon Sep 17 00:00:00 2001
From: Frederic Merizen <frederic.merizen@gmail.com>
Date: Tue, 23 Oct 2018 11:20:16 +0200
Subject: [PATCH 4/9] [#2604] Introduce explicit context in GestionnaireSpec

---
 spec/models/gestionnaire_spec.rb | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/spec/models/gestionnaire_spec.rb b/spec/models/gestionnaire_spec.rb
index b853c9e2c..6a2b1948e 100644
--- a/spec/models/gestionnaire_spec.rb
+++ b/spec/models/gestionnaire_spec.rb
@@ -198,10 +198,20 @@ describe Gestionnaire, type: :model do
   end
 
   describe "procedure_presentation_for_procedure_id" do
-    let!(:pp) { ProcedurePresentation.create(assign_to: procedure_assign) }
+    let(:procedure_presentation) { gestionnaire.procedure_presentation_for_procedure_id(procedure_id) }
 
-    it { expect(gestionnaire.procedure_presentation_for_procedure_id(procedure.id)).to eq(pp) }
-    it { expect(gestionnaire.procedure_presentation_for_procedure_id(procedure_2.id).persisted?).to be_falsey }
+    context 'with explicit presentation' do
+      let(:procedure_id) { procedure.id }
+      let!(:pp) { ProcedurePresentation.create(assign_to: procedure_assign) }
+
+      it { expect(procedure_presentation).to eq(pp) }
+    end
+
+    context 'with default presentation' do
+      let(:procedure_id) { procedure_2.id }
+
+      it { expect(procedure_presentation.persisted?).to be_falsey }
+    end
   end
 
   describe '#notifications_for_dossier' do

From 85f698bc712f94a47438cc64c5534b9ead4f1fe6 Mon Sep 17 00:00:00 2001
From: Frederic Merizen <frederic.merizen@gmail.com>
Date: Tue, 23 Oct 2018 11:21:32 +0200
Subject: [PATCH 5/9] [#2604] Clean up GestionnaireSpec

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

diff --git a/spec/models/gestionnaire_spec.rb b/spec/models/gestionnaire_spec.rb
index 6a2b1948e..fc9ab72bc 100644
--- a/spec/models/gestionnaire_spec.rb
+++ b/spec/models/gestionnaire_spec.rb
@@ -210,7 +210,7 @@ describe Gestionnaire, type: :model do
     context 'with default presentation' do
       let(:procedure_id) { procedure_2.id }
 
-      it { expect(procedure_presentation.persisted?).to be_falsey }
+      it { expect(procedure_presentation).not_to be_persisted }
     end
   end
 

From 84cac7a5c28534cac08189fd1dd784141b7b4863 Mon Sep 17 00:00:00 2001
From: Frederic Merizen <frederic.merizen@gmail.com>
Date: Tue, 23 Oct 2018 11:34:11 +0200
Subject: [PATCH 6/9] [#2604] Remove unneeded self in AssignTo

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

diff --git a/app/models/assign_to.rb b/app/models/assign_to.rb
index e044014c7..af232d267 100644
--- a/app/models/assign_to.rb
+++ b/app/models/assign_to.rb
@@ -4,6 +4,6 @@ class AssignTo < ApplicationRecord
   has_one :procedure_presentation, dependent: :destroy
 
   def procedure_presentation_or_default
-    self.procedure_presentation || build_procedure_presentation
+    procedure_presentation || build_procedure_presentation
   end
 end

From 7baa239095e1f79d72eb87c22e64a120b6477cb3 Mon Sep 17 00:00:00 2001
From: Frederic Merizen <frederic.merizen@gmail.com>
Date: Tue, 23 Oct 2018 11:39:54 +0200
Subject: [PATCH 7/9] [#2604] Introduce errors when retrieving
 ProcedurePresentation

---
 app/controllers/new_gestionnaire/procedures_controller.rb | 6 +++++-
 app/models/assign_to.rb                                   | 4 ++--
 app/models/gestionnaire.rb                                | 4 ++--
 spec/models/assign_to_spec.rb                             | 8 ++++++--
 spec/models/gestionnaire_spec.rb                          | 8 ++++++--
 5 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/app/controllers/new_gestionnaire/procedures_controller.rb b/app/controllers/new_gestionnaire/procedures_controller.rb
index 71ba18093..4c8ac5d56 100644
--- a/app/controllers/new_gestionnaire/procedures_controller.rb
+++ b/app/controllers/new_gestionnaire/procedures_controller.rb
@@ -212,7 +212,11 @@ module NewGestionnaire
     end
 
     def get_procedure_presentation
-      current_gestionnaire.procedure_presentation_for_procedure_id(params[:procedure_id])
+      procedure_presentation, errors = current_gestionnaire.procedure_presentation_and_errors_for_procedure_id(params[:procedure_id])
+      if errors.present?
+        flash[:alert] = "Votre affichage a dû être réinitialisé en raison du problème suivant : " + errors.full_messages.join(', ')
+      end
+      procedure_presentation
     end
 
     def displayed_fields_values
diff --git a/app/models/assign_to.rb b/app/models/assign_to.rb
index af232d267..1e21528b4 100644
--- a/app/models/assign_to.rb
+++ b/app/models/assign_to.rb
@@ -3,7 +3,7 @@ class AssignTo < ApplicationRecord
   belongs_to :gestionnaire
   has_one :procedure_presentation, dependent: :destroy
 
-  def procedure_presentation_or_default
-    procedure_presentation || build_procedure_presentation
+  def procedure_presentation_or_default_and_errors
+    [procedure_presentation || build_procedure_presentation, nil]
   end
 end
diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb
index 675cafebb..ae9030646 100644
--- a/app/models/gestionnaire.rb
+++ b/app/models/gestionnaire.rb
@@ -81,8 +81,8 @@ class Gestionnaire < ApplicationRecord
     end
   end
 
-  def procedure_presentation_for_procedure_id(procedure_id)
-    assign_to.find_by(procedure_id: procedure_id).procedure_presentation_or_default
+  def procedure_presentation_and_errors_for_procedure_id(procedure_id)
+    assign_to.find_by(procedure_id: procedure_id).procedure_presentation_or_default_and_errors
   end
 
   def notifications_for_dossier(dossier)
diff --git a/spec/models/assign_to_spec.rb b/spec/models/assign_to_spec.rb
index 9b520c188..fbd3e59d0 100644
--- a/spec/models/assign_to_spec.rb
+++ b/spec/models/assign_to_spec.rb
@@ -1,18 +1,22 @@
 describe AssignTo, type: :model do
-  describe '#procedure_presentation_or_default' do
+  describe '#procedure_presentation_or_default_and_errors' do
     let(:procedure) { create(:procedure) }
     let(:assign_to) { AssignTo.create(procedure: procedure) }
 
-    let(:procedure_presentation_or_default) { assign_to.procedure_presentation_or_default }
+    let(:procedure_presentation_and_errors) { assign_to.procedure_presentation_or_default_and_errors }
+    let(:procedure_presentation_or_default) { procedure_presentation_and_errors.first }
+    let(:errors) { procedure_presentation_and_errors.second }
 
     context "without a procedure_presentation" do
       it { expect(procedure_presentation_or_default).not_to be_persisted }
+      it { expect(errors).to be_nil }
     end
 
     context "with a procedure_presentation" do
       let!(:procedure_presentation) { ProcedurePresentation.create(assign_to: assign_to) }
 
       it { expect(procedure_presentation_or_default).to eq(procedure_presentation) }
+      it { expect(errors).to be_nil }
     end
   end
 end
diff --git a/spec/models/gestionnaire_spec.rb b/spec/models/gestionnaire_spec.rb
index fc9ab72bc..df119bc25 100644
--- a/spec/models/gestionnaire_spec.rb
+++ b/spec/models/gestionnaire_spec.rb
@@ -197,20 +197,24 @@ describe Gestionnaire, type: :model do
     end
   end
 
-  describe "procedure_presentation_for_procedure_id" do
-    let(:procedure_presentation) { gestionnaire.procedure_presentation_for_procedure_id(procedure_id) }
+  describe "procedure_presentation_and_errors_for_procedure_id" do
+    let(:procedure_presentation_and_errors) { gestionnaire.procedure_presentation_and_errors_for_procedure_id(procedure_id) }
+    let(:procedure_presentation) { procedure_presentation_and_errors.first }
+    let(:errors) { procedure_presentation_and_errors.second }
 
     context 'with explicit presentation' do
       let(:procedure_id) { procedure.id }
       let!(:pp) { ProcedurePresentation.create(assign_to: procedure_assign) }
 
       it { expect(procedure_presentation).to eq(pp) }
+      it { expect(errors).to be_nil }
     end
 
     context 'with default presentation' do
       let(:procedure_id) { procedure_2.id }
 
       it { expect(procedure_presentation).not_to be_persisted }
+      it { expect(errors).to be_nil }
     end
   end
 

From 14fd60bee7bec4a89def311920b12ceec9aa74da Mon Sep 17 00:00:00 2001
From: Frederic Merizen <frederic.merizen@gmail.com>
Date: Tue, 23 Oct 2018 11:48:25 +0200
Subject: [PATCH 8/9] [Fix #2604] Handle ProcedurePresentations that have gone
 invalid

---
 app/models/assign_to.rb          | 17 ++++++++++++++++-
 spec/models/assign_to_spec.rb    | 18 ++++++++++++++++++
 spec/models/gestionnaire_spec.rb | 11 +++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/app/models/assign_to.rb b/app/models/assign_to.rb
index 1e21528b4..b42601a33 100644
--- a/app/models/assign_to.rb
+++ b/app/models/assign_to.rb
@@ -4,6 +4,21 @@ class AssignTo < ApplicationRecord
   has_one :procedure_presentation, dependent: :destroy
 
   def procedure_presentation_or_default_and_errors
-    [procedure_presentation || build_procedure_presentation, nil]
+    errors = reset_procedure_presentation_if_invalid
+    [procedure_presentation || build_procedure_presentation, errors]
+  end
+
+  private
+
+  def reset_procedure_presentation_if_invalid
+    if procedure_presentation&.invalid?
+      # This is a last defense against invalid `ProcedurePresentation`s persistently
+      # hindering instructeurs. Whenever this gets triggered, it means that there is
+      # a bug somewhere else that we need to fix.
+
+      errors = procedure_presentation.errors
+      self.procedure_presentation = nil
+      errors
+    end
   end
 end
diff --git a/spec/models/assign_to_spec.rb b/spec/models/assign_to_spec.rb
index fbd3e59d0..5e1f0e25f 100644
--- a/spec/models/assign_to_spec.rb
+++ b/spec/models/assign_to_spec.rb
@@ -9,6 +9,7 @@ describe AssignTo, type: :model do
 
     context "without a procedure_presentation" do
       it { expect(procedure_presentation_or_default).not_to be_persisted }
+      it { expect(procedure_presentation_or_default).to be_valid }
       it { expect(errors).to be_nil }
     end
 
@@ -16,7 +17,24 @@ describe AssignTo, type: :model do
       let!(:procedure_presentation) { ProcedurePresentation.create(assign_to: assign_to) }
 
       it { expect(procedure_presentation_or_default).to eq(procedure_presentation) }
+      it { expect(procedure_presentation_or_default).to be_valid }
       it { expect(errors).to be_nil }
     end
+
+    context "with an invalid procedure_presentation" do
+      let!(:procedure_presentation) do
+        pp = ProcedurePresentation.new(assign_to: assign_to, displayed_fields: [{ 'table' => 'invalid', 'column' => 'random' }])
+        pp.save(validate: false)
+        pp
+      end
+
+      it { expect(procedure_presentation_or_default).not_to be_persisted }
+      it { expect(procedure_presentation_or_default).to be_valid }
+      it { expect(errors).to be_present }
+      it do
+        procedure_presentation_or_default
+        expect(assign_to.procedure_presentation).not_to be(procedure_presentation)
+      end
+    end
   end
 end
diff --git a/spec/models/gestionnaire_spec.rb b/spec/models/gestionnaire_spec.rb
index df119bc25..277692099 100644
--- a/spec/models/gestionnaire_spec.rb
+++ b/spec/models/gestionnaire_spec.rb
@@ -210,6 +210,17 @@ describe Gestionnaire, type: :model do
       it { expect(errors).to be_nil }
     end
 
+    context 'with invalid presentation' do
+      let(:procedure_id) { procedure.id }
+      before do
+        pp = ProcedurePresentation.create(assign_to: procedure_assign, displayed_fields: [{ 'table' => 'invalid', 'column' => 'random' }])
+        pp.save(:validate => false)
+      end
+
+      it { expect(procedure_presentation).not_to be_persisted }
+      it { expect(errors).to be_present }
+    end
+
     context 'with default presentation' do
       let(:procedure_id) { procedure_2.id }
 

From 51954a6d85f48f1d6ac21163059d713828282473 Mon Sep 17 00:00:00 2001
From: Frederic Merizen <frederic.merizen@gmail.com>
Date: Tue, 23 Oct 2018 12:18:46 +0200
Subject: [PATCH 9/9] [#2604] Warn that we are destroying an invalid
 ProcedurePresentation

---
 app/models/assign_to.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/models/assign_to.rb b/app/models/assign_to.rb
index b42601a33..1e5621fe3 100644
--- a/app/models/assign_to.rb
+++ b/app/models/assign_to.rb
@@ -17,6 +17,10 @@ class AssignTo < ApplicationRecord
       # a bug somewhere else that we need to fix.
 
       errors = procedure_presentation.errors
+      Raven.capture_message(
+        "Destroying invalid ProcedurePresentation",
+        extra: { procedure_presentation: procedure_presentation.as_json }
+      )
       self.procedure_presentation = nil
       errors
     end