From 3a8a50a21614b0f4a66d7eb507cbc09df0aaaf41 Mon Sep 17 00:00:00 2001 From: Damien Le Thiec Date: Wed, 18 Jan 2023 12:52:38 +0100 Subject: [PATCH] Allow prefill pays type de champs (#8344) * Allow prefill pays type de champs * Avoid conditional prefil params for pays champ * Clean pays data with batch update * Fix bug and add test batch update pays value * Improve performance batch_update_pays * Fix associated country code problem * Fix after party task name * Format country name if needed in batch update --- .../batch_update_pays_values_job.rb | 92 +++++++++++++++++++ app/models/champs/pays_champ.rb | 3 + app/models/prefill_params.rb | 3 +- app/models/type_de_champ.rb | 1 + .../prefill_drop_down_list_type_de_champ.rb | 2 +- .../types_de_champ/prefill_type_de_champ.rb | 2 +- .../_types_de_champs.html.haml | 3 +- config/i18n-tasks.yml | 1 + config/locales/en.yml | 27 +++--- config/locales/fr.yml | 28 +++--- config/locales/models/type_de_champ/en.yml | 2 +- .../20230103170917_normalize_pays_values.rake | 16 ++++ .../batch_update_datetime_values_job_spec.rb | 0 .../batch_update_pays_values_job_spec.rb | 47 ++++++++++ spec/models/prefill_params_spec.rb | 2 + spec/models/type_de_champ_spec.rb | 2 +- ...efill_drop_down_list_type_de_champ_spec.rb | 2 +- .../prefill_type_de_champ_spec.rb | 2 +- .../dossier_projection_service_spec.rb | 2 +- 19 files changed, 203 insertions(+), 34 deletions(-) create mode 100644 app/jobs/migrations/batch_update_pays_values_job.rb create mode 100644 lib/tasks/deployment/20230103170917_normalize_pays_values.rake rename spec/jobs/{temporary => migrations}/batch_update_datetime_values_job_spec.rb (100%) create mode 100644 spec/jobs/migrations/batch_update_pays_values_job_spec.rb diff --git a/app/jobs/migrations/batch_update_pays_values_job.rb b/app/jobs/migrations/batch_update_pays_values_job.rb new file mode 100644 index 000000000..6e9ca4de9 --- /dev/null +++ b/app/jobs/migrations/batch_update_pays_values_job.rb @@ -0,0 +1,92 @@ +class Migrations::BatchUpdatePaysValuesJob < ApplicationJob + UNUSUAL_COUNTRY_NAME_MATCHER = { + "ACORES, MADERE" => "Portugal", + "ALASKA" => "États-Unis", + "ANTILLES NEERLANDAISES" => "Pays-Bas", + "BIELORUSSIE" => "Bélarus", + "BONAIRE, SAINT EUSTACHE ET SABA" => "Bonaire, Saint-Eustache et Saba", + "BURKINA" => "Burkina Faso", + "CAIMANES (ILES)" => "îles Caïmans", + "CAMEROUN ET TOGO" => "Cameroun", + "CANARIES (ILES)" => "Espagne", + "CENTRAFRICAINE (REPUBLIQUE)" => "République centrafricaine", + "CHRISTMAS (ILE)" => "Christmas, Île", + "CONGO (REPUBLIQUE DEMOCRATIQUE)" => "République démocratique du Congo", + "COOK (ILES)" => "îles Cook", + "COREE (REPUBLIQUE DE)" => "Corée, République de", + "COREE (REPUBLIQUE POPULAIRE DEMOCRATIQUE DE)" => "Corée, République populaire démocratique de", + "COREE" => "Corée, République de", + "DOMINICAINE (REPUBLIQUE)" => "République dominicaine", + "ETATS MALAIS NON FEDERES" => "Malaisie", + "FEROE (ILES)" => "îles Féroé", + "GUYANE" => "France", + "HAWAII (ILES)" => "États-Unis", + "HEARD ET MACDONALD (ILES)" => "îles Heard-et-MacDonald", + "ILES PORTUGAISES DE L'OCEAN INDIEN" => "Portugal", + "IRLANDE, ou EIRE" => "Irlande", + "KAMTCHATKA" => "Russie, Fédération de", + "LA REUNION" => "Réunion, Île de la", + "LABRADOR" => "Canada", + "MACEDOINE DU NORD (REPUBLIQUE DE)" => "Macédoine du Nord", + "MALOUINES, OU FALKLAND (ILES)" => "Malouines, Îles (Falkland)", + "MAN (ILE)" => "Île de Man", + "MARIANNES DU NORD (ILES)" => "Îles Mariannes du Nord", + "MARSHALL (ILES)" => "Îles Marshall", + "OCEAN INDIEN (TERRITOIRE BRITANNIQUE DE L')" => "Royaume-Uni", + "PALESTINE (Etat de)" => "Palestine, État de", + "POSSESSIONS BRITANNIQUES AU PROCHE-ORIENT" => "Royaume-Uni", + "PROVINCES ESPAGNOLES D'AFRIQUE" => "Espagne", + "REPUBLIQUE DEMOCRATIQUE ALLEMANDE" => "Allemagne", + "REPUBLIQUE FEDERALE D'ALLEMAGNE" => "Allemagne", + "RUSSIE" => "Russie, Fédération de", + "SAINT-MARTIN" => "Saint-Martin (partie française)", + "SAINT-VINCENT-ET-LES GRENADINES" => "Saint-Vincent-et-les-Grenadines", + "SAINTE HELENE, ASCENSION ET TRISTAN DA CUNHA" => "Sainte-Hélène, Ascension et Tristan da Cunha", + "SALOMON (ILES)" => "Salomon, Îles", + "SAMOA OCCIDENTALES" => "Samoa", + "SIBERIE" => "Russie, Fédération de", + "SOUDAN ANGLO-EGYPTIEN, KENYA, OUGANDA" => "Ouganda", + "SYRIE" => "Syrienne, République arabe", + "TANGER" => "Maroc", + "TCHECOSLOVAQUIE" => "Tchéquie", + "TCHEQUE (REPUBLIQUE)" => "Tchéquie", + "TERR. DES ETATS-UNIS D'AMERIQUE EN AMERIQUE" => "États-Unis", + "TERR. DES ETATS-UNIS D'AMERIQUE EN OCEANIE" => "États-Unis", + "TERR. DU ROYAUME-UNI DANS L'ATLANTIQUE SUD" => "Royaume-Uni", + "TERRE-NEUVE" => "Canada", + "TERRITOIRES DU ROYAUME-UNI AUX ANTILLES" => "Royaume-Uni", + "TURKESTAN RUSSE" => "Russie, Fédération de", + "TURKS ET CAIQUES (ILES)" => "îles Turques-et-Caïques", + "TURQUIE D'EUROPE" => "Turquie", + "VATICAN, ou SAINT-SIEGE" => "Saint-Siège (état de la cité du Vatican)", + "VIERGES BRITANNIQUES (ILES)" => "Îles Vierges britanniques", + "VIERGES DES ETATS-UNIS (ILES)" => "Îles Vierges des États-Unis", + "VIET NAM DU NORD" => "Viêt Nam", + "VIET NAM DU SUD" => "Viêt Nam", + "WALLIS-ET-FUTUNA" => "Wallis et Futuna", + "YEMEN (REPUBLIQUE ARABE DU)" => "Yémen", + "YEMEN DEMOCRATIQUE" => "Yémen", + "ZANZIBAR" => "Tanzanie" + } + + private_constant :UNUSUAL_COUNTRY_NAME_MATCHER + + def perform(ids) + ids.each do |id| + pays_champ = Champs::PaysChamp.find(id) + next if pays_champ.valid? + + code = APIGeoService.country_code(pays_champ.value) + value = if code.present? + APIGeoService.country_name(code) + else + UNUSUAL_COUNTRY_NAME_MATCHER[pays_champ.value] + end + + if value.present? || !pays_champ.required? + associated_country_code = APIGeoService.country_code(value) + pays_champ.update_columns(value: value, external_id: associated_country_code) + end + end + end +end diff --git a/app/models/champs/pays_champ.rb b/app/models/champs/pays_champ.rb index e0a00ea43..b00402850 100644 --- a/app/models/champs/pays_champ.rb +++ b/app/models/champs/pays_champ.rb @@ -21,6 +21,9 @@ # type_de_champ_id :integer # class Champs::PaysChamp < Champs::TextChamp + validates :value, inclusion: APIGeoService.countries.pluck(:name), allow_nil: true, allow_blank: false + validates :external_id, inclusion: APIGeoService.countries.pluck(:code), allow_nil: true, allow_blank: false + def for_export [name, code] end diff --git a/app/models/prefill_params.rb b/app/models/prefill_params.rb index 8240ff556..18445185a 100644 --- a/app/models/prefill_params.rb +++ b/app/models/prefill_params.rb @@ -38,7 +38,8 @@ class PrefillParams TypeDeChamp.type_champs.fetch(:datetime), TypeDeChamp.type_champs.fetch(:civilite), TypeDeChamp.type_champs.fetch(:yes_no), - TypeDeChamp.type_champs.fetch(:checkbox) + TypeDeChamp.type_champs.fetch(:checkbox), + TypeDeChamp.type_champs.fetch(:pays) ] attr_reader :champ, :value diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index c9cf55f07..8a8e1c036 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -261,6 +261,7 @@ class TypeDeChamp < ApplicationRecord TypeDeChamp.type_champs.fetch(:phone), TypeDeChamp.type_champs.fetch(:iban), TypeDeChamp.type_champs.fetch(:civilite), + TypeDeChamp.type_champs.fetch(:pays), TypeDeChamp.type_champs.fetch(:date), TypeDeChamp.type_champs.fetch(:datetime), TypeDeChamp.type_champs.fetch(:yes_no), diff --git a/app/models/types_de_champ/prefill_drop_down_list_type_de_champ.rb b/app/models/types_de_champ/prefill_drop_down_list_type_de_champ.rb index 2d245dd9f..7e3a4ad22 100644 --- a/app/models/types_de_champ/prefill_drop_down_list_type_de_champ.rb +++ b/app/models/types_de_champ/prefill_drop_down_list_type_de_champ.rb @@ -3,7 +3,7 @@ class TypesDeChamp::PrefillDropDownListTypeDeChamp < TypesDeChamp::PrefillTypeDe if drop_down_other? drop_down_list_enabled_non_empty_options.insert( 0, - I18n.t("views.prefill_descriptions.edit.possible_values.drop_down_list_other") + I18n.t("views.prefill_descriptions.edit.possible_values.drop_down_list_other_html") ) else drop_down_list_enabled_non_empty_options diff --git a/app/models/types_de_champ/prefill_type_de_champ.rb b/app/models/types_de_champ/prefill_type_de_champ.rb index 82d20f77a..6b9466b4d 100644 --- a/app/models/types_de_champ/prefill_type_de_champ.rb +++ b/app/models/types_de_champ/prefill_type_de_champ.rb @@ -17,7 +17,7 @@ class TypesDeChamp::PrefillTypeDeChamp < SimpleDelegator def possible_values return [] unless prefillable? - [I18n.t("views.prefill_descriptions.edit.possible_values.#{type_champ}")] + [I18n.t("views.prefill_descriptions.edit.possible_values.#{type_champ}_html")] end def example_value diff --git a/app/views/prefill_descriptions/_types_de_champs.html.haml b/app/views/prefill_descriptions/_types_de_champs.html.haml index aa43db426..7321b4bee 100644 --- a/app/views/prefill_descriptions/_types_de_champs.html.haml +++ b/app/views/prefill_descriptions/_types_de_champs.html.haml @@ -20,7 +20,8 @@ %button.fr-btn.fr-btn--secondary{ disabled: true } = t("views.prefill_descriptions.edit.champ_unavailable") - = type_de_champ.description + %p + = type_de_champ.description %table.table.vertical %tbody diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index 6579477af..8bcbb962e 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -108,6 +108,7 @@ ignore_unused: - 'views.pagination.*' - 'time.formats.default' - 'instructeurs.dossiers.filterable_state.*' +- 'views.prefill_descriptions.edit.possible_values.*' # - '{devise,kaminari,will_paginate}.*' # - 'simple_form.{yes,no}' # - 'simple_form.{placeholders,hints,labels}.*' diff --git a/config/locales/en.yml b/config/locales/en.yml index 77dacb55f..0ccb653e8 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -113,18 +113,20 @@ en: champ_unavailable: Unavailable possible_values: title: Values - text: A short text - textarea: A long text - decimal_number: A decimal number - integer_number: An integer number - email: An email address - phone: A phone number - iban: An Iban number - yes_no: '"true" for Yes, "false" pour No' - date: ISO8601 date - datetime: ISO8601 datetime - checkbox: '"true" to check, "false" to uncheck' - drop_down_list_other: Any value + text_html: A short text + textarea_html: A long text + decimal_number_html: A decimal number + integer_number_html: An integer number + email_html: An email address + civilite_html: '"M." for Mister, "Mme" for Madam' + phone_html: A phone number + iban_html: An Iban number + yes_no_html: '"true" for Yes, "false" pour No' + checkbox_html: '"true" to check, "false" to uncheck' + pays_html: An ISO 3166-2 country code + date_html: ISO8601 date + datetime_html: ISO8601 datetime + drop_down_list_other_html: Any value examples: title: Example text: Short text @@ -135,6 +137,7 @@ en: phone: 0612345678 iban: FR7611315000011234567890138 yes_no: "true" + pays: "FR" date: "2023-02-01" datetime: "2023-02-01T10:30" checkbox: "true" diff --git a/config/locales/fr.yml b/config/locales/fr.yml index b9656cf1e..0f06bbc59 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -104,19 +104,20 @@ fr: champ_unavailable: Indisponible possible_values: title: Valeurs - text: Un texte court - textarea: Un texte long - decimal_number: Un nombre décimal - integer_number: Un nombre entier - email: Une adresse email - civilite: '"M." pour Monsieur, "Mme" pour Madame' - phone: Un numéro de téléphone - iban: Un numéro Iban - yes_no: '"true" pour Oui, "false" pour Non' - date: Date au format ISO8601 - datetime: Datetime au format ISO8601 - checkbox: '"true" pour coché, "false" pour décoché' - drop_down_list_other: Toute valeur + text_html: Un texte court + textarea_html: Un texte long + decimal_number_html: Un nombre décimal + integer_number_html: Un nombre entier + email_html: Une adresse email + civilite_html: '"M." pour Monsieur, "Mme" pour Madame' + phone_html: Un numéro de téléphone + iban_html: Un numéro Iban + yes_no_html: '"true" pour Oui, "false" pour Non' + checkbox_html: '"true" pour coché, "false" pour décoché' + pays_html: Un code pays ISO 3166-2 + datetime_html: Datetime au format ISO8601 + date_html: Date au format ISO8601 + drop_down_list_other_html: Toute valeur examples: title: Exemple text: Texte court @@ -128,6 +129,7 @@ fr: iban: FR7611315000011234567890138 yes_no: "true" civilite: "M." + pays: "FR" date: "2023-02-01" datetime: "2023-02-01T10:30" checkbox: "true" diff --git a/config/locales/models/type_de_champ/en.yml b/config/locales/models/type_de_champ/en.yml index d5cb7a88c..1d781ea42 100644 --- a/config/locales/models/type_de_champ/en.yml +++ b/config/locales/models/type_de_champ/en.yml @@ -1,4 +1,4 @@ -fr: +en: activerecord: models: type_de_champ: 'Field type' diff --git a/lib/tasks/deployment/20230103170917_normalize_pays_values.rake b/lib/tasks/deployment/20230103170917_normalize_pays_values.rake new file mode 100644 index 000000000..85b615aaf --- /dev/null +++ b/lib/tasks/deployment/20230103170917_normalize_pays_values.rake @@ -0,0 +1,16 @@ +namespace :after_party do + desc 'Deployment task: normalize_pays_values' + task normalize_pays_values: :environment do + puts "Running deploy task 'normalize_pays_values'" + + # Put your task implementation HERE. + Champs::PaysChamp.in_batches do |pays_champs| + Migrations::BatchUpdatePaysValuesJob.perform_later(pays_champs.pluck(:id)) + end + + # Update task as completed. If you remove the line below, the task will + # run with every deploy (or every time you call after_party:run). + AfterParty::TaskRecord + .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp + end +end diff --git a/spec/jobs/temporary/batch_update_datetime_values_job_spec.rb b/spec/jobs/migrations/batch_update_datetime_values_job_spec.rb similarity index 100% rename from spec/jobs/temporary/batch_update_datetime_values_job_spec.rb rename to spec/jobs/migrations/batch_update_datetime_values_job_spec.rb diff --git a/spec/jobs/migrations/batch_update_pays_values_job_spec.rb b/spec/jobs/migrations/batch_update_pays_values_job_spec.rb new file mode 100644 index 000000000..25531daa6 --- /dev/null +++ b/spec/jobs/migrations/batch_update_pays_values_job_spec.rb @@ -0,0 +1,47 @@ +describe Migrations::BatchUpdatePaysValuesJob, type: :job do + before do + pays_champ.save(validate: false) + end + + subject { described_class.perform_now([pays_champ.id]) } + + context "the value is correct" do + let(:pays_champ) { build(:champ_pays, value: 'France', external_id: 'FR') } + + it 'does not change it' do + subject + expect(pays_champ.reload.value).to eq('France') + expect(pays_champ.reload.external_id).to eq('FR') + end + end + + context "the value is incorrect" do + let(:pays_champ) { build(:champ_pays, value: 'Incorrect') } + + it 'updates value to nil' do + subject + expect(pays_champ.reload.value).to be_nil + expect(pays_champ.reload.external_id).to be_nil + end + end + + context "the value is easily cleanable" do + let(:pays_champ) { build(:champ_pays, value: 'Vietnam') } + + it 'cleans the value' do + subject + expect(pays_champ.reload.value).to eq('Viêt Nam') + expect(pays_champ.reload.external_id).to eq('VN') + end + end + + context "the value is hard to clean" do + let(:pays_champ) { build(:champ_pays, value: 'CHRISTMAS (ILE)') } + + it 'cleans the value' do + subject + expect(pays_champ.reload.value).to eq('Christmas, Île') + expect(pays_champ.reload.external_id).to eq('CX') + end + end +end diff --git a/spec/models/prefill_params_spec.rb b/spec/models/prefill_params_spec.rb index 4fb0b1c4e..c9f4cc7c3 100644 --- a/spec/models/prefill_params_spec.rb +++ b/spec/models/prefill_params_spec.rb @@ -110,6 +110,7 @@ RSpec.describe PrefillParams do it_behaves_like "a champ public value that is authorized", :phone, "value" it_behaves_like "a champ public value that is authorized", :iban, "value" it_behaves_like "a champ public value that is authorized", :civilite, "M." + it_behaves_like "a champ public value that is authorized", :pays, "FR" it_behaves_like "a champ public value that is authorized", :date, "2022-12-22" it_behaves_like "a champ public value that is authorized", :datetime, "2022-12-22T10:30" it_behaves_like "a champ public value that is authorized", :yes_no, "true" @@ -126,6 +127,7 @@ RSpec.describe PrefillParams do it_behaves_like "a champ private value that is authorized", :phone, "value" it_behaves_like "a champ private value that is authorized", :iban, "value" it_behaves_like "a champ private value that is authorized", :civilite, "M." + it_behaves_like "a champ private value that is authorized", :pays, "FR" it_behaves_like "a champ private value that is authorized", :date, "2022-12-22" it_behaves_like "a champ private value that is authorized", :datetime, "2022-12-22T10:30" it_behaves_like "a champ private value that is authorized", :yes_no, "true" diff --git a/spec/models/type_de_champ_spec.rb b/spec/models/type_de_champ_spec.rb index a2c6c74e8..0a71d450d 100644 --- a/spec/models/type_de_champ_spec.rb +++ b/spec/models/type_de_champ_spec.rb @@ -245,6 +245,7 @@ describe TypeDeChamp do it_behaves_like "a prefillable type de champ", :type_de_champ_date it_behaves_like "a prefillable type de champ", :type_de_champ_datetime it_behaves_like "a prefillable type de champ", :type_de_champ_civilite + it_behaves_like "a prefillable type de champ", :type_de_champ_pays it_behaves_like "a prefillable type de champ", :type_de_champ_yes_no it_behaves_like "a prefillable type de champ", :type_de_champ_checkbox it_behaves_like "a prefillable type de champ", :type_de_champ_drop_down_list @@ -265,7 +266,6 @@ describe TypeDeChamp do it_behaves_like "a non-prefillable type de champ", :type_de_champ_mesri it_behaves_like "a non-prefillable type de champ", :type_de_champ_carte it_behaves_like "a non-prefillable type de champ", :type_de_champ_address - it_behaves_like "a non-prefillable type de champ", :type_de_champ_pays it_behaves_like "a non-prefillable type de champ", :type_de_champ_regions it_behaves_like "a non-prefillable type de champ", :type_de_champ_departements it_behaves_like "a non-prefillable type de champ", :type_de_champ_siret diff --git a/spec/models/types_de_champ/prefill_drop_down_list_type_de_champ_spec.rb b/spec/models/types_de_champ/prefill_drop_down_list_type_de_champ_spec.rb index c1a8d17d7..fc7d4e573 100644 --- a/spec/models/types_de_champ/prefill_drop_down_list_type_de_champ_spec.rb +++ b/spec/models/types_de_champ/prefill_drop_down_list_type_de_champ_spec.rb @@ -9,7 +9,7 @@ RSpec.describe TypesDeChamp::PrefillDropDownListTypeDeChamp do it { expect(possible_values).to match( - [I18n.t("views.prefill_descriptions.edit.possible_values.drop_down_list_other")] + type_de_champ.drop_down_list_enabled_non_empty_options + [I18n.t("views.prefill_descriptions.edit.possible_values.drop_down_list_other_html")] + type_de_champ.drop_down_list_enabled_non_empty_options ) } end diff --git a/spec/models/types_de_champ/prefill_type_de_champ_spec.rb b/spec/models/types_de_champ/prefill_type_de_champ_spec.rb index 71faa26d5..28695be9a 100644 --- a/spec/models/types_de_champ/prefill_type_de_champ_spec.rb +++ b/spec/models/types_de_champ/prefill_type_de_champ_spec.rb @@ -38,7 +38,7 @@ RSpec.describe TypesDeChamp::PrefillTypeDeChamp, type: :model do context 'when the type de champ is prefillable' do let(:type_de_champ) { build(:type_de_champ_email) } - it { expect(possible_values).to match([I18n.t("views.prefill_descriptions.edit.possible_values.#{type_de_champ.type_champ}")]) } + it { expect(possible_values).to match([I18n.t("views.prefill_descriptions.edit.possible_values.#{type_de_champ.type_champ}_html")]) } end end diff --git a/spec/services/dossier_projection_service_spec.rb b/spec/services/dossier_projection_service_spec.rb index fbf7efd24..6d557c442 100644 --- a/spec/services/dossier_projection_service_spec.rb +++ b/spec/services/dossier_projection_service_spec.rb @@ -215,7 +215,7 @@ describe DossierProjectionService do dossier.champs_public.first.update(value: "qu'il est beau mon pays") end - it { is_expected.to eq("qu'il est beau mon pays") } + it { is_expected.to eq("") } end end end