From 6fe5b3c071507e6751103a9b31fd1760b4c6996a Mon Sep 17 00:00:00 2001 From: Andrea Dal Ponte Date: Fri, 4 Sep 2015 09:54:56 +0200 Subject: [PATCH] Fix: Mongoid abstract model parsed values --- lib/rails_admin/abstract_model.rb | 13 ++++--- lib/rails_admin/adapters/active_record.rb | 7 ++-- lib/rails_admin/adapters/mongoid.rb | 19 ++++------ .../config/fields/types/bson_object_id.rb | 6 +++- lib/rails_admin/config/fields/types/time.rb | 4 +++ spec/dummy_app/app/mongoid/field_test.rb | 2 +- spec/rails_admin/adapters/mongoid_spec.rb | 8 ++--- .../config/fields/types/time_spec.rb | 36 ++++++++++++++----- 8 files changed, 59 insertions(+), 36 deletions(-) diff --git a/lib/rails_admin/abstract_model.rb b/lib/rails_admin/abstract_model.rb index bb920ba6e5..e1585a4bbf 100644 --- a/lib/rails_admin/abstract_model.rb +++ b/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 diff --git a/lib/rails_admin/adapters/active_record.rb b/lib/rails_admin/adapters/active_record.rb index 56c9d7f5c0..11800bdd86 100644 --- a/lib/rails_admin/adapters/active_record.rb +++ b/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 diff --git a/lib/rails_admin/adapters/mongoid.rb b/lib/rails_admin/adapters/mongoid.rb index d40e3b6b29..be5adc16e2 100644 --- a/lib/rails_admin/adapters/mongoid.rb +++ b/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 diff --git a/lib/rails_admin/config/fields/types/bson_object_id.rb b/lib/rails_admin/config/fields/types/bson_object_id.rb index 171ae0534d..fee8f7d306 100644 --- a/lib/rails_admin/config/fields/types/bson_object_id.rb +++ b/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 diff --git a/lib/rails_admin/config/fields/types/time.rb b/lib/rails_admin/config/fields/types/time.rb index 88ace022d5..51322e0f45 100644 --- a/lib/rails_admin/config/fields/types/time.rb +++ b/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 diff --git a/spec/dummy_app/app/mongoid/field_test.rb b/spec/dummy_app/app/mongoid/field_test.rb index 34ff7eb097..6b22d39809 100644 --- a/spec/dummy_app/app/mongoid/field_test.rb +++ b/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 diff --git a/spec/rails_admin/adapters/mongoid_spec.rb b/spec/rails_admin/adapters/mongoid_spec.rb index 14a8d4aaf7..287c0ae3b4 100644 --- a/spec/rails_admin/adapters/mongoid_spec.rb +++ b/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 diff --git a/spec/rails_admin/config/fields/types/time_spec.rb b/spec/rails_admin/config/fields/types/time_spec.rb index cfbfb22206..0b7d02814b 100644 --- a/spec/rails_admin/config/fields/types/time_spec.rb +++ b/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