From 37c45f0d29e3bcc1a881f0cbb8d2eca5998fe0a1 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Thu, 30 Aug 2018 11:51:35 +0200 Subject: [PATCH 1/3] [#2486] Always order champs to help activerecord cache the champs collection --- .../new_user/dossiers_controller.rb | 2 +- app/facades/dossier_facades.rb | 4 ++-- app/models/dossier.rb | 18 +++++------------- .../dossiers/annotations_privees.html.haml | 4 ++-- .../new_gestionnaire/dossiers/print.html.haml | 4 ++-- app/views/shared/dossiers/_show.html.haml | 2 +- spec/models/dossier_spec.rb | 12 ++++++------ 7 files changed, 19 insertions(+), 27 deletions(-) diff --git a/app/controllers/new_user/dossiers_controller.rb b/app/controllers/new_user/dossiers_controller.rb index 3ffa4fa57..138ee6848 100644 --- a/app/controllers/new_user/dossiers_controller.rb +++ b/app/controllers/new_user/dossiers_controller.rb @@ -189,7 +189,7 @@ module NewUser end def dossier_with_champs - Dossier.with_ordered_champs.find(params[:id]) + Dossier.with_champs.find(params[:id]) end def ensure_ownership! diff --git a/app/facades/dossier_facades.rb b/app/facades/dossier_facades.rb index 635f8da82..5fbe5ae7f 100644 --- a/app/facades/dossier_facades.rb +++ b/app/facades/dossier_facades.rb @@ -10,7 +10,7 @@ class DossierFacades end def champs - @dossier.ordered_champs + @dossier.champs end def etablissement @@ -42,7 +42,7 @@ class DossierFacades end def champs_private - @dossier.ordered_champs_private + @dossier.champs_private end def individual diff --git a/app/models/dossier.rb b/app/models/dossier.rb index a894a1adb..f6205681a 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -18,8 +18,8 @@ class Dossier < ApplicationRecord has_one :attestation has_many :pieces_justificatives, dependent: :destroy - has_many :champs, -> { public_only }, dependent: :destroy - has_many :champs_private, -> { private_only }, class_name: 'Champ', dependent: :destroy + has_many :champs, -> { public_only.ordered }, dependent: :destroy + has_many :champs_private, -> { private_only.ordered }, class_name: 'Champ', dependent: :destroy has_many :quartier_prioritaires, dependent: :destroy has_many :cadastres, dependent: :destroy has_many :commentaires, dependent: :destroy @@ -59,7 +59,7 @@ 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, -> (gestionnaire) { joins(:follows).where(follows: { gestionnaire: gestionnaire }) } - scope :with_ordered_champs, -> { includes(champs: :type_de_champ).order('types_de_champ.order_place') } + scope :with_champs, -> { includes(champs: :type_de_champ) } accepts_nested_attributes_for :individual @@ -120,14 +120,6 @@ class Dossier < ApplicationRecord end end - def ordered_champs - champs.ordered - end - - def ordered_champs_private - champs_private.ordered - end - def ordered_pieces_justificatives champs.joins(', types_de_piece_justificative').where("pieces_justificatives.type_de_piece_justificative_id = types_de_piece_justificative.id AND types_de_piece_justificative.procedure_id = #{procedure.id}").order('order_place ASC') end @@ -340,8 +332,8 @@ class Dossier < ApplicationRecord def sorted_values serialized_dossier = DossierTableExportSerializer.new(self) values = serialized_dossier.attributes.values - values += ordered_champs.map(&:for_export) - values += ordered_champs_private.map(&:for_export) + values += champs.map(&:for_export) + values += champs_private.map(&:for_export) values += export_etablissement_data.values values end diff --git a/app/views/new_gestionnaire/dossiers/annotations_privees.html.haml b/app/views/new_gestionnaire/dossiers/annotations_privees.html.haml index 58b2d42e8..93a976244 100644 --- a/app/views/new_gestionnaire/dossiers/annotations_privees.html.haml +++ b/app/views/new_gestionnaire/dossiers/annotations_privees.html.haml @@ -3,10 +3,10 @@ = render partial: "header", locals: { dossier: @dossier } #dossier-annotations-privees.container - - if @dossier.ordered_champs_private.present? + - if @dossier.champs_private.present? %section = form_for @dossier, url: annotations_gestionnaire_dossier_path(@dossier.procedure, @dossier), html: { class: 'form' } do |f| - = f.fields_for :champs_private, f.object.ordered_champs_private do |champ_form| + = f.fields_for :champs_private, f.object.champs_private do |champ_form| - champ = champ_form.object = render partial: "shared/dossiers/editable_champs/editable_champ", locals: { champ: champ, form: champ_form, seen_at: @annotations_privees_seen_at } diff --git a/app/views/new_gestionnaire/dossiers/print.html.haml b/app/views/new_gestionnaire/dossiers/print.html.haml index ac7af1845..ebb055da4 100644 --- a/app/views/new_gestionnaire/dossiers/print.html.haml +++ b/app/views/new_gestionnaire/dossiers/print.html.haml @@ -13,7 +13,7 @@ %h2 Formulaire -- champs = @dossier.ordered_champs.decorate +- champs = @dossier.champs.decorate - if champs.any? = render partial: "shared/dossiers/champs", locals: { champs: champs, dossier: @dossier, demande_seen_at: nil } @@ -49,7 +49,7 @@ %h2 Annotations privées -- if @dossier.ordered_champs_private.present? +- if @dossier.champs_private.present? %table - @dossier.champs_private.each do |champ| %tr diff --git a/app/views/shared/dossiers/_show.html.haml b/app/views/shared/dossiers/_show.html.haml index 4405f57b3..e5595e499 100644 --- a/app/views/shared/dossiers/_show.html.haml +++ b/app/views/shared/dossiers/_show.html.haml @@ -10,7 +10,7 @@ = render partial: "shared/dossiers/identite_individual", locals: { individual: dossier.individual } .tab-title Formulaire - - champs = dossier.ordered_champs.includes(:type_de_champ).decorate + - champs = dossier.champs.includes(:type_de_champ).decorate - if champs.any? .card = render partial: "shared/dossiers/champs", locals: { champs: champs, demande_seen_at: demande_seen_at } diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index c3d1b283c..baf3162e9 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -12,7 +12,7 @@ describe Dossier do it { expect(Dossier.without_followers.to_a).to eq([dossier2]) } end - describe 'with_ordered_champs' do + describe 'with_champs' do let(:procedure) { create(:procedure) } let(:dossier) { Dossier.create(user: create(:user), procedure: procedure) } @@ -23,7 +23,7 @@ describe Dossier do end it do - expect(Dossier.with_ordered_champs.find(dossier.id).champs.map(&:libelle)).to match(%w(l1 l2 l3)) + expect(Dossier.with_champs.find(dossier.id).champs.map(&:libelle)).to match(%w(l1 l2 l3)) end end @@ -343,7 +343,7 @@ describe Dossier do end end - describe '#ordered_champs' do + describe '#champs' do let(:procedure) { create(:procedure) } let(:dossier) { Dossier.create(user: create(:user), procedure: procedure) } @@ -353,10 +353,10 @@ describe Dossier do create(:type_de_champ, libelle: 'l2', order_place: 2, procedure: procedure) end - it { expect(dossier.ordered_champs.pluck(:libelle)).to match(%w(l1 l2 l3)) } + it { expect(dossier.champs.pluck(:libelle)).to match(%w(l1 l2 l3)) } end - describe '#ordered_champs_private' do + describe '#champs_private' do let(:procedure) { create :procedure } let(:dossier) { Dossier.create(user: create(:user), procedure: procedure) } @@ -366,7 +366,7 @@ describe Dossier do create :type_de_champ, :private, libelle: 'l2', order_place: 2, procedure: procedure end - it { expect(dossier.ordered_champs_private.pluck(:libelle)).to match(%w(l1 l2 l3)) } + it { expect(dossier.champs_private.pluck(:libelle)).to match(%w(l1 l2 l3)) } end describe '#total_follow' do From 9403d4c04254fda246ddc68aba11ed06c367292f Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Thu, 30 Aug 2018 12:25:21 +0200 Subject: [PATCH 2/3] [#2486] followers_gestionnaires should not be a method To help activerecord caching --- app/models/dossier.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index f6205681a..301c11d50 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -27,6 +27,7 @@ class Dossier < ApplicationRecord has_many :invites_user, class_name: 'InviteUser', dependent: :destroy has_many :invites_gestionnaires, class_name: 'InviteGestionnaire', dependent: :destroy has_many :follows + has_many :followers_gestionnaires, through: :follows, source: :gestionnaire has_many :avis, dependent: :destroy belongs_to :procedure @@ -151,10 +152,6 @@ class Dossier < ApplicationRecord end end - def followers_gestionnaires - follows.includes(:gestionnaire).map(&:gestionnaire) - end - def reset! etablissement.destroy From 7326065590f5ba5524c486fc69815f89438fdad6 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Thu, 30 Aug 2018 12:26:09 +0200 Subject: [PATCH 3/3] [fix #2486] Include all needed attributes on downloadable_sorted to avoid N+1 select --- app/models/dossier.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 301c11d50..e96aa26a1 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -56,7 +56,7 @@ class Dossier < ApplicationRecord scope :en_construction, -> { not_archived.state_en_construction } scope :en_instruction, -> { not_archived.state_en_instruction } scope :termine, -> { not_archived.state_termine } - scope :downloadable_sorted, -> { state_not_brouillon.includes(:etablissement, :champs, :champs_private).order(en_construction_at: 'asc') } + scope :downloadable_sorted, -> { state_not_brouillon.includes(:etablissement, :champs, :champs_private, :user, :individual, :followers_gestionnaires).order(en_construction_at: 'asc') } scope :en_cours, -> { not_archived.state_en_construction_ou_instruction } scope :without_followers, -> { left_outer_joins(:follows).where(follows: { id: nil }) } scope :followed_by, -> (gestionnaire) { joins(:follows).where(follows: { gestionnaire: gestionnaire }) }