From 21577a0f0b322ea8790f651cc7c4a0753c0cd0c9 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 18 Sep 2019 11:46:28 +0200 Subject: [PATCH 1/5] Instructeur: dedup procedures with multiple gi --- app/models/instructeur.rb | 2 +- spec/models/instructeur_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 3cdfa45c9..6334d9fa7 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -8,7 +8,7 @@ class Instructeur < ApplicationRecord has_many :assign_to, dependent: :destroy has_many :groupe_instructeurs, through: :assign_to - has_many :procedures, through: :groupe_instructeurs + has_many :procedures, -> { distinct }, through: :groupe_instructeurs has_many :assign_to_with_email_notifications, -> { with_email_notifications }, class_name: 'AssignTo', inverse_of: :instructeur has_many :groupe_instructeur_with_email_notifications, through: :assign_to_with_email_notifications, source: :groupe_instructeur diff --git a/spec/models/instructeur_spec.rb b/spec/models/instructeur_spec.rb index 5b0ba0ed4..7d9c36846 100644 --- a/spec/models/instructeur_spec.rb +++ b/spec/models/instructeur_spec.rb @@ -410,6 +410,19 @@ describe Instructeur, type: :model do end end + describe '#procedures' do + let(:procedure_a) { create(:procedure) } + let(:instructeur_a) { create(:instructeur, groupe_instructeurs: [procedure_a.defaut_groupe_instructeur]) } + + before do + gi2 = procedure_a.groupe_instructeurs.create(label: '2') + + instructeur_a.groupe_instructeurs << gi2 + end + + it { expect(instructeur_a.procedures.all.to_ary).to eq([procedure_a]) } + end + private def assign(procedure_to_assign) From 9b16bd203741f8e757089b83221a66da8e54d2fc Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 18 Sep 2019 13:07:30 +0200 Subject: [PATCH 2/5] Dossier: retrieve all dossiers for a procedure --- app/models/dossier.rb | 2 ++ spec/models/dossier_spec.rb | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index d7a807ca3..1f5cd63b0 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -161,6 +161,8 @@ class Dossier < ApplicationRecord user: []) } + scope :for_procedure, -> (procedure) { includes(:user, :groupe_instructeur).where(groupe_instructeurs: { procedure: procedure }) } + accepts_nested_attributes_for :individual delegate :siret, :siren, to: :etablissement, allow_nil: true diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 08d65d4fe..5da59042b 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -1009,4 +1009,21 @@ describe Dossier do expect(dossier.individual.prenom).to eq user_info.given_name } 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 end From f5bbc9e2f9a5ee0abb0ac09c39bf71e9ddc1698c Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 18 Sep 2019 13:44:00 +0200 Subject: [PATCH 3/5] InstructeurProcedureShow: show dossier for all the gi involved --- .../instructeurs/procedures_controller.rb | 27 ++++++--- .../procedures_controller_spec.rb | 55 ++++++++++++++++++- 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 3d7353b32..414a571df 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -40,29 +40,38 @@ module Instructeurs @procedure_presentation = procedure_presentation @displayed_fields_values = displayed_fields_values - @a_suivre_dossiers = procedure - .defaut_groupe_instructeur + @a_suivre_dossiers = current_instructeur .dossiers - .includes(:user) + .for_procedure(procedure) .without_followers .en_cours @followed_dossiers = current_instructeur .followed_dossiers - .includes(:user) - .where(groupe_instructeur: procedure.defaut_groupe_instructeur) + .where(groupe_instructeur: current_instructeur.groupe_instructeurs) + .for_procedure(procedure) .en_cours @followed_dossiers_id = current_instructeur .followed_dossiers - .where(groupe_instructeur: procedure.defaut_groupe_instructeur) + .where(groupe_instructeur: current_instructeur.groupe_instructeurs) + .for_procedure(procedure) .pluck(:id) - @termines_dossiers = procedure.defaut_groupe_instructeur.dossiers.includes(:user).termine + @termines_dossiers = current_instructeur + .dossiers + .for_procedure(procedure) + .termine - @all_state_dossiers = procedure.defaut_groupe_instructeur.dossiers.includes(:user).all_state + @all_state_dossiers = current_instructeur + .dossiers + .for_procedure(procedure) + .all_state - @archived_dossiers = procedure.defaut_groupe_instructeur.dossiers.includes(:user).archived + @archived_dossiers = current_instructeur + .dossiers + .for_procedure(procedure) + .archived @dossiers = case statut when 'a-suivre' diff --git a/spec/controllers/instructeurs/procedures_controller_spec.rb b/spec/controllers/instructeurs/procedures_controller_spec.rb index 740d61024..ed52b8781 100644 --- a/spec/controllers/instructeurs/procedures_controller_spec.rb +++ b/spec/controllers/instructeurs/procedures_controller_spec.rb @@ -218,10 +218,14 @@ describe Instructeurs::ProceduresController, type: :controller do describe "#show" 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') } - context "when logged in" do + context "when logged in, and belonging to gi_1, gi_2" do before do sign_in(instructeur.user) + + instructeur.groupe_instructeurs << gi_2 end context "without anything" do @@ -257,6 +261,18 @@ describe Instructeurs::ProceduresController, type: :controller do it { expect(assigns(:termines_dossiers)).to be_empty } it { expect(assigns(:all_state_dossiers)).to match([new_unfollow_dossier]) } it { expect(assigns(:archived_dossiers)).to be_empty } + + context 'and dossiers without follower on each of the others groups' do + let!(:new_unfollow_dossier_on_gi_2) { create(:dossier, groupe_instructeur: gi_2, state: Dossier.states.fetch(:en_instruction)) } + let!(:new_unfollow_dossier_on_gi_3) { create(:dossier, groupe_instructeur: gi_3, state: Dossier.states.fetch(:en_instruction)) } + + before do + get :show, params: { procedure_id: procedure.id } + end + + it { expect(assigns(:a_suivre_dossiers)).to match([new_unfollow_dossier, new_unfollow_dossier_on_gi_2]) } + it { expect(assigns(:all_state_dossiers)).to match([new_unfollow_dossier, new_unfollow_dossier_on_gi_2]) } + end end context 'with a new dossier with a follower' do @@ -272,6 +288,20 @@ describe Instructeurs::ProceduresController, type: :controller do it { expect(assigns(:termines_dossiers)).to be_empty } it { expect(assigns(:all_state_dossiers)).to match([new_followed_dossier]) } it { expect(assigns(:archived_dossiers)).to be_empty } + + context 'and dossier with a follower on each of the others groups' do + let!(:new_follow_dossier_on_gi_2) { create(:dossier, groupe_instructeur: gi_2, state: Dossier.states.fetch(:en_instruction)) } + let!(:new_follow_dossier_on_gi_3) { create(:dossier, groupe_instructeur: gi_3, state: Dossier.states.fetch(:en_instruction)) } + + before do + instructeur.followed_dossiers << new_follow_dossier_on_gi_2 << new_follow_dossier_on_gi_3 + get :show, params: { procedure_id: procedure.id } + end + + # followed dossiers on another groupe should not be displayed + it { expect(assigns(:followed_dossiers)).to contain_exactly(new_followed_dossier, new_follow_dossier_on_gi_2) } + it { expect(assigns(:all_state_dossiers)).to contain_exactly(new_followed_dossier, new_follow_dossier_on_gi_2) } + end end context 'with a termine dossier with a follower' do @@ -286,6 +316,18 @@ describe Instructeurs::ProceduresController, type: :controller do it { expect(assigns(:termines_dossiers)).to match([termine_dossier]) } it { expect(assigns(:all_state_dossiers)).to match([termine_dossier]) } it { expect(assigns(:archived_dossiers)).to be_empty } + + context 'and terminer dossiers on each of the others groups' do + let!(:termine_dossier_on_gi_2) { create(:dossier, groupe_instructeur: gi_2, state: Dossier.states.fetch(:accepte)) } + let!(:termine_dossier_on_gi_3) { create(:dossier, groupe_instructeur: gi_3, state: Dossier.states.fetch(:accepte)) } + + before do + get :show, params: { procedure_id: procedure.id } + end + + it { expect(assigns(:termines_dossiers)).to match([termine_dossier, termine_dossier_on_gi_2]) } + it { expect(assigns(:all_state_dossiers)).to match([termine_dossier, termine_dossier_on_gi_2]) } + end end context 'with an archived dossier' do @@ -300,6 +342,17 @@ describe Instructeurs::ProceduresController, type: :controller do it { expect(assigns(:termines_dossiers)).to be_empty } it { expect(assigns(:all_state_dossiers)).to be_empty } it { expect(assigns(:archived_dossiers)).to match([archived_dossier]) } + + context 'and terminer dossiers on each of the others groups' do + let!(:archived_dossier_on_gi_2) { create(:dossier, groupe_instructeur: gi_2, state: Dossier.states.fetch(:en_instruction), archived: true) } + let!(:archived_dossier_on_gi_3) { create(:dossier, groupe_instructeur: gi_3, state: Dossier.states.fetch(:en_instruction), archived: true) } + + before do + get :show, params: { procedure_id: procedure.id } + end + + it { expect(assigns(:archived_dossiers)).to match([archived_dossier, archived_dossier_on_gi_2]) } + end end describe 'statut' do From e2acb0a946a86f846561cb1bbfa293888bad4c4b Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 18 Sep 2019 15:25:05 +0200 Subject: [PATCH 4/5] ProcedurePresentation: can use groupe instructeur --- app/models/procedure_presentation.rb | 11 +++- spec/models/procedure_presentation_spec.rb | 58 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb index 2eb0f51aa..c4773c0a0 100644 --- a/app/models/procedure_presentation.rb +++ b/app/models/procedure_presentation.rb @@ -19,7 +19,8 @@ class ProcedurePresentation < ApplicationRecord field_hash('En construction le', 'self', 'en_construction_at'), field_hash('Mis à jour le', 'self', 'updated_at'), field_hash('Demandeur', 'user', 'email'), - field_hash('Email instructeur', 'followers_instructeurs', 'email') + field_hash('Email instructeur', 'followers_instructeurs', 'email'), + field_hash('Groupe instructeur', 'groupe_instructeur', 'label') ] if procedure.for_individual @@ -93,7 +94,7 @@ class ProcedurePresentation < ApplicationRecord .where("champs.type_de_champ_id = #{column.to_i}") .order("champs.value #{order}") .pluck(:id) - when 'self', 'user', 'individual', 'etablissement', 'followers_instructeurs' + when 'self', 'user', 'individual', 'etablissement', 'followers_instructeurs', 'groupe_instructeur' return (table == 'self' ? dossiers : dossiers.includes(table)) .order("#{self.class.sanitized_column(table, column)} #{order}") .pluck(:id) @@ -133,6 +134,10 @@ class ProcedurePresentation < ApplicationRecord dossiers .includes(table) .filter_ilike(table, column, values) + when 'groupe_instructeur' + dossiers + .includes(table) + .filter_ilike(table, column, values) end.pluck(:id) end.reduce(:&) end @@ -208,6 +213,8 @@ class ProcedurePresentation < ApplicationRecord dossier.champs.find { |c| c.type_de_champ_id == column.to_i }.value when 'type_de_champ_private' dossier.champs_private.find { |c| c.type_de_champ_id == column.to_i }.value + when 'groupe_instructeur' + dossier.groupe_instructeur.label end end diff --git a/spec/models/procedure_presentation_spec.rb b/spec/models/procedure_presentation_spec.rb index e11d9cd64..6460e7bc5 100644 --- a/spec/models/procedure_presentation_spec.rb +++ b/spec/models/procedure_presentation_spec.rb @@ -62,6 +62,7 @@ describe ProcedurePresentation do { "label" => 'Mis à jour le', "table" => 'self', "column" => 'updated_at' }, { "label" => 'Demandeur', "table" => 'user', "column" => 'email' }, { "label" => 'Email instructeur', "table" => 'followers_instructeurs', "column" => 'email' }, + { "label" => 'Groupe instructeur', "table" => 'groupe_instructeur', "column" => 'label' }, { "label" => 'SIREN', "table" => 'etablissement', "column" => 'entreprise_siren' }, { "label" => 'Forme juridique', "table" => 'etablissement', "column" => 'entreprise_forme_juridique' }, { "label" => 'Nom commercial', "table" => 'etablissement', "column" => 'entreprise_nom_commercial' }, @@ -197,6 +198,15 @@ describe ProcedurePresentation do 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' } @@ -676,6 +686,33 @@ describe ProcedurePresentation do end end end + + context 'for groupe_instructeur table' do + let(:filter) { [{ 'table' => 'groupe_instructeur', 'column' => 'label', 'value' => 'défaut' }] } + + let!(:gi_2) { procedure.groupe_instructeurs.create(label: '2') } + let!(:gi_3) { procedure.groupe_instructeurs.create(label: '3') } + + let!(:kept_dossier) { create(:dossier, procedure: procedure) } + let!(:discarded_dossier) { create(:dossier, groupe_instructeur: gi_2) } + + it { is_expected.to contain_exactly(kept_dossier.id) } + + context 'with multiple search values' do + let(:filter) do + [ + { 'table' => 'groupe_instructeur', 'column' => 'label', 'value' => 'défaut' }, + { 'table' => 'groupe_instructeur', 'column' => 'label', 'value' => '3' } + ] + end + + let!(:other_kept_dossier) { create(:dossier, groupe_instructeur: gi_3) } + + it 'returns every dossier that matches any of the search criteria for a given column' do + is_expected.to contain_exactly(kept_dossier.id, other_kept_dossier.id) + end + end + end end describe '#eager_load_displayed_fields' do @@ -696,6 +733,7 @@ describe ProcedurePresentation do 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 @@ -712,6 +750,7 @@ describe ProcedurePresentation do 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 @@ -726,6 +765,7 @@ describe ProcedurePresentation do 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 @@ -740,6 +780,7 @@ describe ProcedurePresentation do 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 @@ -754,6 +795,7 @@ describe ProcedurePresentation do 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 @@ -768,6 +810,22 @@ describe ProcedurePresentation do 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) { '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)).not_to be_loaded + expect(displayed_dossier.association(:groupe_instructeur)).to be_loaded end end From 6533ec420b80eb9cda7e23989c5ee9e2a2ae8fc9 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 19 Sep 2019 09:13:41 +0200 Subject: [PATCH 5/5] Revert "Upload through proxy" This reverts commit 91815ad7093a00384e50978494249da5a5e18c63. --- .../service/ds_proxy_service.rb | 32 +------------------ 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/app/lib/active_storage/service/ds_proxy_service.rb b/app/lib/active_storage/service/ds_proxy_service.rb index bb516d287..fe540d591 100644 --- a/app/lib/active_storage/service/ds_proxy_service.rb +++ b/app/lib/active_storage/service/ds_proxy_service.rb @@ -1,7 +1,6 @@ module ActiveStorage # Wraps an ActiveStorage::Service to route direct upload and direct download URLs through our proxy, - # thus avoiding exposing the storage provider’s URL to our end-users. It also overrides upload and - # object_for methods to fetch and put blobs through encryption proxy. + # thus avoiding exposing the storage provider’s URL to our end-users. class Service::DsProxyService < SimpleDelegator attr_reader :wrapped @@ -24,37 +23,8 @@ module ActiveStorage publicize(url) end - # This method is responsible for writing to object storage. We directly use direct upload - # url to ensure we upload through encryption proxy. - def upload(key, io, checksum: nil, **) - wrapped.send(:instrument, :upload, key: key, checksum: checksum) do - url = url_for_direct_upload(key, expires_in: 1.hour) - data = Fog::Storage.parse_data(io) - - headers = { 'Content-Type' => wrapped.send(:guess_content_type, io) }.merge(data[:headers]) - if checksum - headers['ETag'] = wrapped.send(:convert_base64digest_to_hexdigest, checksum) - end - - response = Typhoeus::Request.new( - url, - method: :put, - body: data[:body].read, - headers: headers - ).run - - if response.success? - response - else - raise ActiveStorage::IntegrityError - end - end - end - private - # This method is responsible for reading from object storage. We use url method - # on the service to ensure we download through encryption proxy. def object_for(key, &block) blob_url = url(key) if block_given?