Skip to content

Commit

Permalink
Development: Improve performance of programming exercise details view (…
Browse files Browse the repository at this point in the history 
…#9785)
  • Loading branch information
@florian-glombik
florian-glombik authored Nov 24, 2024
1 parent dbb2e17 commit 4dde77f
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 123 deletions.
2 changes: 1 addition & 1 deletion src/main/webapp/app/detail-overview-list/detail-overview-list.component.html
View file
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down
22 changes: 8 additions & 14 deletions src/main/webapp/app/detail-overview-list/detail-overview-list.component.ts
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 }[];
Expand All @@ -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,
Expand All @@ -98,6 +94,4 @@ export class DetailOverviewListComponent implements OnInit, OnDestroy {
ngOnDestroy() {
this.profileSubscription?.unsubscribe();
}

protected readonly TooltipPlacement = TooltipPlacement;
}
196 changes: 106 additions & 90 deletions src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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) => {
Expand All @@ -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;
Expand Down Expand Up @@ -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);
});
}

Expand Down
6 changes: 3 additions & 3 deletions src/test/javascript/spec/component/detail-overview-list.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand All @@ -67,7 +67,7 @@ describe('DetailOverviewList', () => {
});

it('should escape all falsy values', () => {
component.sections = [
fixture.componentRef.setInput('sections', [
{
headline: 'some-section',
details: [
Expand All @@ -81,7 +81,7 @@ describe('DetailOverviewList', () => {
},
],
},
];
]);
fixture.detectChanges();
const detailListTitleDOMElements = fixture.nativeElement.querySelectorAll('dt[id^=detail-title]');
expect(detailListTitleDOMElements).toHaveLength(1);
Expand Down
Loading

0 comments on commit 4dde77f

Please sign in to comment.