Merge pull request #10412 from tchak/fix-champ-type-de-champ-mismatch

fix(champ): champ value formatters should check champ type

app/models/type_de_champ.rb

@@ -686,8 +686,7 @@ class TypeDeChamp < ApplicationRecord
   class << self
     def champ_value(type_champ, champ)
       dynamic_type_class = type_champ_to_class_name(type_champ).constantize
-      # special case for linked drop down champ – it's blank implementation is not what you think
+      if use_default_value?(type_champ, champ)
-      if (type_champ != TypeDeChamp.type_champs.fetch(:linked_drop_down_list) || champ.nil? || champ.value.blank?) && champ.blank?
         dynamic_type_class.champ_default_value
       else
         dynamic_type_class.champ_value(champ)
@@ -696,8 +695,7 @@ class TypeDeChamp < ApplicationRecord
 
     def champ_value_for_api(type_champ, champ, version = 2)
       dynamic_type_class = type_champ_to_class_name(type_champ).constantize
-      # special case for linked drop down champ – it's blank implementation is not what you think
+      if use_default_value?(type_champ, champ)
-      if (type_champ != TypeDeChamp.type_champs.fetch(:linked_drop_down_list) || champ.nil? || champ.value.blank?) && champ.blank?
         dynamic_type_class.champ_default_api_value(version)
       else
         dynamic_type_class.champ_value_for_api(champ, version)
@@ -706,8 +704,7 @@ class TypeDeChamp < ApplicationRecord
 
     def champ_value_for_export(type_champ, champ, path = :value)
       dynamic_type_class = type_champ_to_class_name(type_champ).constantize
-      # special case for linked drop down champ – it's blank implementation is not what you think
+      if use_default_value?(type_champ, champ)
-      if (type_champ != TypeDeChamp.type_champs.fetch(:linked_drop_down_list) || champ.nil? || champ.value.blank?) && champ.blank?
         dynamic_type_class.champ_default_export_value(path)
       else
         dynamic_type_class.champ_value_for_export(champ, path)
@@ -715,11 +712,10 @@ class TypeDeChamp < ApplicationRecord
     end
 
     def champ_value_for_tag(type_champ, champ, path = :value)
-      dynamic_type_class = type_champ_to_class_name(type_champ).constantize
+      if use_default_value?(type_champ, champ)
-      # special case for linked drop down champ – it's blank implementation is not what you think
-      if (type_champ != TypeDeChamp.type_champs.fetch(:linked_drop_down_list) || champ.nil? || champ.value.blank?) && champ.blank?
         ''
       else
+        dynamic_type_class = type_champ_to_class_name(type_champ).constantize
         dynamic_type_class.champ_value_for_tag(champ, path)
       end
     end
@@ -731,6 +727,19 @@ class TypeDeChamp < ApplicationRecord
     def type_champ_to_class_name(type_champ)
       "TypesDeChamp::#{type_champ.classify}TypeDeChamp"
     end
+
+    private
+
+    def use_default_value?(type_champ, champ)
+      # no champ
+      return true if champ.nil?
+      # type de champ on the revision changed
+      return true if type_champ_to_champ_class_name(type_champ) != champ.type
+      # special case for linked drop down champ – it's blank implementation is not what you think
+      return champ.value.blank? if type_champ == TypeDeChamp.type_champs.fetch(:linked_drop_down_list)
+
+      champ.blank?
+    end
   end
 
   private

spec/models/champ_spec.rb

@@ -185,6 +185,14 @@ describe Champ do
 
       it { expect(champ.for_export).to eq('Crétinier, Mousserie') }
     end
+
+    context 'when type_de_champ and champ.type mismatch' do
+      let(:champ_yes_no) { create(:champ_yes_no, value: 'true') }
+      let(:champ_text) { create(:champ_text, value: 'Hello') }
+
+      it { expect(TypeDeChamp.champ_value_for_export(champ_text.type_champ, champ_yes_no)).to eq(nil) }
+      it { expect(TypeDeChamp.champ_value_for_export(champ_yes_no.type_champ, champ_text)).to eq('Non') }
+    end
   end
 
   describe '#search_terms' do

spec/serializers/champ_serializer_spec.rb

@@ -12,7 +12,7 @@ describe ChampSerializer do
     end
 
     context 'when type champ is not piece justificative' do
-      let(:champ) { create(:champ, value: "blah") }
+      let(:champ) { create(:champ_text, value: "blah") }
 
       it { is_expected.to include(value: "blah") }
     end