SF-3743 Fix checking slowdown when a project has many questions

src/SIL.XForge.Scripture/ClientApp/src/app/checking/attach-audio/attach-audio.component.ts

@@ -1,5 +1,5 @@
 import { NgClass } from '@angular/common';
-import { Component, Input, ViewChild } from '@angular/core';
+import { ChangeDetectorRef, Component, Input, ViewChild } from '@angular/core';
 import { MatIconButton } from '@angular/material/button';
 import { MatDialogRef } from '@angular/material/dialog';
 import { MatIcon } from '@angular/material/icon';
@@ -36,7 +36,10 @@ export class AttachAudioComponent {
 
   protected uploadAudioFile: File = {} as File;
 
-  constructor(private readonly dialogService: DialogService) {}
+  constructor(
+    private readonly changeDetector: ChangeDetectorRef,
+    private readonly dialogService: DialogService
+  ) {}
 
   get audioUrl(): string | undefined {
     return this.textAndAudio?.input?.audioUrl;
@@ -52,6 +55,7 @@ export class AttachAudioComponent {
       >(AudioRecorderDialogComponent, { data: config });
     const result: AudioRecorderDialogResult | undefined = await firstValueFrom(recorderDialogRef.afterClosed());
     if (result?.audio != null && this.textAndAudio != null) {
+      this.changeDetector.markForCheck();
       this.textAndAudio.setAudioAttachment(result.audio);
     }
   }

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking-overview/checking-overview.component.spec.ts

@@ -1,6 +1,7 @@
 import { Location } from '@angular/common';
 import { DebugElement, NgZone } from '@angular/core';
 import { ComponentFixture, discardPeriodicTasks, fakeAsync, flush, TestBed, tick } from '@angular/core/testing';
+import { MatDialogRef } from '@angular/material/dialog';
 import { MatExpansionPanel } from '@angular/material/expansion';
 import { By } from '@angular/platform-browser';
 import { ActivatedRoute } from '@angular/router';
@@ -26,7 +27,7 @@ import { createTestProjectUserConfig } from 'realtime-server/lib/esm/scripturefo
 import { TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
 import { VerseRefData } from 'realtime-server/lib/esm/scriptureforge/models/verse-ref-data';
 import { of } from 'rxjs';
-import { anything, mock, resetCalls, verify, when } from 'ts-mockito';
+import { anything, instance, mock, resetCalls, verify, when } from 'ts-mockito';
 import { DialogService } from 'xforge-common/dialog.service';
 import { NoticeService } from 'xforge-common/notice.service';
 import { OnlineStatusService } from 'xforge-common/online-status.service';
@@ -51,6 +52,7 @@ import { CheckingOverviewComponent } from './checking-overview.component';
 
 const mockedActivatedRoute = mock(ActivatedRoute);
 const mockedDialogService = mock(DialogService);
+const mockedImportQuestionsDialogRef = mock(MatDialogRef);
 const mockedNoticeService = mock(NoticeService);
 const mockedProjectService = mock(SFProjectService);
 const mockedQuestionsService = mock(CheckingQuestionsService);
@@ -88,13 +90,13 @@ describe('CheckingOverviewComponent', () => {
     }));
 
     it('should not display loading if user is offline', fakeAsync(() => {
-      const env = new TestEnvironment();
-      env.testOnlineStatusService.setIsOnline(false);
-      tick();
-      env.fixture.detectChanges();
+      const env = new TestEnvironment(false);
+      env.onlineStatus = false;
+      expect(env.component.showQuestionsLoadingMessage).toBe(false);
+      expect(env.component.showNoQuestionsMessage).toBe(true);
+      env.waitForQuestions();
       expect(env.loadingQuestionsLabel).toBeNull();
       expect(env.noQuestionsLabel).not.toBeNull();
-      env.waitForQuestions();
     }));
 
     it('should not display "Add question" button for community checker', fakeAsync(() => {
@@ -425,10 +427,7 @@ describe('CheckingOverviewComponent', () => {
       await questionDoc.submitJson0Op(op => {
         op.set(d => d.isArchived, false);
       });
-      env.testOnlineStatusService.setIsOnline(false);
-      env.fixture.detectChanges();
-      tick();
-      env.fixture.detectChanges();
+      env.onlineStatus = false;
       expect(env.loadingArchivedQuestionsLabel).toBeNull();
       expect(env.noArchivedQuestionsLabel).not.toBeNull();
 
@@ -985,6 +984,10 @@ class TestEnvironment {
         projectDoc.submitJson0Op(op => op.set(p => p.texts[textIndex].chapters[chapterIndex].hasAudio, false), false);
       }
     );
+    when(mockedImportQuestionsDialogRef.afterClosed()).thenReturn(of(undefined));
+    when(mockedDialogService.openMatDialog(ImportQuestionsDialogComponent, anything())).thenReturn(
+      instance(mockedImportQuestionsDialogRef)
+    );
     this.setCurrentUser(this.adminUser);
     this.testOnlineStatusService.setIsOnline(true);
 
@@ -1112,6 +1115,8 @@ class TestEnvironment {
     this.testOnlineStatusService.setIsOnline(isOnline);
     tick();
     this.fixture.detectChanges();
+    tick();
+    this.fixture.detectChanges();
   }
 
   get warningSomeActionsUnavailableOffline(): DebugElement {

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking-overview/checking-overview.component.ts

@@ -1,5 +1,6 @@
 import { NgClass } from '@angular/common';
-import { Component, DestroyRef, OnDestroy, OnInit } from '@angular/core';
+import { ChangeDetectionStrategy, ChangeDetectorRef, Component, DestroyRef, OnDestroy, OnInit } from '@angular/core';
+import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
 import { MatButton, MatIconButton, MatMiniFabButton } from '@angular/material/button';
 import { MatCard, MatCardContent } from '@angular/material/card';
 import {
@@ -20,8 +21,8 @@ import { Operation } from 'realtime-server/lib/esm/common/models/project-rights'
 import { SF_PROJECT_RIGHTS, SFProjectDomain } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-rights';
 import { Chapter, TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
 import { toVerseRef, VerseRefData } from 'realtime-server/lib/esm/scriptureforge/models/verse-ref-data';
-import { asyncScheduler, merge, Subscription } from 'rxjs';
-import { map, tap, throttleTime } from 'rxjs/operators';
+import { asyncScheduler, combineLatest, merge, Subscription } from 'rxjs';
+import { map, startWith, tap, throttleTime } from 'rxjs/operators';
 import { DataLoadingComponent } from 'xforge-common/data-loading-component';
 import { DialogService } from 'xforge-common/dialog.service';
 import { DonutChartComponent } from 'xforge-common/donut-chart/donut-chart.component';
@@ -73,22 +74,26 @@ import { QuestionDialogService } from '../question-dialog/question-dialog.servic
     MatCard,
     MatCardContent,
     L10nNumberPipe
-  ]
+  ],
+  changeDetection: ChangeDetectionStrategy.OnPush
 })
 export class CheckingOverviewComponent extends DataLoadingComponent implements OnInit, OnDestroy {
   texts: TextInfo[] = [];
   projectId?: string;
+  questionsLoaded: boolean = false;
 
   private questionDocs = new Map<string, QuestionDoc[]>();
   private textsByBookId?: TextsByBookId;
   private projectDoc?: SFProjectProfileDoc;
   private dataChangesSub?: Subscription;
+  private questionsLoadedSub?: Subscription;
   private projectUserConfigDoc?: SFProjectUserConfigDoc;
   private questionsQuery?: RealtimeQuery<QuestionDoc>;
 
   constructor(
     private readonly destroyRef: DestroyRef,
     private readonly activatedRoute: ActivatedRoute,
+    private readonly changeDetector: ChangeDetectorRef,
     private readonly dialogService: DialogService,
     noticeService: NoticeService,
     readonly i18n: I18nService,
@@ -207,11 +212,6 @@ export class CheckingOverviewComponent extends DataLoadingComponent implements O
     return this.questionsQuery.docs.filter(qd => qd.data != null && !qd.data.isArchived);
   }
 
-  private get questionsLoaded(): boolean {
-    // if the user is offline, 'ready' will never be true, but the query will still return the offline docs
-    return !this.onlineStatusService.isOnline || this.questionsQuery?.ready === true;
-  }
-
   ngOnInit(): void {
     let projectDocPromise: Promise<SFProjectProfileDoc>;
     const projectId$ = this.activatedRoute.params.pipe(
@@ -223,6 +223,7 @@ export class CheckingOverviewComponent extends DataLoadingComponent implements O
     );
     projectId$.pipe(quietTakeUntilDestroyed(this.destroyRef)).subscribe(async projectId => {
       this.loadingStarted();
+      this.changeDetector.markForCheck();
       this.projectId = projectId;
       try {
         this.projectDoc = await projectDocPromise;
@@ -238,14 +239,13 @@ export class CheckingOverviewComponent extends DataLoadingComponent implements O
         this.loadingFinished();
       }
 
-      if (this.dataChangesSub != null) {
-        this.dataChangesSub.unsubscribe();
-      }
+      this.dataChangesSub?.unsubscribe();
       this.dataChangesSub = merge(
         this.projectDoc.remoteChanges$,
         this.questionsQuery.remoteChanges$,
         this.questionsQuery.localChanges$
       )
+        .pipe(quietTakeUntilDestroyed(this.destroyRef))
         // TODO Find a better solution than merely throttling remote changes
         .pipe(throttleTime(1000, asyncScheduler, { leading: true, trailing: true }))
         .subscribe(() => {
@@ -255,11 +255,30 @@ export class CheckingOverviewComponent extends DataLoadingComponent implements O
             }
           }
         });
+
+      this.questionsLoadedSub?.unsubscribe();
+      this.questionsLoadedSub = combineLatest([
+        this.onlineStatusService.onlineStatus$.pipe(startWith(this.onlineStatusService.isOnline)),
+        this.questionsQuery.ready$.pipe(startWith(this.questionsQuery?.ready))
+      ])
+        .pipe(quietTakeUntilDestroyed(this.destroyRef))
+        .pipe(map(([isOnline, ready]) => !isOnline || ready === true))
+        .subscribe(loaded => {
+          // Show the loading indicator if the questions are not yet ready
+          if (!loaded) {
+            this.loadingStarted();
+          } else {
+            this.loadingFinished();
+          }
+
+          // if the user is offline, 'ready' will never be true, but the query will still return the offline docs
+          this.questionsLoaded = loaded;
+          this.changeDetector.markForCheck();
+        });
     });
   }
 
   ngOnDestroy(): void {
-    this.dataChangesSub?.unsubscribe();
     this.questionsQuery?.dispose();
   }
 
@@ -356,6 +375,8 @@ export class CheckingOverviewComponent extends DataLoadingComponent implements O
           if (questionDoc.data!.isArchived !== archive) this.setQuestionArchiveStatus(questionDoc, archive);
         }
       }
+
+      this.changeDetector.markForCheck();
     }
   }
 
@@ -364,6 +385,8 @@ export class CheckingOverviewComponent extends DataLoadingComponent implements O
       for (const questionDoc of this.getQuestionDocs(this.getTextDocIdType(text.bookNum, chapter.number), !archive)) {
         if (questionDoc.data!.isArchived !== archive) this.setQuestionArchiveStatus(questionDoc, archive);
       }
+
+      this.changeDetector.markForCheck();
     }
   }
 
@@ -454,7 +477,15 @@ export class CheckingOverviewComponent extends DataLoadingComponent implements O
       userId: this.userService.currentUserId,
       textsByBookId: this.textsByBookId
     };
-    this.dialogService.openMatDialog(ImportQuestionsDialogComponent, { data });
+    this.changeDetector.detach();
+    const dialogRef = this.dialogService.openMatDialog(ImportQuestionsDialogComponent, { data });
+    dialogRef
+      .afterClosed()
+      .pipe(takeUntilDestroyed(this.destroyRef))
+      .subscribe(() => {
+        this.changeDetector.reattach();
+        this.changeDetector.markForCheck();
+      });
   }
 
   getBookName(text: TextInfo): string {
@@ -511,6 +542,8 @@ export class CheckingOverviewComponent extends DataLoadingComponent implements O
     for (const questionDoc of this.questionsQuery.docs) {
       this.addQuestionDoc(questionDoc);
     }
+
+    this.changeDetector.markForCheck();
   }
 
   private addQuestionDoc(questionDoc: QuestionDoc): void {

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-answers/checking-answers.component.ts

@@ -1,6 +1,7 @@
 import { Dir } from '@angular/cdk/bidi';
 import { NgClass } from '@angular/common';
 import {
+  ChangeDetectorRef,
   Component,
   DestroyRef,
   EventEmitter,
@@ -155,6 +156,7 @@ export class CheckingAnswersComponent implements OnInit {
     private readonly fileService: FileService,
     private readonly onlineStatusService: OnlineStatusService,
     private readonly projectService: SFProjectService,
+    private readonly changeDetector: ChangeDetectorRef,
     private destroyRef: DestroyRef
   ) {}
 
@@ -192,14 +194,16 @@ export class CheckingAnswersComponent implements OnInit {
     if (questionDoc == null) {
       return;
     }
-    void this.updateQuestionDocAudioUrls();
+    this.changeDetector.markForCheck();
+    this.updateQuestionDocAudioUrls();
     if (this.questionChangeSubscription != null) {
       this.questionChangeSubscription!.unsubscribe();
     }
     this.questionChangeSubscription = questionDoc.remoteChanges$
       .pipe(quietTakeUntilDestroyed(this.destroyRef))
       .subscribe(ops => {
-        void this.updateQuestionDocAudioUrls();
+        this.changeDetector.markForCheck();
+        this.updateQuestionDocAudioUrls();
         // If the user hasn't added an answer yet and is able to, then
         // don't hold back any incoming answers from appearing right away
         // as soon as the user adds their answer.
@@ -215,7 +219,10 @@ export class CheckingAnswersComponent implements OnInit {
             const answer = this.allAnswers[op.p[1]];
             if (this.answersHighlightStatus.has(answer.dataId)) {
               this.answersHighlightStatus.set(answer.dataId, false);
-              setTimeout(() => this.answersHighlightStatus.set(answer.dataId, true));
+              setTimeout(() => {
+                this.answersHighlightStatus.set(answer.dataId, true);
+                this.changeDetector.markForCheck();
+              });
             }
           }
         }
@@ -401,7 +408,8 @@ export class CheckingAnswersComponent implements OnInit {
     };
     const dialogResponseDoc: QuestionDoc | undefined = await this.questionDialogService.questionDialog(data);
     if (dialogResponseDoc?.data != null) {
-      void this.updateQuestionDocAudioUrls();
+      this.changeDetector.markForCheck();
+      this.updateQuestionDocAudioUrls();
       this.action.emit({ action: 'edit', questionDoc: dialogResponseDoc });
     }
   }
@@ -526,6 +534,7 @@ export class CheckingAnswersComponent implements OnInit {
     const userDoc = await this.userService.getCurrentUser();
     if (this.onlineStatusService.isOnline && userDoc.data?.isDisplayNameConfirmed !== true) {
       await this.userService.editDisplayName(true);
+      this.changeDetector.markForCheck();
     }
     this.emitAnswerToSave(response);
   }
@@ -577,14 +586,18 @@ export class CheckingAnswersComponent implements OnInit {
     return result;
   }
 
-  private async updateQuestionDocAudioUrls(): Promise<void> {
+  private updateQuestionDocAudioUrls(): void {
     this.fileSources.clear();
     if (this.questionDoc?.data == null) {
       return;
     }
-    void this.cacheFileSource(this.questionDoc, this.questionDoc.data.dataId, this.questionDoc.data.audioUrl);
+    void this.cacheFileSource(this.questionDoc, this.questionDoc.data.dataId, this.questionDoc.data.audioUrl).then(() =>
+      this.changeDetector.markForCheck()
+    );
     for (const answer of this.questionDoc.getAnswers()) {
-      void this.cacheFileSource(this.questionDoc, answer.dataId, answer.audioUrl);
+      void this.cacheFileSource(this.questionDoc, answer.dataId, answer.audioUrl).then(() =>
+        this.changeDetector.markForCheck()
+      );
     }
   }
 
@@ -606,6 +619,7 @@ export class CheckingAnswersComponent implements OnInit {
       for (const answer of this.answers) {
         this.answersHighlightStatus.set(answer.dataId, this.shouldDrawAttentionToAnswer(answer));
       }
+      this.changeDetector.markForCheck();
     });
   }
 
@@ -624,7 +638,8 @@ export class CheckingAnswersComponent implements OnInit {
         this.hideAnswerForm();
         this.submittingAnswer = false;
         this.justEditedAnswer = true;
-        void this.updateQuestionDocAudioUrls();
+        this.changeDetector.markForCheck();
+        this.updateQuestionDocAudioUrls();
       }
     });
   }
@@ -635,6 +650,7 @@ export class CheckingAnswersComponent implements OnInit {
     }
     void this.projectService.isProjectAdmin(this.projectId, this.userService.currentUserId).then(isProjectAdmin => {
       this.isProjectAdmin = isProjectAdmin;
+      this.changeDetector.markForCheck();
     });
   }
 

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-answers/checking-input-form/checking-input-form.component.ts

@@ -1,6 +1,6 @@
 import { BreakpointObserver, BreakpointState } from '@angular/cdk/layout';
 import { NgClass } from '@angular/common';
-import { Component, DestroyRef, EventEmitter, Input, Output, ViewChild } from '@angular/core';
+import { ChangeDetectorRef, Component, DestroyRef, EventEmitter, Input, Output, ViewChild } from '@angular/core';
 import { FormsModule } from '@angular/forms';
 import { MatButton, MatIconButton } from '@angular/material/button';
 import { MatIcon } from '@angular/material/icon';
@@ -84,6 +84,7 @@ export class CheckingInputFormComponent {
     private readonly i18n: I18nService,
     private readonly breakpointObserver: BreakpointObserver,
     private readonly mediaBreakpointService: MediaBreakpointService,
+    private readonly changeDetector: ChangeDetectorRef,
     private destroyRef: DestroyRef
   ) {
     this.breakpointObserver
@@ -131,6 +132,7 @@ export class CheckingInputFormComponent {
         this.selectedText = selection.text;
         this.selectionStartClipped = selection.startClipped;
         this.selectionEndClipped = selection.endClipped;
+        this.changeDetector.markForCheck();
       }
     });
   }