From 5422bd086b12b558f8488a885d9d3cd90d18721c Mon Sep 17 00:00:00 2001 From: Max Shytikov Date: Fri, 29 Dec 2017 23:23:20 +0000 Subject: [PATCH 1/6] Added enum tests (and fileds to field_test model) --- .../dummy_app/app/active_record/field_test.rb | 3 ++ ...1229220713_add_size_enum_to_field_tests.rb | 6 +++ spec/rails_admin/abstract_model_spec.rb | 41 +++++++++++++++++++ 3 files changed, 50 insertions(+) create mode 100644 spec/dummy_app/db/migrate/20171229220713_add_size_enum_to_field_tests.rb diff --git a/spec/dummy_app/app/active_record/field_test.rb b/spec/dummy_app/app/active_record/field_test.rb index f75e3dc3c7..83822fac26 100644 --- a/spec/dummy_app/app/active_record/field_test.rb +++ b/spec/dummy_app/app/active_record/field_test.rb @@ -13,4 +13,7 @@ class FieldTest < ActiveRecord::Base mount_uploader :carrierwave_asset, CarrierwaveUploader attachment :refile_asset if defined?(Refile) + + enum size_string_enum: {S: 's', M: 'm', L: 'l'} + enum size_integer_enum: [:small, :medium, :large] end diff --git a/spec/dummy_app/db/migrate/20171229220713_add_size_enum_to_field_tests.rb b/spec/dummy_app/db/migrate/20171229220713_add_size_enum_to_field_tests.rb new file mode 100644 index 0000000000..143a7e6c7a --- /dev/null +++ b/spec/dummy_app/db/migrate/20171229220713_add_size_enum_to_field_tests.rb @@ -0,0 +1,6 @@ +class AddSizeEnumToFieldTests < MigrationBase + def change + add_column :field_tests, :size_string_enum, :string + add_column :field_tests, :size_integer_enum, :integer + end +end diff --git a/spec/rails_admin/abstract_model_spec.rb b/spec/rails_admin/abstract_model_spec.rb index 6afe572c9d..d494f5ebf6 100644 --- a/spec/rails_admin/abstract_model_spec.rb +++ b/spec/rails_admin/abstract_model_spec.rb @@ -18,6 +18,47 @@ end end + context 'on enum' do + + shared_examples "filter on enum" do + before do + ["S", "M", "L"].each do |size| + FactoryGirl.create(:field_test, size_string_enum: size) + end + + ["small", "medium", "large"].each do |size| + FactoryGirl.create(:field_test, size_integer_enum: size) + end + end + let(:model) { RailsAdmin::AbstractModel.new('FieldTest') } + let(:filters) { {enum_field => {'1' => {v: filter_value, o: 'is'}}} } + subject(:elements) { model.all(filters: filters) } + + it 'lists elements by value' do + expect(elements.count).to eq(expected_elements_count) + expect(elements.map(&enum_field.to_sym)).to all(eq(enum_label)) + end + end + + context "when enum is integer enum" do + it_behaves_like "filter on enum" do + let(:filter_value) { 0 } + let(:enum_field) { "size_integer_enum" } + let(:enum_label) { 'small' } + let(:expected_elements_count) { 1 } + end + end + + context "when enum is string enum where label <> value" do + it_behaves_like "filter on enum" do + let(:filter_value) { 's' } + let(:enum_field) { "size_string_enum" } + let(:enum_label) { 'S' } + let(:expected_elements_count) { 1 } + end + end + end + context 'on dates with :en locale' do before do [Date.new(2012, 1, 1), Date.new(2012, 1, 2), Date.new(2012, 1, 3), Date.new(2012, 1, 4)].each do |date| From d89eb9ea35bae63eee5b384cab84a9f5f3eae0c2 Mon Sep 17 00:00:00 2001 From: Max Shytikov Date: Sun, 24 Dec 2017 01:35:04 +0000 Subject: [PATCH 2/6] Fixed filter by enum filter when label <> value Refactored code to remove unnecessary calls to `parse_value`. Which were affecting filter by enum (rails >= 5.0): If enum defined like ``` enum size: { L: 'l', S: 's' } ``` Due to multiple calls, the chain when filtering by size is following: ``` parse_value("l") => "L" parse_value("L") => null ``` Which leads to non working filtering. NOTE: `parse_value` returns correct result in both cases above --- lib/rails_admin/adapters/active_record.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/rails_admin/adapters/active_record.rb b/lib/rails_admin/adapters/active_record.rb index 00fea84f7b..b416722ca5 100644 --- a/lib/rails_admin/adapters/active_record.rb +++ b/lib/rails_admin/adapters/active_record.rb @@ -101,12 +101,6 @@ def initialize(scope) def add(field, value, operator) field.searchable_columns.flatten.each do |column_infos| - value = - if value.is_a?(Array) - value.map { |v| field.parse_value(v) } - else - field.parse_value(value) - end statement, value1, value2 = StatementBuilder.new(column_infos[:column], column_infos[:type], value, operator).to_statement @statements << statement if statement.present? @values << value1 unless value1.nil? @@ -126,7 +120,8 @@ def build def query_scope(scope, query, fields = config.list.fields.select(&:queryable?)) wb = WhereBuilder.new(scope) fields.each do |field| - wb.add(field, field.parse_value(query), field.search_operator) + value = parse_field_value(field, query) + wb.add(field, value, field.search_operator) end # OR all query statements wb.build From f25260309c44a1a241532e36bff8f263cad51797 Mon Sep 17 00:00:00 2001 From: Max Shytikov Date: Sat, 30 Dec 2017 01:09:26 +0000 Subject: [PATCH 3/6] [rubocop] --- spec/rails_admin/abstract_model_spec.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/spec/rails_admin/abstract_model_spec.rb b/spec/rails_admin/abstract_model_spec.rb index d494f5ebf6..edc73fb7a8 100644 --- a/spec/rails_admin/abstract_model_spec.rb +++ b/spec/rails_admin/abstract_model_spec.rb @@ -19,17 +19,16 @@ end context 'on enum' do - shared_examples "filter on enum" do - before do - ["S", "M", "L"].each do |size| - FactoryGirl.create(:field_test, size_string_enum: size) + before do + ["S", "M", "L"].each do |size| + FactoryGirl.create(:field_test, size_string_enum: size) + end + + ["small", "medium", "large"].each do |size| + FactoryGirl.create(:field_test, size_integer_enum: size) + end end - - ["small", "medium", "large"].each do |size| - FactoryGirl.create(:field_test, size_integer_enum: size) - end - end let(:model) { RailsAdmin::AbstractModel.new('FieldTest') } let(:filters) { {enum_field => {'1' => {v: filter_value, o: 'is'}}} } subject(:elements) { model.all(filters: filters) } From 9f888b27c45558721762663a433162da03f3a608 Mon Sep 17 00:00:00 2001 From: Max Shytikov Date: Sun, 14 Jan 2018 21:45:11 +0000 Subject: [PATCH 4/6] Applied fix from PR #2952 --- .../config/fields/types/active_record_enum.rb | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/rails_admin/config/fields/types/active_record_enum.rb b/lib/rails_admin/config/fields/types/active_record_enum.rb index e6aea3f4a0..80041c7588 100644 --- a/lib/rails_admin/config/fields/types/active_record_enum.rb +++ b/lib/rails_admin/config/fields/types/active_record_enum.rb @@ -30,14 +30,20 @@ def type def parse_value(value) return unless value.present? if ::Rails.version >= '5' - abstract_model.model.attribute_types[name.to_s].deserialize(value) + abstract_model.model.attribute_types[name.to_s].serialize(value) else - enum.invert[type_cast_value(value)] + # Depending on the colum type and AR version, we might get a + # string or an integer, so we need to handle both cases. + enum.fetch(value) do + type_cast_value(value) + end end end def parse_input(params) - params[name] = parse_value(params[name]) if params[name] + value = params[name] + return unless value + params[name] = parse_input_value(value) end def form_value @@ -46,6 +52,14 @@ def form_value private + def parse_input_value(value) + if ::Rails.version >= '5' + abstract_model.model.attribute_types[name.to_s].deserialize(value) + else + enum.invert[type_cast_value(value)] + end + end + def type_cast_value(value) if ::Rails.version >= '4.2' abstract_model.model.column_types[name.to_s].type_cast_from_user(value) From 1720e4900d36e8554da647f02836b462576b122d Mon Sep 17 00:00:00 2001 From: Max Shytikov Date: Sun, 14 Jan 2018 23:24:12 +0000 Subject: [PATCH 5/6] Test only for ActiveRecord native enums --- spec/dummy_app/app/active_record/field_test.rb | 6 ++++-- spec/rails_admin/abstract_model_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/dummy_app/app/active_record/field_test.rb b/spec/dummy_app/app/active_record/field_test.rb index 83822fac26..b64990fde0 100644 --- a/spec/dummy_app/app/active_record/field_test.rb +++ b/spec/dummy_app/app/active_record/field_test.rb @@ -14,6 +14,8 @@ class FieldTest < ActiveRecord::Base attachment :refile_asset if defined?(Refile) - enum size_string_enum: {S: 's', M: 'm', L: 'l'} - enum size_integer_enum: [:small, :medium, :large] + if ::Rails.version >= '4.1' # enum support was added in Rails 4.1 + enum size_string_enum: {S: 's', M: 'm', L: 'l'} + enum size_integer_enum: [:small, :medium, :large] + end end diff --git a/spec/rails_admin/abstract_model_spec.rb b/spec/rails_admin/abstract_model_spec.rb index edc73fb7a8..bbc4d9741e 100644 --- a/spec/rails_admin/abstract_model_spec.rb +++ b/spec/rails_admin/abstract_model_spec.rb @@ -18,7 +18,7 @@ end end - context 'on enum' do + context 'on ActiveRecord native enum' do shared_examples "filter on enum" do before do ["S", "M", "L"].each do |size| @@ -56,7 +56,7 @@ let(:expected_elements_count) { 1 } end end - end + end if CI_ORM == :active_record && ::Rails.version >= '4.1' context 'on dates with :en locale' do before do From b5ccb7854697e0696a30f4be0076359d7cb5c231 Mon Sep 17 00:00:00 2001 From: Max Shytikov Date: Tue, 16 Jan 2018 22:18:59 +0000 Subject: [PATCH 6/6] Fix pg versions to ~> 0.21. Rails does not support yet pg v1.0.0 more details here: https://github.com/rails/rails/issues/31669 --- Gemfile | 2 +- gemfiles/rails_4.0.gemfile | 2 +- gemfiles/rails_4.1.gemfile | 2 +- gemfiles/rails_4.2.gemfile | 2 +- gemfiles/rails_5.0.gemfile | 2 +- gemfiles/rails_5.1.gemfile | 2 +- spec/dummy_app/Gemfile | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Gemfile b/Gemfile index 01a571214a..3bb01a5f68 100644 --- a/Gemfile +++ b/Gemfile @@ -8,7 +8,7 @@ gem 'devise' group :active_record do platforms :ruby, :mswin, :mingw do gem 'mysql2', '~> 0.3.14' - gem 'pg', '>= 0.14' + gem 'pg', "~> 0.21" gem 'sqlite3', '>= 1.3' end end diff --git a/gemfiles/rails_4.0.gemfile b/gemfiles/rails_4.0.gemfile index a8c529331e..b263f60e73 100644 --- a/gemfiles/rails_4.0.gemfile +++ b/gemfiles/rails_4.0.gemfile @@ -16,7 +16,7 @@ group :active_record do platforms :ruby, :mswin, :mingw do gem "mysql2", "~> 0.3.14" - gem "pg", ">= 0.14" + gem "pg", "~> 0.21" gem "sqlite3", ">= 1.3" end diff --git a/gemfiles/rails_4.1.gemfile b/gemfiles/rails_4.1.gemfile index ed59e99723..54dd8b47f9 100644 --- a/gemfiles/rails_4.1.gemfile +++ b/gemfiles/rails_4.1.gemfile @@ -13,7 +13,7 @@ group :active_record do platforms :ruby, :mswin, :mingw do gem "mysql2", "~> 0.3.14" - gem "pg", ">= 0.14" + gem "pg", "~> 0.21" gem "sqlite3", ">= 1.3" end diff --git a/gemfiles/rails_4.2.gemfile b/gemfiles/rails_4.2.gemfile index 44b33f3ef7..39f22d7269 100644 --- a/gemfiles/rails_4.2.gemfile +++ b/gemfiles/rails_4.2.gemfile @@ -14,7 +14,7 @@ group :active_record do platforms :ruby, :mswin, :mingw do gem "mysql2", "~> 0.3.14" - gem "pg", ">= 0.14" + gem "pg", "~> 0.21" gem "sqlite3", ">= 1.3" end diff --git a/gemfiles/rails_5.0.gemfile b/gemfiles/rails_5.0.gemfile index f917b4ea93..ebe2e84eba 100644 --- a/gemfiles/rails_5.0.gemfile +++ b/gemfiles/rails_5.0.gemfile @@ -13,7 +13,7 @@ group :active_record do platforms :ruby, :mswin, :mingw do gem "mysql2", "~> 0.3.14" - gem "pg", ">= 0.14" + gem 'pg', '~> 0.21' gem "sqlite3", ">= 1.3" end diff --git a/gemfiles/rails_5.1.gemfile b/gemfiles/rails_5.1.gemfile index 349c44fc6b..ff5fb64dc2 100644 --- a/gemfiles/rails_5.1.gemfile +++ b/gemfiles/rails_5.1.gemfile @@ -13,7 +13,7 @@ group :active_record do platforms :ruby, :mswin, :mingw do gem "mysql2", "~> 0.3.14" - gem "pg", ">= 0.14" + gem 'pg', '~> 0.21' gem "sqlite3", ">= 1.3" end end diff --git a/spec/dummy_app/Gemfile b/spec/dummy_app/Gemfile index e13b41c3a5..c5bd445f6d 100644 --- a/spec/dummy_app/Gemfile +++ b/spec/dummy_app/Gemfile @@ -15,7 +15,7 @@ group :active_record do platforms :ruby, :mswin, :mingw do gem 'mysql2', '~> 0.3.14' - gem 'pg', '>= 0.14' + gem 'pg', '~> 0.21' gem 'sqlite3', '>= 1.3' end