[MGNLUI-3098] Bulk operation MoveNodeAction: "Move After" is reverting the chosen Node order Created: 13/Aug/14  Updated: 05/Dec/14  Resolved: 19/Aug/14

Status: Closed
Project: Magnolia UI
Component/s: content app
Affects Version/s: 5.2.8, 5.3.2
Fix Version/s: 5.2.8, 5.3.3

Type: Bug Priority: Major
Reporter: Christian Ringele Assignee: Daniel Lipp
Resolution: Fixed Votes: 1
Labels: quickwin, support
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File MoveNodeAction-M5.2.5.patch     Text File MoveNodeAction-M5.3.1.patch    
Issue Links:
causality
Template:
Patch included:
Yes
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   

Summary:
When moving a selection of Nodes with the "Move After" operation, their order is reversed.
The reason is that for each Node the moveAfter() is called, which reverses its original order:
The last Node is placed as last element directly after the target node -> its then the first Node and not the last anymore.

Cause:
Super class of MoveNodeAction abstract class is

info.magnolia.ui.framework.action.AbstractMultiItemAction

It calls in the execute() for every item operating on the abstract method .executeOnItem(JcrItemAdapter).
The MoveNodeAction implementing the method executeOnItem is calling

info.magnolia.ui.workbench.tree.MoveHandler.moveItem(Item, Item, MoveLocation)

which in the end calls

info.magnolia.jcr.util.NodeUtil.moveNodeAfter(Node, Node)

Solution general:
Override the method

info.magnolia.ui.contentapp.movedialog.action.MoveNodeAction.getSortedItems(Comparator<JcrItemAdapter>)

of the super class and revert the list on the MoveLoction.after operation.

Solution patches:
As the code changed form 5.2 to 5.3 of this class, I needed to create two different patches basically containing the same code. Just the line numbers won't match.



 Comments   
Comment by Aleksandr Pchelintcev [ 18/Aug/14 ]

Instead of ensuring the correct item order by overriding the MoveNodeAction#getSortedItems one could override the MoveNodeAction#getItemComparator which would lead to a clearer and less resource-consuming solution.

Comment by Daniel Lipp [ 18/Aug/14 ]

Looked like a superior approach but unfortunately the Comparator only comes into play when nodes are on different level. When moving nodes on the same level as in MoveNodeActionTest#moveAfterPreservesOrder the result in the comparator will always be 0 and hence inverting would change anything.

Generated at Mon Feb 12 09:03:11 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.