Fix issue #2101 #2202
Fix issue #2101 #2202
Conversation
| @@ -450,6 +450,7 @@ def copy_children_to(new_parent) | |||
| new_child = Page.copy(child, { | |||
| language_id: new_parent.language_id, | |||
| language_code: new_parent.language_code, | |||
| parent_id: new_parent.id, | |||
There was a problem hiding this comment.
This should already be handled by move_to_child of. Also move_to_child sets the appropriate lft and rgt values the nested set needs.
| language_id: new_parent.language_id, | ||
| language_code: new_parent.language_code, | ||
| }).and_return(new_child) | ||
| subject |
There was a problem hiding this comment.
Instead of testing the implementation we should test the result. For that we need to create a page with children and verify the children end up on the new parent. That way we can check if the issue is actually in the missing parent_id or not.
|
Please add a more descriptive PR title and describe what you are doing with this PR and explain why you think this solves the issue mentioned. That will help us documenting this in the changelog. |
|
@juryghidinelli Is this related with #2168 ? |
No i don't think, this is related to #2101 |
|
I'm running into this issue as well and would like to help get the fix merged. @juryghidinelli 's fix works, but I agree |
|
As I mentioned in my comments. A better test that tests the result, not the implementation would be nice. A PR description and commit message would be mandatory as well. And I still have the question why awesome-nested-sets move_to_child of is not working here as expected. |
|
Thanks. See #2243. |
|
Done in #2243 |
What is this pull request for?
Closes #2101
Checklist