From 6bb97eff05b5b021b673a295948ef426f626d234 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 11:25:17 +1000 Subject: [PATCH 01/32] Replace nested if/else with elsif --- lib/rails_admin/config/actions/index.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/rails_admin/config/actions/index.rb b/lib/rails_admin/config/actions/index.rb index 757ec13ab3..ed72392e21 100644 --- a/lib/rails_admin/config/actions/index.rb +++ b/lib/rails_admin/config/actions/index.rb @@ -76,12 +76,10 @@ class Index < RailsAdmin::Config::Actions::Base send_data output, type: "text/csv; charset=#{encoding}; #{'header=present' if header}", disposition: "attachment; filename=#{params[:model_name]}_#{DateTime.now.strftime('%Y-%m-%d_%Hh%Mm%S')}.csv" + elsif Rails.version.to_s >= '5' + render plain: output else - if Rails.version.to_s >= '5' - render plain: output - else - render text: output - end + render text: output end end end From 41b9e3b2e62861819287325d7362c45829341f89 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 11:37:41 +1000 Subject: [PATCH 02/32] Upgrade rubocop for consistency with Code Climate --- .rubocop.yml | 5 ++--- Gemfile | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index d6f9cbb716..eb36a4f94c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -76,7 +76,6 @@ Style/RaiseArgs: Style/SpaceInsideHashLiteralBraces: EnforcedStyle: no_space -Style/TrailingComma: - EnforcedStyleForMultiline: 'comma' - +Style/TrailingCommaInLiteral: + EnforcedStyleForMultiline: 'comma' diff --git a/Gemfile b/Gemfile index 58441bc7c9..9858984a37 100644 --- a/Gemfile +++ b/Gemfile @@ -43,7 +43,7 @@ group :test do gem 'pundit' gem 'rack-cache', require: 'rack/cache' gem 'rspec-rails', '>= 2.14' - gem 'rubocop', '~> 0.31.0' + gem 'rubocop', '~> 0.40.0' gem 'simplecov', '>= 0.9', require: false gem 'timecop', '>= 0.5' From 0f708f95eb35e3259fdd9c7eee83530b55005e2b Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 11:42:51 +1000 Subject: [PATCH 03/32] Replace fail with raise --- app/controllers/rails_admin/application_controller.rb | 6 +++--- app/helpers/rails_admin/main_helper.rb | 2 +- lib/rails_admin/abstract_model.rb | 6 +++--- lib/rails_admin/adapters/mongoid.rb | 2 +- lib/rails_admin/adapters/mongoid/association.rb | 4 ++-- lib/rails_admin/bootstrap-sass.rb | 2 +- lib/rails_admin/config.rb | 2 +- lib/rails_admin/config/actions.rb | 2 +- lib/rails_admin/config/configurable.rb | 2 +- lib/rails_admin/config/fields/types.rb | 2 +- lib/rails_admin/config/fields/types/file_upload.rb | 2 +- lib/rails_admin/config/fields/types/text.rb | 6 +++--- lib/rails_admin/extensions/paper_trail/auditing_adapter.rb | 2 +- lib/rails_admin/extensions/pundit/authorization_adapter.rb | 2 +- spec/controllers/rails_admin/main_controller_spec.rb | 2 +- 15 files changed, 22 insertions(+), 22 deletions(-) diff --git a/app/controllers/rails_admin/application_controller.rb b/app/controllers/rails_admin/application_controller.rb index 6a6712d4cc..9e2178eddb 100644 --- a/app/controllers/rails_admin/application_controller.rb +++ b/app/controllers/rails_admin/application_controller.rb @@ -21,13 +21,13 @@ class ApplicationController < Config.parent_controller.constantize def get_model @model_name = to_model_name(params[:model_name]) - fail(RailsAdmin::ModelNotFound) unless (@abstract_model = RailsAdmin::AbstractModel.new(@model_name)) - fail(RailsAdmin::ModelNotFound) if (@model_config = @abstract_model.config).excluded? + raise(RailsAdmin::ModelNotFound) unless (@abstract_model = RailsAdmin::AbstractModel.new(@model_name)) + raise(RailsAdmin::ModelNotFound) if (@model_config = @abstract_model.config).excluded? @properties = @abstract_model.properties end def get_object - fail(RailsAdmin::ObjectNotFound) unless (@object = @abstract_model.get(params[:id])) + raise(RailsAdmin::ObjectNotFound) unless (@object = @abstract_model.get(params[:id])) end def to_model_name(param) diff --git a/app/helpers/rails_admin/main_helper.rb b/app/helpers/rails_admin/main_helper.rb index 59816c7b86..1976f91510 100644 --- a/app/helpers/rails_admin/main_helper.rb +++ b/app/helpers/rails_admin/main_helper.rb @@ -63,7 +63,7 @@ def ordered_filter_string filter_name = filter_for_field.keys.first filter_hash = filter_for_field.values.first unless (field = filterable_fields.find { |f| f.name == filter_name.to_sym }) - fail "#{filter_name} is not currently filterable; filterable fields are #{filterable_fields.map(&:name).join(', ')}" + raise "#{filter_name} is not currently filterable; filterable fields are #{filterable_fields.map(&:name).join(', ')}" end case field.type when :enum diff --git a/lib/rails_admin/abstract_model.rb b/lib/rails_admin/abstract_model.rb index e1585a4bbf..2451c45cb0 100644 --- a/lib/rails_admin/abstract_model.rb +++ b/lib/rails_admin/abstract_model.rb @@ -147,7 +147,7 @@ def build_statement_for_type_generic end def build_statement_for_type - fail('You must override build_statement_for_type in your StatementBuilder') + raise('You must override build_statement_for_type in your StatementBuilder') end def build_statement_for_integer_decimal_or_float @@ -185,11 +185,11 @@ def build_statement_for_datetime_or_timestamp end def unary_operators - fail('You must override unary_operators in your StatementBuilder') + raise('You must override unary_operators in your StatementBuilder') end def range_filter(_min, _max) - fail('You must override range_filter in your StatementBuilder') + raise('You must override range_filter in your StatementBuilder') end class FilteringDuration diff --git a/lib/rails_admin/adapters/mongoid.rb b/lib/rails_admin/adapters/mongoid.rb index dee9147360..10e3c2ce67 100644 --- a/lib/rails_admin/adapters/mongoid.rb +++ b/lib/rails_admin/adapters/mongoid.rb @@ -190,7 +190,7 @@ def sort_by(options, scope) when String field_name, collection_name = options[:sort].split('.').reverse if collection_name && collection_name != table_name - fail('sorting by associated model column is not supported in Non-Relational databases') + raise('sorting by associated model column is not supported in Non-Relational databases') end when Symbol field_name = options[:sort].to_s diff --git a/lib/rails_admin/adapters/mongoid/association.rb b/lib/rails_admin/adapters/mongoid/association.rb index 8ae7532d9a..b74ab6eb2a 100644 --- a/lib/rails_admin/adapters/mongoid/association.rb +++ b/lib/rails_admin/adapters/mongoid/association.rb @@ -28,7 +28,7 @@ def type when :has_and_belongs_to_many, :references_and_referenced_in_many :has_and_belongs_to_many else - fail("Unknown association type: #{macro.inspect}") + raise("Unknown association type: #{macro.inspect}") end end @@ -83,7 +83,7 @@ def read_only? def nested_options nested = nested_attributes_options.try { |o| o[name] } if !nested && [:embeds_one, :embeds_many].include?(macro.to_sym) && !association.cyclic - fail <<-MSG.gsub(/^\s+/, '') + raise <<-MSG.gsub(/^\s+/, '') Embbeded association without accepts_nested_attributes_for can't be handled by RailsAdmin, because embedded model doesn't have top-level access. Please add `accepts_nested_attributes_for :#{association.name}' line to `#{model}' model. diff --git a/lib/rails_admin/bootstrap-sass.rb b/lib/rails_admin/bootstrap-sass.rb index 3aa90d9070..cf7499d745 100755 --- a/lib/rails_admin/bootstrap-sass.rb +++ b/lib/rails_admin/bootstrap-sass.rb @@ -14,7 +14,7 @@ def self.load! require 'sass-rails' if rails? unless rails? || compass? - fail(Bootstrap::FrameworkNotFound.new('bootstrap-sass requires either Rails > 3.1 or Compass, neither of which are loaded')) + raise(Bootstrap::FrameworkNotFound.new('bootstrap-sass requires either Rails > 3.1 or Compass, neither of which are loaded')) end stylesheets = File.expand_path(File.join('..', 'vendor', 'assets', 'stylesheets')) diff --git a/lib/rails_admin/config.rb b/lib/rails_admin/config.rb index c1dde25fa3..88d25898b2 100644 --- a/lib/rails_admin/config.rb +++ b/lib/rails_admin/config.rb @@ -193,7 +193,7 @@ def default_search_operator=(operator) if %w(default like starts_with ends_with is =).include? operator @default_search_operator = operator else - fail(ArgumentError.new("Search operator '#{operator}' not supported")) + raise(ArgumentError.new("Search operator '#{operator}' not supported")) end end diff --git a/lib/rails_admin/config/actions.rb b/lib/rails_admin/config/actions.rb index c4a77822ac..6c4f33d49a 100644 --- a/lib/rails_admin/config/actions.rb +++ b/lib/rails_admin/config/actions.rb @@ -96,7 +96,7 @@ def add_action_custom_key(action, &block) action.instance_eval(&block) if block @@actions ||= [] if action.custom_key.in?(@@actions.collect(&:custom_key)) - fail "Action #{action.custom_key} already exists. Please change its custom key." + raise "Action #{action.custom_key} already exists. Please change its custom key." else @@actions << action end diff --git a/lib/rails_admin/config/configurable.rb b/lib/rails_admin/config/configurable.rb index 2d5f06ac11..e7975962c4 100644 --- a/lib/rails_admin/config/configurable.rb +++ b/lib/rails_admin/config/configurable.rb @@ -79,7 +79,7 @@ def register_deprecated_instance_option(option_name, replacement_option_name = n if block_given? yield else - fail("The #{option_name} configuration option is removed without replacement.") + raise("The #{option_name} configuration option is removed without replacement.") end end end diff --git a/lib/rails_admin/config/fields/types.rb b/lib/rails_admin/config/fields/types.rb index 745506591e..03c3aa8633 100644 --- a/lib/rails_admin/config/fields/types.rb +++ b/lib/rails_admin/config/fields/types.rb @@ -9,7 +9,7 @@ module Types @@registry = {} def self.load(type) - @@registry[type.to_sym] || fail("Unsupported field datatype: #{type}") + @@registry[type.to_sym] || raise("Unsupported field datatype: #{type}") end def self.register(type, klass = nil) diff --git a/lib/rails_admin/config/fields/types/file_upload.rb b/lib/rails_admin/config/fields/types/file_upload.rb index 5f9f5b0b3f..3a01e2b389 100644 --- a/lib/rails_admin/config/fields/types/file_upload.rb +++ b/lib/rails_admin/config/fields/types/file_upload.rb @@ -57,7 +57,7 @@ class FileUpload < RailsAdmin::Config::Fields::Base # virtual class def resource_url - fail('not implemented') + raise('not implemented') end def virtual? diff --git a/lib/rails_admin/config/fields/types/text.rb b/lib/rails_admin/config/fields/types/text.rb index b4eb4ece32..e9c7948132 100644 --- a/lib/rails_admin/config/fields/types/text.rb +++ b/lib/rails_admin/config/fields/types/text.rb @@ -10,19 +10,19 @@ class Text < RailsAdmin::Config::Fields::Base [:ckeditor, :ckeditor_base_location, :ckeditor_config_js, :ckeditor_location].each do |key| register_deprecated_instance_option key do - fail("The 'field(:foo){ ckeditor true }' style DSL is deprecated. Please use 'field :foo, :ck_editor' instead.") + raise("The 'field(:foo){ ckeditor true }' style DSL is deprecated. Please use 'field :foo, :ck_editor' instead.") end end [:codemirror, :codemirror_assets, :codemirror_config, :codemirror_css_location, :codemirror_js_location].each do |key| register_deprecated_instance_option key do - fail("The 'field(:foo){ codemirror true }' style DSL is deprecated. Please use 'field :foo, :code_mirror' instead.") + raise("The 'field(:foo){ codemirror true }' style DSL is deprecated. Please use 'field :foo, :code_mirror' instead.") end end [:bootstrap_wysihtml5, :bootstrap_wysihtml5_config_options, :bootstrap_wysihtml5_css_location, :bootstrap_wysihtml5_js_location].each do |key| register_deprecated_instance_option key do - fail("The 'field(:foo){ bootstrap_wysihtml5 true }' style DSL is deprecated. Please use 'field :foo, :wysihtml5' instead.") + raise("The 'field(:foo){ bootstrap_wysihtml5 true }' style DSL is deprecated. Please use 'field :foo, :wysihtml5' instead.") end end diff --git a/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb b/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb index 648ea986d9..f051c8dd52 100644 --- a/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb +++ b/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb @@ -39,7 +39,7 @@ class AuditingAdapter } def initialize(controller, user_class = 'User', version_class = '::Version') - fail('PaperTrail not found') unless defined?(PaperTrail) + raise('PaperTrail not found') unless defined?(PaperTrail) @controller = controller @controller.send(:set_paper_trail_whodunnit) if @controller begin diff --git a/lib/rails_admin/extensions/pundit/authorization_adapter.rb b/lib/rails_admin/extensions/pundit/authorization_adapter.rb index 3598c131d7..ec8e7c8c41 100644 --- a/lib/rails_admin/extensions/pundit/authorization_adapter.rb +++ b/lib/rails_admin/extensions/pundit/authorization_adapter.rb @@ -17,7 +17,7 @@ def initialize(controller) # instance if it is available. def authorize(action, abstract_model = nil, model_object = nil) record = model_object || abstract_model && abstract_model.model - fail ::Pundit::NotAuthorizedError.new("not allowed to #{action} this #{record}") unless policy(record).send(action_for_pundit(action)) if action + raise ::Pundit::NotAuthorizedError.new("not allowed to #{action} this #{record}") unless policy(record).send(action_for_pundit(action)) if action end # This method is called primarily from the view to determine whether the given user diff --git a/spec/controllers/rails_admin/main_controller_spec.rb b/spec/controllers/rails_admin/main_controller_spec.rb index e54ffdeb7e..4c2b5f9c0d 100644 --- a/spec/controllers/rails_admin/main_controller_spec.rb +++ b/spec/controllers/rails_admin/main_controller_spec.rb @@ -61,7 +61,7 @@ def get(action, params) describe '#check_for_cancel' do before do - allow(controller).to receive(:back_or_index) { fail(StandardError.new('redirected back')) } + allow(controller).to receive(:back_or_index) { raise(StandardError.new('redirected back')) } end it 'redirects to back if params[:bulk_ids] is nil when params[:bulk_action] is present' do From 80fa592de9ae9eab0dba03befc75034c3679a8e3 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 11:44:43 +1000 Subject: [PATCH 04/32] Remove unnecessary spacing --- app/controllers/rails_admin/main_controller.rb | 4 ++-- lib/rails_admin/config/fields/types/has_many_association.rb | 2 +- spec/controllers/rails_admin/main_controller_spec.rb | 2 +- spec/dummy_app/config.ru | 2 +- spec/rails_admin/adapters/active_record_spec.rb | 2 +- spec/rails_admin/support/csv_converter_spec.rb | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/rails_admin/main_controller.rb b/app/controllers/rails_admin/main_controller.rb index f54602e214..49915c7e4d 100644 --- a/app/controllers/rails_admin/main_controller.rb +++ b/app/controllers/rails_admin/main_controller.rb @@ -93,7 +93,7 @@ def sanitize_params_for!(action, model_config = @model_config, target_params = p return unless target_params.present? fields = visible_fields(action, model_config) allowed_methods = fields.collect(&:allowed_methods).flatten.uniq.collect(&:to_s) << 'id' << '_destroy' - fields.each { |field| field.parse_input(target_params) } + fields.each { |field| field.parse_input(target_params) } target_params.slice!(*allowed_methods) target_params.permit! if target_params.respond_to?(:permit!) fields.select(&:nested_form).each do |association| @@ -110,7 +110,7 @@ def handle_save_error(whereto = :new) respond_to do |format| format.html { render whereto, status: :not_acceptable } - format.js { render whereto, layout: false, status: :not_acceptable } + format.js { render whereto, layout: false, status: :not_acceptable } end end diff --git a/lib/rails_admin/config/fields/types/has_many_association.rb b/lib/rails_admin/config/fields/types/has_many_association.rb index 2c62c4d5e9..c89386f86b 100644 --- a/lib/rails_admin/config/fields/types/has_many_association.rb +++ b/lib/rails_admin/config/fields/types/has_many_association.rb @@ -22,7 +22,7 @@ class HasManyAssociation < RailsAdmin::Config::Fields::Association end def method_name - nested_form ? "#{super}_attributes".to_sym : "#{super.to_s.singularize}_ids".to_sym # name_ids + nested_form ? "#{super}_attributes".to_sym : "#{super.to_s.singularize}_ids".to_sym # name_ids end # Reader for validation errors of the bound object diff --git a/spec/controllers/rails_admin/main_controller_spec.rb b/spec/controllers/rails_admin/main_controller_spec.rb index 4c2b5f9c0d..4bac5f00d5 100644 --- a/spec/controllers/rails_admin/main_controller_spec.rb +++ b/spec/controllers/rails_admin/main_controller_spec.rb @@ -104,7 +104,7 @@ def get(action, params) end context 'using active_record, supporting joins', active_record: true do - it 'gives back the local column' do + it 'gives back the local column' do controller.params = {sort: 'team', model_name: 'players'} expect(controller.send(:get_sort_hash, RailsAdmin.config(Player))).to eq(sort: 'teams.name', sort_reverse: true) end diff --git a/spec/dummy_app/config.ru b/spec/dummy_app/config.ru index 5fb67aa8bd..7ae1c84900 100644 --- a/spec/dummy_app/config.ru +++ b/spec/dummy_app/config.ru @@ -1,4 +1,4 @@ # This file is used by Rack-based servers to start the application. -require ::File.expand_path('../config/environment', __FILE__) +require ::File.expand_path('../config/environment', __FILE__) run DummyApp::Application diff --git a/spec/rails_admin/adapters/active_record_spec.rb b/spec/rails_admin/adapters/active_record_spec.rb index 74d809d15b..05424bdeea 100644 --- a/spec/rails_admin/adapters/active_record_spec.rb +++ b/spec/rails_admin/adapters/active_record_spec.rb @@ -323,7 +323,7 @@ class PlayerWithDefaultScope < Player it 'supports datetime type query' do scope = FieldTest.all - expect(predicates_for(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['', 'February 01, 2012 12:00', 'March 01, 2012 12:00'], o: 'between'}}))).to eq(predicates_for(scope.where(['(field_tests.datetime_field BETWEEN ? AND ?)', Time.local(2012, 2, 1), Time.local(2012, 3, 1).end_of_day]))) + expect(predicates_for(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['', 'February 01, 2012 12:00', 'March 01, 2012 12:00'], o: 'between'}}))).to eq(predicates_for(scope.where(['(field_tests.datetime_field BETWEEN ? AND ?)', Time.local(2012, 2, 1), Time.local(2012, 3, 1).end_of_day]))) expect(predicates_for(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['', 'March 01, 2012 12:00', ''], o: 'between'}}))).to eq(predicates_for(scope.where(['(field_tests.datetime_field >= ?)', Time.local(2012, 3, 1)]))) expect(predicates_for(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['', '', 'February 01, 2012 12:00'], o: 'between'}}))).to eq(predicates_for(scope.where(['(field_tests.datetime_field <= ?)', Time.local(2012, 2, 1).end_of_day]))) expect(predicates_for(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['February 01, 2012 12:00'], o: 'default'}}))).to eq(predicates_for(scope.where(['(field_tests.datetime_field BETWEEN ? AND ?)', Time.local(2012, 2, 1), Time.local(2012, 2, 1).end_of_day]))) diff --git a/spec/rails_admin/support/csv_converter_spec.rb b/spec/rails_admin/support/csv_converter_spec.rb index 379c8a2ffe..7c2a2a3e97 100644 --- a/spec/rails_admin/support/csv_converter_spec.rb +++ b/spec/rails_admin/support/csv_converter_spec.rb @@ -68,7 +68,7 @@ expect(subject[1]).to eq 'UTF-8' expect(subject[2].encoding).to eq Encoding::UTF_8 expect(subject[2].unpack('H*').first). - to eq 'efbbbf4e756d6265722c4e616d650a312ce381aae381bee381880a' # have BOM + to eq 'efbbbf4e756d6265722c4e616d650a312ce381aae381bee381880a' # have BOM end end From 347f5d11b55090439a9abd1ca39b81ca73ac2d7d Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 11:47:21 +1000 Subject: [PATCH 05/32] Replace nested if/else with elsif --- app/helpers/rails_admin/application_helper.rb | 4 ++-- lib/rails_admin/config.rb | 8 ++++---- lib/rails_admin/config/configurable.rb | 8 +++----- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/app/helpers/rails_admin/application_helper.rb b/app/helpers/rails_admin/application_helper.rb index 1b08dec4c0..2ef3e967bc 100644 --- a/app/helpers/rails_admin/application_helper.rb +++ b/app/helpers/rails_admin/application_helper.rb @@ -47,8 +47,8 @@ def logout_path if defined?(Devise) scope = Devise::Mapping.find_scope!(_current_user) main_app.send("destroy_#{scope}_session_path") rescue false - else - main_app.logout_path if main_app.respond_to?(:logout_path) + elsif main_app.respond_to?(:logout_path) + main_app.logout_path end end diff --git a/lib/rails_admin/config.rb b/lib/rails_admin/config.rb index 88d25898b2..37676b72dc 100644 --- a/lib/rails_admin/config.rb +++ b/lib/rails_admin/config.rb @@ -113,8 +113,8 @@ def audit_with(*args, &block) @audit = proc do @auditing_adapter = RailsAdmin::AUDITING_ADAPTERS[extension].new(*([self] + args).compact) end - else - @audit = block if block + elsif block + @audit = block end @audit || DEFAULT_AUDIT end @@ -148,8 +148,8 @@ def authorize_with(*args, &block) @authorize = proc do @authorization_adapter = RailsAdmin::AUTHORIZATION_ADAPTERS[extension].new(*([self] + args).compact) end - else - @authorize = block if block + elsif block + @authorize = block end @authorize || DEFAULT_AUTHORIZE end diff --git a/lib/rails_admin/config/configurable.rb b/lib/rails_admin/config/configurable.rb index e7975962c4..4e963c36a1 100644 --- a/lib/rails_admin/config/configurable.rb +++ b/lib/rails_admin/config/configurable.rb @@ -75,12 +75,10 @@ def register_deprecated_instance_option(option_name, replacement_option_name = n if replacement_option_name ActiveSupport::Deprecation.warn("The #{option_name} configuration option is deprecated, please use #{replacement_option_name}.") send(replacement_option_name, *args, &block) + elsif block_given? + yield else - if block_given? - yield - else - raise("The #{option_name} configuration option is removed without replacement.") - end + raise("The #{option_name} configuration option is removed without replacement.") end end end From dc46f8616df5e9346763ff2712bffc5396aec974 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 11:49:19 +1000 Subject: [PATCH 06/32] Enforce trailing commas in arguments inline with existing code style --- .rubocop.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index eb36a4f94c..8bb1c335a8 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -76,6 +76,8 @@ Style/RaiseArgs: Style/SpaceInsideHashLiteralBraces: EnforcedStyle: no_space +Style/TrailingCommaInArguments: + EnforcedStyleForMultiline: 'comma' Style/TrailingCommaInLiteral: EnforcedStyleForMultiline: 'comma' From 771d0c0cbce9f27f38799796acd1fec28ae3bf94 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 12:07:18 +1000 Subject: [PATCH 07/32] Replace length > 0 with !empty? --- app/helpers/rails_admin/form_builder.rb | 9 ++++++++- .../config/fields/types/polymorphic_association.rb | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/helpers/rails_admin/form_builder.rb b/app/helpers/rails_admin/form_builder.rb index 0e39a5bc63..c54b7fa08f 100644 --- a/app/helpers/rails_admin/form_builder.rb +++ b/app/helpers/rails_admin/form_builder.rb @@ -20,7 +20,14 @@ def generate(options = {}) end def fieldset_for(fieldset, nested_in) - return unless (fields = fieldset.with(form: self, object: @object, view: @template, controller: @template.controller).visible_fields).length > 0 + fields = fieldset.with( + form: self, + object: @object, + view: @template, + controller: @template.controller + ).visible_fields + return if fields.empty? + @template.content_tag :fieldset do contents = [] contents << @template.content_tag(:legend, %( #{fieldset.label}).html_safe, style: "#{fieldset.name == :default ? 'display:none' : ''}") diff --git a/lib/rails_admin/config/fields/types/polymorphic_association.rb b/lib/rails_admin/config/fields/types/polymorphic_association.rb index 9e68004ae3..b3fac30941 100644 --- a/lib/rails_admin/config/fields/types/polymorphic_association.rb +++ b/lib/rails_admin/config/fields/types/polymorphic_association.rb @@ -16,7 +16,7 @@ class PolymorphicAssociation < RailsAdmin::Config::Fields::Types::BelongsToAssoc # association checks that any of the child models are included in # configuration. register_instance_option :visible? do - associated_model_config.length > 0 + !associated_model_config.empty? end register_instance_option :formatted_value do From 21c827e9df465dc44837932788c94ef1818a252c Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 12:08:43 +1000 Subject: [PATCH 08/32] Add trailing comma to arguments --- app/helpers/rails_admin/form_builder.rb | 2 +- lib/rails_admin/adapters/active_record.rb | 4 +-- lib/rails_admin/config/fields/base.rb | 6 ++--- ...000000000012_add_avatar_columns_to_user.rb | 26 +++++++++---------- .../00000000000013_add_roles_to_user.rb | 14 +++++----- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/app/helpers/rails_admin/form_builder.rb b/app/helpers/rails_admin/form_builder.rb index c54b7fa08f..162f21ea25 100644 --- a/app/helpers/rails_admin/form_builder.rb +++ b/app/helpers/rails_admin/form_builder.rb @@ -24,7 +24,7 @@ def fieldset_for(fieldset, nested_in) form: self, object: @object, view: @template, - controller: @template.controller + controller: @template.controller, ).visible_fields return if fields.empty? diff --git a/lib/rails_admin/adapters/active_record.rb b/lib/rails_admin/adapters/active_record.rb index 494fdc26c6..d769e6c317 100644 --- a/lib/rails_admin/adapters/active_record.rb +++ b/lib/rails_admin/adapters/active_record.rb @@ -56,8 +56,8 @@ def associations def properties columns = model.columns.reject do |c| c.type.blank? || - DISABLED_COLUMN_TYPES.include?(c.type.to_sym) || - c.try(:array) + DISABLED_COLUMN_TYPES.include?(c.type.to_sym) || + c.try(:array) end columns.collect do |property| Property.new(property, model) diff --git a/lib/rails_admin/config/fields/base.rb b/lib/rails_admin/config/fields/base.rb index b915c70498..0ef1220bbf 100644 --- a/lib/rails_admin/config/fields/base.rb +++ b/lib/rails_admin/config/fields/base.rb @@ -175,9 +175,9 @@ def virtual? (@required ||= {})[context] ||= !!([name] + children_fields).uniq.detect do |column_name| # rubocop:disable DoubleNegation abstract_model.model.validators_on(column_name).detect do |v| !(v.options[:allow_nil] || v.options[:allow_blank]) && - [:presence, :numericality, :attachment_presence].include?(v.kind) && - (v.options[:on] == context || v.options[:on].blank?) && - (v.options[:if].blank? && v.options[:unless].blank?) + [:presence, :numericality, :attachment_presence].include?(v.kind) && + (v.options[:on] == context || v.options[:on].blank?) && + (v.options[:if].blank? && v.options[:unless].blank?) end end end diff --git a/spec/dummy_app/db/migrate/00000000000012_add_avatar_columns_to_user.rb b/spec/dummy_app/db/migrate/00000000000012_add_avatar_columns_to_user.rb index aceb50619e..f00090a655 100644 --- a/spec/dummy_app/db/migrate/00000000000012_add_avatar_columns_to_user.rb +++ b/spec/dummy_app/db/migrate/00000000000012_add_avatar_columns_to_user.rb @@ -1,15 +1,15 @@ - class AddAvatarColumnsToUser < MigrationBase - def self.up - add_column :users, :avatar_file_name, :string - add_column :users, :avatar_content_type, :string - add_column :users, :avatar_file_size, :integer - add_column :users, :avatar_updated_at, :datetime - end +class AddAvatarColumnsToUser < MigrationBase + def self.up + add_column :users, :avatar_file_name, :string + add_column :users, :avatar_content_type, :string + add_column :users, :avatar_file_size, :integer + add_column :users, :avatar_updated_at, :datetime + end - def self.down - remove_column :users, :avatar_file_name - remove_column :users, :avatar_content_type - remove_column :users, :avatar_file_size - remove_column :users, :avatar_updated_at - end + def self.down + remove_column :users, :avatar_file_name + remove_column :users, :avatar_content_type + remove_column :users, :avatar_file_size + remove_column :users, :avatar_updated_at end +end diff --git a/spec/dummy_app/db/migrate/00000000000013_add_roles_to_user.rb b/spec/dummy_app/db/migrate/00000000000013_add_roles_to_user.rb index 9c32f1b5dd..60d4436f51 100644 --- a/spec/dummy_app/db/migrate/00000000000013_add_roles_to_user.rb +++ b/spec/dummy_app/db/migrate/00000000000013_add_roles_to_user.rb @@ -1,9 +1,9 @@ - class AddRolesToUser < MigrationBase - def self.up - add_column :users, :roles, :string - end +class AddRolesToUser < MigrationBase + def self.up + add_column :users, :roles, :string + end - def self.down - remove_column :users, :roles - end + def self.down + remove_column :users, :roles end +end From 7bb1788ecc940d31a229f918d57a834b78438bde Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 12:15:50 +1000 Subject: [PATCH 09/32] Remove unnecessary interpolation --- app/helpers/rails_admin/form_builder.rb | 2 +- lib/rails_admin/adapters/active_record.rb | 2 +- spec/dummy_app/config/application.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/rails_admin/form_builder.rb b/app/helpers/rails_admin/form_builder.rb index 162f21ea25..baf0877bad 100644 --- a/app/helpers/rails_admin/form_builder.rb +++ b/app/helpers/rails_admin/form_builder.rb @@ -30,7 +30,7 @@ def fieldset_for(fieldset, nested_in) @template.content_tag :fieldset do contents = [] - contents << @template.content_tag(:legend, %( #{fieldset.label}).html_safe, style: "#{fieldset.name == :default ? 'display:none' : ''}") + contents << @template.content_tag(:legend, %( #{fieldset.label}).html_safe, style: fieldset.name == :default ? 'display:none' : '') contents << @template.content_tag(:p, fieldset.help) if fieldset.help.present? contents << fields.collect { |field| field_wrapper_for(field, nested_in) }.join contents.join.html_safe diff --git a/lib/rails_admin/adapters/active_record.rb b/lib/rails_admin/adapters/active_record.rb index d769e6c317..ec7ce3a2ca 100644 --- a/lib/rails_admin/adapters/active_record.rb +++ b/lib/rails_admin/adapters/active_record.rb @@ -211,7 +211,7 @@ def build_statement_for_string_or_text when 'ends_with' "%#{@value.downcase}" when 'is', '=' - "#{@value.downcase}" + @value.downcase else return end diff --git a/spec/dummy_app/config/application.rb b/spec/dummy_app/config/application.rb index 27d2163d71..30602e897c 100644 --- a/spec/dummy_app/config/application.rb +++ b/spec/dummy_app/config/application.rb @@ -5,7 +5,7 @@ require 'sprockets/railtie' begin - require "#{CI_ORM}" + require CI_ORM.to_s require "#{CI_ORM}/railtie" rescue LoadError # rubocop:disable HandleExceptions end From 6c545d05f9927551495ada41a8fa396ed2f749ed Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 12:17:20 +1000 Subject: [PATCH 10/32] Remove redundant self --- config/initializers/active_record_extensions.rb | 4 ++-- lib/rails_admin/adapters/mongoid/extension.rb | 2 +- lib/rails_admin/config/fields/base.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/initializers/active_record_extensions.rb b/config/initializers/active_record_extensions.rb index a596845ceb..3dcc266d49 100644 --- a/config/initializers/active_record_extensions.rb +++ b/config/initializers/active_record_extensions.rb @@ -6,11 +6,11 @@ def self.rails_admin(&block) end def rails_admin_default_object_label_method - self.new_record? ? "new #{self.class}" : "#{self.class} ##{id}" + new_record? ? "new #{self.class}" : "#{self.class} ##{id}" end def safe_send(value) - if self.has_attribute?(value) + if has_attribute?(value) read_attribute(value) else send(value) diff --git a/lib/rails_admin/adapters/mongoid/extension.rb b/lib/rails_admin/adapters/mongoid/extension.rb index 6258e3dff0..6c130fe5cd 100644 --- a/lib/rails_admin/adapters/mongoid/extension.rb +++ b/lib/rails_admin/adapters/mongoid/extension.rb @@ -16,7 +16,7 @@ def rails_admin(&block) end def rails_admin_default_object_label_method - self.new_record? ? "new #{self.class}" : "#{self.class} ##{id}" + new_record? ? "new #{self.class}" : "#{self.class} ##{id}" end def safe_send(value) diff --git a/lib/rails_admin/config/fields/base.rb b/lib/rails_admin/config/fields/base.rb index 0ef1220bbf..a33e016745 100644 --- a/lib/rails_admin/config/fields/base.rb +++ b/lib/rails_admin/config/fields/base.rb @@ -92,7 +92,7 @@ def virtual? property = am && am.properties.detect { |p| p.name == f.values.first.to_sym } type = property && property.type else # - am = (self.association? ? associated_model_config.abstract_model : abstract_model) + am = (association? ? associated_model_config.abstract_model : abstract_model) table_name = am.table_name column = f property = am.properties.detect { |p| p.name == f.to_sym } From 0cfc6058e077305c6474477eaf747736a70ee153 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 12:23:52 +1000 Subject: [PATCH 11/32] Disable enforcing of alias over alias_method These methods are not aliases of each other and there are valid reasons for sometimes desiring alias_method --- .rubocop.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 8bb1c335a8..635e1ed131 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -41,6 +41,9 @@ Metrics/PerceivedComplexity: Style/AccessModifierIndentation: EnforcedStyle: outdent +Style/Alias: + Enabled: false + Style/CollectionMethods: PreferredMethods: map: 'collect' From c94e36a983edd43d8fbbf6bd3243dee8c8844b19 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 12:25:51 +1000 Subject: [PATCH 12/32] Freeze constants --- lib/rails_admin/adapters/active_record.rb | 2 +- lib/rails_admin/adapters/mongoid.rb | 2 +- lib/rails_admin/adapters/mongoid/property.rb | 2 +- lib/rails_admin/extension.rb | 8 ++++---- .../extensions/paper_trail/auditing_adapter.rb | 2 +- lib/rails_admin/support/datetime.rb | 2 +- lib/rails_admin/version.rb | 2 +- spec/rails_admin/config/fields/base_spec.rb | 2 +- spec/rails_admin/config_spec.rb | 2 +- spec/spec_helper.rb | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/rails_admin/adapters/active_record.rb b/lib/rails_admin/adapters/active_record.rb index ec7ce3a2ca..cd6b826a69 100644 --- a/lib/rails_admin/adapters/active_record.rb +++ b/lib/rails_admin/adapters/active_record.rb @@ -6,7 +6,7 @@ module RailsAdmin module Adapters module ActiveRecord - DISABLED_COLUMN_TYPES = [:tsvector, :blob, :binary, :spatial, :hstore, :geometry] + DISABLED_COLUMN_TYPES = [:tsvector, :blob, :binary, :spatial, :hstore, :geometry].freeze def new(params = {}) AbstractObject.new(model.new(params)) diff --git a/lib/rails_admin/adapters/mongoid.rb b/lib/rails_admin/adapters/mongoid.rb index 10e3c2ce67..1c0a184364 100644 --- a/lib/rails_admin/adapters/mongoid.rb +++ b/lib/rails_admin/adapters/mongoid.rb @@ -8,7 +8,7 @@ module RailsAdmin module Adapters module Mongoid - DISABLED_COLUMN_TYPES = %w(Range Moped::BSON::Binary BSON::Binary Mongoid::Geospatial::Point) + DISABLED_COLUMN_TYPES = %w(Range Moped::BSON::Binary BSON::Binary Mongoid::Geospatial::Point).freeze def parse_object_id(value) Bson.parse_object_id(value) diff --git a/lib/rails_admin/adapters/mongoid/property.rb b/lib/rails_admin/adapters/mongoid/property.rb index ffe0bb4130..fadf7bbe47 100644 --- a/lib/rails_admin/adapters/mongoid/property.rb +++ b/lib/rails_admin/adapters/mongoid/property.rb @@ -2,7 +2,7 @@ module RailsAdmin module Adapters module Mongoid class Property - STRING_TYPE_COLUMN_NAMES = [:name, :title, :subject] + STRING_TYPE_COLUMN_NAMES = [:name, :title, :subject].freeze attr_reader :property, :model def initialize(property, model) diff --git a/lib/rails_admin/extension.rb b/lib/rails_admin/extension.rb index bc161d01a0..6e5f9c2813 100644 --- a/lib/rails_admin/extension.rb +++ b/lib/rails_admin/extension.rb @@ -1,9 +1,9 @@ module RailsAdmin - EXTENSIONS = [] - AUTHORIZATION_ADAPTERS = {} - AUDITING_ADAPTERS = {} - CONFIGURATION_ADAPTERS = {} + EXTENSIONS = [].freeze + AUTHORIZATION_ADAPTERS = {}.freeze + AUDITING_ADAPTERS = {}.freeze + CONFIGURATION_ADAPTERS = {}.freeze # Extend RailsAdmin # diff --git a/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb b/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb index f051c8dd52..94e6e51c54 100644 --- a/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb +++ b/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb @@ -36,7 +36,7 @@ class AuditingAdapter item: :item_id, created_at: :created_at, message: :event, - } + }.freeze def initialize(controller, user_class = 'User', version_class = '::Version') raise('PaperTrail not found') unless defined?(PaperTrail) diff --git a/lib/rails_admin/support/datetime.rb b/lib/rails_admin/support/datetime.rb index 05b26c8a9c..e175c1b66c 100644 --- a/lib/rails_admin/support/datetime.rb +++ b/lib/rails_admin/support/datetime.rb @@ -23,7 +23,7 @@ class Datetime '%S' => 'ss', # Second of the minute (00..60) '%Y' => 'YYYY', # Year with century '%y' => 'YY', # Year without a century (00..99) - } + }.freeze class << self include RailsAdmin::Support::I18n diff --git a/lib/rails_admin/version.rb b/lib/rails_admin/version.rb index 2f8f4dfebe..33cce2db8a 100644 --- a/lib/rails_admin/version.rb +++ b/lib/rails_admin/version.rb @@ -3,7 +3,7 @@ class Version MAJOR = 1 MINOR = 0 PATCH = 0 - PRE = 'rc' + PRE = 'rc'.freeze class << self # @return [String] diff --git a/spec/rails_admin/config/fields/base_spec.rb b/spec/rails_admin/config/fields/base_spec.rb index 457df88b2a..17b460f710 100644 --- a/spec/rails_admin/config/fields/base_spec.rb +++ b/spec/rails_admin/config/fields/base_spec.rb @@ -44,7 +44,7 @@ class ConditionalValidationTest < Tableless end describe '#children_fields' do - POLYMORPHIC_CHILDREN = [:commentable_id, :commentable_type] + POLYMORPHIC_CHILDREN = [:commentable_id, :commentable_type].freeze it 'is empty by default' do expect(RailsAdmin.config(Team).fields.detect { |f| f.name == :name }.children_fields).to eq([]) diff --git a/spec/rails_admin/config_spec.rb b/spec/rails_admin/config_spec.rb index b69ba01374..96b86260e8 100644 --- a/spec/rails_admin/config_spec.rb +++ b/spec/rails_admin/config_spec.rb @@ -36,7 +36,7 @@ describe '.add_extension' do before do silence_warnings do - RailsAdmin::EXTENSIONS = [] + RailsAdmin::EXTENSIONS = [].freeze end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dff660b621..013dd5b663 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,7 +1,7 @@ # Configure Rails Envinronment ENV['RAILS_ENV'] = 'test' CI_ORM = (ENV['CI_ORM'] || :active_record).to_sym -CI_TARGET_ORMS = [:active_record, :mongoid] +CI_TARGET_ORMS = [:active_record, :mongoid].freeze PK_COLUMN = {active_record: :id, mongoid: :_id}[CI_ORM] require 'simplecov' From aabff1d913943bae4d46f73dad06fa47c9da4b13 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 12:29:14 +1000 Subject: [PATCH 13/32] Use return value of conditional for assignment --- lib/rails_admin/adapters/active_record.rb | 11 ++++++----- lib/rails_admin/config/fields/types/string.rb | 11 ++++++----- .../config/initializers/active_record_migration.rb | 11 ++++++----- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/rails_admin/adapters/active_record.rb b/lib/rails_admin/adapters/active_record.rb index cd6b826a69..b458e8b543 100644 --- a/lib/rails_admin/adapters/active_record.rb +++ b/lib/rails_admin/adapters/active_record.rb @@ -99,11 +99,12 @@ def initialize(scope) def add(field, value, operator) field.searchable_columns.flatten.each do |column_infos| - if value.is_a?(Array) - value = value.map { |v| field.parse_value(v) } - else - value = field.parse_value(value) - end + 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? diff --git a/lib/rails_admin/config/fields/types/string.rb b/lib/rails_admin/config/fields/types/string.rb index 07c3deb68b..ee2a6082cc 100644 --- a/lib/rails_admin/config/fields/types/string.rb +++ b/lib/rails_admin/config/fields/types/string.rb @@ -27,11 +27,12 @@ def generic_help max_length = [length, valid_length[:maximum] || nil].compact.min min_length = [0, valid_length[:minimum] || nil].compact.max if max_length - if min_length == 0 - text += "#{I18n.translate('admin.form.char_length_up_to').capitalize} #{max_length}." - else - text += "#{I18n.translate('admin.form.char_length_of').capitalize} #{min_length}-#{max_length}." - end + text += + if min_length == 0 + "#{I18n.translate('admin.form.char_length_up_to').capitalize} #{max_length}." + else + "#{I18n.translate('admin.form.char_length_of').capitalize} #{min_length}-#{max_length}." + end end end text diff --git a/spec/dummy_app/config/initializers/active_record_migration.rb b/spec/dummy_app/config/initializers/active_record_migration.rb index f3b3a566c1..353cadfa74 100644 --- a/spec/dummy_app/config/initializers/active_record_migration.rb +++ b/spec/dummy_app/config/initializers/active_record_migration.rb @@ -1,7 +1,8 @@ if defined?(ActiveRecord) - if Rails.version >= '5.0' - MigrationBase = ActiveRecord::Migration[5.0] - else - MigrationBase = ActiveRecord::Migration - end + MigrationBase = + if Rails.version >= '5.0' + ActiveRecord::Migration[5.0] + else + ActiveRecord::Migration + end end From ff8c38dc703a275c7a9aad2f157f9939d3d29fa0 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 12:55:41 +1000 Subject: [PATCH 14/32] Increase performance of string replacement --- lib/rails_admin/bootstrap-sass/compass_functions.rb | 2 +- spec/dummy_app/app/active_record/ball.rb | 2 +- spec/dummy_app/app/mongoid/ball.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rails_admin/bootstrap-sass/compass_functions.rb b/lib/rails_admin/bootstrap-sass/compass_functions.rb index 91a55109f5..3e5ff870fa 100755 --- a/lib/rails_admin/bootstrap-sass/compass_functions.rb +++ b/lib/rails_admin/bootstrap-sass/compass_functions.rb @@ -8,7 +8,7 @@ def image_path(source, _options = {}) image_url(source, Sass::Script::Bool.new(true)) else # Revert to the old compass-agnostic path determination - asset_sans_quotes = source.value.gsub('"', '') + asset_sans_quotes = source.value.delete('"') Sass::Script::String.new("/images/#{asset_sans_quotes}", :string) end end diff --git a/spec/dummy_app/app/active_record/ball.rb b/spec/dummy_app/app/active_record/ball.rb index 69e2645e8a..fb6dd84010 100644 --- a/spec/dummy_app/app/active_record/ball.rb +++ b/spec/dummy_app/app/active_record/ball.rb @@ -2,6 +2,6 @@ class Ball < ActiveRecord::Base validates_presence_of :color, on: :create def to_param - color.present? ? color.downcase.gsub(' ', '-') : id + color.present? ? color.downcase.tr(' ', '-') : id end end diff --git a/spec/dummy_app/app/mongoid/ball.rb b/spec/dummy_app/app/mongoid/ball.rb index c64fc5bcc2..9b064d2e51 100644 --- a/spec/dummy_app/app/mongoid/ball.rb +++ b/spec/dummy_app/app/mongoid/ball.rb @@ -7,6 +7,6 @@ class Ball validates_presence_of :color, on: :create def to_param - color.present? ? color.downcase.gsub(' ', '-') : id + color.present? ? color.downcase.tr(' ', '-') : id end end From 5ea270998a6a4d080f755b79f54de3e0e26a3a00 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 12:57:42 +1000 Subject: [PATCH 15/32] Remove useless access modifier --- lib/rails_admin/bootstrap-sass.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/rails_admin/bootstrap-sass.rb b/lib/rails_admin/bootstrap-sass.rb index cf7499d745..77a3980463 100755 --- a/lib/rails_admin/bootstrap-sass.rb +++ b/lib/rails_admin/bootstrap-sass.rb @@ -21,8 +21,6 @@ def self.load! ::Sass.load_paths << stylesheets end - private - def self.asset_pipeline? defined?(::Sprockets) end From 44f2e65117e2868a09318903d81acec87db075c4 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 12:59:29 +1000 Subject: [PATCH 16/32] Replace empty case statement with if/elsif --- lib/rails_admin/config/actions/base.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/rails_admin/config/actions/base.rb b/lib/rails_admin/config/actions/base.rb index 96e5e9db18..3dc5fb635e 100644 --- a/lib/rails_admin/config/actions/base.rb +++ b/lib/rails_admin/config/actions/base.rb @@ -115,12 +115,11 @@ class Base # Breadcrumb parent register_instance_option :breadcrumb_parent do - case - when root? + if root? [:dashboard] - when collection? + elsif collection? [:index, bindings[:abstract_model]] - when member? + elsif member? [:show, bindings[:abstract_model], bindings[:object]] end end From b4f257dc5b1091f09ed6e77af6736f5b327caf3a Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 13:04:13 +1000 Subject: [PATCH 17/32] DRY --- lib/rails_admin/config/actions/bulk_delete.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/rails_admin/config/actions/bulk_delete.rb b/lib/rails_admin/config/actions/bulk_delete.rb index 2d05b42db7..a2aa821272 100644 --- a/lib/rails_admin/config/actions/bulk_delete.rb +++ b/lib/rails_admin/config/actions/bulk_delete.rb @@ -43,13 +43,11 @@ class BulkDelete < RailsAdmin::Config::Actions::Base if destroyed.nil? flash[:error] = t('admin.flash.error', name: pluralize(0, @model_config.label), action: t('admin.actions.delete.done')) - redirect_to back_or_index else flash[:success] = t('admin.flash.successful', name: pluralize(destroyed.count, @model_config.label), action: t('admin.actions.delete.done')) unless destroyed.empty? flash[:error] = t('admin.flash.error', name: pluralize(not_destroyed.count, @model_config.label), action: t('admin.actions.delete.done')) unless not_destroyed.empty? - redirect_to back_or_index end - + redirect_to back_or_index end end end From 174ba1ebda0120a99b140999079e43d5bca9cf5e Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 13:07:32 +1000 Subject: [PATCH 18/32] Remove redundant disabling of Rubocop rules --- lib/rails_admin/config/configurable.rb | 2 +- lib/rails_admin/config/fields/association.rb | 2 +- lib/rails_admin/config/fields/base.rb | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/rails_admin/config/configurable.rb b/lib/rails_admin/config/configurable.rb index 4e963c36a1..6bc9a93306 100644 --- a/lib/rails_admin/config/configurable.rb +++ b/lib/rails_admin/config/configurable.rb @@ -43,7 +43,7 @@ def register_instance_option(option_name, scope = self, &default) # Define getter/setter by the option name scope.send(:define_method, option_name) do |*args, &block| - if !args[0].nil? || block # rubocop:disable NonNilCheck + if !args[0].nil? || block # Invocation with args --> This is the declaration of the option, i.e. setter instance_variable_set("@#{option_name}_registered", args[0].nil? ? block : args[0]) else diff --git a/lib/rails_admin/config/fields/association.rb b/lib/rails_admin/config/fields/association.rb index a561cf54e4..20d0884ac5 100644 --- a/lib/rails_admin/config/fields/association.rb +++ b/lib/rails_admin/config/fields/association.rb @@ -10,7 +10,7 @@ def self.inherited(klass) end # Reader for the association information hash - def association # rubocop:disable TrivialAccessors + def association @properties end diff --git a/lib/rails_admin/config/fields/base.rb b/lib/rails_admin/config/fields/base.rb index a33e016745..afd7d73cea 100644 --- a/lib/rails_admin/config/fields/base.rb +++ b/lib/rails_admin/config/fields/base.rb @@ -57,7 +57,7 @@ def virtual? end register_instance_option :filterable? do - !!searchable # rubocop:disable DoubleNegation + !!searchable end register_instance_option :search_operator do @@ -172,7 +172,7 @@ def virtual? :nil end end - (@required ||= {})[context] ||= !!([name] + children_fields).uniq.detect do |column_name| # rubocop:disable DoubleNegation + (@required ||= {})[context] ||= !!([name] + children_fields).uniq.detect do |column_name| abstract_model.model.validators_on(column_name).detect do |v| !(v.options[:allow_nil] || v.options[:allow_blank]) && [:presence, :numericality, :attachment_presence].include?(v.kind) && @@ -247,7 +247,7 @@ def optional? # # @see RailsAdmin::Config::Fields::Base.register_instance_option :required? def optional(state = nil, &block) - if !state.nil? || block # rubocop:disable NonNilCheck + if !state.nil? || block required state.nil? ? proc { false == (instance_eval(&block)) } : false == state else optional? From db2bfce84c980685525832335cf0ba34adb9896d Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 16:34:05 +1000 Subject: [PATCH 19/32] Move repeated inspect logic into shared class --- lib/rails_admin/config/fields/base.rb | 25 +++++----------- lib/rails_admin/config/inspectable.rb | 39 +++++++++++++++++++++++++ lib/rails_admin/config/model.rb | 21 +++---------- lib/rails_admin/config/sections/base.rb | 21 +++---------- 4 files changed, 54 insertions(+), 52 deletions(-) create mode 100644 lib/rails_admin/config/inspectable.rb diff --git a/lib/rails_admin/config/fields/base.rb b/lib/rails_admin/config/fields/base.rb index afd7d73cea..889edda916 100644 --- a/lib/rails_admin/config/fields/base.rb +++ b/lib/rails_admin/config/fields/base.rb @@ -2,6 +2,7 @@ require 'rails_admin/config/configurable' require 'rails_admin/config/hideable' require 'rails_admin/config/groupable' +require 'rails_admin/config/inspectable' module RailsAdmin module Config @@ -11,11 +12,17 @@ class Base # rubocop:disable ClassLength include RailsAdmin::Config::Configurable include RailsAdmin::Config::Hideable include RailsAdmin::Config::Groupable + include RailsAdmin::Config::Inspectable attr_reader :name, :properties, :abstract_model attr_accessor :defined, :order, :section attr_reader :parent, :root + NAMED_INSTANCE_VARIABLES = [ + :@parent, :@root, :@section, :@children_fields_registered, + :@associated_model_config, :@group, :@bindings + ].freeze + def initialize(parent, name, properties) @parent = parent @root = parent.root @@ -324,24 +331,6 @@ def form_default_value def form_value form_default_value.nil? ? formatted_value : form_default_value end - - def inspect - "#<#{self.class.name}[#{name}] #{ - instance_variables.collect do |v| - value = instance_variable_get(v) - if [:@parent, :@root, :@section, :@children_fields_registered, - :@associated_model_config, :@group, :@bindings].include? v - if value.respond_to? :name - "#{v}=#{value.name.inspect}" - else - "#{v}=#{value.class.name}" - end - else - "#{v}=#{value.inspect}" - end - end.join(', ') - }>" - end end end end diff --git a/lib/rails_admin/config/inspectable.rb b/lib/rails_admin/config/inspectable.rb new file mode 100644 index 0000000000..f5aab5e1db --- /dev/null +++ b/lib/rails_admin/config/inspectable.rb @@ -0,0 +1,39 @@ +module RailsAdmin + module Config + module Inspectable + def inspect + set_named_instance_variables + + instance_name = try(:name) || try(:abstract_model).try(:model).try(:name) + instance_name = instance_name ? "[#{instance_name}]" : '' + + instance_vars = instance_variables.collect do |v| + instance_variable_name(v) + end.join(', ') + + "#<#{self.class.name}#{instance_name} #{instance_vars}>" + end + + private + + def instance_variable_name(variable) + value = instance_variable_get(variable) + if self.class::NAMED_INSTANCE_VARIABLES.include?(variable) + if value.respond_to?(:name) + "#{variable}=#{value.name.inspect}" + else + "#{variable}=#{value.class.name}" + end + else + "#{variable}=#{value.inspect}" + end + end + + def set_named_instance_variables + unless defined?(self.class::NAMED_INSTANCE_VARIABLES) + self.class.const_set('NAMED_INSTANCE_VARIABLES', []) + end + end + end + end +end diff --git a/lib/rails_admin/config/model.rb b/lib/rails_admin/config/model.rb index 71b6ff220f..8d679c7c2c 100644 --- a/lib/rails_admin/config/model.rb +++ b/lib/rails_admin/config/model.rb @@ -9,6 +9,7 @@ require 'rails_admin/config/has_description' require 'rails_admin/config/sections' require 'rails_admin/config/actions' +require 'rails_admin/config/inspectable' module RailsAdmin module Config @@ -18,11 +19,14 @@ class Model include RailsAdmin::Config::Configurable include RailsAdmin::Config::Hideable include RailsAdmin::Config::Sections + include RailsAdmin::Config::Inspectable attr_reader :abstract_model attr_accessor :groups attr_reader :parent, :root + NAMED_INSTANCE_VARIABLES = [:@parent, :@root].freeze + def initialize(entity) @parent = nil @root = self @@ -98,23 +102,6 @@ def pluralize(count) def method_missing(m, *args, &block) send(:base).send(m, *args, &block) end - - def inspect - "#<#{self.class.name}[#{abstract_model.model.name}] #{ - instance_variables.collect do |v| - value = instance_variable_get(v) - if [:@parent, :@root].include? v - if value.respond_to? :name - "#{v}=#{value.name.inspect}" - else - "#{v}=#{value.class.name}" - end - else - "#{v}=#{value.inspect}" - end - end.join(', ') - }>" - end end end end diff --git a/lib/rails_admin/config/sections/base.rb b/lib/rails_admin/config/sections/base.rb index 1185dfbb1c..cdb8a5ffe1 100644 --- a/lib/rails_admin/config/sections/base.rb +++ b/lib/rails_admin/config/sections/base.rb @@ -1,5 +1,6 @@ require 'rails_admin/config/proxyable' require 'rails_admin/config/configurable' +require 'rails_admin/config/inspectable' require 'rails_admin/config/has_fields' require 'rails_admin/config/has_groups' require 'rails_admin/config/has_description' @@ -11,6 +12,7 @@ module Sections class Base include RailsAdmin::Config::Proxyable include RailsAdmin::Config::Configurable + include RailsAdmin::Config::Inspectable include RailsAdmin::Config::HasFields include RailsAdmin::Config::HasGroups @@ -19,29 +21,14 @@ class Base attr_reader :abstract_model attr_reader :parent, :root + NAMED_INSTANCE_VARIABLES = [:@parent, :@root, :@abstract_model].freeze + def initialize(parent) @parent = parent @root = parent.root @abstract_model = root.abstract_model end - - def inspect - "#<#{self.class.name} #{ - instance_variables.collect do |v| - value = instance_variable_get(v) - if [:@parent, :@root, :@abstract_model].include? v - if value.respond_to? :name - "#{v}=#{value.name.inspect}" - else - "#{v}=#{value.class.name}" - end - else - "#{v}=#{value.inspect}" - end - end.join(', ') - }>" - end end end end From e02cb2ae86f4ab27cabec3cb9454b0477a6307e1 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 16:35:33 +1000 Subject: [PATCH 20/32] Outdent access modifier --- lib/rails_admin/config/inspectable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rails_admin/config/inspectable.rb b/lib/rails_admin/config/inspectable.rb index f5aab5e1db..e43ef5fd26 100644 --- a/lib/rails_admin/config/inspectable.rb +++ b/lib/rails_admin/config/inspectable.rb @@ -14,7 +14,7 @@ def inspect "#<#{self.class.name}#{instance_name} #{instance_vars}>" end - private + private def instance_variable_name(variable) value = instance_variable_get(variable) From 3077c0ebbf505ad87613000ca3b4328dfccf503c Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 16:36:59 +1000 Subject: [PATCH 21/32] Remove redundant merge --- lib/rails_admin/engine.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rails_admin/engine.rb b/lib/rails_admin/engine.rb index 8293ecbc94..245cb4bff5 100644 --- a/lib/rails_admin/engine.rb +++ b/lib/rails_admin/engine.rb @@ -12,7 +12,7 @@ module RailsAdmin class Engine < Rails::Engine isolate_namespace RailsAdmin - config.action_dispatch.rescue_responses.merge!('RailsAdmin::ActionNotAllowed' => :forbidden) + config.action_dispatch.rescue_responses['RailsAdmin::ActionNotAllowed'] = :forbidden initializer 'RailsAdmin precompile hook', group: :all do |app| app.config.assets.precompile += %w( From 1b7937b684b0703b5d7f4f878c4ed78d3dd227ba Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 16:49:57 +1000 Subject: [PATCH 22/32] Remove nested modifier --- lib/rails_admin/extensions/pundit/authorization_adapter.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rails_admin/extensions/pundit/authorization_adapter.rb b/lib/rails_admin/extensions/pundit/authorization_adapter.rb index ec8e7c8c41..42355d3305 100644 --- a/lib/rails_admin/extensions/pundit/authorization_adapter.rb +++ b/lib/rails_admin/extensions/pundit/authorization_adapter.rb @@ -17,7 +17,9 @@ def initialize(controller) # instance if it is available. def authorize(action, abstract_model = nil, model_object = nil) record = model_object || abstract_model && abstract_model.model - raise ::Pundit::NotAuthorizedError.new("not allowed to #{action} this #{record}") unless policy(record).send(action_for_pundit(action)) if action + if action && !policy(record).send(action_for_pundit(action)) + raise ::Pundit::NotAuthorizedError.new("not allowed to #{action} this #{record}") + end end # This method is called primarily from the view to determine whether the given user From 06dd59a61b32ecd9d82dfd6a4ac1d29236e5ba96 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 17:04:26 +1000 Subject: [PATCH 23/32] Variables have been incorrectly defined like constants but are actually mutable --- lib/rails_admin/extension.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/rails_admin/extension.rb b/lib/rails_admin/extension.rb index 6e5f9c2813..fd9738a77e 100644 --- a/lib/rails_admin/extension.rb +++ b/lib/rails_admin/extension.rb @@ -1,9 +1,9 @@ module RailsAdmin - EXTENSIONS = [].freeze - AUTHORIZATION_ADAPTERS = {}.freeze - AUDITING_ADAPTERS = {}.freeze - CONFIGURATION_ADAPTERS = {}.freeze + EXTENSIONS = [] # rubocop:disable MutableConstant + AUTHORIZATION_ADAPTERS = {} # rubocop:disable MutableConstant + AUDITING_ADAPTERS = {} # rubocop:disable MutableConstant + CONFIGURATION_ADAPTERS = {} # rubocop:disable MutableConstant # Extend RailsAdmin # From 6a129ec35f076ad6bfc9c9ed9366bb4e0eb85d4f Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 17:04:56 +1000 Subject: [PATCH 24/32] Replace times.collect with Array.new for better performance --- .../rails_admin/main_controller_spec.rb | 18 +++++++++--------- .../rails_admin_basic_bulk_action_spec.rb | 2 +- .../rails_admin_basic_bulk_destroy_spec.rb | 4 ++-- .../create/rails_admin_basic_create_spec.rb | 4 ++-- .../basic/edit/rails_admin_basic_edit_spec.rb | 4 ++-- .../export/rails_admin_basic_export_spec.rb | 4 ++-- .../basic/list/rails_admin_basic_list_spec.rb | 14 +++++++------- .../update/rails_admin_basic_update_spec.rb | 2 +- spec/rails_admin/config/fields/base_spec.rb | 6 +++--- 9 files changed, 29 insertions(+), 29 deletions(-) diff --git a/spec/controllers/rails_admin/main_controller_spec.rb b/spec/controllers/rails_admin/main_controller_spec.rb index 4bac5f00d5..b82f64b56c 100644 --- a/spec/controllers/rails_admin/main_controller_spec.rb +++ b/spec/controllers/rails_admin/main_controller_spec.rb @@ -113,7 +113,7 @@ def get(action, params) describe '#list_entries called from view' do before do - @teams = 21.times.collect { FactoryGirl.create :team } + @teams = Array.new(21) { FactoryGirl.create :team } controller.params = {model_name: 'teams'} end @@ -125,7 +125,7 @@ def get(action, params) describe '#list_entries called from view with kaminari custom param_name' do before do - @teams = 21.times.collect { FactoryGirl.create :team } + @teams = Array.new(21) { FactoryGirl.create :team } controller.params = {model_name: 'teams'} Kaminari.config.param_name = :pagina end @@ -142,7 +142,7 @@ def get(action, params) describe '#list_entries called with bulk_ids' do before do - @teams = 21.times.collect { FactoryGirl.create :team } + @teams = Array.new(21) { FactoryGirl.create :team } controller.params = {model_name: 'teams', bulk_action: 'bulk_delete', bulk_ids: @teams.collect(&:id)} end @@ -159,7 +159,7 @@ def get(action, params) end it "doesn't scope associated collection records when associated_collection_scope is nil" do - @players = 2.times.collect do + @players = Array.new(2) do FactoryGirl.create :player end @@ -173,7 +173,7 @@ def get(action, params) end it 'scopes associated collection records according to associated_collection_scope' do - @players = 4.times.collect do + @players = Array.new(4) do FactoryGirl.create :player end @@ -192,7 +192,7 @@ def get(action, params) @team.revenue = BigDecimal.new('3') @team.save - @players = 5.times.collect do + @players = Array.new(5) do FactoryGirl.create :player end @@ -211,7 +211,7 @@ def get(action, params) end it 'limits associated collection records number to 30 if cache_all is false' do - @players = 40.times.collect do + @players = Array.new(40) do FactoryGirl.create :player end @@ -224,7 +224,7 @@ def get(action, params) end it "doesn't limit associated collection records number to 30 if cache_all is true" do - @players = 40.times.collect do + @players = Array.new(40) do FactoryGirl.create :player end @@ -237,7 +237,7 @@ def get(action, params) end it 'orders associated collection records by id, descending' do - @players = 3.times.collect do + @players = Array.new(3) do FactoryGirl.create :player end diff --git a/spec/integration/basic/bulk_action/rails_admin_basic_bulk_action_spec.rb b/spec/integration/basic/bulk_action/rails_admin_basic_bulk_action_spec.rb index 3670999462..ffa16abee5 100644 --- a/spec/integration/basic/bulk_action/rails_admin_basic_bulk_action_spec.rb +++ b/spec/integration/basic/bulk_action/rails_admin_basic_bulk_action_spec.rb @@ -4,7 +4,7 @@ subject { page } before do - @players = 2.times.collect { FactoryGirl.create :player } + @players = Array.new(2) { FactoryGirl.create :player } end describe 'bulk_delete' do diff --git a/spec/integration/basic/bulk_destroy/rails_admin_basic_bulk_destroy_spec.rb b/spec/integration/basic/bulk_destroy/rails_admin_basic_bulk_destroy_spec.rb index 122698e8f4..12e369a8d6 100644 --- a/spec/integration/basic/bulk_destroy/rails_admin_basic_bulk_destroy_spec.rb +++ b/spec/integration/basic/bulk_destroy/rails_admin_basic_bulk_destroy_spec.rb @@ -7,7 +7,7 @@ before do RailsAdmin::History.destroy_all RailsAdmin.config { |c| c.audit_with :history } - @players = 3.times.collect { FactoryGirl.create(:player) } + @players = Array.new(3) { FactoryGirl.create(:player) } @delete_ids = @players[0..1].collect(&:id) # NOTE: This uses an internal, unsupported capybara API which could break at any moment. We @@ -34,7 +34,7 @@ describe 'cancelled bulk_deletion' do before do - @players = 3.times.collect { FactoryGirl.create(:player) } + @players = Array.new(3) { FactoryGirl.create(:player) } @delete_ids = @players[0..1].collect(&:id) # NOTE: This uses an internal, unsupported capybara API which could break at any moment. We diff --git a/spec/integration/basic/create/rails_admin_basic_create_spec.rb b/spec/integration/basic/create/rails_admin_basic_create_spec.rb index 9e84b310fd..7becec3e15 100644 --- a/spec/integration/basic/create/rails_admin_basic_create_spec.rb +++ b/spec/integration/basic/create/rails_admin_basic_create_spec.rb @@ -75,7 +75,7 @@ describe 'create with has-many association' do before do - @divisions = 3.times.collect { Division.create!(name: "div #{Time.now.to_f}", league: League.create!(name: "league #{Time.now.to_f}")) } + @divisions = Array.new(3) { Division.create!(name: "div #{Time.now.to_f}", league: League.create!(name: "league #{Time.now.to_f}")) } post new_path(model_name: 'league', league: {name: 'National League', division_ids: [@divisions[0].id]}) @league = RailsAdmin::AbstractModel.new('League').all.to_a.last end @@ -90,7 +90,7 @@ describe 'create with has-and-belongs-to-many association' do before do - @teams = 3.times.collect { FactoryGirl.create :team } + @teams = Array.new(3) { FactoryGirl.create :team } post new_path(model_name: 'fan', fan: {name: 'John Doe', team_ids: [@teams[0].id]}) @fan = RailsAdmin::AbstractModel.new('Fan').first end diff --git a/spec/integration/basic/edit/rails_admin_basic_edit_spec.rb b/spec/integration/basic/edit/rails_admin_basic_edit_spec.rb index 023d187099..eac22151a3 100644 --- a/spec/integration/basic/edit/rails_admin_basic_edit_spec.rb +++ b/spec/integration/basic/edit/rails_admin_basic_edit_spec.rb @@ -68,7 +68,7 @@ describe 'edit with has-and-belongs-to-many association' do before do - @teams = 3.times.collect { FactoryGirl.create :team } + @teams = Array.new(3) { FactoryGirl.create :team } @fan = FactoryGirl.create :fan, teams: [@teams[0]] visit edit_path(model_name: 'fan', id: @fan.id) end @@ -95,7 +95,7 @@ describe 'edit with missing label', given: ['a player exists', 'three teams with no name exist'] do before do @player = FactoryGirl.create :player - @teams = 3.times.collect { FactoryGirl.create :team, name: '' } + @teams = Array.new(3) { FactoryGirl.create :team, name: '' } visit edit_path(model_name: 'player', id: @player.id) end end diff --git a/spec/integration/basic/export/rails_admin_basic_export_spec.rb b/spec/integration/basic/export/rails_admin_basic_export_spec.rb index 8ce7f06f75..8909a14baf 100644 --- a/spec/integration/basic/export/rails_admin_basic_export_spec.rb +++ b/spec/integration/basic/export/rails_admin_basic_export_spec.rb @@ -7,11 +7,11 @@ before do Comment.all.collect(&:destroy) # rspec bug => doesn't get destroyed with transaction - @players = 4.times.collect { FactoryGirl.create :player } + @players = Array.new(4) { FactoryGirl.create :player } @player = @players.first @player.team = FactoryGirl.create :team @player.draft = FactoryGirl.create :draft - @player.comments = (@comments = 2.times.collect { FactoryGirl.create(:comment) }) + @player.comments = (@comments = Array.new(2) { FactoryGirl.create(:comment) }) @player.save @abstract_model = RailsAdmin::AbstractModel.new(Player) diff --git a/spec/integration/basic/list/rails_admin_basic_list_spec.rb b/spec/integration/basic/list/rails_admin_basic_list_spec.rb index 42d4e93686..346a264345 100644 --- a/spec/integration/basic/list/rails_admin_basic_list_spec.rb +++ b/spec/integration/basic/list/rails_admin_basic_list_spec.rb @@ -55,7 +55,7 @@ describe 'GET /admin/player' do before do - @teams = 2.times.collect do + @teams = Array.new(2) do FactoryGirl.create(:team) end @players = [ @@ -300,7 +300,7 @@ describe 'GET /admin/player with 2 objects' do before do - @players = 2.times.collect { FactoryGirl.create :player } + @players = Array.new(2) { FactoryGirl.create :player } visit index_path(model_name: 'player') end @@ -311,7 +311,7 @@ describe 'GET /admin/player with 2 objects' do before do - @players = 2.times.collect { FactoryGirl.create :player } + @players = Array.new(2) { FactoryGirl.create :player } visit index_path(model_name: 'player') end @@ -338,7 +338,7 @@ describe 'list with 3 pages, page 3' do before do items_per_page = RailsAdmin.config.default_items_per_page - @players = (items_per_page * 3).times.collect { FactoryGirl.create(:player) } + @players = Array.new((items_per_page * 3)) { FactoryGirl.create(:player) } visit index_path(model_name: 'player', page: 3) end @@ -358,7 +358,7 @@ end it 'responds successfully with multiple models' do - 2.times.collect { FactoryGirl.create :player } + Array.new(2) { FactoryGirl.create :player } visit index_path(model_name: 'player', all: true) expect(find('div.total-count')).to have_content('2 players') end @@ -367,7 +367,7 @@ describe 'GET /admin/player show with pagination disabled by :associated_collection' do it 'responds successfully' do @team = FactoryGirl.create :team - 2.times.collect { FactoryGirl.create :player, team: @team } + Array.new(2) { FactoryGirl.create :player, team: @team } visit index_path(model_name: 'player', associated_collection: 'players', compact: true, current_action: 'update', source_abstract_model: 'team', source_object_id: @team.id) expect(find('div.total-count')).to have_content('2 players') end @@ -375,7 +375,7 @@ describe 'list as compact json' do it 'has_content an array with 2 elements and contain an array of elements with keys id and label' do - 2.times.collect { FactoryGirl.create :player } + Array.new(2) { FactoryGirl.create :player } get index_path(model_name: 'player', compact: true, format: :json) expect(ActiveSupport::JSON.decode(response.body).length).to eq(2) ActiveSupport::JSON.decode(response.body).each do |object| diff --git a/spec/integration/basic/update/rails_admin_basic_update_spec.rb b/spec/integration/basic/update/rails_admin_basic_update_spec.rb index ae200ea5b3..eb1c8f3735 100644 --- a/spec/integration/basic/update/rails_admin_basic_update_spec.rb +++ b/spec/integration/basic/update/rails_admin_basic_update_spec.rb @@ -87,7 +87,7 @@ end @league = FactoryGirl.create :league - @divisions = 3.times.collect { Division.create!(name: "div #{Time.now.to_f}", league: League.create!(name: "league #{Time.now.to_f}")) } + @divisions = Array.new(3) { Division.create!(name: "div #{Time.now.to_f}", league: League.create!(name: "league #{Time.now.to_f}")) } put edit_path(model_name: 'league', id: @league.id, league: {name: 'National League', division_ids: [@divisions[0].id]}) diff --git a/spec/rails_admin/config/fields/base_spec.rb b/spec/rails_admin/config/fields/base_spec.rb index 17b460f710..b6eafc8b68 100644 --- a/spec/rails_admin/config/fields/base_spec.rb +++ b/spec/rails_admin/config/fields/base_spec.rb @@ -178,7 +178,7 @@ class CommentReversed < Tableless end it 'defaults to false if associated collection count >= 100' do - @players = 100.times.collect do + @players = Array.new(100) do FactoryGirl.create :player end expect(RailsAdmin.config(Team).edit.fields.detect { |f| f.name == :players }.associated_collection_cache_all).to be_falsey @@ -189,14 +189,14 @@ class CommentReversed < Tableless RailsAdmin.config.default_associated_collection_limit = 5 end it 'defaults to true if associated collection count less than than limit' do - @players = 4.times.collect do + @players = Array.new(4) do FactoryGirl.create :player end expect(RailsAdmin.config(Team).edit.fields.detect { |f| f.name == :players }.associated_collection_cache_all).to be_truthy end it 'defaults to false if associated collection count >= that limit' do - @players = 5.times.collect do + @players = Array.new(5) do FactoryGirl.create :player end expect(RailsAdmin.config(Team).edit.fields.detect { |f| f.name == :players }.associated_collection_cache_all).to be_falsey From 8acbccd42298cb2c5c7a1ef297204fe407454430 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 17:07:04 +1000 Subject: [PATCH 25/32] Ignore ConstantName warnings --- spec/dummy_app/config/initializers/active_record_migration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/dummy_app/config/initializers/active_record_migration.rb b/spec/dummy_app/config/initializers/active_record_migration.rb index 353cadfa74..f74de6f242 100644 --- a/spec/dummy_app/config/initializers/active_record_migration.rb +++ b/spec/dummy_app/config/initializers/active_record_migration.rb @@ -1,5 +1,5 @@ if defined?(ActiveRecord) - MigrationBase = + MigrationBase = # rubocop:disable ConstantName if Rails.version >= '5.0' ActiveRecord::Migration[5.0] else From cd2cb3332c4412e718b818a9d3381df1c0819bac Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 17:17:37 +1000 Subject: [PATCH 26/32] Consistent alignment of opening and closing braces --- .../edit/rails_admin_config_edit_spec.rb | 6 ++-- .../show/rails_admin_config_show_spec.rb | 28 +++++++++++-------- spec/rails_admin/support/hash_helper_spec.rb | 17 +++++++---- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/spec/integration/config/edit/rails_admin_config_edit_spec.rb b/spec/integration/config/edit/rails_admin_config_edit_spec.rb index 708070e1ec..20665e2936 100644 --- a/spec/integration/config/edit/rails_admin_config_edit_spec.rb +++ b/spec/integration/config/edit/rails_admin_config_edit_spec.rb @@ -784,7 +784,8 @@ class HelpTest < Tableless expect(find('#field_test_nested_field_tests_attributes_0_title').value).to eq('nested title 1') is_expected.not_to have_selector('form .remove_nested_fields') expect(find('div#nested_field_tests_fields_blueprint', visible: false)[:'data-blueprint']).to match( - /]* class="remove_nested_fields"[^>]*>/) + /]* class="remove_nested_fields"[^>]*>/, + ) end end @@ -793,7 +794,8 @@ class HelpTest < Tableless visit new_path(model_name: 'field_test') is_expected.not_to have_selector('select#field_test_nested_field_tests_attributes_new_nested_field_tests_field_test_id') expect(find('div#nested_field_tests_fields_blueprint', visible: false)[:'data-blueprint']).to match( - /]* id="field_test_nested_field_tests_attributes_new_nested_field_tests_another_field_test_id"[^>]*>/) + /]* id="field_test_nested_field_tests_attributes_new_nested_field_tests_another_field_test_id"[^>]*>/, + ) end it 'hides fields that are deeply nested with inverse_of' do diff --git a/spec/integration/config/show/rails_admin_config_show_spec.rb b/spec/integration/config/show/rails_admin_config_show_spec.rb index 9b9ca54ed5..6ed1a33a9f 100644 --- a/spec/integration/config/show/rails_admin_config_show_spec.rb +++ b/spec/integration/config/show/rails_admin_config_show_spec.rb @@ -88,9 +88,10 @@ def do_request is_expected.not_to have_selector('h4', text: 'Basic info') - %w(division name logo_url manager - ballpark mascot founded wins - losses win_percentage revenue + %w( + division name logo_url manager + ballpark mascot founded wins + losses win_percentage revenue ).each do |field| is_expected.not_to have_selector(".#{field}_field") end @@ -186,9 +187,10 @@ def do_request it 'shows all by default' do do_request - %w(division name logo_url manager - ballpark mascot founded wins - losses win_percentage revenue players fans + %w( + division name logo_url manager + ballpark mascot founded wins + losses win_percentage revenue players fans ).each do |field| is_expected.to have_selector(".#{field}_field") end @@ -253,9 +255,10 @@ def do_request do_request - ['Division', 'Name (STRING)', 'Logo url (STRING)', 'Manager (STRING)', - 'Ballpark (STRING)', 'Mascot (STRING)', 'Founded', 'Wins', 'Losses', - 'Win percentage', 'Revenue', 'Players', 'Fans' + [ + 'Division', 'Name (STRING)', 'Logo url (STRING)', 'Manager (STRING)', + 'Ballpark (STRING)', 'Mascot (STRING)', 'Founded', 'Wins', 'Losses', + 'Win percentage', 'Revenue', 'Players', 'Fans' ].each do |text| is_expected.to have_selector('.label', text: text) end @@ -272,9 +275,10 @@ def do_request do_request - ['Division', 'Name (STRING)', 'Logo url (STRING)', 'Manager (STRING)', - 'Ballpark (STRING)', 'Mascot (STRING)', 'Founded', 'Wins', 'Losses', - 'Win percentage', 'Revenue', 'Players', 'Fans' + [ + 'Division', 'Name (STRING)', 'Logo url (STRING)', 'Manager (STRING)', + 'Ballpark (STRING)', 'Mascot (STRING)', 'Founded', 'Wins', 'Losses', + 'Win percentage', 'Revenue', 'Players', 'Fans' ].each do |text| is_expected.to have_selector('.label', text: text) end diff --git a/spec/rails_admin/support/hash_helper_spec.rb b/spec/rails_admin/support/hash_helper_spec.rb index f26fa5357f..bbe39cd990 100644 --- a/spec/rails_admin/support/hash_helper_spec.rb +++ b/spec/rails_admin/support/hash_helper_spec.rb @@ -4,12 +4,17 @@ describe RailsAdmin::HashHelper do let(:hash) do - {'subject' => 'Test', - 'user' => {name: 'Dirk', - 'title' => 'Holistic Detective', - 'clients' => [ - {name: 'Zaphod'}, - {'name' => 'Arthur'}]}} + { + 'subject' => 'Test', + 'user' => { + name: 'Dirk', + 'title' => 'Holistic Detective', + 'clients' => [ + {name: 'Zaphod'}, + {'name' => 'Arthur'}, + ], + }, + } end describe 'symbolize' do From d91d813a2dff3c0f2387b2fa7230c9ee50391177 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 17:20:21 +1000 Subject: [PATCH 27/32] Variable incorrectly defined like a constant is actually mutable --- spec/rails_admin/config_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/rails_admin/config_spec.rb b/spec/rails_admin/config_spec.rb index 96b86260e8..084d42c4dd 100644 --- a/spec/rails_admin/config_spec.rb +++ b/spec/rails_admin/config_spec.rb @@ -36,7 +36,7 @@ describe '.add_extension' do before do silence_warnings do - RailsAdmin::EXTENSIONS = [].freeze + RailsAdmin::EXTENSIONS = [] # rubocop:disable MutableConstant end end From 2d66c8c20a2b1758abed2cc2785f857b6b448e0c Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 17:27:16 +1000 Subject: [PATCH 28/32] Remove parentheses around method calls --- lib/rails_admin/adapters/active_record.rb | 2 +- lib/rails_admin/config/fields/base.rb | 2 +- lib/rails_admin/config/fields/factories/carrierwave.rb | 2 +- lib/rails_admin/config/hideable.rb | 2 +- spec/dummy_app/db/seeds.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/rails_admin/adapters/active_record.rb b/lib/rails_admin/adapters/active_record.rb index b458e8b543..999fe65ee3 100644 --- a/lib/rails_admin/adapters/active_record.rb +++ b/lib/rails_admin/adapters/active_record.rb @@ -116,7 +116,7 @@ def add(field, value, operator) def build scope = @scope.where(@statements.join(' OR '), *@values) - scope = scope.references(*(@tables.uniq)) if @tables.any? + scope = scope.references(*@tables.uniq) if @tables.any? scope end end diff --git a/lib/rails_admin/config/fields/base.rb b/lib/rails_admin/config/fields/base.rb index 889edda916..2b3c2798a7 100644 --- a/lib/rails_admin/config/fields/base.rb +++ b/lib/rails_admin/config/fields/base.rb @@ -255,7 +255,7 @@ def optional? # @see RailsAdmin::Config::Fields::Base.register_instance_option :required? def optional(state = nil, &block) if !state.nil? || block - required state.nil? ? proc { false == (instance_eval(&block)) } : false == state + required state.nil? ? proc { false == instance_eval(&block) } : false == state else optional? end diff --git a/lib/rails_admin/config/fields/factories/carrierwave.rb b/lib/rails_admin/config/fields/factories/carrierwave.rb index 7b1c1614b6..8491ae7f03 100644 --- a/lib/rails_admin/config/fields/factories/carrierwave.rb +++ b/lib/rails_admin/config/fields/factories/carrierwave.rb @@ -4,7 +4,7 @@ RailsAdmin::Config::Fields.register_factory do |parent, properties, fields| model = parent.abstract_model.model - if defined?(::CarrierWave) && (model).is_a?(CarrierWave::Mount) && model.uploaders.include?(attachment_name = properties.name.to_s.chomp('_file_name').to_sym) + if defined?(::CarrierWave) && model.is_a?(CarrierWave::Mount) && model.uploaders.include?(attachment_name = properties.name.to_s.chomp('_file_name').to_sym) columns = [model.uploader_options[attachment_name][:mount_on] || attachment_name, "#{attachment_name}_content_type".to_sym, "#{attachment_name}_file_size".to_sym] field = RailsAdmin::Config::Fields::Types.load(:carrierwave).new(parent, attachment_name, properties) fields << field diff --git a/lib/rails_admin/config/hideable.rb b/lib/rails_admin/config/hideable.rb index 85e889e671..8f8f699ca6 100644 --- a/lib/rails_admin/config/hideable.rb +++ b/lib/rails_admin/config/hideable.rb @@ -16,7 +16,7 @@ def hidden? # Writer to hide object. def hide(&block) - visible block ? proc { false == (instance_eval(&block)) } : false + visible block ? proc { false == instance_eval(&block) } : false end # Writer to show field. diff --git a/spec/dummy_app/db/seeds.rb b/spec/dummy_app/db/seeds.rb index 0152a19c9a..0c1834fa93 100644 --- a/spec/dummy_app/db/seeds.rb +++ b/spec/dummy_app/db/seeds.rb @@ -18,7 +18,7 @@ division.save! end unless team = team_model.where(name: mlb_team.name).first - team = team_model.model.new(name: mlb_team.name, logo_url: mlb_team.logo_url, manager: mlb_team.manager || 'None', ballpark: mlb_team.ballpark, mascot: mlb_team.mascot, founded: mlb_team.founded, wins: mlb_team.wins, losses: mlb_team.losses, win_percentage: (format('%.3f', (mlb_team.wins.to_f / (mlb_team.wins + mlb_team.losses)))).to_f, division: division) + team = team_model.model.new(name: mlb_team.name, logo_url: mlb_team.logo_url, manager: mlb_team.manager || 'None', ballpark: mlb_team.ballpark, mascot: mlb_team.mascot, founded: mlb_team.founded, wins: mlb_team.wins, losses: mlb_team.losses, win_percentage: format('%.3f', (mlb_team.wins.to_f / (mlb_team.wins + mlb_team.losses))).to_f, division: division) team.save! end mlb_team.players.reject { |player| player.number.nil? }.each do |player| From 72edcb9abbf09783f017aa39336e2babc86f50bf Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 17:42:19 +1000 Subject: [PATCH 29/32] Update rubocop for compatibility with Code Climate --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 9858984a37..50ed340519 100644 --- a/Gemfile +++ b/Gemfile @@ -43,7 +43,7 @@ group :test do gem 'pundit' gem 'rack-cache', require: 'rack/cache' gem 'rspec-rails', '>= 2.14' - gem 'rubocop', '~> 0.40.0' + gem 'rubocop', '~> 0.41.2' gem 'simplecov', '>= 0.9', require: false gem 'timecop', '>= 0.5' From 5a960f734843148c2ddc7093a74fdd6960831e62 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Wed, 27 Jul 2016 18:02:45 +1000 Subject: [PATCH 30/32] Update rubocop in gemfiles so CI is in sync with Code Climate --- gemfiles/rails_4.0.gemfile | 2 +- gemfiles/rails_4.1.gemfile | 2 +- gemfiles/rails_4.2.gemfile | 2 +- gemfiles/rails_5.0.gemfile | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gemfiles/rails_4.0.gemfile b/gemfiles/rails_4.0.gemfile index e5e8012930..21c7c5484a 100644 --- a/gemfiles/rails_4.0.gemfile +++ b/gemfiles/rails_4.0.gemfile @@ -49,7 +49,7 @@ group :test do gem "pundit" gem "rack-cache", :require => "rack/cache" gem "rspec-rails", ">= 2.14" - gem "rubocop", "~> 0.31.0" + gem "rubocop", "~> 0.41.2" gem "simplecov", ">= 0.9", :require => false gem "timecop", ">= 0.5" diff --git a/gemfiles/rails_4.1.gemfile b/gemfiles/rails_4.1.gemfile index 4d655cd400..cc01b69436 100644 --- a/gemfiles/rails_4.1.gemfile +++ b/gemfiles/rails_4.1.gemfile @@ -47,7 +47,7 @@ group :test do gem "pundit" gem "rack-cache", :require => "rack/cache" gem "rspec-rails", ">= 2.14" - gem "rubocop", "~> 0.31.0" + gem "rubocop", "~> 0.41.2" gem "simplecov", ">= 0.9", :require => false gem "timecop", ">= 0.5" diff --git a/gemfiles/rails_4.2.gemfile b/gemfiles/rails_4.2.gemfile index a18e1919ad..ad9b04b0b0 100644 --- a/gemfiles/rails_4.2.gemfile +++ b/gemfiles/rails_4.2.gemfile @@ -48,7 +48,7 @@ group :test do gem "pundit" gem "rack-cache", :require => "rack/cache" gem "rspec-rails", ">= 2.14" - gem "rubocop", "~> 0.31.0" + gem "rubocop", "~> 0.41.2" gem "simplecov", ">= 0.9", :require => false gem "timecop", ">= 0.5" diff --git a/gemfiles/rails_5.0.gemfile b/gemfiles/rails_5.0.gemfile index 3a58dcfaf8..114127fc77 100644 --- a/gemfiles/rails_5.0.gemfile +++ b/gemfiles/rails_5.0.gemfile @@ -47,7 +47,7 @@ group :test do gem "pundit" gem "rack-cache", :require => "rack/cache" gem "rspec-rails", ">= 2.14" - gem "rubocop", "~> 0.31.0" + gem "rubocop", "~> 0.41.2" gem "simplecov", ">= 0.9", :require => false gem "timecop", ">= 0.5" From 3fcf5d920d3417afdcb0c144e15e61fb7e4cc372 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Fri, 29 Jul 2016 12:22:06 +1000 Subject: [PATCH 31/32] Replace !empty? with any? --- lib/rails_admin/config/fields/types/polymorphic_association.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rails_admin/config/fields/types/polymorphic_association.rb b/lib/rails_admin/config/fields/types/polymorphic_association.rb index b3fac30941..101c8bc3dc 100644 --- a/lib/rails_admin/config/fields/types/polymorphic_association.rb +++ b/lib/rails_admin/config/fields/types/polymorphic_association.rb @@ -16,7 +16,7 @@ class PolymorphicAssociation < RailsAdmin::Config::Fields::Types::BelongsToAssoc # association checks that any of the child models are included in # configuration. register_instance_option :visible? do - !associated_model_config.empty? + associated_model_config.any? end register_instance_option :formatted_value do From c02ca4ef9617d616bfb123cc28cdf13d5f155c53 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Fri, 29 Jul 2016 12:23:15 +1000 Subject: [PATCH 32/32] Replace Array.new(\d) with FactoryGirl.create_list --- .../rails_admin/main_controller_spec.rb | 30 ++++++------------- .../rails_admin_basic_bulk_action_spec.rb | 2 +- .../create/rails_admin_basic_create_spec.rb | 2 +- .../basic/edit/rails_admin_basic_edit_spec.rb | 2 +- .../export/rails_admin_basic_export_spec.rb | 2 +- .../basic/list/rails_admin_basic_list_spec.rb | 8 ++--- 6 files changed, 17 insertions(+), 29 deletions(-) diff --git a/spec/controllers/rails_admin/main_controller_spec.rb b/spec/controllers/rails_admin/main_controller_spec.rb index b82f64b56c..fe2ef8e210 100644 --- a/spec/controllers/rails_admin/main_controller_spec.rb +++ b/spec/controllers/rails_admin/main_controller_spec.rb @@ -113,7 +113,7 @@ def get(action, params) describe '#list_entries called from view' do before do - @teams = Array.new(21) { FactoryGirl.create :team } + @teams = FactoryGirl.create_list(:team, 21) controller.params = {model_name: 'teams'} end @@ -125,7 +125,7 @@ def get(action, params) describe '#list_entries called from view with kaminari custom param_name' do before do - @teams = Array.new(21) { FactoryGirl.create :team } + @teams = FactoryGirl.create_list(:team, 21) controller.params = {model_name: 'teams'} Kaminari.config.param_name = :pagina end @@ -142,7 +142,7 @@ def get(action, params) describe '#list_entries called with bulk_ids' do before do - @teams = Array.new(21) { FactoryGirl.create :team } + @teams = FactoryGirl.create_list(:team, 21) controller.params = {model_name: 'teams', bulk_action: 'bulk_delete', bulk_ids: @teams.collect(&:id)} end @@ -159,9 +159,7 @@ def get(action, params) end it "doesn't scope associated collection records when associated_collection_scope is nil" do - @players = Array.new(2) do - FactoryGirl.create :player - end + @players = FactoryGirl.create_list(:player, 2) RailsAdmin.config Team do field :players do @@ -173,9 +171,7 @@ def get(action, params) end it 'scopes associated collection records according to associated_collection_scope' do - @players = Array.new(4) do - FactoryGirl.create :player - end + @players = FactoryGirl.create_list(:player, 4) RailsAdmin.config Team do field :players do @@ -192,9 +188,7 @@ def get(action, params) @team.revenue = BigDecimal.new('3') @team.save - @players = Array.new(5) do - FactoryGirl.create :player - end + @players = FactoryGirl.create_list(:player, 5) RailsAdmin.config Team do field :players do @@ -211,9 +205,7 @@ def get(action, params) end it 'limits associated collection records number to 30 if cache_all is false' do - @players = Array.new(40) do - FactoryGirl.create :player - end + @players = FactoryGirl.create_list(:player, 40) RailsAdmin.config Team do field :players do @@ -224,9 +216,7 @@ def get(action, params) end it "doesn't limit associated collection records number to 30 if cache_all is true" do - @players = Array.new(40) do - FactoryGirl.create :player - end + @players = FactoryGirl.create_list(:player, 40) RailsAdmin.config Team do field :players do @@ -237,9 +227,7 @@ def get(action, params) end it 'orders associated collection records by id, descending' do - @players = Array.new(3) do - FactoryGirl.create :player - end + @players = FactoryGirl.create_list(:player, 3) expect(controller.list_entries.to_a).to eq(@players.sort_by(&:id).reverse) end diff --git a/spec/integration/basic/bulk_action/rails_admin_basic_bulk_action_spec.rb b/spec/integration/basic/bulk_action/rails_admin_basic_bulk_action_spec.rb index ffa16abee5..78b0708c7d 100644 --- a/spec/integration/basic/bulk_action/rails_admin_basic_bulk_action_spec.rb +++ b/spec/integration/basic/bulk_action/rails_admin_basic_bulk_action_spec.rb @@ -4,7 +4,7 @@ subject { page } before do - @players = Array.new(2) { FactoryGirl.create :player } + @players = FactoryGirl.create_list(:player, 2) end describe 'bulk_delete' do diff --git a/spec/integration/basic/create/rails_admin_basic_create_spec.rb b/spec/integration/basic/create/rails_admin_basic_create_spec.rb index 7becec3e15..c9f09b774a 100644 --- a/spec/integration/basic/create/rails_admin_basic_create_spec.rb +++ b/spec/integration/basic/create/rails_admin_basic_create_spec.rb @@ -90,7 +90,7 @@ describe 'create with has-and-belongs-to-many association' do before do - @teams = Array.new(3) { FactoryGirl.create :team } + @teams = FactoryGirl.create_list(:team, 3) post new_path(model_name: 'fan', fan: {name: 'John Doe', team_ids: [@teams[0].id]}) @fan = RailsAdmin::AbstractModel.new('Fan').first end diff --git a/spec/integration/basic/edit/rails_admin_basic_edit_spec.rb b/spec/integration/basic/edit/rails_admin_basic_edit_spec.rb index eac22151a3..7362842300 100644 --- a/spec/integration/basic/edit/rails_admin_basic_edit_spec.rb +++ b/spec/integration/basic/edit/rails_admin_basic_edit_spec.rb @@ -68,7 +68,7 @@ describe 'edit with has-and-belongs-to-many association' do before do - @teams = Array.new(3) { FactoryGirl.create :team } + @teams = FactoryGirl.create_list(:team, 3) @fan = FactoryGirl.create :fan, teams: [@teams[0]] visit edit_path(model_name: 'fan', id: @fan.id) end diff --git a/spec/integration/basic/export/rails_admin_basic_export_spec.rb b/spec/integration/basic/export/rails_admin_basic_export_spec.rb index 8909a14baf..75765409f2 100644 --- a/spec/integration/basic/export/rails_admin_basic_export_spec.rb +++ b/spec/integration/basic/export/rails_admin_basic_export_spec.rb @@ -7,7 +7,7 @@ before do Comment.all.collect(&:destroy) # rspec bug => doesn't get destroyed with transaction - @players = Array.new(4) { FactoryGirl.create :player } + @players = FactoryGirl.create_list(:player, 4) @player = @players.first @player.team = FactoryGirl.create :team @player.draft = FactoryGirl.create :draft diff --git a/spec/integration/basic/list/rails_admin_basic_list_spec.rb b/spec/integration/basic/list/rails_admin_basic_list_spec.rb index 346a264345..2d5ab40dcd 100644 --- a/spec/integration/basic/list/rails_admin_basic_list_spec.rb +++ b/spec/integration/basic/list/rails_admin_basic_list_spec.rb @@ -300,7 +300,7 @@ describe 'GET /admin/player with 2 objects' do before do - @players = Array.new(2) { FactoryGirl.create :player } + @players = FactoryGirl.create_list(:player, 2) visit index_path(model_name: 'player') end @@ -311,7 +311,7 @@ describe 'GET /admin/player with 2 objects' do before do - @players = Array.new(2) { FactoryGirl.create :player } + @players = FactoryGirl.create_list(:player, 2) visit index_path(model_name: 'player') end @@ -358,7 +358,7 @@ end it 'responds successfully with multiple models' do - Array.new(2) { FactoryGirl.create :player } + FactoryGirl.create_list(:player, 2) visit index_path(model_name: 'player', all: true) expect(find('div.total-count')).to have_content('2 players') end @@ -375,7 +375,7 @@ describe 'list as compact json' do it 'has_content an array with 2 elements and contain an array of elements with keys id and label' do - Array.new(2) { FactoryGirl.create :player } + FactoryGirl.create_list(:player, 2) get index_path(model_name: 'player', compact: true, format: :json) expect(ActiveSupport::JSON.decode(response.body).length).to eq(2) ActiveSupport::JSON.decode(response.body).each do |object|