From 8f939fa9335cc2dda38c282df706697487b1be51 Mon Sep 17 00:00:00 2001
From: FinnStutzenstein <finn.stutzenstein@hotmail.de>
Date: Fri, 2 Nov 2018 16:25:16 +0100
Subject: [PATCH] refine category editing and motion numbering. Fixes the
 current behavior of double-saving to take effect.

---
 .../category-list.component.html              | 23 +++----
 .../category-list.component.scss              |  4 ++
 .../category-list/category-list.component.ts  | 60 +++++++++++++------
 .../motion-detail/motion-detail.component.ts  | 13 +---
 .../site/motions/models/category-numbering.ts | 22 -------
 .../services/category-repository.service.ts   | 26 +++-----
 6 files changed, 68 insertions(+), 80 deletions(-)
 delete mode 100644 client/src/app/site/motions/models/category-numbering.ts

diff --git a/client/src/app/site/motions/components/category-list/category-list.component.html b/client/src/app/site/motions/components/category-list/category-list.component.html
index 975e3851a..cc05f1b2d 100644
--- a/client/src/app/site/motions/components/category-list/category-list.component.html
+++ b/client/src/app/site/motions/components/category-list/category-list.component.html
@@ -39,7 +39,8 @@
 </mat-card>
 
 <mat-accordion class="os-card">
-    <mat-expansion-panel *ngFor="let category of this.categories" multiple="false">
+    <mat-expansion-panel *ngFor="let category of categories" (opened)="openId = category.id" (closed)="panelClosed(category)"
+    [expanded]="openId === category.id" multiple="false">
 
         <!-- Header shows Prefix and name -->
         <mat-expansion-panel-header>
@@ -50,7 +51,7 @@
                             {{ category.prefix }}
                         </div>
                         <div *ngIf="editId === category.id">
-                            {{ this.updateForm.get('prefix').value }}
+                            {{ updateForm.get('prefix').value }}
                         </div>
                     </div>
                     <div class="header-name">
@@ -58,7 +59,7 @@
                             {{ category.name }}
                         </div>
                         <div *ngIf="editId === category.id">
-                            {{ this.updateForm.get('name').value }}
+                            {{ updateForm.get('name').value }}
                         </div>
                     </div>
                     <div class="header-size">
@@ -69,17 +70,17 @@
         </mat-expansion-panel-header>
 
         <!-- Edit form shows during the edit event -->
-        <form [formGroup]='this.updateForm' *ngIf="editId === category.id" (keydown)="keyDownFunction($event, category)">
-            <span translate>Edit category details:</span><br>
+        <form id="updateForm" [formGroup]='updateForm' *ngIf="editId === category.id" (keydown)="keyDownFunction($event, category)">
+            <span translate>Edit category details</span>:<br>
             <mat-form-field>
-                <input formControlName="prefix" matInput placeholder="{{'Prefix' | translate}}">
-                <mat-hint>
+                <input formControlName="prefix" matInput placeholder="{{'Prefix' | translate}}" required>
+                <mat-hint *ngIf="!updateForm.controls.prefix.valid">
                     <span translate>Required</span>
                 </mat-hint>
             </mat-form-field>
             <mat-form-field>
-                <input formControlName="name" matInput placeholder="{{'Name' | translate}}">
-                <mat-hint>
+                <input formControlName="name" matInput placeholder="{{'Name' | translate}}" required>
+                <mat-hint *ngIf="!updateForm.controls.name.valid">
                     <span translate>Required</span>
                 </mat-hint>
             </mat-form-field>
@@ -87,10 +88,10 @@
 
         <!-- Show and sort corresponding motions-->
         <div *ngIf="motionsInCategory(category).length > 0">
-            <span translate>Motions:</span>
+            <span translate>Motions</span>:
             <div *ngIf="editId !== category.id">
                 <ul *ngFor="let motion of motionsInCategory(category)">
-                    <li>{{motion}}</li>
+                    <li>{{ motion }}</li>
                 </ul>
             </div>
             <div *ngIf="editId === category.id">
diff --git a/client/src/app/site/motions/components/category-list/category-list.component.scss b/client/src/app/site/motions/components/category-list/category-list.component.scss
index 57a45ddf3..1da7013bf 100644
--- a/client/src/app/site/motions/components/category-list/category-list.component.scss
+++ b/client/src/app/site/motions/components/category-list/category-list.component.scss
@@ -40,3 +40,7 @@
         text-align: center;
     }
 }
+
+#updateForm {
+    margin-bottom: 20px;
+}
diff --git a/client/src/app/site/motions/components/category-list/category-list.component.ts b/client/src/app/site/motions/components/category-list/category-list.component.ts
index 0392cba56..91ff85ce9 100644
--- a/client/src/app/site/motions/components/category-list/category-list.component.ts
+++ b/client/src/app/site/motions/components/category-list/category-list.component.ts
@@ -32,10 +32,15 @@ export class CategoryListComponent extends BaseViewComponent implements OnInit {
      */
     public editId: number | null;
 
+    /**
+     * Determine which category is opened.
+     */
+    public openId: number | null;
+
     /**
      * Source of the data
      */
-    public categories: Array<ViewCategory>;
+    public categories: ViewCategory[];
 
     /**
      * For new categories
@@ -146,31 +151,36 @@ export class CategoryListComponent extends BaseViewComponent implements OnInit {
 
     /**
      * Saves the category
+     *
+     * TODO: Do not number the motions. This needs to be a separate button (maybe with propting for confirmation), because
+     * not every body uses this and this would destroy their own order in motion identifiers.
+     * See issue #3969
+     *
      * @param viewCategory
      */
     public async onSaveButton(viewCategory: ViewCategory): Promise<void> {
-        if (this.updateForm.valid) {
-            // TODO: Check the motion sorting code below. If it is removed, change to .then() syntax.
-            try {
-                await this.repo.update(this.updateForm.value as Partial<Category>, viewCategory);
-            } catch (e) {
-                this.raiseError(e);
-            }
-            this.onCancelButton();
-            this.sortDataSource();
+        // get the sorted motions. Save them before updating the category.
+        let sortedMotionIds;
+        if (this.sortSelector) {
+            sortedMotionIds = this.sortSelector.array.map(selectable => selectable.id);
+            this.repo.numberMotionsInCategory(viewCategory.category, sortedMotionIds);
         }
 
-        // get the sorted motions
-        if (this.sortSelector) {
-            const manuallySortedMotions = this.sortSelector.array as Motion[];
-            await this.repo.updateCategoryNumbering(viewCategory.category, manuallySortedMotions);
+        if (this.updateForm.valid) {
+            // wait for the category to update; then the (maybe) changed prefix can be applied to the motions
+            await this.repo.update(this.updateForm.value as Partial<Category>, viewCategory);
+            this.onCancelButton();
+
+            if (this.sortSelector) {
+                this.repo.numberMotionsInCategory(viewCategory.category, sortedMotionIds);
+            }
         }
     }
 
     /**
      * sorts the categories by prefix
      */
-    protected sortDataSource(): void {
+    private sortDataSource(): void {
         this.categories.sort((viewCategory1, viewCategory2) => (viewCategory1 > viewCategory2 ? 1 : -1));
     }
 
@@ -196,10 +206,22 @@ export class CategoryListComponent extends BaseViewComponent implements OnInit {
     /**
      * Returns the motions corresponding to a category
      * @param category target
+     * @returns all motions in the category
      */
-    public motionsInCategory(category: Category): Array<Motion> {
-        const motList = this.repo.getMotionsOfCategory(category);
-        motList.sort((motion1, motion2) => (motion1 > motion2 ? 1 : -1));
-        return motList;
+    public motionsInCategory(category: Category): Motion[] {
+        const motions = this.repo.getMotionsOfCategory(category);
+        motions.sort((motion1, motion2) => (motion1 > motion2 ? 1 : -1));
+        return motions;
+    }
+
+    /**
+     * Is executed when a mat-extension-panel is closed
+     * @param viewCategory the category in the panel
+     */
+    public panelClosed(viewCategory: ViewCategory): void {
+        this.openId = null;
+        if (this.editId) {
+            this.onSaveButton(viewCategory);
+        }
     }
 }
diff --git a/client/src/app/site/motions/components/motion-detail/motion-detail.component.ts b/client/src/app/site/motions/components/motion-detail/motion-detail.component.ts
index b6d9f69ea..e2c96d5d3 100644
--- a/client/src/app/site/motions/components/motion-detail/motion-detail.component.ts
+++ b/client/src/app/site/motions/components/motion-detail/motion-detail.component.ts
@@ -323,16 +323,9 @@ export class MotionDetailComponent extends BaseViewComponent implements OnInit {
      * shows a "are you sure" dialog
      */
     public async deleteMotionButton(): Promise<void> {
-        await this.repo.delete(this.motion).then();
-        this.router.navigate(['./motions/']);
-
-        // This should happen during auto update
-        // const motList = this.categoryRepo.getMotionsOfCategory(this.motion.category);
-        // const index = motList.indexOf(this.motion.motion, 0);
-        // if (index > -1) {
-        //     motList.splice(index, 1);
-        // }
-        // this.categoryRepo.updateCategoryNumbering(this.motion.category, motList);
+        this.repo.delete(this.motion).then(() => {
+            this.router.navigate(['./motions/']);
+        }, this.raiseError);
     }
 
     /**
diff --git a/client/src/app/site/motions/models/category-numbering.ts b/client/src/app/site/motions/models/category-numbering.ts
deleted file mode 100644
index ba97c6448..000000000
--- a/client/src/app/site/motions/models/category-numbering.ts
+++ /dev/null
@@ -1,22 +0,0 @@
-import { Motion } from '../../../shared/models/motions/motion';
-
-/**
- * wrapper class for the HTTP Call
- */
-export class CategoryNumbering {
-    private motions: number[];
-
-    public constructor() {}
-
-    public setMotions(motionList: Motion[]): void {
-        const motion_id_list: number[] = [];
-        motionList.forEach(motion => {
-            motion_id_list.push(motion.id);
-        });
-        this.motions = motion_id_list;
-    }
-
-    public getMotions(): number[] {
-        return this.motions;
-    }
-}
diff --git a/client/src/app/site/motions/services/category-repository.service.ts b/client/src/app/site/motions/services/category-repository.service.ts
index 9aca4ba5b..ec966d5ea 100644
--- a/client/src/app/site/motions/services/category-repository.service.ts
+++ b/client/src/app/site/motions/services/category-repository.service.ts
@@ -5,7 +5,6 @@ import { DataSendService } from '../../../core/services/data-send.service';
 import { DataStoreService } from '../../../core/services/data-store.service';
 import { BaseRepository } from '../../base/base-repository';
 import { Motion } from '../../../shared/models/motions/motion';
-import { CategoryNumbering } from '../models/category-numbering';
 import { HttpService } from '../../../core/services/http.service';
 import { Identifiable } from '../../../shared/models/base/identifiable';
 import { CollectionStringModelMapperService } from '../../../core/services/collectionStringModelMapper.service';
@@ -28,7 +27,9 @@ export class CategoryRepositoryService extends BaseRepository<ViewCategory, Cate
      * Creates a CategoryRepository
      * Converts existing and incoming category to ViewCategories
      * Handles CRUD using an observer to the DataStore
-     * @param DataSend
+     * @param DS
+     * @param dataSend
+     * @param httpService
      */
     public constructor(
         protected DS: DataStoreService,
@@ -64,10 +65,10 @@ export class CategoryRepositoryService extends BaseRepository<ViewCategory, Cate
     }
 
     /**
-     * Returns all Motions belonging to a category
+     * Returns all motions belonging to a category
      * @param category category
      */
-    public getMotionsOfCategory(category: Category): Array<Motion> {
+    public getMotionsOfCategory(category: Category): Motion[] {
         const motList = this.DS.getAll(Motion);
         const retList: Array<Motion> = [];
         motList.forEach(motion => {
@@ -89,23 +90,12 @@ export class CategoryRepositoryService extends BaseRepository<ViewCategory, Cate
     }
 
     /**
-     * Updates a Categories numbering
+     * Updates a categories numbering.
      * @param category the category it should be updated in
      * @param motionList the list of motions on this category
      */
-    public async updateCategoryNumbering(category: Category, motionList: Motion[]): Promise<void> {
-        const categoryNumbering = new CategoryNumbering();
-        categoryNumbering.setMotions(motionList);
-        await this.sentCategoryNumbering(category, categoryNumbering);
-    }
-
-    /**
-     * Save category in the server
-     *
-     * @return Observable from
-     */
-    protected async sentCategoryNumbering(category: Category, categoryNumbering: CategoryNumbering): Promise<void> {
+    public async numberMotionsInCategory(category: Category, motionIds: number[]): Promise<void> {
         const collectionString = 'rest/motions/category/' + category.id + '/numbering/';
-        await this.httpService.post(collectionString, categoryNumbering);
+        await this.httpService.post(collectionString, { motions: motionIds });
     }
 }