From 7ee90b6d85f8afbaec81bb544e1c2a5c1644cb84 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 26 Nov 2024 22:05:26 +0100 Subject: [PATCH] refactor(champ): validate only champs in revision --- app/models/champ.rb | 9 +++- app/models/concerns/champ_revision_concern.rb | 23 +++++++++ ...e_concern.rb => champ_validate_concern.rb} | 14 +++--- app/models/concerns/dossier_champs_concern.rb | 14 +++++- app/models/procedure_revision.rb | 4 -- app/models/type_de_champ.rb | 4 -- spec/models/champs/epci_champ_spec.rb | 2 +- .../champs/integer_number_champ_spec.rb | 2 +- ...spec.rb => champ_validate_concern_spec.rb} | 47 +++++++++++++++++-- 9 files changed, 96 insertions(+), 23 deletions(-) create mode 100644 app/models/concerns/champ_revision_concern.rb rename app/models/concerns/{champs_validate_concern.rb => champ_validate_concern.rb} (70%) rename spec/models/concerns/{champs_validate_concern_spec.rb => champ_validate_concern_spec.rb} (58%) diff --git a/app/models/champ.rb b/app/models/champ.rb index e58dd4fb9..5a9e79520 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -2,7 +2,8 @@ class Champ < ApplicationRecord include ChampConditionalConcern - include ChampsValidateConcern + include ChampValidateConcern + include ChampRevisionConcern self.ignored_columns += [:type_de_champ_id, :parent_id] @@ -69,7 +70,11 @@ class Champ < ApplicationRecord end def child? - row_id.present? + row_id.present? && !is_type?(TypeDeChamp.type_champs.fetch(:repetition)) + end + + def row? + row_id.present? && is_type?(TypeDeChamp.type_champs.fetch(:repetition)) end # used for the `required` html attribute diff --git a/app/models/concerns/champ_revision_concern.rb b/app/models/concerns/champ_revision_concern.rb new file mode 100644 index 000000000..04a346754 --- /dev/null +++ b/app/models/concerns/champ_revision_concern.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module ChampRevisionConcern + extend ActiveSupport::Concern + + protected + + def is_same_type_as_revision? + is_type?(type_de_champ.type_champ) + end + + def in_dossier_revision? + dossier.stable_id_in_revision?(stable_id) + end + + def in_discarded_row? + if child? + repetition_type_de_champ = dossier.revision.parent_of(type_de_champ) + row_ids = dossier.repetition_row_ids(repetition_type_de_champ) + !row_id.in?(row_ids) + end + end +end diff --git a/app/models/concerns/champs_validate_concern.rb b/app/models/concerns/champ_validate_concern.rb similarity index 70% rename from app/models/concerns/champs_validate_concern.rb rename to app/models/concerns/champ_validate_concern.rb index 97c0c791e..a31cfe8b9 100644 --- a/app/models/concerns/champs_validate_concern.rb +++ b/app/models/concerns/champ_validate_concern.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module ChampsValidateConcern +module ChampValidateConcern extend ActiveSupport::Concern included do @@ -16,20 +16,20 @@ module ChampsValidateConcern def validate_champ_value? case validation_context when :champs_public_value - public? && in_dossier_revision? && visible? + public? && can_validate? && visible? when :champs_private_value - private? && in_dossier_revision? && visible? + private? && can_validate? && visible? else false end end + def can_validate? + in_dossier_revision? && is_same_type_as_revision? && !row? && !in_discarded_row? + end + def validate_champ_value_or_prefill? validate_champ_value? || validation_context == :prefill end - - def in_dossier_revision? - dossier.revision.in_revision?(stable_id) && is_type?(type_de_champ.type_champ) - end end end diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb index 6392393a1..406521638 100644 --- a/app/models/concerns/dossier_champs_concern.rb +++ b/app/models/concerns/dossier_champs_concern.rb @@ -135,6 +135,10 @@ module DossierChampsConcern reload_champs_cache end + def stable_id_in_revision?(stable_id) + revision_stable_ids.member?(stable_id.to_i) + end + def reload super.tap { reset_champs_cache } end @@ -142,7 +146,15 @@ module DossierChampsConcern private def champs_by_public_id - @champs_by_public_id ||= champs.sort_by(&:id).index_by(&:public_id) + @champs_by_public_id ||= champs_in_revision.index_by(&:public_id) + end + + def revision_stable_ids + @revision_stable_ids ||= revision.types_de_champ.map(&:stable_id).to_set + end + + def champs_in_revision + champs.filter { stable_id_in_revision?(_1.stable_id) } end def filled_champ(type_de_champ, row_id) diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index cffc1c96b..94f896d62 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -203,10 +203,6 @@ class ProcedureRevision < ApplicationRecord coordinate_for(tdc).parent_type_de_champ end - def in_revision?(stable_id) - types_de_champ.any? { _1.stable_id == stable_id } - end - def dependent_conditions(tdc) stable_id = tdc.stable_id diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index 707848b12..bf4a0046d 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -331,10 +331,6 @@ class TypeDeChamp < ApplicationRecord !private? end - def in_revision?(revision) - revision.types_de_champ.any? { _1.stable_id == stable_id } - end - def child?(revision) revision.coordinate_for(self)&.child? end diff --git a/spec/models/champs/epci_champ_spec.rb b/spec/models/champs/epci_champ_spec.rb index 58d7b7868..4c07d7bbd 100644 --- a/spec/models/champs/epci_champ_spec.rb +++ b/spec/models/champs/epci_champ_spec.rb @@ -8,7 +8,7 @@ describe Champs::EpciChamp, type: :model do let(:champ) { Champs::EpciChamp.new(code_departement: code_departement, dossier: build(:dossier)) } before do allow(champ).to receive(:visible?).and_return(true) - allow(champ).to receive(:in_dossier_revision?).and_return(true) + allow(champ).to receive(:can_validate?).and_return(true) end context 'when nil' do let(:code_departement) { nil } diff --git a/spec/models/champs/integer_number_champ_spec.rb b/spec/models/champs/integer_number_champ_spec.rb index 8a5d2182f..e46c0cdb4 100644 --- a/spec/models/champs/integer_number_champ_spec.rb +++ b/spec/models/champs/integer_number_champ_spec.rb @@ -4,7 +4,7 @@ describe Champs::IntegerNumberChamp do let(:champ) { Champs::IntegerNumberChamp.new(value:, dossier: build(:dossier)) } before do allow(champ).to receive(:visible?).and_return(true) - allow(champ).to receive(:in_dossier_revision?).and_return(true) + allow(champ).to receive(:can_validate?).and_return(true) end subject { champ.validate(:champs_public_value) } diff --git a/spec/models/concerns/champs_validate_concern_spec.rb b/spec/models/concerns/champ_validate_concern_spec.rb similarity index 58% rename from spec/models/concerns/champs_validate_concern_spec.rb rename to spec/models/concerns/champ_validate_concern_spec.rb index edf0e5224..83f5401b4 100644 --- a/spec/models/concerns/champs_validate_concern_spec.rb +++ b/spec/models/concerns/champ_validate_concern_spec.rb @@ -1,15 +1,15 @@ # frozen_string_literal: true -RSpec.describe ChampsValidateConcern do +RSpec.describe ChampValidateConcern do let(:procedure) { create(:procedure, :published, types_de_champ_public:) } let(:dossier) { create(:dossier, :with_populated_champs, procedure:) } let(:type_de_champ) { dossier.revision.types_de_champ_public.first } - + let(:public_id) { type_de_champ.public_id(nil) } let(:types_de_champ_public) { [{ type: :email }] } def update_champ(value) dossier.update_champs_attributes({ - type_de_champ.stable_id.to_s => { value: } + public_id => { value: } }, :public, updated_by: 'test') dossier.save end @@ -68,4 +68,45 @@ RSpec.describe ChampsValidateConcern do } end end + + context 'when in a row' do + let(:types_de_champ_public) { [{ type: :repetition, children: [{ type: :email }], mandatory: true }] } + let(:type_de_champ_in_repetition) { dossier.revision.children_of(type_de_champ).first } + let(:row_id) { dossier.repetition_row_ids(type_de_champ).first } + let(:public_id) { type_de_champ_in_repetition.public_id(row_id) } + + context 'valid' do + before { + update_champ('test@test.com') + dossier.validate(:champs_public_value) + } + it { + expect(dossier.champs).not_to be_empty + expect(dossier.errors).to be_empty + } + end + + context 'invalid' do + before { + update_champ('test') + dossier.validate(:champs_public_value) + } + it { + expect(dossier.champs).not_to be_empty + expect(dossier.errors).not_to be_empty + } + end + + context 'do not validate when in discarded row' do + before { + update_champ('test') + dossier.repetition_remove_row(type_de_champ, row_id, updated_by: 'test') + dossier.validate(:champs_public_value) + } + it { + expect(dossier.champs).not_to be_empty + expect(dossier.errors).to be_empty + } + end + end end