Merge pull request #4390 from tsiegleauq/hotfix-sort-performance

Disable various expensive sorting functions

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

@@ -115,6 +115,8 @@ export class CategoryRepositoryService extends BaseRepository<ViewCategory, Cate
      *
      * @param categories
      * @returns the categories sorted by prefix or name, according to the config setting
+     *
+     * TODO: That operation is HEAVY
      */
     public sortViewCategoriesByConfig(categories: ViewCategory[]): ViewCategory[] {
         const sort = this.configService.instant<'prefix' | 'name'>('motions_category_sorting') || 'prefix';

client/src/app/core/repositories/users/user-repository.service.ts

@@ -305,6 +305,7 @@ export class UserRepositoryService extends BaseRepository<ViewUser, User> {
 
     /**
      * Returns all duplicates of an user (currently: full name matches)
+     *
      * @param user
      */
     public getUserDuplicates(user: ViewUser): ViewUser[] {
@@ -313,6 +314,8 @@ export class UserRepositoryService extends BaseRepository<ViewUser, User> {
 
     /**
      * @returns the observable for users sorted according to configuration
+     *
+     * TODO: This is leading to heavy operations
      */
     public getSortedViewModelListObservable(): Observable<ViewUser[]> {
         const subject = new BehaviorSubject<ViewUser[]>([]);
@@ -329,6 +332,8 @@ export class UserRepositoryService extends BaseRepository<ViewUser, User> {
      * @returns the users sorted by first name, last name or number, according
      * to the config setting. Fallthrough and identical cases will be sorted by
      * 'short_name'
+     *
+     * TODO: That operation is HEAVY
      */
     public sortViewUsersByConfig(users: ViewUser[]): ViewUser[] {
         const sort = this.configService.instant<'first_name' | 'last_name' | 'number'>('users_sort_by') || 'last_name';

client/src/app/site/agenda/components/agenda-list/agenda-list.component.html

@@ -90,9 +90,12 @@
         <ng-container matColumnDef="menu">
             <mat-header-cell *matHeaderCellDef mat-sort-header>Menu</mat-header-cell>
             <mat-cell *matCellDef="let item">
-                <button mat-icon-button
+                <button
+                    mat-icon-button
                     *osPerms="'agenda.can_manage'"
-                [matMenuTriggerFor]="singleItemMenu" [matMenuTriggerData]="{ item: item }">
+                    [matMenuTriggerFor]="singleItemMenu"
+                    [matMenuTriggerData]="{ item: item }"
+                >
                     <mat-icon>more_vert</mat-icon>
                 </button>
             </mat-cell>

client/src/app/site/assignments/models/view-assignment.ts

@@ -65,9 +65,7 @@ export class ViewAssignment extends BaseAgendaViewModel {
         this._tags = tags;
     }
 
-    public updateDependencies(update: BaseViewModel): void {
+    public updateDependencies(update: BaseViewModel): void {}
-        console.log('TODO: assignment updateDependencies');
-    }
 
     public getAgendaItem(): ViewItem {
         return this.agendaItem;

client/src/app/site/base/base-view.ts

@@ -5,6 +5,7 @@ import { MatSnackBar, MatSnackBarRef, SimpleSnackBar } from '@angular/material';
 import { TranslateService } from '@ngx-translate/core';
 
 import { BaseComponent } from '../../base.component';
+import { Subscription } from 'rxjs';
 
 /**
  * A base class for all views. Implements a generic error handling by raising a snack bar
@@ -17,6 +18,11 @@ export abstract class BaseViewComponent extends BaseComponent implements OnDestr
      */
     private messageSnackBar: MatSnackBarRef<SimpleSnackBar>;
 
+    /**
+     * Subscriptions added to this list will be cleared 'on destroy'
+     */
+    protected subscriptions: Subscription[];
+
     /**
      * Constructor for bas elist views
      * @param titleService the title serivce, passed to the base component
@@ -25,6 +31,7 @@ export abstract class BaseViewComponent extends BaseComponent implements OnDestr
      */
     public constructor(titleService: Title, translate: TranslateService, private matSnackBar: MatSnackBar) {
         super(titleService, translate);
+        this.subscriptions = [];
     }
 
     /**
@@ -58,11 +65,18 @@ export abstract class BaseViewComponent extends BaseComponent implements OnDestr
     }
 
     /**
-     * automatically dismisses the error snack bar, if the component is destroyed.
+     * automatically dismisses the error snack bar and clears subscriptions
+     * if the component is destroyed.
      */
     public ngOnDestroy(): void {
         if (this.messageSnackBar) {
             this.messageSnackBar.dismiss();
         }
+
+        if (this.subscriptions.length > 0) {
+            for (const sub of this.subscriptions) {
+                sub.unsubscribe();
+            }
+        }
     }
 }

client/src/app/site/base/list-view-base.ts

@@ -96,20 +96,24 @@ export abstract class ListViewBaseComponent<V extends BaseViewModel, M extends B
      * Standard filtering function. Sufficient for most list views but can be overwritten
      */
     protected onFilter(): void {
-        this.filterService.filter().subscribe(filteredData => (this.sortService.data = filteredData));
+        this.subscriptions.push(
+            this.filterService.filter().subscribe(filteredData => (this.sortService.data = filteredData))
+        );
     }
 
     /**
      * Standard sorting function. Siffucient for most list views but can be overwritten
      */
     protected onSort(): void {
+        this.subscriptions.push(
             this.sortService.sort().subscribe(sortedData => {
                 // the dataArray needs to be cleared (since angular 7)
                 // changes are not detected properly anymore
                 this.dataSource.data = [];
                 this.dataSource.data = sortedData;
                 this.checkSelection();
-        });
+            })
+        );
     }
 
     public onSortButton(itemProperty: string): void {

client/src/app/site/motions/components/manage-submitters/manage-submitters.component.ts

@@ -80,12 +80,6 @@ export class ManageSubmittersComponent extends BaseViewComponent {
         this.addSubmitterForm = new FormGroup({ userId: new FormControl([]) });
         this.editSubmitterObservable = this.editSubmitterSubject.asObservable();
 
-        // get all users for the submitter add form
-        this.users = new BehaviorSubject<ViewUser[]>(
-            this.userRepository.sortViewUsersByConfig(this.userRepository.getViewModelList())
-        );
-        this.userRepository.getSortedViewModelListObservable().subscribe(users => this.users.next(users));
-
         // detect changes in the form
         this.addSubmitterForm.valueChanges.subscribe(formResult => {
             if (formResult && formResult.userId) {
@@ -101,6 +95,10 @@ export class ManageSubmittersComponent extends BaseViewComponent {
         this.isEditMode = true;
         this.editSubmitterSubject.next(this.motion.submitters.map(x => x));
         this.addSubmitterForm.reset();
+
+        // get all users for the submitter add form
+        this.users = new BehaviorSubject<ViewUser[]>(this.userRepository.getViewModelList());
+        this.userRepository.getViewModelListObservable().subscribe(users => this.users.next(users));
     }
 
     /**

client/src/app/site/motions/components/motion-detail/motion-detail.component.ts

@@ -402,16 +402,6 @@ export class MotionDetailComponent extends BaseViewComponent implements OnInit,
     ) {
         super(title, translate, matSnackBar);
 
-        // Initial Filling of the Subjects
-        this.submitterObserver = new BehaviorSubject(
-            this.userRepo.sortViewUsersByConfig(this.userRepo.getViewModelList())
-        );
-        this.supporterObserver = new BehaviorSubject(
-            this.userRepo.sortViewUsersByConfig(this.userRepo.getViewModelList())
-        );
-        this.categoryObserver = new BehaviorSubject(
-            this.categoryRepo.sortViewCategoriesByConfig(this.viewModelStore.getAll(ViewCategory))
-        );
         this.workflowObserver = new BehaviorSubject(this.viewModelStore.getAll(ViewWorkflow));
         this.blockObserver = new BehaviorSubject(this.viewModelStore.getAll(ViewMotionBlock));
         this.mediafilesObserver = new BehaviorSubject(this.viewModelStore.getAll(ViewMediafile));
@@ -419,13 +409,6 @@ export class MotionDetailComponent extends BaseViewComponent implements OnInit,
         this.tagObserver = new BehaviorSubject(this.viewModelStore.getAll(ViewTag));
         this.motionObserver = new BehaviorSubject(this.viewModelStore.getAll(ViewMotion));
 
-        this.userRepo.getSortedViewModelListObservable().subscribe(sortedUsers => {
-            this.submitterObserver.next(sortedUsers);
-            this.supporterObserver.next(sortedUsers);
-        });
-        this.categoryRepo.getSortedViewModelListObservable().subscribe(sortedCategories => {
-            this.categoryObserver.next(sortedCategories);
-        });
         // Make sure the subjects are updated, when a new Model for the type arrives
         // TODO get rid of DS here
         this.DS.changeObservable.subscribe(newModel => {
@@ -477,6 +460,23 @@ export class MotionDetailComponent extends BaseViewComponent implements OnInit,
         this.getMotionByUrl();
         this.setSurroundingMotions();
 
+        // TODO: Changed to un-sort, since it's a really heavy operation
+        this.userRepo.getViewModelListObservable().subscribe(unsortedUsers => {
+            this.submitterObserver.next(unsortedUsers);
+            this.supporterObserver.next(unsortedUsers);
+        });
+
+        this.categoryRepo.getViewModelListObservable().subscribe(unsortedCategories => {
+            this.categoryObserver.next(unsortedCategories);
+        });
+
+        // Initial Filling of the Subjects
+        this.submitterObserver = new BehaviorSubject(this.userRepo.getViewModelList());
+        this.supporterObserver = new BehaviorSubject(this.userRepo.getViewModelList());
+        this.categoryObserver = new BehaviorSubject(
+            this.categoryRepo.sortViewCategoriesByConfig(this.viewModelStore.getAll(ViewCategory))
+        );
+
         this.statuteRepo.getViewModelListObservable().subscribe(newViewStatuteParagraphs => {
             this.statuteParagraphs = newViewStatuteParagraphs;
         });
@@ -501,6 +501,10 @@ export class MotionDetailComponent extends BaseViewComponent implements OnInit,
         });
     }
 
+    /**
+     * Called during view destruction.
+     * Sends a notification to user editors of the motion was edited
+     */
     public ngOnDestroy(): void {
         this.unsubscribeEditNotifications(TypeOfNotificationViewMotion.TYPE_CLOSING_EDITING_MOTION);
     }
@@ -1086,6 +1090,7 @@ export class MotionDetailComponent extends BaseViewComponent implements OnInit,
      * then appending motion without identifiers sorted by title
      */
     public setSurroundingMotions(): void {
+        // TODO: that operation is HEAVY
         this.motionObserver.value.sort((a, b) => {
             if (a.identifier && b.identifier) {
                 return a.identifier.localeCompare(b.identifier, this.translate.currentLang);