Merge pull request #4343 from betagouv/dev

2019-09-19-01

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
+      @a_suivre_dossiers = current_instructeur
-        .defaut_groupe_instructeur
         .dossiers
-        .includes(:user)
+        .for_procedure(procedure)
         .without_followers
         .en_cours
 
       @followed_dossiers = current_instructeur
         .followed_dossiers
-        .includes(:user)
+        .where(groupe_instructeur: current_instructeur.groupe_instructeurs)
-        .where(groupe_instructeur: procedure.defaut_groupe_instructeur)
+        .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'

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
+  # thus avoiding exposing the storage provider’s URL to our end-users.
-  # object_for methods to fetch and put blobs through encryption proxy.
   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?

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

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

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
 

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

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

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)

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