diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index dff060439..1dc1041a5 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -41,21 +41,23 @@ class ProcedureRevision < ApplicationRecord after_stable_id = params.delete(:after_stable_id) after_coordinate, _ = coordinate_and_tdc(after_stable_id) - # the collection is orderd by (position, id), so we can use after_coordinate.position - # if not present, a big number is used to ensure the element is at the tail - position = (after_coordinate&.position) || 100_000 + siblings = siblings_for(parent_coordinate:, private_tdc: params[:private]) tdc = TypeDeChamp.new(params) if tdc.save - h = { type_de_champ: tdc, parent_id: parent_id, position: position } - coordinate = revision_types_de_champ.create!(h) + # moving all the impacted tdc down + position = next_position_for(after_coordinate:, siblings:) + siblings.where("position >= ?", position).update_all("position = position + 1") - renumber(coordinate.reload.siblings) + # insertion of the new tdc + h = { type_de_champ: tdc, parent_id: parent_id, position: position } + revision_types_de_champ.create!(h) end # they are not aware of the addition types_de_champ_public.reset types_de_champ_private.reset + reload tdc rescue => e @@ -74,12 +76,15 @@ class ProcedureRevision < ApplicationRecord def move_type_de_champ(stable_id, position) coordinate, _ = coordinate_and_tdc(stable_id) + siblings = coordinate.siblings - siblings = coordinate.siblings.to_a + if position > coordinate.position + siblings.where(position: coordinate.position..position).update_all("position = position - 1") + else + siblings.where(position: position..coordinate.position).update_all("position = position + 1") + end + coordinate.update_column(:position, position) - siblings.insert(position, siblings.delete_at(siblings.index(coordinate))) - - renumber(siblings) coordinate.reload coordinate @@ -98,11 +103,11 @@ class ProcedureRevision < ApplicationRecord tdc.destroy_if_orphan # they are not aware of the removal + coordinate.siblings.where("position >= ?", coordinate.position).update_all("position = position - 1") + types_de_champ_public.reset types_de_champ_private.reset - renumber(coordinate.siblings) - coordinate end @@ -248,9 +253,23 @@ class ProcedureRevision < ApplicationRecord end end - def renumber(siblings) - siblings.to_a.compact.each.with_index do |sibling, position| - sibling.update_column(:position, position) + def siblings_for(parent_coordinate: nil, private_tdc: false) + if parent_coordinate + parent_coordinate.revision_types_de_champ + elsif private_tdc + revision_types_de_champ_private + else + revision_types_de_champ_public + end + end + + def next_position_for(siblings:, after_coordinate: nil) + if siblings.to_a.empty? # first element of the list, starts at 0 + 0 + elsif after_coordinate # middle of the list, between two items + after_coordinate.position + 1 + else # last element of the list, end with last position + 1 + siblings.to_a.last.position + 1 end end diff --git a/app/tasks/maintenance/update_draft_revision_type_de_champs_task.rb b/app/tasks/maintenance/update_draft_revision_type_de_champs_task.rb index 5ce483abf..af03e79ec 100644 --- a/app/tasks/maintenance/update_draft_revision_type_de_champs_task.rb +++ b/app/tasks/maintenance/update_draft_revision_type_de_champs_task.rb @@ -29,8 +29,7 @@ module Maintenance fail "TypeDeChamp not found ! #{typed_id}" if stable_id.nil? tdc = revision.find_and_ensure_exclusive_use(stable_id) - - revision.move_type_de_champ(stable_id, compute_position(row, tdc.revision_type_de_champ)) + revision.move_type_de_champ(stable_id, Integer(row['new_position'])) tdc.update!( libelle: row["new_libelle"].strip, @@ -38,17 +37,5 @@ module Maintenance mandatory: row["new_required"] == "true" ) end - - private - - def compute_position(row, rtdc) - position = Integer(row["new_position"]) - - if rtdc.child? - position - rtdc.parent.position - 1 - else - position - end - end end end diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 49c1b5e02..50a25c194 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -1911,7 +1911,18 @@ describe Dossier, type: :model do describe "champs_for_export" do context 'with a unconditionnal procedure' do - let(:procedure) { create(:procedure, :with_type_de_champ, :with_datetime, :with_yes_no, :with_explication, :with_commune, :with_repetition, zones: [create(:zone)]) } + let(:procedure) { create(:procedure, types_de_champ_public:, zones: [create(:zone)]) } + let(:types_de_champ_public) do + [ + { type: :text }, + { type: :datetime }, + { type: :yes_no }, + { type: :explication }, + { type: :communes }, + { type: :repetition, children: [{ type: :text }] } + ] + end + let(:text_type_de_champ) { procedure.active_revision.types_de_champ_public.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:text) } } let(:yes_no_type_de_champ) { procedure.active_revision.types_de_champ_public.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:yes_no) } } let(:datetime_type_de_champ) { procedure.active_revision.types_de_champ_public.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:datetime) } } @@ -1931,7 +1942,7 @@ describe Dossier, type: :model do procedure.publish! dossier procedure.draft_revision.remove_type_de_champ(text_type_de_champ.stable_id) - procedure.draft_revision.add_type_de_champ(type_champ: TypeDeChamp.type_champs.fetch(:text), libelle: 'New text field') + coordinate = procedure.draft_revision.add_type_de_champ(type_champ: TypeDeChamp.type_champs.fetch(:text), libelle: 'New text field', after_stable_id: repetition_champ.stable_id) procedure.draft_revision.find_and_ensure_exclusive_use(yes_no_type_de_champ.stable_id).update(libelle: 'Updated yes/no') procedure.draft_revision.find_and_ensure_exclusive_use(commune_type_de_champ.stable_id).update(libelle: 'Commune de naissance') procedure.draft_revision.find_and_ensure_exclusive_use(repetition_type_de_champ.stable_id).update(libelle: 'Repetition') diff --git a/spec/models/procedure_revision_spec.rb b/spec/models/procedure_revision_spec.rb index 59ca5b327..5ec369381 100644 --- a/spec/models/procedure_revision_spec.rb +++ b/spec/models/procedure_revision_spec.rb @@ -28,6 +28,7 @@ describe ProcedureRevision do it 'public' do expect { subject }.to change { draft.types_de_champ_public.size }.from(2).to(3) expect(draft.types_de_champ_public.last).to eq(subject) + expect(draft.revision_types_de_champ_public.map(&:position)).to eq([0, 1, 2]) expect(last_coordinate.position).to eq(2) expect(last_coordinate.type_de_champ).to eq(subject) @@ -37,7 +38,12 @@ describe ProcedureRevision do context 'with a private tdc' do let(:tdc_params) { text_params.merge(private: true) } - it { expect { subject }.to change { draft.types_de_champ_private.count }.from(1).to(2) } + it 'private' do + expect { subject }.to change { draft.types_de_champ_private.count }.from(1).to(2) + expect(draft.types_de_champ_private.last).to eq(subject) + expect(draft.revision_types_de_champ_private.map(&:position)).to eq([0, 1]) + expect(last_coordinate.position).to eq(1) + end end context 'with a repetition child' do @@ -46,6 +52,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(last_coordinate.position).to eq(1) @@ -74,6 +81,7 @@ describe ProcedureRevision do expect(draft.revision_types_de_champ_public.map(&:libelle)).to eq(['l1', 'l2']) subject expect(draft.revision_types_de_champ_public.reload.map(&:libelle)).to eq(['l1', 'in the middle', 'l2']) + expect(draft.revision_types_de_champ_public.map(&:position)).to eq([0, 1, 2]) end end @@ -95,20 +103,19 @@ 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) - + stable_id_before = draft.revision_types_de_champ_public.map(&:stable_id) draft.move_type_de_champ(type_de_champ_public.stable_id, 2) draft.reload - + expect(draft.revision_types_de_champ_public.map(&:position)).to eq([0, 1, 2, 3]) expect(draft.types_de_champ_public.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) - draft.move_type_de_champ(last_type_de_champ.stable_id, 0) draft.reload - + expect(draft.revision_types_de_champ_public.map(&:position)).to eq([0, 1, 2, 3]) expect(draft.types_de_champ_public.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 diff --git a/spec/tasks/maintenance/update_draft_revision_type_de_champs_task_spec.rb b/spec/tasks/maintenance/update_draft_revision_type_de_champs_task_spec.rb index d5332d6f7..57e01ef8d 100644 --- a/spec/tasks/maintenance/update_draft_revision_type_de_champs_task_spec.rb +++ b/spec/tasks/maintenance/update_draft_revision_type_de_champs_task_spec.rb @@ -21,9 +21,9 @@ module Maintenance demarche_id,id,new_libelle,new_description,new_required,new_position,delete_flag #{procedure.id},#{find_by_stable_id(12).to_typed_id},[NEW] Number,[NEW] Number desc,true,0, #{procedure.id},#{find_by_stable_id(13).to_typed_id},Bloc,[NEW] bloc desc,,1, - #{procedure.id},#{find_by_stable_id(132).to_typed_id},[NEW] RepNum,,true,2, - #{procedure.id},#{find_by_stable_id(131).to_typed_id},[NEW] RepText,,,3, - #{procedure.id},#{find_by_stable_id(11).to_typed_id},[supp] Text,,,4,true + #{procedure.id},#{find_by_stable_id(132).to_typed_id},[NEW] RepNum,,true,0, + #{procedure.id},#{find_by_stable_id(131).to_typed_id},[NEW] RepText,,,1, + #{procedure.id},#{find_by_stable_id(11).to_typed_id},[supp] Text,,,2,true CSV end