refactor(type_de_champ): type_de_champ should not expose revision or procedure

app/components/procedure/revision_changes_component.rb

@@ -4,6 +4,7 @@ class Procedure::RevisionChangesComponent < ApplicationComponent
   def initialize(new_revision:, previous_revision:)
     @previous_revision = previous_revision
     @new_revision = new_revision
+    @procedure = new_revision.procedure
 
     @tdc_changes = previous_revision.compare_types_de_champ(new_revision)
     @public_move_changes, @private_move_changes = @tdc_changes.filter { _1.op == :move }.partition { !_1.private? }
@@ -14,6 +15,10 @@ class Procedure::RevisionChangesComponent < ApplicationComponent
 
   private
 
+  def used_by_routing_rules?(type_de_champ)
+    @procedure.used_by_routing_rules?(type_de_champ)
+  end
+
   def total_dossiers
     @total_dossiers ||= @previous_revision.dossiers
       .visible_by_administration

app/components/procedure/revision_changes_component/revision_changes_component.html.haml

@@ -77,7 +77,7 @@
           - if !total_dossiers.zero? && !change.can_rebase?
             .fr-alert.fr-alert--warning.fr-mt-1v
               %p= t('.breaking_change', count: total_dossiers)
-          - if (removed.present? || added.present? ) && change.type_de_champ.used_by_routing_rules?
+          - if (removed.present? || added.present? ) && used_by_routing_rules?(change.type_de_champ)
             .fr-alert.fr-alert--warning.fr-mt-1v
               = t(".#{prefix}.update_drop_down_options_alert", label: change.label)
       - when :drop_down_other

app/components/types_de_champ_editor/champ_component.rb

@@ -78,7 +78,7 @@ class TypesDeChampEditor::ChampComponent < ApplicationComponent
   def piece_justificative_template_options
     {
       attached_file: type_de_champ.piece_justificative_template,
-      auto_attach_url: helpers.auto_attach_url(type_de_champ),
+      auto_attach_url: helpers.auto_attach_url(type_de_champ, procedure_id: procedure.id),
       view_as: :download
     }
   end
@@ -86,7 +86,7 @@ class TypesDeChampEditor::ChampComponent < ApplicationComponent
   def notice_explicative_options
     {
       attached_file: type_de_champ.notice_explicative,
-      auto_attach_url: helpers.auto_attach_url(type_de_champ),
+      auto_attach_url: helpers.auto_attach_url(type_de_champ, procedure_id: procedure.id),
       view_as: :download
     }
   end

app/controllers/administrateurs/types_de_champ_controller.rb

@@ -20,15 +20,14 @@ module Administrateurs
 
     def update
       type_de_champ = draft.find_and_ensure_exclusive_use(params[:stable_id])
+      @coordinate = draft.coordinate_for(type_de_champ)
 
-      if type_de_champ.revision_type_de_champ.used_by_routing_rules? && changing_of_type?(type_de_champ)
+      if @coordinate.used_by_routing_rules? && changing_of_type?(type_de_champ)
-        coordinate = draft.coordinate_for(type_de_champ)
         errors = "« #{type_de_champ.libelle} » est utilisé pour le routage, vous ne pouvez pas modifier son type."
-        @morphed = [champ_component_from(coordinate, focused: false, errors:)]
+        @morphed = [champ_component_from(@coordinate, focused: false, errors:)]
         flash.alert = errors
       elsif type_de_champ.update(type_de_champ_update_params)
         reload_procedure_with_includes
-        @coordinate = draft.coordinate_for(type_de_champ)
         @morphed = champ_components_starting_at(@coordinate)
       else
         flash.alert = type_de_champ.errors.full_messages

app/helpers/champ_helper.rb

@@ -9,13 +9,13 @@ module ChampHelper
     simple_format(auto_linked_text, {}, sanitize: false)
   end
 
-  def auto_attach_url(object, params = {})
+  def auto_attach_url(object, procedure_id: nil)
     if object.is_a?(Champ)
-      champs_piece_justificative_url(object.dossier, object.stable_id, params.merge(row_id: object.row_id))
+      champs_piece_justificative_url(object.dossier, object.stable_id, row_id: object.row_id)
     elsif object.is_a?(TypeDeChamp) && object.piece_justificative?
-      piece_justificative_template_admin_procedure_type_de_champ_url(stable_id: object.stable_id, procedure_id: object.procedure.id, **params)
+      piece_justificative_template_admin_procedure_type_de_champ_url(stable_id: object.stable_id, procedure_id:)
     elsif object.is_a?(TypeDeChamp) && object.explication?
-      notice_explicative_admin_procedure_type_de_champ_url(stable_id: object.stable_id, procedure_id: object.procedure.id, **params)
+      notice_explicative_admin_procedure_type_de_champ_url(stable_id: object.stable_id, procedure_id:)
     end
   end
 end

app/models/champ.rb

@@ -74,7 +74,6 @@ class Champ < ApplicationRecord
   delegate :to_typed_id, :to_typed_id_for_query, to: :type_de_champ, prefix: true
 
   delegate :revision, to: :dossier, prefix: true
-  delegate :used_by_routing_rules?, to: :type_de_champ
 
   scope :updated_since?, -> (date) { where('champs.updated_at > ?', date) }
   scope :prefilled, -> { where(prefilled: true) }
@@ -106,6 +105,10 @@ class Champ < ApplicationRecord
     type_de_champ.champ_blank?(self)
   end
 
+  def used_by_routing_rules?
+    procedure.used_by_routing_rules?(type_de_champ)
+  end
+
   def search_terms
     [to_s]
   end

app/models/dubious_procedure.rb

@@ -19,11 +19,11 @@ class DubiousProcedure
   end
 
   def self.all
-    procedures_with_forbidden_tdcs_sql = TypeDeChamp
+    procedures_with_forbidden_tdcs_sql = ProcedureRevisionTypeDeChamp
-      .joins(:procedure)
+      .joins(:procedure, :type_de_champ)
       .select("string_agg(types_de_champ.libelle, ' - ') as dubious_champs, procedures.id as procedure_id, procedures.libelle as procedure_libelle, procedures.aasm_state as procedure_aasm_state, procedures.hidden_at_as_template as procedure_hidden_at_as_template")
       .where("unaccent(types_de_champ.libelle) ~* unaccent(?)", forbidden_regexp)
-      .where(type_champ: [TypeDeChamp.type_champs.fetch(:text), TypeDeChamp.type_champs.fetch(:textarea)])
+      .where(types_de_champ: { type_champ: [TypeDeChamp.type_champs.fetch(:text), TypeDeChamp.type_champs.fetch(:textarea)] })
       .where(procedures: { closed_at: nil, whitelisted_at: nil })
       .group("procedures.id")
       .order("procedures.id asc")

app/models/procedure.rb

@@ -687,7 +687,7 @@ class Procedure < ApplicationRecord
   end
 
   def routing_champs
-    active_revision.types_de_champ_public.filter(&:used_by_routing_rules?).map(&:libelle)
+    active_revision.revision_types_de_champ_public.filter(&:used_by_routing_rules?).map(&:libelle)
   end
 
   def can_be_deleted_by_administrateur?
@@ -829,8 +829,8 @@ class Procedure < ApplicationRecord
     end
   end
 
-  def stable_ids_used_by_routing_rules
+  def used_by_routing_rules?(type_de_champ)
-    @stable_ids_used_by_routing_rules ||= groupe_instructeurs.flat_map { _1.routing_rule&.sources }.compact
+    type_de_champ.stable_id.in?(stable_ids_used_by_routing_rules)
   end
 
   # We need this to unfuck administrate + aasm
@@ -871,6 +871,10 @@ class Procedure < ApplicationRecord
 
   private
 
+  def stable_ids_used_by_routing_rules
+    @stable_ids_used_by_routing_rules ||= groupe_instructeurs.flat_map { _1.routing_rule&.sources }.compact.uniq
+  end
+
   def published_revisions_types_de_champ(parent = nil)
     # all published revisions
     revision_ids = revisions.ids - [draft_revision_id]

app/models/procedure_revision.rb

@@ -230,7 +230,7 @@ class ProcedureRevision < ApplicationRecord
   end
 
   def coordinate_for(tdc)
-    revision_types_de_champ.find_by!(type_de_champ: tdc)
+    revision_types_de_champ.find { _1.stable_id == tdc.stable_id }
   end
 
   def carte?

app/models/procedure_revision_type_de_champ.rb

@@ -75,7 +75,7 @@ class ProcedureRevisionTypeDeChamp < ApplicationRecord
   end
 
   def used_by_routing_rules?
-    stable_id.in?(procedure.stable_ids_used_by_routing_rules)
+    procedure.used_by_routing_rules?(type_de_champ)
   end
 
   def used_by_ineligibilite_rules?

app/models/type_de_champ.rb

@@ -142,13 +142,9 @@ class TypeDeChamp < ApplicationRecord
                  :header_section_level
 
   has_many :revision_types_de_champ, -> { revision_ordered }, class_name: 'ProcedureRevisionTypeDeChamp', dependent: :destroy, inverse_of: :type_de_champ
-  has_one :revision_type_de_champ, -> { revision_ordered }, class_name: 'ProcedureRevisionTypeDeChamp', inverse_of: false
   has_many :revisions, -> { ordered }, through: :revision_types_de_champ
-  has_one :revision, through: :revision_type_de_champ
-  has_one :procedure, through: :revision
 
   delegate :estimated_fill_duration, :estimated_read_duration, :tags_for_template, :libelles_for_export, :libelle_for_export, :primary_options, :secondary_options, :columns, to: :dynamic_type
-  delegate :used_by_routing_rules?, to: :revision_type_de_champ
 
   class WithIndifferentAccess
     def self.load(options)
@@ -213,7 +209,6 @@ class TypeDeChamp < ApplicationRecord
 
   before_save :remove_attachment, if: -> { type_champ_changed? }
   before_validation :set_drop_down_list_options, if: -> { type_champ_changed? }
-  before_save :remove_block, if: -> { type_champ_changed? }
 
   def valid?(context = nil)
     super
@@ -818,14 +813,6 @@ class TypeDeChamp < ApplicationRecord
     end
   end
 
-  def remove_block
-    if !block? && procedure.present?
-      procedure
-        .draft_revision # action occurs only on draft
-        .remove_children_of(self)
-    end
-  end
-
   def normalize_libelle
     self.libelle&.strip!
   end

app/models/types_de_champ/prefill_type_de_champ.rb

@@ -72,7 +72,7 @@ class TypesDeChamp::PrefillTypeDeChamp < SimpleDelegator
 
     link_to(
       I18n.t("views.prefill_descriptions.edit.possible_values.link.text"),
-      Rails.application.routes.url_helpers.prefill_type_de_champ_path(revision.procedure_path, self),
+      Rails.application.routes.url_helpers.prefill_type_de_champ_path(@revision.procedure_path, self),
       title: new_tab_suffix(I18n.t("views.prefill_descriptions.edit.possible_values.link.title")),
       **external_link_attributes
     )

lib/tasks/deployment/20230602165134_migrate_remaining_data_for_routing_with_dropdown_list.rake

@@ -29,7 +29,7 @@ namespace :after_party do
     procedure_ids = Procedure.with_discarded
       .where(routing_enabled: true)
       .where(migrated_champ_routage: [nil, false])
-      .filter { |p| p.active_revision.types_de_champ.none?(&:used_by_routing_rules?) }
+      .filter { |p| p.active_revision.revision_types_de_champ_public.none?(&:used_by_routing_rules?) }
       .filter { |p| p.groupe_instructeurs.active.count > 1 }
       .pluck(:id)
 

spec/components/types_de_champ_editor/champ_component_spec.rb

@@ -16,7 +16,7 @@ describe TypesDeChampEditor::ChampComponent, type: :component do
     describe 'tdc dropdown' do
       let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :drop_down_list, libelle: 'Votre ville', options: ['Paris', 'Lyon', 'Marseille'] }]) }
       let(:tdc) { procedure.draft_revision.types_de_champ.first }
-      let(:coordinate) { tdc.revision_type_de_champ }
+      let(:coordinate) { procedure.draft_revision.coordinate_for(tdc) }
 
       context 'drop down tdc not used for routing' do
         it do

spec/controllers/administrateurs/types_de_champ_controller_spec.rb

@@ -98,7 +98,7 @@ describe Administrateurs::TypesDeChampController, type: :controller do
 
       it do
         is_expected.to have_http_status(:ok)
-        expect(assigns(:coordinate)).to be_nil
+        expect(assigns(:coordinate)).to eq(second_coordinate)
         expect(flash.alert).to eq(["Le champ « Libelle » doit être rempli"])
       end
     end

spec/factories/champ.rb

@@ -190,7 +190,7 @@ FactoryBot.define do
       end
 
       after(:build) do |champ_repetition, evaluator|
-        revision = champ_repetition.type_de_champ.procedure.active_revision
+        revision = champ_repetition.procedure.active_revision
         parent = revision.revision_types_de_champ.find { _1.type_de_champ == champ_repetition.type_de_champ }
         types_de_champ = revision.revision_types_de_champ.filter { _1.parent == parent }.map(&:type_de_champ)
 

spec/models/procedure_revision_spec.rb

@@ -63,7 +63,7 @@ describe ProcedureRevision do
       it do
         expect { subject }.to change { draft.reload.types_de_champ.count }.from(4).to(5)
         expect(draft.children_of(type_de_champ_repetition).last).to eq(subject)
-        expect(draft.children_of(type_de_champ_repetition).map(&:revision_type_de_champ).map(&:position)).to eq([0, 1])
+        expect(draft.children_of(type_de_champ_repetition).map { draft.coordinate_for(_1).position }).to eq([0, 1])
 
         expect(last_coordinate.position).to eq(1)
 

spec/models/procedure_spec.rb

@@ -717,26 +717,26 @@ describe Procedure do
 
       procedure.draft_revision.types_de_champ_public.zip(subject.draft_revision.types_de_champ_public).each do |ptc, stc|
         expect(stc).to have_same_attributes_as(ptc)
-        expect(stc.revision).to eq(subject.draft_revision)
+        expect(stc.revisions).to include(subject.draft_revision)
       end
 
       public_repetition = type_de_champ_repetition
       cloned_public_repetition = subject.draft_revision.types_de_champ_public.repetition.first
       procedure.draft_revision.children_of(public_repetition).zip(subject.draft_revision.children_of(cloned_public_repetition)).each do |ptc, stc|
         expect(stc).to have_same_attributes_as(ptc)
-        expect(stc.revision).to eq(subject.draft_revision)
+        expect(stc.revisions).to include(subject.draft_revision)
       end
 
       procedure.draft_revision.types_de_champ_private.zip(subject.draft_revision.types_de_champ_private).each do |ptc, stc|
         expect(stc).to have_same_attributes_as(ptc)
-        expect(stc.revision).to eq(subject.draft_revision)
+        expect(stc.revisions).to include(subject.draft_revision)
       end
 
       private_repetition = type_de_champ_private_repetition
       cloned_private_repetition = subject.draft_revision.types_de_champ_private.repetition.first
       procedure.draft_revision.children_of(private_repetition).zip(subject.draft_revision.children_of(cloned_private_repetition)).each do |ptc, stc|
         expect(stc).to have_same_attributes_as(ptc)
-        expect(stc.revision).to eq(subject.draft_revision)
+        expect(stc.revisions).to include(subject.draft_revision)
       end
 
       expect(subject.attestation_template.title).to eq(procedure.attestation_template.title)

spec/tasks/maintenance/delete_draft_revision_type_de_champs_task_spec.rb

@@ -51,9 +51,9 @@ module Maintenance
         tdc = find_by_stable_id(11)
         expect(tdc).to be_nil
 
-        tdc = find_by_stable_id(131)
+        tdc, coord = find_with_coordinate_by_stable_id(131)
         expect(tdc).not_to be_nil
-        expect(tdc.revision_type_de_champ.position).to eq(0) # reindexed
+        expect(coord.position).to eq(0) # reindexed
 
         tdc = find_by_stable_id(132)
         expect(tdc).to be_nil
@@ -63,5 +63,10 @@ module Maintenance
     def find_by_stable_id(stable_id)
       procedure.draft_revision.types_de_champ.find { _1.stable_id == stable_id }
     end
+
+    def find_with_coordinate_by_stable_id(stable_id)
+      tdc = find_by_stable_id(stable_id)
+      [tdc, procedure.draft_revision.coordinate_for(tdc)]
+    end
   end
 end

spec/tasks/maintenance/update_draft_revision_type_de_champs_task_spec.rb

@@ -40,31 +40,31 @@ module Maintenance
       it "updates the type de champ" do
         process
 
-        tdc = find_by_stable_id(12)
+        tdc, coord = find_with_coordinate_by_stable_id(12)
-        expect(tdc.revision_type_de_champ.position).to eq(0)
+        expect(coord.position).to eq(0)
         expect(tdc.libelle).to eq("[NEW] Number")
         expect(tdc.description).to eq("[NEW] Number desc")
         expect(tdc.mandatory).to eq(true)
 
-        tdc = find_by_stable_id(13)
+        tdc, coord = find_with_coordinate_by_stable_id(13)
-        expect(tdc.revision_type_de_champ.position).to eq(1)
+        expect(coord.position).to eq(1)
         expect(tdc.libelle).to eq("Bloc")
         expect(tdc.description).to eq("[NEW] bloc desc")
         expect(tdc.mandatory).to eq(false)
 
-        tdc = find_by_stable_id(132)
+        tdc, coord = find_with_coordinate_by_stable_id(132)
-        expect(tdc.revision_type_de_champ.position).to eq(0)
+        expect(coord.position).to eq(0)
         expect(tdc.libelle).to eq("[NEW] RepNum")
         expect(tdc.mandatory).to eq(true)
 
-        tdc = find_by_stable_id(131)
+        tdc, coord = find_with_coordinate_by_stable_id(131)
-        expect(tdc.revision_type_de_champ.position).to eq(1)
+        expect(coord.position).to eq(1)
         expect(tdc.libelle).to eq("[NEW] RepText")
         expect(tdc.description).to eq("")
         expect(tdc.mandatory).to eq(false)
 
-        tdc = find_by_stable_id(11)
+        tdc, coord = find_with_coordinate_by_stable_id(11)
-        expect(tdc.revision_type_de_champ.position).to eq(2)
+        expect(coord.position).to eq(2)
         expect(tdc.libelle).to eq("[supp] Text")
         expect(tdc.mandatory).to eq(false)
       end
@@ -73,5 +73,10 @@ module Maintenance
     def find_by_stable_id(stable_id)
       procedure.draft_revision.types_de_champ.find { _1.stable_id == stable_id }
     end
+
+    def find_with_coordinate_by_stable_id(stable_id)
+      tdc = find_by_stable_id(stable_id)
+      [tdc, procedure.draft_revision.coordinate_for(tdc)]
+    end
   end
 end