Merge pull request #7295 from tchak/rollback-attempt

rollback type_de_champ revision changes

app/models/procedure_revision.rb

@@ -66,31 +66,35 @@ class ProcedureRevision < ApplicationRecord
   end
 
   def move_type_de_champ(stable_id, position)
-    coordinate = revision_types_de_champ
+    type_de_champ = find_type_de_champ_by_stable_id(stable_id)
-      .joins(:type_de_champ)
-      .find_by(type_de_champ: { stable_id: stable_id })
 
-    siblings = coordinate.siblings.to_a
+    if type_de_champ.parent.present?
+      repetition_type_de_champ = find_or_clone_type_de_champ(stable_id).parent
 
-    siblings.insert(position, siblings.delete_at(siblings.index(coordinate)))
+      move_type_de_champ_hash(repetition_type_de_champ.types_de_champ.to_a, type_de_champ, position).each do |(id, position)|
+        type_de_champ = repetition_type_de_champ.types_de_champ.find(id)
+        type_de_champ.update!(order_place: position)
+        type_de_champ.revision_type_de_champ&.update!(position: position)
+      end
+    else
+      liste = type_de_champ.private? ? types_de_champ_private : types_de_champ_public
 
-    reorder(siblings)
+      move_type_de_champ_hash(liste.to_a, type_de_champ, position).each do |(id, position)|
+        revision_types_de_champ.find_by!(type_de_champ_id: id).update!(position: position)
+      end
+    end
   end
 
   def remove_type_de_champ(stable_id)
-    coordinate = revision_types_de_champ
+    type_de_champ = find_type_de_champ_by_stable_id(stable_id)
-      .joins(:type_de_champ)
-      .find_by(type_de_champ: { stable_id: stable_id })
 
-    tdc = coordinate.type_de_champ
+    if type_de_champ.only_present_on_draft?
-
+      type_de_champ.destroy
-    coordinate.destroy
+    elsif type_de_champ.parent.present?
-
+      find_or_clone_type_de_champ(stable_id).destroy
-    if tdc.revision_types_de_champ.empty?
+    else
-      tdc.destroy
+      types_de_champ.delete(type_de_champ)
     end
-
-    reorder(coordinate.siblings)
   end
 
   def draft?
@@ -146,17 +150,6 @@ class ProcedureRevision < ApplicationRecord
 
   private
 
-  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
-  end
-
   def compare_attestation_template(from_at, to_at)
     changes = []
     if from_at.nil? && to_at.present?

app/models/procedure_revision_type_de_champ.rb

@@ -28,16 +28,6 @@ class ProcedureRevisionTypeDeChamp < ApplicationRecord
     type_de_champ.private?
   end
 
-  def siblings
-    if parent_id.present?
-      revision.revision_types_de_champ.where(parent_id: parent_id).ordered
-    elsif private?
-      revision.revision_types_de_champ_private
-    else
-      revision.revision_types_de_champ_public
-    end
-  end
-
   private
 
   def set_position

spec/models/dossier_spec.rb

@@ -1453,7 +1453,7 @@ describe Dossier do
       it "should have champs from all revisions" do
         expect(dossier.types_de_champ.map(&:libelle)).to eq([text_type_de_champ.libelle, datetime_type_de_champ.libelle, "Yes/no", explication_type_de_champ.libelle, commune_type_de_champ.libelle, repetition_type_de_champ.libelle])
         expect(dossier_second_revision.types_de_champ.map(&:libelle)).to eq([datetime_type_de_champ.libelle, "Updated yes/no", explication_type_de_champ.libelle, 'Commune de naissance', "Repetition", "New text field"])
-        expect(dossier_champs_for_export.map { |(libelle)| libelle }).to eq([datetime_type_de_champ.libelle, text_type_de_champ.libelle, "Updated yes/no", "Commune de naissance", "Commune de naissance (Code insee)", "New text field"])
+        expect(dossier_champs_for_export.map { |(libelle)| libelle }).to eq([text_type_de_champ.libelle, datetime_type_de_champ.libelle, "Updated yes/no", "Commune de naissance", "Commune de naissance (Code insee)", "New text field"])
         expect(dossier_champs_for_export).to eq(dossier_second_revision_champs_for_export)
         expect(repetition_second_revision_champs_for_export.map { |(libelle)| libelle }).to eq(procedure.types_de_champ_for_procedure_presentation.repetition.map(&:libelle_for_export))
         expect(repetition_second_revision_champs_for_export.first.size).to eq(2)

spec/models/procedure_presentation_and_revisions_spec.rb

@@ -41,7 +41,7 @@ describe ProcedurePresentation do
 
         before do
           created_tdc0 = procedure.draft_revision.add_type_de_champ(tdc0)
-          procedure.draft_revision.reload.move_type_de_champ(created_tdc0.stable_id, 0)
+          procedure.draft_revision.move_type_de_champ(created_tdc0.stable_id, 0)
           procedure.publish_revision!
         end
 

spec/models/procedure_revision_spec.rb

@@ -53,21 +53,21 @@ describe ProcedureRevision do
     context 'with 4 types de champ publiques' do
       it 'move down' do
         expect(draft.types_de_champ_public.index(type_de_champ_public)).to eq(0)
-
+        type_de_champ_public.update(order_place: nil)
         draft.move_type_de_champ(type_de_champ_public.stable_id, 2)
         draft.reload
-
         expect(draft.types_de_champ_public.index(type_de_champ_public)).to eq(2)
+        expect(draft.procedure.types_de_champ.index(type_de_champ_public)).to eq(2)
         expect(draft.procedure.types_de_champ_for_procedure_presentation.not_repetition.index(type_de_champ_public)).to eq(2)
       end
 
       it 'move up' do
         expect(draft.types_de_champ_public.index(last_type_de_champ)).to eq(3)
-
+        last_type_de_champ.update(order_place: nil)
         draft.move_type_de_champ(last_type_de_champ.stable_id, 0)
         draft.reload
-
         expect(draft.types_de_champ_public.index(last_type_de_champ)).to eq(0)
+        expect(draft.procedure.types_de_champ.index(last_type_de_champ)).to eq(0)
         expect(draft.procedure.types_de_champ_for_procedure_presentation.not_repetition.index(last_type_de_champ)).to eq(0)
       end
     end
@@ -92,19 +92,17 @@ describe ProcedureRevision do
       end
 
       it 'move down' do
-        expect(draft.children_of(type_de_champ_repetition).index(second_child)).to eq(1)
+        expect(type_de_champ_repetition.types_de_champ.index(second_child)).to eq(1)
-
         draft.move_type_de_champ(second_child.stable_id, 2)
-
+        type_de_champ_repetition.reload
-        expect(draft.children_of(type_de_champ_repetition).index(second_child)).to eq(2)
+        expect(type_de_champ_repetition.types_de_champ.index(second_child)).to eq(2)
       end
 
       it 'move up' do
-        expect(draft.children_of(type_de_champ_repetition).index(last_child)).to eq(2)
+        expect(type_de_champ_repetition.types_de_champ.index(last_child)).to eq(2)
-
         draft.move_type_de_champ(last_child.stable_id, 0)
-
+        type_de_champ_repetition.reload
-        expect(draft.children_of(type_de_champ_repetition).index(last_child)).to eq(0)
+        expect(type_de_champ_repetition.types_de_champ.index(last_child)).to eq(0)
       end
     end
   end
@@ -126,92 +124,15 @@ describe ProcedureRevision do
       end
     end
 
-    context 'with multiple tdc' do
-      context 'in public tdc' do
-        let(:procedure) { create(:procedure, :with_type_de_champ, types_de_champ_count: 3) }
-
-        it 'reorders' do
-          expect(draft.revision_types_de_champ_public.pluck(:position)).to eq([0, 1, 2])
-
-          draft.remove_type_de_champ(draft.types_de_champ_public[1].stable_id)
-
-          expect(draft.revision_types_de_champ_public.pluck(:position)).to eq([0, 1])
-        end
-      end
-
-      context 'in repetition tdc' do
-        let(:procedure) { create(:procedure, :with_repetition) }
-        let!(:second_child) do
-          draft.add_type_de_champ({
-            type_champ: TypeDeChamp.type_champs.fetch(:text),
-            libelle: "second child",
-            parent_id: type_de_champ_repetition.stable_id
-          })
-        end
-
-        let!(:last_child) do
-          draft.add_type_de_champ({
-            type_champ: TypeDeChamp.type_champs.fetch(:text),
-            libelle: "last child",
-            parent_id: type_de_champ_repetition.stable_id
-          })
-        end
-
-        it 'reorders' do
-          children = draft.children_of(type_de_champ_repetition)
-          expect(children.pluck(:position)).to eq([0, 1, 2])
-          expect(children.pluck(:order_place)).to eq([0, 1, 2])
-
-          draft.remove_type_de_champ(children[1].stable_id)
-
-          children.reload
-
-          expect(children.pluck(:position)).to eq([0, 1])
-          expect(children.pluck(:order_place)).to eq([0, 1])
-        end
-      end
-    end
-
     context 'for a type_de_champ_repetition' do
       let(:procedure) { create(:procedure, :with_repetition) }
-      let!(:child) { child = draft.children_of(type_de_champ_repetition).first }
 
       it 'can remove its children' do
-        draft.remove_type_de_champ(child.stable_id)
+        draft.remove_type_de_champ(type_de_champ_repetition.types_de_champ.first.stable_id)
 
         expect(type_de_champ_repetition.types_de_champ).to be_empty
-        expect { child.reload }.to raise_error ActiveRecord::RecordNotFound
         expect(draft.types_de_champ_public.size).to eq(1)
       end
-
-      it 'can remove the parent' do
-        draft.remove_type_de_champ(type_de_champ_repetition.stable_id)
-
-        expect { child.reload }.to raise_error ActiveRecord::RecordNotFound
-        expect { type_de_champ_repetition.reload }.to raise_error ActiveRecord::RecordNotFound
-        expect(draft.types_de_champ_public).to be_empty
-      end
-
-      context 'when there already is a revision with this child' do
-        let!(:new_draft) { procedure.create_new_revision }
-
-        it 'can remove its children only in the new revision' do
-          new_draft.remove_type_de_champ(child.stable_id)
-
-          expect { child.reload }.not_to raise_error
-          expect(draft.children_of(type_de_champ_repetition).size).to eq(1)
-          expect(new_draft.children_of(type_de_champ_repetition)).to be_empty
-        end
-
-        it 'can remove the parent only in the new revision' do
-          new_draft.remove_type_de_champ(type_de_champ_repetition.stable_id)
-
-          expect { child.reload }.not_to raise_error
-          expect { type_de_champ_repetition.reload }.not_to raise_error
-          expect(draft.types_de_champ_public.size).to eq(1)
-          expect(new_draft.types_de_champ_public).to be_empty
-        end
-      end
     end
   end