From 3ef96570dd2daf0eac38e40b89bdc7bcdd1b2afa Mon Sep 17 00:00:00 2001 From: Roel van Dijk Date: Wed, 10 Aug 2016 13:17:02 +0200 Subject: [PATCH 1/6] Refactor class_eval string usage. --- lib/acts_as_list/active_record/acts/list.rb | 124 ++++++++++---------- 1 file changed, 64 insertions(+), 60 deletions(-) diff --git a/lib/acts_as_list/active_record/acts/list.rb b/lib/acts_as_list/active_record/acts/list.rb index 90deffab..cfb8c0c0 100644 --- a/lib/acts_as_list/active_record/acts/list.rb +++ b/lib/acts_as_list/active_record/acts/list.rb @@ -42,102 +42,106 @@ def acts_as_list(options = {}) configuration[:scope] = :"#{configuration[:scope]}_id" end - if configuration[:scope].is_a?(Symbol) - scope_methods = %( - def scope_condition - { #{configuration[:scope]}: send(:#{configuration[:scope]}) } - end - - def scope_changed? - changed.include?(scope_name.to_s) - end - ) - elsif configuration[:scope].is_a?(Array) - scope_methods = %( - def scope_condition - #{configuration[:scope]}.inject({}) do |hash, column| - hash.merge!({ column.to_sym => read_attribute(column.to_sym) }) - end - end + class_calling_acts_as_list = self - def scope_changed? - (scope_condition.keys & changed.map(&:to_sym)).any? - end - ) - else - scope_methods = %( - def scope_condition - "#{configuration[:scope]}" - end - - def scope_changed?() false end - ) - end - - quoted_position_column = connection.quote_column_name(configuration[:column]) - quoted_position_column_with_table_name = "#{quoted_table_name}.#{quoted_position_column}" - - class_eval <<-EOV, __FILE__, __LINE__ + 1 - def self.acts_as_list_top - #{configuration[:top_of_list]}.to_i + class_eval do + define_singleton_method :acts_as_list_top do + configuration[:top_of_list].to_i end - def acts_as_list_top - #{configuration[:top_of_list]}.to_i + define_method :acts_as_list_top do + configuration[:top_of_list].to_i end - def acts_as_list_class - ::#{self.name} + define_method :acts_as_list_class do + class_calling_acts_as_list end - def position_column - '#{configuration[:column]}' + define_method :position_column do + configuration[:column] end - def scope_name - '#{configuration[:scope]}' + define_method :scope_name do + configuration[:scope] end - def add_new_at - '#{configuration[:add_new_at]}' + define_method :add_new_at do + configuration[:add_new_at] end - def #{configuration[:column]}=(position) - write_attribute(:#{configuration[:column]}, position) + define_method :"#{configuration[:column]}=" do |position| + write_attribute(configuration[:column], position) @position_changed = true end - #{scope_methods} + if configuration[:scope].is_a?(Symbol) + define_method :scope_condition do + { configuration[:scope] => send(:"#{configuration[:scope]}") } + end + + define_method :scope_changed? do + changed.include?(scope_name.to_s) + end + elsif configuration[:scope].is_a?(Array) + define_method :scope_condition do + configuration[:scope].inject({}) do |hash, column| + hash.merge!({ column.to_sym => read_attribute(column.to_sym) }) + end + end + + define_method :scope_changed? do + (scope_condition.keys & changed.map(&:to_sym)).any? + end + else + define_method :scope_condition do + eval "%{#{configuration[:scope]}}" + end + + define_method :scope_changed? do + false + end + end # only add to attr_accessible # if the class has some mass_assignment_protection - if defined?(accessible_attributes) and !accessible_attributes.blank? - attr_accessible :#{configuration[:column]} + attr_accessible :"#{configuration[:column]}" end - scope :in_list, lambda { where(%q{#{quoted_position_column_with_table_name} IS NOT NULL}) } + define_singleton_method :quoted_table_name do + @_quoted_table_name ||= acts_as_list_class.quoted_table_name + end - def self.decrement_all - update_all_with_touch %q(#{quoted_position_column} = (#{quoted_position_column_with_table_name} - 1)) + define_singleton_method :quoted_position_column do + connection.quote_column_name(configuration[:column]) end - def self.increment_all - update_all_with_touch %q(#{quoted_position_column} = (#{quoted_position_column_with_table_name} + 1)) + define_singleton_method :quoted_position_column_with_table_name do + @_quoted_position_column_with_table_name ||= "#{quoted_table_name}.#{quoted_position_column}" end - def self.update_all_with_touch(updates) + scope :in_list, lambda { where("#{quoted_position_column_with_table_name} IS NOT NULL") } + + define_singleton_method :decrement_all do + update_all_with_touch "#{quoted_position_column} = (#{quoted_position_column_with_table_name} - 1)" + end + + define_singleton_method :increment_all do + update_all_with_touch "#{quoted_position_column} = (#{quoted_position_column_with_table_name} + 1)" + end + + define_singleton_method :update_all_with_touch do |updates| record = new attrs = record.send(:timestamp_attributes_for_update_in_model) now = record.send(:current_time_from_proper_timezone) - query = attrs.map { |attr| %(\#{connection.quote_column_name(attr)} = :now) } + query = attrs.map { |attr| "#{connection.quote_column_name(attr)} = :now" } query.push updates query = query.join(", ") update_all([query, now: now]) end - EOV + end attr_reader :position_changed From 087f20fa4d41067468adca0c59acc0492630f62d Mon Sep 17 00:00:00 2001 From: Roel van Dijk Date: Wed, 10 Aug 2016 13:31:45 +0200 Subject: [PATCH 2/6] Cache quoted position column. --- lib/acts_as_list/active_record/acts/list.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/acts_as_list/active_record/acts/list.rb b/lib/acts_as_list/active_record/acts/list.rb index cfb8c0c0..6366aa94 100644 --- a/lib/acts_as_list/active_record/acts/list.rb +++ b/lib/acts_as_list/active_record/acts/list.rb @@ -113,7 +113,7 @@ def acts_as_list(options = {}) end define_singleton_method :quoted_position_column do - connection.quote_column_name(configuration[:column]) + @_quoted_position_column ||= connection.quote_column_name(configuration[:column]) end define_singleton_method :quoted_position_column_with_table_name do From 020accac27d769331cd5d83afb4f30b482d12ba2 Mon Sep 17 00:00:00 2001 From: Roel van Dijk Date: Wed, 10 Aug 2016 14:47:33 +0200 Subject: [PATCH 3/6] Force use of Rack < 2.0, which should fix Travis build. --- Gemfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Gemfile b/Gemfile index db32d8fb..61e8aace 100644 --- a/Gemfile +++ b/Gemfile @@ -16,6 +16,8 @@ gemspec gem "rake" gem "appraisal" gem "github_changelog_generator", "1.9.0" +# Rack 2.0.1 breaks the build for Ruby < 2.2.2 +gem "rack", "< 2.0" group :test do gem "minitest", "~> 5.0" From 3cb3623e97cbf8985536c5c498c30451917b6512 Mon Sep 17 00:00:00 2001 From: Roel van Dijk Date: Thu, 11 Aug 2016 08:40:36 +0200 Subject: [PATCH 4/6] Fix automerged Gemfile. --- Gemfile | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 61e8aace..821a9f95 100644 --- a/Gemfile +++ b/Gemfile @@ -16,10 +16,8 @@ gemspec gem "rake" gem "appraisal" gem "github_changelog_generator", "1.9.0" -# Rack 2.0.1 breaks the build for Ruby < 2.2.2 -gem "rack", "< 2.0" group :test do - gem "minitest", "~> 5.0" + gem "minitest", "~> 5.0" gem "test_after_commit", "~> 0.4.2" end From a6a620c1d8acfa929dc0924b490d09aba823d3c8 Mon Sep 17 00:00:00 2001 From: Roel van Dijk Date: Fri, 26 Aug 2016 20:29:17 +0200 Subject: [PATCH 5/6] Renamed variable after PR review. --- lib/acts_as_list/active_record/acts/list.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/acts_as_list/active_record/acts/list.rb b/lib/acts_as_list/active_record/acts/list.rb index 6366aa94..83cb6734 100644 --- a/lib/acts_as_list/active_record/acts/list.rb +++ b/lib/acts_as_list/active_record/acts/list.rb @@ -42,7 +42,7 @@ def acts_as_list(options = {}) configuration[:scope] = :"#{configuration[:scope]}_id" end - class_calling_acts_as_list = self + caller_class = self class_eval do define_singleton_method :acts_as_list_top do @@ -54,7 +54,7 @@ def acts_as_list(options = {}) end define_method :acts_as_list_class do - class_calling_acts_as_list + caller_class end define_method :position_column do From 57923ee108719a7fd3686858ddabe45a1ccb526f Mon Sep 17 00:00:00 2001 From: Roel van Dijk Date: Fri, 26 Aug 2016 21:48:26 +0200 Subject: [PATCH 6/6] Remove unnecessary method introduced during rebase. --- lib/acts_as_list/active_record/acts/list.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/acts_as_list/active_record/acts/list.rb b/lib/acts_as_list/active_record/acts/list.rb index 83cb6734..bacbe839 100644 --- a/lib/acts_as_list/active_record/acts/list.rb +++ b/lib/acts_as_list/active_record/acts/list.rb @@ -108,16 +108,12 @@ def acts_as_list(options = {}) attr_accessible :"#{configuration[:column]}" end - define_singleton_method :quoted_table_name do - @_quoted_table_name ||= acts_as_list_class.quoted_table_name - end - define_singleton_method :quoted_position_column do @_quoted_position_column ||= connection.quote_column_name(configuration[:column]) end define_singleton_method :quoted_position_column_with_table_name do - @_quoted_position_column_with_table_name ||= "#{quoted_table_name}.#{quoted_position_column}" + @_quoted_position_column_with_table_name ||= "#{caller_class.quoted_table_name}.#{quoted_position_column}" end scope :in_list, lambda { where("#{quoted_position_column_with_table_name} IS NOT NULL") }