OZG-6986 impl MR comments

alfa-client/apps/admin/src/app/app.component.spec.ts

@@ -208,13 +208,13 @@ describe('AppComponent', () => {
         expect(component.evaluateNavigationByConfiguration).toHaveBeenCalled();
       });
 
-      it('should do navigation by api root if link is missing', () => {
-        component.doNavigationByApiRoot = jest.fn();
+      it('should navigate by api root if link is missing', () => {
+        component.navigateByApiRoot = jest.fn();
         const apiRootResource: ApiRootResource = createApiRootResource();
 
         component.evaluateNavigationByApiRoot(apiRootResource);
 
-        expect(component.doNavigationByApiRoot).toHaveBeenCalledWith(apiRootResource);
+        expect(component.navigateByApiRoot).toHaveBeenCalledWith(apiRootResource);
       });
     });
 
@@ -223,7 +223,7 @@ describe('AppComponent', () => {
 
       beforeEach(() => {
         configurationService.get.mockReturnValue(of(createStateResource(configurationResource)));
-        component.doNavigationByConfiguration = jest.fn();
+        component.navigateByConfiguration = jest.fn();
       });
 
       it('should call configuration service to get resource', () => {
@@ -232,52 +232,52 @@ describe('AppComponent', () => {
         expect(configurationService.get).toHaveBeenCalled();
       });
 
-      it('should call do navigation by configuration', () => {
+      it('should call navigate by configuration', () => {
         component.evaluateNavigationByConfiguration();
 
-        expect(component.doNavigationByConfiguration).toHaveBeenCalledWith(configurationResource);
+        expect(component.navigateByConfiguration).toHaveBeenCalledWith(configurationResource);
       });
 
-      it('should NOT call do navigation by configuration if resource is loading', () => {
+      it('should NOT call navigate by configuration if resource is loading', () => {
         configurationService.get.mockReturnValue(of(createEmptyStateResource<ConfigurationResource>(true)));
 
         component.evaluateNavigationByConfiguration();
 
-        expect(component.doNavigationByConfiguration).not.toHaveBeenCalled();
+        expect(component.navigateByConfiguration).not.toHaveBeenCalled();
       });
     });
 
-    describe('do navigation by configuration', () => {
+    describe('navigate by configuration', () => {
       beforeEach(() => {
         component.unsubscribe = jest.fn();
       });
 
       it('should navigate to postfach if settings link exists', () => {
-        component.doNavigationByConfiguration(createConfigurationResource([ConfigurationLinkRel.SETTING]));
+        component.navigateByConfiguration(createConfigurationResource([ConfigurationLinkRel.SETTING]));
 
         expect(router.navigate).toHaveBeenCalledWith(['/postfach']);
       });
 
       it('should navigate to statistik if aggregation mapping link exists', () => {
-        component.doNavigationByConfiguration(createConfigurationResource([ConfigurationLinkRel.AGGREGATION_MAPPINGS]));
+        component.navigateByConfiguration(createConfigurationResource([ConfigurationLinkRel.AGGREGATION_MAPPINGS]));
 
         expect(router.navigate).toHaveBeenCalledWith(['/statistik']);
       });
 
       it('should navigate to unavailable page if no link exists', () => {
-        component.doNavigationByConfiguration(createConfigurationResource());
+        component.navigateByConfiguration(createConfigurationResource());
 
         expect(router.navigate).toHaveBeenCalledWith(['/unavailable']);
       });
 
       it('should call unsubscribe', () => {
-        component.doNavigationByConfiguration(createConfigurationResource());
+        component.navigateByConfiguration(createConfigurationResource());
 
         expect(component.unsubscribe).toHaveBeenCalled();
       });
     });
 
-    describe('do navigation by api root', () => {
+    describe('navigate by api root', () => {
       beforeEach(() => {
         component.unsubscribe = jest.fn();
       });
@@ -285,19 +285,19 @@ describe('AppComponent', () => {
       it('should navigate to benutzer und rollen if users link exists', () => {
         const apiRootResource: ApiRootResource = createApiRootResource([ApiRootLinkRel.USERS]);
 
-        component.doNavigationByApiRoot(apiRootResource);
+        component.navigateByApiRoot(apiRootResource);
 
         expect(router.navigate).toHaveBeenCalledWith(['/benutzer_und_rollen']);
       });
 
       it('should navigate to unavailable page if no link exists', () => {
-        component.doNavigationByApiRoot(createApiRootResource());
+        component.navigateByApiRoot(createApiRootResource());
 
         expect(router.navigate).toHaveBeenCalledWith(['/unavailable']);
       });
 
       it('should call unsubscribe', () => {
-        component.doNavigationByApiRoot(createApiRootResource());
+        component.navigateByApiRoot(createApiRootResource());
 
         expect(component.unsubscribe).toHaveBeenCalled();
       });

alfa-client/apps/admin/src/app/app.component.ts

@@ -65,12 +65,12 @@ import { UnavailablePageComponent } from '../pages/unavailable/unavailable-page/
   ],
 })
 export class AppComponent implements OnInit {
-  readonly title = 'admin';
+  readonly title: string = 'admin';
 
-  private readonly authenticationService: AuthenticationService = inject(AuthenticationService);
-  private readonly apiRootService: ApiRootService = inject(ApiRootService);
-  private readonly router: Router = inject(Router);
-  private readonly configurationService: ConfigurationService = inject(ConfigurationService);
+  private readonly authenticationService = inject(AuthenticationService);
+  private readonly apiRootService = inject(ApiRootService);
+  private readonly router = inject(Router);
+  private readonly configurationService = inject(ConfigurationService);
 
   public apiRootStateResource$: Observable<StateResource<ApiRootResource>>;
 
@@ -98,7 +98,7 @@ export class AppComponent implements OnInit {
     if (hasLink(apiRootResource, ApiRootLinkRel.CONFIGURATION)) {
       this.evaluateNavigationByConfiguration();
     } else {
-      this.doNavigationByApiRoot(apiRootResource);
+      this.navigateByApiRoot(apiRootResource);
     }
   }
 
@@ -106,30 +106,30 @@ export class AppComponent implements OnInit {
     this.configurationSubscription = this.configurationService
       .get()
       .pipe(filter(isLoaded), mapToResource<ApiRootResource>())
-      .subscribe((configurationResource: ConfigurationResource) => this.doNavigationByConfiguration(configurationResource));
+      .subscribe((configurationResource: ConfigurationResource) => this.navigateByConfiguration(configurationResource));
   }
 
-  doNavigationByConfiguration(configurationResource: ConfigurationResource): void {
+  navigateByConfiguration(configurationResource: ConfigurationResource): void {
     if (hasLink(configurationResource, ConfigurationLinkRel.SETTING)) {
-      this.doInitialNavigation(ROUTES.POSTFACH);
+      this.navigate(ROUTES.POSTFACH);
     } else if (hasLink(configurationResource, ConfigurationLinkRel.AGGREGATION_MAPPINGS)) {
-      this.doInitialNavigation(ROUTES.STATISTIK);
+      this.navigate(ROUTES.STATISTIK);
     } else {
-      this.doInitialNavigation(ROUTES.UNAVAILABLE);
+      this.navigate(ROUTES.UNAVAILABLE);
     }
     this.unsubscribe();
   }
 
-  doNavigationByApiRoot(apiRootResource: ApiRootResource): void {
+  navigateByApiRoot(apiRootResource: ApiRootResource): void {
     if (hasLink(apiRootResource, ApiRootLinkRel.USERS)) {
-      this.doInitialNavigation(ROUTES.BENUTZER_UND_ROLLEN);
+      this.navigate(ROUTES.BENUTZER_UND_ROLLEN);
     } else {
-      this.doInitialNavigation(ROUTES.UNAVAILABLE);
+      this.navigate(ROUTES.UNAVAILABLE);
     }
     this.unsubscribe();
   }
 
-  private doInitialNavigation(routePath: string): void {
+  private navigate(routePath: string): void {
     this.router.navigate(['/' + routePath]);
   }
 

alfa-client/libs/authentication/src/lib/authentication.service.spec.ts

@@ -109,16 +109,16 @@ describe('AuthenticationService', () => {
     const event: OAuthEvent = createOAuthEvent();
 
     beforeEach(() => {
-      service.shouldProceed = jest.fn().mockReturnValue(true);
+      service.shouldProceedByType = jest.fn().mockReturnValue(true);
       service.setCurrentUser = jest.fn();
       service.unsubscribeEvents = jest.fn();
     });
 
-    it('should call shouldProceed on event trigger', () => {
+    it('should call shouldProceedByType on event trigger', () => {
       service.buildEventPromise();
       eventsSubject.next(event);
 
-      expect(service.shouldProceed).toHaveBeenCalledWith(event);
+      expect(service.shouldProceedByType).toHaveBeenCalledWith(event);
     });
 
     describe('on next', () => {
@@ -165,13 +165,13 @@ describe('AuthenticationService', () => {
     });
   });
 
-  describe('should proceed', () => {
+  describe('should proceed by type', () => {
     const event: OAuthEvent = createOAuthEvent();
 
     it('should call considered as login event', () => {
       service.consideredAsLoginEvent = jest.fn();
 
-      service.shouldProceed(event);
+      service.shouldProceedByType(event);
 
       expect(service.consideredAsLoginEvent).toHaveBeenCalledWith(event.type);
     });
@@ -179,7 +179,7 @@ describe('AuthenticationService', () => {
     it('should return true on login event', () => {
       service.consideredAsLoginEvent = jest.fn().mockReturnValue(true);
 
-      const proceed: boolean = service.shouldProceed(event);
+      const proceed: boolean = service.shouldProceedByType(event);
 
       expect(proceed).toBeTruthy();
     });
@@ -188,7 +188,7 @@ describe('AuthenticationService', () => {
       service.consideredAsLoginEvent = jest.fn().mockReturnValue(false);
       service.consideredAsPageReloadEvent = jest.fn();
 
-      service.shouldProceed(event);
+      service.shouldProceedByType(event);
 
       expect(service.consideredAsPageReloadEvent).toHaveBeenCalledWith(event.type);
     });
@@ -197,7 +197,7 @@ describe('AuthenticationService', () => {
       service.consideredAsLoginEvent = jest.fn().mockReturnValue(false);
       service.consideredAsPageReloadEvent = jest.fn().mockReturnValue(true);
 
-      const proceed: boolean = service.shouldProceed(event);
+      const proceed: boolean = service.shouldProceedByType(event);
 
       expect(proceed).toBeTruthy();
     });
@@ -206,7 +206,7 @@ describe('AuthenticationService', () => {
       service.consideredAsLoginEvent = jest.fn().mockReturnValue(false);
       service.consideredAsPageReloadEvent = jest.fn().mockReturnValue(false);
 
-      const proceed: boolean = service.shouldProceed(event);
+      const proceed: boolean = service.shouldProceedByType(event);
 
       expect(proceed).toBeFalsy();
     });

alfa-client/libs/authentication/src/lib/authentication.service.ts

@@ -51,8 +51,13 @@ export class AuthenticationService {
   }
 
   buildEventPromise(): Promise<void> {
-    return new Promise<void>((resolve, reject) => {
-      this.eventSubscription = this.oAuthService.events.pipe(filter((event: OAuthEvent) => this.shouldProceed(event))).subscribe({
+    return new Promise<void>((resolve, reject) => this.handleOAuthEventsForPromise(resolve, reject));
+  }
+
+  private handleOAuthEventsForPromise(resolve: (value: void | PromiseLike<void>) => void, reject: (reason?: any) => void): void {
+    this.eventSubscription = this.oAuthService.events
+      .pipe(filter((event: OAuthEvent) => this.shouldProceedByType(event)))
+      .subscribe({
         next: () => {
           this.setCurrentUser();
           this.unsubscribeEvents();
@@ -63,10 +68,9 @@ export class AuthenticationService {
           reject(error);
         },
       });
-    });
   }
 
-  shouldProceed(event: OAuthEvent): boolean {
+  shouldProceedByType(event: OAuthEvent): boolean {
     return this.consideredAsLoginEvent(event.type) || this.consideredAsPageReloadEvent(event.type);
   }