Merge pull request #10971 from tchak/refactor-champ-value-blank

refactor(champ): move champ.blank? implementation to type_de_champ
This commit is contained in:
Paul Chavard 2024-11-04 10:28:45 +00:00 committed by GitHub
commit ef8f74c708
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
37 changed files with 185 additions and 142 deletions

View file

@ -99,11 +99,11 @@ class Champ < ApplicationRecord
end
def mandatory_blank?
mandatory? && blank?
type_de_champ.mandatory_blank?(self)
end
def blank?
value.blank?
type_de_champ.champ_blank?(self)
end
def search_terms

View file

@ -85,10 +85,6 @@ class Champs::CarteChamp < Champ
end
end
def blank?
geo_areas.blank?
end
private
def selection_utilisateur_legacy_geometry

View file

@ -1,10 +1,6 @@
# frozen_string_literal: true
class Champs::CheckboxChamp < Champs::BooleanChamp
def mandatory_blank?
mandatory? && (blank? || !true?)
end
def legend_label?
false
end
@ -13,11 +9,6 @@ class Champs::CheckboxChamp < Champs::BooleanChamp
[[I18n.t('activerecord.attributes.type_de_champ.type_champs.checkbox_true'), true], [I18n.t('activerecord.attributes.type_de_champ.type_champs.checkbox_false'), false]]
end
# TODO remove when normalize_checkbox_values is over
def true?
value_with_legacy == TRUE_VALUE
end
def html_label?
false
end
@ -25,11 +16,4 @@ class Champs::CheckboxChamp < Champs::BooleanChamp
def single_checkbox?
true
end
private
# TODO remove when normalize_checkbox_values is over
def value_with_legacy
value == 'on' ? TRUE_VALUE : value
end
end

View file

@ -8,10 +8,6 @@ class Champs::CnafChamp < Champs::TextChamp
store_accessor :value_json, :numero_allocataire, :code_postal
def blank?
external_id.nil?
end
def fetch_external_data?
true
end

View file

@ -20,10 +20,6 @@ class Champs::COJOChamp < Champ
accreditation_success == false
end
def blank?
accreditation_success != true
end
def fetch_external_data?
true
end

View file

@ -7,10 +7,6 @@ class Champs::DgfipChamp < Champs::TextChamp
store_accessor :value_json, :numero_fiscal, :reference_avis
def blank?
external_id.nil?
end
def fetch_external_data?
true
end

View file

@ -35,11 +35,6 @@ class Champs::LinkedDropDownListChamp < Champ
:primary_value
end
def blank?
primary_value.blank? ||
(has_secondary_options_for_primary? && secondary_value.blank?)
end
def search_terms
[primary_value, secondary_value]
end

View file

@ -4,10 +4,6 @@ class Champs::MesriChamp < Champs::TextChamp
# see https://github.com/betagouv/api-particulier/blob/master/src/presentation/middlewares/mesri-input-validation.middleware.ts
store_accessor :value_json, :ine
def blank?
external_id.nil?
end
def fetch_external_data?
true
end

View file

@ -29,10 +29,6 @@ class Champs::MultipleDropDownListChamp < Champ
render_as_checkboxes?
end
def blank?
selected_options.blank?
end
def in?(options)
(selected_options - options).size != selected_options.size
end

View file

@ -35,10 +35,6 @@ class Champs::PaysChamp < Champs::TextChamp
end
end
def blank?
value.blank? && external_id.blank?
end
def code
external_id || APIGeoService.country_code(value)
end

View file

@ -21,12 +21,4 @@ class Champs::PieceJustificativeChamp < Champ
def search_terms
# We dont know how to search inside documents yet
end
def mandatory_blank?
mandatory? && !piece_justificative_file.attached?
end
def blank?
piece_justificative_file.blank?
end
end

View file

@ -4,10 +4,6 @@ class Champs::PoleEmploiChamp < Champs::TextChamp
# see https://github.com/betagouv/api-particulier/blob/master/src/presentation/middlewares/pole-emploi-input-validation.middleware.ts
store_accessor :value_json, :identifiant
def blank?
external_id.nil?
end
def fetch_external_data?
true
end

View file

@ -23,10 +23,6 @@ class Champs::RepetitionChamp < Champ
rows.last&.first&.focusable_input_id
end
def blank?
row_ids.empty?
end
def search_terms
# The user cannot enter any information here so it doesnt make much sense to search
end

View file

@ -27,10 +27,6 @@ class Champs::RNFChamp < Champ
true
end
def blank?
rnf_id.blank?
end
def code_departement
address.present? && address['departmentCode']
end

View file

@ -6,8 +6,4 @@ class Champs::SiretChamp < Champ
def search_terms
etablissement.present? ? etablissement.search_terms : [value]
end
def mandatory_blank?
mandatory? && Siret.new(siret: value).invalid?
end
end

View file

@ -16,12 +16,4 @@ class Champs::TitreIdentiteChamp < Champ
def search_terms
# We dont know how to search inside documents yet
end
def mandatory_blank?
mandatory? && !piece_justificative_file.attached?
end
def blank?
piece_justificative_file.blank?
end
end

View file

@ -709,7 +709,7 @@ class TypeDeChamp < ApplicationRecord
end
def champ_value(champ)
if use_default_value?(champ)
if champ_blank?(champ)
dynamic_type.champ_default_value
else
dynamic_type.champ_value(champ)
@ -717,7 +717,7 @@ class TypeDeChamp < ApplicationRecord
end
def champ_value_for_api(champ, version: 2)
if use_default_value?(champ)
if champ_blank?(champ)
dynamic_type.champ_default_api_value(version)
else
dynamic_type.champ_value_for_api(champ, version:)
@ -725,7 +725,7 @@ class TypeDeChamp < ApplicationRecord
end
def champ_value_for_export(champ, path = :value)
if use_default_value?(champ)
if champ_blank?(champ)
dynamic_type.champ_default_export_value(path)
else
dynamic_type.champ_value_for_export(champ, path)
@ -733,13 +733,35 @@ class TypeDeChamp < ApplicationRecord
end
def champ_value_for_tag(champ, path = :value)
if use_default_value?(champ)
if champ_blank?(champ)
''
else
dynamic_type.champ_value_for_tag(champ, path)
end
end
def champ_blank?(champ)
# no champ
return true if champ.nil?
# type de champ on the revision changed
if champ.last_write_type_champ == type_champ || castable_on_change?(champ.last_write_type_champ, type_champ)
dynamic_type.champ_blank?(champ)
else
true
end
end
def mandatory_blank?(champ)
# no champ
return true if champ.nil?
# type de champ on the revision changed
if champ.last_write_type_champ == type_champ || castable_on_change?(champ.last_write_type_champ, type_champ)
mandatory? && dynamic_type.champ_blank_or_invalid?(champ)
else
true
end
end
def html_id(row_id = nil)
"champ-#{public_id(row_id)}"
end
@ -758,24 +780,12 @@ class TypeDeChamp < ApplicationRecord
private
def use_default_value?(champ)
# no champ
return true if champ.nil?
# type de champ on the revision changed
if champ.last_write_type_champ != type_champ
return !castable_on_change?(champ.last_write_type_champ, type_champ)
end
# special case for linked drop down champ it's blank implementation is not what you think
return champ.value.blank? if type_champ == TypeDeChamp.type_champs.fetch(:linked_drop_down_list)
champ.blank?
end
def castable_on_change?(from_type, to_type)
case [from_type, to_type]
when ['integer_number', 'decimal_number'], # recast numbers automatically
['decimal_number', 'integer_number'], # may lose some data, but who cares ?
['text', 'textarea'], # allow short text to long text
# ['drop_down_list', 'multiple_drop_down_list'], # single list can become multi
['drop_down_list', 'multiple_drop_down_list'], # single list can become multi
['date', 'datetime'], # date <=> datetime
['datetime', 'date'] # may lose some data, but who cares ?
true

View file

@ -28,6 +28,8 @@ class TypesDeChamp::CarteTypeDeChamp < TypesDeChamp::TypeDeChampBase
champ.geo_areas.map(&:label).join("\n")
end
def champ_blank?(champ) = champ.geo_areas.blank?
def columns(procedure_id:, displayable: true, prefix: nil)
[]
end

View file

@ -12,21 +12,17 @@ class TypesDeChamp::CheckboxTypeDeChamp < TypesDeChamp::TypeDeChampBase
end
def champ_value(champ)
champ.true? ? 'Oui' : 'Non'
end
def champ_value_for_tag(champ, path = :value)
champ_value(champ)
champ_value_true?(champ) ? 'Oui' : 'Non'
end
def champ_value_for_export(champ, path = :value)
champ.true? ? 'on' : 'off'
champ_value_true?(champ) ? 'on' : 'off'
end
def champ_value_for_api(champ, version: 2)
case version
when 2
champ.true? ? 'true' : 'false'
champ_value_true?(champ).to_s
else
super
end
@ -48,4 +44,10 @@ class TypesDeChamp::CheckboxTypeDeChamp < TypesDeChamp::TypeDeChampBase
nil
end
end
def champ_blank_or_invalid?(champ) = !champ_value_true?(champ)
private
def champ_value_true?(champ) = champ.value == 'true'
end

View file

@ -4,4 +4,6 @@ class TypesDeChamp::CnafTypeDeChamp < TypesDeChamp::TextTypeDeChamp
def estimated_fill_duration(revision)
FILL_DURATION_MEDIUM
end
def champ_blank?(champ) = champ.external_id.blank?
end

View file

@ -4,4 +4,6 @@ class TypesDeChamp::COJOTypeDeChamp < TypesDeChamp::TextTypeDeChamp
def champ_value(champ)
"#{champ.accreditation_number} #{champ.accreditation_birthdate}"
end
def champ_blank?(champ) = champ.accreditation_success != true
end

View file

@ -4,4 +4,6 @@ class TypesDeChamp::DgfipTypeDeChamp < TypesDeChamp::TextTypeDeChamp
def estimated_fill_duration(revision)
FILL_DURATION_MEDIUM
end
def champ_blank?(champ) = champ.external_id.blank?
end

View file

@ -11,11 +11,6 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
[[path[:libelle], path[:path]]]
end
def add_blank_option_when_not_mandatory(options)
return options if mandatory
options.unshift('')
end
def primary_options
primary_options = unpack_options.map(&:first)
if primary_options.present?
@ -33,15 +28,15 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
end
def champ_value(champ)
[champ.primary_value, champ.secondary_value].filter(&:present?).join(' / ')
[primary_value(champ), secondary_value(champ)].filter(&:present?).join(' / ')
end
def champ_value_for_tag(champ, path = :value)
case path
when :primary
champ.primary_value
primary_value(champ)
when :secondary
champ.secondary_value
secondary_value(champ)
when :value
champ_value(champ)
end
@ -50,23 +45,32 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
def champ_value_for_export(champ, path = :value)
case path
when :primary
champ.primary_value
primary_value(champ)
when :secondary
champ.secondary_value
secondary_value(champ)
when :value
"#{champ.primary_value || ''};#{champ.secondary_value || ''}"
"#{primary_value(champ) || ''};#{secondary_value(champ) || ''}"
end
end
def champ_value_for_api(champ, version: 2)
case version
when 1
{ primary: champ.primary_value, secondary: champ.secondary_value }
{ primary: primary_value(champ), secondary: secondary_value(champ) }
else
super
end
end
def champ_blank?(champ)
primary_value(champ).blank? && secondary_value(champ).blank?
end
def champ_blank_or_invalid?(champ)
primary_value(champ).blank? ||
(has_secondary_options_for_primary?(champ) && secondary_value(champ).blank?)
end
def columns(procedure_id:, displayable: true, prefix: nil)
[
Columns::LinkedDropDownColumn.new(
@ -101,6 +105,19 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
private
def add_blank_option_when_not_mandatory(options)
return options if mandatory
options.unshift('')
end
def primary_value(champ) = unpack_value(champ.value, 0)
def secondary_value(champ) = unpack_value(champ.value, 1)
def unpack_value(value, index) = value&.then { JSON.parse(_1)[index] rescue nil }
def has_secondary_options_for_primary?(champ)
primary_value(champ).present? && secondary_options[primary_value(champ)]&.any?(&:present?)
end
def paths
paths = super
paths.push({

View file

@ -4,4 +4,6 @@ class TypesDeChamp::MesriTypeDeChamp < TypesDeChamp::TextTypeDeChamp
def estimated_fill_duration(revision)
FILL_DURATION_MEDIUM
end
def champ_blank?(champ) = champ.external_id.blank?
end

View file

@ -2,14 +2,30 @@
class TypesDeChamp::MultipleDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBase
def champ_value(champ)
champ.selected_options.join(', ')
selected_options(champ).join(', ')
end
def champ_value_for_tag(champ, path = :value)
ChampPresentations::MultipleDropDownListPresentation.new(champ.selected_options)
ChampPresentations::MultipleDropDownListPresentation.new(selected_options(champ))
end
def champ_value_for_export(champ, path = :value)
champ.selected_options.join(', ')
champ_value(champ)
end
def champ_blank?(champ) = selected_options(champ).blank?
private
def selected_options(champ)
return [] if champ.value.blank?
if champ.last_write_type_champ == TypeDeChamp.type_champs.fetch(:drop_down_list)
[champ.value]
else
JSON.parse(champ.value)
end
rescue JSON::ParserError
[]
end
end

View file

@ -23,6 +23,10 @@ class TypesDeChamp::PaysTypeDeChamp < TypesDeChamp::TextTypeDeChamp
end
end
def champ_blank?(champ)
champ.value.blank? && champ.external_id.blank?
end
private
def paths

View file

@ -23,6 +23,8 @@ class TypesDeChamp::PieceJustificativeTypeDeChamp < TypesDeChamp::TypeDeChampBas
end
end
def champ_blank?(champ) = champ.piece_justificative_file.blank?
def columns(procedure_id:, displayable: true, prefix: nil)
[]
end

View file

@ -4,4 +4,6 @@ class TypesDeChamp::PoleEmploiTypeDeChamp < TypesDeChamp::TextTypeDeChamp
def estimated_fill_duration(revision)
FILL_DURATION_MEDIUM
end
def champ_blank?(champ) = champ.external_id.blank?
end

View file

@ -3,9 +3,7 @@
class TypesDeChamp::RepetitionTypeDeChamp < TypesDeChamp::TypeDeChampBase
def champ_value_for_tag(champ, path = :value)
return nil if path != :value
return champ_default_value if champ.rows.blank?
ChampPresentations::RepetitionPresentation.new(champ.libelle, champ.rows)
ChampPresentations::RepetitionPresentation.new(libelle, champ.dossier.project_rows_for(@type_de_champ))
end
def estimated_fill_duration(revision)
@ -32,4 +30,6 @@ class TypesDeChamp::RepetitionTypeDeChamp < TypesDeChamp::TypeDeChampBase
.all_revisions_types_de_champ(parent: @type_de_champ)
.flat_map { _1.columns(procedure_id:, displayable: false, prefix: libelle) }
end
def champ_blank?(champ) = champ.dossier.repetition_row_ids(@type_de_champ).blank?
end

View file

@ -33,6 +33,8 @@ class TypesDeChamp::RNFTypeDeChamp < TypesDeChamp::TextTypeDeChamp
end
end
def champ_blank?(champ) = champ.external_id.blank?
private
def paths

View file

@ -6,4 +6,6 @@ class TypesDeChamp::SiretTypeDeChamp < TypesDeChamp::TypeDeChampBase
def estimated_fill_duration(revision)
FILL_DURATION_MEDIUM
end
def champ_blank_or_invalid?(champ) = Siret.new(siret: champ.value).invalid?
end

View file

@ -22,6 +22,8 @@ class TypesDeChamp::TitreIdentiteTypeDeChamp < TypesDeChamp::TypeDeChampBase
"absent"
end
def champ_blank?(champ) = champ.piece_justificative_file.blank?
def columns(procedure_id:, displayable: nil, prefix: nil)
[
Columns::TitreIdentiteColumn.new(

View file

@ -92,6 +92,9 @@ class TypesDeChamp::TypeDeChampBase
end
end
def champ_blank?(champ) = champ.value.blank?
def champ_blank_or_invalid?(champ) = champ_blank?(champ)
def columns(procedure_id:, displayable: true, prefix: nil)
if fillable?
[

View file

@ -1,6 +1,6 @@
# frozen_string_literal: true
class TypesDeChamp::YesNoTypeDeChamp < TypesDeChamp::CheckboxTypeDeChamp
class TypesDeChamp::YesNoTypeDeChamp < TypesDeChamp::TypeDeChampBase
def filter_to_human(filter_value)
if filter_value == "true"
I18n.t('activerecord.attributes.type_de_champ.type_champs.yes_no_true')
@ -12,21 +12,17 @@ class TypesDeChamp::YesNoTypeDeChamp < TypesDeChamp::CheckboxTypeDeChamp
end
def champ_value(champ)
champ_formatted_value(champ)
end
def champ_value_for_tag(champ, path = :value)
champ_formatted_value(champ)
champ_value_true?(champ) ? 'Oui' : 'Non'
end
def champ_value_for_export(champ, path = :value)
champ_formatted_value(champ)
champ_value_true?(champ) ? 'Oui' : 'Non'
end
def champ_value_for_api(champ, version: 2)
case version
when 2
champ.true? ? 'true' : 'false'
champ_value_true?(champ).to_s
else
super
end
@ -42,7 +38,7 @@ class TypesDeChamp::YesNoTypeDeChamp < TypesDeChamp::CheckboxTypeDeChamp
private
def champ_formatted_value(champ)
champ.true? ? 'Oui' : 'Non'
def champ_value_true?(champ)
champ.value == 'true'
end
end

View file

@ -5,7 +5,7 @@ describe Champ do
describe 'mandatory_blank?' do
let(:type_de_champ) { build(:type_de_champ, mandatory: mandatory) }
let(:champ) { Champ.new(value: value) }
let(:champ) { Champs::TextChamp.new(value: value) }
let(:value) { '' }
let(:mandatory) { true }

View file

@ -9,12 +9,6 @@ describe Champs::CheckboxChamp do
describe '#true?' do
subject { boolean_champ.true? }
context "when the checkbox value is 'on'" do
let(:value) { 'on' }
it { is_expected.to eq(true) }
end
context "when the checkbox value is 'off'" do
let(:value) { 'off' }

View file

@ -413,4 +413,66 @@ describe TypeDeChamp do
end
end
end
describe 'champ_value with cast' do
let(:procedure) { create(:procedure, types_de_champ_public: [{ type: type_champ }]) }
let(:dossier) { create(:dossier, procedure:) }
let(:type_champ) { :text }
let(:last_write_type_champ) { :text }
let(:champ_value) { 'hello' }
let(:champ_type) { TypeDeChamp.type_champ_to_champ_class_name(last_write_type_champ.to_s) }
let(:type_de_champ) { procedure.active_revision.types_de_champ.first }
let(:champ) { dossier.champs.first }
subject { champ.update_columns(type: champ_type, value: champ_value); type_de_champ.champ_value(champ) }
it { expect(subject).to eq('hello') }
context 'text -> integer_number' do
let(:last_write_type_champ) { :text }
let(:type_champ) { :integer_number }
it { expect(subject).to eq('') }
end
context 'integer_number -> text' do
let(:last_write_type_champ) { :integer_number }
let(:type_champ) { :text }
let(:champ_value) { '42' }
it { expect(subject).to eq('') }
end
context 'integer_number -> decimal_number' do
let(:last_write_type_champ) { :integer_number }
let(:type_champ) { :decimal_number }
let(:champ_value) { '42' }
it { expect(subject).to eq('42') }
end
context 'decimal_number -> integer_number' do
let(:last_write_type_champ) { :decimal_number }
let(:type_champ) { :integer_number }
let(:champ_value) { '42.1' }
it { expect(subject).to eq('42.1') }
end
context 'drop_down_list -> multiple_drop_down_list' do
let(:last_write_type_champ) { :drop_down_list }
let(:type_champ) { :multiple_drop_down_list }
let(:champ_value) { type_de_champ.drop_down_options.first }
it { expect(subject).to eq(champ_value) }
end
context 'multiple_drop_down_list -> drop_down_list' do
let(:last_write_type_champ) { :multiple_drop_down_list }
let(:type_champ) { :drop_down_list }
let(:champ_value) { "[\"#{type_de_champ.drop_down_options.first}\"]" }
it { expect(subject).to eq('') }
end
end
end