Commit b4c312547d8de547c0ea19d3e03fc66a4ca3fd39
1 parent
a5c0c0f3
Exists in
master
and in
29 other branches
Update acts_as_list plugin
Showing
3 changed files
with
216 additions
and
10 deletions
Show diff stats
| @@ -0,0 +1,12 @@ | @@ -0,0 +1,12 @@ | ||
| 1 | +require 'rake' | ||
| 2 | +require 'rake/testtask' | ||
| 3 | + | ||
| 4 | +desc 'Default: run acts_as_list unit tests.' | ||
| 5 | +task :default => :test | ||
| 6 | + | ||
| 7 | +desc 'Test the acts_as_ordered_tree plugin.' | ||
| 8 | +Rake::TestTask.new(:test) do |t| | ||
| 9 | + t.libs << 'lib' | ||
| 10 | + t.pattern = 'test/**/*_test.rb' | ||
| 11 | + t.verbose = true | ||
| 12 | +end |
vendor/plugins/acts_as_list/lib/active_record/acts/list.rb
| @@ -39,11 +39,16 @@ module ActiveRecord | @@ -39,11 +39,16 @@ module ActiveRecord | ||
| 39 | if configuration[:scope].is_a?(Symbol) | 39 | if configuration[:scope].is_a?(Symbol) |
| 40 | scope_condition_method = %( | 40 | scope_condition_method = %( |
| 41 | def scope_condition | 41 | def scope_condition |
| 42 | - if #{configuration[:scope].to_s}.nil? | ||
| 43 | - "#{configuration[:scope].to_s} IS NULL" | ||
| 44 | - else | ||
| 45 | - "#{configuration[:scope].to_s} = \#{#{configuration[:scope].to_s}}" | 42 | + self.class.send(:sanitize_sql_hash_for_conditions, { :#{configuration[:scope].to_s} => send(:#{configuration[:scope].to_s}) }) |
| 43 | + end | ||
| 44 | + ) | ||
| 45 | + elsif configuration[:scope].is_a?(Array) | ||
| 46 | + scope_condition_method = %( | ||
| 47 | + def scope_condition | ||
| 48 | + attrs = %w(#{configuration[:scope].join(" ")}).inject({}) do |memo,column| | ||
| 49 | + memo[column.intern] = send(column.intern); memo | ||
| 46 | end | 50 | end |
| 51 | + self.class.send(:sanitize_sql_hash_for_conditions, attrs) | ||
| 47 | end | 52 | end |
| 48 | ) | 53 | ) |
| 49 | else | 54 | else |
| @@ -63,7 +68,7 @@ module ActiveRecord | @@ -63,7 +68,7 @@ module ActiveRecord | ||
| 63 | 68 | ||
| 64 | #{scope_condition_method} | 69 | #{scope_condition_method} |
| 65 | 70 | ||
| 66 | - before_destroy :remove_from_list | 71 | + before_destroy :decrement_positions_on_lower_items |
| 67 | before_create :add_to_list_bottom | 72 | before_create :add_to_list_bottom |
| 68 | EOV | 73 | EOV |
| 69 | end | 74 | end |
vendor/plugins/acts_as_list/test/list_test.rb
| @@ -6,13 +6,14 @@ require 'active_record' | @@ -6,13 +6,14 @@ require 'active_record' | ||
| 6 | 6 | ||
| 7 | require "#{File.dirname(__FILE__)}/../init" | 7 | require "#{File.dirname(__FILE__)}/../init" |
| 8 | 8 | ||
| 9 | -ActiveRecord::Base.establish_connection(:adapter => "sqlite3", :dbfile => ":memory:") | 9 | +ActiveRecord::Base.establish_connection(:adapter => "sqlite3", :database => ":memory:") |
| 10 | 10 | ||
| 11 | def setup_db | 11 | def setup_db |
| 12 | ActiveRecord::Schema.define(:version => 1) do | 12 | ActiveRecord::Schema.define(:version => 1) do |
| 13 | create_table :mixins do |t| | 13 | create_table :mixins do |t| |
| 14 | t.column :pos, :integer | 14 | t.column :pos, :integer |
| 15 | t.column :parent_id, :integer | 15 | t.column :parent_id, :integer |
| 16 | + t.column :parent_type, :string | ||
| 16 | t.column :created_at, :datetime | 17 | t.column :created_at, :datetime |
| 17 | t.column :updated_at, :datetime | 18 | t.column :updated_at, :datetime |
| 18 | end | 19 | end |
| @@ -46,6 +47,11 @@ class ListWithStringScopeMixin < ActiveRecord::Base | @@ -46,6 +47,11 @@ class ListWithStringScopeMixin < ActiveRecord::Base | ||
| 46 | def self.table_name() "mixins" end | 47 | def self.table_name() "mixins" end |
| 47 | end | 48 | end |
| 48 | 49 | ||
| 50 | +class ArrayScopeListMixin < Mixin | ||
| 51 | + acts_as_list :column => "pos", :scope => [:parent_id, :parent_type] | ||
| 52 | + | ||
| 53 | + def self.table_name() "mixins" end | ||
| 54 | +end | ||
| 49 | 55 | ||
| 50 | class ListTest < Test::Unit::TestCase | 56 | class ListTest < Test::Unit::TestCase |
| 51 | 57 | ||
| @@ -95,7 +101,7 @@ class ListTest < Test::Unit::TestCase | @@ -95,7 +101,7 @@ class ListTest < Test::Unit::TestCase | ||
| 95 | 101 | ||
| 96 | def test_injection | 102 | def test_injection |
| 97 | item = ListMixin.new(:parent_id => 1) | 103 | item = ListMixin.new(:parent_id => 1) |
| 98 | - assert_equal "parent_id = 1", item.scope_condition | 104 | + assert_equal '"mixins"."parent_id" = 1', item.scope_condition |
| 99 | assert_equal "pos", item.position_column | 105 | assert_equal "pos", item.position_column |
| 100 | end | 106 | end |
| 101 | 107 | ||
| @@ -187,8 +193,7 @@ class ListTest < Test::Unit::TestCase | @@ -187,8 +193,7 @@ class ListTest < Test::Unit::TestCase | ||
| 187 | new2.move_higher | 193 | new2.move_higher |
| 188 | assert_equal [new2, new1, new3], ListMixin.find(:all, :conditions => 'parent_id IS NULL', :order => 'pos') | 194 | assert_equal [new2, new1, new3], ListMixin.find(:all, :conditions => 'parent_id IS NULL', :order => 'pos') |
| 189 | end | 195 | end |
| 190 | - | ||
| 191 | - | 196 | + |
| 192 | def test_remove_from_list_should_then_fail_in_list? | 197 | def test_remove_from_list_should_then_fail_in_list? |
| 193 | assert_equal true, ListMixin.find(1).in_list? | 198 | assert_equal true, ListMixin.find(1).in_list? |
| 194 | ListMixin.find(1).remove_from_list | 199 | ListMixin.find(1).remove_from_list |
| @@ -221,6 +226,27 @@ class ListTest < Test::Unit::TestCase | @@ -221,6 +226,27 @@ class ListTest < Test::Unit::TestCase | ||
| 221 | assert_equal 3, ListMixin.find(4).pos | 226 | assert_equal 3, ListMixin.find(4).pos |
| 222 | end | 227 | end |
| 223 | 228 | ||
| 229 | + def test_before_destroy_callbacks_do_not_update_position_to_nil_before_deleting_the_record | ||
| 230 | + assert_equal [1, 2, 3, 4], ListMixin.find(:all, :conditions => 'parent_id = 5', :order => 'pos').map(&:id) | ||
| 231 | + | ||
| 232 | + # We need to trigger all the before_destroy callbacks without actually | ||
| 233 | + # destroying the record so we can see the affect the callbacks have on | ||
| 234 | + # the record. | ||
| 235 | + list = ListMixin.find(2) | ||
| 236 | + if list.respond_to?(:run_callbacks) | ||
| 237 | + list.run_callbacks(:destroy) | ||
| 238 | + else | ||
| 239 | + list.send(:callback, :before_destroy) | ||
| 240 | + end | ||
| 241 | + | ||
| 242 | + assert_equal [1, 2, 3, 4], ListMixin.find(:all, :conditions => 'parent_id = 5', :order => 'pos').map(&:id) | ||
| 243 | + | ||
| 244 | + assert_equal 1, ListMixin.find(1).pos | ||
| 245 | + assert_equal 2, ListMixin.find(2).pos | ||
| 246 | + assert_equal 2, ListMixin.find(3).pos | ||
| 247 | + assert_equal 3, ListMixin.find(4).pos | ||
| 248 | + end | ||
| 249 | + | ||
| 224 | end | 250 | end |
| 225 | 251 | ||
| 226 | class ListSubTest < Test::Unit::TestCase | 252 | class ListSubTest < Test::Unit::TestCase |
| @@ -271,7 +297,7 @@ class ListSubTest < Test::Unit::TestCase | @@ -271,7 +297,7 @@ class ListSubTest < Test::Unit::TestCase | ||
| 271 | 297 | ||
| 272 | def test_injection | 298 | def test_injection |
| 273 | item = ListMixin.new("parent_id"=>1) | 299 | item = ListMixin.new("parent_id"=>1) |
| 274 | - assert_equal "parent_id = 1", item.scope_condition | 300 | + assert_equal '"mixins"."parent_id" = 1', item.scope_condition |
| 275 | assert_equal "pos", item.position_column | 301 | assert_equal "pos", item.position_column |
| 276 | end | 302 | end |
| 277 | 303 | ||
| @@ -330,3 +356,166 @@ class ListSubTest < Test::Unit::TestCase | @@ -330,3 +356,166 @@ class ListSubTest < Test::Unit::TestCase | ||
| 330 | end | 356 | end |
| 331 | 357 | ||
| 332 | end | 358 | end |
| 359 | + | ||
| 360 | +class ArrayScopeListTest < Test::Unit::TestCase | ||
| 361 | + | ||
| 362 | + def setup | ||
| 363 | + setup_db | ||
| 364 | + (1..4).each { |counter| ArrayScopeListMixin.create! :pos => counter, :parent_id => 5, :parent_type => 'ParentClass' } | ||
| 365 | + end | ||
| 366 | + | ||
| 367 | + def teardown | ||
| 368 | + teardown_db | ||
| 369 | + end | ||
| 370 | + | ||
| 371 | + def test_reordering | ||
| 372 | + assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 373 | + | ||
| 374 | + ArrayScopeListMixin.find(2).move_lower | ||
| 375 | + assert_equal [1, 3, 2, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 376 | + | ||
| 377 | + ArrayScopeListMixin.find(2).move_higher | ||
| 378 | + assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 379 | + | ||
| 380 | + ArrayScopeListMixin.find(1).move_to_bottom | ||
| 381 | + assert_equal [2, 3, 4, 1], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 382 | + | ||
| 383 | + ArrayScopeListMixin.find(1).move_to_top | ||
| 384 | + assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 385 | + | ||
| 386 | + ArrayScopeListMixin.find(2).move_to_bottom | ||
| 387 | + assert_equal [1, 3, 4, 2], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 388 | + | ||
| 389 | + ArrayScopeListMixin.find(4).move_to_top | ||
| 390 | + assert_equal [4, 1, 3, 2], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 391 | + end | ||
| 392 | + | ||
| 393 | + def test_move_to_bottom_with_next_to_last_item | ||
| 394 | + assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 395 | + ArrayScopeListMixin.find(3).move_to_bottom | ||
| 396 | + assert_equal [1, 2, 4, 3], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 397 | + end | ||
| 398 | + | ||
| 399 | + def test_next_prev | ||
| 400 | + assert_equal ArrayScopeListMixin.find(2), ArrayScopeListMixin.find(1).lower_item | ||
| 401 | + assert_nil ArrayScopeListMixin.find(1).higher_item | ||
| 402 | + assert_equal ArrayScopeListMixin.find(3), ArrayScopeListMixin.find(4).higher_item | ||
| 403 | + assert_nil ArrayScopeListMixin.find(4).lower_item | ||
| 404 | + end | ||
| 405 | + | ||
| 406 | + def test_injection | ||
| 407 | + item = ArrayScopeListMixin.new(:parent_id => 1, :parent_type => 'ParentClass') | ||
| 408 | + assert_equal '"mixins"."parent_id" = 1 AND "mixins"."parent_type" = \'ParentClass\'', item.scope_condition | ||
| 409 | + assert_equal "pos", item.position_column | ||
| 410 | + end | ||
| 411 | + | ||
| 412 | + def test_insert | ||
| 413 | + new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass') | ||
| 414 | + assert_equal 1, new.pos | ||
| 415 | + assert new.first? | ||
| 416 | + assert new.last? | ||
| 417 | + | ||
| 418 | + new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass') | ||
| 419 | + assert_equal 2, new.pos | ||
| 420 | + assert !new.first? | ||
| 421 | + assert new.last? | ||
| 422 | + | ||
| 423 | + new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass') | ||
| 424 | + assert_equal 3, new.pos | ||
| 425 | + assert !new.first? | ||
| 426 | + assert new.last? | ||
| 427 | + | ||
| 428 | + new = ArrayScopeListMixin.create(:parent_id => 0, :parent_type => 'ParentClass') | ||
| 429 | + assert_equal 1, new.pos | ||
| 430 | + assert new.first? | ||
| 431 | + assert new.last? | ||
| 432 | + end | ||
| 433 | + | ||
| 434 | + def test_insert_at | ||
| 435 | + new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass') | ||
| 436 | + assert_equal 1, new.pos | ||
| 437 | + | ||
| 438 | + new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass') | ||
| 439 | + assert_equal 2, new.pos | ||
| 440 | + | ||
| 441 | + new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass') | ||
| 442 | + assert_equal 3, new.pos | ||
| 443 | + | ||
| 444 | + new4 = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass') | ||
| 445 | + assert_equal 4, new4.pos | ||
| 446 | + | ||
| 447 | + new4.insert_at(3) | ||
| 448 | + assert_equal 3, new4.pos | ||
| 449 | + | ||
| 450 | + new.reload | ||
| 451 | + assert_equal 4, new.pos | ||
| 452 | + | ||
| 453 | + new.insert_at(2) | ||
| 454 | + assert_equal 2, new.pos | ||
| 455 | + | ||
| 456 | + new4.reload | ||
| 457 | + assert_equal 4, new4.pos | ||
| 458 | + | ||
| 459 | + new5 = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass') | ||
| 460 | + assert_equal 5, new5.pos | ||
| 461 | + | ||
| 462 | + new5.insert_at(1) | ||
| 463 | + assert_equal 1, new5.pos | ||
| 464 | + | ||
| 465 | + new4.reload | ||
| 466 | + assert_equal 5, new4.pos | ||
| 467 | + end | ||
| 468 | + | ||
| 469 | + def test_delete_middle | ||
| 470 | + assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 471 | + | ||
| 472 | + ArrayScopeListMixin.find(2).destroy | ||
| 473 | + | ||
| 474 | + assert_equal [1, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 475 | + | ||
| 476 | + assert_equal 1, ArrayScopeListMixin.find(1).pos | ||
| 477 | + assert_equal 2, ArrayScopeListMixin.find(3).pos | ||
| 478 | + assert_equal 3, ArrayScopeListMixin.find(4).pos | ||
| 479 | + | ||
| 480 | + ArrayScopeListMixin.find(1).destroy | ||
| 481 | + | ||
| 482 | + assert_equal [3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 483 | + | ||
| 484 | + assert_equal 1, ArrayScopeListMixin.find(3).pos | ||
| 485 | + assert_equal 2, ArrayScopeListMixin.find(4).pos | ||
| 486 | + end | ||
| 487 | + | ||
| 488 | + def test_remove_from_list_should_then_fail_in_list? | ||
| 489 | + assert_equal true, ArrayScopeListMixin.find(1).in_list? | ||
| 490 | + ArrayScopeListMixin.find(1).remove_from_list | ||
| 491 | + assert_equal false, ArrayScopeListMixin.find(1).in_list? | ||
| 492 | + end | ||
| 493 | + | ||
| 494 | + def test_remove_from_list_should_set_position_to_nil | ||
| 495 | + assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 496 | + | ||
| 497 | + ArrayScopeListMixin.find(2).remove_from_list | ||
| 498 | + | ||
| 499 | + assert_equal [2, 1, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 500 | + | ||
| 501 | + assert_equal 1, ArrayScopeListMixin.find(1).pos | ||
| 502 | + assert_equal nil, ArrayScopeListMixin.find(2).pos | ||
| 503 | + assert_equal 2, ArrayScopeListMixin.find(3).pos | ||
| 504 | + assert_equal 3, ArrayScopeListMixin.find(4).pos | ||
| 505 | + end | ||
| 506 | + | ||
| 507 | + def test_remove_before_destroy_does_not_shift_lower_items_twice | ||
| 508 | + assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 509 | + | ||
| 510 | + ArrayScopeListMixin.find(2).remove_from_list | ||
| 511 | + ArrayScopeListMixin.find(2).destroy | ||
| 512 | + | ||
| 513 | + assert_equal [1, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id) | ||
| 514 | + | ||
| 515 | + assert_equal 1, ArrayScopeListMixin.find(1).pos | ||
| 516 | + assert_equal 2, ArrayScopeListMixin.find(3).pos | ||
| 517 | + assert_equal 3, ArrayScopeListMixin.find(4).pos | ||
| 518 | + end | ||
| 519 | + | ||
| 520 | +end | ||
| 521 | + |