stefankroes / ancestry

Organise ActiveRecord model into a tree structure
MIT License
3.71k stars 458 forks source link

[4-3] add back ancestor_ids_in_database #637

Closed kbrock closed 1 year ago

kbrock commented 1 year ago

backport of #643

this was removed by #589

adding fields back. added tests to exercise

Fixes #636 /cc @Znz

kbrock commented 1 year ago

update:

kbrock commented 1 year ago

update:

kbrock commented 1 year ago

this pr got too big. It depended upon a number of tests I may need to backport my spec changes and then possibly introduce 2 changes.

having trouble reconciling why _before_last_save returns an empty ancestry

znz commented 1 year ago

I tried with gem 'ancestry', git: 'https://github.com/kbrock/ancestry.git', branch: 'add_ancestor_ids_in_database_43', but some tests failed.

After setting parent_id, descendants becomes empty. I expect descendants does not change.

# foo has some children, and bar is in another tree.
foo.descendants # => some children
child = foo.descendants.first
foo.parent_id = bar.id
foo.descendants # => empty (expect some children)
child.parent == name_2 # => true
kbrock commented 1 year ago

The minimal change I can make is: branch: add_ancestor_ids_in_database-simple_43

Descendants worked a little differently before and after 589, so I'm trying to uncover whether the problem was introduced in that PR or if it was hidden in the code. I have a feeling there was an issue before.

As you can tell from this branch, it started to get very big. I tend to see lots of tests added when there is uncertainty. I will make a tests pr pulling these tests from master onto 4.3 and then readdress.

I am also adding a number of tests that exercise the in_database and before_last_save to hopefully prevent issues in the future.

Thank you for your help and patience. I will have much more time to address over this weekend.

kbrock commented 1 year ago

@znz Are you able to add a test case that will fail?

Maybe you can see how I am testing in #638 and will have better luck than I do with this descendant issue.

If you are able to help me write a test, then I can properly track down the issue and changes in 4-2 and 4-3.

If you are not able to write a test, then could you try running tests on 3 branches I created for you: 4-2-a, 4-2-b, and 4-2-c. They are 3 key spots getting us to 4.3 code.

I'm guessing that 4-2-a will work. nothing is really different from 4.2. If either of the other 2 branches fail, then I will be able to use that

Thanks

kbrock commented 1 year ago

found the culprit to the problem.

I am targeting 4.3.1, but I do wonder if 5.0 (master) would works for you. (assuming ancestor_ids_before_save works)

kbrock commented 1 year ago

you can find this on stefankroes/ancestry 4-3-stable or master

gem 'ancestry', git: 'https://github.com/stefankroes/ancestry.git', branch: '4-3-stable'

let me know if you have any other problems

znz commented 1 year ago

Thanks.

4-3-stable branch works well.

kbrock commented 1 year ago

@znz Hope 4.3.1 works for you. Thank you for reporting the issue and helping test this