Merge pull request #6079 from betagouv/optimize_a_bit_of_instructeur_show

Optimisation de la page instructeur show procedure

app/controllers/instructeurs/procedures_controller.rb

@@ -50,38 +50,27 @@ module Instructeurs
       @current_filters = current_filters
       @displayed_fields_options, @displayed_fields_selected = procedure_presentation.displayed_fields_for_select
 
-      @a_suivre_dossiers = current_instructeur
+      @a_suivre_count, @suivis_count, @traites_count, @tous_count, @archives_count = current_instructeur
-        .dossiers
+        .dossiers_count_summary(groupe_instructeur_ids)
-        .for_procedure(procedure)
+        .fetch_values('a_suivre', 'suivis', 'traites', 'tous', 'archives')
+
+      dossiers_visibles = Dossier
+        .where(groupe_instructeur_id: groupe_instructeur_ids)
+
+      @a_suivre_dossiers = dossiers_visibles
         .without_followers
         .en_cours
 
       @followed_dossiers = current_instructeur
         .followed_dossiers
-        .where(groupe_instructeur: current_instructeur.groupe_instructeurs)
+        .where(groupe_instructeur_id: groupe_instructeur_ids)
-        .for_procedure(procedure)
         .en_cours
 
-      @followed_dossiers_id = current_instructeur
+      @followed_dossiers_id = @followed_dossiers.pluck(:id)
-        .followed_dossiers
-        .where(groupe_instructeur: current_instructeur.groupe_instructeurs)
-        .for_procedure(procedure)
-        .pluck(:id)
 
-      @termines_dossiers = current_instructeur
+      @termines_dossiers = dossiers_visibles.termine
-        .dossiers
+      @all_state_dossiers = dossiers_visibles.all_state
-        .for_procedure(procedure)
+      @archived_dossiers = dossiers_visibles.archived
-        .termine
-
-      @all_state_dossiers = current_instructeur
-        .dossiers
-        .for_procedure(procedure)
-        .all_state
-
-      @archived_dossiers = current_instructeur
-        .dossiers
-        .for_procedure(procedure)
-        .archived
 
       @dossiers = case statut
       when 'a-suivre'
@@ -241,12 +230,28 @@ module Instructeurs
       current_instructeur.assign_to.joins(:groupe_instructeur).find_by(groupe_instructeurs: { procedure: procedure })
     end
 
+    def assign_tos
+      @assign_tos ||= current_instructeur
+        .assign_to
+        .joins(:groupe_instructeur)
+        .where(groupe_instructeur: { procedure_id: procedure_id })
+    end
+
+    def groupe_instructeur_ids
+      @groupe_instructeur_ids ||= assign_tos
+        .map(&:groupe_instructeur_id)
+    end
+
     def statut
       @statut ||= (params[:statut].presence || 'a-suivre')
     end
 
+    def procedure_id
+      params[:procedure_id]
+    end
+
     def procedure
-      Procedure.find(params[:procedure_id])
+      Procedure.find(procedure_id)
     end
 
     def ensure_ownership!
@@ -267,7 +272,7 @@ module Instructeurs
     end
 
     def get_procedure_presentation
-      procedure_presentation, errors = current_instructeur.procedure_presentation_and_errors_for_procedure_id(params[:procedure_id])
+      procedure_presentation, errors = current_instructeur.procedure_presentation_and_errors_for_procedure_id(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

app/models/dossier.rb

@@ -312,7 +312,6 @@ class Dossier < ApplicationRecord
       .where.not(user: users_who_submitted)
   end
 
-  scope :for_procedure, -> (procedure) { includes(:user, :groupe_instructeur).where(groupe_instructeurs: { procedure: procedure }) }
   scope :for_api_v2, -> { includes(procedure: [:administrateurs, :attestation_template], etablissement: [], individual: [], traitements: []) }
 
   scope :with_notifications, -> do

app/models/instructeur.rb

@@ -212,6 +212,32 @@ class Instructeur < ApplicationRecord
     "Instructeur:#{id}"
   end
 
+  def dossiers_count_summary(groupe_instructeur_ids)
+    query = <<~EOF
+      SELECT
+        COUNT(DISTINCT dossiers.id) FILTER (where not archived AND dossiers.state in ('en_construction', 'en_instruction') AND follows.id IS NULL) AS a_suivre,
+        COUNT(DISTINCT dossiers.id) FILTER (where not archived AND dossiers.state in ('en_construction', 'en_instruction') AND follows.instructeur_id = :instructeur_id) AS suivis,
+        COUNT(DISTINCT dossiers.id) FILTER (where not archived AND dossiers.state in ('accepte', 'refuse', 'sans_suite')) AS traites,
+        COUNT(DISTINCT dossiers.id) FILTER (where not archived) AS tous,
+        COUNT(DISTINCT dossiers.id) FILTER (where archived)     AS archives
+      FROM "dossiers"
+        LEFT OUTER JOIN follows
+          ON  follows.dossier_id = dossiers.id
+          AND follows.unfollowed_at IS NULL
+      WHERE "dossiers"."hidden_at" IS NULL
+        AND "dossiers"."state" != 'brouillon'
+        AND "dossiers"."groupe_instructeur_id" in (:groupe_instructeur_ids)
+    EOF
+
+    sanitized_query = ActiveRecord::Base.sanitize_sql([
+      query,
+      instructeur_id: id,
+      groupe_instructeur_ids: groupe_instructeur_ids
+    ])
+
+    Dossier.connection.select_all(sanitized_query).first
+  end
+
   private
 
   def annotations_hash(demande, annotations_privees, avis, messagerie)

app/models/procedure.rb

@@ -632,47 +632,6 @@ class Procedure < ApplicationRecord
     draft_revision.deep_clone(include: [:revision_types_de_champ, :revision_types_de_champ_private])
   end
 
-  def dossiers_count_for_instructeur(instructeur)
-    query = <<-EOF
-  SELECT
-      COUNT(*) FILTER ( WHERE "dossiers"."state" in ('en_construction', 'en_instruction') and "follows"."id" IS NULL and not "dossiers"."archived") AS a_suivre,
-      COUNT(*) FILTER ( WHERE "dossiers"."state" in ('en_construction', 'en_instruction') and "follows"."instructeur_id" = :instructeur_id and not "dossiers"."archived" and "follows"."unfollowed_at" IS NULL) AS suivis,
-      COUNT(*) FILTER ( WHERE "dossiers"."state" in ('accepte', 'refuse', 'sans_suite') and not "dossiers"."archived" ) AS termines,
-      COUNT(*) FILTER ( WHERE "dossiers"."state" != 'brouillon' and not "dossiers"."archived" ) AS total,
-      COUNT(*) FILTER ( WHERE "dossiers"."archived" ) AS archived
-  FROM
-      "dossiers"
-  INNER JOIN
-      "groupe_instructeurs"
-          ON "dossiers"."groupe_instructeur_id" = "groupe_instructeurs"."id"
-  INNER JOIN
-      "assign_tos"
-          ON "groupe_instructeurs"."id" = "assign_tos"."groupe_instructeur_id"
-  INNER JOIN
-      "procedures"
-          ON "groupe_instructeurs"."procedure_id" = "procedures"."id"
-  LEFT OUTER JOIN
-      "follows"
-          ON "follows"."dossier_id" = "dossiers"."id"
-          AND "follows"."unfollowed_at" IS NULL
-  WHERE
-      "dossiers"."hidden_at" IS NULL
-      AND "assign_tos"."instructeur_id" = :instructeur_id
-      AND "procedures"."id" = :procedure_id
-  GROUP BY
-      groupe_instructeurs.procedure_id, procedures.libelle
-    EOF
-
-    sanitized_query = ActiveRecord::Base.sanitize_sql([
-      query,
-      instructeur_id: instructeur.id,
-      procedure_id: self.id,
-      now: Time.zone.now
-    ])
-
-    Procedure.connection.select_all(sanitized_query).first || { "a_suivre" => 0, "suivis" => 0, "termines" => 0, "total" => 0, "archived" => 0 }
-  end
-
   private
 
   def before_publish

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

@@ -24,29 +24,29 @@
           = tab_item('à suivre',
             instructeur_procedure_path(@procedure, statut: 'a-suivre'),
             active: @statut == 'a-suivre',
-            badge: number_with_html_delimiter(@a_suivre_dossiers.count))
+            badge: number_with_html_delimiter(@a_suivre_count))
 
-          = tab_item(t('pluralize.followed', count: @followed_dossiers.count),
+          = tab_item(t('pluralize.followed', count: @suivis_count),
             instructeur_procedure_path(@procedure, statut: 'suivis'),
             active: @statut == 'suivis',
-            badge: number_with_html_delimiter(@followed_dossiers.count),
+            badge: number_with_html_delimiter(@suivis_count),
             notification: @has_en_cours_notifications)
 
-          = tab_item(t('pluralize.processed', count: @termines_dossiers.count),
+          = tab_item(t('pluralize.processed', count: @traites_count),
             instructeur_procedure_path(@procedure, statut: 'traites'),
             active: @statut == 'traites',
-            badge: number_with_html_delimiter(@termines_dossiers.count),
+            badge: number_with_html_delimiter(@traites_count),
             notification: @has_termine_notifications)
 
           = tab_item('tous les dossiers',
             instructeur_procedure_path(@procedure, statut: 'tous'),
             active: @statut == 'tous',
-            badge: number_with_html_delimiter(@all_state_dossiers.count))
+            badge: number_with_html_delimiter(@tous_count))
 
-          = tab_item(t('pluralize.archived', count: @archived_dossiers.count),
+          = tab_item(t('pluralize.archived', count: @archives_count),
             instructeur_procedure_path(@procedure, statut: 'archives'),
             active: @statut == 'archives',
-            badge: number_with_html_delimiter(@archived_dossiers.count))
+            badge: number_with_html_delimiter(@archives_count))
 
       .procedure-actions
         = render partial: "download_dossiers",

spec/models/dossier_spec.rb

@@ -1154,23 +1154,6 @@ describe Dossier do
     end
   end
 
-  describe '#for_procedure' do
-    let!(:procedure_1) { create(:procedure)  }
-    let!(:procedure_2) { create(:procedure)  }
-
-    let!(:dossier_1_1) { create(:dossier, procedure: procedure_1) }
-    let!(:dossier_1_2) { create(:dossier, procedure: procedure_1) }
-    let!(:dossier_2_1) { create(:dossier, procedure: procedure_2) }
-
-    before do
-      gi_1_2 = procedure_1.groupe_instructeurs.create(label: 2)
-      gi_1_2.dossiers << dossier_1_2
-    end
-
-    it { expect(Dossier.for_procedure(procedure_1)).to contain_exactly(dossier_1_1, dossier_1_2) }
-    it { expect(Dossier.for_procedure(procedure_2)).to contain_exactly(dossier_2_1) }
-  end
-
   describe '#notify_draft_not_submitted' do
     let!(:user1) { create(:user) }
     let!(:user2) { create(:user) }

spec/models/instructeur_spec.rb

@@ -568,6 +568,152 @@ describe Instructeur, type: :model do
     end
   end
 
+  describe "#dossiers_count_summary" do
+    let(:instructeur_2) { create(:instructeur) }
+    let(:instructeur_3) { create(:instructeur) }
+    let(:procedure) { create(:procedure, instructeurs: [instructeur_2, instructeur_3]) }
+    let(:gi_1) { procedure.groupe_instructeurs.first }
+    let(:gi_2) { procedure.groupe_instructeurs.create(label: '2') }
+    let(:gi_3) { procedure.groupe_instructeurs.create(label: '3') }
+
+    subject do
+      instructeur_2.dossiers_count_summary([gi_1.id, gi_2.id])
+    end
+
+    context "when logged in, and belonging to gi_1, gi_2" do
+      before do
+        instructeur.groupe_instructeurs << gi_2
+      end
+
+      context "without any dossier" do
+        it { expect(subject['a_suivre']).to eq(0) }
+        it { expect(subject['suivis']).to eq(0) }
+        it { expect(subject['traites']).to eq(0) }
+        it { expect(subject['tous']).to eq(0) }
+        it { expect(subject['archives']).to eq(0) }
+      end
+
+      context 'with a new brouillon dossier' do
+        let!(:brouillon_dossier) { create(:dossier, procedure: procedure) }
+
+        it { expect(subject['a_suivre']).to eq(0) }
+        it { expect(subject['suivis']).to eq(0) }
+        it { expect(subject['traites']).to eq(0) }
+        it { expect(subject['tous']).to eq(0) }
+        it { expect(subject['archives']).to eq(0) }
+      end
+
+      context 'with a new dossier without follower' do
+        let!(:new_unfollow_dossier) { create(:dossier, :en_instruction, procedure: procedure) }
+
+        it { expect(subject['a_suivre']).to eq(1) }
+        it { expect(subject['suivis']).to eq(0) }
+        it { expect(subject['traites']).to eq(0) }
+        it { expect(subject['tous']).to eq(1) }
+        it { expect(subject['archives']).to eq(0) }
+
+        context 'and dossiers without follower on each of the others groups' do
+          let!(:new_unfollow_dossier_on_gi_2) { create(:dossier, :en_instruction, groupe_instructeur: gi_2) }
+          let!(:new_unfollow_dossier_on_gi_3) { create(:dossier, :en_instruction, groupe_instructeur: gi_3) }
+
+          before { subject }
+
+          it { expect(subject['a_suivre']).to eq(2) }
+          it { expect(subject['tous']).to eq(2) }
+        end
+      end
+
+      context 'with a new dossier with a follower' do
+        let!(:new_followed_dossier) { create(:dossier, :en_instruction, procedure: procedure) }
+
+        before do
+          instructeur_2.followed_dossiers << new_followed_dossier
+        end
+
+        it { expect(subject['a_suivre']).to eq(0) }
+        it { expect(subject['suivis']).to eq(1) }
+        it { expect(subject['traites']).to eq(0) }
+        it { expect(subject['tous']).to eq(1) }
+        it { expect(subject['archives']).to eq(0) }
+
+        context 'and another one follows the same dossier' do
+          before do
+            instructeur_3.followed_dossiers << new_followed_dossier
+          end
+
+          it { expect(subject['a_suivre']).to eq(0) }
+          it { expect(subject['suivis']).to eq(1) }
+          it { expect(subject['traites']).to eq(0) }
+          it { expect(subject['tous']).to eq(1) }
+          it { expect(subject['archives']).to eq(0) }
+        end
+
+        context 'and dossier with a follower on each of the others groups' do
+          let!(:new_follow_dossier_on_gi_2) { create(:dossier, :en_instruction, groupe_instructeur: gi_2) }
+          let!(:new_follow_dossier_on_gi_3) { create(:dossier, :en_instruction, groupe_instructeur: gi_3) }
+
+          before do
+            instructeur_2.followed_dossiers << new_follow_dossier_on_gi_2 << new_follow_dossier_on_gi_3
+          end
+
+          # followed dossiers on another groupe should not be displayed
+          it { expect(subject['suivis']).to eq(2) }
+          it { expect(subject['tous']).to eq(2) }
+        end
+
+        context 'and dossier with a follower is unfollowed' do
+          before do
+            instructeur_2.unfollow(new_followed_dossier)
+          end
+
+          it { expect(subject['a_suivre']).to eq(1) }
+          it { expect(subject['suivis']).to eq(0) }
+          it { expect(subject['tous']).to eq(1) }
+        end
+      end
+
+      context 'with a termine dossier' do
+        let!(:termine_dossier) { create(:dossier, :accepte, procedure: procedure) }
+
+        it { expect(subject['a_suivre']).to eq(0) }
+        it { expect(subject['suivis']).to eq(0) }
+        it { expect(subject['traites']).to eq(1) }
+        it { expect(subject['tous']).to eq(1) }
+        it { expect(subject['archives']).to eq(0) }
+
+        context 'and terminer dossiers on each of the others groups' do
+          let!(:termine_dossier_on_gi_2) { create(:dossier, :accepte, groupe_instructeur: gi_2) }
+          let!(:termine_dossier_on_gi_3) { create(:dossier, :accepte, groupe_instructeur: gi_3) }
+
+          before { subject }
+
+          it { expect(subject['a_suivre']).to eq(0) }
+          it { expect(subject['suivis']).to eq(0) }
+          it { expect(subject['traites']).to eq(2) }
+          it { expect(subject['tous']).to eq(2) }
+          it { expect(subject['archives']).to eq(0) }
+        end
+      end
+
+      context 'with an archives dossier' do
+        let!(:archives_dossier) { create(:dossier, :en_instruction, procedure: procedure, archived: true) }
+
+        it { expect(subject['a_suivre']).to eq(0) }
+        it { expect(subject['suivis']).to eq(0) }
+        it { expect(subject['traites']).to eq(0) }
+        it { expect(subject['tous']).to eq(0) }
+        it { expect(subject['archives']).to eq(1) }
+
+        context 'and terminer dossiers on each of the others groups' do
+          let!(:archives_dossier_on_gi_2) { create(:dossier, :en_instruction, groupe_instructeur: gi_2, archived: true) }
+          let!(:archives_dossier_on_gi_3) { create(:dossier, :en_instruction, groupe_instructeur: gi_3, archived: true) }
+
+          it { expect(subject['archives']).to eq(2) }
+        end
+      end
+    end
+  end
+
   private
 
   def assign(procedure_to_assign, instructeur_assigne: instructeur)

spec/models/procedure_spec.rb

@@ -1030,136 +1030,4 @@ describe Procedure do
       it { is_expected.to be false }
     end
   end
-
-  describe "#dossiers_count_for_instructeur" do
-    let(:instructeur) { create(:instructeur) }
-    let(:procedure) { create(:procedure, instructeurs: [instructeur]) }
-    let(:gi_2) { procedure.groupe_instructeurs.create(label: '2') }
-    let(:gi_3) { procedure.groupe_instructeurs.create(label: '3') }
-
-    subject do
-      procedure.dossiers_count_for_instructeur(instructeur)
-    end
-
-    context "when logged in, and belonging to gi_1, gi_2" do
-      before do
-        instructeur.groupe_instructeurs << gi_2
-      end
-
-      context "without any dossier" do
-        it { expect(subject['a_suivre']).to eq(0) }
-        it { expect(subject['suivis']).to eq(0) }
-        it { expect(subject['termines']).to eq(0) }
-        it { expect(subject['total']).to eq(0) }
-        it { expect(subject['archived']).to eq(0) }
-      end
-
-      context 'with a new brouillon dossier' do
-        let!(:brouillon_dossier) { create(:dossier, procedure: procedure) }
-
-        it { expect(subject['a_suivre']).to eq(0) }
-        it { expect(subject['suivis']).to eq(0) }
-        it { expect(subject['termines']).to eq(0) }
-        it { expect(subject['total']).to eq(0) }
-        it { expect(subject['archived']).to eq(0) }
-      end
-
-      context 'with a new dossier without follower' do
-        let!(:new_unfollow_dossier) { create(:dossier, :en_instruction, procedure: procedure) }
-
-        it { expect(subject['a_suivre']).to eq(1) }
-        it { expect(subject['suivis']).to eq(0) }
-        it { expect(subject['termines']).to eq(0) }
-        it { expect(subject['total']).to eq(1) }
-        it { expect(subject['archived']).to eq(0) }
-
-        context 'and dossiers without follower on each of the others groups' do
-          let!(:new_unfollow_dossier_on_gi_2) { create(:dossier, :en_instruction, groupe_instructeur: gi_2) }
-          let!(:new_unfollow_dossier_on_gi_3) { create(:dossier, :en_instruction, groupe_instructeur: gi_3) }
-
-          before { subject }
-
-          it { expect(subject['a_suivre']).to eq(2) }
-          it { expect(subject['total']).to eq(2) }
-        end
-      end
-
-      context 'with a new dossier with a follower' do
-        let!(:new_followed_dossier) { create(:dossier, :en_instruction, procedure: procedure) }
-
-        before do
-          instructeur.followed_dossiers << new_followed_dossier
-        end
-
-        it { expect(subject['a_suivre']).to eq(0) }
-        it { expect(subject['suivis']).to eq(1) }
-        it { expect(subject['termines']).to eq(0) }
-        it { expect(subject['total']).to eq(1) }
-        it { expect(subject['archived']).to eq(0) }
-
-        context 'and dossier with a follower on each of the others groups' do
-          let!(:new_follow_dossier_on_gi_2) { create(:dossier, :en_instruction, groupe_instructeur: gi_2) }
-          let!(:new_follow_dossier_on_gi_3) { create(:dossier, :en_instruction, groupe_instructeur: gi_3) }
-
-          before do
-            instructeur.followed_dossiers << new_follow_dossier_on_gi_2 << new_follow_dossier_on_gi_3
-          end
-
-          # followed dossiers on another groupe should not be displayed
-          it { expect(subject['suivis']).to eq(2) }
-          it { expect(subject['total']).to eq(2) }
-        end
-
-        context 'and dossier with a follower is unfollowed' do
-          before do
-            instructeur.unfollow(new_followed_dossier)
-          end
-
-          it { expect(subject['a_suivre']).to eq(1) }
-          it { expect(subject['suivis']).to eq(0) }
-          it { expect(subject['total']).to eq(1) }
-        end
-      end
-
-      context 'with a termine dossier' do
-        let!(:termine_dossier) { create(:dossier, :accepte, procedure: procedure) }
-
-        it { expect(subject['a_suivre']).to eq(0) }
-        it { expect(subject['suivis']).to eq(0) }
-        it { expect(subject['termines']).to eq(1) }
-        it { expect(subject['total']).to eq(1) }
-        it { expect(subject['archived']).to eq(0) }
-
-        context 'and terminer dossiers on each of the others groups' do
-          let!(:termine_dossier_on_gi_2) { create(:dossier, :accepte, groupe_instructeur: gi_2) }
-          let!(:termine_dossier_on_gi_3) { create(:dossier, :accepte, groupe_instructeur: gi_3) }
-
-          before { subject }
-
-          it { expect(subject['a_suivre']).to eq(0) }
-          it { expect(subject['suivis']).to eq(0) }
-          it { expect(subject['termines']).to eq(2) }
-          it { expect(subject['total']).to eq(2) }
-          it { expect(subject['archived']).to eq(0) }
-        end
-      end
-
-      context 'with an archived dossier' do
-        let!(:archived_dossier) { create(:dossier, :en_instruction, procedure: procedure, archived: true) }
-
-        it { expect(subject['a_suivre']).to eq(0) }
-        it { expect(subject['suivis']).to eq(0) }
-        it { expect(subject['termines']).to eq(0) }
-        it { expect(subject['total']).to eq(0) }
-        it { expect(subject['archived']).to eq(1) }
-
-        context 'and terminer dossiers on each of the others groups' do
-          let!(:archived_dossier_on_gi_2) { create(:dossier, :en_instruction, groupe_instructeur: gi_2, archived: true) }
-          let!(:archived_dossier_on_gi_3) { create(:dossier, :en_instruction, groupe_instructeur: gi_3, archived: true) }
-
-          it { expect(subject['archived']).to eq(2) }
-        end
-      end
-    end
-  end
 end