fix(Champs::PaysChamp): validates Champs::PaysChamp respecting it's validation context. Also do not save value when external_id/code can't be found by lookup

app/jobs/migrations/batch_update_pays_values_job.rb

@@ -74,8 +74,8 @@ class Migrations::BatchUpdatePaysValuesJob < ApplicationJob
   def perform(ids)
     ids.each do |id|
       pays_champ = Champs::PaysChamp.find(id)
-      next if pays_champ.valid?
 
+      next if pays_champ.valid?(pays_champ.public? ? :champs_public_value : :champs_private_value)
       code = APIGeoService.country_code(pays_champ.value)
       value = if code.present?
         APIGeoService.country_name(code)

app/models/champs/pays_champ.rb

@@ -1,6 +1,15 @@
 class Champs::PaysChamp < Champs::TextChamp
-  validates :value, inclusion: APIGeoService.countries.pluck(:name), allow_nil: true, allow_blank: false
+  with_options if: :validate_champ_value? do
     validates :external_id, inclusion: APIGeoService.countries.pluck(:code), allow_nil: true, allow_blank: false
+    validates :value, inclusion: APIGeoService.countries.pluck(:name), allow_nil: true, allow_blank: false
+  end
+
+  # def value=(code) can reset champs to nil if value is empty, in case of prefill
+  #   we do not want to try to save the champ with an nil value
+  with_options if: -> { validation_context == :prefill } do
+    validates :external_id, inclusion: APIGeoService.countries.pluck(:code), allow_nil: false, allow_blank: false
+    validates :value, inclusion: APIGeoService.countries.pluck(:name), allow_nil: false, allow_blank: false
+  end
 
   def for_export
     [name, code]
@@ -26,8 +35,13 @@ class Champs::PaysChamp < Champs::TextChamp
       self.external_id = nil
       super(nil)
     elsif code != value
-      self.external_id = APIGeoService.country_code(code)
+      self.external_id = APIGeoService.country_code(code) # lookup by code which is a country name
-      super(code)
+
+      if self.external_id # if we match a country code, lookup for country name with code
+        super(APIGeoService.country_name(self.external_id, locale: 'FR'))
+      else # if we did not match any country code, external_id is nil as well as value
+        super(nil)
+      end
     end
   end
 

spec/jobs/migrations/batch_update_pays_values_job_spec.rb

@@ -1,12 +1,8 @@
 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') }
+    let(:pays_champ) { create(:champ_pays).tap { _1.update_columns(value: 'France', external_id: 'FR') } }
 
     it 'does not change it' do
       subject
@@ -16,7 +12,7 @@ describe Migrations::BatchUpdatePaysValuesJob, type: :job do
   end
 
   context "the value is incorrect" do
-    let(:pays_champ) { build(:champ_pays, value: 'Incorrect') }
+    let(:pays_champ) { create(:champ_pays).tap { _1.update_columns(value: 'Incorrect') } }
 
     it 'updates value to nil' do
       subject
@@ -26,7 +22,7 @@ describe Migrations::BatchUpdatePaysValuesJob, type: :job do
   end
 
   context "the value is easily cleanable" do
-    let(:pays_champ) { build(:champ_pays, value: 'Vietnam') }
+    let(:pays_champ) { create(:champ_pays).tap { _1.update_columns(value: 'Vietnam') } }
 
     it 'cleans the value' do
       subject
@@ -36,7 +32,7 @@ describe Migrations::BatchUpdatePaysValuesJob, type: :job do
   end
 
   context "the value is hard to clean" do
-    let(:pays_champ) { build(:champ_pays, value: 'CHRISTMAS (ILE)') }
+    let(:pays_champ) { create(:champ_pays).tap { _1.update_columns(value: 'CHRISTMAS (ILE)') } }
 
     it 'cleans the value' do
       subject