#705 rewrite team and user service to return a subject and update thi…

…s when necessary
puzzle · Dec 29, 2023 · 746c5bf · 746c5bf
1 parent 6ea9160
commit 746c5bf
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 60 deletions.
diff --git a/frontend/src/app/components/key-result-form/key-result-form.component.ts b/frontend/src/app/components/key-result-form/key-result-form.component.ts
@@ -1,10 +1,10 @@
-import { ChangeDetectionStrategy, Component, Input, OnInit } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Input, OnDestroy, OnInit } from '@angular/core';
 import { FormGroup } from '@angular/forms';
 import { getFullNameFromUser, User } from '../../shared/types/model/User';
 import { KeyResult } from '../../shared/types/model/KeyResult';
 import { KeyResultMetric } from '../../shared/types/model/KeyResultMetric';
 import { KeyResultOrdinal } from '../../shared/types/model/KeyResultOrdinal';
-import { BehaviorSubject, filter, map, Observable, of, startWith, switchMap } from 'rxjs';
+import { BehaviorSubject, filter, map, Observable, of, startWith, Subject, switchMap, takeUntil } from 'rxjs';
 import { UserService } from '../../services/user.service';
 import { Action } from '../../shared/types/model/Action';
 import { formInputCheck, hasFormFieldErrors } from '../../shared/common';
@@ -17,12 +17,13 @@ import { TranslateService } from '@ngx-translate/core';
   styleUrls: ['./key-result-form.component.scss'],
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class KeyResultFormComponent implements OnInit {
+export class KeyResultFormComponent implements OnInit, OnDestroy {
   users$!: Observable<User[]>;
   filteredUsers$: Observable<User[]> | undefined = of([]);
   actionList$: BehaviorSubject<Action[] | null> = new BehaviorSubject<Action[] | null>([] as Action[]);
   protected readonly formInputCheck = formInputCheck;
   protected readonly hasFormFieldErrors = hasFormFieldErrors;
+  private unsubscribe$ = new Subject<void>();
 
   @Input()
   keyResultForm!: FormGroup;
@@ -61,7 +62,7 @@ export class KeyResultFormComponent implements OnInit {
         { id: null, version: 1, action: '', priority: 2, keyResultId: null, isChecked: false },
       ]);
 
-      this.users$.subscribe((users) => {
+      this.users$.pipe(takeUntil(this.unsubscribe$)).subscribe((users) => {
         const loggedInUser = this.getUserName();
         users.forEach((user) => {
           if (getFullNameFromUser(user) === loggedInUser) {
@@ -76,6 +77,11 @@ export class KeyResultFormComponent implements OnInit {
     });
   }
 
+  ngOnDestroy() {
+    this.unsubscribe$.next();
+    this.unsubscribe$.complete();
+  }
+
   isMetricKeyResult() {
     return this.keyResultForm.controls['keyResultType'].value === 'metric';
   }

diff --git a/frontend/src/app/components/team-filter/team-filter.component.ts b/frontend/src/app/components/team-filter/team-filter.component.ts
@@ -1,5 +1,5 @@
-import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core';
-import { BehaviorSubject } from 'rxjs';
+import { ChangeDetectionStrategy, Component, OnDestroy, OnInit } from '@angular/core';
+import { BehaviorSubject, Subject, takeUntil } from 'rxjs';
 import { Team } from '../../shared/types/model/Team';
 import { TeamService } from '../../services/team.service';
 import { ActivatedRoute, Router } from '@angular/router';
@@ -13,10 +13,11 @@ import { extractTeamsFromUser } from '../../shared/types/model/User';
   templateUrl: './team-filter.component.html',
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class TeamFilterComponent implements OnInit {
+export class TeamFilterComponent implements OnInit, OnDestroy {
   teams$: BehaviorSubject<Team[]> = new BehaviorSubject<Team[]>([]);
   activeTeams: number[] = [];
   protected readonly trackByFn = trackByFn;
+  private unsubscribe$ = new Subject<void>();
 
   constructor(
     private teamService: TeamService,
@@ -26,26 +27,37 @@ export class TeamFilterComponent implements OnInit {
     private userService: UserService,
   ) {
     this.refreshDataService.reloadOverviewSubject.subscribe(() => {
-      this.teamService.getAllTeams().subscribe((teams) => {
-        this.teams$.next(teams);
-        this.activeTeams = this.activeTeams.filter((teamId) => this.getAllTeamIds().includes(teamId));
-      });
+      this.teamService
+        .getAllTeams()
+        .pipe(takeUntil(this.unsubscribe$))
+        .subscribe((teams) => {
+          this.teams$.next(teams);
+          this.activeTeams = this.activeTeams.filter((teamId) => this.getAllTeamIds().includes(teamId));
+        });
     });
   }
 
   ngOnInit(): void {
-    this.teamService.getAllTeams().subscribe((teams: Team[]) => {
-      this.teams$.next(teams);
-      const teamQuery = this.route.snapshot.queryParams['teams'];
-      const teamIds = getValueFromQuery(teamQuery);
-      const knownTeams = this.getAllTeamIds().filter((teamId) => teamIds?.includes(teamId));
-      if (knownTeams.length == 0) {
-        this.activeTeams = extractTeamsFromUser(this.userService.getCurrentUser()).map((team) => team.id);
-      } else {
-        this.activeTeams = knownTeams;
-      }
-      this.changeTeamFilterParams();
-    });
+    this.teamService
+      .getAllTeams()
+      .pipe(takeUntil(this.unsubscribe$))
+      .subscribe((teams: Team[]) => {
+        this.teams$.next(teams);
+        const teamQuery = this.route.snapshot.queryParams['teams'];
+        const teamIds = getValueFromQuery(teamQuery);
+        const knownTeams = this.getAllTeamIds().filter((teamId) => teamIds?.includes(teamId));
+        if (knownTeams.length == 0) {
+          this.activeTeams = extractTeamsFromUser(this.userService.getCurrentUser()).map((team) => team.id);
+        } else {
+          this.activeTeams = knownTeams;
+        }
+        this.changeTeamFilterParams();
+      });
+  }
+
+  ngOnDestroy() {
+    this.unsubscribe$.next();
+    this.unsubscribe$.complete();
   }
 
   changeTeamFilterParams() {

diff --git a/frontend/src/app/services/team.service.ts b/frontend/src/app/services/team.service.ts
@@ -1,37 +1,46 @@
 import { Injectable } from '@angular/core';
 import { HttpClient } from '@angular/common/http';
 import { Team } from '../shared/types/model/Team';
-import { Observable, of, take, tap } from 'rxjs';
+import { BehaviorSubject, Observable, tap } from 'rxjs';
 
 @Injectable({
   providedIn: 'root',
 })
 export class TeamService {
   constructor(private http: HttpClient) {}
 
-  private teams: Team[] | undefined;
+  private teams: BehaviorSubject<Team[]> = new BehaviorSubject<Team[]>([]);
+  private teamsLoaded = false;
 
   getAllTeams(): Observable<Team[]> {
-    if (this.teams) {
-      return of(this.teams).pipe(take(1));
+    if (!this.teamsLoaded) {
+      this.reloadTeams().subscribe();
+      this.teamsLoaded = true;
     }
-    return this.http.get<Team[]>('/api/v2/teams').pipe(tap((teams) => (this.teams = teams)));
+    return this.teams.asObservable();
   }
 
   reloadTeams(): Observable<Team[]> {
-    this.teams = undefined;
-    return this.getAllTeams();
+    return this.http.get<Team[]>('/api/v2/teams').pipe(
+      tap((teams) => {
+        if (!this.teams) {
+          this.teams = new BehaviorSubject<Team[]>(teams);
+          return;
+        }
+        this.teams.next(teams);
+      }),
+    );
   }
 
   createTeam(team: Team): Observable<Team> {
-    return this.http.post<Team>('/api/v2/teams', team);
+    return this.http.post<Team>('/api/v2/teams', team).pipe(tap(() => this.reloadTeams().subscribe()));
   }
 
   updateTeam(team: Team): Observable<Team> {
-    return this.http.put<Team>(`/api/v2/teams/${team.id}`, team);
+    return this.http.put<Team>(`/api/v2/teams/${team.id}`, team).pipe(tap(() => this.reloadTeams().subscribe()));
   }
 
   deleteTeam(id: number): Observable<any> {
-    return this.http.delete(`/api/v2/teams/${id}`);
+    return this.http.delete(`/api/v2/teams/${id}`).pipe(tap(() => this.reloadTeams().subscribe()));
   }
 }
diff --git a/frontend/src/app/services/user.service.ts b/frontend/src/app/services/user.service.ts
@@ -1,5 +1,5 @@
 import { Injectable } from '@angular/core';
-import { Observable, of, take, tap } from 'rxjs';
+import { BehaviorSubject, Observable, of, tap } from 'rxjs';
 import { HttpClient } from '@angular/common/http';
 import { User } from '../shared/types/model/User';
 
@@ -10,7 +10,8 @@ export class UserService {
   private readonly API_URL = 'api/v1/users';
 
   private _user: User | undefined;
-  private users: User[] | undefined;
+  private users: BehaviorSubject<User[]> = new BehaviorSubject<User[]>([]);
+  private usersLoaded = false;
 
   constructor(private httpClient: HttpClient) {}
 
@@ -22,15 +23,15 @@ export class UserService {
   }
 
   public getUsers(): Observable<User[]> {
-    if (this.users) {
-      return of(this.users).pipe(take(1));
+    if (!this.usersLoaded) {
+      this.usersLoaded = true;
+      this.reloadUsers().subscribe();
     }
-    return this.httpClient.get<User[]>(this.API_URL).pipe(tap((users) => (this.users = users)));
+    return this.users.asObservable();
   }
 
   public reloadUsers(): Observable<User[]> {
-    this.users = undefined;
-    return this.getUsers();
+    return this.httpClient.get<User[]>(this.API_URL).pipe(tap((users) => this.users.next(users)));
   }
 
   public getCurrentUser(): User {

diff --git a/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.ts b/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.ts
@@ -1,10 +1,10 @@
-import { ChangeDetectionStrategy, Component, Inject, OnInit } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Inject, OnDestroy, OnInit } from '@angular/core';
 import { FormControl, FormGroup, Validators } from '@angular/forms';
 import { Quarter } from '../../types/model/Quarter';
 import { TeamService } from '../../../services/team.service';
 import { Team } from '../../types/model/Team';
 import { QuarterService } from '../../../services/quarter.service';
-import { forkJoin, Observable, of, Subject } from 'rxjs';
+import { forkJoin, Observable, of, Subject, takeUntil } from 'rxjs';
 import { ObjectiveService } from '../../../services/objective.service';
 import { MAT_DIALOG_DATA, MatDialog, MatDialogRef } from '@angular/material/dialog';
 import { State } from '../../types/enums/State';
@@ -22,7 +22,7 @@ import { TranslateService } from '@ngx-translate/core';
   styleUrls: ['./objective-form.component.scss'],
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class ObjectiveFormComponent implements OnInit {
+export class ObjectiveFormComponent implements OnInit, OnDestroy {
   objectiveForm = new FormGroup({
     title: new FormControl<string>('', [Validators.required, Validators.minLength(2), Validators.maxLength(250)]),
     description: new FormControl<string>('', [Validators.maxLength(4096)]),
@@ -38,6 +38,7 @@ export class ObjectiveFormComponent implements OnInit {
   version!: number;
   protected readonly formInputCheck = formInputCheck;
   protected readonly hasFormFieldErrors = hasFormFieldErrors;
+  private unsubscribe$ = new Subject<void>();
 
   constructor(
     private route: ActivatedRoute,
@@ -79,7 +80,7 @@ export class ObjectiveFormComponent implements OnInit {
 
   ngOnInit(): void {
     const isCreating: boolean = !!this.data.objective.objectiveId;
-    this.teams$ = this.teamService.getAllTeams();
+    this.teams$ = this.teamService.getAllTeams().pipe(takeUntil(this.unsubscribe$));
     this.quarters$ = this.quarterService.getAllQuarters();
     const objective$ = isCreating
       ? this.objectiveService.getFullObjective(this.data.objective.objectiveId!)
@@ -103,6 +104,11 @@ export class ObjectiveFormComponent implements OnInit {
     });
   }
 
+  ngOnDestroy() {
+    this.unsubscribe$.next();
+    this.unsubscribe$.complete();
+  }
+
   getSubmitFunction(id: number, objectiveDTO: any): Observable<Objective> {
     if (this.data.action == 'duplicate') {
       objectiveDTO.id = null;

diff --git a/frontend/src/app/team-management/member-list/member-list.component.ts b/frontend/src/app/team-management/member-list/member-list.component.ts
@@ -1,7 +1,7 @@
 import { ChangeDetectorRef, Component, OnDestroy, OnInit } from '@angular/core';
 import { UserService } from '../../services/user.service';
 import { ActivatedRoute } from '@angular/router';
-import { combineLatest, map, ReplaySubject, Subscription } from 'rxjs';
+import { combineLatest, map, ReplaySubject, Subject, takeUntil } from 'rxjs';
 import { User } from '../../shared/types/model/User';
 import { convertFromUsers, UserTableEntry } from '../../shared/types/model/UserTableEntry';
 import { TeamService } from '../../services/team.service';
@@ -18,7 +18,7 @@ export class MemberListComponent implements OnInit, OnDestroy {
 
   private allUsersSubj: ReplaySubject<User[]> = new ReplaySubject<User[]>(1);
 
-  private subscription!: Subscription;
+  private unsubscribe$ = new Subject<void>();
   private allColumns = ['icon', 'name', 'roles', 'teams'];
   private teamColumns = ['icon', 'name', 'roles'];
 
@@ -32,16 +32,17 @@ export class MemberListComponent implements OnInit, OnDestroy {
   ) {}
 
   public ngOnInit(): void {
-    this.userService.getUsers().subscribe((users) => this.allUsersSubj.next(users));
+    this.userService
+      .getUsers()
+      .pipe(takeUntil(this.unsubscribe$))
+      .subscribe((users) => this.allUsersSubj.next(users));
     const teamId$ = this.route.paramMap.pipe(map((params) => params.get('teamId')));
-    this.subscription = combineLatest([
-      this.allUsersSubj.asObservable(),
-      teamId$,
-      this.teamService.getAllTeams(),
-    ]).subscribe(([users, teamIdParam, teams]) => {
-      this.setDataSource(users, teamIdParam);
-      this.setSelectedTeam(teams, teamIdParam);
-    });
+    combineLatest([this.allUsersSubj.asObservable(), teamId$, this.teamService.getAllTeams()])
+      .pipe(takeUntil(this.unsubscribe$))
+      .subscribe(([users, teamIdParam, teams]) => {
+        this.setDataSource(users, teamIdParam);
+        this.setSelectedTeam(teams, teamIdParam);
+      });
   }
 
   private setDataSource(users: User[], teamIdParam: string | null) {
@@ -58,7 +59,8 @@ export class MemberListComponent implements OnInit, OnDestroy {
   }
 
   public ngOnDestroy(): void {
-    this.subscription.unsubscribe();
+    this.unsubscribe$.next();
+    this.unsubscribe$.complete();
   }
 
   private setSelectedTeam(teams: Team[], teamIdParam: string | null) {

diff --git a/frontend/src/app/team-management/team-list/team-list.component.ts b/frontend/src/app/team-management/team-list/team-list.component.ts
@@ -1,6 +1,6 @@
 import { Component, OnDestroy, OnInit } from '@angular/core';
 import { TeamService } from '../../services/team.service';
-import { Observable, Subscription } from 'rxjs';
+import { Observable, Subject, takeUntil } from 'rxjs';
 import { Team } from '../../shared/types/model/Team';
 import { ActivatedRoute } from '@angular/router';
 
@@ -12,7 +12,7 @@ import { ActivatedRoute } from '@angular/router';
 export class TeamListComponent implements OnInit, OnDestroy {
   public teams$: Observable<Team[]>;
   public selectedTeamId: number | undefined;
-  private subscription!: Subscription;
+  private unsubscribe$ = new Subject<void>();
 
   constructor(
     private readonly teamService: TeamService,
@@ -22,13 +22,14 @@ export class TeamListComponent implements OnInit, OnDestroy {
   }
 
   public ngOnInit(): void {
-    this.subscription = this.route.paramMap.subscribe((params) => {
+    this.route.paramMap.pipe(takeUntil(this.unsubscribe$)).subscribe((params) => {
       const teamId = params.get('teamId');
       this.selectedTeamId = teamId ? parseInt(teamId) : undefined;
     });
   }
 
   public ngOnDestroy(): void {
-    this.subscription.unsubscribe();
+    this.unsubscribe$.next();
+    this.unsubscribe$.complete();
   }
 }