From a63dfd4e420ac1c2626e02225f468042f3b718e1 Mon Sep 17 00:00:00 2001
From: paf <paf@titelfrei.de>
Date: Wed, 7 Apr 2021 08:41:39 +0200
Subject: [PATCH] fixes a bug that made the checkall-checkbox flicker on
 loading

At the time the stats for checked sessions where updated, the main sessions object was not updated, so the number of checked sessions where compared with the number of sessions during the last change. At the first time the previous number was always null.
---
 .../group-monitor.component.html              | 45 ++++++------
 .../group-monitor.component.spec.ts           |  2 +-
 .../group-monitor/group-monitor.component.ts  |  6 +-
 .../group-monitor/group-monitor.interfaces.ts |  2 +-
 .../group-monitor.service.spec.ts             |  2 +-
 .../group-monitor/group-monitor.service.ts    | 68 +++++++++++--------
 .../test-session.service.spec.ts              |  2 +-
 7 files changed, 69 insertions(+), 58 deletions(-)

diff --git a/src/app/group-monitor/group-monitor.component.html b/src/app/group-monitor/group-monitor.component.html
index 71b5d87e..6f8a0da7 100644
--- a/src/app/group-monitor/group-monitor.component.html
+++ b/src/app/group-monitor/group-monitor.component.html
@@ -110,7 +110,7 @@
         <mat-slide-toggle
             color="accent"
             (change)="toggleAlwaysCheckAll($event)"
-            [disabled]="!gms.checkingOptions.disableAutoCheckAll"
+            [disabled]="!gms.checkingOptions.enableAutoCheckAll"
             [checked]="gms.checkingOptions.autoCheckAll"
         >
           Alle Tests gleichzeitig steuern
@@ -207,7 +207,7 @@
                     (change)="toggleCheckAll($event)"
                     [checked]="checkedStats.all"
                     (contextmenu)="invertChecked($event)"
-                  ></mat-checkbox> <!-- TODO cheked!! -->
+                  ></mat-checkbox>
                 </td>
                 <td mat-sort-header="_superState">
                   <mat-icon>person</mat-icon>
@@ -235,15 +235,15 @@
 
             <ng-container *ngFor="let session of gms.sessions$ | async; trackBy: trackSession">
               <tc-test-view
-                  [testSession]="session"
-                  [displayOptions]="displayOptions"
-                  [marked]="markedElement"
-                  (markedElement$)="markElement($event)"
-                  [selected]="selectedElement"
-                  (selectedElement$)="selectElement($event)"
-                  [checked]="gms.isChecked(session)"
-                  (checked$)="toggleChecked($event, session)"
-                  [ngStyle]="{background: getSessionColor(session)}"
+                [testSession]="session"
+                [displayOptions]="displayOptions"
+                [marked]="markedElement"
+                (markedElement$)="markElement($event)"
+                [selected]="selectedElement"
+                (selectedElement$)="selectElement($event)"
+                [checked]="gms.isChecked(session)"
+                (checked$)="toggleChecked($event, session)"
+                [ngStyle]="{background: getSessionColor(session)}"
               >
               </tc-test-view>
             </ng-container>
@@ -254,21 +254,22 @@
   </mat-sidenav-container>
 
   <button
-      class="drawer-button-close"
-      mat-icon-button
-      (click)="sidenav.toggle()"
-      matTooltip=""
-      matTooltipPosition="right"
+    class="drawer-button-close"
+    mat-icon-button
+    (click)="sidenav.toggle()"
+    matTooltip=""
+    matTooltipPosition="right"
   >
     <mat-icon>chevron_right</mat-icon>
   </button>
 
-  <button *ngIf="sidenav.opened"
-          class="drawer-button-open"
-          mat-icon-button
-          (click)="sidenav.toggle()"
-          matTooltip="{{'Test-Steuerung verbergen' | customtext:'gm_hide_controls_tooltip' | async}}"
-          matTooltipPosition="above"
+  <button
+    *ngIf="sidenav.opened"
+    class="drawer-button-open"
+    mat-icon-button
+    (click)="sidenav.toggle()"
+    matTooltip="{{'Test-Steuerung verbergen' | customtext:'gm_hide_controls_tooltip' | async}}"
+    matTooltipPosition="above"
   >
     <mat-icon>chevron_left</mat-icon>
   </button>
diff --git a/src/app/group-monitor/group-monitor.component.spec.ts b/src/app/group-monitor/group-monitor.component.spec.ts
index acdfb6d7..8066ea77 100644
--- a/src/app/group-monitor/group-monitor.component.spec.ts
+++ b/src/app/group-monitor/group-monitor.component.spec.ts
@@ -54,7 +54,7 @@ class MockBackendService {
 
 class MockGroupMonitorService {
   checkingOptions: CheckingOptions = {
-    disableAutoCheckAll: false,
+    enableAutoCheckAll: false,
     autoCheckAll: true
   };
 
diff --git a/src/app/group-monitor/group-monitor.component.ts b/src/app/group-monitor/group-monitor.component.ts
index e901b229..e41c3d30 100644
--- a/src/app/group-monitor/group-monitor.component.ts
+++ b/src/app/group-monitor/group-monitor.component.ts
@@ -112,7 +112,7 @@ export class GroupMonitorComponent implements OnInit, OnDestroy {
   private onSessionsUpdate(stats: TestSessionSetStats): void {
     this.displayOptions.highlightSpecies = (stats.differentBookletSpecies > 1);
 
-    if (!this.gms.checkingOptions.disableAutoCheckAll) {
+    if (!this.gms.checkingOptions.enableAutoCheckAll) {
       this.displayOptions.manualChecking = true;
     }
   }
@@ -196,7 +196,7 @@ export class GroupMonitorComponent implements OnInit, OnDestroy {
     dialogRef.afterClosed().subscribe((confirmed: boolean) => {
       if (confirmed) {
         this.isClosing = true;
-        this.gms.finishEverything()
+        this.gms.commandFinishEverything()
           .subscribe(() => {
             setTimeout(() => { this.router.navigateByUrl('/r/login'); }, 5000); // go away
           });
@@ -235,7 +235,7 @@ export class GroupMonitorComponent implements OnInit, OnDestroy {
   }
 
   toggleAlwaysCheckAll(event: MatSlideToggleChange): void {
-    if (this.gms.checkingOptions.disableAutoCheckAll && !event.checked) {
+    if (this.gms.checkingOptions.enableAutoCheckAll && !event.checked) {
       this.gms.checkAll();
       this.displayOptions.manualChecking = false;
       this.gms.checkingOptions.autoCheckAll = true;
diff --git a/src/app/group-monitor/group-monitor.interfaces.ts b/src/app/group-monitor/group-monitor.interfaces.ts
index 9ca8795a..2600e48b 100644
--- a/src/app/group-monitor/group-monitor.interfaces.ts
+++ b/src/app/group-monitor/group-monitor.interfaces.ts
@@ -111,7 +111,7 @@ export interface TestViewDisplayOptions {
 }
 
 export interface CheckingOptions {
-  disableAutoCheckAll: boolean;
+  enableAutoCheckAll: boolean;
   autoCheckAll: boolean;
 }
 
diff --git a/src/app/group-monitor/group-monitor.service.spec.ts b/src/app/group-monitor/group-monitor.service.spec.ts
index 58876f0f..5408c9cf 100644
--- a/src/app/group-monitor/group-monitor.service.spec.ts
+++ b/src/app/group-monitor/group-monitor.service.spec.ts
@@ -161,7 +161,7 @@ describe('GroupMonitorService', () => {
 
   describe('getSessionSetStats', () => {
     it('should fetch correct stats from sessions', () => {
-      const result = service.getSessionSetStats(unitTestExampleSessions);
+      const result = GroupMonitorService.getSessionSetStats(unitTestExampleSessions, 2);
       const expectation: TestSessionSetStats = {
         number: 3,
         differentBooklets: 3,
diff --git a/src/app/group-monitor/group-monitor.service.ts b/src/app/group-monitor/group-monitor.service.ts
index ff9d488e..7230be0d 100644
--- a/src/app/group-monitor/group-monitor.service.ts
+++ b/src/app/group-monitor/group-monitor.service.ts
@@ -31,6 +31,8 @@ import { ConnectionStatus } from '../shared/websocket-backend.service';
  * #--> STAND. getCurrent zählt freie units zum letzten Block ?! (unit test schreiben!)
  * - select all checkbox ist zunächst angewählt
  * - unter-testlet lässt sich auswählen!
+ * - filter namen customisieren
+ * - alle gleichzeitig schlater muss neu rendern zur Folge ahben
  * # kombinierte hintergrundfarben
  * descendantCount -> unit count (not direct!)
  * tidy:
@@ -115,7 +117,7 @@ export class GroupMonitorService {
     this.sortBy$ = new BehaviorSubject<Sort>({ direction: 'asc', active: 'personLabel' });
     this.filters$ = new BehaviorSubject<TestSessionFilter[]>([]);
     this.checkingOptions = {
-      disableAutoCheckAll: true,
+      enableAutoCheckAll: true,
       autoCheckAll: true
     };
 
@@ -159,6 +161,7 @@ export class GroupMonitorService {
     );
   }
 
+  // todo unit test
   private static filterSessions(sessions: TestSession[], filters: TestSessionFilter[]): TestSession[] {
     return sessions
       .filter(session => session.data.testId && session.data.testId > -1) // testsession without testId is deprecated
@@ -204,11 +207,11 @@ export class GroupMonitorService {
   }
 
   private synchronizeChecked(sessions: TestSession[]): void {
-    const sessionsStats = this.getSessionSetStats(sessions);
+    const sessionsStats = GroupMonitorService.getSessionSetStats(sessions);
 
-    this.checkingOptions.disableAutoCheckAll = (sessionsStats.differentBookletSpecies < 2);
+    this.checkingOptions.enableAutoCheckAll = (sessionsStats.differentBookletSpecies < 2);
 
-    if (!this.checkingOptions.disableAutoCheckAll) {
+    if (!this.checkingOptions.enableAutoCheckAll) {
       this.checkingOptions.autoCheckAll = false;
     }
 
@@ -221,7 +224,7 @@ export class GroupMonitorService {
       });
     this._checked = newCheckedSessions;
 
-    this._checkedStats$.next(this.getSessionSetStats(Object.values(this._checked)));
+    this._checkedStats$.next(GroupMonitorService.getSessionSetStats(Object.values(this._checked), sessions.length));
     this._sessionsStats$.next(sessionsStats);
   }
 
@@ -263,6 +266,7 @@ export class GroupMonitorService {
       });
   }
 
+  // todo unit test
   testCommandResume(): void {
     const testIds = this.checked
       .filter(TestSessionService.isPaused)
@@ -276,6 +280,7 @@ export class GroupMonitorService {
     );
   }
 
+  // todo unit test
   testCommandPause(): void {
     const testIds = this.checked
       .filter(session => !TestSessionService.isPaused(session))
@@ -289,6 +294,7 @@ export class GroupMonitorService {
     );
   }
 
+  // todo unit test
   testCommandGoto(selection: Selection): void {
     const allTestIds: number[] = [];
     const groupedByBooklet: {
@@ -324,6 +330,7 @@ export class GroupMonitorService {
     });
   }
 
+  // todo unit test
   testCommandUnlock(): void {
     const testIds = this.checked
       .filter(TestSessionService.isLocked)
@@ -337,10 +344,33 @@ export class GroupMonitorService {
     );
   }
 
+  // todo unit test
+  commandFinishEverything(): Observable<CommandResponse> {
+    const getUnlockedConnectedTestIds = () => Object.values(this._sessions$.getValue())
+      .filter(session => !TestSessionService.hasState(session.data.testState, 'status', 'locked') &&
+        !TestSessionService.hasState(session.data.testState, 'CONTROLLER', 'TERMINATED') &&
+        (TestSessionService.hasState(session.data.testState, 'CONNECTION', 'POLLING') ||
+          TestSessionService.hasState(session.data.testState, 'CONNECTION', 'WEBSOCKET')))
+      .map(session => session.data.testId);
+    const getUnlockedTestIds = () => Object.values(this._sessions$.getValue())
+      .filter(session => session.data.testId > 0)
+      .filter(session => !TestSessionService.hasState(session.data.testState, 'status', 'locked'))
+      .map(session => session.data.testId);
+
+    this.filters$.next([]);
+
+    return this.bs.command('terminate', [], getUnlockedConnectedTestIds())
+      .pipe(
+        delay(1900),
+        flatMap(() => this.bs.lock(this.groupName, getUnlockedTestIds()))
+      );
+  }
+
   isChecked(session: TestSession): boolean {
     return (typeof this._checked[session.id] !== 'undefined');
   }
 
+  // todo unit test
   checkSessionsBySelection(selected: Selection): void {
     if (this.checkingOptions.autoCheckAll) {
       return;
@@ -410,10 +440,11 @@ export class GroupMonitorService {
   }
 
   private onCheckedChanged(): void {
-    this._checkedStats$.next(this.getSessionSetStats(this.checked));
+    this._checkedStats$.next(GroupMonitorService.getSessionSetStats(this.checked));
   }
 
-  getSessionSetStats(sessionSet: TestSession[]): TestSessionSetStats { // TODO only private for test
+  // TODO only public for test
+  static getSessionSetStats(sessionSet: TestSession[], allCount: number = sessionSet.length): TestSessionSetStats {
     const booklets = new Set();
     const bookletSpecies = new Set();
     let paused = 0;
@@ -431,30 +462,9 @@ export class GroupMonitorService {
       number: sessionSet.length,
       differentBooklets: booklets.size,
       differentBookletSpecies: bookletSpecies.size,
-      all: (this.sessions.length === sessionSet.length),
+      all: (allCount === sessionSet.length),
       paused,
       locked
     };
   }
-
-  finishEverything(): Observable<CommandResponse> {
-    const getUnlockedConnectedTestIds = () => Object.values(this._sessions$.getValue())
-      .filter(session => !TestSessionService.hasState(session.data.testState, 'status', 'locked') &&
-                         !TestSessionService.hasState(session.data.testState, 'CONTROLLER', 'TERMINATED') &&
-                         (TestSessionService.hasState(session.data.testState, 'CONNECTION', 'POLLING') ||
-                         TestSessionService.hasState(session.data.testState, 'CONNECTION', 'WEBSOCKET')))
-      .map(session => session.data.testId);
-    const getUnlockedTestIds = () => Object.values(this._sessions$.getValue())
-      .filter(session => session.data.testId > 0)
-      .filter(session => !TestSessionService.hasState(session.data.testState, 'status', 'locked'))
-      .map(session => session.data.testId);
-
-    this.filters$.next([]);
-
-    return this.bs.command('terminate', [], getUnlockedConnectedTestIds())
-      .pipe(
-        delay(1900),
-        flatMap(() => this.bs.lock(this.groupName, getUnlockedTestIds()))
-      );
-  }
 }
diff --git a/src/app/group-monitor/test-session.service.spec.ts b/src/app/group-monitor/test-session.service.spec.ts
index 5922184a..f2f428c5 100644
--- a/src/app/group-monitor/test-session.service.spec.ts
+++ b/src/app/group-monitor/test-session.service.spec.ts
@@ -21,7 +21,7 @@ describe('TestSessionService', () => {
       .toBeTruthy();
   });
 
-  fdescribe('getCurrent()', () => {
+  describe('getCurrent()', () => {
     it('should find correct indices for unit, parent and ancestor ( = top-level-testlet or root)', () => {
       const fakeTestlet = (id: string): Testlet => ({
         id,
-- 
GitLab