refactor(revision): remove old code

app/controllers/administrateurs/types_de_champ_controller.rb

@@ -26,7 +26,7 @@ module Administrateurs
     end
 
     def move
-      @procedure.draft_revision.move_type_de_champ(params[:id], (params[:position] || params[:order_place]).to_i)
+      @procedure.draft_revision.move_type_de_champ(params[:id], params[:position].to_i)
 
       head :no_content
     end

app/models/champ.rb

@@ -34,7 +34,6 @@ class Champ < ApplicationRecord
   delegate :libelle,
     :type_champ,
     :procedure,
-    :order_place,
     :mandatory?,
     :description,
     :drop_down_list_options,

app/models/procedure.rb

@@ -165,7 +165,7 @@ class Procedure < ApplicationRecord
     if brouillon?
       draft_types_de_champ
     else
-      TypeDeChamp.root
+      TypeDeChamp
         .public_only
         .fillable
         .joins(:revisions)
@@ -181,7 +181,7 @@ class Procedure < ApplicationRecord
     if brouillon?
       draft_types_de_champ_private
     else
-      TypeDeChamp.root
+      TypeDeChamp
         .private_only
         .fillable
         .joins(:revisions)
@@ -460,12 +460,7 @@ class Procedure < ApplicationRecord
     populate_champ_stable_ids
     include_list = {
       draft_revision: {
-        revision_types_de_champ_public: {
-          type_de_champ: :types_de_champ
-        },
-        revision_types_de_champ_private: {
-          type_de_champ: :types_de_champ
-        },
+        revision_types_de_champ: [:type_de_champ],
         attestation_template: [],
         dossier_submitted_message: []
       }
@@ -515,7 +510,8 @@ class Procedure < ApplicationRecord
     end
 
     procedure.save
-    TypeDeChamp.where(parent: procedure.draft_revision.types_de_champ.repetition).find_each(&:migrate_parent!)
+
+    move_new_children_to_new_parent_coordinate(procedure.draft_revision)
 
     if is_different_admin || from_library
       procedure.draft_types_de_champ.each { |tdc| tdc.options&.delete(:old_pj) }
@@ -736,21 +732,7 @@ class Procedure < ApplicationRecord
       .deep_clone(include: [:revision_types_de_champ])
       .tap(&:save!)
 
-    children = new_draft.revision_types_de_champ.where.not(parent_id: nil)
-    children.each do |child|
-      old_parent = draft_revision.revision_types_de_champ.find(child.parent_id)
-      new_parent = new_draft.revision_types_de_champ.find_by(type_de_champ_id: old_parent.type_de_champ_id)
-      child.update!(parent_id: new_parent.id)
-    end
-
-    new_draft.revision_types_de_champ.reload
-
-    # Some revisions do not have links to children types de champ
-    new_draft
-      .types_de_champ
-      .filter(&:repetition?)
-      .flat_map(&:types_de_champ)
-      .each(&:migrate_parent!)
+    move_new_children_to_new_parent_coordinate(new_draft)
 
     new_draft
   end
@@ -795,6 +777,16 @@ class Procedure < ApplicationRecord
 
   private
 
+  def move_new_children_to_new_parent_coordinate(new_draft)
+    children = new_draft.revision_types_de_champ.where.not(parent_id: nil)
+    children.each do |child|
+      old_parent = draft_revision.revision_types_de_champ.find(child.parent_id)
+      new_parent = new_draft.revision_types_de_champ.find_by(type_de_champ_id: old_parent.type_de_champ_id)
+      child.update!(parent_id: new_parent.id)
+    end
+    new_draft.revision_types_de_champ.reload
+  end
+
   def validate_for_publication?
     validation_context == :publication || publiee?
   end

app/models/procedure_revision.rb

@@ -39,22 +39,15 @@ class ProcedureRevision < ApplicationRecord
   end
 
   def add_type_de_champ(params)
-    parent_stable_id = params[:parent_id]
+    parent_stable_id = params.delete(:parent_id)
 
     coordinate = {}
 
     if parent_stable_id.present?
-      # Ensure that if this is a child, it's parent is cloned to the new revision
-      clone_parent_to_draft_revision(parent_stable_id)
-
       parent_coordinate, parent = coordinate_and_tdc(parent_stable_id)
 
       coordinate[:parent_id] = parent_coordinate.id
       coordinate[:position] = children_of(parent).count
-
-      # old system
-      params[:order_place] = coordinate[:position]
-      params[:parent_id] = parent.id
     elsif params[:private]
       coordinate[:position] = revision_types_de_champ_private.count
     else
@@ -72,25 +65,17 @@ class ProcedureRevision < ApplicationRecord
     TypeDeChamp.new.tap { |tdc| tdc.errors.add(:base, e.message) }
   end
 
-  # Only called by by controller update
   def find_or_clone_type_de_champ(stable_id)
-    # Ensure that if this is a child, it's parent is cloned to the new revision
-    clone_parent_to_draft_revision(stable_id)
-
     coordinate, tdc = coordinate_and_tdc(stable_id)
 
     if tdc.only_present_on_draft?
       tdc
     else
-      replace_tdc_and_children_by_clones(coordinate)
+      replace_type_de_champ_by_clone(coordinate)
     end
   end
 
   def move_type_de_champ(stable_id, position)
-    # Ensure that if this is a child, it's parent is cloned to the new revision
-    # Needed because the order could be based on the ancient system
-    clone_parent_to_draft_revision(stable_id)
-
     coordinate, _ = coordinate_and_tdc(stable_id)
 
     siblings = coordinate.siblings.to_a
@@ -101,17 +86,13 @@ class ProcedureRevision < ApplicationRecord
   end
 
   def remove_type_de_champ(stable_id)
-    # Ensure that if this is a child, it's parent is cloned to the new revision
-    # Needed because the order could be based on the ancient system
-    clone_parent_to_draft_revision(stable_id)
-
     coordinate, tdc = coordinate_and_tdc(stable_id)
 
+    children = children_of(tdc).to_a
     coordinate.destroy
 
-    if tdc.revision_types_de_champ.empty?
-      tdc.destroy
-    end
+    children.each(&:destroy_if_orphan)
+    tdc.destroy_if_orphan
 
     reorder(coordinate.siblings)
   end
@@ -166,6 +147,12 @@ class ProcedureRevision < ApplicationRecord
       .order("procedure_revision_types_de_champ.position")
   end
 
+  def remove_children_of(tdc)
+    children_of(tdc).each do |child|
+      remove_type_de_champ(child.stable_id)
+    end
+  end
+
   def types_de_champ_public_as_json
     types_de_champ = types_de_champ_public.includes(piece_justificative_template_attachment: :blob)
     tdcs_as_json = types_de_champ.map(&:as_json_for_editor)
@@ -216,13 +203,8 @@ class ProcedureRevision < ApplicationRecord
 
   def reorder(siblings)
     siblings.to_a.compact.each.with_index do |sibling, position|
-        sibling.update(position: position)
-
-        # FIXME: to remove when order_place is no longer used
-        if sibling.parent_id.present?
-          sibling.type_de_champ.update!(order_place: position)
-        end
-      end
+      sibling.update(position: position)
+    end
   end
 
   def compare_attestation_template(from_at, to_at)
@@ -451,29 +433,11 @@ class ProcedureRevision < ApplicationRecord
     changes
   end
 
-  def replace_tdc_and_children_by_clones(coordinate)
-    cloned_type_de_champ = coordinate.type_de_champ.deep_clone(include: [:types_de_champ]) do |original, kopy|
+  def replace_type_de_champ_by_clone(coordinate)
+    cloned_type_de_champ = coordinate.type_de_champ.deep_clone do |original, kopy|
       PiecesJustificativesService.clone_attachments(original, kopy)
     end
-    cloned_child_types_de_champ = cloned_type_de_champ.types_de_champ
     coordinate.update!(type_de_champ: cloned_type_de_champ)
-
-    # sync old and new system
-    children_coordinates = revision_types_de_champ.where(parent: coordinate)
-
-    children_coordinates.find_each do |child_coordinate|
-      cloned_child_type_de_champ = cloned_child_types_de_champ.find { |tdc| tdc.stable_id == child_coordinate.type_de_champ.stable_id }
-      child_coordinate.update!(type_de_champ: cloned_child_type_de_champ)
-    end
-
     cloned_type_de_champ
   end
-
-  def clone_parent_to_draft_revision(stable_id)
-    coordinate, tdc = coordinate_and_tdc(stable_id)
-
-    if coordinate.child? && !tdc.only_present_on_draft?
-      replace_tdc_and_children_by_clones(coordinate.parent)
-    end
-  end
 end

app/models/type_de_champ.rb

@@ -7,16 +7,14 @@
 #  libelle     :string
 #  mandatory   :boolean          default(FALSE)
 #  options     :jsonb
-#  order_place :integer
 #  private     :boolean          default(FALSE), not null
 #  type_champ  :string
 #  created_at  :datetime
 #  updated_at  :datetime
-#  parent_id   :bigint
 #  stable_id   :bigint
 #
 class TypeDeChamp < ApplicationRecord
-  self.ignored_columns = [:migrated_parent, :revision_id]
+  self.ignored_columns = [:migrated_parent, :revision_id, :parent_id, :order_place]
 
   enum type_champs: {
     text: 'text',
@@ -56,9 +54,6 @@ class TypeDeChamp < ApplicationRecord
     mesri: 'mesri'
   }
 
-  belongs_to :parent, class_name: 'TypeDeChamp', optional: true
-  has_many :types_de_champ, -> { ordered }, foreign_key: :parent_id, class_name: 'TypeDeChamp', inverse_of: :parent, dependent: :destroy
-
   store_accessor :options, :cadastres, :old_pj, :drop_down_options, :skip_pj_validation, :skip_content_type_pj_validation, :drop_down_secondary_libelle, :drop_down_secondary_description, :drop_down_other
   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
@@ -87,8 +82,6 @@ class TypeDeChamp < ApplicationRecord
 
   scope :public_only, -> { where(private: false) }
   scope :private_only, -> { where(private: true) }
-  scope :ordered, -> { order(order_place: :asc) }
-  scope :root, -> { where(parent_id: nil) }
   scope :repetition, -> { where(type_champ: type_champs.fetch(:repetition)) }
   scope :not_repetition, -> { where.not(type_champ: type_champs.fetch(:repetition)) }
   scope :fillable, -> { where.not(type_champ: [type_champs.fetch(:header_section), type_champs.fetch(:explication)]) }
@@ -112,8 +105,6 @@ class TypeDeChamp < ApplicationRecord
 
   has_one_attached :piece_justificative_template
 
-  accepts_nested_attributes_for :types_de_champ, reject_if: proc { |attributes| attributes['libelle'].blank? }, allow_destroy: true
-
   validates :libelle, presence: true, allow_blank: false, allow_nil: false
   validates :type_champ, presence: true, allow_blank: false, allow_nil: false
 
@@ -344,8 +335,6 @@ class TypeDeChamp < ApplicationRecord
       except: [
         :created_at,
         :options,
-        :order_place,
-        :parent_id,
         :private,
         :stable_id,
         :type,
@@ -371,16 +360,10 @@ class TypeDeChamp < ApplicationRecord
     end
   end
 
-  def migrate_parent!
-    if parent_id.present? && revision_types_de_champ.empty?
-      parent.revision_types_de_champ.each do |revision_type_de_champ|
-        ProcedureRevisionTypeDeChamp.create(parent: revision_type_de_champ,
-          type_de_champ: self,
-          revision_id: revision_type_de_champ.revision_id,
-          position: order_place)
-      end
+  def destroy_if_orphan
+    if revision_types_de_champ.empty?
+      destroy
     end
-    self
   end
 
   private
@@ -413,8 +396,7 @@ class TypeDeChamp < ApplicationRecord
     if !repetition? && procedure.present?
       procedure
         .draft_revision # action occurs only on draft
-        .children_of(self)
-        .destroy_all
+        .remove_children_of(self)
     end
   end
 end

app/models/types_de_champ/repetition_type_de_champ.rb

@@ -8,8 +8,8 @@ class TypesDeChamp::RepetitionTypeDeChamp < TypesDeChamp::TypeDeChampBase
 
   def estimated_fill_duration(revision)
     estimated_rows_in_repetition = 2.5
-    estimated_row_duration = @type_de_champ
-      .types_de_champ
+    estimated_row_duration = revision
+      .children_of(@type_de_champ)
       .map { |child_tdc| child_tdc.estimated_fill_duration(revision) }
       .sum
     estimated_row_duration * estimated_rows_in_repetition

app/serializers/type_de_champ_serializer.rb

@@ -8,4 +8,8 @@ class TypeDeChampSerializer < ActiveModel::Serializer
   def id
     object.stable_id || object.id
   end
+
+  def order_place
+    -1
+  end
 end

app/views/fields/types_de_champ_collection_field/_show.html.haml

@@ -8,7 +8,7 @@
         %td.cell-label Type de champ
         %td.cell-label Modifier le modèle
     %tbody
-      - field.data.order(:order_place).each do |type_de_champ|
+      - field.data.each do |type_de_champ|
         = render partial: 'fields/types_de_champ_collection_field/type_champ_line',
           locals: { type_de_champ: type_de_champ }