refactor(pj_list): extract pj list in a concern and simplify

app/models/concerns/pieces_jointes_list_concern.rb

@@ -0,0 +1,52 @@
+module PiecesJointesListConcern
+  extend ActiveSupport::Concern
+
+  included do
+    def pieces_jointes_list?
+      pieces_jointes_list(public_only: true)
+    end
+
+    def pieces_jointes_list_without_conditionnal
+      pieces_jointes_list(public_only: true, wrap_with_parent: true, without_conditional: true)
+    end
+
+    def pieces_jointes_list_with_conditionnal
+      pieces_jointes_list(public_only: true, wrap_with_parent: true, conditional_only: true)
+    end
+
+    def pieces_jointes_exportables_list
+      pieces_jointes_list(with_titre_identite: false)
+    end
+
+    def pieces_jointes_list(
+      with_titre_identite: true,
+      public_only: false,
+      without_conditional: false,
+      conditional_only: false,
+      wrap_with_parent: false
+    )
+      coordinates =  active_revision.revision_types_de_champ
+        .includes(:type_de_champ, revision_types_de_champ: :type_de_champ)
+
+      coordinates = coordinates.public_only if public_only
+
+      type_champ = ['piece_justificative']
+      type_champ << 'titre_identite' if with_titre_identite
+
+      coordinates = coordinates.where(types_de_champ: { type_champ: })
+
+      if conditional_only
+        coordinates = coordinates.where.not(types_de_champ: { condition: nil })
+      elsif without_conditional
+        coordinates = coordinates.where(types_de_champ: { condition: nil })
+      end
+
+      return coordinates.map(&:type_de_champ) if !wrap_with_parent
+
+      # we want pj in the form of [[pj1], [pj2, repetition], [pj3, repetition]]
+      coordinates
+        .map { |c| c.child? ? [c, c.parent] : [c] }
+        .map { _1.map(&:type_de_champ) }
+    end
+  end
+end

app/models/procedure.rb

@@ -5,6 +5,7 @@ class Procedure < ApplicationRecord
   include ProcedureGroupeInstructeurAPIHackConcern
   include ProcedureSVASVRConcern
   include ProcedureChorusConcern
+  include PiecesJointesListConcern
 
   include Discard::Model
   self.discard_column = :hidden_at
@@ -981,28 +982,6 @@ def self.human_attribute_name(attribute, options = {})
     end
   end
 
-  def pieces_jointes_list?
-    pieces_jointes_list_without_conditionnal.present? || pieces_jointes_list_with_conditionnal.present?
-  end
-
-  def pieces_jointes_list_without_conditionnal
-    pieces_jointes_list do |base_scope|
-      base_scope.where(types_de_champ: { condition: nil })
-    end
-  end
-
-  def pieces_jointes_exportables_list
-    pieces_jointes_list(with_private: true, with_titre_identite: false, with_repetition_parent: false) do |base_scope|
-      base_scope
-    end.flatten
-  end
-
-  def pieces_jointes_list_with_conditionnal
-    pieces_jointes_list do |base_scope|
-      base_scope.where.not(types_de_champ: { condition: nil })
-    end
-  end
-
   def toggle_routing
     update!(routing_enabled: self.groupe_instructeurs.active.many?)
   end
@@ -1030,34 +1009,6 @@ def reset_closing_params
 
   private
 
-  def pieces_jointes_list(with_private: false, with_titre_identite: true, with_repetition_parent: true)
-    types_de_champ = with_private ?
-      active_revision.revision_types_de_champ_private_and_public :
-      active_revision.revision_types_de_champ_public
-
-    type_champs = ['repetition', 'piece_justificative']
-    type_champs << 'titre_identite' if with_titre_identite
-
-    scope = yield types_de_champ
-      .includes(:type_de_champ, revision_types_de_champ: :type_de_champ)
-      .where(types_de_champ: { type_champ: [type_champs] })
-
-    scope.each_with_object([]) do |rtdc, list|
-      if rtdc.type_de_champ.repetition?
-        rtdc.revision_types_de_champ.each do |rtdc_in_repetition|
-          if rtdc_in_repetition.type_de_champ.piece_justificative?
-            to_add = []
-            to_add << rtdc_in_repetition.type_de_champ
-            to_add << rtdc.type_de_champ if with_repetition_parent
-            list << to_add
-          end
-        end
-      else
-        list << [rtdc.type_de_champ]
-      end
-    end
-  end
-
   def validate_auto_archive_on_in_the_future
     return if auto_archive_on.nil?
     return if auto_archive_on.future?

app/models/procedure_revision.rb

@@ -8,7 +8,6 @@ class ProcedureRevision < ApplicationRecord
   has_many :revision_types_de_champ, -> { order(:position, :id) }, class_name: 'ProcedureRevisionTypeDeChamp', foreign_key: :revision_id, dependent: :destroy, inverse_of: :revision
   has_many :revision_types_de_champ_public, -> { root.public_only.ordered }, class_name: 'ProcedureRevisionTypeDeChamp', foreign_key: :revision_id, dependent: :destroy, inverse_of: :revision
   has_many :revision_types_de_champ_private, -> { root.private_only.ordered }, class_name: 'ProcedureRevisionTypeDeChamp', foreign_key: :revision_id, dependent: :destroy, inverse_of: :revision
-  has_many :revision_types_de_champ_private_and_public, -> { root.ordered }, class_name: 'ProcedureRevisionTypeDeChamp', foreign_key: :revision_id, dependent: :destroy, inverse_of: :revision
   has_many :types_de_champ, through: :revision_types_de_champ, source: :type_de_champ
   has_many :types_de_champ_public, through: :revision_types_de_champ_public, source: :type_de_champ
   has_many :types_de_champ_private, through: :revision_types_de_champ_private, source: :type_de_champ

spec/models/concerns/pieces_jointes_list_concern_spec.rb

@@ -0,0 +1,50 @@
+describe PiecesJointesListConcern do
+  describe '#pieces_jointes_list' do
+    include Logic
+    let(:procedure) { create(:procedure, types_de_champ_public:, types_de_champ_private:) }
+    let(:types_de_champ_public) do
+      [
+        { type: :integer_number, stable_id: 900 },
+        { type: :piece_justificative, libelle: "pj1", stable_id: 910 },
+        { type: :piece_justificative, libelle: "pj-cond", stable_id: 911, condition: ds_eq(champ_value(900), constant(1)) },
+        { type: :repetition, libelle: "Répétition", stable_id: 920, children: [{ type: :piece_justificative, libelle: "pj2", stable_id: 921 }] },
+        { type: :titre_identite, libelle: "pj3", stable_id: 930 }
+      ]
+    end
+
+    let(:types_de_champ_private) do
+      [
+        { type: :integer_number, stable_id: 950 },
+        { type: :piece_justificative, libelle: "pj5", stable_id: 960 },
+        { type: :piece_justificative, libelle: "pj-cond2", stable_id: 961, condition: ds_eq(champ_value(900), constant(1)) },
+        { type: :repetition, libelle: "Répétition2", stable_id: 970, children: [{ type: :piece_justificative, libelle: "pj6", stable_id: 971 }] }
+      ]
+    end
+
+    let(:types_de_champ) { procedure.active_revision.types_de_champ }
+    def find_by_stable_id(stable_id) = types_de_champ.find { _1.stable_id == stable_id }
+
+    let(:pj1) { find_by_stable_id(910) }
+    let(:pjcond) { find_by_stable_id(911) }
+    let(:repetition) { find_by_stable_id(920) }
+    let(:pj2) { find_by_stable_id(921) }
+    let(:pj3) { find_by_stable_id(930) }
+
+    let(:pj5) { find_by_stable_id(960) }
+    let(:pjcond2) { find_by_stable_id(961) }
+    let(:repetition2) { find_by_stable_id(970) }
+    let(:pj6) { find_by_stable_id(971) }
+
+    it "returns the list of pieces jointes without conditional" do
+      expect(procedure.pieces_jointes_list_without_conditionnal).to match_array([[pj1], [pj2, repetition], [pj3]])
+    end
+
+    it "returns the list of pieces jointes having conditional" do
+      expect(procedure.pieces_jointes_list_with_conditionnal).to match_array([[pjcond]])
+    end
+
+    it "returns the list of pieces jointes with private, without parent repetition, without titre identite" do
+      expect(procedure.pieces_jointes_exportables_list.map(&:libelle)).to match_array([pj1, pj2, pjcond, pj5, pjcond2, pj6].map(&:libelle))
+    end
+  end
+end

spec/models/procedure_spec.rb

@@ -1741,52 +1741,6 @@
     end
   end
 
-  describe '#pieces_jointes_list' do
-    include Logic
-    let(:procedure) { create(:procedure, types_de_champ_public:, types_de_champ_private:) }
-    let(:types_de_champ_public) do
-      [
-        { type: :integer_number, stable_id: 900 },
-        { type: :piece_justificative, libelle: "PJ", mandatory: true, stable_id: 910 },
-        { type: :piece_justificative, libelle: "PJ-cond", mandatory: true, stable_id: 911, condition: ds_eq(champ_value(900), constant(1)) },
-        { type: :repetition, libelle: "Répétition", stable_id: 920, children: [{ type: :piece_justificative, libelle: "PJ2", stable_id: 921 }] },
-        { type: :titre_identite, libelle: "CNI", mandatory: true, stable_id: 930 }
-      ]
-    end
-
-    let(:types_de_champ_private) do
-      [
-        { type: :integer_number, stable_id: 950 },
-        { type: :piece_justificative, libelle: "PJ", mandatory: true, stable_id: 960 },
-        { type: :piece_justificative, libelle: "PJ-cond", mandatory: true, stable_id: 961, condition: ds_eq(champ_value(900), constant(1)) },
-        { type: :repetition, libelle: "Répétition", stable_id: 970, children: [{ type: :piece_justificative, libelle: "PJ2", stable_id: 971 }] }
-      ]
-    end
-
-    let(:pj1) { procedure.active_revision.types_de_champ.find { _1.stable_id == 910 } }
-    let(:pjcond) { procedure.active_revision.types_de_champ.find { _1.stable_id == 911 } }
-    let(:repetition) { procedure.active_revision.types_de_champ.find { _1.stable_id == 920 } }
-    let(:pj2) { procedure.active_revision.types_de_champ.find { _1.stable_id == 921 } }
-    let(:pj3) { procedure.active_revision.types_de_champ.find { _1.stable_id == 930 } }
-
-    let(:pj5) { procedure.active_revision.types_de_champ.find { _1.stable_id == 960 } }
-    let(:pjcond2) { procedure.active_revision.types_de_champ.find { _1.stable_id == 961 } }
-    let(:repetition2) { procedure.active_revision.types_de_champ.find { _1.stable_id == 970 } }
-    let(:pj6) { procedure.active_revision.types_de_champ.find { _1.stable_id == 971 } }
-
-    it "returns the list of pieces jointes without conditional" do
-      expect(procedure.pieces_jointes_list_without_conditionnal).to match_array([[pj1], [pj2, repetition], [pj3]])
-    end
-
-    it "returns the list of pieces jointes having conditional" do
-      expect(procedure.pieces_jointes_list_with_conditionnal).to match_array([[pjcond]])
-    end
-
-    it "returns the list of pieces jointes with private, without parent repetition, without titre identite" do
-      expect(procedure.pieces_jointes_exportables_list).to match_array([pj1, pj2, pjcond, pj5, pjcond2, pj6])
-    end
-  end
-
   describe "#attestation_template" do
     let(:procedure) { create(:procedure) }