From cea92c048816acf65c33c1f0b5a494e06b195bcb Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Fri, 25 Nov 2022 19:51:24 +0100 Subject: [PATCH] perf: remove n+1 queries to count admin --- .../administrateurs/procedures_controller.rb | 14 +++++--- .../administrateurs/procedures/all.html.haml | 21 ++++------- .../procedures_controller_spec.rb | 36 +++++++++++-------- 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/app/controllers/administrateurs/procedures_controller.rb b/app/controllers/administrateurs/procedures_controller.rb index ce6a48e09..c5109cdaf 100644 --- a/app/controllers/administrateurs/procedures_controller.rb +++ b/app/controllers/administrateurs/procedures_controller.rb @@ -333,12 +333,15 @@ module Administrateurs def all @filter = ProceduresFilter.new(current_administrateur, params) - @procedures = paginate(filter_procedures(@filter), published_at: :desc) + all_procedures = filter_procedures(@filter) + all_procedures = Kaminari.paginate_array(all_procedures.to_a, offset: 0, limit: ITEMS_PER_PAGE, total_count: all_procedures.count) + @procedures = all_procedures.page(params[:page]).per(25) end def administrateurs @filter = ProceduresFilter.new(current_administrateur, params) - @admins = Administrateur.includes(:user, :procedures).where(id: AdministrateursProcedure.where(procedure: filter_procedures(@filter)).select(:administrateur_id)) + pids = AdministrateursProcedure.select(:administrateur_id).where(procedure: filter_procedures(@filter).map { |p| p["id"] }) + @admins = Administrateur.includes(:user, :procedures).where(id: pids) @admins = @admins.where('unaccent(users.email) ILIKE unaccent(?)', "%#{@filter.email}%") if @filter.email.present? @admins = paginate(@admins, 'users.email') end @@ -346,12 +349,15 @@ module Administrateurs private def filter_procedures(filter) - procedures_result = Procedure.joins(:procedures_zones).distinct.publiees_ou_closes + procedures_result = Procedure.select(:id).joins(:procedures_zones).distinct.publiees_ou_closes procedures_result = procedures_result.where(procedures_zones: { zone_id: filter.zone_ids }) if filter.zone_ids.present? procedures_result = procedures_result.where(aasm_state: filter.statuses) if filter.statuses.present? procedures_result = procedures_result.where('published_at >= ?', filter.from_publication_date) if filter.from_publication_date.present? procedures_result = procedures_result.where('unaccent(libelle) ILIKE unaccent(?)', "%#{filter.libelle}%") if filter.libelle.present? - procedures_result + procedures_sql = procedures_result.to_sql + + sql = "select id, libelle, published_at, aasm_state, count(administrateurs_procedures.administrateur_id) as admin_count from administrateurs_procedures inner join procedures on procedures.id = administrateurs_procedures.procedure_id where procedures.id in (#{procedures_sql}) group by procedures.id order by published_at desc" + ActiveRecord::Base.connection.execute(sql) end def paginate(result, ordered_by) diff --git a/app/views/administrateurs/procedures/all.html.haml b/app/views/administrateurs/procedures/all.html.haml index 03183f58a..c96c36e82 100644 --- a/app/views/administrateurs/procedures/all.html.haml +++ b/app/views/administrateurs/procedures/all.html.haml @@ -10,6 +10,7 @@ = f.label :libelle, 'Rechercher des démarches par libellé', class: 'fr-label' = f.search_field 'libelle', size: 40, class: 'fr-input' .switch= link_to 'Voir la liste des administrateurs', administrateurs_admin_procedures_path(@filter.params), class: 'fr-btn fr-btn--secondary' + .switch= link_to 'Exporter les résultats', all_admin_procedures_path(@filter.params.merge(format: :xlsx)), class: 'fr-btn fr-btn--secondary' .fr-table.fr-table--bordered %table#all-demarches %caption @@ -43,19 +44,9 @@ %tr.procedure{ 'data-action': 'click->expand#toggle' } %td %button.fr-icon-add-line.fr-icon--sm.fr-mr-1w.fr-mb-1w.fr-text-action-high--blue-france{ 'aria-hidden': 'true', 'data-expand-target': 'icon' } - %td= procedure.libelle - %td= procedure.id - %td= procedure.administrateurs.count - %td= t procedure.aasm_state, scope: 'activerecord.attributes.procedure.aasm_state' - %td= l(procedure.published_at, format: :message_date_without_time) - %tr.hidden{ 'data-expand-target': 'content' } - %td.fr-highlight--beige-gris-galet{ colspan: '6' } - .fr-container - .fr-grid-row - .fr-col-6 - - procedure.zones.uniq.each do |zone| - = zone.label_at(procedure.published_or_created_at) - .fr-col-6 - - procedure.administrateurs.uniq.each do |admin| - = admin.email + %td= procedure["libelle"] + %td= procedure["id"] + %td= procedure["admin_count"] + %td= t procedure["aasm_state"], scope: 'activerecord.attributes.procedure.aasm_state' + %td= l(procedure["published_at"], format: :message_date_without_time) .fr-mt-2w= paginate @procedures, views_prefix: 'administrateurs' diff --git a/spec/controllers/administrateurs/procedures_controller_spec.rb b/spec/controllers/administrateurs/procedures_controller_spec.rb index c961c67e5..6f8984c8b 100644 --- a/spec/controllers/administrateurs/procedures_controller_spec.rb +++ b/spec/controllers/administrateurs/procedures_controller_spec.rb @@ -97,13 +97,13 @@ describe Administrateurs::ProceduresController, type: :controller do it 'display published or closed procedures' do subject - expect(assigns(:procedures)).to include(published_procedure) - expect(assigns(:procedures)).to include(closed_procedure) + expect(values_for_field(assigns(:procedures), "id")).to include(published_procedure.id) + expect(values_for_field(assigns(:procedures), "id")).to include(closed_procedure.id) end it 'doesn’t display draft procedures' do subject - expect(assigns(:procedures)).not_to include(draft_procedure) + expect(values_for_field(assigns(:procedures), "id")).not_to include(draft_procedure.id) end context "for specific zones" do @@ -116,8 +116,8 @@ describe Administrateurs::ProceduresController, type: :controller do it 'display only procedures for specified zones' do subject - expect(assigns(:procedures)).to include(procedure2) - expect(assigns(:procedures)).not_to include(procedure1) + expect(values_for_field(assigns(:procedures), "id")).to include(procedure2.id) + expect(values_for_field(assigns(:procedures), "id")).not_to include(procedure1.id) end end @@ -127,14 +127,14 @@ describe Administrateurs::ProceduresController, type: :controller do it 'display only published procedures' do get :all, params: { statuses: ['publiee'] } - expect(assigns(:procedures)).to include(procedure1) - expect(assigns(:procedures)).not_to include(procedure2) + expect(values_for_field(assigns(:procedures), "id")).to include(procedure1.id) + expect(values_for_field(assigns(:procedures), "id")).not_to include(procedure2.id) end it 'display only closed procedures' do get :all, params: { statuses: ['close'] } - expect(assigns(:procedures)).to include(procedure2) - expect(assigns(:procedures)).not_to include(procedure1) + expect(values_for_field(assigns(:procedures), "id")).to include(procedure2.id) + expect(values_for_field(assigns(:procedures), "id")).not_to include(procedure1.id) end end @@ -146,9 +146,9 @@ describe Administrateurs::ProceduresController, type: :controller do it 'display only procedures published after specific date' do get :all, params: { from_publication_date: after } - expect(assigns(:procedures)).to include(procedure1) - expect(assigns(:procedures)).to include(procedure2) - expect(assigns(:procedures)).not_to include(procedure3) + expect(values_for_field(assigns(:procedures), "id")).to include(procedure1.id) + expect(values_for_field(assigns(:procedures), "id")).to include(procedure2.id) + expect(values_for_field(assigns(:procedures), "id")).not_to include(procedure3.id) end end @@ -159,9 +159,9 @@ describe Administrateurs::ProceduresController, type: :controller do it 'returns procedures with specific terms in libelle' do get :all, params: { libelle: 'entrepreneur' } - expect(assigns(:procedures)).to include(procedure2) - expect(assigns(:procedures)).to include(procedure3) - expect(assigns(:procedures)).not_to include(procedure1) + expect(values_for_field(assigns(:procedures), "id")).to include(procedure2.id) + expect(values_for_field(assigns(:procedures), "id")).to include(procedure3.id) + expect(values_for_field(assigns(:procedures), "id")).not_to include(procedure1.id) end end end @@ -944,3 +944,9 @@ describe Administrateurs::ProceduresController, type: :controller do end end end + +def values_for_field(array, field) + array.map do |procedure| + procedure[field] + end +end