diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index d1c72bf0b..1ff1145c4 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -108,10 +108,10 @@ module Instructeurs @dossiers = @dossiers.where(id: filtered_sorted_paginated_ids) - @dossiers = procedure_presentation.eager_load_displayed_fields(@dossiers) - @dossiers = @dossiers.sort_by { |d| filtered_sorted_paginated_ids.index(d.id) } + @projected_dossiers = DossierProjectionService.project(filtered_sorted_paginated_ids, procedure_presentation.displayed_fields) + kaminarize(page, filtered_sorted_ids.count) assign_exports diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb index daab7a86f..d64dff58f 100644 --- a/app/models/procedure_presentation.rb +++ b/app/models/procedure_presentation.rb @@ -90,10 +90,6 @@ class ProcedurePresentation < ApplicationRecord ] end - def displayed_fields_values(dossier) - displayed_fields.map { |field| get_value(dossier, field[TABLE], field[COLUMN]) } - end - def sorted_ids(dossiers, instructeur) table, column, order = sort.values_at(TABLE, COLUMN, 'order') @@ -178,25 +174,6 @@ class ProcedurePresentation < ApplicationRecord end.reduce(:&) end - def eager_load_displayed_fields(dossiers) - relations_to_include = displayed_fields - .pluck(TABLE) - .reject { |table| table == 'self' } - .map do |table| - case table - when TYPE_DE_CHAMP - { champs: :type_de_champ } - when TYPE_DE_CHAMP_PRIVATE - { champs_private: :type_de_champ } - else - table - end - end - .uniq - - dossiers.includes(relations_to_include) - end - def human_value_for_filter(filter) case filter[TABLE] when TYPE_DE_CHAMP, TYPE_DE_CHAMP_PRIVATE @@ -314,23 +291,6 @@ class ProcedurePresentation < ApplicationRecord end end - def get_value(dossier, table, column) - case table - when 'self' - dossier.send(column)&.strftime('%d/%m/%Y') - when 'user', 'individual', 'etablissement' - dossier.send(table)&.send(column) - when 'followers_instructeurs' - dossier.send(table)&.map { |g| g.send(column) }&.join(', ') - when TYPE_DE_CHAMP - dossier.champs.find { |c| c.stable_id == column.to_i }.to_s - when TYPE_DE_CHAMP_PRIVATE - dossier.champs_private.find { |c| c.stable_id == column.to_i }.to_s - when 'groupe_instructeur' - dossier.groupe_instructeur.label - end - end - def field_hash(label, table, column) { 'label' => label, diff --git a/app/services/dossier_projection_service.rb b/app/services/dossier_projection_service.rb new file mode 100644 index 000000000..a53905cec --- /dev/null +++ b/app/services/dossier_projection_service.rb @@ -0,0 +1,88 @@ +class DossierProjectionService + class DossierProjection < Struct.new(:dossier, :columns) + end + + TABLE = 'table' + COLUMN = 'column' + + # Returns [DossierProjection(dossier, columns)] ordered by dossiers_ids + # and the columns orderd by fields. + # + # It tries to be fast by using `pluck` (or at least `select`) + # to avoid deserializing entire records. + # + # It stores its intermediary queries results in an hash in the corresponding field. + # ex: field_email[:id_value_h] = { dossier_id_1: email_1, dossier_id_3: email_3 } + # + # Those hashes are needed because: + # - the order of the intermediary query results are unknown + # - some values can be missing (if a revision added or removed them) + def self.project(dossiers_ids, fields) + champ_fields, other_fields = fields + .partition { |f| ['type_de_champ', 'type_de_champ_private'].include?(f[TABLE]) } + + if champ_fields.present? + Champ + .includes(:type_de_champ) + .where( + # as querying the champs table is costly + # we fetch all the requested champs at once + types_de_champ: { stable_id: champ_fields.map { |f| f[COLUMN] } }, + dossier_id: dossiers_ids + ) + .select(:dossier_id, :value, :type_de_champ_id, :stable_id) # we cannot pluck :value, as we need the champ.to_s method + .group_by(&:stable_id) # the champs are redispatched to their respective fields + .map do |stable_id, champs| + field = champ_fields.find { |f| f[COLUMN] == stable_id.to_s } + field[:id_value_h] = champs.to_h { |c| [c.dossier_id, c.to_s] } + end + end + + other_fields.each do |field| + field[:id_value_h] = case field[TABLE] + when 'self' + Dossier + .where(id: dossiers_ids) + .pluck(:id, field[COLUMN].to_sym) + .to_h { |id, col| [id, col&.strftime('%d/%m/%Y')] } + when 'user' + Dossier + .joins(:user) + .where(id: dossiers_ids) + .pluck('dossiers.id, users.email') + .to_h + when 'individual' + Individual + .where(dossier_id: dossiers_ids) + .pluck(:dossier_id, field[COLUMN].to_sym) + .to_h + when 'etablissement' + Etablissement + .where(dossier_id: dossiers_ids) + .pluck(:dossier_id, field[COLUMN].to_sym) + .to_h + when 'groupe_instructeur' + Dossier + .joins(:groupe_instructeur) + .where(id: dossiers_ids) + .pluck('dossiers.id, groupe_instructeurs.label') + .to_h + when 'followers_instructeurs' + Follow + .active + .joins(instructeur: :user) + .where(dossier_id: dossiers_ids) + .pluck('dossier_id, users.email') + .group_by { |dossier_id, _| dossier_id } + .to_h { |dossier_id, dossier_id_emails| [dossier_id, dossier_id_emails.map { |_, email| email }&.join(', ')] } + end + end + + Dossier + .select(:id, :state, :archived) # the dossier object is needed in the view + .find(dossiers_ids) # keeps dossiers_ids order and raise exception if one is missing + .map do |dossier| + DossierProjection.new(dossier, fields.map { |f| f[:id_value_h][dossier.id] }) + end + end +end diff --git a/app/views/instructeurs/procedures/show.html.haml b/app/views/instructeurs/procedures/show.html.haml index a3ff15df1..90d95fcb9 100644 --- a/app/views/instructeurs/procedures/show.html.haml +++ b/app/views/instructeurs/procedures/show.html.haml @@ -129,25 +129,27 @@ = submit_tag "Enregistrer", class: 'button' %tbody - - @dossiers.each do |dossier| + - @projected_dossiers.each do |p| + - dossier = p.dossier + - path = instructeur_dossier_path(@procedure, dossier.id) + %tr %td.folder-col - = link_to(instructeur_dossier_path(@procedure, dossier), class: 'cell-link') do + %a.cell-link{ href: path } %span.icon.folder - if @not_archived_notifications_dossier_ids.include?(dossier.id) %span.notifications{ 'aria-label': 'notifications' } %td.number-col - = link_to(instructeur_dossier_path(@procedure, dossier), class: 'cell-link') do - = dossier.id + %a.cell-link{ href: path }= dossier.id - - @procedure_presentation.displayed_fields_values(dossier).each do |value| + - p.columns.each do |column| %td - = link_to(value, instructeur_dossier_path(@procedure, dossier), class: 'cell-link') + %a.cell-link{ href: path }= column %td.status-col - = link_to(instructeur_dossier_path(@procedure, dossier), class: 'cell-link') do - = status_badge(dossier.state) + %a.cell-link{ href: path }= status_badge(dossier.state) + %td.action-col.follow-col= render partial: 'dossier_actions', locals: { procedure: @procedure, dossier: dossier, dossier_is_followed: @followed_dossiers_id.include?(dossier.id) } = paginate @dossiers - else diff --git a/spec/models/procedure_presentation_spec.rb b/spec/models/procedure_presentation_spec.rb index 1949fb4b9..815096316 100644 --- a/spec/models/procedure_presentation_spec.rb +++ b/spec/models/procedure_presentation_spec.rb @@ -125,126 +125,6 @@ describe ProcedurePresentation do it { expect(subject.displayed_fields_for_select).to eq([[["label1", "table1/column1"], ["label2", "table2/column2"]], ["user/email"]]) } end - describe '#get_value' do - let(:procedure_presentation) { create(:procedure_presentation, procedure: procedure, assign_to: assign_to, displayed_fields: [{ 'table' => table, 'column' => column }]) } - - subject { procedure_presentation.displayed_fields_values(dossier).first } - - context 'for self table' do - let(:table) { 'self' } - - context 'for created_at column' do - let(:column) { 'created_at' } - let(:dossier) { Timecop.freeze(Time.zone.local(1992, 3, 22)) { create(:dossier, procedure: procedure) } } - - it { is_expected.to eq('22/03/1992') } - end - - context 'for en_construction_at column' do - let(:column) { 'en_construction_at' } - let(:dossier) { create(:dossier, :en_construction, procedure: procedure, en_construction_at: Time.zone.local(2018, 10, 17)) } - - it { is_expected.to eq('17/10/2018') } - end - - context 'for updated_at column' do - let(:column) { 'updated_at' } - let(:dossier) { create(:dossier, procedure: procedure) } - - before { dossier.touch(time: Time.zone.local(2018, 9, 25)) } - - it { is_expected.to eq('25/09/2018') } - end - end - - context 'for user table' do - let(:table) { 'user' } - let(:column) { 'email' } - - let(:dossier) { create(:dossier, procedure: procedure, user: create(:user, email: 'bla@yopmail.com')) } - - it { is_expected.to eq('bla@yopmail.com') } - end - - context 'for individual table' do - let(:table) { 'individual' } - let(:procedure) { create(:procedure, :for_individual, :with_type_de_champ, :with_type_de_champ_private) } - let(:dossier) { create(:dossier, procedure: procedure, individual: create(:individual, nom: 'Martin', prenom: 'Jacques', gender: 'M.')) } - - context 'for prenom column' do - let(:column) { 'prenom' } - - it { is_expected.to eq('Jacques') } - end - - context 'for nom column' do - let(:column) { 'nom' } - - it { is_expected.to eq('Martin') } - end - - context 'for gender column' do - let(:column) { 'gender' } - - it { is_expected.to eq('M.') } - end - end - - context 'for etablissement table' do - let(:table) { 'etablissement' } - let(:column) { 'code_postal' } # All other columns work the same, no extra test required - - let!(:dossier) { create(:dossier, procedure: procedure, etablissement: create(:etablissement, code_postal: '75008')) } - - it { is_expected.to eq('75008') } - end - - context 'for groupe_instructeur table' do - let(:table) { 'groupe_instructeur' } - let(:column) { 'label' } - - let!(:dossier) { create(:dossier, procedure: procedure) } - - it { is_expected.to eq('défaut') } - end - - context 'for followers_instructeurs table' do - let(:table) { 'followers_instructeurs' } - let(:column) { 'email' } - - let(:dossier) { create(:dossier, procedure: procedure) } - let!(:follow1) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'user1@host')) } - let!(:follow2) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'user2@host')) } - - it "return emails of followers instructeurs" do - emails_to_display = procedure_presentation.displayed_fields_values(dossier).first.split(', ').sort - expect(emails_to_display).to eq ["user1@host", "user2@host"] - end - end - - context 'for type_de_champ table' do - let(:table) { 'type_de_champ' } - let(:column) { procedure.types_de_champ.first.stable_id.to_s } - - let(:dossier) { create(:dossier, procedure: procedure) } - - before { dossier.champs.first.update(value: 'kale') } - - it { is_expected.to eq('kale') } - end - - context 'for type_de_champ_private table' do - let(:table) { 'type_de_champ_private' } - let(:column) { procedure.types_de_champ_private.first.stable_id.to_s } - - let(:dossier) { create(:dossier, procedure: procedure) } - - before { dossier.champs_private.first.update(value: 'quinoa') } - - it { is_expected.to eq('quinoa') } - end - end - describe '#sorted_ids' do let(:instructeur) { create(:instructeur) } let(:assign_to) { create(:assign_to, procedure: procedure, instructeur: instructeur) } @@ -755,136 +635,6 @@ describe ProcedurePresentation do end end - describe '#eager_load_displayed_fields' do - let(:procedure_presentation) { create(:procedure_presentation, procedure: procedure, assign_to: assign_to, displayed_fields: [{ 'table' => table, 'column' => column }]) } - let!(:dossier) { create(:dossier, :en_construction, procedure: procedure) } - let(:displayed_dossier) { procedure_presentation.eager_load_displayed_fields(procedure.dossiers).first } - - context 'for type de champ' do - let(:table) { 'type_de_champ' } - let(:column) { procedure.types_de_champ.first.stable_id.to_s } - - it 'preloads the champs relation' do - # Ideally, we would only preload the champs for the matching column - - expect(displayed_dossier.association(:champs)).to be_loaded - expect(displayed_dossier.association(:champs_private)).not_to be_loaded - expect(displayed_dossier.association(:user)).not_to be_loaded - expect(displayed_dossier.association(:individual)).not_to be_loaded - expect(displayed_dossier.association(:etablissement)).not_to be_loaded - expect(displayed_dossier.association(:followers_instructeurs)).not_to be_loaded - expect(displayed_dossier.association(:groupe_instructeur)).not_to be_loaded - end - end - - context 'for type de champ private' do - let(:table) { 'type_de_champ_private' } - let(:column) { procedure.types_de_champ_private.first.stable_id.to_s } - - it 'preloads the champs relation' do - # Ideally, we would only preload the champs for the matching column - - expect(displayed_dossier.association(:champs)).not_to be_loaded - expect(displayed_dossier.association(:champs_private)).to be_loaded - expect(displayed_dossier.association(:user)).not_to be_loaded - expect(displayed_dossier.association(:individual)).not_to be_loaded - expect(displayed_dossier.association(:etablissement)).not_to be_loaded - expect(displayed_dossier.association(:followers_instructeurs)).not_to be_loaded - expect(displayed_dossier.association(:groupe_instructeur)).not_to be_loaded - end - end - - context 'for user' do - let(:table) { 'user' } - let(:column) { 'email' } - - it 'preloads the user relation' do - expect(displayed_dossier.association(:champs)).not_to be_loaded - expect(displayed_dossier.association(:champs_private)).not_to be_loaded - expect(displayed_dossier.association(:user)).to be_loaded - expect(displayed_dossier.association(:individual)).not_to be_loaded - expect(displayed_dossier.association(:etablissement)).not_to be_loaded - expect(displayed_dossier.association(:followers_instructeurs)).not_to be_loaded - expect(displayed_dossier.association(:groupe_instructeur)).not_to be_loaded - end - end - - context 'for individual' do - let(:procedure) { create(:procedure, :for_individual, :with_type_de_champ, :with_type_de_champ_private) } - let(:table) { 'individual' } - let(:column) { 'nom' } - - it 'preloads the individual relation' do - expect(displayed_dossier.association(:champs)).not_to be_loaded - expect(displayed_dossier.association(:champs_private)).not_to be_loaded - expect(displayed_dossier.association(:user)).not_to be_loaded - expect(displayed_dossier.association(:individual)).to be_loaded - expect(displayed_dossier.association(:etablissement)).not_to be_loaded - expect(displayed_dossier.association(:followers_instructeurs)).not_to be_loaded - expect(displayed_dossier.association(:groupe_instructeur)).not_to be_loaded - end - end - - context 'for etablissement' do - let(:table) { 'etablissement' } - let(:column) { 'siret' } - - it 'preloads the etablissement relation' do - expect(displayed_dossier.association(:champs)).not_to be_loaded - expect(displayed_dossier.association(:champs_private)).not_to be_loaded - expect(displayed_dossier.association(:user)).not_to be_loaded - expect(displayed_dossier.association(:individual)).not_to be_loaded - expect(displayed_dossier.association(:etablissement)).to be_loaded - expect(displayed_dossier.association(:followers_instructeurs)).not_to be_loaded - expect(displayed_dossier.association(:groupe_instructeur)).not_to be_loaded - end - end - - context 'for followers_instructeurs' do - let(:table) { 'followers_instructeurs' } - let(:column) { 'email' } - - it 'preloads the followers_instructeurs relation' do - expect(displayed_dossier.association(:champs)).not_to be_loaded - expect(displayed_dossier.association(:champs_private)).not_to be_loaded - expect(displayed_dossier.association(:user)).not_to be_loaded - expect(displayed_dossier.association(:individual)).not_to be_loaded - expect(displayed_dossier.association(:etablissement)).not_to be_loaded - expect(displayed_dossier.association(:followers_instructeurs)).to be_loaded - expect(displayed_dossier.association(:groupe_instructeur)).not_to be_loaded - end - end - - context 'for groupe_instructeur' do - let(:table) { 'groupe_instructeur' } - let(:column) { 'label' } - - it 'preloads the groupe_instructeur relation' do - expect(displayed_dossier.association(:champs)).not_to be_loaded - expect(displayed_dossier.association(:champs_private)).not_to be_loaded - expect(displayed_dossier.association(:user)).not_to be_loaded - expect(displayed_dossier.association(:individual)).not_to be_loaded - expect(displayed_dossier.association(:etablissement)).not_to be_loaded - expect(displayed_dossier.association(:followers_instructeurs)).not_to be_loaded - expect(displayed_dossier.association(:groupe_instructeur)).to be_loaded - end - end - - context 'for self' do - let(:table) { 'self' } - let(:column) { 'created_at' } - - it 'does not preload anything' do - expect(displayed_dossier.association(:champs)).not_to be_loaded - expect(displayed_dossier.association(:champs_private)).not_to be_loaded - expect(displayed_dossier.association(:user)).not_to be_loaded - expect(displayed_dossier.association(:individual)).not_to be_loaded - expect(displayed_dossier.association(:etablissement)).not_to be_loaded - expect(displayed_dossier.association(:followers_instructeurs)).not_to be_loaded - end - end - end - describe "#human_value_for_filter" do let(:filters) { { "suivis" => [{ "label" => "label1", "table" => "type_de_champ", "column" => first_type_de_champ_id, "value" => "true" }] } } diff --git a/spec/services/dossier_projection_service_spec.rb b/spec/services/dossier_projection_service_spec.rb new file mode 100644 index 000000000..aef955f73 --- /dev/null +++ b/spec/services/dossier_projection_service_spec.rb @@ -0,0 +1,157 @@ +describe DossierProjectionService do + describe '#project' do + subject { described_class.project(dossiers_ids, fields) } + + context 'with multiple dossier' do + let!(:procedure) { create(:procedure, :with_type_de_champ) } + let!(:dossier_1) { create(:dossier, procedure: procedure) } + let!(:dossier_2) { create(:dossier, procedure: procedure) } + let!(:dossier_3) { create(:dossier, procedure: procedure) } + + let(:dossiers_ids) { [dossier_3.id, dossier_1.id, dossier_2.id] } + let(:fields) do + [ + { + "table" => "type_de_champ", + "column" => procedure.types_de_champ[0].stable_id.to_s + } + ] + end + + before do + dossier_1.champs.first.update(value: 'champ_1') + dossier_2.champs.first.update(value: 'champ_2') + dossier_3.champs.first.destroy + end + + let(:result) { subject } + + it 'respects the dossiers_ids order and returns nil for empty result' do + expect(result.length).to eq(3) + expect(result[0].dossier.id).to eq(dossier_3.id) + expect(result[1].dossier.id).to eq(dossier_1.id) + expect(result[2].dossier.id).to eq(dossier_2.id) + + expect(result[0].columns[0]).to be nil + expect(result[1].columns[0]).to eq('champ_1') + expect(result[2].columns[0]).to eq('champ_2') + end + end + + context 'attributes by attributes' do + let(:fields) { [{ "table" => table, "column" => column }] } + let(:dossiers_ids) { [dossier.id] } + + subject { super()[0].columns[0] } + + context 'for self table' do + let(:table) { 'self' } + + context 'for created_at column' do + let(:column) { 'created_at' } + let(:dossier) { Timecop.freeze(Time.zone.local(1992, 3, 22)) { create(:dossier) } } + + it { is_expected.to eq('22/03/1992') } + end + + context 'for en_construction_at column' do + let(:column) { 'en_construction_at' } + let(:dossier) { create(:dossier, :en_construction, en_construction_at: Time.zone.local(2018, 10, 17)) } + + it { is_expected.to eq('17/10/2018') } + end + + context 'for updated_at column' do + let(:column) { 'updated_at' } + let(:dossier) { create(:dossier) } + + before { dossier.touch(time: Time.zone.local(2018, 9, 25)) } + + it { is_expected.to eq('25/09/2018') } + end + end + + context 'for user table' do + let(:table) { 'user' } + let(:column) { 'email' } + + let(:dossier) { create(:dossier, user: create(:user, email: 'bla@yopmail.com')) } + + it { is_expected.to eq('bla@yopmail.com') } + end + + context 'for individual table' do + let(:table) { 'individual' } + let(:procedure) { create(:procedure, :for_individual, :with_type_de_champ, :with_type_de_champ_private) } + let(:dossier) { create(:dossier, procedure: procedure, individual: create(:individual, nom: 'Martin', prenom: 'Jacques', gender: 'M.')) } + + context 'for prenom column' do + let(:column) { 'prenom' } + + it { is_expected.to eq('Jacques') } + end + + context 'for nom column' do + let(:column) { 'nom' } + + it { is_expected.to eq('Martin') } + end + + context 'for gender column' do + let(:column) { 'gender' } + + it { is_expected.to eq('M.') } + end + end + + context 'for etablissement table' do + let(:table) { 'etablissement' } + let(:column) { 'code_postal' } # All other columns work the same, no extra test required + + let!(:dossier) { create(:dossier, etablissement: create(:etablissement, code_postal: '75008')) } + + it { is_expected.to eq('75008') } + end + + context 'for groupe_instructeur table' do + let(:table) { 'groupe_instructeur' } + let(:column) { 'label' } + + let!(:dossier) { create(:dossier) } + + it { is_expected.to eq('défaut') } + end + + context 'for followers_instructeurs table' do + let(:table) { 'followers_instructeurs' } + let(:column) { 'email' } + + let(:dossier) { create(:dossier) } + let!(:follow1) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'user1@host')) } + let!(:follow2) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'user2@host')) } + + it { is_expected.to eq "user1@host, user2@host" } + end + + context 'for type_de_champ table' do + let(:table) { 'type_de_champ' } + let(:dossier) { create(:dossier) } + let(:column) { dossier.procedure.types_de_champ.first.stable_id.to_s } + + before { dossier.champs.first.update(value: 'kale') } + + it { is_expected.to eq('kale') } + end + + context 'for type_de_champ_private table' do + let(:table) { 'type_de_champ_private' } + let(:dossier) { create(:dossier) } + let(:column) { dossier.procedure.types_de_champ_private.first.stable_id.to_s } + + before { dossier.champs_private.first.update(value: 'quinoa') } + + it { is_expected.to eq('quinoa') } + end + end + end +end