Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Dec 22, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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),
),
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -788,14 +796,7 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {
return link;
}

/**
*
* @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 {
private processGitDiffReport(gitDiffReport: ProgrammingExerciseGitDiffReport | undefined): void {
const isGitDiffReportUpdated =
gitDiffReport &&
(this.programmingExercise.gitDiffReport?.templateRepositoryCommitHash !== gitDiffReport.templateRepositoryCommitHash ||
Expand All @@ -809,19 +810,29 @@ export class ProgrammingExerciseDetailComponent implements OnInit, OnDestroy {

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);
this.programmingExerciseService.getDiffReport(this.programmingExercise.id!).subscribe({
next: (gitDiffReport) => {
this.processGitDiffReport(gitDiffReport);
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: () => {
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/i18n/de/programmingExercise.json
Original file line number Diff line number Diff line change
Expand Up @@ -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}}",
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/i18n/en/programmingExercise.json
Original file line number Diff line number Diff line change
Expand Up @@ -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}}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -220,6 +220,16 @@ describe('ProgrammingExerciseDetailComponent', () => {
}
},
);

it('should create detail sections after getDiffReport error', () => {
const errorSpy = jest.spyOn(alertService, 'error');
gitDiffReportStub.mockReturnValue(throwError(() => new HttpErrorResponse({ status: 500 })));

comp.ngOnInit();

expect(errorSpy).toHaveBeenCalledOnce();
expect(comp.exerciseDetailSections).toBeDefined();
});
});

describe('onInit for exam exercise', () => {
Expand Down
Loading