From 059488740183489469c8461188682ee51744090c Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 26 Nov 2024 22:12:07 +0100 Subject: [PATCH] refactor(dossier): update clone and apply diff to work with rows --- app/models/concerns/dossier_clone_concern.rb | 50 +++++++------ .../concerns/dossier_clone_concern_spec.rb | 73 ++++++++++++++++--- 2 files changed, 90 insertions(+), 33 deletions(-) diff --git a/app/models/concerns/dossier_clone_concern.rb b/app/models/concerns/dossier_clone_concern.rb index 6f5aab6f2..582dec0d5 100644 --- a/app/models/concerns/dossier_clone_concern.rb +++ b/app/models/concerns/dossier_clone_concern.rb @@ -89,7 +89,11 @@ module DossierCloneConcern dossier_attributes += [:groupe_instructeur_id] if fork relationships = [:individual, :etablissement] - cloned_champs = champs + discarded_row_ids = champs_in_revision + .filter { _1.row? && _1.discarded? } + .to_set(&:row_id) + cloned_champs = champs_in_revision + .reject { discarded_row_ids.member?(_1.row_id) } .index_by(&:id) .transform_values { [_1, _1.clone(fork)] } @@ -143,34 +147,38 @@ module DossierCloneConcern end def apply_diff(diff) - champs_added = diff[:added].filter(&:persisted?) - champs_updated = diff[:updated].filter(&:persisted?) - champs_removed = diff[:removed].filter(&:persisted?) + added_row_ids = {} + diff[:added].each do |champ| + next if !champ.child? + next if added_row_ids.key?(champ.row_id) + added_row_ids[champ.row_id] = revision.parent_of(champ.type_de_champ) + end - champs_added.each { _1.update_column(:dossier_id, id) } + removed_row_ids = {} + diff[:removed].each do |champ| + next if !champ.child? + next if removed_row_ids.key?(champ.row_id) + removed_row_ids[champ.row_id] = revision.parent_of(champ.type_de_champ) + end - if champs_updated.present? + added_champs = diff[:added].filter { _1.persisted? && _1.fillable? } + updated_champs = diff[:updated].filter { _1.persisted? && _1.fillable? } + + added_champs.each { _1.update_column(:dossier_id, id) } + + if updated_champs.present? champs_index = filled_champs_public.index_by(&:public_id) - champs_updated.each do |champ| + updated_champs.each do |champ| champs_index[champ.public_id]&.destroy! champ.update_column(:dossier_id, id) end end - champs_removed.each(&:destroy!) - end - - protected - - # This is a temporary method that is only used by diff/merge algorithm. Once it's gone, this method should be removed. - def project_champs_public_all - revision.types_de_champ_public.flat_map do |type_de_champ| - champ = project_champ(type_de_champ, nil) - if type_de_champ.repetition? - [champ] + project_rows_for(type_de_champ).flatten - else - champ - end + added_row_ids.each do |row_id, repetition_type_de_champ| + champ_for_update(repetition_type_de_champ, row_id, updated_by: user.email).save! + end + removed_row_ids.each do |row_id, repetition_type_de_champ| + champ_for_update(repetition_type_de_champ, row_id, updated_by: user.email).discard! end end end diff --git a/spec/models/concerns/dossier_clone_concern_spec.rb b/spec/models/concerns/dossier_clone_concern_spec.rb index f5339d842..853454cdb 100644 --- a/spec/models/concerns/dossier_clone_concern_spec.rb +++ b/spec/models/concerns/dossier_clone_concern_spec.rb @@ -135,8 +135,8 @@ RSpec.describe DossierCloneConcern do context 'for Champs::Repetition with rows, original_champ.repetition and rows are duped' do let(:types_de_champ_public) { [{ type: :repetition, children: [{}, {}] }] } - let(:champ_repetition) { dossier.champs.find(&:repetition?) } - let(:cloned_champ_repetition) { new_dossier.champs.find(&:repetition?) } + let(:champ_repetition) { dossier.project_champs_public.find(&:repetition?) } + let(:cloned_champ_repetition) { new_dossier.project_champs_public.find(&:repetition?) } it do expect(cloned_champ_repetition.rows.flatten.count).to eq(4) @@ -308,7 +308,7 @@ RSpec.describe DossierCloneConcern do end context 'with new revision' do - let(:added_champ) { forked_dossier.champs.find { _1.libelle == "Un nouveau champ text" } } + let(:added_champ) { forked_dossier.project_champs_public.find { _1.libelle == "Un nouveau champ text" } } let(:removed_champ) { dossier.champs.find { _1.stable_id == 99 } } let(:new_dossier) { dossier.clone } @@ -325,12 +325,16 @@ RSpec.describe DossierCloneConcern do expect(dossier.revision_id).to eq(procedure.revisions.first.id) expect(new_dossier.revision_id).to eq(procedure.published_revision.id) expect(forked_dossier.revision_id).to eq(procedure.published_revision_id) - is_expected.to eq(added: [added_champ], updated: [], removed: [removed_champ]) + expect(subject[:added].map(&:stable_id)).to eq([added_champ.stable_id]) + expect(subject[:added].first.new_record?).to be_truthy + expect(subject[:updated]).to be_empty + expect(subject[:removed]).to eq([removed_champ]) } end end describe '#merge_fork' do + let(:dossier) { create(:dossier, :en_construction, :with_populated_champs, procedure:) } subject { dossier.merge_fork(forked_dossier) } context 'with updated champ' do @@ -348,11 +352,11 @@ RSpec.describe DossierCloneConcern do dossier.debounce_index_search_terms_flag.remove end - it { expect { subject }.to change { dossier.champs.size }.by(0) } + it { expect { subject }.to change { dossier.champs.reload.size }.by(0) } it { expect { subject }.not_to change { dossier.champs.order(:created_at).reject { _1.stable_id.in?([99, 994]) }.map(&:value) } } it { expect { subject }.to have_enqueued_job(DossierIndexSearchTermsJob).with(dossier) } it { expect { subject }.to change { dossier.champs.find { _1.stable_id == 99 }.value }.from('old value').to('new value') } - it { expect { subject }.to change { dossier.champs.find { _1.stable_id == 994 }.value }.from('old value').to('new value in repetition') } + it { expect { subject }.to change { dossier.reload.champs.find { _1.stable_id == 994 }.value }.from('old value').to('new value in repetition') } it 'fork is hidden after merge' do subject @@ -362,8 +366,16 @@ RSpec.describe DossierCloneConcern do end context 'with new revision' do - let(:added_champ) { forked_dossier.champs.find { _1.libelle == "Un nouveau champ text" } } - let(:added_repetition_champ) { forked_dossier.champs.find { _1.libelle == "Texte en répétition" } } + let(:added_champ) { + tdc = forked_dossier.revision.types_de_champ.find { _1.libelle == "Un nouveau champ text" } + forked_dossier.champ_for_update(tdc, nil, updated_by: 'test') + } + let(:added_repetition_champ) { + tdc_repetition = forked_dossier.revision.types_de_champ.find { _1.stable_id == 993 } + tdc = forked_dossier.revision.types_de_champ.find { _1.libelle == "Texte en répétition" } + row_id = forked_dossier.repetition_row_ids(tdc_repetition).first + forked_dossier.champ_for_update(tdc, row_id, updated_by: 'test') + } let(:removed_champ) { dossier.champs.find { _1.stable_id == 99 } } let(:updated_champ) { dossier.champs.find { _1.stable_id == 991 } } @@ -393,8 +405,8 @@ RSpec.describe DossierCloneConcern do super() } - it { expect { subject }.to change { dossier.champs.size }.by(1) } - it { expect { subject }.to change { dossier.champs.order(:created_at).map(&:to_s) }.from(['old value', 'old value', 'Non', 'old value', 'old value']).to(['new value for updated champ', 'Non', 'old value', 'old value', 'new value for added champ', 'new value in repetition champ']) } + it { expect { subject }.to change { dossier.filled_champs.size }.by(1) } + it { expect { subject }.to change { dossier.filled_champs.sort_by(&:created_at).map(&:to_s) }.from(['old value', 'old value', 'Non', 'old value', 'old value']).to(['new value for updated champ', 'Non', 'old value', 'old value', 'new value for added champ', 'new value in repetition champ']) } it "dossier after merge should be on last published revision" do expect(dossier.revision_id).to eq(procedure.revisions.first.id) @@ -404,13 +416,13 @@ RSpec.describe DossierCloneConcern do perform_enqueued_jobs only: DestroyRecordLaterJob expect(dossier.revision_id).to eq(procedure.published_revision_id) - expect(dossier.champs.all? { dossier.revision.in?(_1.type_de_champ.revisions) }).to be_truthy + expect(dossier.filled_champs.all? { dossier.revision.in?(_1.type_de_champ.revisions) }).to be_truthy expect(Dossier.exists?(forked_dossier.id)).to be_falsey end end context 'with old revision having repetition' do - let(:removed_champ) { dossier.champs.find(&:repetition?) } + let(:removed_champ) { dossier.project_champs_public.find(&:repetition?) } before do dossier.champs.each do |champ| @@ -423,5 +435,42 @@ RSpec.describe DossierCloneConcern do expect { subject }.not_to raise_error end end + + context 'with added row' do + let(:repetition_champ) { forked_dossier.project_champs_public.find(&:repetition?) } + + def dossier_rows(dossier) = dossier.champs.filter(&:row?) + + before do + repetition_champ.add_row(updated_by: 'test') + end + + it { + expect(dossier_rows(dossier).size).to eq(2) + expect { subject }.to change { dossier_rows(dossier).size }.by(1) + } + end + + context 'with removed row' do + let(:repetition_champ) { forked_dossier.project_champs_public.find(&:repetition?) } + let(:row_id) { repetition_champ.row_ids.first } + + def dossier_rows(dossier) = dossier.champs.filter(&:row?) + def dossier_discarded_rows(dossier) = dossier_rows(dossier).filter(&:discarded?) + + before do + repetition_champ.remove_row(row_id, updated_by: 'test') + end + + it { + expect(dossier_rows(dossier).size).to eq(2) + expect { subject }.to change { dossier_rows(dossier).size }.by(0) + } + + it { + expect(dossier_discarded_rows(dossier).size).to eq(0) + expect { subject }.to change { dossier_discarded_rows(dossier).size }.by(1) + } + end end end