Merge pull request #508 from frankframework/fix/frontend-architecture

Author: stijnpotters1

src/main/frontend/src/app/app.component.ts

@@ -42,7 +42,13 @@ export class AppComponent implements OnInit {
   ngOnInit(): void {
     const wasExtended = this.graphStateService.restoreAndClearOAuthExtended();
 
-    this.authService.checkAuthStatus().subscribe();
+    this.authService.checkAuthStatus().subscribe({
+      next: (user) => {
+        if (user) {
+          this.authService.setAuthenticated(user);
+        }
+      },
+    });
 
     this.route.queryParams.subscribe((parameters) => {
       const currentUrl = this.router.url;

src/main/frontend/src/app/app.routes.ts

@@ -10,19 +10,22 @@ import { FrankFrameworkMemberGuard } from './guards/frankframework-member.guard'
 
 export const routes: Routes = [
   { path: '', redirectTo: 'graph', pathMatch: 'full' },
-  { path: 'graph', component: ReleaseGraphComponent },
-  { path: 'graph/:id', component: ReleaseDetailsComponent },
-  { path: 'roadmap', component: ReleaseRoadmapComponent },
-  { path: 'release-manage/:id', component: ReleaseManageComponent, canActivate: [FrankFrameworkMemberGuard] },
   {
-    path: 'release-manage/:id/business-values',
-    component: BusinessValueManageComponent,
-    canActivate: [FrankFrameworkMemberGuard],
+    path: 'graph',
+    children: [
+      { path: '', component: ReleaseGraphComponent },
+      { path: ':id', component: ReleaseDetailsComponent },
+    ],
   },
+  { path: 'roadmap', component: ReleaseRoadmapComponent },
   {
-    path: 'release-manage/:id/vulnerabilities',
-    component: VulnerabilityImpactManageComponent,
+    path: 'release-manage/:id',
     canActivate: [FrankFrameworkMemberGuard],
+    children: [
+      { path: '', component: ReleaseManageComponent },
+      { path: 'business-values', component: BusinessValueManageComponent },
+      { path: 'vulnerabilities', component: VulnerabilityImpactManageComponent },
+    ],
   },
   { path: 'not-found', component: NotFoundComponent },
   { path: '**', redirectTo: 'not-found' },

src/main/frontend/src/app/app.service.spec.ts

@@ -24,98 +24,6 @@ describe('AppService', () => {
     expect(service).toBeTruthy();
   });
 
-  describe('get()', () => {
-    const testUrl = 'test-endpoint';
-    const mockData = { message: 'success' };
-
-    it('should make a GET request with parameters', () => {
-      const parameters = { name: 'frank', version: 8 };
-
-      service.get(testUrl, parameters).subscribe((data) => {
-        expect(data).toEqual(mockData);
-      });
-
-      const request = httpMock.expectOne((r) => r.url === testUrl);
-
-      expect(request.request.method).toBe('GET');
-      expect(request.request.params.get('name')).toBe('frank');
-      expect(request.request.params.get('version')).toBe('8');
-      request.flush(mockData);
-    });
-  });
-
-  describe('post()', () => {
-    const testUrl = 'test-endpoint';
-    const mockBody = { data: 'test' };
-    const mockResponse = { id: 1 };
-
-    it('should make a POST request', () => {
-      service.post(testUrl, mockBody).subscribe((data) => {
-        expect(data).toEqual(mockResponse);
-      });
-
-      const request = httpMock.expectOne(testUrl);
-
-      expect(request.request.method).toBe('POST');
-      expect(request.request.body).toEqual(mockBody);
-      request.flush(mockResponse);
-    });
-
-    it('should support custom headers', () => {
-      const headers = { 'X-Custom': 'Value' };
-      service.post(testUrl, mockBody, headers).subscribe();
-
-      const request = httpMock.expectOne(testUrl);
-
-      expect(request.request.headers.get('X-Custom')).toBe('Value');
-      request.flush(mockResponse);
-    });
-  });
-
-  describe('put()', () => {
-    const testUrl = 'test-endpoint/1';
-    const mockBody = { data: 'updated' };
-    const mockResponse = { id: 1, data: 'updated' };
-
-    it('should make a PUT request', () => {
-      service.put(testUrl, mockBody).subscribe((data) => {
-        expect(data).toEqual(mockResponse);
-      });
-
-      const request = httpMock.expectOne(testUrl);
-
-      expect(request.request.method).toBe('PUT');
-      expect(request.request.body).toEqual(mockBody);
-      request.flush(mockResponse);
-    });
-  });
-
-  describe('delete()', () => {
-    const testUrl = 'test-endpoint/1';
-
-    it('should make a DELETE request', () => {
-      service.delete(testUrl).subscribe((data) => {
-        expect(data).toBeNull();
-      });
-
-      const request = httpMock.expectOne(testUrl);
-
-      expect(request.request.method).toBe('DELETE');
-      request.flush(null);
-    });
-
-    it('should support custom headers in DELETE', () => {
-      const headers = { 'X-Auth': 'Token' };
-      service.delete(testUrl, headers).subscribe();
-
-      const request = httpMock.expectOne(testUrl);
-
-      expect(request.request.method).toBe('DELETE');
-      expect(request.request.headers.get('X-Auth')).toBe('Token');
-      request.flush(null);
-    });
-  });
-
   describe('createAPIUrl()', () => {
     it('should correctly construct an API URL', () => {
       expect(service.createAPIUrl('users')).toBe('/api/users');

src/main/frontend/src/app/app.service.ts

@@ -1,6 +1,4 @@
-import { Injectable, inject } from '@angular/core';
-import { HttpClient, HttpParams } from '@angular/common/http';
-import { Observable } from 'rxjs';
+import { Injectable } from '@angular/core';
 
 export const GitHubStates = {
   OPEN: 'OPEN',
@@ -13,82 +11,6 @@ export type GitHubState = (typeof GitHubStates)[keyof typeof GitHubStates];
   providedIn: 'root',
 })
 export class AppService {
-  private http = inject(HttpClient);
-
-  /**
-   * Performs a GET request to the given URL with optional query parameters.
-   *
-   * @template T - The expected response type.
-   * @param url - The API endpoint URL.
-   * @param parameters - Optional query parameters as key-value pairs. Values can be primitive types or HTTP options.
-   * @returns An Observable of type `T` containing the response data.
-   */
-  public get<T>(url: string, parameters?: Record<string, string | number | boolean>): Observable<T> {
-    let httpParameters = new HttpParams();
-
-    if (parameters) {
-      for (const key of Object.keys(parameters)) {
-        const value = parameters[key];
-
-        if (value !== undefined && value !== null && value !== '') {
-          httpParameters = httpParameters.set(key, String(value));
-        }
-      }
-    }
-
-    return this.http.get<T>(url, { params: httpParameters });
-  }
-
-  /**
-   * Performs a POST request to the given URL with optional body and options.
-   *
-   * @template T - The expected response type.
-   * @template B - The request body type.
-   * @param url - The API endpoint URL.
-   * @param body - Optional request body.
-   * @param options - Optional HTTP headers as key-value pairs.
-   * @returns An Observable of type `T` containing the response data.
-   */
-  public post<T, B = unknown>(url: string, body?: B, options?: Record<string, string>): Observable<T> {
-    const httpOptions: { headers?: Record<string, string> } = {};
-
-    if (options && Object.keys(options).length > 0) {
-      httpOptions.headers = options;
-    }
-
-    return this.http.post<T>(url, body ?? null, httpOptions);
-  }
-
-  /**
-   * Performs a PUT request to the given URL with optional body and options.
-   *
-   * @template T - The expected response type.
-   * @template B - The request body type.
-   * @param url - The API endpoint URL.
-   * @param body - Optional request body.
-   * @param options - Optional HTTP headers as key-value pairs.
-   * @returns An Observable of type `T` containing the response data.
-   */
-  public put<T, B = unknown>(url: string, body?: B, options?: Record<string, string>): Observable<T> {
-    const httpOptions: { headers?: Record<string, string> } = {};
-
-    if (options && Object.keys(options).length > 0) {
-      httpOptions.headers = options;
-    }
-
-    return this.http.put<T>(url, body ?? null, httpOptions);
-  }
-
-  public delete<T>(url: string, options?: Record<string, string>): Observable<T> {
-    const httpOptions: { headers?: Record<string, string> } = {};
-
-    if (options && Object.keys(options).length > 0) {
-      httpOptions.headers = options;
-    }
-
-    return this.http.delete<T>(url, httpOptions);
-  }
-
   /**
    * Constructs a qualified API URL by appending a given endpoint to the base URL.
    *

src/main/frontend/src/app/pages/header/header.component.html

@@ -23,7 +23,7 @@ <h3>Roadmap</h3>
     </li>
   </ul>
 
-  <div class="auth-section">
+  <section class="auth-section">
     @if (authService.isAuthenticated()) {
       <div
         class="user-profile"
@@ -37,8 +37,8 @@ <h3>Roadmap</h3>
         <span class="username">{{ authService.currentUser()?.username }}</span>
 
         @if (showUserMenu) {
-          <div class="user-menu" tabindex="0" (click)="$event.stopPropagation()">
-            <div class="user-menu-header">
+          <menu class="user-menu" tabindex="0" (click)="$event.stopPropagation()">
+            <header class="user-menu-header">
               <img
                 class="avatar-large"
                 [src]="authService.currentUser()?.avatarUrl"
@@ -48,7 +48,7 @@ <h3>Roadmap</h3>
                 <span class="username-large">{{ authService.currentUser()?.username }}</span>
                 <span class="user-id">ID: {{ authService.currentUser()?.githubId }}</span>
               </div>
-            </div>
+            </header>
             <div class="user-menu-divider"></div>
             <button class="user-menu-item logout-btn" (click)="onLogout()">
               <svg height="16" width="16" viewBox="0 0 16 16" fill="currentColor">
@@ -58,7 +58,7 @@ <h3>Roadmap</h3>
               </svg>
               Logout
             </button>
-          </div>
+          </menu>
         }
       </div>
     } @else {
@@ -77,7 +77,7 @@ <h3>Roadmap</h3>
         }
       </button>
     }
-  </div>
+  </section>
 </nav>
 
 @if (authService.authError()) {

src/main/frontend/src/app/pages/release-details/release-business-value-modal/release-business-value-modal.component.html

@@ -2,12 +2,12 @@
   <div class="business-value-modal-content">
     @if (businessValue()) {
       <div class="business-value-details">
-        <div class="description-section">
+        <section class="description-section">
           <h3>Description</h3>
           <p class="description-text">{{ businessValue()!.description }}</p>
-        </div>
+        </section>
 
-        <div class="issues-section">
+        <section class="issues-section">
           <div class="issues-header">
             <h3>Connected Issues</h3>
             <span class="issue-count"
@@ -37,7 +37,7 @@ <h3>Connected Issues</h3>
               <p>No issues connected to this business value.</p>
             </div>
           }
-        </div>
+        </section>
       </div>
     }
   </div>

src/main/frontend/src/app/pages/release-details/release-business-value/release-business-value.component.ts

@@ -2,7 +2,7 @@ import { Component, Input, OnChanges, SimpleChanges, inject, signal } from '@ang
 import { CommonModule } from '@angular/common';
 import { BusinessValue, BusinessValueService } from '../../../services/business-value.service';
 import { ReleaseBusinessValueModalComponent } from '../release-business-value-modal/release-business-value-modal.component';
-import { catchError, of } from 'rxjs';
+import { catchError, finalize, of } from 'rxjs';
 
 @Component({
   selector: 'app-release-business-value',
@@ -45,10 +45,8 @@ export class ReleaseBusinessValueComponent implements OnChanges {
           console.error('Failed to load business values:', error);
           return of([]);
         }),
+        finalize(() => this.isLoadingBusinessValues.set(false)),
       )
-      .subscribe((businessValues) => {
-        this.businessValues.set(businessValues);
-        this.isLoadingBusinessValues.set(false);
-      });
+      .subscribe((businessValues) => this.businessValues.set(businessValues));
   }
 }

src/main/frontend/src/app/pages/release-details/release-details.component.html

@@ -1,6 +1,6 @@
 <div class="release-details-container">
   @if (!isLoading && release) {
-    <div class="release-details-header">
+    <header class="release-details-header">
       <div class="header-actions">
         <button class="back-button" (click)="goBack()">
           <img src="assets/icons/chevron-left.svg" alt="" width="20" height="20" />
@@ -14,11 +14,11 @@
         <h2>{{ release.name }}</h2>
         <span>{{ release.publishedAt | date: 'longDate' }}</span>
       </div>
-    </div>
+    </header>
 
     <div class="release-details-body">
       <div class="release-details-content">
-        <div class="left-panel">
+        <section class="left-panel">
           @if (highlightedLabels === undefined && releaseIssues === undefined) {
             <div class="no-new-features">
               <p>No new features found towards previous release.</p>
@@ -56,17 +56,17 @@ <h2>{{ release.name }}</h2>
               </div>
             }
           }
-        </div>
+        </section>
 
-        <div class="right-panel">
+        <section class="right-panel">
           @if (vulnerabilities !== undefined) {
             <app-release-vulnerabilities [vulnerabilities]="vulnerabilities" [lastScanned]="release.lastScanned" />
           } @else {
             <div class="placeholder">
               <p>Loading vulnerability information...</p>
             </div>
           }
-        </div>
+        </section>
       </div>
     </div>
   } @else if (isLoading) {