Skip to content

Tech: dettes de refactoring restantes du screening de préremplissage (validators partagés) #13458

Description

@tchak

Follow-ups from the multi-agent review of the prefill screening / shared validators stack (July 2026). None of these are bugs today — each was verified to have no observable divergence in current behavior. They are drift-risk and duplication items, ranked by expected payoff.

1. Give the drop-down "value in options" rule a single owner

Where: app/validators/drop_down_options_validator.rb, app/models/champs/drop_down_list_champ.rb, app/models/types_de_champ/prefill_drop_down_list_type_de_champ.rb

Current state. The rule "a drop-down value must be one of the configured options" is now enforced by three cooperating but independent pieces:

  • DropDownOptionsValidator.violations — used by prefill screening (single and multiple lists) and, via its instance validate, by Champs::MultipleDropDownListChamp. Reads type_de_champ.drop_down_options; advanced mode checks against caller-supplied referentiel_keys.
  • Champs::DropDownListChamp#validate_value_is_in_options — the single drop-down champ never routes through the validator; it goes value_is_in_options?TypeDeChamp#options_for_selectoptions_for_select_with_other, a different method chain that happens to bottom out on the same drop_down_options for simple lists, and on referentiel items (all items, via DB) for advanced lists.
  • PrefillDropDownListTypeDeChamp#screenable? (drop_down_simple? && !drop_down_other?) — the decision of which modes get screened at all lives in the prefill class, not in the validator that owns the rest of the mode logic.

Risk. A change to advanced/other handling, or to how options are compared (trimming, case), must be replicated in three places; prefill screening and champ validation can silently disagree for single drop-downs.

Suggested direction. Make Champs::DropDownListChamp validate through DropDownOptionsValidator, and fold the mode-gating into the validator. The awkward part to resolve first: advanced single lists validate against all referentiel items (options_for_select), while advanced multiple lists validate against the referentiel items stored on the champ (referentiels.keys). Either the validator learns both allowed-set sources (e.g. a small per-champ adapter supplying allowed_values), or the two champs are first unified on one source. There's also a policy question to settle: prefill currently accepts anything for advanced single lists but rejects everything for advanced multiple lists (both inherited from pre-existing behavior).

Effort: medium — the only item here needing a design decision, not just mechanics.

2. Extract geo resolution helpers shared by champ setters and prefill screens

Where: app/models/types_de_champ/prefill_departement_type_de_champ.rb, prefill_region_type_de_champ.rb, prefill_pays_type_de_champ.rb, plus Champs::DepartementChamp#value=, RegionChamp#value=, PaysChamp#value=

Current state. Each prefill class re-implements its champ setter's size-based code-vs-name dispatch, and says so (# Mirrors Champs::DepartementChamp#value=): [2, 3].include?(size) for departement, size == 2 for region/pays, locale: 'FR' for country name lookups. The review verified the mirrors are exact today, including the lenient humanize/transliterate country-name lookup.

Risk. Any change to a setter (new accepted format, fuzzy matching, locale change) silently desynchronizes the screen — it will either reject prefill values the setter would accept, or accept values the setter then nils out (the very hole the screening was built to close).

Suggested direction. One resolution helper per referential, e.g. APIGeoService.resolve_departement(input) → { code:, name: } | nil (and resolve_region, resolve_country). The setter assigns external_id/value from the resolved pair; the prefill screen just checks resolve_*(value).present?. The "Mirrors …" comments disappear, and the three near-identical prefill classes collapse into one class parameterized by the resolver (same shape as item 4).

Effort: medium-small; pure refactor, fully covered by existing champ and prefill specs.

3. One parser for multiple drop-down input

Where: app/models/types_de_champ/prefill_multiple_drop_down_list_type_de_champ.rb (extract_values), Champs::MultipleDropDownListChamp#value=

Current state. extract_values duplicates the setter's three-branch parsing (Array / JSON string starting with [ / scalar), with two intentional-looking deviations: .compact_blank vs the setter's .without(''), and no append to selected_options (a no-op at prefill time since the champ is fresh — verified). The champ-side validation re-applies compact_blank, which is why nothing diverges today.

Suggested direction. Extract one parser — e.g. a class method Champs::MultipleDropDownListChamp.parse_values(input) — used by both the setter (which adds its append semantics on top) and the prefill screen. While there, extract_values can drop its own compact_blank since DropDownOptionsValidator.violations compacts internally.

Effort: small.

4. Merge the byte-identical number prefill classes

Where: app/models/types_de_champ/prefill_integer_number_type_de_champ.rb, prefill_decimal_number_type_de_champ.rb, the two case branches in TypesDeChamp::PrefillTypeDeChamp.build

Current state. The two to_assignable_attributes bodies are byte-for-byte identical — the integer/decimal distinction already lives entirely inside NumberFormatValidator and NumberLimitValidator via type_de_champ.integer_number?.

Verified safe to merge: example_value's I18n.t("...examples.#{type_champ}") keys off the delegated type_champ (integer_number / decimal_number), not the Ruby class, so a single PrefillNumberTypeDeChamp mapped from both build branches preserves all type-specific behavior.

Effort: trivial — delete one file, merge two case branches; keep both spec files.

5. Share the date/datetime screening flow

Where: app/models/types_de_champ/prefill_date_type_de_champ.rb, prefill_datetime_type_de_champ.rb

Current state. The two classes differ by exactly one token: DateDetectionUtils.convert_to_iso8601_date vs convert_to_iso8601_datetime. Everything else (String guard, nil check, DateLimitValidator.violations, return shape) is duplicated.

Suggested direction. A shared base with one overridable hook (to_iso8601(value)), or a single class branching on datetime?. Same I18n-safety argument as item 4 (verified).

Effort: trivial. Items 4 and 5 make a natural single "Tech:" PR.

Considered and rejected — don't re-open without new evidence

  • Individual::GENDERS shared constant. The stack strictly improved civilite (replaced hardcoded "M."/"Mme" with the Individual constants). The remaining ~5 copies of the pair (prefill_identity.rb, columns_concern.rb, FranceConnect concern…) predate the stack. Extracting Individual::GENDERS is a fine standalone cleanup, but it's codebase-wide, not a stack follow-up.
  • Collapsing :not_a_number + :not_a_float in NumberFormatValidator. The dual keys faithfully reproduce the old numericality + format pair, which could already emit both errors for one input. Collapsing them changes user-visible error messages — a product decision, not a refactor.
  • Changing the violations → [[key, details]] return shape (details is always {} in three of the five validators). The shape matches DateLimitValidator/NumberLimitValidator, which genuinely need details; introducing a second shape would cost more than the empty hashes do.

Open decision to confirm

Boolean cast semantics (shipped in the stack): ActiveModel::Type::Boolean casts any unrecognized non-empty string ("oui", "value") to true — standard Rails checkbox-param behavior, and better than the previous coerce-everything-to-false, but if rejecting unrecognized strings at prefill is preferred, swap the cast for an explicit allowlist in PrefillBooleanTypeDeChamp#cast_value.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions