collectiveidea / awesome_nested_set

An awesome replacement for acts_as_nested_set and better_nested_set.
MIT License
2.39k stars 492 forks source link

The behavior of `#parent` in `#validate` has changed, is this intentional? #461

Closed s4na closed 2 years ago

s4na commented 2 years ago

When I changed config.active_record.has_many_inversing to true, which was added in Rails 6.1, the behavior changed.

Is this intentional?

Changed behavior

Referencing #parent inside #validate when updating parent_id will refer to the old parent.

Benefits of modification

Validation can be used when the model is saved.

Steps to reproduce

The detailed source code is here.

When the setting is config.active_record.has_many_inversing = true -> https://github.com/s4na/issue-awesome_nested_set

When the setting is config.active_record.has_many_inversing = false -> https://github.com/s4na/issue-awesome_nested_set/tree/has_many_inversing-false

config

config.active_record.has_many_inversing = true

module RailsAwesomeNestedSetSandbox
  class Application < Rails::Application
    # ~~~

    config.active_record.has_many_inversing = true
  end
end

config.active_record.has_many_inversing = false

module RailsAwesomeNestedSetSandbox
  class Application < Rails::Application
    # ~~~

    config.active_record.has_many_inversing = false
  end
end

Model

class Category < ApplicationRecord
  acts_as_nested_set

  validate :check_parent_id

  def check_parent_id
    p 'parent'
    p parent
  end
end

Schema

  create_table "categories", force: :cascade do |t|
    t.string "name"
    t.integer "parent_id"
    t.integer "lft", null: false
    t.integer "rgt", null: false
    t.integer "depth", default: 0, null: false
    t.integer "children_count", default: 0, null: false
    t.datetime "created_at", precision: 6, null: false
    t.datetime "updated_at", precision: 6, null: false
    t.index ["lft"], name: "index_categories_on_lft"
    t.index ["parent_id"], name: "index_categories_on_parent_id"
    t.index ["rgt"], name: "index_categories_on_rgt"
  end

Test

class CategoryTest < ActiveSupport::TestCase
  test "the truth" do
    # https://github.com/collectiveidea/awesome_nested_set/wiki/Awesome-nested-set-cheat-sheet
    old_parent = Category.create!(name: 'old_parent')
    child = Category.create!(name: 'child')
    child.move_to_child_of(old_parent)
    child.save
    new_parent = Category.create!(name: 'new_parent')

    p '---------'
    child.update(parent_id: new_parent.id)
  end
end

Results

config.active_record.has_many_inversing = true

Referring to the old parent.

❯ ./bin/rails test
Running via Spring preloader in process 81041
Running via Spring preloader in process 81043
Run options: --seed 8659

# Running:

"parent"
nil
"parent"
nil
"parent"
#<Category id: 1, name: "old_parent", parent_id: nil, lft: 1, rgt: 4, depth: 0, children_count: 0, created_at: "2022-01-07 15:01:25.510447000 +0000", updated_at: "2022-01-07 15:01:25.526711000 +0000">
"parent"
nil
"---------"
"parent"
#<Category id: 1, name: "old_parent", parent_id: nil, lft: 1, rgt: 4, depth: 0, children_count: 0, created_at: "2022-01-07 15:01:25.510447000 +0000", updated_at: "2022-01-07 15:01:25.526711000 +0000">
.

Finished in 0.526165s, 1.9005 runs/s, 0.0000 assertions/s.
1 runs, 0 assertions, 0 failures, 0 errors, 0 skips

config.active_record.has_many_inversing = false

❯ ./bin/rails test
Running via Spring preloader in process 69280
Run options: --seed 48924

# Running:

"parent"
nil
"parent"
nil
"parent"
#<Category id: 1, name: "old_parent", parent_id: nil, lft: 1, rgt: 4, depth: 0, children_count: 0, created_at: "2022-01-11 05:29:55.604048000 +0000", updated_at: "2022-01-11 05:29:55.625369000 +0000">
"parent"
nil
"---------"
"parent"
#<Category id: 3, name: "new_parent", parent_id: nil, lft: 5, rgt: 6, depth: 0, children_count: 0, created_at: "2022-01-11 05:29:55.654131000 +0000", updated_at: "2022-01-11 05:29:55.654131000 +0000">
.

Finished in 0.316523s, 3.1593 runs/s, 0.0000 assertions/s.
1 runs, 0 assertions, 0 failures, 0 errors, 0 skips

Best regards, s4na

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

phansch commented 2 years ago

I think we're also running into this. Did you find a workaround @s4na?

s4na commented 2 years ago

@phansch

Workaround

class Category < ApplicationRecord
  acts_as_nested_set

  validate :check_parent_id

  def check_parent_id
    finded_parent = Category.find_by(id: parent_id)
    p finded_parent
  end
end