Development: Remove lifecycle hooks in learning path related componen…

…ts (#9248)
ls1intum · Sep 23, 2024 · ea25cea · ea25cea
1 parent 3d5c819
commit ea25cea
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 41 deletions.
diff --git a/...ing-paths/components/learning-path-lecture-unit/learning-path-lecture-unit.component.html b/...ing-paths/components/learning-path-lecture-unit/learning-path-lecture-unit.component.html
@@ -1,4 +1,4 @@
-@if (isLectureUnitLoading()) {
+@if (isLoading()) {
     <div class="row justify-content-center m-4">
         <div class="spinner-border text-primary" role="status">
             <span class="sr-only" jhiTranslate="loading"></span>

diff --git a/...rning-paths/components/learning-path-lecture-unit/learning-path-lecture-unit.component.ts b/...rning-paths/components/learning-path-lecture-unit/learning-path-lecture-unit.component.ts
@@ -1,12 +1,11 @@
-import { Component, InputSignal, Signal, WritableSignal, inject, input, signal } from '@angular/core';
+import { Component, InputSignal, WritableSignal, effect, inject, input, signal } from '@angular/core';
 import { LectureUnitService } from 'app/lecture/lecture-unit/lecture-unit-management/lectureUnit.service';
 import { AlertService } from 'app/core/util/alert.service';
 import { LectureUnit, LectureUnitType } from 'app/entities/lecture-unit/lectureUnit.model';
 import { ArtemisLectureUnitsModule } from 'app/overview/course-lectures/lecture-units.module';
 import { LectureUnitCompletionEvent } from 'app/overview/course-lectures/course-lecture-details.component';
 import { LearningPathNavigationService } from 'app/course/learning-paths/services/learning-path-navigation.service';
-import { Observable, lastValueFrom, switchMap } from 'rxjs';
-import { toObservable, toSignal } from '@angular/core/rxjs-interop';
+import { lastValueFrom } from 'rxjs';
 import { ArtemisSharedModule } from 'app/shared/shared.module';
 import { VideoUnitComponent } from 'app/overview/course-lectures/video-unit/video-unit.component';
 import { TextUnitComponent } from 'app/overview/course-lectures/text-unit/text-unit.component';
@@ -27,18 +26,22 @@ export class LearningPathLectureUnitComponent {
     private readonly alertService: AlertService = inject(AlertService);
 
     readonly lectureUnitId: InputSignal<number> = input.required<number>();
-    readonly isLectureUnitLoading: WritableSignal<boolean> = signal(false);
-    private readonly lectureUnit$: Observable<LectureUnit | undefined> = toObservable(this.lectureUnitId).pipe(switchMap((lectureUnitId) => this.getLectureUnit(lectureUnitId)));
-    readonly lectureUnit: Signal<LectureUnit | undefined> = toSignal(this.lectureUnit$);
+    readonly isLoading: WritableSignal<boolean> = signal(false);
+    readonly lectureUnit = signal<LectureUnit | undefined>(undefined);
 
-    async getLectureUnit(lectureUnitId: number): Promise<LectureUnit | undefined> {
+    constructor() {
+        effect(() => this.loadLectureUnit(this.lectureUnitId()), { allowSignalWrites: true });
+    }
+
+    async loadLectureUnit(lectureUnitId: number): Promise<void> {
         try {
-            this.isLectureUnitLoading.set(true);
-            return await lastValueFrom(this.lectureUnitService.getLectureUnitById(lectureUnitId));
+            this.isLoading.set(true);
+            const lectureUnit = await lastValueFrom(this.lectureUnitService.getLectureUnitById(lectureUnitId));
+            this.lectureUnit.set(lectureUnit);
         } catch (error) {
             this.alertService.error(error);
         } finally {
-            this.isLectureUnitLoading.set(false);
+            this.isLoading.set(false);
         }
     }
 

diff --git a/...th-nav-overview-learning-objects/learning-path-nav-overview-learning-objects.component.ts b/...th-nav-overview-learning-objects/learning-path-nav-overview-learning-objects.component.ts
@@ -1,4 +1,4 @@
-import { Component, InputSignal, OnInit, OutputEmitterRef, Signal, WritableSignal, computed, inject, input, output, signal } from '@angular/core';
+import { Component, InputSignal, OutputEmitterRef, Signal, WritableSignal, computed, effect, inject, input, output, signal, untracked } from '@angular/core';
 import { ArtemisSharedModule } from 'app/shared/shared.module';
 import { AlertService } from 'app/core/util/alert.service';
 import { LearningPathApiService } from 'app/course/learning-paths/services/learning-path-api.service';
@@ -15,7 +15,7 @@ import { NgbAccordionModule } from '@ng-bootstrap/ng-bootstrap';
     templateUrl: './learning-path-nav-overview-learning-objects.component.html',
     styleUrl: './learning-path-nav-overview-learning-objects.component.scss',
 })
-export class LearningPathNavOverviewLearningObjectsComponent implements OnInit {
+export class LearningPathNavOverviewLearningObjectsComponent {
     protected readonly faCheckCircle: IconDefinition = faCheckCircle;
     protected readonly faLock: IconDefinition = faLock;
 
@@ -38,8 +38,13 @@ export class LearningPathNavOverviewLearningObjectsComponent implements OnInit {
 
     readonly onLearningObjectSelected: OutputEmitterRef<void> = output();
 
-    ngOnInit(): void {
-        this.loadLearningObjects();
+    constructor() {
+        effect(
+            () => {
+                untracked(async () => await this.loadLearningObjects());
+            },
+            { allowSignalWrites: true },
+        );
     }
 
     async loadLearningObjects(): Promise<void> {
@@ -57,9 +62,9 @@ export class LearningPathNavOverviewLearningObjectsComponent implements OnInit {
         }
     }
 
-    selectLearningObject(learningObject: LearningPathNavigationObjectDTO): void {
+    async selectLearningObject(learningObject: LearningPathNavigationObjectDTO): Promise<void> {
         if (!learningObject.unreleased) {
-            this.learningPathNavigationService.loadRelativeLearningPathNavigation(this.learningPathId(), learningObject);
+            await this.learningPathNavigationService.loadRelativeLearningPathNavigation(this.learningPathId(), learningObject);
             this.onLearningObjectSelected.emit();
         }
     }

diff --git a/...rning-paths/components/learning-path-nav-overview/learning-path-nav-overview.component.ts b/...rning-paths/components/learning-path-nav-overview/learning-path-nav-overview.component.ts
@@ -1,4 +1,4 @@
-import { Component, InputSignal, OutputEmitterRef, Signal, WritableSignal, computed, inject, input, output, signal, viewChild } from '@angular/core';
+import { Component, InputSignal, OutputEmitterRef, Signal, WritableSignal, computed, effect, inject, input, output, signal, viewChild } from '@angular/core';
 import { NgbAccordionDirective, NgbAccordionModule, NgbDropdownModule, NgbModal } from '@ng-bootstrap/ng-bootstrap';
 import { CommonModule } from '@angular/common';
 import { FontAwesomeModule } from '@fortawesome/angular-fontawesome';
@@ -10,8 +10,6 @@ import { LearningPathApiService } from 'app/course/learning-paths/services/learn
 import { CompetencyGraphModalComponent } from 'app/course/learning-paths/components/competency-graph-modal/competency-graph-modal.component';
 import { LearningPathNavOverviewLearningObjectsComponent } from 'app/course/learning-paths/components/learning-path-nav-overview-learning-objects/learning-path-nav-overview-learning-objects.component';
 import { LearningPathNavigationService } from 'app/course/learning-paths/services/learning-path-navigation.service';
-import { toObservable, toSignal } from '@angular/core/rxjs-interop';
-import { Observable, switchMap } from 'rxjs';
 
 @Component({
     selector: 'jhi-learning-path-nav-overview',
@@ -34,25 +32,26 @@ export class LearningPathNavOverviewComponent {
 
     readonly onLearningObjectSelected: OutputEmitterRef<void> = output();
     readonly isLoading: WritableSignal<boolean> = signal(false);
-    private readonly competencies$: Observable<LearningPathCompetencyDTO[]> = toObservable(this.learningPathId).pipe(
-        switchMap((learningPathId) => this.loadCompetencies(learningPathId)),
-    );
-    readonly competencies: Signal<LearningPathCompetencyDTO[] | undefined> = toSignal(this.competencies$);
+    readonly competencies = signal<LearningPathCompetencyDTO[]>([]);
 
     // competency id of currently selected learning object
     readonly currentCompetencyId: Signal<number | undefined> = computed(() => this.learningPathNavigationService.currentLearningObject()?.competencyId);
     // current competency of learning path (not the one of the selected learning object)
     readonly currentCompetencyOnPath: Signal<LearningPathCompetencyDTO | undefined> = computed(() => this.competencies()?.find((competency) => competency.masteryProgress < 1));
 
-    async loadCompetencies(learningPathId: number): Promise<LearningPathCompetencyDTO[]> {
+    constructor() {
+        effect(async () => await this.loadCompetencies(this.learningPathId()), { allowSignalWrites: true });
+    }
+
+    async loadCompetencies(learningPathId: number): Promise<void> {
         try {
             this.isLoading.set(true);
             const competencies = await this.learningPathApiService.getLearningPathCompetencies(learningPathId);
-            this.isLoading.set(false);
-            return competencies;
+            this.competencies.set(competencies);
         } catch (error) {
             this.alertService.error(error);
-            return [];
+        } finally {
+            this.isLoading.set(false);
         }
     }
 

diff --git a/...earning-paths/components/learning-path-student-nav/learning-path-student-nav.component.ts b/...earning-paths/components/learning-path-student-nav/learning-path-student-nav.component.ts
@@ -1,4 +1,4 @@
-import { Component, InputSignal, OnInit, Signal, WritableSignal, computed, inject, input, signal } from '@angular/core';
+import { Component, InputSignal, Signal, WritableSignal, computed, effect, inject, input, signal } from '@angular/core';
 import { LearningPathNavigationObjectDTO } from 'app/entities/competency/learning-path.model';
 import { CommonModule } from '@angular/common';
 import { NgbAccordionModule, NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap';
@@ -15,7 +15,7 @@ import { LearningPathNavigationService } from 'app/course/learning-paths/service
     templateUrl: './learning-path-student-nav.component.html',
     styleUrl: './learning-path-student-nav.component.scss',
 })
-export class LearningPathNavComponent implements OnInit {
+export class LearningPathNavComponent {
     protected readonly faChevronDown: IconDefinition = faChevronDown;
     protected readonly faCheckCircle: IconDefinition = faCheckCircle;
     protected readonly faFlag: IconDefinition = faFlag;
@@ -37,12 +37,12 @@ export class LearningPathNavComponent implements OnInit {
 
     readonly isDropdownOpen: WritableSignal<boolean> = signal(false);
 
-    ngOnInit(): void {
-        this.learningPathNavigationService.loadLearningPathNavigation(this.learningPathId());
+    constructor() {
+        effect(async () => await this.learningPathNavigationService.loadLearningPathNavigation(this.learningPathId()), { allowSignalWrites: true });
     }
 
-    selectLearningObject(selectedLearningObject: LearningPathNavigationObjectDTO): void {
-        this.learningPathNavigationService.loadRelativeLearningPathNavigation(this.learningPathId(), selectedLearningObject);
+    async selectLearningObject(selectedLearningObject: LearningPathNavigationObjectDTO): Promise<void> {
+        await this.learningPathNavigationService.loadRelativeLearningPathNavigation(this.learningPathId(), selectedLearningObject);
     }
 
     completeLearningPath(): void {

diff --git a/...e/learning-paths/pages/learning-path-student-page/learning-path-student-page.component.ts b/...e/learning-paths/pages/learning-path-student-page/learning-path-student-page.component.ts
@@ -1,4 +1,4 @@
-import { Component, OnInit, Signal, inject, signal } from '@angular/core';
+import { Component, effect, inject, signal } from '@angular/core';
 import { LearningObjectType } from 'app/entities/competency/learning-path.model';
 import { map } from 'rxjs';
 import { toSignal } from '@angular/core/rxjs-interop';
@@ -29,7 +29,7 @@ import { ArtemisSharedModule } from 'app/shared/shared.module';
         ArtemisSharedModule,
     ],
 })
-export class LearningPathStudentPageComponent implements OnInit {
+export class LearningPathStudentPageComponent {
     protected readonly LearningObjectType = LearningObjectType;
 
     private readonly learningApiService: LearningPathApiService = inject(LearningPathApiService);
@@ -39,12 +39,12 @@ export class LearningPathStudentPageComponent implements OnInit {
 
     readonly isLearningPathIdLoading = signal(false);
     readonly learningPathId = signal<number | undefined>(undefined);
-    readonly courseId: Signal<number> = toSignal(this.activatedRoute.parent!.parent!.params.pipe(map((params) => params.courseId)));
+    readonly courseId = toSignal(this.activatedRoute.parent!.parent!.params.pipe(map((params) => Number(params.courseId))), { requireSync: true });
     readonly currentLearningObject = this.learningPathNavigationService.currentLearningObject;
     readonly isLearningPathNavigationLoading = this.learningPathNavigationService.isLoading;
 
-    ngOnInit(): void {
-        this.loadLearningPathId(this.courseId());
+    constructor() {
+        effect(async () => await this.loadLearningPathId(this.courseId()), { allowSignalWrites: true });
     }
 
     private async loadLearningPathId(courseId: number): Promise<void> {

diff --git a/...ipt/spec/component/learning-paths/components/learning-path-lecture-unit.component.spec.ts b/...ipt/spec/component/learning-paths/components/learning-path-lecture-unit.component.spec.ts
@@ -86,7 +86,7 @@ describe('LearningPathLectureUnitComponent', () => {
     });
 
     it('should get lecture unit', async () => {
-        const getLectureUnitSpy = jest.spyOn(component, 'getLectureUnit');
+        const getLectureUnitSpy = jest.spyOn(component, 'loadLectureUnit');
 
         fixture.detectChanges();
         await fixture.whenStable();
@@ -102,7 +102,7 @@ describe('LearningPathLectureUnitComponent', () => {
     });
 
     it('should set loading state correctly', async () => {
-        const setIsLoadingSpy = jest.spyOn(component.isLectureUnitLoading, 'set');
+        const setIsLoadingSpy = jest.spyOn(component.isLoading, 'set');
         fixture.detectChanges();
         await fixture.whenStable();
         fixture.detectChanges();