Code review fixes

sillsdev · Apr 17, 2024 · ebf0566 · ebf0566
1 parent 72bba16
commit ebf0566
Show file tree

Hide file tree

Showing 12 changed files with 98 additions and 105 deletions.
diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/app.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/app.component.spec.ts
@@ -244,12 +244,10 @@ describe('AppComponent', () => {
     expect(env.installBadge).toBeNull();
     expect(env.installButton).toBeNull();
     env.canInstall$.next(true);
-    env.avatarIcon.nativeElement.click();
-    env.wait();
+    env.showHideUserMenu();
     expect(env.installBadge).not.toBeNull();
     expect(env.installButton).not.toBeNull();
-    env.avatarIcon.nativeElement.click();
-    env.wait();
+    env.showHideUserMenu();
   }));
 
   it('hide install badge after avatar menu click', fakeAsync(() => {
@@ -262,17 +260,15 @@ describe('AppComponent', () => {
     expect(env.installBadge).not.toBeNull();
 
     when(mockedPwaService.installPromptLastShownTime).thenReturn(Date.now());
-    env.avatarIcon.nativeElement.click();
-    env.wait();
+    env.showHideUserMenu();
     expect(env.installBadge).toBeNull();
 
     // The install badge should be visible again
     tick(PWA_BEFORE_PROMPT_CAN_BE_SHOWN_AGAIN);
     env.wait();
     expect(env.installBadge).not.toBeNull();
 
-    env.avatarIcon.nativeElement.click();
-    env.wait();
+    env.showHideUserMenu();
   }));
 
   it('user added to project after init', fakeAsync(() => {
@@ -324,8 +320,7 @@ describe('AppComponent', () => {
       const env = new TestEnvironment('online');
       env.init();
 
-      env.avatarIcon.nativeElement.click();
-      env.wait();
+      env.showHideUserMenu();
       expect(env.userMenu).not.toBeNull();
       env.clickEditDisplayName();
       verify(mockedUserService.editDisplayName(false)).once();
@@ -336,8 +331,7 @@ describe('AppComponent', () => {
       env.setCurrentUser('user02');
       env.init();
 
-      env.avatarIcon.nativeElement.click();
-      env.wait();
+      env.showHideUserMenu();
       expect(env.userMenu).not.toBeNull();
       expect(env.editNameButton).not.toBeNull();
       env.clickEditDisplayName();
@@ -353,17 +347,15 @@ describe('AppComponent', () => {
       env.init();
 
       // Show the user menu
-      env.avatarIcon.nativeElement.click();
-      env.wait();
+      env.showHideUserMenu();
 
       // Verify the menu item is visible
-      expect(env.component.isServalAdmin).toBeTruthy();
+      expect(env.component.isServalAdmin).toBe(true);
       expect(env.userMenu).not.toBeNull();
-      expect(env.userMenu.query(By.css('#serval-admin-btn'))).not.toBeNull();
+      expect(env.servalAdminButton).not.toBeNull();
 
       // Hide the user menu
-      env.avatarIcon.nativeElement.click();
-      env.wait();
+      env.showHideUserMenu();
     }));
 
     it('does not show system administration menu item', fakeAsync(() => {
@@ -372,17 +364,15 @@ describe('AppComponent', () => {
       env.init();
 
       // Show the user menu
-      env.avatarIcon.nativeElement.click();
-      env.wait();
+      env.showHideUserMenu();
 
       // Verify the menu item is not visible
-      expect(env.component.isSystemAdmin).toBeFalsy();
+      expect(env.component.isSystemAdmin).toBe(false);
       expect(env.userMenu).not.toBeNull();
-      expect(env.userMenu.query(By.css('#system-admin-btn'))).toBeNull();
+      expect(env.systemAdminButton).toBeNull();
 
       // Hide the user menu
-      env.avatarIcon.nativeElement.click();
-      env.wait();
+      env.showHideUserMenu();
     }));
   });
 
@@ -393,17 +383,15 @@ describe('AppComponent', () => {
       env.init();
 
       // Show the user menu
-      env.avatarIcon.nativeElement.click();
-      env.wait();
+      env.showHideUserMenu();
 
       // Verify the menu item is visible
-      expect(env.component.isSystemAdmin).toBeTruthy();
+      expect(env.component.isSystemAdmin).toBe(true);
       expect(env.userMenu).not.toBeNull();
-      expect(env.userMenu.query(By.css('#system-admin-btn'))).not.toBeNull();
+      expect(env.systemAdminButton).not.toBeNull();
 
       // Hide the user menu
-      env.avatarIcon.nativeElement.click();
-      env.wait();
+      env.showHideUserMenu();
     }));
 
     it('does not show serval administration menu item', fakeAsync(() => {
@@ -412,17 +400,15 @@ describe('AppComponent', () => {
       env.init();
 
       // Show the user menu
-      env.avatarIcon.nativeElement.click();
-      env.wait();
+      env.showHideUserMenu();
 
       // Verify the menu item is not visible
-      expect(env.component.isServalAdmin).toBeFalsy();
+      expect(env.component.isServalAdmin).toBe(false);
       expect(env.userMenu).not.toBeNull();
-      expect(env.userMenu.query(By.css('#serval-admin-btn'))).toBeNull();
+      expect(env.servalAdminButton).toBeNull();
 
       // Hide the user menu
-      env.avatarIcon.nativeElement.click();
-      env.wait();
+      env.showHideUserMenu();
     }));
   });
 });
@@ -548,6 +534,14 @@ class TestEnvironment {
     return this.userMenu.query(By.css('#edit-name-btn'));
   }
 
+  get servalAdminButton(): DebugElement {
+    return this.userMenu.query(By.css('#serval-admin-btn'));
+  }
+
+  get systemAdminButton(): DebugElement {
+    return this.userMenu.query(By.css('#system-admin-btn'));
+  }
+
   get navBar(): DebugElement {
     return this.fixture.debugElement.query(By.css('mat-toolbar'));
   }
@@ -665,6 +659,11 @@ class TestEnvironment {
     this.wait();
   }
 
+  showHideUserMenu(): void {
+    this.avatarIcon.nativeElement.click();
+    this.wait();
+  }
+
   private addUser(userId: string, name: string, authId: string): void {
     this.realtimeService.addSnapshot<User>(UserDoc.COLLECTION, {
       id: userId,

diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/core/paratext.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/core/paratext.service.ts
@@ -5,6 +5,7 @@ import { Observable } from 'rxjs';
 import { map } from 'rxjs/operators';
 import { AuthService } from 'xforge-common/auth.service';
 import { Snapshot } from 'xforge-common/models/snapshot';
+import { PARATEXT_API_NAMESPACE } from 'xforge-common/url-constants';
 import { ParatextProject } from './models/paratext-project';
 
 /**
@@ -35,17 +36,21 @@ export class ParatextService {
 
   getParatextUsername(): Observable<string | undefined> {
     return this.http
-      .get<string | null>('paratext-api/username', { headers: this.headers })
+      .get<string | null>(`${PARATEXT_API_NAMESPACE}/username`, { headers: this.headers })
       .pipe(map(r => r ?? undefined));
   }
 
   getProjects(): Promise<ParatextProject[] | undefined> {
-    return this.http.get<ParatextProject[] | undefined>('paratext-api/projects', { headers: this.headers }).toPromise();
+    return this.http
+      .get<ParatextProject[] | undefined>(`${PARATEXT_API_NAMESPACE}/projects`, { headers: this.headers })
+      .toPromise();
   }
 
   getResources(): Promise<SelectableProject[] | undefined> {
     return this.http
-      .get<{ [id: string]: [shortName: string, name: string] }>('paratext-api/resources', { headers: this.headers })
+      .get<{ [id: string]: [shortName: string, name: string] }>(`${PARATEXT_API_NAMESPACE}/resources`, {
+        headers: this.headers
+      })
       .toPromise()
       .then(result =>
         result == null
@@ -60,9 +65,12 @@ export class ParatextService {
 
   async getRevisions(projectId: string, book: string, chapter: number): Promise<Revision[] | undefined> {
     return await this.http
-      .get<Revision[] | undefined>(`paratext-api/history/revisions/${projectId}_${book}_${chapter}_target`, {
-        headers: this.headers
-      })
+      .get<Revision[] | undefined>(
+        `${PARATEXT_API_NAMESPACE}/history/revisions/${projectId}_${book}_${chapter}_target`,
+        {
+          headers: this.headers
+        }
+      )
       .toPromise();
   }
 
@@ -74,7 +82,7 @@ export class ParatextService {
   ): Promise<Snapshot<TextData> | undefined> {
     return await this.http
       .get<Snapshot<TextData>>(
-        `paratext-api/history/snapshot/${projectId}_${book}_${chapter}_target?timestamp=${timestamp}`,
+        `${PARATEXT_API_NAMESPACE}/history/snapshot/${projectId}_${book}_${chapter}_target?timestamp=${timestamp}`,
         {
           headers: this.headers
         }

diff --git a/....XForge.Scripture/ClientApp/src/app/serval-administration/serval-admin-auth.guard.spec.ts b/....XForge.Scripture/ClientApp/src/app/serval-administration/serval-admin-auth.guard.spec.ts
@@ -23,23 +23,23 @@ describe('ServalAdminAuthGuard', () => {
     const env = new TestEnvironment(true, SystemRole.ServalAdmin);
 
     env.service.canActivate({} as ActivatedRouteSnapshot, {} as RouterStateSnapshot).subscribe(result => {
-      expect(result).toBeTruthy();
+      expect(result).toBe(true);
     });
   });
 
   it('cannot activate if user is not logged in', () => {
     const env = new TestEnvironment(false, SystemRole.None);
 
     env.service.canActivate({} as ActivatedRouteSnapshot, {} as RouterStateSnapshot).subscribe(result => {
-      expect(result).toBeFalsy();
+      expect(result).toBe(false);
     });
   });
 
   it('cannot activate if user is logged in but does not have ServalAdmin role', () => {
     const env = new TestEnvironment(true, SystemRole.SystemAdmin);
 
     env.service.canActivate({} as ActivatedRouteSnapshot, {} as RouterStateSnapshot).subscribe(result => {
-      expect(result).toBeFalsy();
+      expect(result).toBe(false);
     });
   });
 

diff --git a/...ge.Scripture/ClientApp/src/app/serval-administration/serval-administration.component.html b/...ge.Scripture/ClientApp/src/app/serval-administration/serval-administration.component.html
@@ -1,5 +1,3 @@
-<div fxLayout="column" fxLayoutAlign="center start" class="body-content">
-  <h1>Serval Administration</h1>
-</div>
+<h1>Serval Administration</h1>
 
 <app-serval-projects></app-serval-projects>
diff --git a/...e.Scripture/ClientApp/src/app/serval-administration/serval-administration.service.spec.ts b/...e.Scripture/ClientApp/src/app/serval-administration/serval-administration.service.spec.ts
@@ -5,6 +5,7 @@ import { CommandService } from 'xforge-common/command.service';
 import { RealtimeService } from 'xforge-common/realtime.service';
 import { RetryingRequestService } from 'xforge-common/retrying-request.service';
 import { configureTestingModule } from 'xforge-common/test-utils';
+import { PARATEXT_API_NAMESPACE } from 'xforge-common/url-constants';
 import { ServalAdministrationService } from './serval-administration.service';
 
 const mockedCommandService = mock(CommandService);
@@ -25,13 +26,13 @@ describe('ServalAdministrationService', () => {
     it('should return true for a resource id', () => {
       const env = new TestEnvironment();
       const id = '1234567890abcdef';
-      expect(env.service.isResource(id)).toBeTruthy();
+      expect(env.service.isResource(id)).toBe(true);
     });
 
-    it('should return true for a project id', () => {
+    it('should return false for a project id', () => {
       const env = new TestEnvironment();
       const id = '123456781234567890abcdef1234567890abcdef1234567890abcdef';
-      expect(env.service.isResource(id)).toBeFalsy();
+      expect(env.service.isResource(id)).toBe(false);
     });
   });
 
@@ -44,7 +45,7 @@ describe('ServalAdministrationService', () => {
         expect(blob).toEqual(mockBlob);
       });
 
-      const request = env.httpTestingController.expectOne(`paratext-api/projects/${projectId}/download`);
+      const request = env.httpTestingController.expectOne(`${PARATEXT_API_NAMESPACE}/projects/${projectId}/download`);
       expect(request.request.method).toBe('GET');
       request.flush(mockBlob);
       env.httpTestingController.verify();

diff --git a/...XForge.Scripture/ClientApp/src/app/serval-administration/serval-administration.service.ts b/...XForge.Scripture/ClientApp/src/app/serval-administration/serval-administration.service.ts
@@ -6,6 +6,7 @@ import { CommandService } from 'xforge-common/command.service';
 import { ProjectService } from 'xforge-common/project.service';
 import { RealtimeService } from 'xforge-common/realtime.service';
 import { RetryingRequestService } from 'xforge-common/retrying-request.service';
+import { PARATEXT_API_NAMESPACE } from 'xforge-common/url-constants';
 import { SFProjectProfileDoc } from '../core/models/sf-project-profile-doc';
 import { SF_PROJECT_ROLES } from '../core/models/sf-project-role-info';
 
@@ -30,7 +31,7 @@ export class ServalAdministrationService extends ProjectService<SFProjectProfile
    * @returns An observable.
    */
   downloadProject(projectId: string): Observable<Blob> {
-    return this.httpClient.get(`paratext-api/projects/${projectId}/download`, {
+    return this.httpClient.get(`${PARATEXT_API_NAMESPACE}/projects/${projectId}/download`, {
       responseType: 'blob' // Set responseType to 'blob' to handle binary data
     });
   }

diff --git a/...IL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-project.component.html b/...IL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-project.component.html
@@ -1,39 +1,35 @@
-<div fxLayout="column" fxLayoutAlign="center start" class="body-content">
-  <h1>{{ projectName }}</h1>
-  <div>
-    <a mat-raised-button color="primary" [appRouterLink]="'/serval-administration'">Back to Projects</a>
-  </div>
-  <h2>Pre-Translation Configuration</h2>
-  <div>
-    <mat-checkbox [checked]="preTranslate" (change)="onUpdatePreTranslate($event.checked)">
-      Pre-Translation Drafting Enabled
-    </mat-checkbox>
-  </div>
-  <h2>Downloads</h2>
-  <app-notice icon="info">
-    The Zip archives contain the Paratext files for the project at the time of last sync.
-  </app-notice>
-  <div class="table-container">
-    <table mat-table [dataSource]="rows">
-      <ng-container
-        *ngFor="let column of columnsToDisplay | slice : 0 : columnsToDisplay.length - 1"
-        [matColumnDef]="column"
-      >
-        <th mat-header-cell *matHeaderCellDef>{{ headingsToDisplay[column] }}</th>
-        <td mat-cell *matCellDef="let row">{{ row[column] }}</td>
-      </ng-container>
+<h1>{{ projectName }}</h1>
+<a mat-raised-button color="primary" [appRouterLink]="'/serval-administration'">Back to Projects</a>
 
-      <ng-container matColumnDef="id">
-        <th mat-header-cell *matHeaderCellDef></th>
-        <td mat-cell *matCellDef="let row">
-          <button mat-raised-button color="primary" (click)="downloadProject(row['id'], row['fileName'])">
-            Download
-          </button>
-        </td>
-      </ng-container>
+<h2>Pre-Translation Configuration</h2>
+<mat-checkbox [checked]="preTranslate" (change)="onUpdatePreTranslate($event.checked)">
+  Pre-Translation Drafting Enabled
+</mat-checkbox>
 
-      <tr mat-header-row *matHeaderRowDef="columnsToDisplay"></tr>
-      <tr mat-row *matRowDef="let myRowData; columns: columnsToDisplay"></tr>
-    </table>
-  </div>
+<h2>Downloads</h2>
+<app-notice icon="info">
+  The Zip archives contain the Paratext files for the project at the time of last sync.
+</app-notice>
+<div class="table-container">
+  <table mat-table [dataSource]="rows">
+    <ng-container
+      *ngFor="let column of columnsToDisplay | slice : 0 : columnsToDisplay.length - 1"
+      [matColumnDef]="column"
+    >
+      <th mat-header-cell *matHeaderCellDef>{{ headingsToDisplay[column] }}</th>
+      <td mat-cell *matCellDef="let row">{{ row[column] }}</td>
+    </ng-container>
+
+    <ng-container matColumnDef="id">
+      <th mat-header-cell *matHeaderCellDef></th>
+      <td mat-cell *matCellDef="let row">
+        <button mat-raised-button color="primary" (click)="downloadProject(row['id'], row['fileName'])">
+          Download
+        </button>
+      </td>
+    </ng-container>
+
+    <tr mat-header-row *matHeaderRowDef="columnsToDisplay"></tr>
+    <tr mat-row *matRowDef="let myRowData; columns: columnsToDisplay"></tr>
+  </table>
 </div>