Fix: Mongoid abstract model parsed values

Author: dalpo

lib/rails_admin/abstract_model.rb

@@ -111,6 +111,10 @@ def initialize_mongoid
       extend Adapters::Mongoid
     end
 
+    def parse_field_value(field, value)
+      value.is_a?(Array) ? value.map { |v| field.parse_value(v) } : field.parse_value(value)
+    end
+
     class StatementBuilder
       def initialize(column, type, value, operator)
         @column = column
@@ -121,7 +125,6 @@ def initialize(column, type, value, operator)
 
       def to_statement
         return if [@operator, @value].any? { |v| v == '_discard' }
-
         unary_operators[@operator] || unary_operators[@value] ||
           build_statement_for_type_generic
       end
@@ -169,15 +172,15 @@ def build_statement_for_integer_decimal_or_float
 
       def build_statement_for_date
         start_date, end_date = get_filtering_duration
-        start_date = start_date.to_date if start_date
-        end_date = end_date.to_date if end_date
+        start_date = (start_date.to_date rescue nil) if start_date
+        end_date = (end_date.to_date rescue nil) if end_date
         range_filter(start_date, end_date)
       end
 
       def build_statement_for_datetime_or_timestamp
         start_date, end_date = get_filtering_duration
-        start_date = start_date.to_time.beginning_of_day if start_date
-        end_date = end_date.to_time.end_of_day if end_date
+        start_date = start_date.to_time.try(:beginning_of_day) if start_date
+        end_date = end_date.to_time.try(:end_of_day) if end_date
         range_filter(start_date, end_date)
       end
 

lib/rails_admin/adapters/active_record.rb

@@ -131,11 +131,8 @@ def filter_scope(scope, filters, fields = config.list.fields.select(&:filterable
           filters_dump.each do |_, filter_dump|
             wb = WhereBuilder.new(scope)
             field = fields.detect { |f| f.name.to_s == field_name }
-            if filter_dump[:v].is_a?(Array)
-              value = filter_dump[:v].map { |v| field.parse_value(v) }
-            else
-              value = field.parse_value(filter_dump[:v])
-            end
+            value = parse_field_value(field, filter_dump[:v])
+
             wb.add(field, value, (filter_dump[:o] || 'default'))
             # AND current filter statements to other filter statements
             scope = wb.build

lib/rails_admin/adapters/mongoid.rb

@@ -8,7 +8,6 @@ module RailsAdmin
   module Adapters
     module Mongoid
       DISABLED_COLUMN_TYPES = ['Range', 'Moped::BSON::Binary', 'BSON::Binary', 'Mongoid::Geospatial::Point']
-      ObjectId = defined?(Moped::BSON) ? Moped::BSON::ObjectId : BSON::ObjectId # rubocop:disable ConstantName
 
       def new(params = {})
         AbstractObject.new(model.new(params))
@@ -67,9 +66,7 @@ def associations
 
       def properties
         fields = model.fields.reject { |_name, field| DISABLED_COLUMN_TYPES.include?(field.type.to_s) }
-        fields.collect do |_name, field|
-          Property.new(field, model)
-        end
+        fields.collect { |_name, field| Property.new(field, model) }
       end
 
       def table_name
@@ -102,6 +99,7 @@ def make_field_conditions(field, value, operator)
         conditions_per_collection = {}
         field.searchable_columns.each do |column_infos|
           collection_name, column_name = parse_collection_name(column_infos[:column])
+          value = parse_field_value(field, value)
           statement = build_statement(column_name, column_infos[:type], value, operator)
           next unless statement
           conditions_per_collection[collection_name] ||= []
@@ -114,7 +112,8 @@ def query_conditions(query, fields = config.list.fields.select(&:queryable?))
         statements = []
 
         fields.each do |field|
-          conditions_per_collection = make_field_conditions(field, query, field.search_operator)
+          value = parse_field_value(field, query)
+          conditions_per_collection = make_field_conditions(field, value, field.search_operator)
           statements.concat make_condition_for_current_collection(field, conditions_per_collection)
         end
 
@@ -134,7 +133,8 @@ def filter_conditions(filters, fields = config.list.fields.select(&:filterable?)
           filters_dump.each do |_, filter_dump|
             field = fields.detect { |f| f.name.to_s == field_name }
             next unless field
-            conditions_per_collection = make_field_conditions(field, filter_dump[:v], (filter_dump[:o] || 'default'))
+            value = parse_field_value(field, filter_dump[:v])
+            conditions_per_collection = make_field_conditions(field, value, (filter_dump[:o] || 'default'))
             field_statements = make_condition_for_current_collection(field, conditions_per_collection)
             if field_statements.many?
               statements << {'$or' => field_statements}
@@ -265,8 +265,7 @@ def build_statement_for_enum
         end
 
         def build_statement_for_belongs_to_association_or_bson_object_id
-          object_id = (object_id_from_string(@value) rescue nil)
-          {@column => object_id} if object_id
+          {@column => @value} if @value
         end
 
         def range_filter(min, max)
@@ -278,10 +277,6 @@ def range_filter(min, max)
             {@column => {'$lte' => max}}
           end
         end
-
-        def object_id_from_string(str)
-          ObjectId.from_string(str)
-        end
       end
     end
   end

lib/rails_admin/config/fields/types/bson_object_id.rb

@@ -1,4 +1,5 @@
 require 'rails_admin/config/fields/types/string'
+require 'moped'
 
 module RailsAdmin
   module Config
@@ -7,6 +8,7 @@ module Types
         class BsonObjectId < RailsAdmin::Config::Fields::Types::String
           # Register field type for the type loader
           RailsAdmin::Config::Fields::Types.register(self)
+          OBJECT_ID = defined?(Moped::BSON) ? Moped::BSON::ObjectId : BSON::ObjectId
 
           register_instance_option :label do
             label = ((@label ||= {})[::I18n.locale] ||= abstract_model.model.human_attribute_name name)
@@ -27,7 +29,9 @@ def generic_help
           end
 
           def parse_value(value)
-            value.present? ? abstract_model.object_id_from_string(value) : nil
+            value.present? ? OBJECT_ID.from_string(value) : nil
+          rescue BSON::ObjectId::Invalid
+            nil
           rescue => e
             unless ['BSON::InvalidObjectId', 'Moped::Errors::InvalidObjectId'].include? e.class.to_s
               raise e

lib/rails_admin/config/fields/types/time.rb

@@ -7,6 +7,10 @@ module Types
         class Time < RailsAdmin::Config::Fields::Types::Datetime
           RailsAdmin::Config::Fields::Types.register(self)
 
+          def parse_value(value)
+            super(value).try(:utc)
+          end
+
           register_instance_option :strftime_format do
             '%H:%M'
           end

spec/dummy_app/app/mongoid/field_test.rb

@@ -12,7 +12,7 @@ class FieldTest
   field :array_field, type: Array
   field :big_decimal_field, type: BigDecimal
   field :boolean_field, type: Boolean
-  field :bson_object_id_field, type: RailsAdmin::Adapters::Mongoid::ObjectId
+  field :bson_object_id_field, type: RailsAdmin::Config::Fields::Types::BsonObjectId::OBJECT_ID
   field :bson_binary_field, type: BSON::Binary
   field :date_field, type: Date
   field :datetime_field, type: DateTime

spec/rails_admin/adapters/mongoid_spec.rb

@@ -348,10 +348,10 @@
     end
 
     it 'supports datetime type query' do
-      expect(@abstract_model.send(:filter_conditions, 'datetime_field' => {'1' => {v: ['', 'January 02, 2012', 'January 03, 2012'], o: 'between'}})).to eq('$and' => [{'datetime_field' => {'$gte' => Time.local(2012, 1, 2), '$lte' => Time.local(2012, 1, 3).end_of_day}}])
-      expect(@abstract_model.send(:filter_conditions, 'datetime_field' => {'1' => {v: ['', 'January 03, 2012', ''], o: 'between'}})).to eq('$and' => [{'datetime_field' => {'$gte' => Time.local(2012, 1, 3)}}])
-      expect(@abstract_model.send(:filter_conditions, 'datetime_field' => {'1' => {v: ['', '', 'January 02, 2012'], o: 'between'}})).to eq('$and' => [{'datetime_field' => {'$lte' => Time.local(2012, 1, 2).end_of_day}}])
-      expect(@abstract_model.send(:filter_conditions, 'datetime_field' => {'1' => {v: ['January 02, 2012'], o: 'default'}})).to eq('$and' => [{'datetime_field' => {'$gte' => Time.local(2012, 1, 2), '$lte' => Time.local(2012, 1, 2).end_of_day}}])
+      expect(@abstract_model.send(:filter_conditions, 'datetime_field' => {'1' => {v: ['', 'January 02, 2012 00:00', 'January 03, 2012 00:00'], o: 'between'}})).to eq('$and' => [{'datetime_field' => {'$gte' => Time.local(2012, 1, 2), '$lte' => Time.local(2012, 1, 3).end_of_day}}])
+      expect(@abstract_model.send(:filter_conditions, 'datetime_field' => {'1' => {v: ['', 'January 03, 2012 00:00', ''], o: 'between'}})).to eq('$and' => [{'datetime_field' => {'$gte' => Time.local(2012, 1, 3)}}])
+      expect(@abstract_model.send(:filter_conditions, 'datetime_field' => {'1' => {v: ['', '', 'January 02, 2012 00:00'], o: 'between'}})).to eq('$and' => [{'datetime_field' => {'$lte' => Time.local(2012, 1, 2).end_of_day}}])
+      expect(@abstract_model.send(:filter_conditions, 'datetime_field' => {'1' => {v: ['January 02, 2012 00:00'], o: 'default'}})).to eq('$and' => [{'datetime_field' => {'$gte' => Time.local(2012, 1, 2), '$lte' => Time.local(2012, 1, 2).end_of_day}}])
     end
 
     it 'supports enum type query' do

spec/rails_admin/config/fields/types/time_spec.rb

@@ -4,7 +4,17 @@
   it_behaves_like 'a generic field type', :time_field, :time
 
   describe '#parse_input' do
-    let(:field) { RailsAdmin.config(FieldTest).fields.detect { |f| f.name == :time_field } }
+    let(:field) do
+      RailsAdmin.config(FieldTest).fields.detect do |f|
+        f.name == :time_field
+      end
+    end
+
+    before do
+      RailsAdmin.config(FieldTest) do
+        field :time_field, :time
+      end
+    end
 
     before :each do
       @object = FactoryGirl.create(:field_test)
@@ -23,18 +33,28 @@
     it 'interprets time value as UTC when timezone is specified' do
       Time.zone = 'Eastern Time (US & Canada)' # -05:00
       @object.time_field = field.parse_input(time_field: @time.strftime('%H:%M'))
-      expect(@object.time_field.strftime('%H:%M')).to eq(@time.strftime('%H:%M'))
+      expect(@object.time_field.utc.strftime('%H:%M')).to eq(@time.strftime('%H:%M'))
     end
 
-    it 'has a customization option' do
-      RailsAdmin.config FieldTest do
-        field :time_field do
-          strftime_format '%I:%M %p'
+    context 'with a custom strftime_format' do
+      let(:field) do
+        RailsAdmin.config(FieldTest).fields.detect do |f|
+          f.name == :time_field
         end
       end
 
-      @object.time_field = field.parse_input(time_field: @time.strftime('%I:%M %p'))
-      expect(@object.time_field.strftime('%H:%M')).to eq(@time.strftime('%H:%M'))
+      before do
+        RailsAdmin.config(FieldTest) do
+          field :time_field, :time do
+            strftime_format '%I:%M %p'
+          end
+        end
+      end
+
+      it 'has a customization option' do
+        @object.time_field = field.parse_input(time_field: @time.strftime('%I:%M %p'))
+        expect(@object.time_field.strftime('%H:%M')).to eq(@time.strftime('%H:%M'))
+      end
     end
   end
 end

spec/spec_helper.rb

@@ -65,7 +65,10 @@ def password_digest(password)
 
   config.include Capybara::DSL, type: :request
 
-  config.before do
+  config.before do |example|
+    DatabaseCleaner.strategy = (CI_ORM == :mongoid || example.metadata[:js]) ? :truncation : :transaction
+    DatabaseCleaner.start
+
     RailsAdmin::Config.reset
     RailsAdmin::AbstractModel.reset
     RailsAdmin::Config.audit_with(:history) if CI_ORM == :active_record
@@ -74,6 +77,7 @@ def password_digest(password)
 
   config.after(:each) do
     Warden.test_reset!
+    DatabaseCleaner.clean
   end
 
   CI_TARGET_ORMS.each do |orm|
@@ -83,20 +87,4 @@ def password_digest(password)
       config.filter_run_excluding orm => true
     end
   end
-
-  # Configure Database Cleaner
-  config.use_transactional_fixtures = false
-
-  config.before(:suite) do
-    DatabaseCleaner.clean_with(:truncation)
-  end
-
-  config.before(:each) do |example|
-    DatabaseCleaner.strategy = (CI_ORM == :mongoid || example.metadata[:js]) ? :truncation : :transaction
-    DatabaseCleaner.start
-  end
-
-  config.after(:each) do
-    DatabaseCleaner.clean
-  end
 end