Refs #6092; Add specs on CustomlFields6092-update-custom-fields

- Show validation errors on fields
- Smarter attachments

7 files changed, 176 insertions, 30 deletions

diff --git a/Gemfile b/Gemfile
index 95f366fc1..302db408f 100644
--- a/Gemfile
+++ b/Gemfile
@@ -130,6 +130,7 @@ gem 'acts_as_tree', '~> 2.1.0', require: 'acts_as_tree'
 
 gem 'rabl'
 gem 'carrierwave', '~> 1.0'
+gem 'rmagick'
 
 gem 'sidekiq', require: ['sidekiq', 'sidekiq/web']
 gem 'whenever', github: 'af83/whenever', require: false # '~> 0.9'
diff --git a/Gemfile.lock b/Gemfile.lock
index 2071eee78..db412abd0 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -444,6 +444,7 @@ GEM
     rgeo-activerecord (4.0.5)
       activerecord (~> 4.2)
       rgeo (~> 0.3)
+    rmagick (2.16.0)
     roo (2.7.1)
       nokogiri (~> 1)
       rubyzip (~> 1.1, < 2.0.0)
@@ -681,6 +682,7 @@ DEPENDENCIES
   reflex!
   responders
   rgeo (~> 0.5.2)
+  rmagick
   roo
   rspec-rails (~> 3.5.0)
   rspec-snapshot
diff --git a/app/assets/stylesheets/components/_lists.sass b/app/assets/stylesheets/components/_lists.sass
index 3cce20021..f9b449541 100644
--- a/app/assets/stylesheets/components/_lists.sass
+++ b/app/assets/stylesheets/components/_lists.sass
@@ -30,6 +30,8 @@ $dlWidth: 40%
     // overflow: hidden
     vertical-align: top
     padding: 5px 15px 6px 15px
+    img
+      max-width: 100%
 
   // Definition term
   .dl-term
@@ -58,4 +60,4 @@ $dlWidth: 40%
     ul
       list-style: none
       padding-left: 0
-      margin-bottom: 0
\ No newline at end of file
+      margin-bottom: 0
diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb
index deb0326f8..de090fac2 100644
--- a/app/models/custom_field.rb
+++ b/app/models/custom_field.rb
@@ -2,10 +2,11 @@ class CustomField < ApplicationModel
 
   extend Enumerize
   belongs_to :workgroup
-  enumerize :field_type, in: %i{list integer string attachment}
+  enumerize :field_type, in: %i{list integer float string attachment}
 
   validates :name, uniqueness: {scope: [:resource_type, :workgroup_id]}
   validates :code, uniqueness: {scope: [:resource_type, :workgroup_id], case_sensitive: false}, presence: true
+  validates :workgroup, :resource_type, :field_type, presence: true
 
   class Collection < HashWithIndifferentAccess
     def initialize object, workgroup=nil
@@ -47,7 +48,7 @@ class CustomField < ApplicationModel
       end
 
       def options
-        @custom_field.options || {}
+        @custom_field.options&.stringify_keys || {}
       end
 
       def validate
@@ -59,6 +60,10 @@ class CustomField < ApplicationModel
         @valid
       end
 
+      def required?
+        !!options["required"]
+      end
+
       def value
         @raw_value
       end
@@ -76,7 +81,8 @@ class CustomField < ApplicationModel
       end
 
       def errors_key
-        "custom_fields.#{code}"
+        # this must match the ID used in the inputs
+        "custom_field_#{code}"
       end
 
       def to_hash
@@ -91,7 +97,7 @@ class CustomField < ApplicationModel
       end
 
       def preprocess_value_for_assignment val
-        val
+        val || default_value
       end
 
       def render_partial
@@ -111,7 +117,7 @@ class CustomField < ApplicationModel
           @instance.custom_field
         end
 
-        delegate :custom_field, :value, :options, to: :@instance
+        delegate :custom_field, :value, :options, :required?, to: :@instance
         delegate :code, :name, :field_type, to: :custom_field
 
         def to_s
@@ -122,7 +128,7 @@ class CustomField < ApplicationModel
         protected
 
         def form_input_id
-          "custom_field_#{code}"
+          "custom_field_#{code}".to_sym
         end
 
         def form_input_name
@@ -144,26 +150,64 @@ class CustomField < ApplicationModel
 
     class Integer < Base
       def value
-        @raw_value&.to_i
+        @raw_value.present? ? @raw_value.to_i : nil
       end
 
       def validate
         @valid = true
-        return if @raw_value.is_a?(Fixnum) || @raw_value.is_a?(Float)
-        unless @raw_value.to_s =~ /\A\d*\Z/
+        return if @raw_value.is_a?(Fixnum)
+        unless @raw_value.to_s =~ /\A-?\d*\Z/
           @owner.errors.add errors_key, "'#{@raw_value}' is not a valid integer"
           @valid = false
         end
       end
+
+      class Input < Base::Input
+        def form_input_options
+          super.update({
+            as: :integer
+          })
+        end
+      end
+    end
+
+    class Float < Integer
+      def value
+        @raw_value.present? ? @raw_value.to_f : nil
+      end
+
+      def validate
+        @valid = true
+        return if @raw_value.is_a?(Fixnum) || @raw_value.is_a?(Float)
+        unless @raw_value.to_s =~ /\A-?\d*(\.\d+)?\Z/
+          @owner.errors.add errors_key, "'#{@raw_value}' is not a valid float"
+          @valid = false
+        end
+      end
+
+      class Input < Base::Input
+        def form_input_options
+          super.update({
+            as: :float
+          })
+        end
+      end
     end
 
     class List < Integer
       def validate
         super
         return unless value.present?
-        unless value >= 0 && value < options["list_values"].size
-          @owner.errors.add errors_key, "'#{@raw_value}' is not a valid value"
-          @valid = false
+        if options["list_values"].is_a?(Hash)
+          unless options["list_values"].keys.map(&:to_s).include?(value.to_s)
+            @owner.errors.add errors_key, "'#{@raw_value}' is not a valid value"
+            @valid = false
+          end
+        else
+          unless value >= 0 && value < options["list_values"].size
+            @owner.errors.add errors_key, "'#{@raw_value}' is not a valid value"
+            @valid = false
+          end
         end
       end
 
@@ -178,6 +222,7 @@ class CustomField < ApplicationModel
           collection = options["list_values"]
           collection = collection.each_with_index.to_a if collection.is_a?(Array)
           collection = collection.map(&:reverse) if collection.is_a?(Hash)
+          collection = [["", ""]] + collection unless required?
           super.update({
             selected: value,
             collection: collection
diff --git a/app/uploaders/custom_field_attachment_uploader.rb b/app/uploaders/custom_field_attachment_uploader.rb
index 411b65bc3..94a14f7e4 100644
--- a/app/uploaders/custom_field_attachment_uploader.rb
+++ b/app/uploaders/custom_field_attachment_uploader.rb
@@ -1,5 +1,5 @@
 class CustomFieldAttachmentUploader < CarrierWave::Uploader::Base
-
+  include CarrierWave::RMagick
   storage :file
 
   def store_dir
@@ -9,4 +9,28 @@ class CustomFieldAttachmentUploader < CarrierWave::Uploader::Base
   def extension_whitelist
     model.send "#{mounted_as}_extension_whitelist"
   end
+
+  process :dynamic_versions
+
+  def method_missing mid, *args
+    unless @dynamic_versions_loaded
+      dynamic_versions
+      @versions = nil
+      cache!
+    end
+    send mid, *args
+  end
+
+  def dynamic_versions
+    custom_field = model.custom_fields[mounted_as.to_s.gsub('custom_field_', '').to_sym]
+    _versions = custom_field.options["versions"] || {}
+
+    _versions.each do |name, size|
+      size = size.split('x')
+      self.class.version name do
+        process :resize_to_fit => size
+      end
+    end
+    @dynamic_versions_loaded = true
+  end
 end
diff --git a/app/views/shared/custom_fields/_attachment.html.slim b/app/views/shared/custom_fields/_attachment.html.slim
index 32d0fda4d..13714d239 100644
--- a/app/views/shared/custom_fields/_attachment.html.slim
+++ b/app/views/shared/custom_fields/_attachment.html.slim
@@ -1,4 +1,10 @@
 - if field.value.present?
-  = link_to I18n.t("custom_fields.#{field.owner.class.name.demodulize.underscore}.#{field.code}.link"), field.value.url
+  - if field.options["display_inline"]
+    - version = field.options["display_inline"].is_a?(String) && field.options["display_inline"]
+    = link_to field.value.url do
+      = image_tag version ? field.value.send(version).url : field.value.url, class: "custom_field_attachment"
+  - else
+    - label = field.options["label"] || I18n.t("custom_fields.#{field.owner.class.name.demodulize.underscore}.#{field.code}.link")
+    = link_to label, field.value.url
 - else
   = "-"
diff --git a/spec/models/custom_field_spec.rb b/spec/models/custom_field_spec.rb
index 0c2644499..4c8ec2910 100644
--- a/spec/models/custom_field_spec.rb
+++ b/spec/models/custom_field_spec.rb
@@ -52,18 +52,11 @@ RSpec.describe CustomField, type: :model do
       let(:ref2){ create :workbench_referential }
       before do
         create :custom_field, field_type: :integer, code: :ref1_energy, name: :energy, workgroup: ref1.workgroup, options: {default: 12}
-        class CustomField
-          enumerize :field_type, in: %i{list integer string attachment float_test}
-          class Instance
-            class FloatTest < Integer
-              def preprocess_value_for_assignment val
-                val&.to_f
-              end
-            end
-          end
-        end
-        create :custom_field, field_type: :float_test, code: :ref1_energy, name: :energy, workgroup: ref1.workgroup, options: {default: 12}, resource_type: "Company"
+
+        create :custom_field, field_type: :float, code: :ref1_energy, name: :energy, workgroup: ref1.workgroup, options: {default: 12}, resource_type: "Company"
         create :custom_field, field_type: :integer, code: :ref2_energy, name: :energy, workgroup: ref2.workgroup
+        expect_any_instance_of(CustomField::Instance::Float).to receive(:preprocess_value_for_assignment) {|_, v| v.to_f }
+
       end
       it "should only initialize fields from the right workgroup" do
         ref1.switch
@@ -81,17 +74,22 @@ RSpec.describe CustomField, type: :model do
   end
 
   context "with a 'list' field_type" do
-    let!(:field){ [create(:custom_field, code: :energy, field_type: 'list', options: {list_values: %w(foo bar baz)}, workgroup: workgroup)] }
+    let!(:field){ [create(:custom_field, code: :energy, field_type: 'list', options: {list_values: %w(foo bar baz), default: 1}, workgroup: workgroup)] }
     let!( :vj ){ create :vehicle_journey, custom_field_values: {energy: "1"} }
     it "should cast the value" do
       expect(vj.custom_fields[:energy].value).to eq 1
       expect(vj.custom_fields[:energy].display_value).to eq "bar"
     end
 
-    it "should not break initailizartion if the model does not have the :custom_field_values attribute" do
+    it "should not break initialization if the model does not have the :custom_field_values attribute" do
       expect{Chouette::VehicleJourney.where(id: vj.id).select(:id).last}.to_not raise_error
     end
 
+    it "should use the default value" do
+      vj = Chouette::VehicleJourney.new
+      expect(vj.custom_fields[:energy].value).to eq 1
+    end
+
     it "should validate the value" do
       {
         "1" => true,
@@ -105,7 +103,37 @@ RSpec.describe CustomField, type: :model do
           expect(vj.validate).to be_truthy
         else
           expect(vj.validate).to be_falsy
-          expect(vj.errors.messages[:"custom_fields.energy"]).to be_present
+          expect(vj.errors.messages[:"custom_field_energy"]).to be_present
+        end
+      end
+    end
+
+    context 'with the values defined in a Hash' do
+      let!(:field){ [create(:custom_field, code: :energy, field_type: 'list', options: {list_values:{"1" => "foo", "2" => "bar", "3" => "BAZ"}}, workgroup: workgroup)] }
+      it "should cast the value" do
+        expect(vj.custom_fields[:energy].value).to eq 1
+        expect(vj.custom_fields[:energy].display_value).to eq "foo"
+      end
+
+      it "should not break initialization if the model does not have the :custom_field_values attribute" do
+        expect{Chouette::VehicleJourney.where(id: vj.id).select(:id).last}.to_not raise_error
+      end
+
+      it "should validate the value" do
+        {
+          "1" => true,
+          1 => true,
+          "azerty" => false,
+          "10" => false,
+          10 => false
+        }.each do |val, valid|
+          vj = build :vehicle_journey, custom_field_values: {energy: val}
+          if valid
+            expect(vj.validate).to be_truthy
+          else
+            expect(vj.validate).to be_falsy
+            expect(vj.errors.messages[:"custom_field_energy"]).to be_present
+          end
         end
       end
     end
@@ -122,6 +150,44 @@ RSpec.describe CustomField, type: :model do
       {
         99 => true,
         "99" => true,
+        "-99" => true,
+        -99 => true,
+        99.1 => false,
+        "99.1" => false,
+        "-99.1" => false,
+        -99.1 => false,
+        "azerty" => false,
+        "91a" => false,
+        "a91" => false
+      }.each do |val, valid|
+        vj = build :vehicle_journey, custom_field_values: {energy: val}
+        if valid
+          expect(vj.validate).to be_truthy
+        else
+          expect(vj.validate).to be_falsy, "#{val} should not ba a valid value"
+          expect(vj.errors.messages[:"custom_field_energy"]).to be_present
+        end
+      end
+    end
+  end
+
+  context "with a 'float' field_type" do
+    let!(:field){ [create(:custom_field, code: :energy, field_type: 'float', workgroup: workgroup)] }
+    let!( :vj ){ create :vehicle_journey, custom_field_values: {energy: "99"} }
+    it "should cast the value" do
+      expect(vj.custom_fields[:energy].value).to eq 99.0
+    end
+
+    it "should validate the value" do
+      {
+        99 => true,
+        "99" => true,
+        "-99" => true,
+        -99 => true,
+        99.1 => true,
+        "99.1" => true,
+        "-99.1" => true,
+        -99.1 => true,
         "azerty" => false,
         "91a" => false,
         "a91" => false
@@ -131,7 +197,7 @@ RSpec.describe CustomField, type: :model do
           expect(vj.validate).to be_truthy
         else
           expect(vj.validate).to be_falsy, "#{val} should not ba a valid value"
-          expect(vj.errors.messages[:"custom_fields.energy"]).to be_present
+          expect(vj.errors.messages[:"custom_field_energy"]).to be_present
         end
       end
     end