Merge pull request #10982 from tchak/refactor-used_by_roules

ETQ dev je ne veux pas que TypeDeChamp expose `revision` et `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/procedures_controller.rb

@@ -307,10 +307,6 @@ module Administrateurs
 
       @procedure.publish_or_reopen!(current_administrateur)
 
-      if @procedure.draft_changed?
-        @procedure.publish_revision!
-      end
-
       if params[:old_procedure].present? && @procedure.errors.empty?
         current_administrateur
           .procedures

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/concerns/addressable_column_concern.rb

@@ -4,7 +4,7 @@ module AddressableColumnConcern
   extend ActiveSupport::Concern
 
   included do
-    def columns(procedure_id:, displayable: true, prefix: nil)
+    def columns(procedure:, displayable: true, prefix: nil)
       super.concat([
         ["code postal (5 chiffres)", '$.postal_code', :text],
         ["commune", '$.city_name', :text],
@@ -12,7 +12,7 @@ module AddressableColumnConcern
         ["region", '$.region_name', :enum]
       ].map do |(label, jsonpath, type)|
         Columns::JSONPathColumn.new(
-          procedure_id:,
+          procedure_id: procedure.id,
           stable_id:,
           tdc_type: type_champ,
           label: "#{libelle_with_prefix(prefix)} – #{label}",

app/models/concerns/columns_concern.rb

@@ -154,7 +154,7 @@ module ColumnsConcern
     end
 
     def types_de_champ_columns
-      all_revisions_types_de_champ.flat_map { _1.columns(procedure_id: id) }
+      all_revisions_types_de_champ.flat_map { _1.columns(procedure: self) }
     end
   end
 end

app/models/concerns/procedure_publish_concern.rb

@@ -0,0 +1,129 @@
+# frozen_string_literal: true
+
+module ProcedurePublishConcern
+  extend ActiveSupport::Concern
+
+  def publish_or_reopen!(administrateur)
+    Procedure.transaction do
+      if brouillon?
+        reset!
+      end
+
+      other_procedure = other_procedure_with_path(path)
+      if other_procedure.present? && administrateur.owns?(other_procedure)
+        other_procedure.unpublish!
+        publish!(other_procedure.canonical_procedure || other_procedure)
+      else
+        publish!
+      end
+    end
+  end
+
+  def publish_revision!
+    reset!
+
+    transaction { publish_new_revision }
+
+    dossiers
+      .state_not_termine
+      .find_each(&:rebase_later)
+  end
+
+  def reset_draft_revision!
+    if published_revision.present? && draft_changed?
+      reset!
+      transaction do
+        draft_revision.types_de_champ.filter(&:only_present_on_draft?).each(&:destroy)
+        draft_revision.update(dossier_submitted_message: nil)
+        draft_revision.destroy
+        update!(draft_revision: create_new_revision(published_revision))
+      end
+    end
+  end
+
+  def reset!
+    if !locked? || draft_changed?
+      dossier_ids_to_destroy = draft_revision.dossiers.ids
+      if dossier_ids_to_destroy.present?
+        Rails.logger.info("Resetting #{dossier_ids_to_destroy.size} dossiers on procedure #{id}: #{dossier_ids_to_destroy}")
+        draft_revision.dossiers.destroy_all
+      end
+    end
+  end
+
+  def before_publish
+    assign_attributes(closed_at: nil, unpublished_at: nil)
+  end
+
+  def after_publish(canonical_procedure = nil)
+    self.canonical_procedure = canonical_procedure
+    touch(:published_at)
+    publish_new_revision
+  end
+
+  def after_republish(canonical_procedure = nil)
+    touch(:published_at)
+  end
+
+  def after_close
+    touch(:closed_at)
+  end
+
+  def after_unpublish
+    touch(:unpublished_at)
+  end
+
+  def create_new_revision(revision = nil)
+    transaction do
+      new_revision = (revision || draft_revision)
+        .deep_clone(include: [:revision_types_de_champ])
+        .tap { |revision| revision.published_at = nil }
+        .tap(&:save!)
+
+      move_new_children_to_new_parent_coordinate(new_revision)
+
+      # they are not aware of the new tdcs
+      new_revision.types_de_champ_public.reset
+      new_revision.types_de_champ_private.reset
+
+      new_revision
+    end
+  end
+
+  private
+
+  def publish_new_revision
+    cleanup_types_de_champ_options!
+    cleanup_types_de_champ_children!
+    self.published_revision = draft_revision
+    self.draft_revision = create_new_revision
+    save!(context: :publication)
+    published_revision.touch(:published_at)
+  end
+
+  def move_new_children_to_new_parent_coordinate(new_draft)
+    children = new_draft.revision_types_de_champ
+      .includes(parent: :type_de_champ)
+      .where.not(parent_id: nil)
+    coordinates_by_stable_id = new_draft.revision_types_de_champ
+      .includes(:type_de_champ)
+      .index_by(&:stable_id)
+
+    children.each do |child|
+      child.update!(parent: coordinates_by_stable_id.fetch(child.parent.stable_id))
+    end
+    new_draft.reload
+  end
+
+  def cleanup_types_de_champ_options!
+    draft_revision.types_de_champ.each do |type_de_champ|
+      type_de_champ.update!(options: type_de_champ.clean_options)
+    end
+  end
+
+  def cleanup_types_de_champ_children!
+    draft_revision.types_de_champ.reject(&:repetition?).each do |type_de_champ|
+      draft_revision.remove_children_of(type_de_champ)
+    end
+  end
+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

@@ -8,6 +8,7 @@ class Procedure < ApplicationRecord
   include ProcedureGroupeInstructeurAPIHackConcern
   include ProcedureSVASVRConcern
   include ProcedureChorusConcern
+  include ProcedurePublishConcern
   include PiecesJointesListConcern
   include ColumnsConcern
 
@@ -329,39 +330,6 @@ class Procedure < ApplicationRecord
     dossiers.close_to_expiration.count
   end
 
-  def publish_or_reopen!(administrateur)
-    Procedure.transaction do
-      if brouillon?
-        reset!
-        cleanup_types_de_champ_options!
-      end
-
-      other_procedure = other_procedure_with_path(path)
-      if other_procedure.present? && administrateur.owns?(other_procedure)
-        other_procedure.unpublish!
-        publish!(other_procedure.canonical_procedure || other_procedure)
-      else
-        publish!
-      end
-    end
-  end
-
-  def reset!
-    if !locked? || draft_changed?
-      dossier_ids_to_destroy = draft_revision.dossiers.ids
-      if dossier_ids_to_destroy.present?
-        Rails.logger.info("Resetting #{dossier_ids_to_destroy.size} dossiers on procedure #{id}: #{dossier_ids_to_destroy}")
-        draft_revision.dossiers.destroy_all
-      end
-    end
-  end
-
-  def cleanup_types_de_champ_options!
-    draft_revision.types_de_champ.each do |type_de_champ|
-      type_de_champ.update!(options: type_de_champ.clean_options)
-    end
-  end
-
   def suggested_path(administrateur)
     if path_customized?
       return path
@@ -719,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?
@@ -770,23 +738,6 @@ class Procedure < ApplicationRecord
     "Procedure;#{id}"
   end
 
-  def create_new_revision(revision = nil)
-    transaction do
-      new_revision = (revision || draft_revision)
-        .deep_clone(include: [:revision_types_de_champ])
-        .tap { |revision| revision.published_at = nil }
-        .tap(&:save!)
-
-      move_new_children_to_new_parent_coordinate(new_revision)
-
-      # they are not aware of the new tdcs
-      new_revision.types_de_champ_public.reset
-      new_revision.types_de_champ_private.reset
-
-      new_revision
-    end
-  end
-
   def average_dossier_weight
     if dossiers.termine.any?
       dossiers_sample = dossiers.termine.limit(100)
@@ -801,32 +752,6 @@ class Procedure < ApplicationRecord
     end
   end
 
-  def publish_revision!
-    reset!
-    cleanup_types_de_champ_options!
-    transaction do
-      self.published_revision = draft_revision
-      self.draft_revision = create_new_revision
-      save!(context: :publication)
-      published_revision.touch(:published_at)
-    end
-    dossiers
-      .state_not_termine
-      .find_each(&:rebase_later)
-  end
-
-  def reset_draft_revision!
-    if published_revision.present? && draft_changed?
-      reset!
-      transaction do
-        draft_revision.types_de_champ.filter(&:only_present_on_draft?).each(&:destroy)
-        draft_revision.update(dossier_submitted_message: nil)
-        draft_revision.destroy
-        update!(draft_revision: create_new_revision(published_revision))
-      end
-    end
-  end
-
   def cnaf_enabled?
     api_particulier_sources['cnaf'].present?
   end
@@ -865,45 +790,6 @@ class Procedure < ApplicationRecord
     end
   end
 
-  def move_new_children_to_new_parent_coordinate(new_draft)
-    children = new_draft.revision_types_de_champ
-      .includes(parent: :type_de_champ)
-      .where.not(parent_id: nil)
-    coordinates_by_stable_id = new_draft.revision_types_de_champ
-      .includes(:type_de_champ)
-      .index_by(&:stable_id)
-
-    children.each do |child|
-      child.update!(parent: coordinates_by_stable_id.fetch(child.parent.stable_id))
-    end
-    new_draft.reload
-  end
-
-  def before_publish
-    assign_attributes(closed_at: nil, unpublished_at: nil)
-  end
-
-  def after_publish(canonical_procedure = nil)
-    self.canonical_procedure = canonical_procedure
-    self.published_revision = draft_revision
-    self.draft_revision = create_new_revision
-    save!(context: :publication)
-    touch(:published_at)
-    published_revision.touch(:published_at)
-  end
-
-  def after_republish(canonical_procedure = nil)
-    touch(:published_at)
-  end
-
-  def after_close
-    touch(:closed_at)
-  end
-
-  def after_unpublish
-    touch(:unpublished_at)
-  end
-
   def update_juridique_required
     self.juridique_required ||= (cadre_juridique.present? || deliberation.attached?)
     true
@@ -943,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
@@ -985,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/carte_type_de_champ.rb

@@ -30,7 +30,7 @@ class TypesDeChamp::CarteTypeDeChamp < TypesDeChamp::TypeDeChampBase
 
   def champ_blank?(champ) = champ.geo_areas.blank?
 
-  def columns(procedure_id:, displayable: true, prefix: nil)
+  def columns(procedure:, displayable: true, prefix: nil)
     []
   end
 end

app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb

@@ -71,10 +71,10 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
       (has_secondary_options_for_primary?(champ) && secondary_value(champ).blank?)
   end
 
-  def columns(procedure_id:, displayable: true, prefix: nil)
+  def columns(procedure:, displayable: true, prefix: nil)
     [
       Columns::LinkedDropDownColumn.new(
-        procedure_id:,
+        procedure_id: procedure.id,
         label: libelle_with_prefix(prefix),
         stable_id:,
         tdc_type: type_champ,
@@ -83,7 +83,7 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
         displayable:
       ),
       Columns::LinkedDropDownColumn.new(
-        procedure_id:,
+        procedure_id: procedure.id,
         stable_id:,
         tdc_type: type_champ,
         label: "#{libelle_with_prefix(prefix)} (Primaire)",
@@ -92,7 +92,7 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
         displayable: false
       ),
       Columns::LinkedDropDownColumn.new(
-        procedure_id:,
+        procedure_id: procedure.id,
         stable_id:,
         tdc_type: type_champ,
         label: "#{libelle_with_prefix(prefix)} (Secondaire)",

app/models/types_de_champ/piece_justificative_type_de_champ.rb

@@ -25,10 +25,10 @@ class TypesDeChamp::PieceJustificativeTypeDeChamp < TypesDeChamp::TypeDeChampBas
 
   def champ_blank?(champ) = champ.piece_justificative_file.blank?
 
-  def columns(procedure_id:, displayable: true, prefix: nil)
+  def columns(procedure:, displayable: true, prefix: nil)
     [
       Columns::AttachedManyColumn.new(
-        procedure_id:,
+        procedure_id: procedure.id,
         stable_id:,
         tdc_type: type_champ,
         label: libelle_with_prefix(prefix),

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
     )

app/models/types_de_champ/repetition_type_de_champ.rb

@@ -25,10 +25,10 @@ class TypesDeChamp::RepetitionTypeDeChamp < TypesDeChamp::TypeDeChampBase
     ActiveStorage::Filename.new(str.delete('[]*?')).sanitized
   end
 
-  def columns(procedure_id:, displayable: nil, prefix: nil)
+  def columns(procedure:, displayable: nil, prefix: nil)
-    @type_de_champ.procedure
+    procedure
       .all_revisions_types_de_champ(parent: @type_de_champ)
-      .flat_map { _1.columns(procedure_id:, displayable: false, prefix: libelle) }
+      .flat_map { _1.columns(procedure:, displayable: false, prefix: libelle) }
   end
 
   def champ_blank?(champ) = champ.dossier.repetition_row_ids(@type_de_champ).blank?

app/models/types_de_champ/titre_identite_type_de_champ.rb

@@ -24,10 +24,10 @@ class TypesDeChamp::TitreIdentiteTypeDeChamp < TypesDeChamp::TypeDeChampBase
 
   def champ_blank?(champ) = champ.piece_justificative_file.blank?
 
-  def columns(procedure_id:, displayable: nil, prefix: nil)
+  def columns(procedure:, displayable: nil, prefix: nil)
     [
       Columns::AttachedManyColumn.new(
-        procedure_id:,
+        procedure_id: procedure.id,
         stable_id:,
         tdc_type: type_champ,
         label: libelle_with_prefix(prefix),

app/models/types_de_champ/type_de_champ_base.rb

@@ -95,11 +95,11 @@ class TypesDeChamp::TypeDeChampBase
   def champ_blank?(champ) = champ.value.blank?
   def champ_blank_or_invalid?(champ) = champ_blank?(champ)
 
-  def columns(procedure_id:, displayable: true, prefix: nil)
+  def columns(procedure:, displayable: true, prefix: nil)
     if fillable?
       [
         Columns::ChampColumn.new(
-          procedure_id:,
+          procedure_id: procedure.id,
           stable_id:,
           tdc_type: type_champ,
           label: libelle_with_prefix(prefix),

app/policies/champ_policy.rb

@@ -1,45 +0,0 @@
-# frozen_string_literal: true
-
-class ChampPolicy < ApplicationPolicy
-  # Scope for WRITING to a champ.
-  #
-  # (If the need for a scope to READ a champ emerges, we can implement another scope
-  # in this file, following this example: https://github.com/varvet/pundit/issues/368#issuecomment-196111115)
-  class Scope < ApplicationScope
-    def resolve
-      if user.blank?
-        return scope.none
-      end
-
-      # The join must be the same for all elements of the WHERE clause.
-      #
-      # NB: here we want to do `.left_outer_joins(dossier: [:invites, { :groupe_instructeur: :instructeurs }]))`,
-      # but for some reasons ActiveRecord <= 5.2 generates bogus SQL. Hence the manual version of it below.
-      joined_scope = scope
-        .joins('LEFT OUTER JOIN dossiers ON dossiers.id = champs.dossier_id')
-        .joins('LEFT OUTER JOIN invites ON invites.dossier_id = dossiers.id OR invites.dossier_id = dossiers.editing_fork_origin_id')
-        .joins('LEFT OUTER JOIN groupe_instructeurs ON groupe_instructeurs.id = dossiers.groupe_instructeur_id')
-        .joins('LEFT OUTER JOIN assign_tos ON assign_tos.groupe_instructeur_id = groupe_instructeurs.id')
-        .joins('LEFT OUTER JOIN instructeurs ON instructeurs.id = assign_tos.instructeur_id')
-
-      # Users can access public champs on their own dossiers.
-      resolved_scope = joined_scope
-        .where('dossiers.user_id': user.id, private: false)
-
-      # Invited users can access public champs on dossiers they are invited to
-      invite_clause = joined_scope
-        .where('invites.user_id': user.id, private: false)
-      resolved_scope = resolved_scope.or(invite_clause)
-
-      if instructeur.present?
-        # Additionnaly, instructeurs can access private champs
-        # on dossiers they are allowed to instruct.
-        instructeur_clause = joined_scope
-          .where('instructeurs.id': instructeur.id, private: true)
-        resolved_scope = resolved_scope.or(instructeur_clause)
-      end
-
-      resolved_scope.or(joined_scope.where('dossiers.for_procedure_preview': true))
-    end
-  end
-end

app/policies/type_de_champ_policy.rb

@@ -1,15 +0,0 @@
-# frozen_string_literal: true
-
-class TypeDeChampPolicy < ApplicationPolicy
-  class Scope < ApplicationScope
-    def resolve
-      if administrateur.present?
-        scope
-          .joins(procedure: [:administrateurs])
-          .where({ administrateurs: { id: administrateur.id } })
-      else
-        scope.none
-      end
-    end
-  end
-end

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/columns/champ_column_spec.rb

@@ -122,7 +122,7 @@ describe Columns::ChampColumn do
   def expect_type_de_champ_values(type, assertion)
     type_de_champ = types_de_champ.find { _1.type_champ == type }
     champ = dossier.send(:filled_champ, type_de_champ, nil)
-    columns = type_de_champ.columns(procedure_id: procedure.id)
+    columns = type_de_champ.columns(procedure:)
     expect(columns.map { _1.value(champ) }).to assertion
   end
 

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/models/type_de_champ_spec.rb

@@ -80,23 +80,6 @@ describe TypeDeChamp do
       end
     end
 
-    describe 'changing the type_champ from a repetition' do
-      let!(:procedure) { create(:procedure) }
-      let(:tdc) { create(:type_de_champ_repetition, :with_types_de_champ, procedure: procedure) }
-
-      before do
-        tdc.update(type_champ: target_type_champ)
-      end
-
-      context 'when the target type_champ is not repetition' do
-        let(:target_type_champ) { TypeDeChamp.type_champs.fetch(:text) }
-
-        it 'removes the children types de champ' do
-          expect(procedure.draft_revision.reload.children_of(tdc)).to be_empty
-        end
-      end
-    end
-
     describe 'changing the type_champ from a drop_down_list' do
       let(:tdc) { create(:type_de_champ_drop_down_list) }
 

spec/policies/champ_policy_spec.rb

@@ -1,100 +0,0 @@
-# frozen_string_literal: true
-
-describe ChampPolicy do
-  let(:procedure) { create(:procedure, :with_type_de_champ, :with_type_de_champ_private) }
-  let(:dossier) { create(:dossier, procedure: procedure, user: dossier_owner) }
-  let(:dossier_owner) { create(:user) }
-
-  let(:signed_in_user) { create(:user) }
-  let(:account) { { user: signed_in_user } }
-
-  subject { Pundit.policy_scope(account, Champ) }
-
-  let(:champ) { dossier.project_champs_public.first }
-  let(:champ_private) { dossier.project_champs_private.first }
-
-  shared_examples_for 'they can access a public champ' do
-    it { expect(subject.find_by(id: champ.id)).to eq(champ) }
-  end
-
-  shared_examples_for 'they can’t access a public champ' do
-    it { expect(subject.find_by(id: champ.id)).to eq(nil) }
-  end
-
-  shared_examples_for 'they can access a private champ' do
-    it { expect(subject.find_by(id: champ_private.id)).to eq(champ_private) }
-  end
-
-  shared_examples_for 'they can’t access a private champ' do
-    it { expect(subject.find_by(id: champ_private.id)).to eq(nil) }
-  end
-
-  context 'when an user only has user rights' do
-    context 'as the dossier owner' do
-      let(:signed_in_user) { dossier_owner }
-
-      it_behaves_like 'they can access a public champ'
-      it_behaves_like 'they can’t access a private champ'
-    end
-
-    context 'as a person invited on the dossier' do
-      let(:invite) { create(:invite, :with_user, dossier: dossier) }
-      let(:signed_in_user) { invite.user }
-
-      it_behaves_like 'they can access a public champ'
-      it_behaves_like 'they can’t access a private champ'
-    end
-
-    context 'as another user' do
-      let(:signed_in_user) { create(:user) }
-
-      it_behaves_like 'they can’t access a public champ'
-      it_behaves_like 'they can’t access a private champ'
-    end
-  end
-
-  context 'when the user also has instruction rights' do
-    let(:instructeur) { create(:instructeur, user: signed_in_user) }
-    let(:account) { { user: signed_in_user, instructeur: instructeur } }
-
-    context 'as the dossier instructeur and owner' do
-      let(:signed_in_user) { dossier_owner }
-      before { instructeur.assign_to_procedure(dossier.procedure) }
-
-      it_behaves_like 'they can access a public champ'
-      it_behaves_like 'they can access a private champ'
-    end
-
-    context 'as the dossier instructeur (but not owner)' do
-      let(:signed_in_user) { create(:user) }
-      before { instructeur.assign_to_procedure(dossier.procedure) }
-
-      it_behaves_like 'they can’t access a public champ'
-      it_behaves_like 'they can access a private champ'
-    end
-
-    context 'as an instructeur not assigned to the procedure' do
-      let(:signed_in_user) { create(:user) }
-
-      it_behaves_like 'they can’t access a public champ'
-      it_behaves_like 'they can’t access a private champ'
-    end
-  end
-
-  context 'when the champ is on a forked dossier' do
-    let(:signed_in_user) { dossier_owner }
-    let(:origin) { create(:dossier, procedure: procedure, user: dossier_owner) }
-    let(:dossier) { origin.find_or_create_editing_fork(dossier_owner) }
-
-    it_behaves_like 'they can access a public champ'
-    it_behaves_like 'they can’t access a private champ'
-
-    context 'when the user is invited on the origin dossier' do
-      let(:invite) { create(:invite, :with_user, dossier: origin) }
-      let(:signed_in_user) { invite.user }
-
-      it_behaves_like 'they can access a public champ'
-      it_behaves_like 'they can’t access a private champ'
-    end
-  end
-end

spec/policies/type_de_champ_policy_spec.rb

@@ -1,32 +0,0 @@
-# frozen_string_literal: true
-
-describe TypeDeChampPolicy do
-  let(:procedure) { create(:procedure) }
-  let!(:type_de_champ) { create(:type_de_champ_text, procedure: procedure) }
-
-  let(:user) { create(:user) }
-  let(:administrateur) { nil }
-
-  let(:account) do
-    {
-      user: user,
-      administrateur: administrateur
-    }.compact
-  end
-
-  subject { Pundit.policy_scope(account, TypeDeChamp) }
-
-  context 'when the user has only user rights' do
-    it 'can not access' do
-      expect(subject.find_by(id: type_de_champ.id)).to eq(nil)
-    end
-  end
-
-  context 'when the user has administrateur rights' do
-    let(:administrateur) { procedure.administrateurs.first }
-
-    it 'can access' do
-      expect(subject.find(type_de_champ.id)).to eq(type_de_champ)
-    end
-  end
-end

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