DGNum/demarches-normaliennes
15
1
Fork 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity

Merge pull request #4349 from betagouv/more-optimize-notifications-queries

Browse source 
Optimisation des requêtes de notifications
This commit is contained in:
LeSim 2019-09-25 14:15:13 +02:00 committed by GitHub
commit ab72ac0325
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 88 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
app/models/dossier.rb
View file

@ -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) }
@ -165,6 +164,31 @@ class Dossier < ApplicationRecord
scope :for_procedure, -> (procedure) { includes(:user, :groupe_instructeur).where(groupe_instructeurs: { procedure: procedure }) }
scope :for_api_v2, -> { includes(procedure: [:administrateurs], etablissement: [], individual: []) }
scope :with_notifications, -> do
# This scope is meant to be composed, typically with Instructeur.followed_dossiers, which means that the :follows table is already INNER JOINed;
# it will fail otherwise
# Relations passed to #or must be “structurally compatible”, i.e. query the same tables.
joined_dossiers = left_outer_joins(:champs, :champs_private, :avis, :commentaires)
updated_demandes = joined_dossiers
.where('champs.updated_at > follows.demande_seen_at')
# We join `:champs` twice, the second time with `has_many :champs_privates`. ActiveRecord generates the SQL: 'LEFT OUTER JOIN "champs" "champs_privates_dossiers" ON …'. We can then use this `champs_privates_dossiers` alias to disambiguate the table in this WHERE clause.
updated_annotations = joined_dossiers
.where('champs_privates_dossiers.updated_at > follows.annotations_privees_seen_at')
updated_avis = joined_dossiers
.where('avis.updated_at > follows.avis_seen_at')
updated_messagerie = joined_dossiers
.where('commentaires.updated_at > follows.messagerie_seen_at')
.where.not(commentaires: { email: OLD_CONTACT_EMAIL })
.where.not(commentaires: { email: CONTACT_EMAIL })
updated_demandes.or(updated_annotations).or(updated_avis).or(updated_messagerie).distinct
end
accepts_nested_attributes_for :individual
delegate :siret, :siren, to: :etablissement, allow_nil: true

64
app/models/instructeur.rb
View file

@ -109,57 +109,25 @@ 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
end
def dossiers_id_with_notifications(dossiers)
dossiers = dossiers.followed_by(self)
# Relations passed to #or must be “structurally compatible”, i.e. query the same tables.
joined_dossiers = dossiers
.left_outer_joins(:champs, :champs_private, :avis, :commentaires)
updated_demandes = joined_dossiers
.where('champs.updated_at > follows.demande_seen_at')
# We join `:champs` twice, the second time with `has_many :champs_privates`. ActiveRecord generates the SQL: 'LEFT OUTER JOIN "champs" "champs_privates_dossiers" ON …'. We can then use this `champs_privates_dossiers` alias to disambiguate the table in this WHERE clause.
updated_annotations = joined_dossiers
.where('champs_privates_dossiers.updated_at > follows.annotations_privees_seen_at')
updated_avis = joined_dossiers
.where('avis.updated_at > follows.avis_seen_at')
updated_messagerie = joined_dossiers
.where('commentaires.updated_at > follows.messagerie_seen_at')
.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
Procedure
.where(id: dossiers.joins(:groupe_instructeur)
.select('groupe_instructeurs.procedure_id')
.distinct)
.distinct
end
def mark_tab_as_seen(dossier, tab)

2
app/models/procedure_presentation.rb
View file

@ -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.with_notifications.merge(instructeur.followed_dossiers).ids
if order == 'desc'
return dossiers_id_with_notification +
(dossiers.order('dossiers.updated_at desc').ids - dossiers_id_with_notification)

4
app/views/instructeurs/procedures/index.html.haml
View file

@ -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

6
app/views/instructeurs/procedures/show.html.haml
View file

@ -24,13 +24,13 @@
instructeur_procedure_path(@procedure, statut: 'suivis'),
active: @statut == 'suivis',
badge: @followed_dossiers.count,
notification: current_instructeur.notifications_for_procedure(@procedure, :en_cours).present?)
notification: current_instructeur.notifications_for_procedure(@procedure, :en_cours).exists?)
= tab_item(t('pluralize.processed', count: @termines_dossiers.count),
instructeur_procedure_path(@procedure, statut: 'traites'),
active: @statut == 'traites',
badge: @termines_dossiers.count,
notification: current_instructeur.notifications_for_procedure(@procedure, :termine).present?)
notification: current_instructeur.notifications_for_procedure(@procedure, :termine).exists?)
= tab_item('tous les dossiers',
instructeur_procedure_path(@procedure, statut: 'tous'),
@ -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

7
db/migrate/20190920122228_add_indexes_to_dossier.rb Normal file
View file

@ -0,0 +1,7 @@
class AddIndexesToDossier < ActiveRecord::Migration[5.2]
def change
add_index :dossiers, :state
add_index :dossiers, :archived
add_index :follows, :unfollowed_at
end
end

5
db/schema.rb
View file

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2019_09_17_151652) do
ActiveRecord::Schema.define(version: 2019_09_20_122228) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -254,9 +254,11 @@ ActiveRecord::Schema.define(version: 2019_09_17_151652) do
t.bigint "groupe_instructeur_id"
t.index "to_tsvector('french'::regconfig, (search_terms || private_search_terms))", name: "index_dossiers_on_search_terms_private_search_terms", using: :gin
t.index "to_tsvector('french'::regconfig, search_terms)", name: "index_dossiers_on_search_terms", using: :gin
t.index ["archived"], name: "index_dossiers_on_archived"
t.index ["groupe_instructeur_id"], name: "index_dossiers_on_groupe_instructeur_id"
t.index ["hidden_at"], name: "index_dossiers_on_hidden_at"
t.index ["procedure_id"], name: "index_dossiers_on_procedure_id"
t.index ["state"], name: "index_dossiers_on_state"
t.index ["user_id"], name: "index_dossiers_on_user_id"
end
@ -353,6 +355,7 @@ ActiveRecord::Schema.define(version: 2019_09_17_151652) do
t.index ["dossier_id"], name: "index_follows_on_dossier_id"
t.index ["instructeur_id", "dossier_id", "unfollowed_at"], name: "uniqueness_index", unique: true
t.index ["instructeur_id"], name: "index_follows_on_instructeur_id"
t.index ["unfollowed_at"], name: "index_follows_on_unfollowed_at"
end
create_table "france_connect_informations", id: :serial, force: :cascade do |t|

21
spec/models/dossier_spec.rb
View file

@ -53,6 +53,27 @@ describe Dossier do
end
end
describe 'with_notifications' do
let(:dossier) { create(:dossier) }
let(:instructeur) { create(:instructeur) }
before do
create(:follow, dossier: dossier, instructeur: instructeur, messagerie_seen_at: 2.hours.ago)
end
subject { instructeur.followed_dossiers.with_notifications }
context('without changes') do
it { is_expected.to eq [] }
end
context('with changes') do
before { dossier.commentaires << create(:commentaire, email: 'test@exemple.fr') }
it { is_expected.to match([dossier]) }
end
end
describe 'methods' do
let(:dossier) { create(:dossier, :with_entreprise, user: user) }
let(:etablissement) { dossier.etablissement }

18
spec/models/instructeur_spec.rb
View file

@ -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