From 8c8bb870fc730c1713868e86afda427db281d54b Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 7 Oct 2024 11:45:05 +0200 Subject: [PATCH 1/3] refactor(dossier): filled champs --- app/models/concerns/dossier_champs_concern.rb | 28 +++++++++++++++++ app/models/dossier.rb | 10 +++--- .../concerns/dossier_champs_concern_spec.rb | 31 +++++++++++++++++++ spec/models/dossier_spec.rb | 2 +- ...ustificative_file_not_visible_task_spec.rb | 2 +- 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb index 657b93325..f251734bb 100644 --- a/app/models/concerns/dossier_champs_concern.rb +++ b/app/models/concerns/dossier_champs_concern.rb @@ -39,6 +39,34 @@ module DossierChampsConcern revision.types_de_champ_private.map { project_champ(_1, nil) } end + def filled_champs_public + project_champs_public.flat_map do |champ| + if champ.repetition? + champ.rows.flatten.filter { _1.persisted? && _1.fillable? } + elsif champ.persisted? && champ.fillable? + champ + else + [] + end + end + end + + def filled_champs_private + project_champs_private.flat_map do |champ| + if champ.repetition? + champ.rows.flatten.filter { _1.persisted? && _1.fillable? } + elsif champ.persisted? && champ.fillable? + champ + else + [] + end + end + end + + def filled_champs + filled_champs_public + filled_champs_private + end + def project_rows_for(type_de_champ) return [] if !type_de_champ.repetition? diff --git a/app/models/dossier.rb b/app/models/dossier.rb index b2d495d01..fc80584f4 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -517,7 +517,7 @@ class Dossier < ApplicationRecord def can_passer_en_construction? return true if !revision.ineligibilite_enabled || !revision.ineligibilite_rules - !revision.ineligibilite_rules.compute(champs_for_revision(scope: :public)) + !revision.ineligibilite_rules.compute(filled_champs_public) end def can_passer_en_instruction? @@ -567,7 +567,7 @@ class Dossier < ApplicationRecord def any_etablissement_as_degraded_mode? return true if etablissement&.as_degraded_mode? - return true if champs_for_revision(scope: :public).any? { _1.etablissement&.as_degraded_mode? } + return true if filled_champs_public.any? { _1.etablissement&.as_degraded_mode? } false end @@ -1031,7 +1031,7 @@ class Dossier < ApplicationRecord end def linked_dossiers_for(instructeur_or_expert) - dossier_ids = champs_for_revision.filter(&:dossier_link?).filter_map(&:value) + dossier_ids = filled_champs.filter(&:dossier_link?).filter_map(&:value) instructeur_or_expert.dossiers.where(id: dossier_ids) end @@ -1040,7 +1040,7 @@ class Dossier < ApplicationRecord end def geo_data? - GeoArea.exists?(champ_id: champs_for_revision) + GeoArea.exists?(champ_id: filled_champs) end def to_feature_collection @@ -1195,7 +1195,7 @@ class Dossier < ApplicationRecord end def geo_areas - champs_for_revision.flat_map(&:geo_areas) + filled_champs.flat_map(&:geo_areas) end def bounding_box diff --git a/spec/models/concerns/dossier_champs_concern_spec.rb b/spec/models/concerns/dossier_champs_concern_spec.rb index 79689ff44..6b864f28a 100644 --- a/spec/models/concerns/dossier_champs_concern_spec.rb +++ b/spec/models/concerns/dossier_champs_concern_spec.rb @@ -110,6 +110,7 @@ RSpec.describe DossierChampsConcern do subject { dossier.project_champs_public } it { expect(subject.size).to eq(4) } + it { expect(subject.find { _1.libelle == 'Nom' }).to be_falsey } end describe '#project_champs_private' do @@ -118,6 +119,36 @@ RSpec.describe DossierChampsConcern do it { expect(subject.size).to eq(1) } end + describe '#filled_champs_public' do + let(:types_de_champ_public) do + [ + { type: :header_section }, + { type: :text, libelle: "Un champ text" }, + { type: :text, libelle: "Un autre champ text" }, + { type: :yes_no, libelle: "Un champ yes no" }, + { type: :repetition, libelle: "Un champ répétable", mandatory: true, children: [{ type: :text, libelle: 'Nom' }] }, + { type: :explication } + ] + end + subject { dossier.filled_champs_public } + + it { expect(subject.size).to eq(4) } + it { expect(subject.find { _1.libelle == 'Nom' }).to be_truthy } + end + + describe '#filled_champs_private' do + let(:types_de_champ_private) do + [ + { type: :header_section }, + { type: :text, libelle: "Une annotation" }, + { type: :explication } + ] + end + subject { dossier.filled_champs_private } + + it { expect(subject.size).to eq(1) } + end + describe '#repetition_row_ids' do let(:type_de_champ_repetition) { dossier.find_type_de_champ_by_stable_id(993) } subject { dossier.repetition_row_ids(type_de_champ_repetition) } diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 8deed619b..9062e9e4f 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -1852,7 +1852,7 @@ describe Dossier, type: :model do let(:types_de_champ_public) { [{ type: :carte }, { type: :carte }, { type: :carte }] } it do - dossier.champs_for_revision + dossier.filled_champs count = 0 diff --git a/spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb b/spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb index d6d52631f..e7e68ff2b 100644 --- a/spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb +++ b/spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb @@ -13,7 +13,7 @@ module Maintenance before { expect(champ).to receive(:visible?).and_return(visible) } context 'when piece_justificative' do - let(:champ) { dossier.champs_for_revision(scope: :public).find(&:piece_justificative?) } + let(:champ) { dossier.filled_champs_public.find(&:piece_justificative?) } context 'when not visible' do let(:visible) { false } From dd97c2fffd7578f50b5894760ec5ddf1c277e20c Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Sun, 6 Oct 2024 18:28:19 +0200 Subject: [PATCH 2/3] refactor(dossier): diff and merge --- app/models/concerns/dossier_clone_concern.rb | 39 +++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/app/models/concerns/dossier_clone_concern.rb b/app/models/concerns/dossier_clone_concern.rb index dd417cd8a..193561fe4 100644 --- a/app/models/concerns/dossier_clone_concern.rb +++ b/app/models/concerns/dossier_clone_concern.rb @@ -44,10 +44,10 @@ module DossierCloneConcern end def make_diff(editing_fork) - origin_champs_index = champs_for_revision(scope: :public).index_by(&:public_id) - forked_champs_index = editing_fork.champs_for_revision(scope: :public).index_by(&:public_id) + origin_champs_index = project_champs_public_all.index_by(&:public_id) + forked_champs_index = editing_fork.project_champs_public_all.index_by(&:public_id) updated_champs_index = editing_fork - .champs_for_revision(scope: :public) + .project_champs_public_all .filter { _1.updated_at > editing_fork.created_at } .index_by(&:public_id) @@ -83,7 +83,7 @@ module DossierCloneConcern dossier_attributes += [:groupe_instructeur_id] if fork relationships = [:individual, :etablissement] - cloned_champs = champs_for_revision + cloned_champs = champs .index_by(&:id) .transform_values { [_1, _1.clone(fork)] } @@ -149,15 +149,34 @@ module DossierCloneConcern end def apply_diff(diff) - champs_index = (champs_for_revision(scope: :public) + diff[:added]).index_by(&:public_id) + champs_added = diff[:added].filter(&:persisted?) + champs_updated = diff[:updated].filter(&:persisted?) + champs_removed = diff[:removed].filter(&:persisted?) - diff[:added].each { _1.update_column(:dossier_id, id) } + champs_added.each { _1.update_column(:dossier_id, id) } - diff[:updated].each do |champ| - champs_index.fetch(champ.public_id)&.destroy! - champ.update_column(:dossier_id, id) + if champs_updated.present? + champs_index = filled_champs_public.index_by(&:public_id) + champs_updated.each do |champ| + champs_index[champ.public_id]&.destroy! + champ.update_column(:dossier_id, id) + end end - diff[:removed].each(&:destroy!) + 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 + end end end From 0529718c4b529f90704a83459659d82cf22887ea Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 7 Oct 2024 14:22:21 +0200 Subject: [PATCH 3/3] refactor(dossier): the end of champs_for_revision --- app/models/concerns/dossier_champs_concern.rb | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb index f251734bb..70795761a 100644 --- a/app/models/concerns/dossier_champs_concern.rb +++ b/app/models/concerns/dossier_champs_concern.rb @@ -3,24 +3,6 @@ module DossierChampsConcern extend ActiveSupport::Concern - def champs_for_revision(scope: nil) - champs_index = champs.group_by(&:stable_id) - revision.types_de_champ_for(scope:) - .flat_map { champs_index[_1.stable_id] || [] } - end - - # Get all the champs values for the types de champ in the final list. - # Dossier might not have corresponding champ – display nil. - # To do so, we build a virtual champ when there is no value so we can call for_export with all indexes - def champs_for_export(types_de_champ, row_id = nil) - types_de_champ.flat_map do |type_de_champ| - champ = champ_for_export(type_de_champ, row_id) - type_de_champ.libelles_for_export.map do |(libelle, path)| - [libelle, TypeDeChamp.champ_value_for_export(type_de_champ.type_champ, champ, path)] - end - end - end - def project_champ(type_de_champ, row_id) check_valid_row_id?(type_de_champ, row_id) champ = champs_by_public_id[type_de_champ.public_id(row_id)] @@ -97,6 +79,15 @@ module DossierChampsConcern .map { _1.repetition? ? project_champ(_1, nil) : champ_for_update(_1, nil, updated_by: nil) } end + def champs_for_export(types_de_champ, row_id = nil) + types_de_champ.flat_map do |type_de_champ| + champ = champ_for_export(type_de_champ, row_id) + type_de_champ.libelles_for_export.map do |(libelle, path)| + [libelle, TypeDeChamp.champ_value_for_export(type_de_champ.type_champ, champ, path)] + end + end + end + def champ_for_update(type_de_champ, row_id, updated_by:) champ, attributes = champ_with_attributes_for_update(type_de_champ, row_id, updated_by:) champ.assign_attributes(attributes)