From 5e26acb0e154db4a04d285b439599de8ffa89a81 Mon Sep 17 00:00:00 2001 From: Damien Le Thiec Date: Thu, 12 Jan 2023 17:42:02 +0100 Subject: [PATCH] Make date and datetime prefillable (#8304) * Make date and datetime prefillable * Format in ISO8601 format --- .../batch_update_datetime_values_job.rb | 24 +++++++ app/models/champs/date_champ.rb | 28 ++++++--- app/models/champs/datetime_champ.rb | 25 ++++++-- app/models/prefill_params.rb | 4 +- app/models/type_de_champ.rb | 2 + config/locales/en.yml | 6 ++ config/locales/fr.yml | 6 ++ ...21221170137_normalize_datetime_values.rake | 16 +++++ .../instructeurs/dossiers_controller_spec.rb | 2 +- .../batch_update_datetime_values_job_spec.rb | 63 +++++++++++++++++++ spec/models/champ_shared_example.rb | 25 -------- spec/models/champ_spec.rb | 4 +- spec/models/champs/date_champ_spec.rb | 45 +++++++++++++ spec/models/champs/datetime_champ_spec.rb | 51 +++++++++++++++ spec/models/dossier_rebase_concern_spec.rb | 2 +- spec/models/prefill_params_spec.rb | 5 ++ spec/models/type_de_champ_spec.rb | 4 +- spec/system/users/brouillon_spec.rb | 2 +- 18 files changed, 268 insertions(+), 46 deletions(-) create mode 100644 app/jobs/migrations/batch_update_datetime_values_job.rb create mode 100644 lib/tasks/deployment/20221221170137_normalize_datetime_values.rake create mode 100644 spec/jobs/temporary/batch_update_datetime_values_job_spec.rb create mode 100644 spec/models/champs/date_champ_spec.rb create mode 100644 spec/models/champs/datetime_champ_spec.rb diff --git a/app/jobs/migrations/batch_update_datetime_values_job.rb b/app/jobs/migrations/batch_update_datetime_values_job.rb new file mode 100644 index 000000000..3818a3b58 --- /dev/null +++ b/app/jobs/migrations/batch_update_datetime_values_job.rb @@ -0,0 +1,24 @@ +class Migrations::BatchUpdateDatetimeValuesJob < ApplicationJob + def perform(ids) + Champs::DatetimeChamp.where(id: ids).each do |datetime_champ| + current_value_in_time = Time.zone.parse(datetime_champ.value) + + if current_value_in_time.present? + datetime_champ.update_columns(value: current_value_in_time.iso8601) + else + update_value_to_nil_if_possible(datetime_champ) + end + + rescue TypeError + update_value_to_nil_if_possible(datetime_champ) + end + end + + private + + def update_value_to_nil_if_possible(datetime_champ) + return if datetime_champ.value.nil? + + datetime_champ.update_columns(value: nil) unless datetime_champ.required? + end +end diff --git a/app/models/champs/date_champ.rb b/app/models/champs/date_champ.rb index 18510a0af..5d955c57c 100644 --- a/app/models/champs/date_champ.rb +++ b/app/models/champs/date_champ.rb @@ -22,7 +22,8 @@ # type_de_champ_id :integer # class Champs::DateChamp < Champ - before_save :format_before_save + before_validation :convert_to_iso8601, unless: -> { validation_context == :prefill } + validate :iso_8601 def search_terms # Text search is pretty useless for dates so we’re not including these champs @@ -38,12 +39,23 @@ class Champs::DateChamp < Champ private - def format_before_save - self.value = - begin - Time.zone.parse(value).to_date.iso8601 - rescue - nil - end + def convert_to_iso8601 + return if valid_iso8601? + + self.value = if /^\d{2}\/\d{2}\/\d{4}$/.match?(value) + Date.parse(value).iso8601 + else + nil + end + end + + def iso_8601 + return if valid_iso8601? || value.blank? + # i18n-tasks-use t('errors.messages.not_a_date') + errors.add :date, errors.generate_message(:value, :not_a_date) + end + + def valid_iso8601? + /^\d{4}-\d{2}-\d{2}$/.match?(value) end end diff --git a/app/models/champs/datetime_champ.rb b/app/models/champs/datetime_champ.rb index fccde406f..79b1ac812 100644 --- a/app/models/champs/datetime_champ.rb +++ b/app/models/champs/datetime_champ.rb @@ -22,7 +22,8 @@ # type_de_champ_id :integer # class Champs::DatetimeChamp < Champ - before_save :format_before_save + before_validation :convert_to_iso8601, unless: -> { validation_context == :prefill } + validate :iso_8601 def search_terms # Text search is pretty useless for datetimes so we’re not including these champs @@ -42,20 +43,34 @@ class Champs::DatetimeChamp < Champ private - def format_before_save + def convert_to_iso8601 if (value =~ /=>/).present? self.value = begin hash_date = YAML.safe_load(value.gsub('=>', ': ')) year, month, day, hour, minute = hash_date.values_at(1, 2, 3, 4, 5) - Time.zone.local(year, month, day, hour, minute).strftime("%d/%m/%Y %H:%M") + Time.zone.local(year, month, day, hour, minute).iso8601 rescue nil end elsif /^\d{2}\/\d{2}\/\d{4}\s\d{2}:\d{2}$/.match?(value) # old browsers can send with dd/mm/yyyy hh:mm format - self.value = Time.zone.strptime(value, "%d/%m/%Y %H:%M").strftime("%Y-%m-%d %H:%M") - elsif !(/^\d{4}-\d{2}-\d{2}\s\d{2}:\d{2}$/.match?(value)) # a datetime not correctly formatted should not be stored + self.value = Time.zone.strptime(value, "%d/%m/%Y %H:%M").iso8601 + elsif /^\d{4}-\d{2}-\d{2}\s\d{2}:\d{2}$/.match?(value) + self.value = Time.zone.strptime(value, "%Y-%m-%d %H:%M").iso8601 + elsif valid_iso8601? # a correct iso8601 datetime + self.value = Time.zone.strptime(value, "%Y-%m-%dT%H:%M").iso8601 + else self.value = nil end end + + def iso_8601 + return if valid_iso8601? || value.blank? + # i18n-tasks-use t('errors.messages.not_a_datetime') + errors.add :datetime, errors.generate_message(:value, :not_a_datetime) + end + + def valid_iso8601? + /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(:\d{2}\+\d{2}:\d{2})?$/.match?(value) + end end diff --git a/app/models/prefill_params.rb b/app/models/prefill_params.rb index 631f7225d..8240ff556 100644 --- a/app/models/prefill_params.rb +++ b/app/models/prefill_params.rb @@ -34,6 +34,8 @@ class PrefillParams NEED_VALIDATION_TYPES_DE_CHAMPS = [ TypeDeChamp.type_champs.fetch(:decimal_number), TypeDeChamp.type_champs.fetch(:integer_number), + TypeDeChamp.type_champs.fetch(:date), + TypeDeChamp.type_champs.fetch(:datetime), TypeDeChamp.type_champs.fetch(:civilite), TypeDeChamp.type_champs.fetch(:yes_no), TypeDeChamp.type_champs.fetch(:checkbox) @@ -63,7 +65,7 @@ class PrefillParams return true unless NEED_VALIDATION_TYPES_DE_CHAMPS.include?(champ.type_champ) champ.value = value - champ.valid? + champ.valid?(:prefill) end end end diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index f508b9776..5eeef4527 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -261,6 +261,8 @@ class TypeDeChamp < ApplicationRecord TypeDeChamp.type_champs.fetch(:phone), TypeDeChamp.type_champs.fetch(:iban), TypeDeChamp.type_champs.fetch(:civilite), + TypeDeChamp.type_champs.fetch(:date), + TypeDeChamp.type_champs.fetch(:datetime), TypeDeChamp.type_champs.fetch(:yes_no), TypeDeChamp.type_champs.fetch(:checkbox) ]) diff --git a/config/locales/en.yml b/config/locales/en.yml index 3c7c2f9ad..82e77d0ce 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -121,6 +121,8 @@ en: 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' examples: title: Example @@ -132,6 +134,8 @@ en: phone: 0612345678 iban: FR7611315000011234567890138 yes_no: "true" + date: "2023-02-01" + datetime: "2023-02-01T10:30" checkbox: "true" prefill_link_title: Prefill link (GET) prefill_link_info: Use the button to copy the link, then remplace the values with your data. @@ -457,6 +461,8 @@ en: # one: "Aucune parcelle cadastrale sur la zone sélectionnée" # other: "Aucune parcelle cadastrale sur les zones sélectionnées" not_an_integer: "must be an integer (without decimal)" + not_a_date: "must be a correct date" + not_a_datetime: "must be a correct datetime" blank: "can't be blank" time: formats: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index efd13e163..b7595279c 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -113,6 +113,8 @@ fr: 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é' examples: title: Exemple @@ -125,6 +127,8 @@ fr: iban: FR7611315000011234567890138 yes_no: "true" civilite: "M." + date: "2023-02-01" + datetime: "2023-02-01T10:30" checkbox: "true" prefill_link_title: Lien de préremplissage (GET) prefill_link_info: Copiez le lien grâce au bouton ci-dessous et remplacez les valeurs par les données dont vous disposez. @@ -460,6 +464,8 @@ fr: one: "Aucune parcelle cadastrale sur la zone sélectionnée" other: "Aucune parcelle cadastrale sur les zones sélectionnées" not_an_integer: "doit être un nombre entier (sans chiffres après la virgule)" + not_a_date: "doit être une date correctement formatée" + not_a_datetime: "doit être une date et heure correctement formatée" blank: "doit être rempli" time: formats: diff --git a/lib/tasks/deployment/20221221170137_normalize_datetime_values.rake b/lib/tasks/deployment/20221221170137_normalize_datetime_values.rake new file mode 100644 index 000000000..e7b4c46f6 --- /dev/null +++ b/lib/tasks/deployment/20221221170137_normalize_datetime_values.rake @@ -0,0 +1,16 @@ +namespace :after_party do + desc 'Deployment task: normalize_datetime_values' + task normalize_datetime_values: :environment do + puts "Running deploy task 'normalize_datetime_values'" + + # Put your task implementation HERE. + Champs::DateTimeChamp.in_batches do |datetime_champs| + Migrations::BatchUpdateDatetimeValueJob.perform_later(datetime_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/controllers/instructeurs/dossiers_controller_spec.rb b/spec/controllers/instructeurs/dossiers_controller_spec.rb index 98da86854..6126b213e 100644 --- a/spec/controllers/instructeurs/dossiers_controller_spec.rb +++ b/spec/controllers/instructeurs/dossiers_controller_spec.rb @@ -805,7 +805,7 @@ describe Instructeurs::DossiersController, type: :controller do expect(champ_multiple_drop_down_list.value).to eq('["un", "deux"]') expect(champ_linked_drop_down_list.primary_value).to eq('primary') expect(champ_linked_drop_down_list.secondary_value).to eq('secondary') - expect(champ_datetime.value).to eq('21/12/2019 13:17') + expect(champ_datetime.value).to eq('2019-12-21T13:17:00+01:00') expect(champ_repetition.champs.first.value).to eq('text') expect(dossier.reload.last_champ_private_updated_at).to eq(now) expect(response).to redirect_to(annotations_privees_instructeur_dossier_path(dossier.procedure, dossier)) diff --git a/spec/jobs/temporary/batch_update_datetime_values_job_spec.rb b/spec/jobs/temporary/batch_update_datetime_values_job_spec.rb new file mode 100644 index 000000000..5cdf4f516 --- /dev/null +++ b/spec/jobs/temporary/batch_update_datetime_values_job_spec.rb @@ -0,0 +1,63 @@ +describe Migrations::BatchUpdateDatetimeValuesJob, type: :job do + before do + datetime_champ.save(validate: false) + end + + context "when the value is a valid ISO8601 date" do + let!(:datetime_champ) { build(:champ_datetime, value: "2023-01-10T00:00:00+01:00") } + + subject { described_class.perform_now([datetime_champ.id]) } + + it "keeps the existing value" do + subject + expect(datetime_champ.reload.value).to eq("2023-01-10T00:00:00+01:00") + end + end + + context "when the value is a date convertible to IS8061" do + let!(:datetime_champ) { build(:champ_datetime, value: "2023-01-10") } + + subject { described_class.perform_now([datetime_champ.id]) } + + it "updates the value to ISO8601" do + subject + expect(datetime_champ.reload.value).to eq("2023-01-10T00:00:00+01:00") + end + end + + context "when the value is a date not convertible to IS8061" do + let!(:datetime_champ) { build(:champ_datetime, value: "blabla") } + + subject { described_class.perform_now([datetime_champ.id]) } + + it "updates the value to nil" do + subject + expect(datetime_champ.reload.value).to be_nil + end + end + + context "when the value is a date not convertible to IS8061 and the champ is required" do + let!(:datetime_champ) { build(:champ_datetime, value: "blabla") } + + subject { described_class.perform_now([datetime_champ.id]) } + + it "keeps the existing value" do + allow_any_instance_of(Champs::DatetimeChamp).to receive(:required?).and_return(true) + + subject + + expect(datetime_champ.reload.value).to eq("blabla") + end + end + + context "when the value is nil" do + let!(:datetime_champ) { build(:champ_datetime, value: nil) } + + subject { described_class.perform_now([datetime_champ.id]) } + + it "keeps the value to nil" do + subject + expect(datetime_champ.reload.value).to be_nil + end + end +end diff --git a/spec/models/champ_shared_example.rb b/spec/models/champ_shared_example.rb index a50ecc255..e53803228 100644 --- a/spec/models/champ_shared_example.rb +++ b/spec/models/champ_shared_example.rb @@ -53,29 +53,4 @@ shared_examples 'champ_spec' do it { expect(champ.mandatory_blank?).to be(false) } end end - - context "when type_champ=date" do - let(:champ) { build(:champ_date) } - - it "should convert %d/%m/%Y format to ISO" do - champ.value = "31/12/2017" - champ.save - champ.reload - expect(champ.value).to eq("2017-12-31") - end - - it "should convert to nil if date parse failed" do - champ.value = "bla" - champ.save - champ.reload - expect(champ.value).to be(nil) - end - - it "should convert empty string to nil" do - champ.value = "" - champ.save - champ.reload - expect(champ.value).to be(nil) - end - end end diff --git a/spec/models/champ_spec.rb b/spec/models/champ_spec.rb index d36f75773..b49606697 100644 --- a/spec/models/champ_spec.rb +++ b/spec/models/champ_spec.rb @@ -108,13 +108,13 @@ describe Champ do context 'when the value is sent by a modern browser' do let(:value) { '2017-12-31 10:23' } - it { expect(champ.value).to eq(value) } + it { expect(champ.value).to eq("2017-12-31T10:23:00+01:00") } end context 'when the value is sent by a old browser' do let(:value) { '31/12/2018 09:26' } - it { expect(champ.value).to eq('2018-12-31 09:26') } + it { expect(champ.value).to eq("2018-12-31T09:26:00+01:00") } end end diff --git a/spec/models/champs/date_champ_spec.rb b/spec/models/champs/date_champ_spec.rb new file mode 100644 index 000000000..9579a43eb --- /dev/null +++ b/spec/models/champs/date_champ_spec.rb @@ -0,0 +1,45 @@ +describe Champs::DateChamp do + let(:date_champ) { build(:champ_date) } + + describe '#convert_to_iso8601' do + it 'preserves nil' do + champ = champ_with_value(nil) + champ.save + expect(champ.reload.value).to be_nil + end + + it 'converts to nil if empty string' do + champ = champ_with_value("") + champ.save + expect(champ.reload.value).to be_nil + end + + it 'converts to nil if not ISO8601' do + champ = champ_with_value("12-21-2023") + champ.save + expect(champ.reload.value).to be_nil + end + + it 'converts to nil if not date' do + champ = champ_with_value("value") + champ.save + expect(champ.reload.value).to be_nil + end + + it "converts %d/%m/%Y format to ISO" do + champ = champ_with_value("31/12/2017") + champ.save + expect(champ.reload.value).to eq("2017-12-31") + end + + it 'preserves if ISO8601' do + champ = champ_with_value("2023-12-21") + champ.save + expect(champ.reload.value).to eq("2023-12-21") + end + end + + def champ_with_value(number) + date_champ.tap { |c| c.value = number } + end +end diff --git a/spec/models/champs/datetime_champ_spec.rb b/spec/models/champs/datetime_champ_spec.rb new file mode 100644 index 000000000..f61561e8a --- /dev/null +++ b/spec/models/champs/datetime_champ_spec.rb @@ -0,0 +1,51 @@ +describe Champs::DatetimeChamp do + let(:datetime_champ) { build(:champ_datetime) } + + describe '#convert_to_iso8601' do + it 'preserves nil' do + champ = champ_with_value(nil) + champ.save + expect(champ.reload.value).to be_nil + end + + it 'converts to nil if empty string' do + champ = champ_with_value("") + champ.save + expect(champ.reload.value).to be_nil + end + + it 'converts to nil if not ISO8601' do + champ = champ_with_value("12-21-2023 03:20") + champ.save + expect(champ.reload.value).to be_nil + end + + it 'converts to nil if not datetime' do + champ = champ_with_value("value") + champ.save + expect(champ.reload.value).to be_nil + end + + it 'preserves if ISO8601' do + champ = champ_with_value("2023-12-21T03:20") + champ.save + expect(champ.reload.value).to eq("2023-12-21T03:20:00+01:00") + end + + it 'converts to ISO8601 if form format' do + champ = champ_with_value("{3=>21, 2=>12, 1=>2023, 4=>3, 5=>20}") + champ.save + expect(champ.reload.value).to eq("2023-12-21T03:20:00+01:00") + end + + it 'converts to ISO8601 if old browser form format' do + champ = champ_with_value("21/12/2023 03:20") + champ.save + expect(champ.reload.value).to eq("2023-12-21T03:20:00+01:00") + end + end + + def champ_with_value(number) + datetime_champ.tap { |c| c.value = number } + end +end diff --git a/spec/models/dossier_rebase_concern_spec.rb b/spec/models/dossier_rebase_concern_spec.rb index a30bdf623..1e5183412 100644 --- a/spec/models/dossier_rebase_concern_spec.rb +++ b/spec/models/dossier_rebase_concern_spec.rb @@ -273,7 +273,7 @@ describe Dossier do }) procedure.draft_revision.remove_type_de_champ(yes_no_type_de_champ.stable_id) - datetime_champ.update(value: Date.today.to_s) + datetime_champ.update(value: Time.zone.now.to_s) text_champ.update(value: 'bonjour') # Add two rows then remove previous to last row in order to create a "hole" in the sequence repetition_champ.add_row(repetition_champ.dossier.revision) diff --git a/spec/models/prefill_params_spec.rb b/spec/models/prefill_params_spec.rb index d33a4e97b..f2bc4c51a 100644 --- a/spec/models/prefill_params_spec.rb +++ b/spec/models/prefill_params_spec.rb @@ -110,6 +110,8 @@ 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", :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" it_behaves_like "a champ public value that is authorized", :yes_no, "false" it_behaves_like "a champ public value that is authorized", :checkbox, "true" @@ -123,6 +125,8 @@ 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", :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" it_behaves_like "a champ private value that is authorized", :yes_no, "false" it_behaves_like "a champ private value that is authorized", :checkbox, "true" @@ -137,6 +141,7 @@ RSpec.describe PrefillParams do it_behaves_like "a champ public value that is unauthorized", :civilite, "value" it_behaves_like "a champ public value that is unauthorized", :date, "value" it_behaves_like "a champ public value that is unauthorized", :datetime, "value" + it_behaves_like "a champ public value that is unauthorized", :datetime, "12-22-2022T10:30" it_behaves_like "a champ public value that is unauthorized", :drop_down_list, "value" it_behaves_like "a champ public value that is unauthorized", :multiple_drop_down_list, "value" it_behaves_like "a champ public value that is unauthorized", :linked_drop_down_list, "value" diff --git a/spec/models/type_de_champ_spec.rb b/spec/models/type_de_champ_spec.rb index e73989315..916f7d68a 100644 --- a/spec/models/type_de_champ_spec.rb +++ b/spec/models/type_de_champ_spec.rb @@ -242,6 +242,8 @@ describe TypeDeChamp do it_behaves_like "a prefillable type de champ", :type_de_champ_email it_behaves_like "a prefillable type de champ", :type_de_champ_phone it_behaves_like "a prefillable type de champ", :type_de_champ_iban + 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_yes_no it_behaves_like "a prefillable type de champ", :type_de_champ_checkbox @@ -250,8 +252,6 @@ describe TypeDeChamp do it_behaves_like "a non-prefillable type de champ", :type_de_champ_communes it_behaves_like "a non-prefillable type de champ", :type_de_champ_dossier_link it_behaves_like "a non-prefillable type de champ", :type_de_champ_titre_identite - it_behaves_like "a non-prefillable type de champ", :type_de_champ_date - it_behaves_like "a non-prefillable type de champ", :type_de_champ_datetime it_behaves_like "a non-prefillable type de champ", :type_de_champ_drop_down_list it_behaves_like "a non-prefillable type de champ", :type_de_champ_multiple_drop_down_list it_behaves_like "a non-prefillable type de champ", :type_de_champ_linked_drop_down_list diff --git a/spec/system/users/brouillon_spec.rb b/spec/system/users/brouillon_spec.rb index d2dd85cc0..2df1194d8 100644 --- a/spec/system/users/brouillon_spec.rb +++ b/spec/system/users/brouillon_spec.rb @@ -57,7 +57,7 @@ describe 'The user' do expect(champ_value_for('text')).to eq('super texte') expect(champ_value_for('textarea')).to eq('super textarea') expect(champ_value_for('date')).to eq('2012-12-12') - expect(champ_value_for('datetime')).to eq('06/01/2030 07:05') + expect(champ_value_for('datetime')).to eq('2030-01-06T07:05:00+01:00') expect(champ_value_for('number')).to eq('42') expect(champ_value_for('decimal_number')).to eq('17') expect(champ_value_for('integer_number')).to eq('12')