Programming exercises: Improve error handling when the diff report cannot be generated

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

@@ -57,7 +57,7 @@ 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 { catchError, 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';
 
@@ -225,9 +225,16 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
                         this.programmingExercise.submissionPolicy = submissionPolicy;
                     }),
                     mergeMap(() => this.programmingExerciseService.getDiffReport(exerciseId)),
+                    catchError(() => {
+                        this.alertService.error('artemisApp.programmingExercise.diffReportError');
+                        return of(undefined);
+                    }),
                     tap((gitDiffReport) => {
-                        this.processGitDiffReport(gitDiffReport, false);
+                        this.processGitDiffReport(gitDiffReport);
                     }),
+                )
+                // split pipe to keep type checks
+                .pipe(
                     mergeMap(() =>
                         this.programmingExercise.isAtLeastEditor ? this.programmingExerciseService.getBuildLogStatistics(exerciseId!) : of([] as BuildLogStatisticsDTO),
                     ),
@@ -246,7 +253,7 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
                         ).plagiarismCheckSupported;
 
                         /** we make sure to await the results of the subscriptions (switchMap) to only call {@link getExerciseDetails} once */
-                        this.exerciseDetailSections = this.getExerciseDetails();
+                        this.updateDetailSections();
                     },
                     error: (error) => {
                         this.alertService.error(error.message);
@@ -458,17 +465,18 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
                         type: ProgrammingExerciseParticipationType.SOLUTION,
                     },
                 },
-                {
-                    type: DetailType.ProgrammingDiffReport,
-                    title: 'artemisApp.programmingExercise.diffReport.title',
-                    titleHelpText: 'artemisApp.programmingExercise.diffReport.detailedTooltip',
-                    data: {
-                        addedLineCount: this.addedLineCount,
-                        removedLineCount: this.removedLineCount,
-                        isLoadingDiffReport: this.isLoadingDiffReport,
-                        gitDiffReport: exercise.gitDiffReport,
+                this.addedLineCount !== undefined &&
+                    this.removedLineCount !== undefined && {
+                        type: DetailType.ProgrammingDiffReport,
+                        title: 'artemisApp.programmingExercise.diffReport.title',
+                        titleHelpText: 'artemisApp.programmingExercise.diffReport.detailedTooltip',
+                        data: {
+                            addedLineCount: this.addedLineCount,
+                            removedLineCount: this.removedLineCount,
+                            isLoadingDiffReport: this.isLoadingDiffReport,
+                            gitDiffReport: exercise.gitDiffReport,
+                        },
                     },
-                },
                 !!exercise.buildConfig?.buildScript &&
                     !!exercise.buildConfig?.windfile?.metadata?.docker?.image && {
                         type: DetailType.Text,
@@ -789,39 +797,56 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
     }
 
     /**
-     *
+     * Calculates the added and removed lines of the diff
      * @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}
+     * @returns whether the report has changed compared to the last run
      */
-    private processGitDiffReport(gitDiffReport: ProgrammingExerciseGitDiffReport | undefined, updateDetailSections: boolean = true): void {
+    private processGitDiffReport(gitDiffReport: ProgrammingExerciseGitDiffReport | undefined): boolean {
         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();
-            }
+        if (!isGitDiffReportUpdated) {
+            return false;
         }
+
+        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');
+
+        return true;
     }
 
     loadGitDiffReport() {
-        this.programmingExerciseService.getDiffReport(this.programmingExercise.id!).subscribe((gitDiffReport) => {
-            this.processGitDiffReport(gitDiffReport);
+        this.programmingExerciseService.getDiffReport(this.programmingExercise.id!).subscribe({
+            next: (gitDiffReport) => {
+                const diffReportChanged = this.processGitDiffReport(gitDiffReport);
+                if (diffReportChanged) {
+                    this.updateDetailSections();
+                }
+            },
+            error: () => {
+                this.alertService.error('artemisApp.programmingExercise.diffReportError');
+            },
         });
     }
 
+    /**
+     * <strong>BE CAREFUL WHEN CALLING THIS METHOD!</strong><br>
+     * Warnings of {@link getExerciseDetails} apply.
+     */
+    private updateDetailSections(): void {
+        this.exerciseDetailSections = this.getExerciseDetails();
+    }
+
     createStructuralSolutionEntries() {
         this.programmingExerciseService.createStructuralSolutionEntries(this.programmingExercise.id!).subscribe({
             next: () => {

src/main/webapp/i18n/de/programmingExercise.json

@@ -216,6 +216,7 @@
             "checkPlagiarismSuccess": "Plagiatsprüfung abgeschlossen. Ergebnisse sind in der zip-Datei enthalten.",
             "combineTemplateCommitsWarning": "Diese Aktion führt alle Commits im Template-Repository in einen Commit zusammen. ACHTUNG: Hierdurch gehen alle nicht committeten Änderungen aus dem Online-Editor verloren!",
             "combineTemplateCommitsError": "Template-Repository-Commits konnten nicht zusammengeführt werden.",
+            "diffReportError": "Der Diff Report konnte nicht erstellt werden.",
             "combineTemplateCommitsSuccess": "Template-Repository-Commits wurden zusammengeführt.",
             "extractTasksFromProblemStatementWarning": "Nur zu Testzwecken: Extrahiere alle Aufgaben und Tests aus der Aufgabenstellung.",
             "extractTasksFromProblemStatementSuccess": "{{numberTasks}} extrahierte Aufgaben mit {{numberTestCases}} Tests geladen:\n{{detailedResult}}",

src/main/webapp/i18n/en/programmingExercise.json

@@ -216,6 +216,7 @@
             "checkPlagiarismSuccess": "Plagiarism check finished. Results are included in the zip file.",
             "combineTemplateCommitsWarning": "This action combines all commits in the template repository into one commit. ATTENTION: All uncommitted changes from the online editor will be lost!",
             "combineTemplateCommitsError": "Template repository commits could not be combined.",
+            "diffReportError": "Diff report could not be generated.",
             "combineTemplateCommitsSuccess": "Template repository commits have been successfully combined.",
             "extractTasksFromProblemStatementWarning": "Only for testing purpose: Extract all tasks and test cases from problems statement.",
             "extractTasksFromProblemStatementSuccess": "Loaded {{numberTasks}} extracted tasks with {{numberTestCases}} tests from the problem statement:\n{{detailedResult}}",

src/test/javascript/spec/component/programming-exercise/programming-exercise-detail.component.spec.ts

@@ -25,7 +25,7 @@ import { MockProgrammingExerciseGradingService } from '../../helpers/mocks/servi
 import { ProgrammingExerciseSolutionEntry } from 'app/entities/hestia/programming-exercise-solution-entry.model';
 import { TemplateProgrammingExerciseParticipation } from 'app/entities/participation/template-programming-exercise-participation.model';
 import { SolutionProgrammingExerciseParticipation } from 'app/entities/participation/solution-programming-exercise-participation.model';
-import { HttpResponse } from '@angular/common/http';
+import { HttpErrorResponse, HttpResponse } from '@angular/common/http';
 import { ProfileInfo } from 'app/shared/layouts/profiles/profile-info.model';
 import {
     ProgrammingLanguageFeature,
@@ -220,6 +220,19 @@ describe('ProgrammingExerciseDetailComponent', () => {
                 }
             },
         );
+
+        it('should create detail sections after getDiffReport error', fakeAsync(() => {
+            const errorSpy = jest.spyOn(alertService, 'error');
+            gitDiffReportStub.mockReturnValue(throwError(() => new HttpErrorResponse({ status: 500 })));
+
+            comp.ngOnInit();
+            tick();
+
+            expect(errorSpy).toHaveBeenCalledOnce();
+            expect(comp.exerciseDetailSections).toBeDefined();
+            expect(comp.addedLineCount).toBeUndefined();
+            expect(comp.removedLineCount).toBeUndefined();
+        }));
     });
 
     describe('onInit for exam exercise', () => {