From 2f4e85f1634ca0694188a4bbf5399dce03467099 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 26 Jan 2023 17:58:41 +0100 Subject: [PATCH 1/2] fix(dossier): set rebased_at only on changed champs --- app/models/concerns/dossier_rebase_concern.rb | 28 +++++++++++-------- spec/models/dossier_rebase_concern_spec.rb | 9 ++++-- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/app/models/concerns/dossier_rebase_concern.rb b/app/models/concerns/dossier_rebase_concern.rb index 5e2f5be67..22896b1bb 100644 --- a/app/models/concerns/dossier_rebase_concern.rb +++ b/app/models/concerns/dossier_rebase_concern.rb @@ -56,8 +56,7 @@ module DossierRebaseConcern # add champ changes_by_op[:add] - .map(&:stable_id) - .map { target_coordinates_by_stable_id[_1] } + .map { target_coordinates_by_stable_id[_1.stable_id] } .each { add_new_champs_for_revision(_1) } # remove champ @@ -72,11 +71,9 @@ module DossierRebaseConcern # due to repetition tdc clone on update or erase # we must reassign tdc to the latest version - Champ - .includes(:type_de_champ) - .where(dossier: self) - .map { [_1, target_coordinates_by_stable_id[_1.stable_id].type_de_champ] } - .each { |champ, target_tdc| champ.update_columns(type_de_champ_id: target_tdc.id, rebased_at: Time.zone.now) } + champs + .map { [_1, target_coordinates_by_stable_id[_1.stable_id].type_de_champ_id] } + .each { |champ, type_de_champ_id| champ.update_columns(type_de_champ_id:) } # update dossier revision self.update_column(:revision_id, target_revision.id) @@ -92,18 +89,27 @@ module DossierRebaseConcern value: nil, value_json: nil, external_id: nil, - data: nil) + data: nil, + rebased_at: Time.zone.now) when :drop_down_options # we are removing options, we need to remove the value if it contains one of the removed options removed_options = change.from - change.to if removed_options.present? && champs.any? { _1.in?(removed_options) } - champs.filter { _1.in?(removed_options) }.each { _1.remove_option(removed_options) } + champs.filter { _1.in?(removed_options) }.each do + _1.remove_option(removed_options) + _1.update_column(:rebased_at, Time.zone.now) + end end when :carte_layers # if we are removing cadastres layer, we need to remove cadastre geo areas if change.from.include?(:cadastres) && !change.to.include?(:cadastres) - champs.each { _1.cadastres.each(&:destroy) } + champs.filter { _1.cadastres.present? }.each do + _1.cadastres.each(&:destroy) + _1.update_column(:rebased_at, Time.zone.now) + end end + else + champs.update_all(rebased_at: Time.zone.now) end end @@ -126,7 +132,7 @@ module DossierRebaseConcern end def create_champ(target_coordinate, parent, row_id: nil) - params = { revision: target_coordinate.revision, row_id: }.compact + params = { revision: target_coordinate.revision, rebased_at: Time.zone.now, row_id: }.compact champ = target_coordinate .type_de_champ .build_champ(params) diff --git a/spec/models/dossier_rebase_concern_spec.rb b/spec/models/dossier_rebase_concern_spec.rb index 1e5183412..afeef5926 100644 --- a/spec/models/dossier_rebase_concern_spec.rb +++ b/spec/models/dossier_rebase_concern_spec.rb @@ -239,7 +239,7 @@ describe Dossier do end describe "#rebase" do - let(:procedure) { create(:procedure, :with_type_de_champ_mandatory, :with_yes_no, :with_repetition, :with_datetime) } + let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :text, mandatory: true }, { type: :repetition, children: [{ type: :text }] }, { type: :datetime }, { type: :yes_no }, { type: :integer_number }]) } let(:dossier) { create(:dossier, procedure: procedure) } let(:yes_no_type_de_champ) { procedure.active_revision.types_de_champ_public.find { |tdc| tdc.type_champ == TypeDeChamp.type_champs.fetch(:yes_no) } } @@ -248,6 +248,8 @@ describe Dossier do let(:text_champ) { dossier.champs_public.find(&:mandatory?) } let(:rebased_text_champ) { dossier.champs_public.find { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:text) } } + let(:rebased_number_champ) { dossier.champs_public.find { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:integer_number) } } + let(:datetime_type_de_champ) { procedure.active_revision.types_de_champ_public.find { |tdc| tdc.type_champ == TypeDeChamp.type_champs.fetch(:datetime) } } let(:datetime_champ) { dossier.champs_public.find { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:datetime) } } let(:rebased_datetime_champ) { dossier.champs_public.find { |c| c.type_champ == TypeDeChamp.type_champs.fetch(:date) } } @@ -287,7 +289,7 @@ describe Dossier do libelle = text_type_de_champ.libelle expect(dossier.revision).to eq(procedure.published_revision) - expect(dossier.champs_public.size).to eq(4) + expect(dossier.champs_public.size).to eq(5) expect(repetition_champ.rows.size).to eq(2) expect(repetition_champ.rows[0].size).to eq(1) expect(repetition_champ.rows[1].size).to eq(1) @@ -299,7 +301,7 @@ describe Dossier do expect(procedure.revisions.size).to eq(3) expect(dossier.revision).to eq(procedure.published_revision) - expect(dossier.champs_public.size).to eq(4) + expect(dossier.champs_public.size).to eq(5) expect(rebased_text_champ.value).to eq(text_champ.value) expect(rebased_text_champ.type_de_champ_id).not_to eq(text_champ.type_de_champ_id) expect(rebased_datetime_champ.type_champ).to eq(TypeDeChamp.type_champs.fetch(:date)) @@ -309,6 +311,7 @@ describe Dossier do expect(rebased_repetition_champ.rows[1].size).to eq(2) expect(rebased_text_champ.rebased_at).not_to be_nil expect(rebased_datetime_champ.rebased_at).not_to be_nil + expect(rebased_number_champ.rebased_at).to be_nil end end From 6cacfbdf327d1f986387162ee48b335bb1b25591 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 30 Jan 2023 11:30:50 +0100 Subject: [PATCH 2/2] refactor(dossier): refactor rebase --- app/models/concerns/dossier_rebase_concern.rb | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/app/models/concerns/dossier_rebase_concern.rb b/app/models/concerns/dossier_rebase_concern.rb index 22896b1bb..54f1445fa 100644 --- a/app/models/concerns/dossier_rebase_concern.rb +++ b/app/models/concerns/dossier_rebase_concern.rb @@ -54,29 +54,32 @@ module DossierRebaseConcern .group_by(&:op) .tap { _1.default = [] } + champs_by_stable_id = champs + .joins(:type_de_champ) + .group_by(&:stable_id) + .transform_values { Champ.where(id: _1) } + # add champ changes_by_op[:add] - .map { target_coordinates_by_stable_id[_1.stable_id] } - .each { add_new_champs_for_revision(_1) } + .each { add_new_champs_for_revision(target_coordinates_by_stable_id[_1.stable_id]) } # remove champ changes_by_op[:remove] - .each { delete_champs_for_revision(_1.stable_id) } + .each { champs_by_stable_id[_1.stable_id].destroy_all } if brouillon? changes_by_op[:update] - .map { |change| [change, champs.joins(:type_de_champ).where(type_de_champ: { stable_id: change.stable_id })] } - .each { |change, champs| apply(change, champs) } + .each { apply(_1, champs_by_stable_id[_1.stable_id]) } end # due to repetition tdc clone on update or erase # we must reassign tdc to the latest version - champs - .map { [_1, target_coordinates_by_stable_id[_1.stable_id].type_de_champ_id] } - .each { |champ, type_de_champ_id| champ.update_columns(type_de_champ_id:) } + champs_by_stable_id + .filter_map { |stable_id, champs| [target_coordinates_by_stable_id[stable_id].type_de_champ_id, champs] if champs.present? } + .each { |type_de_champ_id, champs| champs.update_all(type_de_champ_id:) } # update dossier revision - self.update_column(:revision_id, target_revision.id) + update_column(:revision_id, target_revision.id) end def apply(change, champs) @@ -139,13 +142,6 @@ module DossierRebaseConcern parent.champs << champ end - def delete_champs_for_revision(stable_id) - champs - .joins(:type_de_champ) - .where(types_de_champ: { stable_id: }) - .destroy_all - end - def purge_piece_justificative_file(champ) ActiveStorage::Attachment.where(id: champ.piece_justificative_file.ids).delete_all end