From b3701a12b4898ed3c07ad4f80c7717f681557f3a Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 9 Dec 2024 15:15:04 +0100 Subject: [PATCH 1/3] fix(champ): always check if a champ is in a valid revision before validate --- .../champs/piece_justificative_controller.rb | 3 +- app/models/champs/boolean_champ.rb | 2 +- app/models/champs/civilite_champ.rb | 2 +- app/models/champs/cnaf_champ.rb | 4 +-- app/models/champs/commune_champ.rb | 2 +- app/models/champs/decimal_number_champ.rb | 2 +- app/models/champs/departement_champ.rb | 4 +-- app/models/champs/dgfip_champ.rb | 4 +-- app/models/champs/drop_down_list_champ.rb | 2 +- app/models/champs/email_champ.rb | 2 +- .../champs/engagement_juridique_champ.rb | 2 +- app/models/champs/epci_champ.rb | 6 ++-- .../champs/expression_reguliere_champ.rb | 2 +- app/models/champs/iban_champ.rb | 2 +- app/models/champs/integer_number_champ.rb | 2 +- .../champs/multiple_drop_down_list_champ.rb | 2 +- app/models/champs/phone_champ.rb | 2 +- .../champs/piece_justificative_champ.rb | 5 ++-- app/models/champs/region_champ.rb | 4 +-- app/models/champs/rna_champ.rb | 2 +- app/models/champs/titre_identite_champ.rb | 6 ++-- app/models/concerns/champ_validate_concern.rb | 6 ++-- spec/models/champs/dossier_link_champ_spec.rb | 24 ++++++++-------- spec/models/champs/epci_champ_spec.rb | 14 ++++++---- .../champs/piece_justificative_champ_spec.rb | 17 ++++------- .../concerns/champ_validate_concern_spec.rb | 28 +++++++++++++++++++ spec/models/dossier_spec.rb | 8 +++--- spec/models/prefill_champs_spec.rb | 17 ++++++----- spec/system/users/dossier_prefill_get_spec.rb | 3 +- .../system/users/dossier_prefill_post_spec.rb | 3 +- 30 files changed, 107 insertions(+), 75 deletions(-) diff --git a/app/controllers/champs/piece_justificative_controller.rb b/app/controllers/champs/piece_justificative_controller.rb index 45cc6a848..aaa1052dd 100644 --- a/app/controllers/champs/piece_justificative_controller.rb +++ b/app/controllers/champs/piece_justificative_controller.rb @@ -27,7 +27,8 @@ class Champs::PieceJustificativeController < Champs::ChampController ActiveStorage::Attachment.transaction do @champ.piece_justificative_file.attach(params[:blob_signed_id]) - save_succeed = @champ.save + context = @champ.public? ? :champs_public_value : :champs_private_value + save_succeed = @champ.save(context:) end if save_succeed && dossier.brouillon? diff --git a/app/models/champs/boolean_champ.rb b/app/models/champs/boolean_champ.rb index 1d4f8f63c..fd1e3d71e 100644 --- a/app/models/champs/boolean_champ.rb +++ b/app/models/champs/boolean_champ.rb @@ -7,7 +7,7 @@ class Champs::BooleanChamp < Champ before_validation :set_value_to_nil, if: -> { value.blank? } before_validation :set_value_to_false, unless: -> { ([nil, TRUE_VALUE, FALSE_VALUE]).include?(value) } - validates :value, inclusion: [TRUE_VALUE, FALSE_VALUE], allow_nil: true, allow_blank: false, if: :validate_champ_value_or_prefill? + validates :value, inclusion: [TRUE_VALUE, FALSE_VALUE], allow_nil: true, allow_blank: false, if: :validate_champ_value? def true? value == TRUE_VALUE diff --git a/app/models/champs/civilite_champ.rb b/app/models/champs/civilite_champ.rb index 7a1f2a254..5b1e1d2e6 100644 --- a/app/models/champs/civilite_champ.rb +++ b/app/models/champs/civilite_champ.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Champs::CiviliteChamp < Champ - validates :value, inclusion: ["M.", "Mme"], allow_nil: true, allow_blank: false, if: :validate_champ_value_or_prefill? + validates :value, inclusion: ["M.", "Mme"], allow_nil: true, allow_blank: false, if: :validate_champ_value? def legend_label? true diff --git a/app/models/champs/cnaf_champ.rb b/app/models/champs/cnaf_champ.rb index c0cdc9383..5a6350432 100644 --- a/app/models/champs/cnaf_champ.rb +++ b/app/models/champs/cnaf_champ.rb @@ -3,8 +3,8 @@ class Champs::CnafChamp < Champs::TextChamp # see https://github.com/betagouv/api-particulier/blob/master/src/presentation/middlewares/cnaf-input-validation.middleware.ts - validates :numero_allocataire, format: { with: /\A\d{1,7}\z/ }, if: -> { code_postal.present? && validate_champ_value_or_prefill? } - validates :code_postal, format: { with: /\A\w{5}\z/ }, if: -> { numero_allocataire.present? && validate_champ_value_or_prefill? } + validates :numero_allocataire, format: { with: /\A\d{1,7}\z/ }, if: -> { code_postal.present? && validate_champ_value? } + validates :code_postal, format: { with: /\A\w{5}\z/ }, if: -> { numero_allocataire.present? && validate_champ_value? } store_accessor :value_json, :numero_allocataire, :code_postal diff --git a/app/models/champs/commune_champ.rb b/app/models/champs/commune_champ.rb index a1e69ba48..a3a548b5e 100644 --- a/app/models/champs/commune_champ.rb +++ b/app/models/champs/commune_champ.rb @@ -4,7 +4,7 @@ class Champs::CommuneChamp < Champs::TextChamp store_accessor :value_json, :code_departement, :code_postal, :code_region before_save :on_codes_change, if: :should_refresh_after_code_change? - validates :external_id, presence: true, if: -> { validate_champ_value_or_prefill? && value.present? } + validates :external_id, presence: true, if: -> { value.present? && validate_champ_value? } after_validation :instrument_external_id_error, if: -> { errors.include?(:external_id) } def departement_name diff --git a/app/models/champs/decimal_number_champ.rb b/app/models/champs/decimal_number_champ.rb index 04e1d9af5..917fe1701 100644 --- a/app/models/champs/decimal_number_champ.rb +++ b/app/models/champs/decimal_number_champ.rb @@ -17,7 +17,7 @@ class Champs::DecimalNumberChamp < Champ message: -> (object, _data) { object.errors.generate_message(:value, :not_a_number) } - }, if: :validate_champ_value_or_prefill? + }, if: :validate_champ_value? private diff --git a/app/models/champs/departement_champ.rb b/app/models/champs/departement_champ.rb index b6a84f421..fe8c14fc0 100644 --- a/app/models/champs/departement_champ.rb +++ b/app/models/champs/departement_champ.rb @@ -3,8 +3,8 @@ class Champs::DepartementChamp < Champs::TextChamp store_accessor :value_json, :code_region - validate :value_in_departement_names, if: -> { validate_champ_value_or_prefill? && !value.nil? } - validate :external_id_in_departement_codes, if: -> { validate_champ_value_or_prefill? && !external_id.nil? } + validate :value_in_departement_names, if: -> { !value.nil? && validate_champ_value? } + validate :external_id_in_departement_codes, if: -> { !external_id.nil? && validate_champ_value? } before_save :store_code_region def selected diff --git a/app/models/champs/dgfip_champ.rb b/app/models/champs/dgfip_champ.rb index c64e55b9a..b118d7a1f 100644 --- a/app/models/champs/dgfip_champ.rb +++ b/app/models/champs/dgfip_champ.rb @@ -2,8 +2,8 @@ class Champs::DgfipChamp < Champs::TextChamp # see https://github.com/betagouv/api-particulier/blob/master/src/presentation/middlewares/dgfip-input-validation.middleware.ts - validates :numero_fiscal, format: { with: /\A\w{13,14}\z/ }, if: -> { reference_avis.present? && validate_champ_value_or_prefill? } - validates :reference_avis, format: { with: /\A\w{13,14}\z/ }, if: -> { numero_fiscal.present? && validate_champ_value_or_prefill? } + validates :numero_fiscal, format: { with: /\A\w{13,14}\z/ }, if: -> { validate_champ_value? && reference_avis.present? } + validates :reference_avis, format: { with: /\A\w{13,14}\z/ }, if: -> { validate_champ_value? && numero_fiscal.present? } store_accessor :value_json, :numero_fiscal, :reference_avis diff --git a/app/models/champs/drop_down_list_champ.rb b/app/models/champs/drop_down_list_champ.rb index 4c5306f6c..d5b08db9d 100644 --- a/app/models/champs/drop_down_list_champ.rb +++ b/app/models/champs/drop_down_list_champ.rb @@ -6,7 +6,7 @@ class Champs::DropDownListChamp < Champ THRESHOLD_NB_OPTIONS_AS_AUTOCOMPLETE = 20 OTHER = '__other__' delegate :options_without_empty_value_when_mandatory, to: :type_de_champ - validate :value_is_in_options, if: -> { !(value.blank? || drop_down_other?) && validate_champ_value_or_prefill? } + validate :value_is_in_options, if: -> { validate_champ_value? && !(value.blank? || drop_down_other?) } def render_as_radios? drop_down_options.size <= THRESHOLD_NB_OPTIONS_AS_RADIO diff --git a/app/models/champs/email_champ.rb b/app/models/champs/email_champ.rb index 6a1f50306..f546c7b2b 100644 --- a/app/models/champs/email_champ.rb +++ b/app/models/champs/email_champ.rb @@ -4,5 +4,5 @@ class Champs::EmailChamp < Champs::TextChamp include EmailSanitizableConcern before_validation -> { sanitize_email(:value) } - validates :value, allow_blank: true, format: { with: StrictEmailValidator::REGEXP }, if: :validate_champ_value_or_prefill? + validates :value, allow_blank: true, format: { with: StrictEmailValidator::REGEXP }, if: :validate_champ_value? end diff --git a/app/models/champs/engagement_juridique_champ.rb b/app/models/champs/engagement_juridique_champ.rb index 31118063a..7151a74e1 100644 --- a/app/models/champs/engagement_juridique_champ.rb +++ b/app/models/champs/engagement_juridique_champ.rb @@ -5,5 +5,5 @@ class Champs::EngagementJuridiqueChamp < Champ validates_with ExpressionReguliereValidator, expression_reguliere: /([A-Z]|[0-9]|\-|\_|\+|\/)+/, expression_reguliere_error_message: "Le numéro d'EJ ne peut contenir que des caractères alphanumérique et les caractères spéciaux suivant : “-“ ; “_“ ; “+“ ; “/“", - if: :validate_champ_value_or_prefill? + if: :validate_champ_value? end diff --git a/app/models/champs/epci_champ.rb b/app/models/champs/epci_champ.rb index bdedee60a..fed43a82c 100644 --- a/app/models/champs/epci_champ.rb +++ b/app/models/champs/epci_champ.rb @@ -5,9 +5,9 @@ class Champs::EpciChamp < Champs::TextChamp before_validation :on_departement_change before_validation :on_epci_name_changes - validate :code_departement_in_departement_codes, if: -> { !(code_departement.nil?) && validate_champ_value_or_prefill? } - validate :external_id_in_departement_epci_codes, if: -> { !(code_departement.nil? || external_id.nil?) && validate_champ_value_or_prefill? } - validate :value_in_departement_epci_names, if: -> { !(code_departement.nil? || external_id.nil? || value.nil?) && validate_champ_value_or_prefill? } + validate :code_departement_in_departement_codes, if: -> { !(code_departement.nil?) && validate_champ_value? } + validate :external_id_in_departement_epci_codes, if: -> { !(code_departement.nil? || external_id.nil?) && validate_champ_value? } + validate :value_in_departement_epci_names, if: -> { !(code_departement.nil? || external_id.nil? || value.nil?) && validate_champ_value? } def departement_name APIGeoService.departement_name(code_departement) diff --git a/app/models/champs/expression_reguliere_champ.rb b/app/models/champs/expression_reguliere_champ.rb index 9fb44375f..cbcbe0573 100644 --- a/app/models/champs/expression_reguliere_champ.rb +++ b/app/models/champs/expression_reguliere_champ.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true class Champs::ExpressionReguliereChamp < Champ - validates_with ExpressionReguliereValidator, if: :validate_champ_value_or_prefill? + validates_with ExpressionReguliereValidator, if: :validate_champ_value? end diff --git a/app/models/champs/iban_champ.rb b/app/models/champs/iban_champ.rb index cc26eafa8..18f816271 100644 --- a/app/models/champs/iban_champ.rb +++ b/app/models/champs/iban_champ.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Champs::IbanChamp < Champ - validates_with IbanValidator, if: :validate_champ_value_or_prefill? + validates_with IbanValidator, if: :validate_champ_value? after_validation :format_iban private diff --git a/app/models/champs/integer_number_champ.rb b/app/models/champs/integer_number_champ.rb index 0456ba1b6..369759af0 100644 --- a/app/models/champs/integer_number_champ.rb +++ b/app/models/champs/integer_number_champ.rb @@ -11,7 +11,7 @@ class Champs::IntegerNumberChamp < Champ # i18n-tasks-use t('errors.messages.not_an_integer') object.errors.generate_message(:value, :not_an_integer) } - }, if: :validate_champ_value_or_prefill? + }, if: :validate_champ_value? def format_value return if value.blank? diff --git a/app/models/champs/multiple_drop_down_list_champ.rb b/app/models/champs/multiple_drop_down_list_champ.rb index 46bbabfb8..0e1ff8ff6 100644 --- a/app/models/champs/multiple_drop_down_list_champ.rb +++ b/app/models/champs/multiple_drop_down_list_champ.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Champs::MultipleDropDownListChamp < Champ - validate :values_are_in_options, if: -> { value.present? && validate_champ_value_or_prefill? } + validate :values_are_in_options, if: -> { value.present? && validate_champ_value? } THRESHOLD_NB_OPTIONS_AS_CHECKBOX = 5 diff --git a/app/models/champs/phone_champ.rb b/app/models/champs/phone_champ.rb index a29dc7f19..23ec03e5c 100644 --- a/app/models/champs/phone_champ.rb +++ b/app/models/champs/phone_champ.rb @@ -7,5 +7,5 @@ class Champs::PhoneChamp < Champs::TextChamp allow_blank: true, message: I18n.t(:not_a_phone, scope: 'activerecord.errors.messages') }, - if: -> { !Phonelib.valid_for_countries?(value, TypesDeChamp::PhoneTypeDeChamp::DEFAULT_COUNTRY_CODES) && validate_champ_value_or_prefill? } + if: -> { validate_champ_value? && !Phonelib.valid_for_countries?(value, TypesDeChamp::PhoneTypeDeChamp::DEFAULT_COUNTRY_CODES) } end diff --git a/app/models/champs/piece_justificative_champ.rb b/app/models/champs/piece_justificative_champ.rb index b282aee35..5ab1c86de 100644 --- a/app/models/champs/piece_justificative_champ.rb +++ b/app/models/champs/piece_justificative_champ.rb @@ -5,14 +5,13 @@ class Champs::PieceJustificativeChamp < Champ has_many_attached :piece_justificative_file - # TODO: if: -> { validate_champ_value? || validation_context == :prefill } validates :piece_justificative_file, size: { less_than: FILE_MAX_SIZE }, - if: -> { !type_de_champ.skip_pj_validation } + if: -> { can_validate? && !type_de_champ.skip_pj_validation } validates :piece_justificative_file, content_type: AUTHORIZED_CONTENT_TYPES, - if: -> { !type_de_champ.skip_content_type_pj_validation } + if: -> { can_validate? && !type_de_champ.skip_content_type_pj_validation } def main_value_name :piece_justificative_file diff --git a/app/models/champs/region_champ.rb b/app/models/champs/region_champ.rb index 9bbff77b6..c123036b6 100644 --- a/app/models/champs/region_champ.rb +++ b/app/models/champs/region_champ.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true class Champs::RegionChamp < Champs::TextChamp - validate :value_in_region_names, if: -> { !value.nil? && validate_champ_value_or_prefill? } - validate :external_id_in_region_codes, if: -> { !external_id.nil? && validate_champ_value_or_prefill? } + validate :value_in_region_names, if: -> { !value.nil? && validate_champ_value? } + validate :external_id_in_region_codes, if: -> { !external_id.nil? && validate_champ_value? } def selected code diff --git a/app/models/champs/rna_champ.rb b/app/models/champs/rna_champ.rb index b738922d8..0cfad8977 100644 --- a/app/models/champs/rna_champ.rb +++ b/app/models/champs/rna_champ.rb @@ -5,7 +5,7 @@ class Champs::RNAChamp < Champ validates :value, allow_blank: true, format: { with: /\AW[0-9A-Z]{9}\z/, message: I18n.t(:not_a_rna, scope: 'activerecord.errors.messages') - }, if: :validate_champ_value_or_prefill? + }, if: :validate_champ_value? delegate :id, to: :procedure, prefix: true diff --git a/app/models/champs/titre_identite_champ.rb b/app/models/champs/titre_identite_champ.rb index 790259f7c..eeb4378ce 100644 --- a/app/models/champs/titre_identite_champ.rb +++ b/app/models/champs/titre_identite_champ.rb @@ -6,8 +6,10 @@ class Champs::TitreIdentiteChamp < Champ has_many_attached :piece_justificative_file - # TODO: if: -> { validate_champ_value? || validation_context == :prefill } - validates :piece_justificative_file, content_type: ACCEPTED_FORMATS, size: { less_than: FILE_MAX_SIZE } + validates :piece_justificative_file, + content_type: ACCEPTED_FORMATS, + size: { less_than: FILE_MAX_SIZE }, + if: :validate_champ_value? def main_value_name :piece_justificative_file diff --git a/app/models/concerns/champ_validate_concern.rb b/app/models/concerns/champ_validate_concern.rb index a31cfe8b9..ab8472a77 100644 --- a/app/models/concerns/champ_validate_concern.rb +++ b/app/models/concerns/champ_validate_concern.rb @@ -19,6 +19,8 @@ module ChampValidateConcern public? && can_validate? && visible? when :champs_private_value private? && can_validate? && visible? + when :prefill + true else false end @@ -27,9 +29,5 @@ module ChampValidateConcern 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 end end diff --git a/spec/models/champs/dossier_link_champ_spec.rb b/spec/models/champs/dossier_link_champ_spec.rb index d5f5e4550..4360d0b60 100644 --- a/spec/models/champs/dossier_link_champ_spec.rb +++ b/spec/models/champs/dossier_link_champ_spec.rb @@ -1,9 +1,18 @@ # frozen_string_literal: true describe Champs::DossierLinkChamp, type: :model do + let(:champ) { Champs::DossierLinkChamp.new(value:, dossier: build(:dossier)) } + let(:mandatory) { false } + + before do + allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_dossier_link, mandatory:)) + allow(champ).to receive(:in_dossier_revision?).and_return(true) + end + describe 'prefilling validations' do + let(:linked_dossier) { create(:dossier) } describe 'value' do - subject { described_class.new(value:, dossier: build(:dossier)).valid?(:prefill) } + subject { champ.valid?(:prefill) } context 'when nil' do let(:value) { nil } @@ -18,13 +27,13 @@ describe Champs::DossierLinkChamp, type: :model do end context 'when an integer' do - let(:value) { 42 } + let(:value) { linked_dossier.id } it { expect(subject).to eq(true) } end context 'when a string representing an integer' do - let(:value) { "42" } + let(:value) { linked_dossier.id.to_s } it { expect(subject).to eq(true) } end @@ -38,14 +47,7 @@ describe Champs::DossierLinkChamp, type: :model do end describe 'validation' do - let(:champ) { Champs::DossierLinkChamp.new(value:, dossier: build(:dossier)) } - - before do - allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_dossier_link, mandatory:)) - allow(champ).to receive(:in_dossier_revision?).and_return(true) - champ.run_callbacks(:validation) - end - + before { champ.run_callbacks(:validation) } subject { champ.validate(:champs_public_value) } context 'when not mandatory' do diff --git a/spec/models/champs/epci_champ_spec.rb b/spec/models/champs/epci_champ_spec.rb index 4c07d7bbd..b362773ee 100644 --- a/spec/models/champs/epci_champ_spec.rb +++ b/spec/models/champs/epci_champ_spec.rb @@ -1,15 +1,18 @@ # frozen_string_literal: true describe Champs::EpciChamp, type: :model do + let(:champ) { Champs::EpciChamp.new(code_departement: code_departement, dossier: build(:dossier)) } + let(:code_departement) { nil } + + before do + allow(champ).to receive(:visible?).and_return(true) + allow(champ).to receive(:can_validate?).and_return(true) + end + describe 'validations' do subject { champ.validate(:champs_public_value) } describe 'code_departement' 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(:can_validate?).and_return(true) - end context 'when nil' do let(:code_departement) { nil } @@ -156,7 +159,6 @@ describe Champs::EpciChamp, type: :model do end describe 'value' do - let(:champ) { described_class.new } let(:epci) { APIGeoService.epcis('01').first } it 'with departement and code' do diff --git a/spec/models/champs/piece_justificative_champ_spec.rb b/spec/models/champs/piece_justificative_champ_spec.rb index ae5b6dfb6..c4931719d 100644 --- a/spec/models/champs/piece_justificative_champ_spec.rb +++ b/spec/models/champs/piece_justificative_champ_spec.rb @@ -5,10 +5,12 @@ require 'active_storage_validations/matchers' describe Champs::PieceJustificativeChamp do include ActiveStorageValidations::Matchers + let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :piece_justificative }]) } + let(:dossier) { create(:dossier, :with_populated_champs, procedure:) } + let(:champ) { dossier.champs.first } + describe "validations" do - let(:champ) { Champs::PieceJustificativeChamp.new } subject { champ } - before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_piece_justificative)) } context "by default" do it { is_expected.to validate_size_of(:piece_justificative_file).less_than(Champs::PieceJustificativeChamp::FILE_MAX_SIZE) } @@ -19,20 +21,17 @@ describe Champs::PieceJustificativeChamp do context "when validation is disabled" do before { champ.type_de_champ.update(skip_pj_validation: true) } - it { is_expected.not_to validate_size_of(:piece_justificative_file).less_than(Champs::PieceJustificativeChamp::FILE_MAX_SIZE) } + it { is_expected.not_to validate_size_of(:piece_justificative_file).on(:champs_public_value).less_than(Champs::PieceJustificativeChamp::FILE_MAX_SIZE) } end context "when content-type validation is disabled" do before { champ.type_de_champ.update(skip_content_type_pj_validation: true) } - it { is_expected.not_to validate_content_type_of(:piece_justificative_file).rejecting('application/x-ms-dos-executable') } + it { is_expected.not_to validate_content_type_of(:piece_justificative_file).on(:champs_public_value).rejecting('application/x-ms-dos-executable') } end end describe "#for_export" do - let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :piece_justificative }]) } - let(:dossier) { create(:dossier, :with_populated_champs, procedure:) } - let(:champ) { dossier.champs.first } subject { champ.type_de_champ.champ_value_for_export(champ) } it { is_expected.to eq('toto.txt') } @@ -44,10 +43,6 @@ describe Champs::PieceJustificativeChamp do end describe '#for_api' do - let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :piece_justificative }]) } - let(:dossier) { create(:dossier, :with_populated_champs, procedure:) } - let(:champ) { dossier.champs.first } - before { champ.piece_justificative_file.first.blob.update(virus_scan_result:) } subject { champ.type_de_champ.champ_value_for_api(champ, version: 1) } diff --git a/spec/models/concerns/champ_validate_concern_spec.rb b/spec/models/concerns/champ_validate_concern_spec.rb index 83f5401b4..5e9d7402d 100644 --- a/spec/models/concerns/champ_validate_concern_spec.rb +++ b/spec/models/concerns/champ_validate_concern_spec.rb @@ -51,6 +51,34 @@ RSpec.describe ChampValidateConcern do expect(dossier.errors).to be_empty } end + + context 'attachments' do + let(:types_de_champ_public) { [{ type: :piece_justificative }, { type: :titre_identite }] } + + before { + dossier.revision.revision_types_de_champ.delete_all + dossier.validate(:champs_public_value) + } + it { + expect(dossier.revision.revision_types_de_champ).to be_empty + expect(dossier.champs).not_to be_empty + expect(dossier.errors).to be_empty + } + end + + context 'drop_down_list' do + let(:types_de_champ_public) { [{ type: :drop_down_list }] } + + before { + dossier.revision.revision_types_de_champ.delete_all + dossier.validate(:champs_public_value) + } + it { + expect(dossier.revision.revision_types_de_champ).to be_empty + expect(dossier.champs).not_to be_empty + expect(dossier.errors).to be_empty + } + end end context 'when type changed' do diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 3c919cfd6..5a7d12a8c 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -2133,21 +2133,21 @@ describe Dossier, type: :model do expect(champ_titre_identite.piece_justificative_file.attached?).to be_truthy expect(champ_titre_identite_vide.piece_justificative_file.attached?).to be_falsey dossier.accepter!(instructeur: dossier.followers_instructeurs.first, motivation: "yolo!") - expect(champ_titre_identite.piece_justificative_file.attached?).to be_falsey + expect(Champ.exists?(champ_titre_identite.id)).to be_falsey end it "clean up titres identite on refuser" do expect(champ_titre_identite.piece_justificative_file.attached?).to be_truthy expect(champ_titre_identite_vide.piece_justificative_file.attached?).to be_falsey dossier.refuser!(instructeur: dossier.followers_instructeurs.first, motivation: "yolo!") - expect(champ_titre_identite.piece_justificative_file.attached?).to be_falsey + expect(Champ.exists?(champ_titre_identite.id)).to be_falsey end it "clean up titres identite on classer_sans_suite" do expect(champ_titre_identite.piece_justificative_file.attached?).to be_truthy expect(champ_titre_identite_vide.piece_justificative_file.attached?).to be_falsey dossier.classer_sans_suite!(instructeur: dossier.followers_instructeurs.first, motivation: "yolo!") - expect(champ_titre_identite.piece_justificative_file.attached?).to be_falsey + expect(Champ.exists?(champ_titre_identite.id)).to be_falsey end context 'en_construction' do @@ -2158,7 +2158,7 @@ describe Dossier, type: :model do expect(champ_titre_identite.piece_justificative_file.attached?).to be_truthy expect(champ_titre_identite_vide.piece_justificative_file.attached?).to be_falsey dossier.accepter_automatiquement! - expect(champ_titre_identite.piece_justificative_file.attached?).to be_falsey + expect(Champ.exists?(champ_titre_identite.id)).to be_falsey end end end diff --git a/spec/models/prefill_champs_spec.rb b/spec/models/prefill_champs_spec.rb index fc6e5d4a4..ed3f2e241 100644 --- a/spec/models/prefill_champs_spec.rb +++ b/spec/models/prefill_champs_spec.rb @@ -3,7 +3,8 @@ RSpec.describe PrefillChamps do describe "#to_a", vcr: { cassette_name: 'api_geo_all' } do let(:procedure) { create(:procedure, :published, types_de_champ_public:, types_de_champ_private:) } - let(:dossier) { create(:dossier, :brouillon, procedure: procedure) } + let(:dossier) { create(:dossier, :brouillon, procedure:) } + let(:linked_dossier) { create(:dossier, :en_construction, procedure:) } let(:types_de_champ_public) { [] } let(:types_de_champ_private) { [] } @@ -68,11 +69,12 @@ RSpec.describe PrefillChamps do let(:types_de_champ_public) { [{ type: type_de_champ_type }] } let(:type_de_champ) { procedure.published_revision.types_de_champ_public.first } let(:champ) { find_champ_by_stable_id(dossier, type_de_champ.stable_id) } + let(:champ_value) { value == 'linked_dossier_id' ? linked_dossier.id : value } - let(:params) { { "champ_#{type_de_champ.to_typed_id_for_query}" => value } } + let(:params) { { "champ_#{type_de_champ.to_typed_id_for_query}" => champ_value } } it "builds an array of hash matching the given params" do - expect(prefill_champs_array).to match([{ id: champ.id }.merge(attributes(champ, value))]) + expect(prefill_champs_array).to match([{ id: champ.id }.merge(attributes(champ, champ_value))]) end end end @@ -82,11 +84,12 @@ RSpec.describe PrefillChamps do let(:types_de_champ_private) { [{ type: type_de_champ_type }] } let(:type_de_champ) { procedure.published_revision.types_de_champ_private.first } let(:champ) { find_champ_by_stable_id(dossier, type_de_champ.stable_id) } + let(:champ_value) { value == 'linked_dossier_id' ? linked_dossier.id : value } - let(:params) { { "champ_#{type_de_champ.to_typed_id_for_query}" => value } } + let(:params) { { "champ_#{type_de_champ.to_typed_id_for_query}" => champ_value } } it "builds an array of hash matching the given params" do - expect(prefill_champs_array).to match([{ id: champ.id }.merge(attributes(champ, value))]) + expect(prefill_champs_array).to match([{ id: champ.id }.merge(attributes(champ, champ_value))]) end end end @@ -125,7 +128,7 @@ RSpec.describe PrefillChamps do it_behaves_like "a champ public value that is authorized", :communes, ['01540', '01457'] it_behaves_like "a champ public value that is authorized", :address, "20 avenue de Ségur 75007 Paris" it_behaves_like "a champ public value that is authorized", :multiple_drop_down_list, ["val1", "val2"] - it_behaves_like "a champ public value that is authorized", :dossier_link, "1" + it_behaves_like "a champ public value that is authorized", :dossier_link, 'linked_dossier_id' it_behaves_like "a champ public value that is authorized", :epci, ['01', '200042935'] it_behaves_like "a champ public value that is authorized", :siret, "13002526500013" @@ -167,7 +170,7 @@ RSpec.describe PrefillChamps do it_behaves_like "a champ private value that is authorized", :communes, ['01540', '01457'] it_behaves_like "a champ private value that is authorized", :address, "20 avenue de Ségur 75007 Paris" it_behaves_like "a champ private value that is authorized", :multiple_drop_down_list, ["val1", "val2"] - it_behaves_like "a champ private value that is authorized", :dossier_link, "1" + it_behaves_like "a champ private value that is authorized", :dossier_link, 'linked_dossier_id' it_behaves_like "a champ private value that is authorized", :epci, ['01', '200042935'] context "when the private type de champ is authorized (repetition)" do diff --git a/spec/system/users/dossier_prefill_get_spec.rb b/spec/system/users/dossier_prefill_get_spec.rb index b6271fd8c..fe1e33e4c 100644 --- a/spec/system/users/dossier_prefill_get_spec.rb +++ b/spec/system/users/dossier_prefill_get_spec.rb @@ -19,6 +19,7 @@ describe 'Prefilling a dossier (with a GET request):', js: true do end let(:procedure) { create(:procedure, :for_individual, :published, opendata: true, types_de_champ_public:) } let(:dossier) { procedure.dossiers.last } + let(:linked_dossier) { create(:dossier, :en_construction, procedure:) } let(:types_de_champ) { procedure.active_revision.types_de_champ_public } let(:type_de_champ_text) { types_de_champ[0] } @@ -43,7 +44,7 @@ describe 'Prefilling a dossier (with a GET request):', js: true do ] } let(:epci_value) { ['01', '200029999'] } - let(:dossier_link_value) { '42' } + let(:dossier_link_value) { linked_dossier.id } let(:commune_value) { ['01540', '01457'] } let(:commune_libelle) { 'Vonnas (01540)' } let(:address_value) { "20 Avenue de Ségur 75007 Paris" } diff --git a/spec/system/users/dossier_prefill_post_spec.rb b/spec/system/users/dossier_prefill_post_spec.rb index 1614a3f63..0c8c06582 100644 --- a/spec/system/users/dossier_prefill_post_spec.rb +++ b/spec/system/users/dossier_prefill_post_spec.rb @@ -19,6 +19,7 @@ describe 'Prefilling a dossier (with a POST request):', js: true do end let(:procedure) { create(:procedure, :for_individual, :published, types_de_champ_public:) } let(:dossier) { procedure.dossiers.last } + let(:linked_dossier) { create(:dossier, :en_construction, procedure:) } let(:types_de_champ) { procedure.active_revision.types_de_champ_public } let(:type_de_champ_text) { types_de_champ[0] } @@ -51,7 +52,7 @@ describe 'Prefilling a dossier (with a POST request):', js: true do let(:integer_repetition_libelle) { sub_type_de_champs_repetition.second.libelle } let(:text_repetition_value) { "First repetition text" } let(:integer_repetition_value) { "42" } - let(:dossier_link_value) { '42' } + let(:dossier_link_value) { linked_dossier.id } let(:prenom_value) { 'Jean' } let(:nom_value) { 'Dupont' } let(:genre_value) { 'M.' } From 7462a43886d9c3eb3cff220fbc6e0469eafb5d23 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 10 Dec 2024 10:19:55 +0100 Subject: [PATCH 2/3] fix(dossier): add champs to dossiers in factories --- spec/factories/dossier.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index 47a443dfb..d3a77ed5b 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -295,7 +295,7 @@ def dossier_factory_create_champ_or_repetition(type_de_champ, dossier) types_de_champ = dossier.revision.children_of(type_de_champ) 2.times do row_id = ULID.generate - type_de_champ.build_champ(dossier:, row_id:).save! + dossier.champs << type_de_champ.build_champ(row_id:) types_de_champ.each do |type_de_champ| dossier_factory_create_champ(type_de_champ, dossier, row_id:) end @@ -313,6 +313,6 @@ def dossier_factory_create_champ(type_de_champ, dossier, row_id: nil) elsif type_de_champ.multiple_drop_down_list? type_de_champ.drop_down_options.first(2).to_json end - attrs = { stable_id: type_de_champ.stable_id, private: type_de_champ.private?, row_id:, dossier:, value: }.compact - create(:"champ_do_not_use_#{type_de_champ.type_champ}", **attrs) + attrs = { stable_id: type_de_champ.stable_id, private: type_de_champ.private?, row_id:, value: }.compact + dossier.champs << build(:"champ_do_not_use_#{type_de_champ.type_champ}", **attrs) end From 5dd25fd1b66a0b59b0e885af3a766d50d4900f4d Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 10 Dec 2024 11:20:29 +0100 Subject: [PATCH 3/3] fix(champ): bypass blank? implementation when champ is not in revision --- app/models/champ.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/champ.rb b/app/models/champ.rb index 5a9e79520..1062221dc 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -89,7 +89,8 @@ class Champ < ApplicationRecord end def blank? - type_de_champ.champ_blank?(self) + # FIXME: temporary fix to avoid breaking validation + in_dossier_revision? ? type_de_champ.champ_blank?(self) : value.blank? end def used_by_routing_rules?