From a5b8b936d5b6d813593c731d121c1c9a65099940 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 31 Oct 2024 18:28:47 +0100 Subject: [PATCH 01/12] a column can override its column_id --- app/models/column.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/column.rb b/app/models/column.rb index b9c223fe8..c4d54b6c5 100644 --- a/app/models/column.rb +++ b/app/models/column.rb @@ -27,7 +27,7 @@ class Column # the h_id is a Hash and hold enough information to find the column # in the ColumnType class, aka be able to do the h_id -> column conversion - def h_id = { procedure_id: @procedure_id, column_id: "#{table}/#{column}" } + def h_id = { procedure_id: @procedure_id, column_id: } def ==(other) = h_id == other.h_id # using h_id instead of id to avoid inversion of keys @@ -65,6 +65,8 @@ class Column private + def column_id = "#{table}/#{column}" + def typed_value(champ) value = string_value(champ) parse_value(value, type: champ.last_write_column_type) From d03d5d0daee0281e00d2be5d839e80f583574188 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 31 Oct 2024 18:29:23 +0100 Subject: [PATCH 02/12] add ChampColumn --- app/models/column.rb | 89 --------------- app/models/columns/champ_column.rb | 105 ++++++++++++++++++ .../types_de_champ/type_de_champ_base.rb | 5 +- .../column_filter_value_component_spec.rb | 2 +- spec/models/column_spec.rb | 91 --------------- spec/models/columns/champ_column_spec.rb | 61 ++++++++++ spec/models/columns/dossier_column_spec.rb | 39 +++++++ 7 files changed, 208 insertions(+), 184 deletions(-) create mode 100644 app/models/columns/champ_column.rb create mode 100644 spec/models/columns/champ_column_spec.rb create mode 100644 spec/models/columns/dossier_column_spec.rb diff --git a/app/models/column.rb b/app/models/column.rb index c4d54b6c5..87d53aae1 100644 --- a/app/models/column.rb +++ b/app/models/column.rb @@ -52,96 +52,7 @@ class Column procedure.find_column(h_id: h_id) end - def value(champ) - return if champ.nil? - - value = typed_value(champ) - if default_column? - cast_value(value, from_type: champ.last_write_column_type, to_type: type) - else - value - end - end - private def column_id = "#{table}/#{column}" - - def typed_value(champ) - value = string_value(champ) - parse_value(value, type: champ.last_write_column_type) - end - - def string_value(champ) = champ.public_send(value_column) - def default_column? = value_column.in?([:value, :external_id]) - - def parse_value(value, type:) - return if value.blank? - - case type - when :boolean - parse_boolean(value) - when :integer - value.to_i - when :decimal - value.to_f - when :datetime - parse_datetime(value) - when :date - parse_datetime(value)&.to_date - when :enums - parse_enums(value) - else - value - end - end - - def cast_value(value, from_type:, to_type:) - return if value.blank? - return value if from_type == to_type - - case [from_type, to_type] - when [:integer, :decimal] # recast numbers automatically - value.to_f - when [:decimal, :integer] # may lose some data, but who cares ? - value.to_i - when [:integer, :text], [:decimal, :text] # number to text - value.to_s - when [:enum, :enums] # single list can become multi - [value] - when [:enum, :text] # single list can become text - value - when [:enums, :enum] # multi list can become single list - value.first - when [:enums, :text] # multi list can become text - value.join(', ') - when [:date, :datetime] # date <=> datetime - value.to_datetime - when [:datetime, :date] # may lose some data, but who cares ? - value.to_date - else - nil - end - end - - def parse_boolean(value) - case value - when 'true', 'on', '1' - true - when 'false' - false - end - end - - def parse_enums(value) - JSON.parse(value) - rescue JSON::ParserError - nil - end - - def parse_datetime(value) - Time.zone.parse(value) - rescue ArgumentError - nil - end end diff --git a/app/models/columns/champ_column.rb b/app/models/columns/champ_column.rb new file mode 100644 index 000000000..e60472284 --- /dev/null +++ b/app/models/columns/champ_column.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +class Columns::ChampColumn < Column + attr_reader :stable_id + + def initialize(procedure_id:, label:, stable_id:, displayable: true, filterable: true, type: :text, value_column: :value) + @stable_id = stable_id + + super( + procedure_id:, + table: 'type_de_champ', + column: stable_id.to_s, + label:, + type:, + value_column:, + displayable:, + filterable: + ) + end + + def value(champ) + return if champ.nil? + + value = typed_value(champ) + if default_column? + cast_value(value, from_type: champ.last_write_column_type, to_type: type) + else + value + end + end + + private + + def column_id = "type_de_champ/#{stable_id}" + + def typed_value(champ) + value = string_value(champ) + parse_value(value, type: champ.last_write_column_type) + end + + def string_value(champ) = champ.public_send(value_column) + def default_column? = value_column.in?([:value, :external_id]) + + def parse_value(value, type:) + return if value.blank? + + case type + when :boolean + parse_boolean(value) + when :integer + value.to_i + when :decimal + value.to_f + when :datetime + parse_datetime(value) + when :date + parse_datetime(value)&.to_date + when :enums + parse_enums(value) + else + value + end + end + + def cast_value(value, from_type:, to_type:) + return if value.blank? + return value if from_type == to_type + + case [from_type, to_type] + when [:integer, :decimal] # recast numbers automatically + value.to_f + when [:decimal, :integer] # may lose some data, but who cares ? + value.to_i + when [:integer, :text], [:decimal, :text] # number to text + value.to_s + when [:enum, :enums] # single list can become multi + [value] + when [:enum, :text] # single list can become text + value + when [:enums, :enum] # multi list can become single list + value.first + when [:enums, :text] # multi list can become text + value.join(', ') + when [:date, :datetime] # date <=> datetime + value.to_datetime + when [:datetime, :date] # may lose some data, but who cares ? + value.to_date + else + nil + end + end + + def parse_boolean(value) + case value + when 'true', 'on', '1' + true + when 'false' + false + end + end + + def parse_enums(value) = JSON.parse(value) rescue nil + + def parse_datetime(value) = Time.zone.parse(value) rescue nil +end diff --git a/app/models/types_de_champ/type_de_champ_base.rb b/app/models/types_de_champ/type_de_champ_base.rb index 336193dea..ec5940e2a 100644 --- a/app/models/types_de_champ/type_de_champ_base.rb +++ b/app/models/types_de_champ/type_de_champ_base.rb @@ -98,10 +98,9 @@ class TypesDeChamp::TypeDeChampBase def columns(procedure_id:, displayable: true, prefix: nil) if fillable? [ - Column.new( + Columns::ChampColumn.new( procedure_id:, - table: Column::TYPE_DE_CHAMP_TABLE, - column: stable_id.to_s, + stable_id: stable_id, label: libelle_with_prefix(prefix), type: TypeDeChamp.column_type(type_champ), value_column: TypeDeChamp.value_column(type_champ), diff --git a/spec/components/instructeurs/column_filter_value_component_spec.rb b/spec/components/instructeurs/column_filter_value_component_spec.rb index fd9a1f9a9..11452b963 100644 --- a/spec/components/instructeurs/column_filter_value_component_spec.rb +++ b/spec/components/instructeurs/column_filter_value_component_spec.rb @@ -25,7 +25,7 @@ describe Instructeurs::ColumnFilterValueComponent, type: :component do let(:types_de_champ_public) { [{ type: :drop_down_list, libelle: 'Votre ville', options: ['Paris', 'Lyon', 'Marseille'] }] } let(:procedure) { create(:procedure, :published, types_de_champ_public:) } let(:drop_down_stable_id) { procedure.active_revision.types_de_champ.first.stable_id } - let(:column) { Column.new(procedure_id:, table: 'type_de_champ', scope: nil, column: drop_down_stable_id) } + let(:column) { procedure.find_column(label: 'Votre ville') } it 'find most recent tdc' do is_expected.to eq(['Paris', 'Lyon', 'Marseille']) diff --git a/spec/models/column_spec.rb b/spec/models/column_spec.rb index 74634c718..7b1f4b3d9 100644 --- a/spec/models/column_spec.rb +++ b/spec/models/column_spec.rb @@ -1,95 +1,4 @@ # frozen_string_literal: true describe Column do - describe 'value' do - let(:groupe_instructeur) { create(:groupe_instructeur, instructeurs: [create(:instructeur)]) } - - context 'when dossier columns' do - context 'when procedure for individual' do - let(:individual) { create(:individual, nom: "Sim", prenom: "Paul", gender: 'M.') } - let(:procedure) { create(:procedure, for_individual: true, groupe_instructeurs: [groupe_instructeur]) } - let(:dossier) { create(:dossier, individual:, mandataire_first_name: "Martin", mandataire_last_name: "Christophe", for_tiers: true) } - - it 'retrieve individual information' do - expect(procedure.find_column(label: "Prénom").value(dossier)).to eq("Paul") - expect(procedure.find_column(label: "Nom").value(dossier)).to eq("Sim") - expect(procedure.find_column(label: "Civilité").value(dossier)).to eq("M.") - end - end - - context 'when procedure for entreprise' do - let(:procedure) { create(:procedure, for_individual: false, groupe_instructeurs: [groupe_instructeur]) } - let(:dossier) { create(:dossier, :en_instruction, :with_entreprise, procedure:) } - - it 'retrieve entreprise information' do - expect(procedure.find_column(label: "Libellé NAF").value(dossier)).to eq('Transports par conduites') - end - end - - context 'when sva/svr enabled' do - let(:procedure) { create(:procedure, :sva, for_individual: true, groupe_instructeurs: [groupe_instructeur]) } - let(:dossier) { create(:dossier, :en_instruction, procedure:) } - - it 'does not fail' do - expect(procedure.find_column(label: "Date décision SVA").value(dossier)).to eq(nil) - end - end - end - - context 'when champ columns' do - let(:procedure) { create(:procedure, :with_all_champs_mandatory, groupe_instructeurs: [groupe_instructeur]) } - let(:dossier) { create(:dossier, :with_populated_champs, procedure:) } - let(:types_de_champ) { procedure.all_revisions_types_de_champ } - - it 'extracts values for columns and type de champ' do - expect_type_de_champ_values('civilite', ["M."]) - expect_type_de_champ_values('email', ['yoda@beta.gouv.fr']) - expect_type_de_champ_values('phone', ['0666666666']) - expect_type_de_champ_values('address', ["2 rue des Démarches"]) - expect_type_de_champ_values('communes', ["Coye-la-Forêt"]) - expect_type_de_champ_values('departements', ['01']) - expect_type_de_champ_values('regions', ['01']) - expect_type_de_champ_values('pays', ['France']) - expect_type_de_champ_values('epci', [nil]) - expect_type_de_champ_values('iban', [nil]) - expect_type_de_champ_values('siret', ["44011762001530", "postal_code", "city_name", "departement_code", "region_name"]) - expect_type_de_champ_values('text', ['text']) - expect_type_de_champ_values('textarea', ['textarea']) - expect_type_de_champ_values('number', ['42']) - expect_type_de_champ_values('decimal_number', [42.1]) - expect_type_de_champ_values('integer_number', [42]) - expect_type_de_champ_values('date', [Time.zone.parse('2019-07-10').to_date]) - expect_type_de_champ_values('datetime', [Time.zone.parse("1962-09-15T15:35:00+01:00")]) - expect_type_de_champ_values('checkbox', [true]) - expect_type_de_champ_values('drop_down_list', ['val1']) - expect_type_de_champ_values('multiple_drop_down_list', [["val1", "val2"]]) - expect_type_de_champ_values('linked_drop_down_list', [nil, "categorie 1", "choix 1"]) - expect_type_de_champ_values('yes_no', [true]) - expect_type_de_champ_values('annuaire_education', [nil]) - expect_type_de_champ_values('carte', []) - expect_type_de_champ_values('piece_justificative', []) - expect_type_de_champ_values('titre_identite', [true]) - expect_type_de_champ_values('cnaf', [nil]) - expect_type_de_champ_values('dgfip', [nil]) - expect_type_de_champ_values('pole_emploi', [nil]) - expect_type_de_champ_values('mesri', [nil]) - expect_type_de_champ_values('cojo', [nil]) - expect_type_de_champ_values('expression_reguliere', [nil]) - end - end - end - - private - - def expect_type_de_champ_values(type, values) - type_de_champ = types_de_champ.find { _1.type_champ == type } - champ = dossier.send(:filled_champ, type_de_champ, nil) - columns = type_de_champ.columns(procedure_id: procedure.id) - expect(columns.map { _1.value(champ) }).to eq(values) - end - - def retrieve_champ(type) - type_de_champ = types_de_champ.find { _1.type_champ == type } - dossier.send(:filled_champ, type_de_champ, nil) - end end diff --git a/spec/models/columns/champ_column_spec.rb b/spec/models/columns/champ_column_spec.rb new file mode 100644 index 000000000..effaf0914 --- /dev/null +++ b/spec/models/columns/champ_column_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +describe Columns::ChampColumn do + describe '#value' do + context 'when champ columns' do + let(:procedure) { create(:procedure, :with_all_champs_mandatory) } + let(:dossier) { create(:dossier, :with_populated_champs, procedure:) } + let(:types_de_champ) { procedure.all_revisions_types_de_champ } + + it 'extracts values for columns and type de champ' do + expect_type_de_champ_values('civilite', ["M."]) + expect_type_de_champ_values('email', ['yoda@beta.gouv.fr']) + expect_type_de_champ_values('phone', ['0666666666']) + expect_type_de_champ_values('address', ["2 rue des Démarches"]) + expect_type_de_champ_values('communes', ["Coye-la-Forêt"]) + expect_type_de_champ_values('departements', ['01']) + expect_type_de_champ_values('regions', ['01']) + expect_type_de_champ_values('pays', ['France']) + expect_type_de_champ_values('epci', [nil]) + expect_type_de_champ_values('iban', [nil]) + expect_type_de_champ_values('siret', ["44011762001530", "postal_code", "city_name", "departement_code", "region_name"]) + expect_type_de_champ_values('text', ['text']) + expect_type_de_champ_values('textarea', ['textarea']) + expect_type_de_champ_values('number', ['42']) + expect_type_de_champ_values('decimal_number', [42.1]) + expect_type_de_champ_values('integer_number', [42]) + expect_type_de_champ_values('date', [Time.zone.parse('2019-07-10').to_date]) + expect_type_de_champ_values('datetime', [Time.zone.parse("1962-09-15T15:35:00+01:00")]) + expect_type_de_champ_values('checkbox', [true]) + expect_type_de_champ_values('drop_down_list', ['val1']) + expect_type_de_champ_values('multiple_drop_down_list', [["val1", "val2"]]) + expect_type_de_champ_values('linked_drop_down_list', [nil, "categorie 1", "choix 1"]) + expect_type_de_champ_values('yes_no', [true]) + expect_type_de_champ_values('annuaire_education', [nil]) + expect_type_de_champ_values('carte', []) + expect_type_de_champ_values('piece_justificative', []) + expect_type_de_champ_values('titre_identite', [true]) + expect_type_de_champ_values('cnaf', [nil]) + expect_type_de_champ_values('dgfip', [nil]) + expect_type_de_champ_values('pole_emploi', [nil]) + expect_type_de_champ_values('mesri', [nil]) + expect_type_de_champ_values('cojo', [nil]) + expect_type_de_champ_values('expression_reguliere', [nil]) + end + end + end + + private + + def expect_type_de_champ_values(type, values) + type_de_champ = types_de_champ.find { _1.type_champ == type } + champ = dossier.send(:filled_champ, type_de_champ, nil) + columns = type_de_champ.columns(procedure_id: procedure.id) + expect(columns.map { _1.value(champ) }).to eq(values) + end + + def retrieve_champ(type) + type_de_champ = types_de_champ.find { _1.type_champ == type } + dossier.send(:filled_champ, type_de_champ, nil) + end +end diff --git a/spec/models/columns/dossier_column_spec.rb b/spec/models/columns/dossier_column_spec.rb new file mode 100644 index 000000000..5b8651d2a --- /dev/null +++ b/spec/models/columns/dossier_column_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +describe Columns::DossierColumn do + describe 'value' do + let(:groupe_instructeur) { create(:groupe_instructeur, instructeurs: [create(:instructeur)]) } + + context 'when dossier columns' do + context 'when procedure for individual' do + let(:individual) { create(:individual, nom: "Sim", prenom: "Paul", gender: 'M.') } + let(:procedure) { create(:procedure, for_individual: true, groupe_instructeurs: [groupe_instructeur]) } + let(:dossier) { create(:dossier, individual:, mandataire_first_name: "Martin", mandataire_last_name: "Christophe", for_tiers: true) } + + it 'retrieve individual information' do + expect(procedure.find_column(label: "Prénom").value(dossier)).to eq("Paul") + expect(procedure.find_column(label: "Nom").value(dossier)).to eq("Sim") + expect(procedure.find_column(label: "Civilité").value(dossier)).to eq("M.") + end + end + + context 'when procedure for entreprise' do + let(:procedure) { create(:procedure, for_individual: false, groupe_instructeurs: [groupe_instructeur]) } + let(:dossier) { create(:dossier, :en_instruction, :with_entreprise, procedure:) } + + it 'retrieve entreprise information' do + expect(procedure.find_column(label: "Libellé NAF").value(dossier)).to eq('Transports par conduites') + end + end + + context 'when sva/svr enabled' do + let(:procedure) { create(:procedure, :sva, for_individual: true, groupe_instructeurs: [groupe_instructeur]) } + let(:dossier) { create(:dossier, :en_instruction, procedure:) } + + it 'does not fail' do + expect(procedure.find_column(label: "Date décision SVA").value(dossier)).to eq(nil) + end + end + end + end +end From a4617abb0ed855ad5e5756de6a9011c632f31980 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Nov 2024 10:18:07 +0100 Subject: [PATCH 03/12] rework JSONPathColumn to be a ChampColumn --- app/models/columns/json_path_column.rb | 58 ++++++++----------- .../concerns/addressable_column_concern.rb | 19 +++--- config/brakeman.ignore | 48 +++++++-------- spec/models/columns/json_path_column_spec.rb | 47 +++++++++++++++ spec/services/dossier_filter_service_spec.rb | 2 +- 5 files changed, 105 insertions(+), 69 deletions(-) create mode 100644 spec/models/columns/json_path_column_spec.rb diff --git a/app/models/columns/json_path_column.rb b/app/models/columns/json_path_column.rb index f78585688..523b065ff 100644 --- a/app/models/columns/json_path_column.rb +++ b/app/models/columns/json_path_column.rb @@ -1,19 +1,32 @@ # frozen_string_literal: true -class Columns::JSONPathColumn < Column - def column - "#{@column}->#{value_column}" # override column otherwise json path facets will have same id as other +class Columns::JSONPathColumn < Columns::ChampColumn + attr_reader :jsonpath + + def initialize(procedure_id:, label:, stable_id:, jsonpath:, displayable:, type: :text) + @jsonpath = quote_string(jsonpath) + + super( + procedure_id:, + label:, + stable_id:, + displayable:, + type: + ) end - def filtered_ids(dossiers, search_occurences) - queries = Array.new(search_occurences.count, "(#{json_path_query_part} ILIKE ?)").join(' OR ') + def filtered_ids(dossiers, search_terms) + value = quote_string(search_terms.join('|')) + + condition = %{champs.value_json @? '#{jsonpath} ? (@ like_regex "#{value}" flag "i")'} + dossiers.with_type_de_champ(stable_id) - .where(queries, *(search_occurences.map { |value| "%#{value}%" })) + .where(condition) .ids end def options_for_select - case value_column.last + case jsonpath.split('.').last when 'departement_code' APIGeoService.departements.map { ["#{_1[:code]} – #{_1[:name]}", _1[:code]] } when 'region_name' @@ -25,34 +38,11 @@ class Columns::JSONPathColumn < Column private + def column_id = "type_de_champ/#{stable_id}-#{jsonpath}" + def typed_value(champ) - champ.value_json&.dig(*value_column) + champ.value_json&.dig(*jsonpath.split('.')[1..]) end - def stable_id - @column - end - - # given a value_column as ['value_json', 'address', 'postal_code'] - # build SQL query as 'champs'.'value_json'->'address'->>'postal_code' - # see: https://www.postgresql.org/docs/9.5/functions-json.html - def json_path_query_part - *json_segments, key = value_column - - if json_segments.blank? # not nested, only access using ->> Get JSON array element as text - "#{quote_table_column('champs')}.#{quote_table_column('value_json')}->>#{quote_json_segment(key)}" - else # nested, have to dig in json using -> Get JSON object field by key - field_accessor = json_segments.map(&method(:quote_json_segment)).join('->') - - "#{quote_table_column('champs')}.#{quote_table_column('value_json')}->#{field_accessor}->>#{quote_json_segment(key)}" - end - end - - def quote_table_column(table_or_column) - ActiveRecord::Base.connection.quote_column_name(table_or_column) - end - - def quote_json_segment(path) - "'#{path}'" - end + def quote_string(string) = ActiveRecord::Base.connection.quote_string(string) end diff --git a/app/models/concerns/addressable_column_concern.rb b/app/models/concerns/addressable_column_concern.rb index b439d50bc..23c80acb7 100644 --- a/app/models/concerns/addressable_column_concern.rb +++ b/app/models/concerns/addressable_column_concern.rb @@ -6,19 +6,18 @@ module AddressableColumnConcern included do def columns(procedure_id:, displayable: true, prefix: nil) super.concat([ - ["code postal (5 chiffres)", ['postal_code'], :text], - ["commune", ['city_name'], :text], - ["département", ['departement_code'], :enum], - ["region", ['region_name'], :enum] - ].map do |(label, value_column, type)| + ["code postal (5 chiffres)", '$.postal_code', :text], + ["commune", '$.city_name', :text], + ["département", '$.departement_code', :enum], + ["region", '$.region_name', :enum] + ].map do |(label, jsonpath, type)| Columns::JSONPathColumn.new( procedure_id:, - table: Column::TYPE_DE_CHAMP_TABLE, - column: stable_id, + stable_id:, label: "#{libelle_with_prefix(prefix)} – #{label}", - displayable: false, - type:, - value_column: + jsonpath:, + displayable:, + type: ) end) end diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 753bf47e5..78ea26d4c 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -67,29 +67,6 @@ ], "note": "filtered by rails query params where(something: ?, values)" }, - { - "warning_type": "SQL Injection", - "warning_code": 0, - "fingerprint": "5ba3f5d525b15c710215829e0db49f58e8cca06d68eff5931ebfd7d0ca0e35de", - "check_name": "SQL", - "message": "Possible SQL injection", - "file": "app/models/columns/json_path_column.rb", - "line": 11, - "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", - "code": "dossiers.with_type_de_champ(stable_id).where(\"#{search_occurences.count} OR #{\"(#{json_path_query_part} ILIKE ?)\"}\", *search_occurences.map do\n \"%#{value}%\"\n end)", - "render_path": null, - "location": { - "type": "method", - "class": "Columns::JSONPathColumn", - "method": "filtered_ids" - }, - "user_input": "search_occurences.count", - "confidence": "Weak", - "cwe_id": [ - 89 - ], - "note": "already sanitized" - }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -136,6 +113,29 @@ ], "note": "" }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "83b5a474065af330c47603d1f60fc31edaab55be162825385d53b77c1c98a6d7", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/columns/json_path_column.rb", + "line": 24, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "dossiers.with_type_de_champ(stable_id).where(\"champs.value_json @? '#{jsonpath} ? (@ like_regex \\\"#{quote_string(search_terms.join(\"|\"))}\\\" flag \\\"i\\\")'\")", + "render_path": null, + "location": { + "type": "method", + "class": "Columns::JSONPathColumn", + "method": "filtered_ids" + }, + "user_input": "jsonpath", + "confidence": "Weak", + "cwe_id": [ + 89 + ], + "note": "escaped by hand" + }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -295,6 +295,6 @@ "note": "Current is not a model" } ], - "updated": "2024-10-16 18:07:17 +0200", + "updated": "2024-11-04 09:56:55 +0100", "brakeman_version": "6.1.2" } diff --git a/spec/models/columns/json_path_column_spec.rb b/spec/models/columns/json_path_column_spec.rb new file mode 100644 index 000000000..0da283764 --- /dev/null +++ b/spec/models/columns/json_path_column_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +describe Columns::JSONPathColumn do + let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :address }]) } + let(:dossier) { create(:dossier, procedure:) } + let(:champ) { dossier.champs.first } + let(:stable_id) { champ.stable_id } + let(:column) { described_class.new(procedure_id: procedure.id, label: 'label', stable_id:, jsonpath:, displayable: true) } + + describe '#value' do + let(:jsonpath) { '$.city_name' } + + subject { column.value(champ) } + + context 'when champ has value_json' do + before { champ.update(value_json: { city_name: 'Grenoble' }) } + + it { is_expected.to eq('Grenoble') } + end + + context 'when champ has no value_json' do + it { is_expected.to be_nil } + end + end + + describe '#filtered_ids' do + let(:jsonpath) { '$.city_name' } + + subject { column.filtered_ids(Dossier.all, ['reno', 'Lyon']) } + + context 'when champ has value_json' do + before { champ.update(value_json: { city_name: 'Grenoble' }) } + + it { is_expected.to eq([dossier.id]) } + end + + context 'when champ has no value_json' do + it { is_expected.to eq([]) } + end + end + + describe '#initializer' do + let(:jsonpath) { %{$.'city_name} } + + it { expect(column.jsonpath).to eq(%{$.''city_name}) } + end +end diff --git a/spec/services/dossier_filter_service_spec.rb b/spec/services/dossier_filter_service_spec.rb index cb8e395bd..c1dbc539a 100644 --- a/spec/services/dossier_filter_service_spec.rb +++ b/spec/services/dossier_filter_service_spec.rb @@ -528,7 +528,7 @@ describe DossierFilterService do context "when searching by postal_code (text)" do let(:value) { "60580" } - let(:filter) { ["rna – code postal (5 chiffres)", value] } + let(:filter) { ["rna – code postal (5 chiffres)", value] } before do kept_dossier.project_champs_public.find { _1.stable_id == 1 }.update(value_json: { "postal_code" => value }) From 9d6304e7d4c03de052d00e2699f158463512d35a Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Nov 2024 16:13:55 +0100 Subject: [PATCH 04/12] TitreIdentite is a ChampColumn --- app/models/columns/titre_identite_column.rb | 2 +- app/models/types_de_champ/titre_identite_type_de_champ.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/models/columns/titre_identite_column.rb b/app/models/columns/titre_identite_column.rb index 9ce491e51..80ddcaf63 100644 --- a/app/models/columns/titre_identite_column.rb +++ b/app/models/columns/titre_identite_column.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class Columns::TitreIdentiteColumn < Column +class Columns::TitreIdentiteColumn < Columns::ChampColumn private def typed_value(champ) diff --git a/app/models/types_de_champ/titre_identite_type_de_champ.rb b/app/models/types_de_champ/titre_identite_type_de_champ.rb index 0cbd1e906..bb8e31785 100644 --- a/app/models/types_de_champ/titre_identite_type_de_champ.rb +++ b/app/models/types_de_champ/titre_identite_type_de_champ.rb @@ -28,8 +28,7 @@ class TypesDeChamp::TitreIdentiteTypeDeChamp < TypesDeChamp::TypeDeChampBase [ Columns::TitreIdentiteColumn.new( procedure_id:, - table: Column::TYPE_DE_CHAMP_TABLE, - column: stable_id.to_s, + stable_id:, label: libelle_with_prefix(prefix), type: TypeDeChamp.column_type(type_champ), value_column: TypeDeChamp.value_column(type_champ), From de8cad888e6b5068f6a89eab1ffb5cc49402d014 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 31 Oct 2024 18:34:30 +0100 Subject: [PATCH 05/12] LinkedDropDownColumn is also a ChampColumn by the way --- app/models/columns/linked_drop_down_column.rb | 22 +++++++++++++++---- .../linked_drop_down_list_type_de_champ.rb | 9 +++----- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/app/models/columns/linked_drop_down_column.rb b/app/models/columns/linked_drop_down_column.rb index bab15cd68..5492d3c70 100644 --- a/app/models/columns/linked_drop_down_column.rb +++ b/app/models/columns/linked_drop_down_column.rb @@ -1,9 +1,15 @@ # frozen_string_literal: true -class Columns::LinkedDropDownColumn < Column - def column - return super if default_column? - "#{@column}->#{value_column}" # override column otherwise json path facets will have same id as other +class Columns::LinkedDropDownColumn < Columns::ChampColumn + def initialize(procedure_id:, label:, stable_id:, value_column:, displayable:, type: :text) + super( + procedure_id:, + label:, + stable_id:, + displayable:, + type:, + value_column: + ) end def filtered_ids(dossiers, values) @@ -14,6 +20,14 @@ class Columns::LinkedDropDownColumn < Column private + def column_id + if value_column == :value + "type_de_champ/#{stable_id}" + else + "type_de_champ/#{stable_id}->#{path}" + end + end + def typed_value(champ) return nil if default_column? diff --git a/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb b/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb index 577936c9b..dc291c2c2 100644 --- a/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb +++ b/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb @@ -75,17 +75,15 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas [ Columns::LinkedDropDownColumn.new( procedure_id:, - table: Column::TYPE_DE_CHAMP_TABLE, - column: stable_id.to_s, label: libelle_with_prefix(prefix), + stable_id: stable_id, type: :text, value_column: :value, displayable: ), Columns::LinkedDropDownColumn.new( procedure_id:, - table: Column::TYPE_DE_CHAMP_TABLE, - column: stable_id.to_s, + stable_id:, label: "#{libelle_with_prefix(prefix)} (Primaire)", type: :enum, value_column: :primary, @@ -93,8 +91,7 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas ), Columns::LinkedDropDownColumn.new( procedure_id:, - table: Column::TYPE_DE_CHAMP_TABLE, - column: stable_id.to_s, + stable_id:, label: "#{libelle_with_prefix(prefix)} (Secondaire)", type: :enum, value_column: :secondary, From 74e6834ce2d69ca1b61fd3941f3848007510c3dc Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Sun, 3 Nov 2024 22:07:38 +0100 Subject: [PATCH 06/12] use path instead of value_column in linked_drop_down --- app/models/columns/linked_drop_down_column.rb | 15 +++++++++------ app/models/type_de_champ.rb | 2 +- .../linked_drop_down_list_type_de_champ.rb | 6 +++--- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/app/models/columns/linked_drop_down_column.rb b/app/models/columns/linked_drop_down_column.rb index 5492d3c70..41bfcd8ac 100644 --- a/app/models/columns/linked_drop_down_column.rb +++ b/app/models/columns/linked_drop_down_column.rb @@ -1,14 +1,17 @@ # frozen_string_literal: true class Columns::LinkedDropDownColumn < Columns::ChampColumn - def initialize(procedure_id:, label:, stable_id:, value_column:, displayable:, type: :text) + attr_reader :path + + def initialize(procedure_id:, label:, stable_id:, path:, displayable:, type: :text) + @path = path + super( procedure_id:, label:, stable_id:, displayable:, - type:, - value_column: + type: ) end @@ -21,7 +24,7 @@ class Columns::LinkedDropDownColumn < Columns::ChampColumn private def column_id - if value_column == :value + if path == :value "type_de_champ/#{stable_id}" else "type_de_champ/#{stable_id}->#{path}" @@ -29,10 +32,10 @@ class Columns::LinkedDropDownColumn < Columns::ChampColumn end def typed_value(champ) - return nil if default_column? + return nil if path == :value primary_value, secondary_value = unpack_values(champ.value) - case value_column + case path when :primary primary_value when :secondary diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index b979fdf45..420b4a2cf 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -561,7 +561,7 @@ class TypeDeChamp < ApplicationRecord elsif region? APIGeoService.regions.map { [_1[:name], _1[:code]] } elsif linked_drop_down_list? - if column.value_column == :primary + if column.path == :primary primary_options else secondary_options.values.flatten diff --git a/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb b/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb index dc291c2c2..d50fa4121 100644 --- a/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb +++ b/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb @@ -78,7 +78,7 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas label: libelle_with_prefix(prefix), stable_id: stable_id, type: :text, - value_column: :value, + path: :value, displayable: ), Columns::LinkedDropDownColumn.new( @@ -86,7 +86,7 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas stable_id:, label: "#{libelle_with_prefix(prefix)} (Primaire)", type: :enum, - value_column: :primary, + path: :primary, displayable: false ), Columns::LinkedDropDownColumn.new( @@ -94,7 +94,7 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas stable_id:, label: "#{libelle_with_prefix(prefix)} (Secondaire)", type: :enum, - value_column: :secondary, + path: :secondary, displayable: false ) ] From 8507733250bf818a073fcaaaadb72701a7604431 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 31 Oct 2024 18:35:20 +0100 Subject: [PATCH 07/12] use champ_column.stable_id --- .../instructeurs/column_filter_value_component.rb | 5 ++--- app/components/instructeurs/filter_buttons_component.rb | 6 ++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/app/components/instructeurs/column_filter_value_component.rb b/app/components/instructeurs/column_filter_value_component.rb index d83a239ea..bb4298e5f 100644 --- a/app/components/instructeurs/column_filter_value_component.rb +++ b/app/components/instructeurs/column_filter_value_component.rb @@ -54,12 +54,11 @@ class Instructeurs::ColumnFilterValueComponent < ApplicationComponent end end else - find_type_de_champ(column.column).options_for_select(column) + find_type_de_champ(column.stable_id).options_for_select(column) end end - def find_type_de_champ(column) - stable_id = column.to_s.split('->').first + def find_type_de_champ(stable_id) TypeDeChamp .joins(:revision_types_de_champ) .where(revision_types_de_champ: { revision_id: ProcedureRevision.where(procedure_id:) }) diff --git a/app/components/instructeurs/filter_buttons_component.rb b/app/components/instructeurs/filter_buttons_component.rb index 950adee9b..23ffcb851 100644 --- a/app/components/instructeurs/filter_buttons_component.rb +++ b/app/components/instructeurs/filter_buttons_component.rb @@ -49,7 +49,7 @@ class Instructeurs::FilterButtonsComponent < ApplicationComponent column, filter = filter_column.column, filter_column.filter if column.type_de_champ? - find_type_de_champ(column.column).dynamic_type.filter_to_human(filter) + find_type_de_champ(column.stable_id).dynamic_type.filter_to_human(filter) elsif column.dossier_state? if filter == 'pending_correction' Dossier.human_attribute_name("pending_correction.for_instructeur") @@ -66,9 +66,7 @@ class Instructeurs::FilterButtonsComponent < ApplicationComponent end end - def find_type_de_champ(column) - stable_id = column.to_s.split('->').first - + def find_type_de_champ(stable_id) TypeDeChamp .joins(:revision_types_de_champ) .where(revision_types_de_champ: { revision_id: @procedure_presentation.procedure.revisions }) From 7e9b68ed0ed71cd6e11349e6651209c37156697c Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 31 Oct 2024 18:36:14 +0100 Subject: [PATCH 08/12] fix projection --- app/services/dossier_projection_service.rb | 28 ++++++++++++++++------ 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/app/services/dossier_projection_service.rb b/app/services/dossier_projection_service.rb index dfe9039b7..0577d93a5 100644 --- a/app/services/dossier_projection_service.rb +++ b/app/services/dossier_projection_service.rb @@ -27,6 +27,7 @@ class DossierProjectionService TABLE = 'table' COLUMN = 'column' + STABLE_ID = 'stable_id' # Returns [DossierProjection(dossier, columns)] ordered by dossiers_ids # and the columns orderd by fields. @@ -41,7 +42,13 @@ class DossierProjectionService # - the order of the intermediary query results are unknown # - some values can be missing (if a revision added or removed them) def self.project(dossiers_ids, columns) - fields = columns.map { |c| { TABLE => c.table, COLUMN => c.column } } + fields = columns.map do |c| + if c.is_a?(Columns::ChampColumn) + { TABLE => c.table, STABLE_ID => c.stable_id, original_column: c } + else + { TABLE => c.table, COLUMN => c.column } + end + end champ_value = champ_value_formatter(dossiers_ids, fields) state_field = { TABLE => 'self', COLUMN => 'state' } @@ -64,14 +71,17 @@ class DossierProjectionService when 'type_de_champ', 'type_de_champ_private' Champ .where( - stable_id: fields.map { |f| f[COLUMN] }, + stable_id: fields.map { |f| f[STABLE_ID] }, dossier_id: dossiers_ids ) .select(:dossier_id, :value, :stable_id, :type, :external_id, :data, :value_json) # we cannot pluck :value, as we need the champ.to_s method .group_by(&:stable_id) # the champs are redispatched to their respective fields .map do |stable_id, champs| - field = fields.find { |f| f[COLUMN] == stable_id.to_s } - field[:id_value_h] = champs.to_h { |c| [c.dossier_id, champ_value.(c)] } + fields + .filter { |f| f[STABLE_ID] == stable_id } + .each do |field| + field[:id_value_h] = champs.to_h { |c| [c.dossier_id, champ_value.(c, field[:original_column])] } + end end when 'self' Dossier @@ -173,7 +183,7 @@ class DossierProjectionService private def champ_value_formatter(dossiers_ids, fields) - stable_ids = fields.filter { _1[TABLE].in?(['type_de_champ', 'type_de_champ_private']) }.map { _1[COLUMN] } + stable_ids = fields.filter { _1[TABLE].in?(['type_de_champ', 'type_de_champ_private']) }.map { _1[STABLE_ID] } revision_ids_by_dossier_ids = Dossier.where(id: dossiers_ids).pluck(:id, :revision_id).to_h stable_ids_and_types_de_champ_by_revision_ids = ProcedureRevisionTypeDeChamp.includes(:type_de_champ) .where(revision_id: revision_ids_by_dossier_ids.values.uniq, type_de_champ: { stable_id: stable_ids }) @@ -181,10 +191,14 @@ class DossierProjectionService .group_by(&:first) .transform_values { _1.map { |_, type_de_champ| [type_de_champ.stable_id, type_de_champ] }.to_h } stable_ids_and_types_de_champ_by_dossier_ids = revision_ids_by_dossier_ids.transform_values { stable_ids_and_types_de_champ_by_revision_ids[_1] }.compact - -> (champ) { + -> (champ, column) { type_de_champ = stable_ids_and_types_de_champ_by_dossier_ids.fetch(champ.dossier_id, {})[champ.stable_id] if type_de_champ.present? && type_de_champ.type_champ == champ.last_write_type_champ - type_de_champ.champ_value(champ) + if column.is_a?(Columns::JSONPathColumn) + column.get_value(champ) + else + type_de_champ.champ_value(champ) + end else '' end From 0b2bc68d483e36cf3080ddd8173bf354641242d3 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 1 Nov 2024 22:22:40 +0100 Subject: [PATCH 09/12] remove unused `type_de_champ_private` string --- app/services/dossier_projection_service.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/dossier_projection_service.rb b/app/services/dossier_projection_service.rb index 0577d93a5..7cf8d88ca 100644 --- a/app/services/dossier_projection_service.rb +++ b/app/services/dossier_projection_service.rb @@ -68,7 +68,7 @@ class DossierProjectionService .group_by { |f| f[TABLE] } # one query per table .each do |table, fields| case table - when 'type_de_champ', 'type_de_champ_private' + when 'type_de_champ' Champ .where( stable_id: fields.map { |f| f[STABLE_ID] }, @@ -183,7 +183,7 @@ class DossierProjectionService private def champ_value_formatter(dossiers_ids, fields) - stable_ids = fields.filter { _1[TABLE].in?(['type_de_champ', 'type_de_champ_private']) }.map { _1[STABLE_ID] } + stable_ids = fields.filter { _1[TABLE].in?(['type_de_champ']) }.map { _1[STABLE_ID] } revision_ids_by_dossier_ids = Dossier.where(id: dossiers_ids).pluck(:id, :revision_id).to_h stable_ids_and_types_de_champ_by_revision_ids = ProcedureRevisionTypeDeChamp.includes(:type_de_champ) .where(revision_id: revision_ids_by_dossier_ids.values.uniq, type_de_champ: { stable_id: stable_ids }) From 52aaff5ffcd2d3f16003025b9d4163b7b180c51c Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Nov 2024 08:26:57 +0100 Subject: [PATCH 10/12] breaking: simplify linked_drop_down_column column_id --- app/models/columns/linked_drop_down_column.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/app/models/columns/linked_drop_down_column.rb b/app/models/columns/linked_drop_down_column.rb index 41bfcd8ac..2be2724d3 100644 --- a/app/models/columns/linked_drop_down_column.rb +++ b/app/models/columns/linked_drop_down_column.rb @@ -23,13 +23,7 @@ class Columns::LinkedDropDownColumn < Columns::ChampColumn private - def column_id - if path == :value - "type_de_champ/#{stable_id}" - else - "type_de_champ/#{stable_id}->#{path}" - end - end + def column_id = "type_de_champ/#{stable_id}->#{path}" def typed_value(champ) return nil if path == :value From ea57a97c065031ce4b7b5127ee97128745ed15d4 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Nov 2024 16:34:00 +0100 Subject: [PATCH 11/12] introduce tdc_type --- app/models/columns/champ_column.rb | 3 ++- app/models/columns/json_path_column.rb | 3 ++- app/models/columns/linked_drop_down_column.rb | 3 ++- app/models/concerns/addressable_column_concern.rb | 1 + .../types_de_champ/linked_drop_down_list_type_de_champ.rb | 5 ++++- app/models/types_de_champ/titre_identite_type_de_champ.rb | 1 + app/models/types_de_champ/type_de_champ_base.rb | 3 ++- spec/models/columns/json_path_column_spec.rb | 3 ++- 8 files changed, 16 insertions(+), 6 deletions(-) diff --git a/app/models/columns/champ_column.rb b/app/models/columns/champ_column.rb index e60472284..2d6ee6c4a 100644 --- a/app/models/columns/champ_column.rb +++ b/app/models/columns/champ_column.rb @@ -3,8 +3,9 @@ class Columns::ChampColumn < Column attr_reader :stable_id - def initialize(procedure_id:, label:, stable_id:, displayable: true, filterable: true, type: :text, value_column: :value) + def initialize(procedure_id:, label:, stable_id:, tdc_type:, displayable: true, filterable: true, type: :text, value_column: :value) @stable_id = stable_id + @tdc_type = tdc_type super( procedure_id:, diff --git a/app/models/columns/json_path_column.rb b/app/models/columns/json_path_column.rb index 523b065ff..89e4e15f3 100644 --- a/app/models/columns/json_path_column.rb +++ b/app/models/columns/json_path_column.rb @@ -3,13 +3,14 @@ class Columns::JSONPathColumn < Columns::ChampColumn attr_reader :jsonpath - def initialize(procedure_id:, label:, stable_id:, jsonpath:, displayable:, type: :text) + def initialize(procedure_id:, label:, stable_id:, tdc_type:, jsonpath:, displayable:, type: :text) @jsonpath = quote_string(jsonpath) super( procedure_id:, label:, stable_id:, + tdc_type:, displayable:, type: ) diff --git a/app/models/columns/linked_drop_down_column.rb b/app/models/columns/linked_drop_down_column.rb index 2be2724d3..4530bf97f 100644 --- a/app/models/columns/linked_drop_down_column.rb +++ b/app/models/columns/linked_drop_down_column.rb @@ -3,13 +3,14 @@ class Columns::LinkedDropDownColumn < Columns::ChampColumn attr_reader :path - def initialize(procedure_id:, label:, stable_id:, path:, displayable:, type: :text) + def initialize(procedure_id:, label:, stable_id:, tdc_type:, path:, displayable:, type: :text) @path = path super( procedure_id:, label:, stable_id:, + tdc_type:, displayable:, type: ) diff --git a/app/models/concerns/addressable_column_concern.rb b/app/models/concerns/addressable_column_concern.rb index 23c80acb7..4f10b62b5 100644 --- a/app/models/concerns/addressable_column_concern.rb +++ b/app/models/concerns/addressable_column_concern.rb @@ -14,6 +14,7 @@ module AddressableColumnConcern Columns::JSONPathColumn.new( procedure_id:, stable_id:, + tdc_type: type_champ, label: "#{libelle_with_prefix(prefix)} – #{label}", jsonpath:, displayable:, diff --git a/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb b/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb index d50fa4121..cee354c02 100644 --- a/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb +++ b/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb @@ -76,7 +76,8 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas Columns::LinkedDropDownColumn.new( procedure_id:, label: libelle_with_prefix(prefix), - stable_id: stable_id, + stable_id:, + tdc_type: type_champ, type: :text, path: :value, displayable: @@ -84,6 +85,7 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas Columns::LinkedDropDownColumn.new( procedure_id:, stable_id:, + tdc_type: type_champ, label: "#{libelle_with_prefix(prefix)} (Primaire)", type: :enum, path: :primary, @@ -92,6 +94,7 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas Columns::LinkedDropDownColumn.new( procedure_id:, stable_id:, + tdc_type: type_champ, label: "#{libelle_with_prefix(prefix)} (Secondaire)", type: :enum, path: :secondary, diff --git a/app/models/types_de_champ/titre_identite_type_de_champ.rb b/app/models/types_de_champ/titre_identite_type_de_champ.rb index bb8e31785..02cd74fb6 100644 --- a/app/models/types_de_champ/titre_identite_type_de_champ.rb +++ b/app/models/types_de_champ/titre_identite_type_de_champ.rb @@ -29,6 +29,7 @@ class TypesDeChamp::TitreIdentiteTypeDeChamp < TypesDeChamp::TypeDeChampBase Columns::TitreIdentiteColumn.new( procedure_id:, stable_id:, + tdc_type: type_champ, label: libelle_with_prefix(prefix), type: TypeDeChamp.column_type(type_champ), value_column: TypeDeChamp.value_column(type_champ), diff --git a/app/models/types_de_champ/type_de_champ_base.rb b/app/models/types_de_champ/type_de_champ_base.rb index ec5940e2a..2c81fc430 100644 --- a/app/models/types_de_champ/type_de_champ_base.rb +++ b/app/models/types_de_champ/type_de_champ_base.rb @@ -100,7 +100,8 @@ class TypesDeChamp::TypeDeChampBase [ Columns::ChampColumn.new( procedure_id:, - stable_id: stable_id, + stable_id:, + tdc_type: type_champ, label: libelle_with_prefix(prefix), type: TypeDeChamp.column_type(type_champ), value_column: TypeDeChamp.value_column(type_champ), diff --git a/spec/models/columns/json_path_column_spec.rb b/spec/models/columns/json_path_column_spec.rb index 0da283764..062da2e57 100644 --- a/spec/models/columns/json_path_column_spec.rb +++ b/spec/models/columns/json_path_column_spec.rb @@ -5,7 +5,8 @@ describe Columns::JSONPathColumn do let(:dossier) { create(:dossier, procedure:) } let(:champ) { dossier.champs.first } let(:stable_id) { champ.stable_id } - let(:column) { described_class.new(procedure_id: procedure.id, label: 'label', stable_id:, jsonpath:, displayable: true) } + let(:tdc_type) { champ.type_champ } + let(:column) { described_class.new(procedure_id: procedure.id, label: 'label', stable_id:, tdc_type:, jsonpath:, displayable: true) } describe '#value' do let(:jsonpath) { '$.city_name' } From c364be12f18153500236e5502525d4c7b7a3929f Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 4 Nov 2024 16:34:29 +0100 Subject: [PATCH 12/12] rework ChampColumn.value --- app/models/columns/champ_column.rb | 55 ++++++++--------- spec/models/columns/champ_column_spec.rb | 77 +++++++++++++++++++++++- 2 files changed, 101 insertions(+), 31 deletions(-) diff --git a/app/models/columns/champ_column.rb b/app/models/columns/champ_column.rb index 2d6ee6c4a..c75b223ab 100644 --- a/app/models/columns/champ_column.rb +++ b/app/models/columns/champ_column.rb @@ -22,11 +22,11 @@ class Columns::ChampColumn < Column def value(champ) return if champ.nil? - value = typed_value(champ) - if default_column? - cast_value(value, from_type: champ.last_write_column_type, to_type: type) + # nominal case + if @tdc_type == champ.last_write_type_champ + typed_value(champ) else - value + cast_value(champ) end end @@ -34,15 +34,11 @@ class Columns::ChampColumn < Column def column_id = "type_de_champ/#{stable_id}" + def string_value(champ) = champ.public_send(value_column) + def typed_value(champ) value = string_value(champ) - parse_value(value, type: champ.last_write_column_type) - end - def string_value(champ) = champ.public_send(value_column) - def default_column? = value_column.in?([:value, :external_id]) - - def parse_value(value, type:) return if value.blank? case type @@ -63,29 +59,30 @@ class Columns::ChampColumn < Column end end - def cast_value(value, from_type:, to_type:) - return if value.blank? - return value if from_type == to_type + def cast_value(champ) + value = string_value(champ) - case [from_type, to_type] - when [:integer, :decimal] # recast numbers automatically + return if value.blank? + + case [champ.last_write_type_champ, @tdc_type] + when ['integer_number', 'decimal_number'] # recast numbers automatically value.to_f - when [:decimal, :integer] # may lose some data, but who cares ? + when ['decimal_number', 'integer_number'] # may lose some data, but who cares ? value.to_i - when [:integer, :text], [:decimal, :text] # number to text - value.to_s - when [:enum, :enums] # single list can become multi - [value] - when [:enum, :text] # single list can become text + when ['integer_number', 'text'], ['decimal_number', 'text'] # number to text value - when [:enums, :enum] # multi list can become single list - value.first - when [:enums, :text] # multi list can become text - value.join(', ') - when [:date, :datetime] # date <=> datetime - value.to_datetime - when [:datetime, :date] # may lose some data, but who cares ? - value.to_date + when ['drop_down_list', 'multiple_drop_down_list'] # single list can become multi + [value] + when ['drop_down_list', 'text'] # single list can become text + value + when ['multiple_drop_down_list', 'drop_down_list'] # multi list can become single + parse_enums(value).first + when ['multiple_drop_down_list', 'text'] # single list can become text + parse_enums(value).join(', ') + when ['date', 'datetime'] # date <=> datetime + parse_datetime(value)&.to_datetime + when ['datetime', 'date'] # may lose some data, but who cares ? + parse_datetime(value)&.to_date else nil end diff --git a/spec/models/columns/champ_column_spec.rb b/spec/models/columns/champ_column_spec.rb index effaf0914..8318cf2c9 100644 --- a/spec/models/columns/champ_column_spec.rb +++ b/spec/models/columns/champ_column_spec.rb @@ -2,8 +2,9 @@ describe Columns::ChampColumn do describe '#value' do - context 'when champ columns' do - let(:procedure) { create(:procedure, :with_all_champs_mandatory) } + let(:procedure) { create(:procedure, :with_all_champs_mandatory) } + + context 'without any cast' do let(:dossier) { create(:dossier, :with_populated_champs, procedure:) } let(:types_de_champ) { procedure.all_revisions_types_de_champ } @@ -43,6 +44,78 @@ describe Columns::ChampColumn do expect_type_de_champ_values('expression_reguliere', [nil]) end end + + context 'with cast' do + def column(label) = procedure.find_column(label:) + + context 'from a integer_number' do + let(:champ) { double(last_write_type_champ: 'integer_number', value: '42') } + + it do + expect(column('decimal_number').value(champ)).to eq(42.0) + expect(column('text').value(champ)).to eq('42') + end + end + + context 'from a decimal_number' do + let(:champ) { double(last_write_type_champ: 'decimal_number', value: '42.1') } + + it do + expect(column('integer_number').value(champ)).to eq(42) + expect(column('text').value(champ)).to eq('42.1') + end + end + + context 'from a date' do + let(:champ) { double(last_write_type_champ: 'date', value:) } + + describe 'when the value is valid' do + let(:value) { '2019-07-10' } + + it { expect(column('datetime').value(champ)).to eq(Time.zone.parse('2019-07-10')) } + end + + describe 'when the value is invalid' do + let(:value) { 'invalid' } + + it { expect(column('datetime').value(champ)).to be_nil } + end + end + + context 'from a datetime' do + let(:champ) { double(last_write_type_champ: 'datetime', value:) } + + describe 'when the value is valid' do + let(:value) { '1962-09-15T15:35:00+01:00' } + + it { expect(column('date').value(champ)).to eq('1962-09-15'.to_date) } + end + + describe 'when the value is invalid' do + let(:value) { 'invalid' } + + it { expect(column('date').value(champ)).to be_nil } + end + end + + context 'from a drop_down_list' do + let(:champ) { double(last_write_type_champ: 'drop_down_list', value: 'val1') } + + it do + expect(column('multiple_drop_down_list').value(champ)).to eq(['val1']) + expect(column('text').value(champ)).to eq('val1') + end + end + + context 'from a multiple_drop_down_list' do + let(:champ) { double(last_write_type_champ: 'multiple_drop_down_list', value: '["val1","val2"]') } + + it do + expect(column('simple_drop_down_list').value(champ)).to eq('val1') + expect(column('text').value(champ)).to eq('val1, val2') + end + end + end end private