Display “previously following” gestionnaires (#3940)

Instructeur : affiche tous les instructeurs ayant suivi le dossier à un moment donné

app/controllers/gestionnaires/dossiers_controller.rb

@@ -37,6 +37,8 @@ module Gestionnaires
 
     def personnes_impliquees
       @following_instructeurs_emails = dossier.followers_gestionnaires.pluck(:email)
+      previous_followers = dossier.previous_followers_gestionnaires - dossier.followers_gestionnaires
+      @previous_following_instructeurs_emails = previous_followers.pluck(:email)
       @avis_emails = dossier.avis.includes(:gestionnaire).map(&:email_to_display)
       @invites_emails = dossier.invites.map(&:email)
       @potential_recipients = procedure.gestionnaires.reject { |g| g == current_gestionnaire }

app/models/dossier.rb

@@ -26,8 +26,10 @@ class Dossier < ApplicationRecord
   has_many :champs_private, -> { root.private_only.ordered }, class_name: 'Champ', dependent: :destroy
   has_many :commentaires, dependent: :destroy
   has_many :invites, dependent: :destroy
-  has_many :follows
+  has_many :follows, -> { active }
+  has_many :previous_follows, -> { inactive }, class_name: 'Follow'
   has_many :followers_gestionnaires, through: :follows, source: :gestionnaire
+  has_many :previous_followers_gestionnaires, -> { distinct }, through: :previous_follows, source: :gestionnaire
   has_many :avis, dependent: :destroy
 
   has_many :dossier_operation_logs, dependent: :destroy

app/models/follow.rb

@@ -2,10 +2,13 @@ class Follow < ApplicationRecord
   belongs_to :gestionnaire
   belongs_to :dossier
 
-  validates :gestionnaire_id, uniqueness: { scope: :dossier_id }
+  validates :gestionnaire_id, uniqueness: { scope: [:dossier_id, :unfollowed_at] }
 
   before_create :set_default_date
 
+  scope :active, -> { where(unfollowed_at: nil) }
+  scope :inactive, -> { where.not(unfollowed_at: nil) }
+
   private
 
   def set_default_date

app/models/gestionnaire.rb

@@ -16,8 +16,10 @@ class Gestionnaire < ApplicationRecord
   has_many :procedures_with_email_notifications, through: :assign_to_with_email_notifications, source: :procedure
 
   has_many :dossiers, -> { state_not_brouillon }, through: :procedures
-  has_many :follows
+  has_many :follows, -> { active }
+  has_many :previous_follows, -> { inactive }, class_name: 'Follow'
   has_many :followed_dossiers, through: :follows, source: :dossier
+  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
@@ -27,26 +29,23 @@ class Gestionnaire < ApplicationRecord
   end
 
   def follow(dossier)
-    if follow?(dossier)
-      return
-    end
-
     begin
       followed_dossiers << dossier
+      # If the user tries to follow a dossier she already follows,
+      # we just fail silently: it means the goal is already reached.
     rescue ActiveRecord::RecordNotUnique
-      # Altough we checked before the insertion that the gestionnaire wasn't
-      # already following this dossier, this was done at the Rails level:
-      # at the database level, the dossier was already followed, and a
-      # "invalid constraint" exception is raised.
-      #
-      # We can ignore this safely, as it means the goal is already reached:
-      # the gestionnaire follows the dossier.
-      return
+      # Database uniqueness constraint
+    rescue ActiveRecord::RecordInvalid => e
+      # ActiveRecord validation
+      raise unless e.record.errors.details.dig(:gestionnaire_id, 0, :error) == :taken
     end
   end
 
   def unfollow(dossier)
-    followed_dossiers.delete(dossier)
+    f = follows.find_by(dossier: dossier)
+    if f.present?
+      f.update(unfollowed_at: Time.zone.now)
+    end
   end
 
   def follow?(dossier)
@@ -94,26 +93,20 @@ class Gestionnaire < ApplicationRecord
       .find_by(gestionnaire: self, dossier: dossier)
 
     if follow.present?
-      # retirer le seen_at.present? une fois la contrainte de presence en base (et les migrations ad hoc)
-      champs_publiques = follow.demande_seen_at.present? &&
-        follow.dossier.champs.updated_since?(follow.demande_seen_at).any?
+      champs_publiques = follow.dossier.champs.updated_since?(follow.demande_seen_at).any?
 
-      pieces_justificatives = follow.demande_seen_at.present? &&
-        follow.dossier.pieces_justificatives.updated_since?(follow.demande_seen_at).any?
+      pieces_justificatives = follow.dossier.pieces_justificatives.updated_since?(follow.demande_seen_at).any?
 
       demande = champs_publiques || pieces_justificatives
 
-      annotations_privees = follow.annotations_privees_seen_at.present? &&
-        follow.dossier.champs_private.updated_since?(follow.annotations_privees_seen_at).any?
+      annotations_privees = follow.dossier.champs_private.updated_since?(follow.annotations_privees_seen_at).any?
 
-      avis_notif = follow.avis_seen_at.present? &&
-        follow.dossier.avis.updated_since?(follow.avis_seen_at).any?
+      avis_notif = follow.dossier.avis.updated_since?(follow.avis_seen_at).any?
 
-      messagerie = follow.messagerie_seen_at.present? &&
-        dossier.commentaires
-          .where.not(email: OLD_CONTACT_EMAIL)
-          .where.not(email: CONTACT_EMAIL)
-          .updated_since?(follow.messagerie_seen_at).any?
+      messagerie = dossier.commentaires
+        .where.not(email: OLD_CONTACT_EMAIL)
+        .where.not(email: CONTACT_EMAIL)
+        .updated_since?(follow.messagerie_seen_at).any?
 
       annotations_hash(demande, annotations_privees, avis_notif, messagerie)
     else

app/views/gestionnaires/dossiers/personnes_impliquees.html.haml

@@ -5,7 +5,10 @@
 .personnes-impliquees.container
   = render partial: 'gestionnaires/dossiers/envoyer_dossier_block', locals: { dossier: @dossier, potential_recipients: @potential_recipients }
 
-  = render partial: 'gestionnaires/dossiers/personnes_impliquees_block', locals: { emails_collection: @following_instructeurs_emails, title: "Instructeurs qui suivent le dossier", blank: "Aucun instructeur ne suit ce dossier" }
+  = render partial: 'gestionnaires/dossiers/personnes_impliquees_block', locals: { emails_collection: @following_instructeurs_emails, title: "Instructeurs qui suivent actuellement le dossier", blank: "Aucun instructeur ne suit ce dossier" }
+
+  - if @previous_following_instructeurs_emails.present?
+    = render partial: 'gestionnaires/dossiers/personnes_impliquees_block', locals: { emails_collection: @previous_following_instructeurs_emails, title: "Instructeurs ayant précédemment suivi le dossier", blank: " " }
 
   = render partial: 'gestionnaires/dossiers/personnes_impliquees_block', locals: { emails_collection: @avis_emails, title: "Personnes à qui un avis a été demandé", blank: "Aucun avis n'a été demandé" }
 

db/migrate/20190607122941_ensure_follow_dates_not_null.rb

@@ -0,0 +1,8 @@
+class EnsureFollowDatesNotNull < ActiveRecord::Migration[5.2]
+  def change
+    change_column_null :follows, :demande_seen_at, false
+    change_column_null :follows, :annotations_privees_seen_at, false
+    change_column_null :follows, :avis_seen_at, false
+    change_column_null :follows, :messagerie_seen_at, false
+  end
+end

db/migrate/20190607124156_add_follow_unfollowed_at.rb

@@ -0,0 +1,17 @@
+class AddFollowUnfollowedAt < ActiveRecord::Migration[5.2]
+  # We need up/down migrations because `remove_index` doesn’t allow `unique: true` and can’t be properly rolled back.
+  def up
+    add_column :follows, :unfollowed_at, :datetime
+
+    remove_index :follows, [:gestionnaire_id, :dossier_id]
+    add_index :follows, [:gestionnaire_id, :dossier_id, :unfollowed_at], unique: true,
+      name: :uniqueness_index # We need a custom name because the autogenerated name would be too long
+  end
+
+  def down
+    remove_column :follows, :unfollowed_at
+    # We don’t need to remove the index: dropping the column automatically deletes it.
+
+    add_index :follows, [:gestionnaire_id, :dossier_id], unique: true
+  end
+end

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_03_27_102360) do
+ActiveRecord::Schema.define(version: 2019_06_07_124156) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -327,14 +327,15 @@ ActiveRecord::Schema.define(version: 2019_03_27_102360) do
   create_table "follows", id: :serial, force: :cascade do |t|
     t.integer "gestionnaire_id", null: false
     t.integer "dossier_id", null: false
-    t.datetime "demande_seen_at"
-    t.datetime "annotations_privees_seen_at"
-    t.datetime "avis_seen_at"
-    t.datetime "messagerie_seen_at"
+    t.datetime "demande_seen_at", null: false
+    t.datetime "annotations_privees_seen_at", null: false
+    t.datetime "avis_seen_at", null: false
+    t.datetime "messagerie_seen_at", null: false
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.datetime "unfollowed_at"
     t.index ["dossier_id"], name: "index_follows_on_dossier_id"
-    t.index ["gestionnaire_id", "dossier_id"], name: "index_follows_on_gestionnaire_id_and_dossier_id", unique: true
+    t.index ["gestionnaire_id", "dossier_id", "unfollowed_at"], name: "uniqueness_index", unique: true
     t.index ["gestionnaire_id"], name: "index_follows_on_gestionnaire_id"
   end
 

spec/models/gestionnaire_spec.rb

@@ -67,6 +67,7 @@ describe Gestionnaire, type: :model do
       end
 
       it { expect(gestionnaire.follow?(already_followed_dossier)).to be false }
+      it { expect(gestionnaire.previously_followed_dossiers).to include(already_followed_dossier) }
     end
   end