diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index 37887fe17..2dfc57708 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -66,35 +66,31 @@ class ProcedureRevision < ApplicationRecord end def move_type_de_champ(stable_id, position) - type_de_champ = find_type_de_champ_by_stable_id(stable_id) + coordinate = revision_types_de_champ + .joins(:type_de_champ) + .find_by(type_de_champ: { stable_id: stable_id }) - if type_de_champ.parent.present? - repetition_type_de_champ = find_or_clone_type_de_champ(stable_id).parent + siblings = coordinate.siblings.to_a - 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 + siblings.insert(position, siblings.delete_at(siblings.index(coordinate))) - 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 + reorder(siblings) end def remove_type_de_champ(stable_id) - type_de_champ = find_type_de_champ_by_stable_id(stable_id) + coordinate = revision_types_de_champ + .joins(:type_de_champ) + .find_by(type_de_champ: { stable_id: stable_id }) - if type_de_champ.only_present_on_draft? - type_de_champ.destroy - elsif type_de_champ.parent.present? - find_or_clone_type_de_champ(stable_id).destroy - else - types_de_champ.delete(type_de_champ) + tdc = coordinate.type_de_champ + + coordinate.destroy + + if tdc.revision_types_de_champ.empty? + tdc.destroy end + + reorder(coordinate.siblings) end def draft? @@ -150,6 +146,17 @@ 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? diff --git a/app/models/procedure_revision_type_de_champ.rb b/app/models/procedure_revision_type_de_champ.rb index 5007c6a69..3772f1e99 100644 --- a/app/models/procedure_revision_type_de_champ.rb +++ b/app/models/procedure_revision_type_de_champ.rb @@ -28,6 +28,16 @@ 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 diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 83e284b96..b8f84183e 100644 --- a/spec/models/dossier_spec.rb +++ b/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([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.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).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) diff --git a/spec/models/procedure_presentation_and_revisions_spec.rb b/spec/models/procedure_presentation_and_revisions_spec.rb index 52fa41b5d..482fb0e14 100644 --- a/spec/models/procedure_presentation_and_revisions_spec.rb +++ b/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.move_type_de_champ(created_tdc0.stable_id, 0) + procedure.draft_revision.reload.move_type_de_champ(created_tdc0.stable_id, 0) procedure.publish_revision! end diff --git a/spec/models/procedure_revision_spec.rb b/spec/models/procedure_revision_spec.rb index b2b5640c0..b686b34a1 100644 --- a/spec/models/procedure_revision_spec.rb +++ b/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,17 +92,19 @@ describe ProcedureRevision do end it 'move down' do - expect(type_de_champ_repetition.types_de_champ.index(second_child)).to eq(1) + expect(draft.children_of(type_de_champ_repetition).index(second_child)).to eq(1) + draft.move_type_de_champ(second_child.stable_id, 2) - type_de_champ_repetition.reload - expect(type_de_champ_repetition.types_de_champ.index(second_child)).to eq(2) + + expect(draft.children_of(type_de_champ_repetition).index(second_child)).to eq(2) end it 'move up' do - expect(type_de_champ_repetition.types_de_champ.index(last_child)).to eq(2) + expect(draft.children_of(type_de_champ_repetition).index(last_child)).to eq(2) + draft.move_type_de_champ(last_child.stable_id, 0) - type_de_champ_repetition.reload - expect(type_de_champ_repetition.types_de_champ.index(last_child)).to eq(0) + + expect(draft.children_of(type_de_champ_repetition).index(last_child)).to eq(0) end end end @@ -124,15 +126,92 @@ 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(type_de_champ_repetition.types_de_champ.first.stable_id) + draft.remove_type_de_champ(child.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