Refactor Instructeur.notifications_* methods

- rename `dossiers_id_with_notifications` to `followed_dossiers_with_notifications`,
- rename `notifications_per_procedure` to `procedures_with_notifications`,
- return an ActiveRecord::Relation instead of the result of the query, so that the call place can compose it,
- `merge` with the wanted Dossier scope in the call places, don’t bother passing it as a parameter,
- use the “state” (now “scope”) parameter as a scope method that can be just applied on `Dossier`.

app/models/dossier.rb

@@ -132,7 +132,6 @@ class Dossier < ApplicationRecord
   }
   scope :en_cours,                    -> { not_archived.state_en_construction_ou_instruction }
   scope :without_followers,           -> { left_outer_joins(:follows).where(follows: { id: nil }) }
-  scope :followed_by,                 -> (instructeur) { joins(:follows).where(follows: { instructeur: instructeur }) }
   scope :with_champs,                 -> { includes(champs: :type_de_champ) }
   scope :nearing_end_of_retention,    -> (duration = '1 month') { joins(:procedure).where("en_instruction_at + (duree_conservation_dossiers_dans_ds * interval '1 month') - now() < interval ?", duration) }
   scope :since,                       -> (since) { where('dossiers.en_construction_at >= ?', since) }

app/models/instructeur.rb

@@ -109,39 +109,28 @@ class Instructeur < ApplicationRecord
     end
   end
 
-  def notifications_for_procedure(procedure, state)
-    dossiers = case state
-    when :en_cours
-      procedure.defaut_groupe_instructeur.dossiers.en_cours
-    when :termine
-      procedure.defaut_groupe_instructeur.dossiers.termine
-    when :not_archived
-      procedure.defaut_groupe_instructeur.dossiers.not_archived
-    when :all
-      procedure.defaut_groupe_instructeur.dossiers
-    end
-
-    dossiers_id_with_notifications(dossiers)
+  def notifications_for_procedure(procedure, scope)
+    procedure
+      .defaut_groupe_instructeur.dossiers
+      .send(scope) # :en_cours or :termine or :not_archived (or any other Dossier scope)
+      .merge(followed_dossiers_with_notifications)
   end
 
-  def notifications_per_procedure(state)
-    dossiers = case state
-    when :en_cours
-      Dossier.en_cours
-    when :termine
-      Dossier.termine
-    when :not_archived
-      Dossier.not_archived
-    end
+  def procedures_with_notifications(scope)
+    dossiers = Dossier
+      .send(scope) # :en_cours or :termine (or any other Dossier scope)
+      .merge(followed_dossiers_with_notifications)
 
-    Dossier.joins(:groupe_instructeur).where(id: dossiers_id_with_notifications(dossiers)).group('groupe_instructeurs.procedure_id').count
+    Procedure
+      .where(id: dossiers.joins(:groupe_instructeur)
+        .select('groupe_instructeurs.procedure_id')
+        .distinct)
+      .distinct
   end
 
-  def dossiers_id_with_notifications(dossiers)
-    dossiers = dossiers.followed_by(self)
-
+  def followed_dossiers_with_notifications
     # Relations passed to #or must be “structurally compatible”, i.e. query the same tables.
-    joined_dossiers = dossiers
+    joined_dossiers = self.followed_dossiers
       .left_outer_joins(:champs, :champs_private, :avis, :commentaires)
 
     updated_demandes = joined_dossiers
@@ -159,7 +148,7 @@ class Instructeur < ApplicationRecord
       .where.not(commentaires: { email: OLD_CONTACT_EMAIL })
       .where.not(commentaires: { email: CONTACT_EMAIL })
 
-    updated_demandes.or(updated_annotations).or(updated_avis).or(updated_messagerie).ids
+    updated_demandes.or(updated_annotations).or(updated_avis).or(updated_messagerie)
   end
 
   def mark_tab_as_seen(dossier, tab)

app/models/procedure_presentation.rb

@@ -80,7 +80,7 @@ class ProcedurePresentation < ApplicationRecord
 
     case table
     when 'notifications'
-      dossiers_id_with_notification = instructeur.dossiers_id_with_notifications(dossiers)
+      dossiers_id_with_notification = dossiers.merge(instructeur.followed_dossiers_with_notifications).ids
       if order == 'desc'
         return dossiers_id_with_notification +
             (dossiers.order('dossiers.updated_at desc').ids - dossiers_id_with_notification)

app/views/instructeurs/procedures/index.html.haml

@@ -27,7 +27,7 @@
                 %li
                   %object
                     = link_to(instructeur_procedure_path(p, statut: 'suivis')) do
-                      - if current_instructeur.notifications_per_procedure(:en_cours)[p.id].present?
+                      - if current_instructeur.procedures_with_notifications(:en_cours).include?(p)
                         %span.notifications{ 'aria-label': "notifications" }
                       - followed_count = @followed_dossiers_count_per_procedure[p.id] || 0
                       .stats-number
@@ -37,7 +37,7 @@
                 %li
                   %object
                     = link_to(instructeur_procedure_path(p, statut: 'traites')) do
-                      - if current_instructeur.notifications_per_procedure(:termine)[p.id].present?
+                      - if current_instructeur.procedures_with_notifications(:termine).include?(p)
                         %span.notifications{ 'aria-label': "notifications" }
                       - termines_count = @dossiers_termines_count_per_procedure[p.id] || 0
                       .stats-number

app/views/instructeurs/procedures/show.html.haml

@@ -116,7 +116,7 @@
               %td.folder-col
                 = link_to(instructeur_dossier_path(@procedure, dossier), class: 'cell-link') do
                   %span.icon.folder
-                    - if current_instructeur.notifications_for_procedure(@procedure, :not_archived).include?(dossier.id)
+                    - if current_instructeur.notifications_for_procedure(@procedure, :not_archived).include?(dossier)
                       %span.notifications{ 'aria-label': 'notifications' }
 
               %td.number-col

spec/models/instructeur_spec.rb

@@ -257,14 +257,14 @@ describe Instructeur, type: :model do
     context 'when there is a modification on public champs' do
       before { dossier.champs.first.update_attribute('value', 'toto') }
 
-      it { is_expected.to match([dossier.id]) }
-      it { expect(instructeur_2.notifications_for_procedure(procedure, :en_cours)).to match([dossier.id]) }
+      it { is_expected.to match([dossier]) }
+      it { expect(instructeur_2.notifications_for_procedure(procedure, :en_cours)).to match([dossier]) }
       it { expect(instructeur_on_procedure_2.notifications_for_procedure(procedure, :en_cours)).to match([]) }
 
       context 'and there is a modification on private champs' do
         before { dossier.champs_private.first.update_attribute('value', 'toto') }
 
-        it { is_expected.to match([dossier.id]) }
+        it { is_expected.to match([dossier]) }
       end
 
       context 'when instructeur update it s public champs last seen' do
@@ -273,7 +273,7 @@ describe Instructeur, type: :model do
         before { follow.update_attribute('demande_seen_at', Time.zone.now) }
 
         it { is_expected.to match([]) }
-        it { expect(instructeur_2.notifications_for_procedure(procedure, :en_cours)).to match([dossier.id]) }
+        it { expect(instructeur_2.notifications_for_procedure(procedure, :en_cours)).to match([dossier]) }
       end
     end
 
@@ -286,20 +286,20 @@ describe Instructeur, type: :model do
     context 'when there is a modification on private champs' do
       before { dossier.champs_private.first.update_attribute('value', 'toto') }
 
-      it { is_expected.to match([dossier.id]) }
+      it { is_expected.to match([dossier]) }
     end
 
     context 'when there is a modification on avis' do
       before { create(:avis, dossier: dossier) }
 
-      it { is_expected.to match([dossier.id]) }
+      it { is_expected.to match([dossier]) }
     end
 
     context 'the messagerie' do
       context 'when there is a new commentaire' do
         before { create(:commentaire, dossier: dossier, email: 'a@b.com') }
 
-        it { is_expected.to match([dossier.id]) }
+        it { is_expected.to match([dossier]) }
       end
 
       context 'when there is a new commentaire issued by tps' do
@@ -315,12 +315,12 @@ describe Instructeur, type: :model do
     let(:instructeur) { dossier.follows.first.instructeur }
     let(:procedure) { dossier.procedure }
 
-    subject { instructeur.notifications_per_procedure(:en_cours) }
+    subject { instructeur.procedures_with_notifications(:en_cours) }
 
     context 'when there is a modification on public champs' do
       before { dossier.champs.first.update_attribute('value', 'toto') }
 
-      it { is_expected.to match({ procedure.id => 1 }) }
+      it { is_expected.to match([procedure]) }
     end
   end