normalize boolean values (#8320)

* extract parent for yes no and checkbox champs

* checkbox stores true / false instead of on / off

* normalize blank value to nil

* normalize invalid value to false

* after party task: normalize checkbox values

* after party task: normalize yes_no values

app/components/editable_champ/checkbox_component/checkbox_component.html.haml

@@ -1,4 +1,4 @@
 = @form.check_box :value,
-  { required: @champ.required?, id: @champ.input_id, aria: { describedby: @champ.describedby_id } },
-  'on',
-  'off'
+  { required: @champ.required?, id: @champ.input_id, checked: @champ.true?, aria: { describedby: @champ.describedby_id } },
+  'true',
+  'false'

app/components/editable_champ/engagement_component/engagement_component.html.haml

@@ -1,4 +0,0 @@
-= @form.check_box :value,
-  { required: @champ.required?, id: @champ.input_id, aria: { describedby: @champ.describedby_id } },
-  'on',
-  'off'

app/graphql/mutations/dossier_modifier_annotation_checkbox.rb

@@ -5,12 +5,8 @@ module Mutations
     argument :value, Boolean, required: true
 
     def resolve(dossier:, annotation_id:, instructeur:, value:)
-      resolve_with_type(dossier:, annotation_id:, instructeur:, value:) do |type_champ, value|
-        if type_champ == TypeDeChamp.type_champs.fetch(:yes_no)
-          value ? 'true' : 'false'
-        else
-          value ? 'on' : 'off'
-        end
+      resolve_with_type(dossier:, annotation_id:, instructeur:, value:) do |_, value|
+        value ? 'true' : 'false'
       end
     end
 

app/models/champs/boolean_champ.rb

@@ -0,0 +1,69 @@
+# == Schema Information
+#
+# Table name: champs
+#
+#  id                             :integer          not null, primary key
+#  data                           :jsonb
+#  fetch_external_data_exceptions :string           is an Array
+#  prefilled                      :boolean          default(FALSE)
+#  private                        :boolean          default(FALSE), not null
+#  rebased_at                     :datetime
+#  row                            :integer
+#  type                           :string
+#  value                          :string
+#  value_json                     :jsonb
+#  created_at                     :datetime
+#  updated_at                     :datetime
+#  dossier_id                     :integer
+#  etablissement_id               :integer
+#  external_id                    :string
+#  parent_id                      :bigint
+#  type_de_champ_id               :integer
+#
+class Champs::BooleanChamp < Champ
+  TRUE_VALUE = 'true'
+  FALSE_VALUE = 'false'
+
+  before_validation :set_value_to_nil, if: -> { value.blank? }
+  before_validation :set_value_to_false, unless: -> { ([nil, TRUE_VALUE, FALSE_VALUE]).include?(value) }
+
+  def true?
+    value == TRUE_VALUE
+  end
+
+  def search_terms
+    if true?
+      [libelle]
+    end
+  end
+
+  def to_s
+    processed_value
+  end
+
+  def for_tag
+    processed_value
+  end
+
+  def for_export
+    processed_value
+  end
+
+  def for_api_v2
+    true? ? 'true' : 'false'
+  end
+
+  private
+
+  def processed_value
+    true? ? 'Oui' : 'Non'
+  end
+
+  def set_value_to_nil
+    self.value = nil
+  end
+
+  def set_value_to_false
+    self.value = FALSE_VALUE
+  end
+end

app/models/champs/checkbox_champ.rb

@@ -20,11 +20,7 @@
 #  parent_id                      :bigint
 #  type_de_champ_id               :integer
 #
-class Champs::CheckboxChamp < Champs::YesNoChamp
-  def true?
-    value == 'on'
-  end
-
+class Champs::CheckboxChamp < Champs::BooleanChamp
   def for_export
     true? ? 'on' : 'off'
   end
@@ -32,4 +28,16 @@ class Champs::CheckboxChamp < Champs::YesNoChamp
   def mandatory_blank?
     mandatory? && (blank? || !true?)
   end
+
+  # TODO remove when normalize_checkbox_values is over
+  def true?
+    value_with_legacy == TRUE_VALUE
+  end
+
+  private
+
+  # TODO remove when normalize_checkbox_values is over
+  def value_with_legacy
+    value == 'on' ? TRUE_VALUE : value
+  end
 end

app/models/champs/yes_no_champ.rb

@@ -20,36 +20,5 @@
 #  parent_id                      :bigint
 #  type_de_champ_id               :integer
 #
-class Champs::YesNoChamp < Champ
-  def search_terms
-    if true?
-      [libelle]
-    end
-  end
-
-  def to_s
-    processed_value
-  end
-
-  def for_tag
-    processed_value
-  end
-
-  def for_export
-    processed_value
-  end
-
-  def true?
-    value == 'true'
-  end
-
-  def for_api_v2
-    true? ? 'true' : 'false'
-  end
-
-  private
-
-  def processed_value
-    true? ? 'Oui' : 'Non'
-  end
+class Champs::YesNoChamp < Champs::BooleanChamp
 end

config/locales/en.yml

@@ -121,7 +121,7 @@ en:
           phone: A phone number
           iban: An Iban number
           yes_no: '"true" for Yes, "false" pour No'
-          checkbox: '"on" to check, "off" to uncheck'
+          checkbox: '"true" to check, "false" to uncheck'
         examples:
           title: Example
           text: Short text
@@ -132,7 +132,7 @@ en:
           phone: 0612345678
           iban: FR7611315000011234567890138
           yes_no: "true"
-          checkbox: "on"
+          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.
         prefill_link_too_long: Warning, the prefill link is too long and may not work on all browsers.

config/locales/fr.yml

@@ -113,7 +113,7 @@ fr:
           phone: Un numéro de téléphone
           iban: Un numéro Iban
           yes_no: '"true" pour Oui, "false" pour Non'
-          checkbox: '"on" pour coché, "off" pour décoché'
+          checkbox: '"true" pour coché, "false" pour décoché'
         examples:
           title: Exemple
           text: Texte court
@@ -125,7 +125,7 @@ fr:
           iban: FR7611315000011234567890138
           yes_no: "true"
           civilite: "M."
-          checkbox: "on"
+          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.
         prefill_link_too_long: Attention, ce lien de préremplissage est trop long et risque de ne pas fonctionner sur certains navigateurs.

config/locales/models/type_de_champ/en.yml

@@ -45,7 +45,6 @@ fr:
           annuaire_education: 'Schooling directory'
           rna: 'RNA'
           carte: 'Card'
-          engagement: 'Commitment'
           cnaf: 'Data from Caisse nationale des allocations familiales'
           dgfip: 'Data from Direction générale des Finances publiques'
           pole_emploi: 'Pôle emploi status'

config/locales/models/type_de_champ/fr.yml

@@ -45,7 +45,6 @@ fr:
           annuaire_education: 'Annuaire de l’éducation'
           rna: 'RNA'
           carte: 'Carte'
-          engagement: 'Engagement'
           cnaf: 'Données de la Caisse nationale des allocations familiales'
           dgfip: 'Données de la Direction générale des Finances publiques'
           pole_emploi: 'Situation Pôle emploi'

lib/tasks/deployment/20221221153640_normalize_checkbox_values.rake

@@ -0,0 +1,32 @@
+namespace :after_party do
+  desc 'Deployment task: normalize_checkbox_values'
+  task normalize_checkbox_values: :environment do
+    puts "Running deploy task 'normalize_checkbox_values'"
+
+    scope_blank = Champs::CheckboxChamp.where(value: '')
+    scope_on = Champs::CheckboxChamp.where(value: 'on')
+    scope_off = Champs::CheckboxChamp.where(value: 'off')
+    scope_invalid = Champs::CheckboxChamp.where.not(value: [nil, 'true', 'false'])
+
+    progress = ProgressReport.new(scope_blank.count + scope_on.count + scope_off.count + scope_invalid.count)
+    update_all(scope_blank, nil, progress)
+    update_all(scope_on, 'true', progress)
+    update_all(scope_off, 'false', progress)
+    update_all(scope_invalid, 'false', progress)
+    progress.finish
+
+    # 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
+
+  private
+
+  def update_all(scope, value, progress)
+    scope.in_batches(of: 10_000) do |checkboxes|
+      progress.inc(checkboxes.count)
+      checkboxes.update_all(value: value)
+    end
+  end
+end

lib/tasks/deployment/20221221155508_normalize_yes_no_values.rake

@@ -0,0 +1,28 @@
+namespace :after_party do
+  desc 'Deployment task: normalize_yes_no_values'
+  task normalize_yes_no_values: :environment do
+    puts "Running deploy task 'normalize_yes_no_values'"
+
+    scope_blank = Champs::YesNoChamp.where(value: '')
+    scope_invalid = Champs::YesNoChamp.where.not(value: [nil, 'true', 'false'])
+
+    progress = ProgressReport.new(scope_blank.count + scope_invalid.count)
+    update_all(scope_blank, nil, progress)
+    update_all(scope_invalid, 'false', progress)
+    progress.finish
+
+    # 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
+
+  private
+
+  def update_all(scope, value, progress)
+    scope.in_batches(of: 10_000) do |yes_no|
+      progress.inc(yes_no.count)
+      yes_no.update_all(value: value)
+    end
+  end
+end

spec/factories/champ.rb

@@ -58,7 +58,7 @@ FactoryBot.define do
 
     factory :champ_checkbox, class: 'Champs::CheckboxChamp' do
       type_de_champ { association :type_de_champ_checkbox, procedure: dossier.procedure }
-      value { 'on' }
+      value { 'true' }
     end
 
     factory :champ_civilite, class: 'Champs::CiviliteChamp' do

spec/graphql/dossier_spec.rb

@@ -65,7 +65,7 @@ RSpec.describe Types::DossierType, type: :graphql do
     let(:dossier) { create(:dossier, :accepte, :with_populated_champs, procedure: procedure) }
     let(:query) { DOSSIER_WITH_CHAMPS_QUERY }
     let(:variables) { { number: dossier.id } }
-    let(:checkbox_value) { 'on' }
+    let(:checkbox_value) { 'true' }
 
     before do
       dossier.champs_public.first.update(value: checkbox_value)
@@ -78,7 +78,7 @@ RSpec.describe Types::DossierType, type: :graphql do
     end
 
     context 'when checkbox is false' do
-      let(:checkbox_value) { 'off' }
+      let(:checkbox_value) { 'false' }
       it { expect(data[:dossier][:champs].size).to eq 1 }
       it { expect(data[:dossier][:champs][0][:__typename]).to eq "CheckboxChamp" }
     end

spec/lib/tasks/deployment/20221221153640_normalize_checkbox_values_spec.rake

@@ -0,0 +1,26 @@
+describe '20221221153640_normalize_checkbox_values' do
+  shared_examples "a checkbox value normalizer" do |value, expected_value|
+    let(:rake_task) { Rake::Task['after_party:normalize_checkbox_values'] }
+    let(:checkbox) { create(:champ_checkbox) }
+
+    subject(:run_task) { rake_task.invoke }
+
+    context "when the value is #{value}" do
+      before do
+        checkbox.value = value
+        checkbox.save(validate: false)
+      end
+
+      after { rake_task.reenable }
+
+      it "normalizes the value to #{expected_value}" do
+        expect { run_task }.to change { checkbox.reload.value }.from(value).to(expected_value)
+      end
+    end
+  end
+
+  it_behaves_like 'a checkbox value normalizer', '', nil
+  it_behaves_like 'a checkbox value normalizer', 'on', 'true'
+  it_behaves_like 'a checkbox value normalizer', 'off', 'false'
+  it_behaves_like 'a checkbox value normalizer', 'random value', 'false'
+end

spec/lib/tasks/deployment/20221221155508_normalize_yes_no_values_spec.rake

@@ -0,0 +1,24 @@
+describe '20221221155508_normalize_yes_no_values' do
+  shared_examples "a yes_no value normalizer" do |value, expected_value|
+    let(:rake_task) { Rake::Task['after_party:normalize_yes_no_values'] }
+    let(:yes_no) { create(:champ_yes_no) }
+
+    subject(:run_task) { rake_task.invoke }
+
+    context "when the value is #{value}" do
+      before do
+        yes_no.value = value
+        yes_no.save(validate: false)
+      end
+
+      after { rake_task.reenable }
+
+      it "normalizes the value to #{expected_value}" do
+        expect { run_task }.to change { yes_no.reload.value }.from(value).to(expected_value)
+      end
+    end
+  end
+
+  it_behaves_like 'a yes_no value normalizer', '', nil
+  it_behaves_like 'a yes_no value normalizer', 'random value', 'false'
+end

spec/models/champ_spec.rb

@@ -212,13 +212,13 @@ describe Champ do
         let(:type_de_champ) { build(:type_de_champ_checkbox, libelle: libelle) }
 
         context 'when the box is checked' do
-          let(:value) { 'on' }
+          let(:value) { 'true' }
 
           it { is_expected.to eq([libelle]) }
         end
 
         context 'when the box is unchecked' do
-          let(:value) { 'off' }
+          let(:value) { 'false' }
 
           it { is_expected.to be_nil }
         end

spec/models/champs/checkbox_champ_spec.rb

@@ -1,19 +1,23 @@
 describe Champs::CheckboxChamp do
-  let(:checkbox) { Champs::CheckboxChamp.new(value: value) }
+  it_behaves_like "a boolean champ" do
+    let(:boolean_champ) { Champs::CheckboxChamp.new(value: value) }
+  end
 
-  describe '#to_s' do
-    subject { checkbox.to_s }
+  # TODO remove when normalize_checkbox_values is over
+  describe '#true?' do
+    let(:checkbox_champ) { Champs::CheckboxChamp.new(value: value) }
+    subject { checkbox_champ.true? }
 
-    context 'when the value is on' do
+    context "when the checkbox value is 'on'" do
       let(:value) { 'on' }
 
-      it { is_expected.to eq('Oui') }
+      it { is_expected.to eq(true) }
     end
 
-    context 'when the value is off' do
+    context "when the checkbox value is 'off'" do
       let(:value) { 'off' }
 
-      it { is_expected.to eq('Non') }
+      it { is_expected.to eq(false) }
     end
   end
 end

spec/models/champs/yes_no_champ_spec.rb

@@ -1,23 +1,5 @@
 describe Champs::YesNoChamp do
-  describe '#to_s' do
-    subject { Champs::YesNoChamp.new(value: value).to_s }
-
-    context 'when the value is false' do
-      let(:value) { "false" }
-
-      it { is_expected.to eq("Non") }
-    end
-
-    context 'when the value is true' do
-      let(:value) { "true" }
-
-      it { is_expected.to eq("Oui") }
-    end
-
-    context 'when the value is nil' do
-      let(:value) { nil }
-
-      it { is_expected.to eq("Non") }
-    end
+  it_behaves_like "a boolean champ" do
+    let(:boolean_champ) { Champs::YesNoChamp.new(value: value) }
   end
 end

spec/models/logic/champ_value_spec.rb

@@ -70,7 +70,7 @@ describe Logic::ChampValue do
   end
 
   context 'checkbox tdc' do
-    let(:champ) { create(:champ_checkbox, value: 'on') }
+    let(:champ) { create(:champ_checkbox, value: 'true') }
 
     it { expect(champ_value(champ.stable_id).type([champ.type_de_champ])).to eq(:boolean) }
     it { is_expected.to eq(true) }

spec/serializers/champ_serializer_spec.rb

@@ -134,16 +134,16 @@ describe ChampSerializer do
     end
 
     context 'when type champ checkbox' do
-      context 'on' do
-        let(:champ) { create(:champ_checkbox, value: 'on') }
+      context 'true' do
+        let(:champ) { create(:champ_checkbox, value: 'true') }
 
-        it { is_expected.to include(value: 'on') }
+        it { is_expected.to include(value: 'true') }
       end
 
-      context 'off' do
-        let(:champ) { create(:champ_checkbox, value: 'off') }
+      context 'false' do
+        let(:champ) { create(:champ_checkbox, value: 'false') }
 
-        it { is_expected.to include(value: 'off') }
+        it { is_expected.to include(value: 'false') }
       end
 
       context 'nil' do

spec/support/shared_examples_for_boolean_champs.rb

@@ -0,0 +1,59 @@
+RSpec.shared_examples "a boolean champ" do
+  describe 'before validation' do
+    subject { boolean_champ.valid? }
+
+    context "when the value is blank" do
+      let(:value) { "" }
+
+      it "normalizes the value to nil" do
+        expect { subject }.to change { boolean_champ.value }.from(value).to(nil)
+      end
+    end
+
+    context "when the value is something else" do
+      let(:value) { "something else" }
+
+      it "normalizes the value to 'false'" do
+        expect { subject }.to change { boolean_champ.value }.from(value).to(Champs::BooleanChamp::FALSE_VALUE)
+      end
+    end
+  end
+
+  describe '#true?' do
+    subject { boolean_champ.true? }
+
+    context "when the checkbox value is 'true'" do
+      let(:value) { 'true' }
+
+      it { is_expected.to eq(true) }
+    end
+
+    context "when the checkbox value is 'false'" do
+      let(:value) { 'false' }
+
+      it { is_expected.to eq(false) }
+    end
+  end
+
+  describe '#to_s' do
+    subject { boolean_champ.to_s }
+
+    context 'when the value is false' do
+      let(:value) { 'false' }
+
+      it { is_expected.to eq('Non') }
+    end
+
+    context 'when the value is true' do
+      let(:value) { 'true' }
+
+      it { is_expected.to eq('Oui') }
+    end
+
+    context 'when the value is nil' do
+      let(:value) { nil }
+
+      it { is_expected.to eq("Non") }
+    end
+  end
+end

spec/system/users/brouillon_spec.rb

@@ -61,7 +61,7 @@ describe 'The user' do
     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')
-    expect(champ_value_for('checkbox')).to eq('on')
+    expect(champ_value_for('checkbox')).to eq('true')
     expect(champ_value_for('civilite')).to eq('Mme')
     expect(champ_value_for('email')).to eq('loulou@yopmail.com')
     expect(champ_value_for('phone')).to eq('0123456789')

spec/views/shared/dossiers/_champs.html.haml_spec.rb

@@ -12,7 +12,7 @@ describe 'shared/dossiers/champs.html.haml', type: :view do
 
   context "there are some champs" do
     let(:dossier) { create(:dossier) }
-    let(:champ1) { create(:champ_checkbox, dossier: dossier, value: "on") }
+    let(:champ1) { create(:champ_checkbox, dossier: dossier, value: 'true') }
     let(:champ2) { create(:champ_header_section, dossier: dossier, value: "Section") }
     let(:champ3) { create(:champ_explication, dossier: dossier, value: "mazette") }
     let(:champ4) { create(:champ_dossier_link, dossier: dossier, value: dossier.id) }
@@ -22,7 +22,7 @@ describe 'shared/dossiers/champs.html.haml', type: :view do
 
     it "renders titles and values of champs" do
       expect(subject).to include(champ1.libelle)
-      expect(subject).to include(champ1.value)
+      expect(subject).to include('Oui')
 
       expect(subject).to have_css(".header-section")
       expect(subject).to include(champ2.libelle)
@@ -70,7 +70,7 @@ describe 'shared/dossiers/champs.html.haml', type: :view do
     context "with seen_at" do
       let(:dossier) { create(:dossier) }
       let(:nouveau_groupe_instructeur) { create(:groupe_instructeur, procedure: dossier.procedure) }
-      let(:champ1) { create(:champ_checkbox, dossier: dossier, value: "on") }
+      let(:champ1) { create(:champ_checkbox, dossier: dossier, value: 'true') }
       let(:champs) { [champ1] }
 
       context "with a demande_seen_at after groupe_instructeur_updated_at" do
@@ -113,7 +113,7 @@ describe 'shared/dossiers/champs.html.haml', type: :view do
 
   context "with seen_at" do
     let(:dossier) { create(:dossier) }
-    let(:champ1) { create(:champ_checkbox, dossier: dossier, value: "on") }
+    let(:champ1) { create(:champ_checkbox, dossier: dossier, value: 'true') }
     let(:champs) { [champ1] }
 
     context "with a demande_seen_at after champ updated_at" do

spec/views/shared/dossiers/_edit.html.haml_spec.rb

@@ -8,7 +8,7 @@ describe 'shared/dossiers/edit.html.haml', type: :view do
 
   context 'when there are some champs' do
     let(:dossier) { create(:dossier) }
-    let(:champ_checkbox) { create(:champ_checkbox, dossier: dossier, value: 'on') }
+    let(:champ_checkbox) { create(:champ_checkbox, dossier: dossier, value: 'true') }
     let(:champ_header_section) { create(:champ_header_section, dossier: dossier, value: 'Section') }
     let(:champ_explication) { create(:champ_explication, dossier: dossier, value: 'mazette') }
     let(:champ_dossier_link) { create(:champ_dossier_link, dossier: dossier, value: dossier.id) }