tree sorting: fix sorting multiple items

client/src/app/core/repositories/motions/motion-repository.service.ts

@@ -438,19 +438,6 @@ export class MotionRepositoryService extends BaseIsAgendaItemAndListOfSpeakersCo
         await this.httpService.post('/rest/motions/motion/sort/', data);
     }
 
-    /**
-     * Sends the changed nodes to the server, with only the top nodes being submitted.
-     *
-     * @param data The reordered data from the sorting, as list of ViewMotions
-     * @param parent a parent id
-     */
-    public async sortMotionBranches(data: ViewMotion[], parent?: number): Promise<void> {
-        const url = '/rest/motions/motion/sort/';
-        const nodes = data.map(motion => ({ id: motion.id }));
-        const params = parent ? { nodes: nodes, parent_id: parent } : { nodes: nodes };
-        await this.httpService.post(url, params);
-    }
-
     /**
      * Supports the motion
      *

client/src/app/core/ui-services/tree.service.ts

@@ -218,43 +218,137 @@ export class TreeService {
     }
 
     /**
-     * Reduce a list of items to nodes independent from each other in a given tree
+     * Searches a tree for a list of given items and fetches all branches that include
+     * these items and their dependants
      *
-     * @param tree the tree to traverse
+     * @param tree an array of OsTreeNode branches
-     * @param items the items to check
+     * @param items the items that need to be included
-     * @returns the selection of items that belong to different branches
+     *
+     * @returns an array of OsTreeNodes with the top-most item being included
+     * in the input list
      */
-    public getTopItemsFromTree<T extends Identifiable & Displayable>(tree: OSTreeNode<T>[], items: T[]): T[] {
+    public getBranchesFromTree<T extends Identifiable & Displayable>(
-        let results: T[] = [];
+        tree: OSTreeNode<T>[],
+        items: T[]
+    ): OSTreeNode<T>[] {
+        let results: OSTreeNode<T>[] = [];
         tree.forEach(branch => {
-            const i = this.getTopItemsFromBranch(branch, items);
+            if (items.some(item => item.id === branch.item.id)) {
-            if (i.length) {
+                results.push(branch);
-                results = results.concat(i);
+            } else if (branch.children && branch.children.length) {
+                results = results.concat(this.getBranchesFromTree(branch.children, items));
             }
         });
         return results;
     }
 
     /**
-     * Return all items not being hierarchically dependant on the items in the input arrray
+     * Inserts OSTreeNode branches into another tree at the position specified
+     *
+     * @param tree A (partial) tree the branches need to be inserted into. It
+     * is assumed that this tree does not contain the branches to be inserted.
+     * See also {@link getTreeWithoutSelection}
+     * @param branches OsTreeNodes to be inserted. See also {@link getBranchesFromTree}
+     * @param parentId the id of a parent node under which the branches should be inserted
+     * @param olderSibling (optional) the id of the item on the same level
+     * the tree is to be inserted behind
+     * @returns the re-arranged tree containing the branches
+     */
+    public insertBranchesIntoTree<T extends Identifiable & Displayable>(
+        tree: OSTreeNode<T>[],
+        branches: OSTreeNode<T>[],
+        parentId: number,
+        olderSibling?: number
+    ): OSTreeNode<T>[] {
+        if (!parentId && olderSibling) {
+            const older = tree.findIndex(branch => branch.id === olderSibling);
+            if (older >= 0) {
+                return [...tree.slice(0, older + 1), ...branches, ...tree.slice(older + 1)];
+            } else {
+                for (const branch of tree) {
+                    if (branch.children && branch.children.length) {
+                        branch.children = this.insertBranchesIntoTree(branch.children, branches, null, olderSibling);
+                    }
+                }
+                return tree;
+            }
+        } else if (parentId) {
+            for (const branch of tree) {
+                if (branch.id !== parentId) {
+                    if (branch.children && branch.children.length) {
+                        branch.children = this.insertBranchesIntoTree(
+                            branch.children,
+                            branches,
+                            parentId,
+                            olderSibling
+                        );
+                    }
+                } else {
+                    if (!branch.children) {
+                        branch.children = branches;
+                    } else {
+                        if (olderSibling) {
+                            const older = branch.children.findIndex(child => child.id === olderSibling);
+                            if (older >= 0) {
+                                branch.children = [
+                                    ...branch.children.slice(0, older + 1),
+                                    ...branches,
+                                    ...branch.children.slice(older + 1)
+                                ];
+                            }
+                        } else {
+                            branch.children = [...branch.children, ...branches];
+                        }
+                    }
+                }
+            }
+            return tree;
+        } else {
+            throw new Error('This should not happen. Invalid sorting items given');
+        }
+    }
+
+    /**
+     * Return the part of a tree not including or being hierarchically dependant
+     * on the items in the input arrray
      *
      * @param tree
      * @param items
-     * @returns all items that are neither in the input nor dependants of  items in the input
+     * @returns all the branch without the given items or their dependants
      */
-    public getTreeWithoutSelection<T extends Identifiable & Displayable>(tree: OSTreeNode<T>[], items: T[]): T[] {
+    public getTreeWithoutSelection<T extends Identifiable & Displayable>(
-        let result: T[] = [];
+        tree: OSTreeNode<T>[],
+        items: T[]
+    ): OSTreeNode<T>[] {
+        const result: OSTreeNode<T>[] = [];
         tree.forEach(branch => {
             if (!items.find(i => i.id === branch.item.id)) {
-                result.push(branch.item);
                 if (branch.children) {
-                    result = result.concat(this.getTreeWithoutSelection(branch.children, items));
+                    branch.children = this.getTreeWithoutSelection(branch.children, items);
                 }
+                result.push(branch);
             }
         });
         return result;
     }
 
+    /**
+     * Helper to turn a tree into an array of items
+     *
+     * @param tree
+     * @returns the items contained in the tree.
+     */
+    public getFlatItemsFromTree<T extends Identifiable & Displayable>(tree: OSTreeNode<T>[]): T[] {
+        let result = [];
+        for (const branch of tree) {
+            result.push(branch.item);
+            if (branch.children && branch.children.length) {
+                result = result.concat(this.getFlatItemsFromTree(branch.children));
+            }
+        }
+        return result;
+    }
+
     /**
      * Helper function to go recursively through the children of given node.
      *

client/src/app/shared/components/choice-dialog/choice-dialog.component.html

@@ -17,7 +17,6 @@
             <mat-radio-button *ngIf="data.clearChoice" [value]="null">
                 {{ data.clearChoice | translate }}
             </mat-radio-button>
-
         </mat-radio-group>
 
         <mat-list *ngIf="data.multiSelect && data.choices">
@@ -30,12 +29,22 @@
     </div>
     <mat-dialog-actions>
         <div *ngIf="data.actionButtons">
-            <button *ngFor="let button of data.actionButtons" mat-button (click)="closeDialog(true, button)">
+            <button
+                *ngFor="let button of data.actionButtons"
+                mat-button
+                (click)="closeDialog(true, button)"
+                [disabled]="isSelectionEmpty"
+            >
                 <span>{{ button | translate }}</span>
             </button>
         </div>
         <div *ngIf="!data.actionButtons">
-            <button *ngIf="!data.multiSelect || data.choices.length" mat-button (click)="closeDialog(true)">
+            <button
+                *ngIf="!data.multiSelect || data.choices.length"
+                mat-button
+                (click)="closeDialog(true)"
+                [disabled]="isSelectionEmpty"
+            >
                 <span>OK</span>
             </button>
         </div>

client/src/app/shared/components/choice-dialog/choice-dialog.component.ts

@@ -69,6 +69,20 @@ export class ChoiceDialogComponent {
      */
     public selectedChoice: number;
 
+    /**
+     * Checks if there is nothing selected
+     *
+     * @returns true if there is no selection chosen (and the dialog should not
+     * be closed 'successfully')
+     */
+    public get isSelectionEmpty(): boolean {
+        if (this.data.multiSelect) {
+            return this.selectedMultiChoices.length === 0;
+        } else {
+            return this.selectedChoice === undefined;
+        }
+    }
+
     /**
      * All selected ids, if this is a multiselect choice
      */

client/src/app/site/motions/services/motion-multiselect.service.ts

@@ -331,24 +331,33 @@ export class MotionMultiselectService {
         const options = [this.translate.instant('Set as parent'), this.translate.instant('Insert after')];
         const allMotions = this.repo.getViewModelList();
         const tree = this.treeService.makeTree(allMotions, 'weight', 'sort_parent_id');
-        const itemsToMove = this.treeService.getTopItemsFromTree(tree, motions);
+        const itemsToMove = this.treeService.getBranchesFromTree(tree, motions);
-        const selectableItems = this.treeService.getTreeWithoutSelection(tree, motions);
+        const partialTree = this.treeService.getTreeWithoutSelection(tree, motions);
-
+        const availableMotions = this.treeService.getFlatItemsFromTree(partialTree);
-        const selectedChoice = await this.choiceService.open(title, selectableItems, false, options);
+        if (!availableMotions.length) {
-        if (selectedChoice) {
+            throw new Error(this.translate.instant('There are no items left to chose from'));
-            if (selectedChoice.action === options[0]) {
+        } else {
-                // set choice as parent
+            const selectedChoice = await this.choiceService.open(title, availableMotions, false, options);
-                this.repo.sortMotionBranches(itemsToMove, selectedChoice.items as number);
+            if (selectedChoice) {
-            } else if (selectedChoice.action === options[1]) {
+                if (!selectedChoice.items) {
-                // insert after chosen
+                    throw this.translate.instant('No items selected');
-                const olderSibling = this.repo.getViewModel(selectedChoice.items as number);
+                }
-                const parentId = olderSibling ? olderSibling.sort_parent_id : null;
+                if (selectedChoice.action === options[0]) {
-                const siblings = allMotions.filter(motion => motion.sort_parent_id === parentId);
+                    const sortedChildTree = this.treeService.insertBranchesIntoTree(
-                const idx = siblings.findIndex(sib => sib.id === olderSibling.id);
+                        partialTree,
-                const before = siblings.slice(0, idx + 1);
+                        itemsToMove,
-                const after = siblings.slice(idx + 1);
+                        selectedChoice.items as number
-                const sum = [].concat(before, itemsToMove, after);
+                    );
-                this.repo.sortMotionBranches(sum, parentId);
+                    this.repo.sortMotions(this.treeService.stripTree(sortedChildTree));
+                } else if (selectedChoice.action === options[1]) {
+                    const sortedSiblingTree = this.treeService.insertBranchesIntoTree(
+                        partialTree,
+                        itemsToMove,
+                        this.repo.getViewModel(selectedChoice.items as number).parent_id,
+                        selectedChoice.items as number
+                    );
+                    this.repo.sortMotions(this.treeService.stripTree(sortedSiblingTree));
+                }
             }
         }
     }