closure_tree icon indicating copy to clipboard operation
closure_tree copied to clipboard

Deterministic order problem

Open burmashave opened this issue 9 years ago • 5 comments

I added this gem to a Rails project to test it and am having some difficulty with sort order. I have a simple model:

class Question < ActiveRecord::Base
    has_closure_tree order: 'sort_order'
end

Using this seeds.rb file, I create five questions, then append the third, fourth, and fifth records as children of the first, followed by the second:

names = [ "root", "q2", "q3", "q4", "q5" ]
names.each do |name|
    instance_variable_set("@#{name}", Question.create!)
end
@root.append_child(@q3)
@root.append_child(@q4)
@root.append_child(@q5)
@root.append_child(@q2)

I expected to see the second record (q2) appended as the last child of root. When I look at the database, though, I can't make sense of the results. The children appear to be ordered: q3, q2, q4, q5.

id, parent_id, sort_order, created_at, updated_at
1,NULL,0,"2015-10-07 20:14:04.867462","2015-10-07 20:14:04.867462"
2,1,1,"2015-10-07 20:14:05.024568","2015-10-07 20:14:05.779063"
3,1,0,"2015-10-07 20:14:05.134556","2015-10-07 20:14:05.445102"
4,1,2,"2015-10-07 20:14:05.236059","2015-10-07 20:14:05.545712"
5,1,3,"2015-10-07 20:14:05.328957","2015-10-07 20:14:05.670609"

Am I just misunderstanding how this is supposed to work? The test project's repo is here:

https://github.com/burmashave/closure_tree_test

Thanks!

burmashave avatar Oct 07 '15 21:10 burmashave

There are numerous tests for deterministic ordering, you should take a look at how they are set up and what method calls are expected to behave correctly.

mceachen avatar Oct 08 '15 00:10 mceachen

The following two tests fail when I create the objects first, then invoke append_child. However, they pass when I simply instantiate the objects with @q2, @q3, @q4, @q5 = 4.times.map { Question.new }

Is that expected behavior? The tests I looked at in closure_tree weren't using create.

require 'rails_helper'
require 'closure_tree/test/matcher'

RSpec.describe Question, type: :model do

    it { is_expected.to be_a_closure_tree.ordered }

    context "Deterministic ordering" do

        before :each do
            @root = Question.create!
            @q2, @q3, @q4, @q5 = 4.times.map { Question.create! }
            @root.append_child(@q3)
            @root.append_child(@q4)
            @root.append_child(@q5)
            @root.append_child(@q2) #append_child should place @q2 last in the children array
            @root.reload
        end

        it "appends children in order" do
            expect(@root.children).to eq [@q3, @q4, @q5, @q2]
        end

        it "identifies earlier siblings" do
            expect(@q2.siblings_before).to eq [@q3, @q4, @q5]
        end

    end

end

https://github.com/burmashave/closure_tree_test

Thanks...

burmashave avatar Oct 09 '15 20:10 burmashave

Practically speaking, I can't imagine why you'd want to both create and then immediately update the instance with the correct parent—why do two db round trips when one could suffice?

In any event, I think this use case should probably be supported, so I'll mark it as a defect.

mceachen avatar Nov 02 '15 05:11 mceachen

Yeah, I know it doesn't look efficient in test form, but I needed to frame the problem this way to isolate the issue. At any rate, thanks for supporting it.

burmashave avatar Nov 03 '15 04:11 burmashave

I have same problem, append_child works wrong with persisted records.

kolasss avatar Apr 29 '16 07:04 kolasss