[MAGNOLIA-5769] When moving nodes, NodeUtil should check if parent node is the same Created: 11/May/14  Updated: 16/May/14  Resolved: 15/May/14

Status: Closed
Project: Magnolia
Component/s: core
Affects Version/s: 5.2
Fix Version/s: 5.3

Type: Bug Priority: Major
Reporter: Magnolia International Assignee: Jan Haderka
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relation
is related to MGNLUI-2901 Consolidate MoveAction (w/ dialog) an... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

When calling session.move() for a node and moving it into its current parent, Jackrabbit throws an exception (due to same-name sibling checks). We should simply do that check ourselves and ignore the move operation. This affects especially methods like moveNodeBefore/After, since they're used to re-order nodes in the tree, by move action/dialog etc.



 Comments   
Comment by Magnolia International [ 14/May/14 ]

Reopening to make sure we re-review this
The fix implies that the below doesn't work ? i.e you can't call move() if you don't actually move to a different parent ?

target = x.parent;
moveNode(x, target)

I'd expect two things: if my assumption above is right, either a fix to that (even if it's a silly thing to do, it should be silently accepted, espectially in a util like this), an inline comment that says why we do this check; i'd also move the check to the move() method, unless there's a good reason not to (again with a comment, then); lastly, i'd expect a test that shows the fix works (eg that you can move/reorded within the same parent), not just a test that shows the default case works (although that was indeed missing)

(and a short description in the issue would help clarifying that by "changed" you meant "the parent node is another node than the current one", not "modified")

Comment by Jan Haderka [ 15/May/14 ]

lastly, i'd expect a test that shows the fix works (eg that you can move/reorded within the same parent)

Actually that's exactly what the test was doing - moving nodes within same folder. Now, after moving the check to move() method I've also added same test for this method.

Generated at Mon Feb 12 04:08:13 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.