Skip to content

Commit f71a8f7

Browse files
committed
Remove the :prefill validation context
Prefill input is now fully screened by the PrefillTypeDeChamp classes, so champs no longer need to validate under a :prefill context: - PrefillChamps::PrefillValue no longer assigns attributes to run champ.valid?(:prefill); prefillable? only checks that screening produced attributes. - ChampValidateConcern drops the :prefill branch. - DossierLinkChamp drops the prefill-only value_integerable validation. - PaysChamp drops its stricter prefill-only inclusion validations. - DateChamp converts its guarded before_validation into normalizes, for symmetry with DatetimeChamp. Since screening no longer mutates the champ, a rejected prefill value can no longer leak to the database through the dossier champ_data autosave; a regression spec covers this.
1 parent 45797fb commit f71a8f7

10 files changed

Lines changed: 35 additions & 121 deletions

File tree

app/models/champs/date_champ.rb

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ class Champs::DateChamp < Champ
44
attr_accessor :prefilling_from_france_connect_information
55

66
validates_with DateLimitValidator, if: :should_validate_in_current_context?
7-
before_validation :convert_to_iso8601_date, unless: -> { validation_context == :prefill }
7+
normalizes :value, with: -> { DateDetectionUtils.convert_to_iso8601_date(it) }
88
before_save :clear_prefilled_from_france_connect_information_flag_if_modified
99
validate :iso_8601
1010

@@ -22,10 +22,6 @@ def clear_prefilled_from_france_connect_information_flag_if_modified
2222
data.delete("prefilled_from_france_connect_information")
2323
end
2424

25-
def convert_to_iso8601_date
26-
self.value = DateDetectionUtils.convert_to_iso8601_date(value)
27-
end
28-
2925
def iso_8601
3026
return if DateDetectionUtils.parsable_iso8601_date?(value) || value.blank?
3127

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,5 @@
11
# frozen_string_literal: true
22

33
class Champs::DossierLinkChamp < Champ
4-
validate :value_integerable, if: -> { value.present? }, on: :prefill
54
validates_with DossierLinkValidator, if: -> { should_validate_in_current_context? && value.present? }
6-
7-
private
8-
9-
def value_integerable
10-
Integer(value)
11-
rescue ArgumentError
12-
errors.add(:value, :not_integerable)
13-
end
145
end

app/models/champs/pays_champ.rb

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,6 @@ class Champs::PaysChamp < Champs::TextChamp
66
validates :value, inclusion: APIGeoService.countries.pluck(:name), allow_nil: true, allow_blank: false
77
end
88

9-
# def value=(code) can reset champs to nil if value is empty, in case of prefill
10-
# we do not want to try to save the champ with an nil value
11-
with_options if: -> { validation_context == :prefill } do
12-
validates :external_id, inclusion: APIGeoService.countries.pluck(:code), allow_nil: false, allow_blank: false
13-
validates :value, inclusion: APIGeoService.countries.pluck(:name), allow_nil: false, allow_blank: false
14-
end
15-
169
def selected
1710
code || value
1811
end

app/models/concerns/champ_validate_concern.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ def should_validate_in_current_context?
1313
case validation_context
1414
when :champ_value
1515
visible?
16-
when :prefill
17-
true
1816
else
1917
false
2018
end

app/models/prefill_champs.rb

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -31,22 +31,6 @@ def build_prefill_values
3131
end
3232

3333
class PrefillValue
34-
NEED_VALIDATION_TYPES_DE_CHAMPS = [
35-
TypeDeChamp.type_champs.fetch(:decimal_number),
36-
TypeDeChamp.type_champs.fetch(:integer_number),
37-
TypeDeChamp.type_champs.fetch(:date),
38-
TypeDeChamp.type_champs.fetch(:datetime),
39-
TypeDeChamp.type_champs.fetch(:civilite),
40-
TypeDeChamp.type_champs.fetch(:yes_no),
41-
TypeDeChamp.type_champs.fetch(:checkbox),
42-
TypeDeChamp.type_champs.fetch(:pays),
43-
TypeDeChamp.type_champs.fetch(:regions),
44-
TypeDeChamp.type_champs.fetch(:departements),
45-
TypeDeChamp.type_champs.fetch(:multiple_drop_down_list),
46-
TypeDeChamp.type_champs.fetch(:epci),
47-
TypeDeChamp.type_champs.fetch(:dossier_link),
48-
]
49-
5034
attr_reader :champ, :value, :dossier
5135

5236
def initialize(champ:, value:, dossier:)
@@ -56,7 +40,7 @@ def initialize(champ:, value:, dossier:)
5640
end
5741

5842
def prefillable?
59-
champ.prefillable? && champ_attributes.present? && valid?
43+
champ.prefillable? && champ_attributes.present?
6044
end
6145

6246
# An array of [champ, attributes] pairs; a repetition champ expands to
@@ -70,14 +54,5 @@ def champ_attributes
7054
.build(champ.type_de_champ, dossier.revision)
7155
.to_assignable_attributes(champ, value)
7256
end
73-
74-
private
75-
76-
def valid?
77-
return true unless NEED_VALIDATION_TYPES_DE_CHAMPS.include?(champ.type_champ)
78-
79-
champ.assign_attributes(champ_attributes)
80-
champ.valid?(:prefill)
81-
end
8257
end
8358
end

config/locales/en.yml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -855,10 +855,6 @@ en:
855855
not_in_departement_epci_codes: "must be a valid EPCI code of the matching department"
856856
value:
857857
not_in_departement_epci_names: "must be a valid EPCI name of the matching department"
858-
"champs/dossier_link_champ":
859-
attributes:
860-
value:
861-
not_integerable: "must be an integer"
862858
"champs/repetition_champ":
863859
attributes:
864860
value:

config/locales/fr.yml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -866,10 +866,6 @@ fr:
866866
not_in_departement_epci_codes: "doit être un code d’EPCI du département correspondant"
867867
value:
868868
not_in_departement_epci_names: "doit être un nom d’EPCI du département correspondant"
869-
"champs/dossier_link_champ":
870-
attributes:
871-
value:
872-
not_integerable: "doit être un entier"
873869
"champs/repetition_champ":
874870
attributes:
875871
value:

spec/models/champs/date_champ_spec.rb

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,53 +6,37 @@
66
let(:dossier) { create(:dossier, procedure:) }
77
let(:date_champ) { dossier.champ_data.first }
88

9-
describe '#convert_to_iso8601_date' do
9+
describe 'value normalization' do
1010
it 'preserves nil' do
11-
champ = champ_with_value(nil)
12-
champ.validate
13-
expect(champ.value).to be_nil
11+
expect(champ_with_value(nil).value).to be_nil
1412
end
1513

1614
it 'converts to nil if empty string' do
17-
champ = champ_with_value("")
18-
champ.validate
19-
expect(champ.value).to be_nil
15+
expect(champ_with_value("").value).to be_nil
2016
end
2117

2218
it 'converts unambiguous US format to ISO' do
23-
champ = champ_with_value("12-21-2023")
24-
champ.validate
25-
expect(champ.value).to eq("2023-12-21")
19+
expect(champ_with_value("12-21-2023").value).to eq("2023-12-21")
2620
end
2721

2822
it 'converts to nil if not a valid date' do
29-
champ = champ_with_value("13/13/2023")
30-
champ.validate
31-
expect(champ.value).to be_nil
23+
expect(champ_with_value("13/13/2023").value).to be_nil
3224
end
3325

3426
it 'converts to nil if not date' do
35-
champ = champ_with_value("value")
36-
champ.validate
37-
expect(champ.value).to be_nil
27+
expect(champ_with_value("value").value).to be_nil
3828
end
3929

4030
it "converts %d/%m/%Y format to ISO" do
41-
champ = champ_with_value("31/12/2017")
42-
champ.validate
43-
expect(champ.value).to eq("2017-12-31")
31+
expect(champ_with_value("31/12/2017").value).to eq("2017-12-31")
4432
end
4533

4634
it 'preserves if ISO8601' do
47-
champ = champ_with_value("2023-12-21")
48-
champ.validate
49-
expect(champ.value).to eq("2023-12-21")
35+
expect(champ_with_value("2023-12-21").value).to eq("2023-12-21")
5036
end
5137

5238
it 'converts to nil if false iso' do
53-
champ = champ_with_value("2023-27-02")
54-
champ.validate
55-
expect(champ.value).to eq(nil)
39+
expect(champ_with_value("2023-27-02").value).to eq(nil)
5640
end
5741
end
5842

@@ -62,8 +46,8 @@
6246
expect(date_champ.to_s).to eq("20 juin 2020")
6347
end
6448

65-
it "does not fail when value is not iso" do
66-
champ_with_value("2023-30-01")
49+
it "does not fail when a legacy value stored in database is not iso" do
50+
allow(date_champ).to receive(:value).and_return("2023-30-01")
6751
expect(date_champ.to_s).to eq("2023-30-01")
6852
end
6953
end

spec/models/champs/dossier_link_champ_spec.rb

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,43 +8,6 @@
88
let(:value) { nil }
99
let(:mandatory) { false }
1010

11-
describe 'prefilling validations' do
12-
let(:linked_dossier) { create(:dossier, :en_construction) }
13-
describe 'value' do
14-
subject { champ.valid?(:prefill) }
15-
16-
context 'when nil' do
17-
let(:value) { nil }
18-
19-
it { expect(subject).to eq(true) }
20-
end
21-
22-
context 'when empty' do
23-
let(:value) { '' }
24-
25-
it { expect(subject).to eq(true) }
26-
end
27-
28-
context 'when an integer' do
29-
let(:value) { linked_dossier.id }
30-
31-
it { expect(subject).to eq(true) }
32-
end
33-
34-
context 'when a string representing an integer' do
35-
let(:value) { linked_dossier.id.to_s }
36-
37-
it { expect(subject).to eq(true) }
38-
end
39-
40-
context 'when it can be casted as integer' do
41-
let(:value) { 'totoro' }
42-
43-
it { expect(subject).to eq(false) }
44-
end
45-
end
46-
end
47-
4811
describe 'validation' do
4912
subject { champ.validate(:champ_value) }
5013

spec/models/concerns/dossier_prefillable_concern_spec.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,28 @@
140140
end
141141
end
142142

143+
context 'when a prefill value was rejected by the PrefillTypeDeChamp screening' do
144+
# Regression: rejected prefill values used to be assigned to the champ while
145+
# screening with champ.valid?(:prefill), and could leak to the database
146+
# through the dossier champ_data autosave when a valid champ triggered a save.
147+
let(:types_de_champ_public) { [{ type: :text }, { type: :date }] }
148+
let(:text_type_de_champ) { procedure.published_revision.types_de_champ_public.first }
149+
let(:date_type_de_champ) { procedure.published_revision.types_de_champ_public.second }
150+
let(:params) do
151+
{
152+
"champ_#{text_type_de_champ.to_typed_id_for_query}" => "any value",
153+
"champ_#{date_type_de_champ.to_typed_id_for_query}" => "not a date",
154+
}
155+
end
156+
let(:values) { PrefillChamps.new(dossier, params).to_a }
157+
158+
it 'prefills the valid champ and does not persist the rejected value' do
159+
fill
160+
expect(find_champ_by_stable_id(dossier, text_type_de_champ.stable_id).value).to eq("any value")
161+
expect(find_champ_by_stable_id(dossier, date_type_de_champ.stable_id)&.value).to be_nil
162+
end
163+
end
164+
143165
context 'when dossier contains champs with external_id' do
144166
let(:types_de_champ_public) { [{ type: :siret }] }
145167
let(:values) { [[champ_1, { external_id: value_1 }]] }

0 commit comments

Comments
 (0)