Development: Improve performance of programming exercise details view

src/main/webapp/app/detail-overview-list/detail-overview-list.component.html

@@ -1,7 +1,7 @@
 @if (headlines?.length && headlines.length > 1) {
     <jhi-detail-overview-navigation-bar [sectionHeadlines]="headlines" />
 }
-@for (section of sections; track section) {
+@for (section of sections(); track section) {
     <h3 class="section-headline" [id]="headlinesRecord[section.headline]">{{ section.headline | artemisTranslate }}</h3>
     <dl class="section-detail-list">
         @for (detail of section.details; track $index) {

src/main/webapp/app/detail-overview-list/detail-overview-list.component.ts

@@ -1,7 +1,7 @@
-import { Component, Input, OnDestroy, OnInit, ViewEncapsulation } from '@angular/core';
+import { Component, OnDestroy, OnInit, ViewEncapsulation, inject, input } from '@angular/core';
 import { isEmpty } from 'lodash-es';
 import { FeatureToggle } from 'app/shared/feature-toggle/feature-toggle.service';
-import { ButtonSize, TooltipPlacement } from 'app/shared/components/button.component';
+import { ButtonSize } from 'app/shared/components/button.component';
 import { IrisSubSettingsType } from 'app/entities/iris/settings/iris-sub-settings.model';
 import { ModelingExerciseService } from 'app/exercises/modeling/manage/modeling-exercise.service';
 import { AlertService } from 'app/core/util/alert.service';
@@ -50,11 +50,13 @@ export class DetailOverviewListComponent implements OnInit, OnDestroy {
     protected readonly FeatureToggle = FeatureToggle;
     protected readonly ButtonSize = ButtonSize;
     protected readonly ProgrammingExerciseParticipationType = ProgrammingExerciseParticipationType;
+    protected readonly CHAT = IrisSubSettingsType.CHAT;
 
-    readonly CHAT = IrisSubSettingsType.CHAT;
+    private readonly modelingExerciseService = inject(ModelingExerciseService);
+    private readonly alertService = inject(AlertService);
+    private readonly profileService = inject(ProfileService);
 
-    @Input()
-    sections: DetailOverviewSection[];
+    sections = input.required<DetailOverviewSection[]>();
 
     // headline list for navigation bar
     headlines: { id: string; translationKey: string }[];
@@ -64,14 +66,8 @@ export class DetailOverviewListComponent implements OnInit, OnDestroy {
     profileSubscription: Subscription;
     isLocalVC = false;
 
-    constructor(
-        private modelingExerciseService: ModelingExerciseService,
-        private alertService: AlertService,
-        private profileService: ProfileService,
-    ) {}
-
     ngOnInit() {
-        this.headlines = this.sections.map((section) => {
+        this.headlines = this.sections().map((section) => {
             return {
                 id: section.headline.replaceAll('.', '-'),
                 translationKey: section.headline,
@@ -98,6 +94,4 @@ export class DetailOverviewListComponent implements OnInit, OnDestroy {
     ngOnDestroy() {
         this.profileSubscription?.unsubscribe();
     }
-
-    protected readonly TooltipPlacement = TooltipPlacement;
 }

src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts

@@ -2,7 +2,7 @@ import { Component, OnDestroy, OnInit, ViewEncapsulation } from '@angular/core';
 import { ActivatedRoute, Router } from '@angular/router';
 import { SafeHtml } from '@angular/platform-browser';
 import { ProgrammingExerciseBuildConfig } from 'app/entities/programming/programming-exercise-build.config';
-import { Subject, Subscription } from 'rxjs';
+import { Subject, Subscription, of } from 'rxjs';
 import { ProgrammingExercise, ProgrammingLanguage } from 'app/entities/programming/programming-exercise.model';
 import { ProgrammingExerciseService } from 'app/exercises/programming/manage/services/programming-exercise.service';
 import { AlertService, AlertType } from 'app/core/util/alert.service';
@@ -57,6 +57,9 @@ import { IrisSubSettingsType } from 'app/entities/iris/settings/iris-sub-setting
 import { Detail } from 'app/detail-overview-list/detail.model';
 import { Competency } from 'app/entities/competency.model';
 import { AeolusService } from 'app/exercises/programming/shared/service/aeolus.service';
+import { mergeMap, tap } from 'rxjs/operators';
+import { ProgrammingExerciseGitDiffReport } from 'app/entities/hestia/programming-exercise-git-diff-report.model';
+import { BuildLogStatisticsDTO } from 'app/entities/programming/build-log-statistics-dto';
 
 @Component({
     selector: 'jhi-programming-exercise-detail',
@@ -65,15 +68,32 @@ import { AeolusService } from 'app/exercises/programming/shared/service/aeolus.s
     encapsulation: ViewEncapsulation.None,
 })
 export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
-    readonly dayjs = dayjs;
-    readonly ActionType = ActionType;
-    readonly ProgrammingExerciseParticipationType = ProgrammingExerciseParticipationType;
-    readonly FeatureToggle = FeatureToggle;
-    readonly ProgrammingLanguage = ProgrammingLanguage;
-    readonly PROGRAMMING = ExerciseType.PROGRAMMING;
-    readonly ButtonSize = ButtonSize;
-    readonly AssessmentType = AssessmentType;
-    readonly documentationType: DocumentationType = 'Programming';
+    protected readonly dayjs = dayjs;
+    protected readonly ActionType = ActionType;
+    protected readonly ProgrammingExerciseParticipationType = ProgrammingExerciseParticipationType;
+    protected readonly FeatureToggle = FeatureToggle;
+    protected readonly ProgrammingLanguage = ProgrammingLanguage;
+    protected readonly PROGRAMMING = ExerciseType.PROGRAMMING;
+    protected readonly ButtonSize = ButtonSize;
+    protected readonly AssessmentType = AssessmentType;
+    protected readonly documentationType: DocumentationType = 'Programming';
+
+    protected readonly faUndo = faUndo;
+    protected readonly faTrash = faTrash;
+    protected readonly faBook = faBook;
+    protected readonly faWrench = faWrench;
+    protected readonly faCheckDouble = faCheckDouble;
+    protected readonly faTable = faTable;
+    protected readonly faExclamationTriangle = faExclamationTriangle;
+    protected readonly faFileSignature = faFileSignature;
+    protected readonly faListAlt = faListAlt;
+    protected readonly faChartBar = faChartBar;
+    protected readonly faLightbulb = faLightbulb;
+    protected readonly faPencilAlt = faPencilAlt;
+    protected readonly faUsers = faUsers;
+    protected readonly faEye = faEye;
+    protected readonly faUserCheck = faUserCheck;
+    protected readonly faRobot = faRobot;
 
     programmingExercise: ProgrammingExercise;
     programmingExerciseBuildConfig?: ProgrammingExerciseBuildConfig;
@@ -106,35 +126,14 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
 
     private activatedRouteSubscription: Subscription;
     private templateAndSolutionParticipationSubscription: Subscription;
-    private profileInfoSubscription: Subscription;
     private irisSettingsSubscription: Subscription;
-    private submissionPolicySubscription: Subscription;
-    private buildLogsSubscription: Subscription;
     private exerciseStatisticsSubscription: Subscription;
 
     private dialogErrorSource = new Subject<string>();
     dialogError$ = this.dialogErrorSource.asObservable();
 
     exerciseDetailSections: DetailOverviewSection[];
 
-    // Icons
-    faUndo = faUndo;
-    faTrash = faTrash;
-    faBook = faBook;
-    faWrench = faWrench;
-    faCheckDouble = faCheckDouble;
-    faTable = faTable;
-    faExclamationTriangle = faExclamationTriangle;
-    faFileSignature = faFileSignature;
-    faListAlt = faListAlt;
-    faChartBar = faChartBar;
-    faLightbulb = faLightbulb;
-    faPencilAlt = faPencilAlt;
-    faUsers = faUsers;
-    faEye = faEye;
-    faUserCheck = faUserCheck;
-    faRobot = faRobot;
-
     constructor(
         private activatedRoute: ActivatedRoute,
         private accountService: AccountService,
@@ -184,13 +183,15 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
 
             this.templateAndSolutionParticipationSubscription = this.programmingExerciseService
                 .findWithTemplateAndSolutionParticipationAndLatestResults(programmingExercise.id!)
-                .subscribe((updatedProgrammingExercise) => {
-                    this.programmingExercise = updatedProgrammingExercise.body!;
-
-                    this.setLatestCoveredLineRatio();
-                    this.loadingTemplateParticipationResults = false;
-                    this.loadingSolutionParticipationResults = false;
-                    this.profileInfoSubscription = this.profileService.getProfileInfo().subscribe(async (profileInfo) => {
+                .pipe(
+                    tap((updatedProgrammingExercise) => {
+                        this.programmingExercise = updatedProgrammingExercise.body!;
+                        this.setLatestCoveredLineRatio();
+                        this.loadingTemplateParticipationResults = false;
+                        this.loadingSolutionParticipationResults = false;
+                    }),
+                    mergeMap(() => this.profileService.getProfileInfo()),
+                    tap((profileInfo) => {
                         if (profileInfo) {
                             if (this.programmingExercise.projectKey && this.programmingExercise.templateParticipation?.buildPlanId) {
                                 this.programmingExercise.templateParticipation.buildPlanUrl = createBuildPlanUrl(
@@ -215,38 +216,41 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
                             if (this.irisEnabled) {
                                 this.irisSettingsSubscription = this.irisSettingsService.getCombinedCourseSettings(this.courseId).subscribe((settings) => {
                                     this.irisChatEnabled = settings?.irisChatSettings?.enabled ?? false;
-                                    this.exerciseDetailSections = this.getExerciseDetails();
                                 });
                             }
                         }
+                    }),
+                    mergeMap(() => this.programmingExerciseSubmissionPolicyService.getSubmissionPolicyOfProgrammingExercise(exerciseId)),
+                    tap((submissionPolicy) => {
+                        this.programmingExercise.submissionPolicy = submissionPolicy;
+                    }),
+                    mergeMap(() => this.programmingExerciseService.getDiffReport(exerciseId)),
+                    tap((gitDiffReport) => {
+                        this.processGitDiffReport(gitDiffReport, false);
+                    }),
+                    mergeMap(() =>
+                        this.programmingExercise.isAtLeastEditor ? this.programmingExerciseService.getBuildLogStatistics(exerciseId!) : of([] as BuildLogStatisticsDTO),
+                    ),
+                    tap((buildLogStatistics) => {
+                        if (this.programmingExercise.isAtLeastEditor) {
+                            this.programmingExercise.buildLogStatistics = buildLogStatistics;
+                        }
+                    }),
+                )
+                .subscribe({
+                    next: () => {
+                        this.setLatestCoveredLineRatio();
+                        this.checkAndAlertInconsistencies();
+                        this.plagiarismCheckSupported = this.programmingLanguageFeatureService.getProgrammingLanguageFeature(
+                            programmingExercise.programmingLanguage,
+                        ).plagiarismCheckSupported;
+
+                        /** we make sure to await the results of the subscriptions (switchMap) to only call {@link getExerciseDetails} once */
                         this.exerciseDetailSections = this.getExerciseDetails();
-                    });
-
-                    this.submissionPolicySubscription = this.programmingExerciseSubmissionPolicyService
-                        .getSubmissionPolicyOfProgrammingExercise(exerciseId!)
-                        .subscribe((submissionPolicy) => {
-                            this.programmingExercise.submissionPolicy = submissionPolicy;
-                            this.exerciseDetailSections = this.getExerciseDetails();
-                        });
-
-                    this.loadGitDiffReport();
-
-                    // the build logs endpoint requires at least editor privileges
-                    if (this.programmingExercise.isAtLeastEditor) {
-                        this.buildLogsSubscription = this.programmingExerciseService
-                            .getBuildLogStatistics(exerciseId!)
-                            .subscribe((buildLogStatistics) => (this.programmingExercise.buildLogStatistics = buildLogStatistics));
-                        this.exerciseDetailSections = this.getExerciseDetails();
-                    }
-
-                    this.setLatestCoveredLineRatio();
-
-                    this.checkAndAlertInconsistencies();
-
-                    this.plagiarismCheckSupported = this.programmingLanguageFeatureService.getProgrammingLanguageFeature(
-                        programmingExercise.programmingLanguage,
-                    ).plagiarismCheckSupported;
-                    this.exerciseDetailSections = this.getExerciseDetails();
+                    },
+                    error: (error) => {
+                        this.alertService.error(error.message);
+                    },
                 });
 
             this.exerciseStatisticsSubscription = this.statisticsService.getExerciseStatistics(exerciseId!).subscribe((statistics: ExerciseManagementStatisticsDto) => {
@@ -259,13 +263,17 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
         this.dialogErrorSource.unsubscribe();
         this.activatedRouteSubscription?.unsubscribe();
         this.templateAndSolutionParticipationSubscription?.unsubscribe();
-        this.profileInfoSubscription?.unsubscribe();
         this.irisSettingsSubscription?.unsubscribe();
-        this.submissionPolicySubscription?.unsubscribe();
-        this.buildLogsSubscription?.unsubscribe();
         this.exerciseStatisticsSubscription?.unsubscribe();
     }
 
+    /**
+     * <strong>BE CAREFUL WHEN CALLING THIS METHOD!</strong><br>
+     * This method can cause child components to re-render, which can lead to re-initializations resulting
+     * in unnecessary requests putting load on the server.
+     *
+     * <strong>When adding a new call to this method, make sure that no duplicated and unnecessary requests are made.</strong>
+     */
     getExerciseDetails(): DetailOverviewSection[] {
         const exercise = this.programmingExercise;
         exercise.buildConfig = this.programmingExerciseBuildConfig;
@@ -780,29 +788,37 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
         return link;
     }
 
-    loadGitDiffReport() {
-        this.programmingExerciseService.getDiffReport(this.programmingExercise.id!).subscribe((gitDiffReport) => {
-            if (
-                gitDiffReport &&
-                (this.programmingExercise.gitDiffReport?.templateRepositoryCommitHash !== gitDiffReport.templateRepositoryCommitHash ||
-                    this.programmingExercise.gitDiffReport?.solutionRepositoryCommitHash !== gitDiffReport.solutionRepositoryCommitHash)
-            ) {
-                this.programmingExercise.gitDiffReport = gitDiffReport;
-                gitDiffReport.programmingExercise = this.programmingExercise;
-                this.addedLineCount =
-                    gitDiffReport.entries
-                        ?.map((entry) => entry.lineCount)
-                        .filter((lineCount) => lineCount)
-                        .map((lineCount) => lineCount!)
-                        .reduce((lineCount1, lineCount2) => lineCount1 + lineCount2, 0) ?? 0;
-                this.removedLineCount =
-                    gitDiffReport.entries
-                        ?.map((entry) => entry.previousLineCount)
-                        .filter((lineCount) => lineCount)
-                        .map((lineCount) => lineCount!)
-                        .reduce((lineCount1, lineCount2) => lineCount1 + lineCount2, 0) ?? 0;
+    /**
+     *
+     * @param gitDiffReport
+     * @param updateDetailSections set to false when called from OnInit, as another method will take care to update the
+     *                             {@link exerciseDetailSections} to prevent unnecessary renderings and duplicated requests,
+     *                             see description of {@link getExerciseDetails}
+     */
+    private processGitDiffReport(gitDiffReport: ProgrammingExerciseGitDiffReport | undefined, updateDetailSections: boolean = true): void {
+        const isGitDiffReportUpdated =
+            gitDiffReport &&
+            (this.programmingExercise.gitDiffReport?.templateRepositoryCommitHash !== gitDiffReport.templateRepositoryCommitHash ||
+                this.programmingExercise.gitDiffReport?.solutionRepositoryCommitHash !== gitDiffReport.solutionRepositoryCommitHash);
+        if (isGitDiffReportUpdated) {
+            this.programmingExercise.gitDiffReport = gitDiffReport;
+            gitDiffReport.programmingExercise = this.programmingExercise;
+
+            const calculateLineCount = (entries: { lineCount?: number; previousLineCount?: number }[] = [], key: 'lineCount' | 'previousLineCount') =>
+                entries.map((entry) => entry[key] ?? 0).reduce((sum, count) => sum + count, 0);
+
+            this.addedLineCount = calculateLineCount(gitDiffReport.entries, 'lineCount');
+            this.removedLineCount = calculateLineCount(gitDiffReport.entries, 'previousLineCount');
+
+            if (updateDetailSections) {
                 this.exerciseDetailSections = this.getExerciseDetails();
             }
+        }
+    }
+
+    loadGitDiffReport() {
+        this.programmingExerciseService.getDiffReport(this.programmingExercise.id!).subscribe((gitDiffReport) => {
+            this.processGitDiffReport(gitDiffReport);
         });
     }
 

src/test/javascript/spec/component/detail-overview-list.component.spec.ts

@@ -56,7 +56,7 @@ describe('DetailOverviewList', () => {
     });
 
     it('should initialize and destroy', () => {
-        component.sections = sections;
+        fixture.componentRef.setInput('sections', sections);
         fixture.detectChanges();
         expect(component.headlines).toStrictEqual([{ id: 'headline-1', translationKey: 'headline.1' }]);
         expect(component.headlinesRecord).toStrictEqual({ 'headline.1': 'headline-1' });
@@ -67,7 +67,7 @@ describe('DetailOverviewList', () => {
     });
 
     it('should escape all falsy values', () => {
-        component.sections = [
+        fixture.componentRef.setInput('sections', [
             {
                 headline: 'some-section',
                 details: [
@@ -81,7 +81,7 @@ describe('DetailOverviewList', () => {
                     },
                 ],
             },
-        ];
+        ]);
         fixture.detectChanges();
         const detailListTitleDOMElements = fixture.nativeElement.querySelectorAll('dt[id^=detail-title]');
         expect(detailListTitleDOMElements).toHaveLength(1);