[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: |
|
||||||||
| 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 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 ] |
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. |