From 1f6175d8a45f7ae823bf783e366ca271d29c9d00 Mon Sep 17 00:00:00 2001 From: Ilkham Gaysin Date: Fri, 3 Mar 2017 03:22:55 +0300 Subject: [PATCH] acts_as_list_no_update allows one to supply additional classes (#260) --- README.md | 41 +++++-- .../active_record/acts/no_update.rb | 86 ++++++++++++--- test/helper.rb | 27 ++++- test/test_joined_list.rb | 13 +-- test/test_list.rb | 29 +---- test/test_no_update_for_extra_classes.rb | 104 ++++++++++++++++++ 6 files changed, 240 insertions(+), 60 deletions(-) create mode 100644 test/test_no_update_for_extra_classes.rb diff --git a/README.md b/README.md index 9476009c..4b7898fa 100644 --- a/README.md +++ b/README.md @@ -27,27 +27,27 @@ At first, you need to add a `position` column to desired table: rails g migration AddPositionToTodoItem position:integer rake db:migrate - -After that you can use `acts_as_list` method in the model: + +After that you can use `acts_as_list` method in the model: ```ruby class TodoList < ActiveRecord::Base has_many :todo_items, -> { order(position: :asc) } end - + class TodoItem < ActiveRecord::Base belongs_to :todo_list acts_as_list scope: :todo_list end -todo_list = TodoList.find(...) +todo_list = TodoList.find(...) todo_list.todo_items.first.move_to_bottom todo_list.todo_items.last.move_higher ``` ## Instance Methods Added To ActiveRecord Models -You'll have a number of methods added to each instance of the ActiveRecord model that to which `acts_as_list` is added. +You'll have a number of methods added to each instance of the ActiveRecord model that to which `acts_as_list` is added. In `acts_as_list`, "higher" means further up the list (a lower `position`), and "lower" means further down the list (a higher `position`). That can be confusing, so it might make sense to add tests that validate that you're using the right method given your context. @@ -132,7 +132,34 @@ TodoItem.acts_as_list_no_update do end ``` In an `acts_as_list_no_update` block, all callbacks are disabled, and positions are not updated. New records will be created with - the default value from the database. It is your responsibility to correctly manage `positions` values. + the default value from the database. It is your responsibility to correctly manage `positions` values. + +You can also pass an array of classes as an argument to disable database updates on just those classes. It can be any ActiveRecord class that has acts_as_list enabled. +```ruby +class TodoList < ActiveRecord::Base + has_many :todo_items, -> { order(position: :asc) } + acts_as_list +end + +class TodoItem < ActiveRecord::Base + belongs_to :todo_list + has_many :todo_attachments, -> { order(position: :asc) } + + acts_as_list scope: :todo_list +end + +class TodoAttachment < ActiveRecord::Base + belongs_to :todo_list + acts_as_list scope: :todo_item +end + +TodoItem.acts_as_list_no_update([TodoAttachment]) do + TodoItem.find(10).update(position: 2) + TodoAttachment.find(10).update(position: 1) + TodoAttachment.find(11).update(position: 2) + TodoList.find(2).update(position: 3) # For this instance the callbacks will be called because we haven't passed the class as an argument +end +``` ## Versions Version `0.9.0` adds `acts_as_list_no_update` (https://github.com/swanandp/acts_as_list/pull/244) and compatibility with not-null and uniqueness constraints on the database (https://github.com/swanandp/acts_as_list/pull/246). These additions shouldn't break compatibility with existing implementations. @@ -152,7 +179,7 @@ All versions `0.1.5` onwards require Rails 3.0.x and higher. 1. Sort based feature ## Contributing to `acts_as_list` - + - Check out the latest master to make sure the feature hasn't been implemented or the bug hasn't been fixed yet - Check out the issue tracker to make sure someone already hasn't requested it and/or contributed it - Fork the project diff --git a/lib/acts_as_list/active_record/acts/no_update.rb b/lib/acts_as_list/active_record/acts/no_update.rb index 140bb0aa..fcbb202e 100644 --- a/lib/acts_as_list/active_record/acts/no_update.rb +++ b/lib/acts_as_list/active_record/acts/no_update.rb @@ -4,39 +4,99 @@ module List module NoUpdate extend ActiveSupport::Concern + class ArrayTypeError < SyntaxError + def initialize + super("The first argument must be an array") + end + end + + class DisparityClassesError < NotImplementedError + def initialize + super("The first argument should contain ActiveRecord or ApplicationRecord classes") + end + end + module ClassMethods # Lets you selectively disable all act_as_list database updates # for the duration of a block. # # ==== Examples - # ActiveRecord::Acts::List.acts_as_list_no_update do - # TodoList.... - # end # - # TodoList.acts_as_list_no_update do - # TodoList.... - # end + # class TodoList < ActiveRecord::Base + # has_many :todo_items, -> { order(position: :asc) } + # end + # + # class TodoItem < ActiveRecord::Base + # belongs_to :todo_list + # + # acts_as_list scope: :todo_list + # end + # + # TodoItem.acts_as_list_no_update do + # TodoList.first.update(position: 2) + # end + # + # You can also pass an array of classes as an argument to disable database updates on just those classes. + # It can be any ActiveRecord class that has acts_as_list enabled. + # + # ==== Examples + # + # class TodoList < ActiveRecord::Base + # has_many :todo_items, -> { order(position: :asc) } + # acts_as_list + # end + # + # class TodoItem < ActiveRecord::Base + # belongs_to :todo_list + # has_many :todo_attachments, -> { order(position: :asc) } + # + # acts_as_list scope: :todo_list + # end # - def acts_as_list_no_update(&block) - NoUpdate.apply_to(self, &block) + # class TodoAttachment < ActiveRecord::Base + # belongs_to :todo_list + # acts_as_list scope: :todo_item + # end + # + # TodoItem.acts_as_list_no_update([TodoAttachment]) do + # TodoItem.find(10).update(position: 2) + # TodoAttachment.find(10).update(position: 1) + # TodoAttachment.find(11).update(position: 2) + # TodoList.find(2).update(position: 3) # For this instance the callbacks will be called because we haven't passed the class as an argument + # end + + def acts_as_list_no_update(extra_classes = [], &block) + return raise ArrayTypeError unless extra_classes.is_a?(Array) + + extra_classes << self + + return raise DisparityClassesError unless active_record_objects?(extra_classes) + + NoUpdate.apply_to(extra_classes, &block) + end + + private + + def active_record_objects?(extra_classes) + extra_classes.all? { |klass| klass.ancestors.include? ActiveRecord::Base } end end class << self - def apply_to(klass) - klasses.push(klass) + def apply_to(klasses) + extracted_klasses.push(*klasses) yield ensure - klasses.pop + extracted_klasses.clear end def applied_to?(klass) - klasses.any? { |k| k >= klass } + extracted_klasses.any? { |k| k == klass } end private - def klasses + def extracted_klasses Thread.current[:act_as_list_no_update] ||= [] end end diff --git a/test/helper.rb b/test/helper.rb index 564ef6b8..0d4614c9 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -20,6 +20,31 @@ ActiveRecord::Base.raise_in_transactional_callbacks = true end +db_config = YAML.load_file(File.expand_path("../database.yml", __FILE__)).fetch(ENV["DB"] || "sqlite") +ActiveRecord::Base.establish_connection(db_config) +ActiveRecord::Schema.verbose = false + +# Returns true if ActiveRecord is rails 3, 4 version +def rails_3 + defined?(ActiveRecord::VERSION) && ActiveRecord::VERSION::MAJOR >= 3 +end + +def rails_4 + defined?(ActiveRecord::VERSION) && ActiveRecord::VERSION::MAJOR >= 4 +end + +def teardown_db + if ActiveRecord::VERSION::MAJOR >= 5 + tables = ActiveRecord::Base.connection.data_sources + else + tables = ActiveRecord::Base.connection.tables + end + + tables.each do |table| + ActiveRecord::Base.connection.drop_table(table) + end +end + require "shared" # ActiveRecord::Base.logger = Logger.new(STDOUT) @@ -30,4 +55,4 @@ def assert_equal_or_nil(a, b) else assert_equal a, b end -end \ No newline at end of file +end diff --git a/test/test_joined_list.rb b/test/test_joined_list.rb index df62129b..e029230e 100644 --- a/test/test_joined_list.rb +++ b/test/test_joined_list.rb @@ -1,9 +1,5 @@ require 'helper' -db_config = YAML.load_file(File.expand_path("../database.yml", __FILE__)).fetch(ENV["DB"] || "sqlite") -ActiveRecord::Base.establish_connection(db_config) -ActiveRecord::Schema.verbose = false - class Section < ActiveRecord::Base has_many :items acts_as_list @@ -37,14 +33,7 @@ def setup end def teardown - if ActiveRecord::VERSION::MAJOR >= 5 - tables = ActiveRecord::Base.connection.data_sources - else - tables = ActiveRecord::Base.connection.tables - end - tables.each do |table| - ActiveRecord::Base.connection.drop_table(table) - end + teardown_db super end end diff --git a/test/test_list.rb b/test/test_list.rb index 76a7bb99..cbdfc615 100644 --- a/test/test_list.rb +++ b/test/test_list.rb @@ -1,13 +1,9 @@ # NOTE: following now done in helper.rb (better Readability) require 'helper' -db_config = YAML.load_file(File.expand_path("../database.yml", __FILE__)).fetch(ENV["DB"] || "sqlite") -ActiveRecord::Base.establish_connection(db_config) -ActiveRecord::Schema.verbose = false - def setup_db(position_options = {}) $default_position = position_options[:default] - + # sqlite cannot drop/rename/alter columns and add constraints after table creation sqlite = ENV.fetch("DB", "sqlite") == "sqlite" @@ -25,7 +21,7 @@ def setup_db(position_options = {}) if position_options[:unique] && !(sqlite && position_options[:positive]) ActiveRecord::Base.connection.add_index :mixins, :pos, unique: true end - + if position_options[:positive] if sqlite # SQLite cannot add constraint after table creation, also cannot add unique inside ADD COLUMN @@ -57,27 +53,6 @@ def setup_db_with_default setup_db default: 0 end -# Returns true if ActiveRecord is rails3,4 version -def rails_3 - defined?(ActiveRecord::VERSION) && ActiveRecord::VERSION::MAJOR >= 3 -end - -def rails_4 - defined?(ActiveRecord::VERSION) && ActiveRecord::VERSION::MAJOR >= 4 -end - -def teardown_db - if ActiveRecord::VERSION::MAJOR >= 5 - tables = ActiveRecord::Base.connection.data_sources - else - tables = ActiveRecord::Base.connection.tables - end - - tables.each do |table| - ActiveRecord::Base.connection.drop_table(table) - end -end - class Mixin < ActiveRecord::Base self.table_name = 'mixins' end diff --git a/test/test_no_update_for_extra_classes.rb b/test/test_no_update_for_extra_classes.rb new file mode 100644 index 00000000..973fca54 --- /dev/null +++ b/test/test_no_update_for_extra_classes.rb @@ -0,0 +1,104 @@ +require 'helper' + +class TodoList < ActiveRecord::Base + has_many :todo_items + acts_as_list +end + +class TodoItem < ActiveRecord::Base + belongs_to :todo_list + has_many :todo_item_attachments + acts_as_list scope: :todo_list +end + +class TodoItemAttachment < ActiveRecord::Base + belongs_to :todo_item + acts_as_list scope: :todo_item +end + +class NoUpdateForCollectionClassesTestCase < Minitest::Test + def setup + ActiveRecord::Base.connection.create_table :todo_lists do |t| + t.column :position, :integer + end + + ActiveRecord::Base.connection.create_table :todo_items do |t| + t.column :position, :integer + t.column :todo_list_id, :integer + end + + ActiveRecord::Base.connection.create_table :todo_item_attachments do |t| + t.column :position, :integer + t.column :todo_item_id, :integer + end + + ActiveRecord::Base.connection.schema_cache.clear! + [TodoList, Item, TodoItemAttachment].each(&:reset_column_information) + super + end + + def teardown + teardown_db + super + end +end + +class NoUpdateForCollectionClassesTest < NoUpdateForCollectionClassesTestCase + def setup + super + @list_1, @list_2 = (1..2).map { |counter| TodoList.create!(position: counter) } + + @item_1, @item_2 = (1..2).map { |counter| TodoItem.create!(position: counter, todo_list_id: @list_1.id) } + @attachment_1, @attachment_2 = (1..2).map { |counter| TodoItemAttachment.create!(position: counter, todo_item_id: @item_1.id) } + end + + def test_update + @item_1.update_attributes(position: 2) + assert_equal 2, @item_1.reload.position + assert_equal 1, @item_2.reload.position + end + + def test_no_update_for_single_class_instances + TodoItem.acts_as_list_no_update { @item_1.update_attributes(position: 2) } + + assert_equal 2, @item_1.reload.position + assert_equal 2, @item_2.reload.position + end + + def test_no_update_for_different_class_instances + TodoItem.acts_as_list_no_update([TodoItemAttachment]) { update_records! } + + assert_equal 2, @item_1.reload.position + assert_equal 2, @item_2.reload.position + + assert_equal 2, @attachment_1.reload.position + assert_equal 2, @attachment_2.reload.position + + assert_equal 2, @list_1.reload.position + assert_equal 1, @list_2.reload.position + end + + def test_raising_array_type_error + exception = assert_raises ActiveRecord::Acts::List::NoUpdate::ArrayTypeError do + TodoList.acts_as_list_no_update(nil) + end + + assert_equal("The first argument must be an array", exception.message ) + end + + def test_non_disparity_classes_error + exception = assert_raises ActiveRecord::Acts::List::NoUpdate::DisparityClassesError do + TodoList.acts_as_list_no_update([Class]) + end + + assert_equal("The first argument should contain ActiveRecord or ApplicationRecord classes", exception.message ) + end + + private + + def update_records! + @item_1.update_attributes(position: 2) + @attachment_1.update_attributes(position: 2) + @list_1.update_attributes(position: 2) + end +end