Merge pull request #7354 from tchak/fix-types-de-champs-editor

fix(revision): update children coordinate instead of duplicating

app/models/procedure_revision.rb

@@ -44,6 +44,7 @@ class ProcedureRevision < ApplicationRecord
     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)
@@ -71,20 +72,23 @@ 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)
-    type_de_champ = types_de_champ.find_by!(stable_id: 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)
 
-    if type_de_champ.only_present_on_draft?
+    coordinate, tdc = coordinate_and_tdc(stable_id)
-      type_de_champ
+
-    elsif type_de_champ.parent.present?
+    if tdc.only_present_on_draft?
-      find_or_clone_type_de_champ(type_de_champ.parent.stable_id).types_de_champ.find_by!(stable_id: stable_id)
+      tdc
     else
-      revise_type_de_champ(type_de_champ)
+      replace_tdc_and_children_by_clones(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)
@@ -98,6 +102,7 @@ class ProcedureRevision < ApplicationRecord
 
   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)
@@ -413,21 +418,29 @@ class ProcedureRevision < ApplicationRecord
     changes
   end
 
-  def revise_type_de_champ(type_de_champ)
+  def replace_tdc_and_children_by_clones(coordinate)
-    revision_type_de_champ = revision_types_de_champ.find_by!(type_de_champ: type_de_champ)
+    cloned_type_de_champ = coordinate.type_de_champ.deep_clone(include: [:types_de_champ]) do |original, kopy|
-    cloned_type_de_champ = type_de_champ.deep_clone(include: [:types_de_champ]) do |original, kopy|
       PiecesJustificativesService.clone_attachments(original, kopy)
     end
-    revision_type_de_champ.update!(type_de_champ: cloned_type_de_champ)
+    cloned_child_types_de_champ = cloned_type_de_champ.types_de_champ
-    cloned_type_de_champ.types_de_champ.each(&:migrate_parent!)
+    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)
-    type_de_champ = types_de_champ.find_by!(stable_id: stable_id)
+    coordinate, tdc = coordinate_and_tdc(stable_id)
 
-    if type_de_champ.parent_id.present? && type_de_champ.only_present_on_draft?
+    if coordinate.child? && !tdc.only_present_on_draft?
-      find_or_clone_type_de_champ(type_de_champ.parent.stable_id)
+      replace_tdc_and_children_by_clones(coordinate.parent)
     end
   end
 end

app/models/procedure_revision_type_de_champ.rb

@@ -26,6 +26,10 @@ class ProcedureRevisionTypeDeChamp < ApplicationRecord
     type_de_champ.private?
   end
 
+  def child?
+    parent_id.present?
+  end
+
   def siblings
     if parent_id.present?
       revision.revision_types_de_champ.where(parent_id: parent_id).ordered

spec/models/procedure_revision_spec.rb

@@ -275,6 +275,27 @@ describe ProcedureRevision do
     end
   end
 
+  describe '#update_type_de_champ' do
+    let(:procedure) { create(:procedure, :with_repetition) }
+    let(:last_coordinate) { draft.revision_types_de_champ.last }
+    let(:last_type_de_champ) { last_coordinate.type_de_champ }
+
+    context 'bug with duplicated repetition child' do
+      before do
+        procedure.publish!
+        procedure.reload
+        draft.find_or_clone_type_de_champ(last_type_de_champ.stable_id).update(libelle: 'new libelle')
+        procedure.reload
+        draft.reload
+      end
+
+      it do
+        expect(procedure.revisions.size).to eq(2)
+        expect(draft.revision_types_de_champ.where.not(parent_id: nil).size).to eq(1)
+      end
+    end
+  end
+
   describe '#compare' do
     let(:first_tdc) { draft.types_de_champ_public.first }
     let(:new_draft) { procedure.create_new_revision }