From 0ca60c5cf6aaa84d27c9c0f4346716f629292587 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Sat, 19 Oct 2024 16:57:03 +0200 Subject: [PATCH 01/19] refactor --- .../assessment/service/ResultService.java | 59 ++++++++++--------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index 07b038b9cab2..56b0e2dd230c 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java @@ -12,6 +12,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import jakarta.annotation.Nullable; @@ -482,42 +483,44 @@ public Map getLogsAvailabilityForResults(List results, Par @NotNull private List saveFeedbackWithHibernateWorkaround(@NotNull Result result, List feedbackList) { - // Avoid hibernate exception List savedFeedbacks = new ArrayList<>(); - // Collect ids of feedbacks that have long feedback. - List feedbackIdsWithLongFeedback = feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId) - .toList(); - // Get long feedback list from the database. - List longFeedbackTextList = longFeedbackTextRepository.findByFeedbackIds(feedbackIdsWithLongFeedback); - // Convert list to map for accessing later. - Map longLongFeedbackTextMap = longFeedbackTextList.stream() - .collect(Collectors.toMap(longFeedbackText -> longFeedbackText.getFeedback().getId(), longFeedbackText -> longFeedbackText)); - feedbackList.forEach(feedback -> { - // cut association to parent object - feedback.setResult(null); - LongFeedbackText longFeedback = null; - // look for long feedback that parent feedback has and cut the association between them. - if (feedback.getId() != null && feedback.getHasLongFeedbackText()) { - longFeedback = longLongFeedbackTextMap.get(feedback.getId()); - if (longFeedback != null) { - feedback.clearLongFeedback(); - } - } - // persist the child object without an association to the parent object. - feedback = feedbackRepository.saveAndFlush(feedback); - // restore the association to the parent object - feedback.setResult(result); + // Fetch long feedback texts associated with feedback that has long feedback + Map longFeedbackTextMap = longFeedbackTextRepository + .findByFeedbackIds(feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId).toList()).stream() + .collect(Collectors.toMap(longFeedbackText -> longFeedbackText.getFeedback().getId(), Function.identity())); - // restore the association of the long feedback to the parent feedback - if (longFeedback != null) { - feedback.setDetailText(longFeedback.getText()); - } + feedbackList.forEach(feedback -> { + handleFeedbackPersistence(feedback, result, longFeedbackTextMap); savedFeedbacks.add(feedback); }); + return savedFeedbacks; } + private void handleFeedbackPersistence(Feedback feedback, Result result, Map longFeedbackTextMap) { + // Temporarily detach feedback from the parent result to avoid Hibernate issues + feedback.setResult(null); + + // Clear the long feedback if it exists in the map + if (feedback.getId() != null && feedback.getHasLongFeedbackText()) { + LongFeedbackText longFeedback = longFeedbackTextMap.get(feedback.getId()); + if (longFeedback != null) { + feedback.clearLongFeedback(); + } + } + + // Persist the feedback entity without the parent association + feedback = feedbackRepository.saveAndFlush(feedback); + + // Restore associations to the result and long feedback after persistence + feedback.setResult(result); + LongFeedbackText longFeedback = longFeedbackTextMap.get(feedback.getId()); + if (longFeedback != null) { + feedback.setDetailText(longFeedback.getText()); + } + } + @NotNull private Result shouldSaveResult(@NotNull Result result, boolean shouldSave) { if (shouldSave) { From 874049a06dc1ca788bcc529adbae19d5ed578ba6 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Mon, 21 Oct 2024 12:20:12 +0200 Subject: [PATCH 02/19] test --- .../assessment/service/ResultService.java | 21 +++++++++++++++++++ .../web/ProgrammingSubmissionResource.java | 8 ++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index 56b0e2dd230c..bf2dab5e02ad 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java @@ -521,6 +521,27 @@ private void handleFeedbackPersistence(Feedback feedback, Result result, Map feedbackList) { + + List feedbackIdsWithLongFeedback = feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId) + .toList(); + + List longFeedbackTextList = new ArrayList<>(); + for (Long feedbackId : feedbackIdsWithLongFeedback) { + Optional longFeedbackText = longFeedbackTextRepository.findWithFeedbackAndResultAndParticipationByFeedbackId(feedbackId); + longFeedbackText.ifPresent(longFeedbackTextList::add); + } + + for (Feedback feedback : feedbackList) { + if (feedback.getId() != null && feedback.getHasLongFeedbackText()) { + longFeedbackTextList.stream().filter(lft -> lft.getFeedback().getId().equals(feedback.getId())).findFirst().ifPresent(longFeedback -> { + feedback.clearLongFeedback(); + feedback.setDetailText(longFeedback.getText()); + }); + } + } + } + @NotNull private Result shouldSaveResult(@NotNull Result result, boolean shouldSave) { if (shouldSave) { diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java index db197fed2c26..c5a75475e2c3 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java @@ -24,6 +24,7 @@ import de.tum.cit.aet.artemis.assessment.domain.GradingCriterion; import de.tum.cit.aet.artemis.assessment.domain.Result; import de.tum.cit.aet.artemis.assessment.repository.GradingCriterionRepository; +import de.tum.cit.aet.artemis.assessment.service.ResultService; import de.tum.cit.aet.artemis.core.domain.User; import de.tum.cit.aet.artemis.core.exception.AccessForbiddenException; import de.tum.cit.aet.artemis.core.exception.BadRequestAlertException; @@ -95,13 +96,15 @@ public class ProgrammingSubmissionResource { private final ExerciseDateService exerciseDateService; + private final ResultService resultService; + public ProgrammingSubmissionResource(ProgrammingSubmissionService programmingSubmissionService, ProgrammingTriggerService programmingTriggerService, ProgrammingMessagingService programmingMessagingService, ExerciseRepository exerciseRepository, ParticipationRepository participationRepository, ProgrammingExerciseRepository programmingExerciseRepository, AuthorizationCheckService authCheckService, ParticipationAuthorizationCheckService participationAuthCheckService, ProgrammingExerciseStudentParticipationRepository programmingExerciseStudentParticipationRepository, GradingCriterionRepository gradingCriterionRepository, SubmissionRepository submissionRepository, Optional continuousIntegrationService, UserRepository userRepository, - ExerciseDateService exerciseDateService) { + ExerciseDateService exerciseDateService, ResultService resultService) { this.programmingSubmissionService = programmingSubmissionService; this.programmingTriggerService = programmingTriggerService; this.programmingMessagingService = programmingMessagingService; @@ -116,6 +119,7 @@ public ProgrammingSubmissionResource(ProgrammingSubmissionService programmingSub this.continuousIntegrationService = continuousIntegrationService; this.userRepository = userRepository; this.exerciseDateService = exerciseDateService; + this.resultService = resultService; } /** @@ -349,6 +353,8 @@ public ResponseEntity lockAndGetProgrammingSubmission(@Pa programmingSubmission.setResults(Collections.emptyList()); } else { + Result manualResult = manualResults.get(correctionRound); + resultService.fetchAndSetLongFeedbackTexts(manualResult.getFeedbacks()); programmingSubmission.setResults(Collections.singletonList(manualResults.get(correctionRound))); } programmingSubmission.getParticipation().setResults(new HashSet<>(programmingSubmission.getResults())); From 4d88cfed1ca8181836204db94f15d2e20c962f70 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Mon, 21 Oct 2024 23:11:24 +0200 Subject: [PATCH 03/19] Revert "test" This reverts commit 874049a06dc1ca788bcc529adbae19d5ed578ba6. --- .../assessment/service/ResultService.java | 21 ------------------- .../web/ProgrammingSubmissionResource.java | 8 +------ 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index bf2dab5e02ad..56b0e2dd230c 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java @@ -521,27 +521,6 @@ private void handleFeedbackPersistence(Feedback feedback, Result result, Map feedbackList) { - - List feedbackIdsWithLongFeedback = feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId) - .toList(); - - List longFeedbackTextList = new ArrayList<>(); - for (Long feedbackId : feedbackIdsWithLongFeedback) { - Optional longFeedbackText = longFeedbackTextRepository.findWithFeedbackAndResultAndParticipationByFeedbackId(feedbackId); - longFeedbackText.ifPresent(longFeedbackTextList::add); - } - - for (Feedback feedback : feedbackList) { - if (feedback.getId() != null && feedback.getHasLongFeedbackText()) { - longFeedbackTextList.stream().filter(lft -> lft.getFeedback().getId().equals(feedback.getId())).findFirst().ifPresent(longFeedback -> { - feedback.clearLongFeedback(); - feedback.setDetailText(longFeedback.getText()); - }); - } - } - } - @NotNull private Result shouldSaveResult(@NotNull Result result, boolean shouldSave) { if (shouldSave) { diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java index c5a75475e2c3..db197fed2c26 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java @@ -24,7 +24,6 @@ import de.tum.cit.aet.artemis.assessment.domain.GradingCriterion; import de.tum.cit.aet.artemis.assessment.domain.Result; import de.tum.cit.aet.artemis.assessment.repository.GradingCriterionRepository; -import de.tum.cit.aet.artemis.assessment.service.ResultService; import de.tum.cit.aet.artemis.core.domain.User; import de.tum.cit.aet.artemis.core.exception.AccessForbiddenException; import de.tum.cit.aet.artemis.core.exception.BadRequestAlertException; @@ -96,15 +95,13 @@ public class ProgrammingSubmissionResource { private final ExerciseDateService exerciseDateService; - private final ResultService resultService; - public ProgrammingSubmissionResource(ProgrammingSubmissionService programmingSubmissionService, ProgrammingTriggerService programmingTriggerService, ProgrammingMessagingService programmingMessagingService, ExerciseRepository exerciseRepository, ParticipationRepository participationRepository, ProgrammingExerciseRepository programmingExerciseRepository, AuthorizationCheckService authCheckService, ParticipationAuthorizationCheckService participationAuthCheckService, ProgrammingExerciseStudentParticipationRepository programmingExerciseStudentParticipationRepository, GradingCriterionRepository gradingCriterionRepository, SubmissionRepository submissionRepository, Optional continuousIntegrationService, UserRepository userRepository, - ExerciseDateService exerciseDateService, ResultService resultService) { + ExerciseDateService exerciseDateService) { this.programmingSubmissionService = programmingSubmissionService; this.programmingTriggerService = programmingTriggerService; this.programmingMessagingService = programmingMessagingService; @@ -119,7 +116,6 @@ public ProgrammingSubmissionResource(ProgrammingSubmissionService programmingSub this.continuousIntegrationService = continuousIntegrationService; this.userRepository = userRepository; this.exerciseDateService = exerciseDateService; - this.resultService = resultService; } /** @@ -353,8 +349,6 @@ public ResponseEntity lockAndGetProgrammingSubmission(@Pa programmingSubmission.setResults(Collections.emptyList()); } else { - Result manualResult = manualResults.get(correctionRound); - resultService.fetchAndSetLongFeedbackTexts(manualResult.getFeedbacks()); programmingSubmission.setResults(Collections.singletonList(manualResults.get(correctionRound))); } programmingSubmission.getParticipation().setResults(new HashSet<>(programmingSubmission.getResults())); From 952d9d863aa15e5a25ed3cdd6589796529a0ed7f Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 22 Oct 2024 16:03:47 +0200 Subject: [PATCH 04/19] test --- .../assessment/service/ResultService.java | 2 +- .../unreferenced-feedback-detail.component.ts | 21 +++++++++-- .../services/base-api-http.service.ts | 16 ++++++--- .../file-upload-assessment.component.html | 1 + .../modeling-assessment-editor.component.html | 36 +++++++++++-------- ...example-modeling-submission.component.html | 1 + ...-tutor-assessment-container.component.html | 21 ++++++----- .../shared/feedback/feedback.service.ts | 8 ++++- .../unreferenced-feedback.component.html | 2 ++ .../unreferenced-feedback.component.ts | 1 + .../text-submission-assessment.component.html | 2 ++ .../example-text-submission.component.html | 8 ++++- 12 files changed, 85 insertions(+), 34 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index 56b0e2dd230c..b6db6c40ff7b 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java @@ -485,7 +485,7 @@ public Map getLogsAvailabilityForResults(List results, Par private List saveFeedbackWithHibernateWorkaround(@NotNull Result result, List feedbackList) { List savedFeedbacks = new ArrayList<>(); - // Fetch long feedback texts associated with feedback that has long feedback + // Fetch long feedback texts associated with the provided feedback list Map longFeedbackTextMap = longFeedbackTextRepository .findByFeedbackIds(feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId).toList()).stream() .collect(Collectors.toMap(longFeedbackText -> longFeedbackText.getFeedback().getId(), Function.identity())); diff --git a/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts b/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts index 7aebe252c32c..a7b25c900022 100644 --- a/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts +++ b/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts @@ -1,17 +1,19 @@ -import { Component, EventEmitter, Input, Output } from '@angular/core'; +import { Component, EventEmitter, Input, OnInit, Output, inject } from '@angular/core'; import { faCheck, faExclamation, faExclamationTriangle, faQuestionCircle, faTrash, faTrashAlt } from '@fortawesome/free-solid-svg-icons'; import { Feedback, FeedbackType } from 'app/entities/feedback.model'; import { StructuredGradingCriterionService } from 'app/exercises/shared/structured-grading-criterion/structured-grading-criterion.service'; import { ButtonSize } from 'app/shared/components/button.component'; import { Subject } from 'rxjs'; +import { FeedbackService } from 'app/exercises/shared/feedback/feedback.service'; @Component({ selector: 'jhi-unreferenced-feedback-detail', templateUrl: './unreferenced-feedback-detail.component.html', styleUrls: ['./unreferenced-feedback-detail.component.scss'], }) -export class UnreferencedFeedbackDetailComponent { +export class UnreferencedFeedbackDetailComponent implements OnInit { @Input() public feedback: Feedback; + @Input() resultId: number; @Input() isSuggestion: boolean; @Input() public readOnly: boolean; @Input() highlightDifferences: boolean; @@ -21,6 +23,7 @@ export class UnreferencedFeedbackDetailComponent { @Output() public onFeedbackDelete = new EventEmitter(); @Output() onAcceptSuggestion = new EventEmitter(); @Output() onDiscardSuggestion = new EventEmitter(); + private feedbackService = inject(FeedbackService); // Icons faTrashAlt = faTrashAlt; @@ -39,6 +42,20 @@ export class UnreferencedFeedbackDetailComponent { constructor(public structuredGradingCriterionService: StructuredGradingCriterionService) {} + ngOnInit() { + this.loadLongFeedback(); + } + + /** + * Call this method to load long feedback if needed + */ + public async loadLongFeedback() { + if (this.feedback.hasLongFeedbackText) { + this.feedback.detailText = await this.feedbackService.getLongFeedbackText(this.resultId, this.feedback.id!); + this.onFeedbackChange.emit(this.feedback); + } + } + /** * Emits assessment changes to parent component */ diff --git a/src/main/webapp/app/course/learning-paths/services/base-api-http.service.ts b/src/main/webapp/app/course/learning-paths/services/base-api-http.service.ts index 898a2de8d301..ba1e3cd814ff 100644 --- a/src/main/webapp/app/course/learning-paths/services/base-api-http.service.ts +++ b/src/main/webapp/app/course/learning-paths/services/base-api-http.service.ts @@ -10,7 +10,7 @@ export abstract class BaseApiHttpService { private readonly baseUrl = 'api'; /** - * Debounces a function call to prevent it from being called multiple times in a short period. + * Debounce a function call to prevent it from being called multiple times in a short period. * @param callback The function to debounce. * @param delay The delay in milliseconds to wait before calling the function. */ @@ -52,17 +52,18 @@ export abstract class BaseApiHttpService { | { [param: string]: string | number | boolean | ReadonlyArray; }; - responseType?: 'json'; + responseType?: 'json' | 'text'; }, ): Promise { try { const response = await lastValueFrom( - this.httpClient.request(method, `${this.baseUrl}/${url}`, { - observe: 'response', + this.httpClient.request(method, `${this.baseUrl}/${url}`, { + observe: 'body', ...options, + responseType: options?.responseType ?? 'json', }), ); - return response.body!; + return response as T; } catch (error) { throw error as HttpErrorResponse; } @@ -108,6 +109,7 @@ export abstract class BaseApiHttpService { | { [param: string]: string | number | boolean | ReadonlyArray; }; + responseType?: 'json' | 'text'; }, ): Promise { return await this.request(HttpMethod.Get, url, options); @@ -139,6 +141,7 @@ export abstract class BaseApiHttpService { | { [param: string]: string | number | boolean | ReadonlyArray; }; + responseType?: 'json' | 'text'; }, ): Promise { return await this.request(HttpMethod.Post, url, { body: body, ...options }); @@ -168,6 +171,7 @@ export abstract class BaseApiHttpService { | { [param: string]: string | number | boolean | ReadonlyArray; }; + responseType?: 'json' | 'text'; }, ): Promise { return await this.request(HttpMethod.Delete, url, options); @@ -198,6 +202,7 @@ export abstract class BaseApiHttpService { | { [param: string]: string | number | boolean | ReadonlyArray; }; + responseType?: 'json' | 'text'; }, ): Promise { return await this.request(HttpMethod.Patch, url, { body: body, ...options }); @@ -228,6 +233,7 @@ export abstract class BaseApiHttpService { | { [param: string]: string | number | boolean | ReadonlyArray; }; + responseType?: 'json' | 'text'; }, ): Promise { return await this.request(HttpMethod.Put, url, { body: body, ...options }); diff --git a/src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.component.html b/src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.component.html index 002e35fe3ee3..359aefa07389 100644 --- a/src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.component.html +++ b/src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.component.html @@ -54,6 +54,7 @@ (feedbacksChange)="validateAssessment()" [readOnly]="readOnly" [highlightDifferences]="highlightDifferences" + [resultId]="resultId" /> } @else { diff --git a/src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.component.html b/src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.component.html index 44a7f145c8ad..3909dcfd25d7 100644 --- a/src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.component.html +++ b/src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.component.html @@ -64,15 +64,18 @@ }
- + @if (result?.id) { + + } @if ((hasAutomaticFeedback || highlightMissingFeedback) && !result?.completionDate) {

@@ -144,12 +147,15 @@

}

- + @if (result?.id) { + + } @if ((hasAutomaticFeedback || highlightMissingFeedback) && !result?.completionDate) {

diff --git a/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html b/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html index 8f414f19509e..cc1d565f477e 100644 --- a/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html +++ b/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html @@ -188,6 +188,7 @@

[readOnly]="readOnly" [addReferenceIdForExampleSubmission]="true" (feedbacksChange)="onUnReferencedFeedbackChanged($event)" + [resultId]="result.id!" /> }

diff --git a/src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.html b/src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.html index 4d1a6fa449e2..893937f7d217 100644 --- a/src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.html +++ b/src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.html @@ -138,15 +138,18 @@
- + @if (manualResult?.id) { + + }
diff --git a/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts b/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts index b619364d0ec9..37c8f777afec 100644 --- a/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts +++ b/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts @@ -1,8 +1,9 @@ import { Injectable } from '@angular/core'; import { Feedback } from 'app/entities/feedback.model'; +import { BaseApiHttpService } from 'app/course/learning-paths/services/base-api-http.service'; @Injectable({ providedIn: 'root' }) -export class FeedbackService { +export class FeedbackService extends BaseApiHttpService { /** * Filters the feedback based on the filter input * Used e.g. when we want to show certain test cases viewed from the exercise description @@ -15,4 +16,9 @@ export class FeedbackService { } return feedbacks.filter((feedback) => feedback.testCase?.id && filter.includes(feedback.testCase.id)); }; + + public async getLongFeedbackText(resultId: number, feedbackId: number): Promise { + const url = `results/${resultId}/feedbacks/${feedbackId}/long-feedback`; + return await this.get(url, { responseType: 'text' }); + } } diff --git a/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.html b/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.html index 0f6975db8be5..1091dc0c8c7d 100644 --- a/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.html +++ b/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.html @@ -24,6 +24,7 @@

[readOnly]="readOnly" [highlightDifferences]="highlightDifferences" [useDefaultFeedbackSuggestionBadgeText]="useDefaultFeedbackSuggestionBadgeText" + [resultId]="resultId" />
} @@ -36,6 +37,7 @@

(onAcceptSuggestion)="acceptSuggestion($event)" (onDiscardSuggestion)="discardSuggestion($event)" [useDefaultFeedbackSuggestionBadgeText]="useDefaultFeedbackSuggestionBadgeText" + [resultId]="resultId" />
} diff --git a/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.ts b/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.ts index d0e7ac4a9c00..92c12a2cda57 100644 --- a/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.ts +++ b/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.ts @@ -17,6 +17,7 @@ export class UnreferencedFeedbackComponent { @Input() readOnly: boolean; @Input() highlightDifferences: boolean; @Input() useDefaultFeedbackSuggestionBadgeText: boolean = false; + @Input() resultId: number; /** * In order to make it possible to mark unreferenced feedback based on the correction status, we assign reference ids to the unreferenced feedback diff --git a/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html b/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html index c9c661d2f05f..c2320dbaa853 100644 --- a/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html +++ b/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html @@ -56,6 +56,7 @@ (feedbacksChange)="validateFeedback()" [readOnly]="readOnly" [useDefaultFeedbackSuggestionBadgeText]="true" + [resultId]="resultId" /> } @else { @@ -107,6 +108,7 @@ (feedbacksChange)="validateFeedback()" [readOnly]="readOnly" [useDefaultFeedbackSuggestionBadgeText]="true" + [resultId]="resultId" /> diff --git a/src/main/webapp/app/exercises/text/manage/example-text-submission/example-text-submission.component.html b/src/main/webapp/app/exercises/text/manage/example-text-submission/example-text-submission.component.html index b723733cb65e..dce96496b924 100644 --- a/src/main/webapp/app/exercises/text/manage/example-text-submission/example-text-submission.component.html +++ b/src/main/webapp/app/exercises/text/manage/example-text-submission/example-text-submission.component.html @@ -159,7 +159,13 @@
+ } @if (toComplete) {
From bc1d2228396c62b11a182ab5b9ce69ee8daacc28 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 22 Oct 2024 17:28:47 +0200 Subject: [PATCH 05/19] bugfix implemented --- .../file-upload-assessment.component.html | 31 +++++++++------- .../modeling-assessment-editor.component.html | 4 +-- ...-tutor-assessment-container.component.html | 4 +-- .../text-submission-assessment.component.html | 36 ++++++++++--------- 4 files changed, 42 insertions(+), 33 deletions(-) diff --git a/src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.component.html b/src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.component.html index 359aefa07389..c019fc8ae1f0 100644 --- a/src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.component.html +++ b/src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.component.html @@ -49,13 +49,15 @@ @if (invalidError) { } - + @if (result && result.id) { + + }
} @else { @if (!loadingInitialSubmission) { @@ -97,11 +99,14 @@ @if (invalidError) { } - + @if (result && result.id) { + + } diff --git a/src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.component.html b/src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.component.html index 3909dcfd25d7..e80ce157b398 100644 --- a/src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.component.html +++ b/src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.component.html @@ -64,7 +64,7 @@ }
- @if (result?.id) { + @if (result && result.id) { }
- @if (result?.id) { + @if (result && result.id) {
- @if (manualResult?.id) { + @if (manualResult && manualResult.id) { }
diff --git a/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html b/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html index c2320dbaa853..27f10b3c1e4a 100644 --- a/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html +++ b/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html @@ -50,14 +50,16 @@
- + @if (result && result.id) { + + }
} @else { @if (!loadingInitialSubmission) { @@ -102,13 +104,15 @@
- + @if (result && result.id) { + + }
From af47faff37143f5d7085771c7194a69184f8cf7e Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 22 Oct 2024 23:42:04 +0200 Subject: [PATCH 06/19] bugfix implemented --- .../assessment/service/AssessmentService.java | 17 +++++++++++++++++ .../example-modeling-submission.component.html | 16 +++++++++------- .../example-text-submission.component.html | 16 +++++++++------- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java index 1a451ad49aff..4002c8cfa7d0 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Optional; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Profile; import org.springframework.stereotype.Service; @@ -13,10 +14,12 @@ import de.tum.cit.aet.artemis.assessment.domain.AssessmentType; import de.tum.cit.aet.artemis.assessment.domain.ComplaintResponse; import de.tum.cit.aet.artemis.assessment.domain.Feedback; +import de.tum.cit.aet.artemis.assessment.domain.LongFeedbackText; import de.tum.cit.aet.artemis.assessment.domain.Result; import de.tum.cit.aet.artemis.assessment.dto.AssessmentUpdateBaseDTO; import de.tum.cit.aet.artemis.assessment.repository.ComplaintRepository; import de.tum.cit.aet.artemis.assessment.repository.FeedbackRepository; +import de.tum.cit.aet.artemis.assessment.repository.LongFeedbackTextRepository; import de.tum.cit.aet.artemis.assessment.repository.ResultRepository; import de.tum.cit.aet.artemis.assessment.web.ResultWebsocketService; import de.tum.cit.aet.artemis.communication.service.notifications.SingleUserNotificationService; @@ -251,6 +254,9 @@ private Result submitManualAssessment(long resultId, Exercise exercise) { return result; } + @Autowired + private LongFeedbackTextRepository longFeedbackTextRepository; + /** * This function is used for saving a manual assessment/result. It sets the assessment type to MANUAL and sets the assessor attribute. Furthermore, it saves the result in the * database. If a result with the given id exists, it will be overridden. if not, a new result will be created. @@ -283,6 +289,17 @@ public Result saveManualAssessment(final Submission submission, final List existingLongFeedbackText = longFeedbackTextRepository.findByFeedbackId(feedback.getId()); + if (existingLongFeedbackText.isPresent()) { + LongFeedbackText longFeedbackText = existingLongFeedbackText.get(); + longFeedbackText.setText(feedback.getDetailText()); + longFeedbackTextRepository.delete(longFeedbackText); + } + } + } + result.updateAllFeedbackItems(feedbackList, false); result.determineAssessmentType(); diff --git a/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html b/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html index cc1d565f477e..735c38e092f4 100644 --- a/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html +++ b/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html @@ -183,13 +183,15 @@

} @if (assessmentMode) { - + @if (result && result.id) { + + } } @if (exercise) { diff --git a/src/main/webapp/app/exercises/text/manage/example-text-submission/example-text-submission.component.html b/src/main/webapp/app/exercises/text/manage/example-text-submission/example-text-submission.component.html index dce96496b924..481bba4660a8 100644 --- a/src/main/webapp/app/exercises/text/manage/example-text-submission/example-text-submission.component.html +++ b/src/main/webapp/app/exercises/text/manage/example-text-submission/example-text-submission.component.html @@ -159,13 +159,15 @@
+ @if (result && result.id) { + + } } @if (toComplete) {
From e23f45ff333b045d230ae0192170c0c8d45fc6a7 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Thu, 24 Oct 2024 20:34:28 +0200 Subject: [PATCH 07/19] distinguish bugfix --- .../assessment/service/AssessmentService.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java index 4002c8cfa7d0..5aa2729a14f0 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java @@ -14,7 +14,6 @@ import de.tum.cit.aet.artemis.assessment.domain.AssessmentType; import de.tum.cit.aet.artemis.assessment.domain.ComplaintResponse; import de.tum.cit.aet.artemis.assessment.domain.Feedback; -import de.tum.cit.aet.artemis.assessment.domain.LongFeedbackText; import de.tum.cit.aet.artemis.assessment.domain.Result; import de.tum.cit.aet.artemis.assessment.dto.AssessmentUpdateBaseDTO; import de.tum.cit.aet.artemis.assessment.repository.ComplaintRepository; @@ -288,18 +287,6 @@ public Result saveManualAssessment(final Submission submission, final List existingLongFeedbackText = longFeedbackTextRepository.findByFeedbackId(feedback.getId()); - if (existingLongFeedbackText.isPresent()) { - LongFeedbackText longFeedbackText = existingLongFeedbackText.get(); - longFeedbackText.setText(feedback.getDetailText()); - longFeedbackTextRepository.delete(longFeedbackText); - } - } - } - result.updateAllFeedbackItems(feedbackList, false); result.determineAssessmentType(); From 6b28beca54140b770da25509fce8985daac92562 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Thu, 24 Oct 2024 20:37:23 +0200 Subject: [PATCH 08/19] distinguish bugfix --- .../aet/artemis/assessment/service/AssessmentService.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java index 5aa2729a14f0..20ab20684bbe 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java @@ -6,7 +6,6 @@ import java.util.List; import java.util.Optional; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Profile; import org.springframework.stereotype.Service; @@ -18,7 +17,6 @@ import de.tum.cit.aet.artemis.assessment.dto.AssessmentUpdateBaseDTO; import de.tum.cit.aet.artemis.assessment.repository.ComplaintRepository; import de.tum.cit.aet.artemis.assessment.repository.FeedbackRepository; -import de.tum.cit.aet.artemis.assessment.repository.LongFeedbackTextRepository; import de.tum.cit.aet.artemis.assessment.repository.ResultRepository; import de.tum.cit.aet.artemis.assessment.web.ResultWebsocketService; import de.tum.cit.aet.artemis.communication.service.notifications.SingleUserNotificationService; @@ -253,9 +251,6 @@ private Result submitManualAssessment(long resultId, Exercise exercise) { return result; } - @Autowired - private LongFeedbackTextRepository longFeedbackTextRepository; - /** * This function is used for saving a manual assessment/result. It sets the assessment type to MANUAL and sets the assessor attribute. Furthermore, it saves the result in the * database. If a result with the given id exists, it will be overridden. if not, a new result will be created. From a331eeab1e949f87208347764da6035a9dabaa84 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Thu, 24 Oct 2024 22:25:38 +0200 Subject: [PATCH 09/19] bugfix for longfeedbacktexts not editable --- .../assessment/service/AssessmentService.java | 10 +++++++- .../assessment/service/ResultService.java | 25 +++++++++++++++++++ .../service/ProgrammingAssessmentService.java | 11 ++++++-- .../text/service/TextAssessmentService.java | 5 ++-- 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java index 20ab20684bbe..cb0d52351420 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java @@ -17,6 +17,7 @@ import de.tum.cit.aet.artemis.assessment.dto.AssessmentUpdateBaseDTO; import de.tum.cit.aet.artemis.assessment.repository.ComplaintRepository; import de.tum.cit.aet.artemis.assessment.repository.FeedbackRepository; +import de.tum.cit.aet.artemis.assessment.repository.LongFeedbackTextRepository; import de.tum.cit.aet.artemis.assessment.repository.ResultRepository; import de.tum.cit.aet.artemis.assessment.web.ResultWebsocketService; import de.tum.cit.aet.artemis.communication.service.notifications.SingleUserNotificationService; @@ -67,10 +68,12 @@ public class AssessmentService { protected final ResultWebsocketService resultWebsocketService; + private final LongFeedbackTextRepository longFeedbackTextRepository; + public AssessmentService(ComplaintResponseService complaintResponseService, ComplaintRepository complaintRepository, FeedbackRepository feedbackRepository, ResultRepository resultRepository, StudentParticipationRepository studentParticipationRepository, ResultService resultService, SubmissionService submissionService, SubmissionRepository submissionRepository, ExamDateService examDateService, UserRepository userRepository, Optional ltiNewResultService, - SingleUserNotificationService singleUserNotificationService, ResultWebsocketService resultWebsocketService) { + SingleUserNotificationService singleUserNotificationService, ResultWebsocketService resultWebsocketService, LongFeedbackTextRepository longFeedbackTextRepository) { this.complaintResponseService = complaintResponseService; this.complaintRepository = complaintRepository; this.feedbackRepository = feedbackRepository; @@ -84,6 +87,7 @@ public AssessmentService(ComplaintResponseService complaintResponseService, Comp this.ltiNewResultService = ltiNewResultService; this.singleUserNotificationService = singleUserNotificationService; this.resultWebsocketService = resultWebsocketService; + this.longFeedbackTextRepository = longFeedbackTextRepository; } /** @@ -282,6 +286,10 @@ public Result saveManualAssessment(final Submission submission, final List feedbacks = new ArrayList<>(result.getFeedbacks()); + result.updateAllFeedbackItems(feedbacks, true); // Note: This also saves the feedback objects in the database because of the 'cascade = CascadeType.ALL' option. return resultRepository.save(result); } @@ -626,4 +630,25 @@ public FeedbackAnalysisResponseDTO getFeedbackDetailsOnPage(long exerciseId, Fee public long getMaxCountForExercise(long exerciseId) { return studentParticipationRepository.findMaxCountForExercise(exerciseId); } + + /** + * Deletes long feedback texts for the provided list of feedback items. + *
+ * This method iterates over the provided list of feedback and checks if each feedback has an associated long feedback text. + * If the feedback has a long feedback text and its ID is not null, the method fetches the corresponding {@link LongFeedbackText} + * from the repository and deletes it. + *

+ * This is useful in cases where long feedback texts need to be removed, such as during feedback cleanup operations. + * + * @param feedbackList The list of {@link Feedback} objects for which the long feedback texts are to be deleted. + * Only feedback items that have long feedback texts and a non-null ID will be processed. + */ + public void deleteLongFeedback(List feedbackList) { + for (Feedback feedback : feedbackList) { + if (feedback.getHasLongFeedbackText() && feedback.getId() != null) { + Optional longFeedbackTextOpt = longFeedbackTextRepository.findByFeedbackId(feedback.getId()); + longFeedbackTextOpt.ifPresent(longFeedbackTextRepository::delete); + } + } + } } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java index a97887171fae..d1350e334c2d 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java @@ -3,6 +3,7 @@ import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE; import java.time.ZonedDateTime; +import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -16,6 +17,7 @@ import de.tum.cit.aet.artemis.assessment.repository.ComplaintRepository; import de.tum.cit.aet.artemis.assessment.repository.FeedbackRepository; import de.tum.cit.aet.artemis.assessment.repository.GradingCriterionRepository; +import de.tum.cit.aet.artemis.assessment.repository.LongFeedbackTextRepository; import de.tum.cit.aet.artemis.assessment.repository.ResultRepository; import de.tum.cit.aet.artemis.assessment.service.AssessmentService; import de.tum.cit.aet.artemis.assessment.service.ComplaintResponseService; @@ -48,9 +50,10 @@ public ProgrammingAssessmentService(ComplaintResponseService complaintResponseSe ResultRepository resultRepository, StudentParticipationRepository studentParticipationRepository, ResultService resultService, SubmissionService submissionService, SubmissionRepository submissionRepository, ExamDateService examDateService, UserRepository userRepository, GradingCriterionRepository gradingCriterionRepository, Optional ltiNewResultService, SingleUserNotificationService singleUserNotificationService, ResultWebsocketService resultWebsocketService, - ProgrammingExerciseParticipationService programmingExerciseParticipationService, Optional athenaFeedbackSendingService) { + ProgrammingExerciseParticipationService programmingExerciseParticipationService, Optional athenaFeedbackSendingService, + LongFeedbackTextRepository longFeedbackTextRepository) { super(complaintResponseService, complaintRepository, feedbackRepository, resultRepository, studentParticipationRepository, resultService, submissionService, - submissionRepository, examDateService, userRepository, ltiNewResultService, singleUserNotificationService, resultWebsocketService); + submissionRepository, examDateService, userRepository, ltiNewResultService, singleUserNotificationService, resultWebsocketService, longFeedbackTextRepository); this.programmingExerciseParticipationService = programmingExerciseParticipationService; this.athenaFeedbackSendingService = athenaFeedbackSendingService; } @@ -88,6 +91,10 @@ private Result saveManualAssessment(Result result, User assessor) { * @return the new saved result */ public Result saveAndSubmitManualAssessment(StudentParticipation participation, Result newManualResult, Result existingManualResult, User assessor, boolean submit) { + // we do this to avoid problems of editing longFeedbackTexts in manual Assesements + resultService.deleteLongFeedback(newManualResult.getFeedbacks()); + List feedbacks = new ArrayList<>(newManualResult.getFeedbacks()); + newManualResult.updateAllFeedbackItems(feedbacks, true); // make sure that the submission cannot be manipulated on the client side var submission = (ProgrammingSubmission) existingManualResult.getSubmission(); ProgrammingExercise exercise = (ProgrammingExercise) participation.getExercise(); diff --git a/src/main/java/de/tum/cit/aet/artemis/text/service/TextAssessmentService.java b/src/main/java/de/tum/cit/aet/artemis/text/service/TextAssessmentService.java index b46a28bedb66..ad1e367c2735 100644 --- a/src/main/java/de/tum/cit/aet/artemis/text/service/TextAssessmentService.java +++ b/src/main/java/de/tum/cit/aet/artemis/text/service/TextAssessmentService.java @@ -16,6 +16,7 @@ import de.tum.cit.aet.artemis.assessment.repository.ComplaintRepository; import de.tum.cit.aet.artemis.assessment.repository.FeedbackRepository; import de.tum.cit.aet.artemis.assessment.repository.GradingCriterionRepository; +import de.tum.cit.aet.artemis.assessment.repository.LongFeedbackTextRepository; import de.tum.cit.aet.artemis.assessment.repository.ResultRepository; import de.tum.cit.aet.artemis.assessment.service.AssessmentService; import de.tum.cit.aet.artemis.assessment.service.ComplaintResponseService; @@ -41,9 +42,9 @@ public TextAssessmentService(UserRepository userRepository, ComplaintResponseSer FeedbackRepository feedbackRepository, ResultRepository resultRepository, StudentParticipationRepository studentParticipationRepository, ResultService resultService, SubmissionRepository submissionRepository, TextBlockService textBlockService, ExamDateService examDateService, GradingCriterionRepository gradingCriterionRepository, SubmissionService submissionService, Optional ltiNewResultService, SingleUserNotificationService singleUserNotificationService, - ResultWebsocketService resultWebsocketService) { + ResultWebsocketService resultWebsocketService, LongFeedbackTextRepository longFeedbackTextRepository) { super(complaintResponseService, complaintRepository, feedbackRepository, resultRepository, studentParticipationRepository, resultService, submissionService, - submissionRepository, examDateService, userRepository, ltiNewResultService, singleUserNotificationService, resultWebsocketService); + submissionRepository, examDateService, userRepository, ltiNewResultService, singleUserNotificationService, resultWebsocketService, longFeedbackTextRepository); this.textBlockService = textBlockService; } From 49a0a3e9b18d7fa849c2e829901cd44d10daf36e Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Thu, 24 Oct 2024 22:52:58 +0200 Subject: [PATCH 10/19] client tests adjusted --- ...ferenced-feedback-detail.component.spec.ts | 3 +- .../service/feedback/feedback-service.spec.ts | 29 ++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts b/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts index bf2ba70ea18d..8f8d667ac3fa 100644 --- a/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts +++ b/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts @@ -15,6 +15,7 @@ import { AssessmentCorrectionRoundBadgeComponent } from 'app/assessment/unrefere import { StructuredGradingCriterionService } from 'app/exercises/shared/structured-grading-criterion/structured-grading-criterion.service'; import { QuotePipe } from 'app/shared/pipes/quote.pipe'; import { FeedbackContentPipe } from 'app/shared/pipes/feedback-content.pipe'; +import { FeedbackService } from 'app/exercises/shared/feedback/feedback.service'; describe('Unreferenced Feedback Detail Component', () => { let comp: UnreferencedFeedbackDetailComponent; @@ -36,7 +37,7 @@ describe('Unreferenced Feedback Detail Component', () => { MockDirective(DeleteButtonDirective), MockComponent(AssessmentCorrectionRoundBadgeComponent), ], - providers: [{ provide: TranslateService, useClass: MockTranslateService }, MockProvider(StructuredGradingCriterionService)], + providers: [{ provide: TranslateService, useClass: MockTranslateService }, MockProvider(StructuredGradingCriterionService), MockProvider(FeedbackService)], }) .compileComponents() .then(() => { diff --git a/src/test/javascript/spec/service/feedback/feedback-service.spec.ts b/src/test/javascript/spec/service/feedback/feedback-service.spec.ts index 230e6ec2987e..1c5071c65ee5 100644 --- a/src/test/javascript/spec/service/feedback/feedback-service.spec.ts +++ b/src/test/javascript/spec/service/feedback/feedback-service.spec.ts @@ -1,11 +1,24 @@ +import { TestBed } from '@angular/core/testing'; +import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; +import { provideHttpClient } from '@angular/common/http'; import { FeedbackService } from 'app/exercises/shared/feedback/feedback.service'; import { Feedback } from 'app/entities/feedback.model'; describe('FeedbackService', () => { let service: FeedbackService; + let httpMock: HttpTestingController; beforeEach(() => { - service = new FeedbackService(); + TestBed.configureTestingModule({ + providers: [FeedbackService, provideHttpClient(), provideHttpClientTesting()], + }); + + service = TestBed.inject(FeedbackService); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => { + httpMock.verify(); }); it('should filter feedbacks by test ids', () => { @@ -18,4 +31,18 @@ describe('FeedbackService', () => { expect(service.filterFeedback(feedbacks, [25, 26])).toEqual(includedFeedbacks); }); + + it('should get long feedback text from server', async () => { + const resultId = 1; + const feedbackId = 42; + const expectedResponse = 'This is a long feedback text.'; + const promise = service.getLongFeedbackText(resultId, feedbackId); + + const req = httpMock.expectOne(`api/results/${resultId}/feedbacks/${feedbackId}/long-feedback`); + expect(req.request.method).toBe('GET'); + + req.flush(expectedResponse); + const feedbackText = await promise; + expect(feedbackText).toBe(expectedResponse); + }); }); From 097113fbd5d1d06c544dc2ae11ffe6fd57970b16 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Thu, 24 Oct 2024 23:06:05 +0200 Subject: [PATCH 11/19] small fix --- .../text/assess/text-submission-assessment.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html b/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html index 27f10b3c1e4a..381cc03053c5 100644 --- a/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html +++ b/src/main/webapp/app/exercises/text/assess/text-submission-assessment.component.html @@ -111,7 +111,7 @@ (feedbacksChange)="validateFeedback()" [readOnly]="readOnly" [useDefaultFeedbackSuggestionBadgeText]="true" - [resultId]="resultId" + [resultId]="result.id" /> }

From fe52b84874f3501976ff2a176fa90e1190aeb4de Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Fri, 25 Oct 2024 01:01:52 +0200 Subject: [PATCH 12/19] server test fix --- .../artemis/assessment/service/ResultService.java | 3 +++ .../ProgrammingAssessmentIntegrationTest.java | 13 ++----------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index d889f7a696f1..31744918ff32 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java @@ -644,6 +644,9 @@ public long getMaxCountForExercise(long exerciseId) { * Only feedback items that have long feedback texts and a non-null ID will be processed. */ public void deleteLongFeedback(List feedbackList) { + if (feedbackList == null) { + return; + } for (Feedback feedback : feedbackList) { if (feedback.getHasLongFeedbackText() && feedback.getId() != null) { Optional longFeedbackTextOpt = longFeedbackTextRepository.findByFeedbackId(feedback.getId()); diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java index 137bbab18fb0..97663444f6d7 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java @@ -30,7 +30,6 @@ import de.tum.cit.aet.artemis.assessment.domain.ComplaintResponse; import de.tum.cit.aet.artemis.assessment.domain.Feedback; import de.tum.cit.aet.artemis.assessment.domain.FeedbackType; -import de.tum.cit.aet.artemis.assessment.domain.LongFeedbackText; import de.tum.cit.aet.artemis.assessment.domain.Result; import de.tum.cit.aet.artemis.assessment.dto.AssessmentUpdateDTO; import de.tum.cit.aet.artemis.core.config.Constants; @@ -546,16 +545,12 @@ void updateManualProgrammingExerciseResult_addFeedbackAfterManualLongFeedback() assertThat(savedAutomaticLongFeedback).isNotNull(); - // Retrieve long feedback text with id. - String longFeedbackText = request.get(String.format("/api/results/%d/feedbacks/%d/long-feedback", response.getId(), savedAutomaticLongFeedback.getId()), HttpStatus.OK, - String.class); - assertThat(response.getScore()).isEqualTo(2); assertThat(response.getFeedbacks()).anySatisfy(feedback -> { assertThat(feedback.getHasLongFeedbackText()).isTrue(); assertThat(feedback.getType()).isEqualTo(FeedbackType.MANUAL_UNREFERENCED); }); - assertThat(longFeedbackText).isEqualTo(manualLongFeedback.getLongFeedback().map(LongFeedbackText::getText).orElse("")); + assertThat(savedAutomaticLongFeedback.getDetailText()).isEqualTo(manualLongFeedback.getDetailText()); } @Test @@ -583,16 +578,12 @@ void updateManualProgrammingExerciseResult_addFeedbackAfterAutomaticLongFeedback assertThat(savedAutomaticLongFeedback).isNotNull(); - // Retrieve long feedback text with id. - String longFeedbackText = request.get(String.format("/api/results/%d/feedbacks/%d/long-feedback", response.getId(), savedAutomaticLongFeedback.getId()), HttpStatus.OK, - String.class); - assertThat(response.getScore()).isEqualTo(2); assertThat(response.getFeedbacks()).anySatisfy(feedback -> { assertThat(feedback.getType()).isEqualTo(FeedbackType.AUTOMATIC); assertThat(feedback.getHasLongFeedbackText()).isTrue(); }); - assertThat(longFeedbackText).isEqualTo(automaticLongFeedback.getLongFeedback().map(LongFeedbackText::getText).orElse("")); + assertThat(savedAutomaticLongFeedback.getDetailText()).isEqualTo(automaticLongFeedback.getDetailText()); } private Result setUpManualResultForUpdate(List feedbacks) { From 4fd4d493ceaefec1edb93a4eba84870121abce16 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Sat, 26 Oct 2024 17:40:58 +0200 Subject: [PATCH 13/19] johannes florian feedback done --- .../artemis/assessment/service/AssessmentService.java | 4 ++-- .../aet/artemis/assessment/service/ResultService.java | 10 +++++----- .../service/ProgrammingAssessmentService.java | 7 ++----- .../unreferenced-feedback-detail.component.ts | 6 +++--- .../example-modeling-submission.component.html | 2 +- .../app/exercises/shared/feedback/feedback.service.ts | 4 ++-- .../unreferenced-feedback.component.html | 4 ++-- .../unreferenced-feedback-detail.component.spec.ts | 8 ++++++++ 8 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java index cb0d52351420..6b7b45c8c65b 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java @@ -287,8 +287,8 @@ public Result saveManualAssessment(final Submission submission, final List feedbacks = new ArrayList<>(result.getFeedbacks()); - result.updateAllFeedbackItems(feedbacks, true); + // long feedback text is deleted as it otherwise causes duplicate entries errors and will be saved again with {@link resultRepository.save} + deleteLongFeedback(result.getFeedbacks(), result); // Note: This also saves the feedback objects in the database because of the 'cascade = CascadeType.ALL' option. return resultRepository.save(result); } @@ -643,10 +641,12 @@ public long getMaxCountForExercise(long exerciseId) { * @param feedbackList The list of {@link Feedback} objects for which the long feedback texts are to be deleted. * Only feedback items that have long feedback texts and a non-null ID will be processed. */ - public void deleteLongFeedback(List feedbackList) { + public void deleteLongFeedback(List feedbackList, Result result) { if (feedbackList == null) { return; } + List feedbacks = new ArrayList<>(feedbackList); + result.updateAllFeedbackItems(feedbacks, true); for (Feedback feedback : feedbackList) { if (feedback.getHasLongFeedbackText() && feedback.getId() != null) { Optional longFeedbackTextOpt = longFeedbackTextRepository.findByFeedbackId(feedback.getId()); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java index d1350e334c2d..03ec7f527429 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java @@ -3,7 +3,6 @@ import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE; import java.time.ZonedDateTime; -import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -91,10 +90,8 @@ private Result saveManualAssessment(Result result, User assessor) { * @return the new saved result */ public Result saveAndSubmitManualAssessment(StudentParticipation participation, Result newManualResult, Result existingManualResult, User assessor, boolean submit) { - // we do this to avoid problems of editing longFeedbackTexts in manual Assesements - resultService.deleteLongFeedback(newManualResult.getFeedbacks()); - List feedbacks = new ArrayList<>(newManualResult.getFeedbacks()); - newManualResult.updateAllFeedbackItems(feedbacks, true); + // long feedback text is deleted as it otherwise causes duplicate entries errors and will be saved again with {@link resultRepository.save} + resultService.deleteLongFeedback(newManualResult.getFeedbacks(), newManualResult); // make sure that the submission cannot be manipulated on the client side var submission = (ProgrammingSubmission) existingManualResult.getSubmission(); ProgrammingExercise exercise = (ProgrammingExercise) participation.getExercise(); diff --git a/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts b/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts index a7b25c900022..88ee35c52edc 100644 --- a/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts +++ b/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts @@ -1,4 +1,4 @@ -import { Component, EventEmitter, Input, OnInit, Output, inject } from '@angular/core'; +import { Component, EventEmitter, Input, OnInit, Output, inject, input } from '@angular/core'; import { faCheck, faExclamation, faExclamationTriangle, faQuestionCircle, faTrash, faTrashAlt } from '@fortawesome/free-solid-svg-icons'; import { Feedback, FeedbackType } from 'app/entities/feedback.model'; import { StructuredGradingCriterionService } from 'app/exercises/shared/structured-grading-criterion/structured-grading-criterion.service'; @@ -13,7 +13,7 @@ import { FeedbackService } from 'app/exercises/shared/feedback/feedback.service' }) export class UnreferencedFeedbackDetailComponent implements OnInit { @Input() public feedback: Feedback; - @Input() resultId: number; + resultId = input.required; @Input() isSuggestion: boolean; @Input() public readOnly: boolean; @Input() highlightDifferences: boolean; @@ -51,7 +51,7 @@ export class UnreferencedFeedbackDetailComponent implements OnInit { */ public async loadLongFeedback() { if (this.feedback.hasLongFeedbackText) { - this.feedback.detailText = await this.feedbackService.getLongFeedbackText(this.resultId, this.feedback.id!); + this.feedback.detailText = await this.feedbackService.getLongFeedbackText(this.resultId(), this.feedback.id!); this.onFeedbackChange.emit(this.feedback); } } diff --git a/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html b/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html index 735c38e092f4..8f0ff9e7f5dd 100644 --- a/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html +++ b/src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.component.html @@ -189,7 +189,7 @@

[readOnly]="readOnly" [addReferenceIdForExampleSubmission]="true" (feedbacksChange)="onUnReferencedFeedbackChanged($event)" - [resultId]="result.id!" + [resultId]="result.id" /> } } diff --git a/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts b/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts index 37c8f777afec..7ae359b1666c 100644 --- a/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts +++ b/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@angular/core'; +import { Injectable, InputSignal } from '@angular/core'; import { Feedback } from 'app/entities/feedback.model'; import { BaseApiHttpService } from 'app/course/learning-paths/services/base-api-http.service'; @@ -17,7 +17,7 @@ export class FeedbackService extends BaseApiHttpService { return feedbacks.filter((feedback) => feedback.testCase?.id && filter.includes(feedback.testCase.id)); }; - public async getLongFeedbackText(resultId: number, feedbackId: number): Promise { + public async getLongFeedbackText(resultId: InputSignal, feedbackId: number): Promise { const url = `results/${resultId}/feedbacks/${feedbackId}/long-feedback`; return await this.get(url, { responseType: 'text' }); } diff --git a/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.html b/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.html index 1091dc0c8c7d..021c4d88fce9 100644 --- a/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.html +++ b/src/main/webapp/app/exercises/shared/unreferenced-feedback/unreferenced-feedback.component.html @@ -19,12 +19,12 @@

} @@ -32,12 +32,12 @@

} diff --git a/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts b/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts index 8f8d667ac3fa..a786e514be7d 100644 --- a/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts +++ b/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts @@ -47,6 +47,14 @@ describe('Unreferenced Feedback Detail Component', () => { }); }); + it('should call loadLongFeedback on init if feedback has long text', () => { + comp.feedback = { id: 42, hasLongFeedbackText: true } as Feedback; + const loadLongFeedbackSpy = jest.spyOn(comp, 'loadLongFeedback'); + comp.ngOnInit(); + fixture.detectChanges(); + expect(loadLongFeedbackSpy).toHaveBeenCalledOnce(); + }); + it('should update feedback with SGI and emit to parent', () => { const instruction: GradingInstruction = { id: 1, credits: 2, feedback: 'test', gradingScale: 'good', instructionDescription: 'description of instruction', usageCount: 0 }; comp.feedback = { From 089f01021a15ae541a636da3804d57313195230b Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Sun, 27 Oct 2024 01:00:01 +0200 Subject: [PATCH 14/19] tests fix --- .../unreferenced-feedback-detail.component.ts | 6 +++--- ...eferenced-feedback-detail.component.spec.ts | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts b/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts index 88ee35c52edc..3b4308b752ab 100644 --- a/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts +++ b/src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts @@ -1,4 +1,4 @@ -import { Component, EventEmitter, Input, OnInit, Output, inject, input } from '@angular/core'; +import { Component, EventEmitter, Input, InputSignal, OnInit, Output, inject, input } from '@angular/core'; import { faCheck, faExclamation, faExclamationTriangle, faQuestionCircle, faTrash, faTrashAlt } from '@fortawesome/free-solid-svg-icons'; import { Feedback, FeedbackType } from 'app/entities/feedback.model'; import { StructuredGradingCriterionService } from 'app/exercises/shared/structured-grading-criterion/structured-grading-criterion.service'; @@ -13,7 +13,7 @@ import { FeedbackService } from 'app/exercises/shared/feedback/feedback.service' }) export class UnreferencedFeedbackDetailComponent implements OnInit { @Input() public feedback: Feedback; - resultId = input.required; + resultId: InputSignal = input.required(); @Input() isSuggestion: boolean; @Input() public readOnly: boolean; @Input() highlightDifferences: boolean; @@ -51,7 +51,7 @@ export class UnreferencedFeedbackDetailComponent implements OnInit { */ public async loadLongFeedback() { if (this.feedback.hasLongFeedbackText) { - this.feedback.detailText = await this.feedbackService.getLongFeedbackText(this.resultId(), this.feedback.id!); + this.feedback.detailText = await this.feedbackService.getLongFeedbackText(this.resultId, this.feedback.id!); this.onFeedbackChange.emit(this.feedback); } } diff --git a/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts b/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts index a786e514be7d..a54667f4824a 100644 --- a/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts +++ b/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts @@ -20,6 +20,7 @@ import { FeedbackService } from 'app/exercises/shared/feedback/feedback.service' describe('Unreferenced Feedback Detail Component', () => { let comp: UnreferencedFeedbackDetailComponent; let fixture: ComponentFixture; + let feedbackService: FeedbackService; let sgiService: StructuredGradingCriterionService; beforeEach(() => { @@ -43,16 +44,21 @@ describe('Unreferenced Feedback Detail Component', () => { .then(() => { fixture = TestBed.createComponent(UnreferencedFeedbackDetailComponent); comp = fixture.componentInstance; - sgiService = fixture.debugElement.injector.get(StructuredGradingCriterionService); + feedbackService = TestBed.inject(FeedbackService); }); }); - it('should call loadLongFeedback on init if feedback has long text', () => { - comp.feedback = { id: 42, hasLongFeedbackText: true } as Feedback; - const loadLongFeedbackSpy = jest.spyOn(comp, 'loadLongFeedback'); + it('should call getLongFeedbackText on init if feedback has long text', async () => { + const feedbackId = 42; + const resultId = 1; + const exampleText = 'This is a long feedback text'; + + comp.feedback = { id: feedbackId, hasLongFeedbackText: true } as Feedback; + fixture.componentRef.setInput('resultId', resultId); + const getLongFeedbackTextSpy = jest.spyOn(feedbackService, 'getLongFeedbackText').mockResolvedValue(exampleText); + comp.ngOnInit(); - fixture.detectChanges(); - expect(loadLongFeedbackSpy).toHaveBeenCalledOnce(); + expect(getLongFeedbackTextSpy).toHaveBeenCalledExactlyOnceWith(fixture.componentInstance.resultId, feedbackId); }); it('should update feedback with SGI and emit to parent', () => { From 2544be4107cc5b1f262403eef8e26e5d26a65811 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Sun, 27 Oct 2024 12:42:52 +0100 Subject: [PATCH 15/19] tests fix --- .../assessment/service/ResultService.java | 30 +++++++++++-------- .../shared/feedback/feedback.service.ts | 7 +++-- .../service/feedback/feedback-service.spec.ts | 6 ++-- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index 8cca7269a31a..a129d1483f57 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java @@ -630,28 +630,32 @@ public long getMaxCountForExercise(long exerciseId) { } /** - * Deletes long feedback texts for the provided list of feedback items. + * Deletes long feedback texts for the provided list of feedback items to prevent duplicate entries in the {@link LongFeedbackTextRepository}. *
- * This method iterates over the provided list of feedback and checks if each feedback has an associated long feedback text. - * If the feedback has a long feedback text and its ID is not null, the method fetches the corresponding {@link LongFeedbackText} - * from the repository and deletes it. + * This method processes the provided list of feedback items, identifies those with associated long feedback texts, and removes them in bulk + * from the repository to avoid potential duplicate entry errors when saving new feedback entries. *

- * This is useful in cases where long feedback texts need to be removed, such as during feedback cleanup operations. + * Primarily used to ensure data consistency in the {@link LongFeedbackTextRepository}, especially during operations where feedback entries are + * overridden or updated. The deletion is performed only for feedback items with a non-null ID and an associated long feedback text. + *

+ * This approach reduces the need for individual deletion calls and performs batch deletion in a single database operation. * - * @param feedbackList The list of {@link Feedback} objects for which the long feedback texts are to be deleted. - * Only feedback items that have long feedback texts and a non-null ID will be processed. + * @param feedbackList The list of {@link Feedback} objects for which the long feedback texts are to be deleted. Only feedback items that have long feedback texts and a + * non-null ID will be processed. + * @param result The {@link Result} object associated with the feedback items, used to update feedback list before processing. */ public void deleteLongFeedback(List feedbackList, Result result) { if (feedbackList == null) { return; } + + List feedbackIdsWithLongText = feedbackList.stream().filter(feedback -> feedback.getHasLongFeedbackText() && feedback.getId() != null).map(Feedback::getId) + .collect(Collectors.toList()); + + List longFeedbackTextsToDelete = longFeedbackTextRepository.findByFeedbackIds(feedbackIdsWithLongText); + longFeedbackTextRepository.deleteAll(longFeedbackTextsToDelete); + List feedbacks = new ArrayList<>(feedbackList); result.updateAllFeedbackItems(feedbacks, true); - for (Feedback feedback : feedbackList) { - if (feedback.getHasLongFeedbackText() && feedback.getId() != null) { - Optional longFeedbackTextOpt = longFeedbackTextRepository.findByFeedbackId(feedback.getId()); - longFeedbackTextOpt.ifPresent(longFeedbackTextRepository::delete); - } - } } } diff --git a/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts b/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts index 7ae359b1666c..0a73480b8163 100644 --- a/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts +++ b/src/main/webapp/app/exercises/shared/feedback/feedback.service.ts @@ -1,4 +1,4 @@ -import { Injectable, InputSignal } from '@angular/core'; +import { Injectable, Signal } from '@angular/core'; import { Feedback } from 'app/entities/feedback.model'; import { BaseApiHttpService } from 'app/course/learning-paths/services/base-api-http.service'; @@ -17,8 +17,9 @@ export class FeedbackService extends BaseApiHttpService { return feedbacks.filter((feedback) => feedback.testCase?.id && filter.includes(feedback.testCase.id)); }; - public async getLongFeedbackText(resultId: InputSignal, feedbackId: number): Promise { - const url = `results/${resultId}/feedbacks/${feedbackId}/long-feedback`; + public async getLongFeedbackText(resultId: Signal, feedbackId: number): Promise { + const resultIdValue = resultId(); + const url = `results/${resultIdValue}/feedbacks/${feedbackId}/long-feedback`; return await this.get(url, { responseType: 'text' }); } } diff --git a/src/test/javascript/spec/service/feedback/feedback-service.spec.ts b/src/test/javascript/spec/service/feedback/feedback-service.spec.ts index 1c5071c65ee5..10e19cd70630 100644 --- a/src/test/javascript/spec/service/feedback/feedback-service.spec.ts +++ b/src/test/javascript/spec/service/feedback/feedback-service.spec.ts @@ -3,6 +3,7 @@ import { HttpTestingController, provideHttpClientTesting } from '@angular/common import { provideHttpClient } from '@angular/common/http'; import { FeedbackService } from 'app/exercises/shared/feedback/feedback.service'; import { Feedback } from 'app/entities/feedback.model'; +import { createSignal } from '@angular/core/primitives/signals'; describe('FeedbackService', () => { let service: FeedbackService; @@ -33,10 +34,11 @@ describe('FeedbackService', () => { }); it('should get long feedback text from server', async () => { - const resultId = 1; const feedbackId = 42; + const resultId = 1; + const resultIdSignal = createSignal(resultId); const expectedResponse = 'This is a long feedback text.'; - const promise = service.getLongFeedbackText(resultId, feedbackId); + const promise = service.getLongFeedbackText(resultIdSignal, feedbackId); const req = httpMock.expectOne(`api/results/${resultId}/feedbacks/${feedbackId}/long-feedback`); expect(req.request.method).toBe('GET'); From 8a2a656af397c2005714fd656fbf1f0aa7e82fad Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Sun, 27 Oct 2024 13:01:33 +0100 Subject: [PATCH 16/19] johannes feedback --- .../repository/LongFeedbackTextRepository.java | 10 ++++++++++ .../aet/artemis/assessment/service/ResultService.java | 6 ++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java b/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java index 6ad61c4ef7ff..200a06757080 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java @@ -3,8 +3,10 @@ import java.util.List; import java.util.Optional; +import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; +import org.springframework.transaction.annotation.Transactional; import de.tum.cit.aet.artemis.assessment.domain.LongFeedbackText; import de.tum.cit.aet.artemis.core.repository.base.ArtemisJpaRepository; @@ -36,6 +38,14 @@ public interface LongFeedbackTextRepository extends ArtemisJpaRepository findWithFeedbackAndResultAndParticipationByFeedbackId(@Param("feedbackId") final Long feedbackId); + @Modifying + @Transactional + @Query(""" + DELETE FROM LongFeedbackText longFeedback + WHERE longFeedback.feedback.id IN :feedbackIds + """) + void deleteByFeedbackIds(@Param("feedbackIds") List feedbackIds); + default LongFeedbackText findByFeedbackIdWithFeedbackAndResultAndParticipationElseThrow(final Long feedbackId) { return getValueElseThrow(findWithFeedbackAndResultAndParticipationByFeedbackId(feedbackId), feedbackId); } diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index a129d1483f57..d04404ddfab0 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java @@ -649,11 +649,9 @@ public void deleteLongFeedback(List feedbackList, Result result) { return; } - List feedbackIdsWithLongText = feedbackList.stream().filter(feedback -> feedback.getHasLongFeedbackText() && feedback.getId() != null).map(Feedback::getId) - .collect(Collectors.toList()); + List feedbackIdsWithLongText = feedbackList.stream().filter(feedback -> feedback.getHasLongFeedbackText() && feedback.getId() != null).map(Feedback::getId).toList(); - List longFeedbackTextsToDelete = longFeedbackTextRepository.findByFeedbackIds(feedbackIdsWithLongText); - longFeedbackTextRepository.deleteAll(longFeedbackTextsToDelete); + longFeedbackTextRepository.deleteByFeedbackIds(feedbackIdsWithLongText); List feedbacks = new ArrayList<>(feedbackList); result.updateAllFeedbackItems(feedbacks, true); From 27693f154d0c166bf1a53c937d93b47ff34c7eef Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Sun, 27 Oct 2024 13:12:34 +0100 Subject: [PATCH 17/19] johannes feedback --- .../assessment/repository/LongFeedbackTextRepository.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java b/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java index 200a06757080..90dbc49b3e5b 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java @@ -6,11 +6,13 @@ import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; +import org.springframework.stereotype.Repository; import org.springframework.transaction.annotation.Transactional; import de.tum.cit.aet.artemis.assessment.domain.LongFeedbackText; import de.tum.cit.aet.artemis.core.repository.base.ArtemisJpaRepository; +@Repository public interface LongFeedbackTextRepository extends ArtemisJpaRepository { @Query(""" From 8dde96c4bfec1828476ec5113b47070ac8c1a3d4 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Sun, 27 Oct 2024 13:22:35 +0100 Subject: [PATCH 18/19] johannes feedback --- .../assessment/repository/LongFeedbackTextRepository.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java b/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java index 90dbc49b3e5b..4e0c502a75db 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java @@ -1,8 +1,11 @@ package de.tum.cit.aet.artemis.assessment.repository; +import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE; + import java.util.List; import java.util.Optional; +import org.springframework.context.annotation.Profile; import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; @@ -12,6 +15,7 @@ import de.tum.cit.aet.artemis.assessment.domain.LongFeedbackText; import de.tum.cit.aet.artemis.core.repository.base.ArtemisJpaRepository; +@Profile(PROFILE_CORE) @Repository public interface LongFeedbackTextRepository extends ArtemisJpaRepository { From 349f61d77250deb626c76e48e37d4c9ce338dc78 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Sun, 27 Oct 2024 14:09:10 +0100 Subject: [PATCH 19/19] server test --- .../cit/aet/artemis/assessment/service/ResultService.java | 2 +- .../unreferenced-feedback-detail.component.spec.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index d04404ddfab0..14b4b02c259b 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java @@ -654,6 +654,6 @@ public void deleteLongFeedback(List feedbackList, Result result) { longFeedbackTextRepository.deleteByFeedbackIds(feedbackIdsWithLongText); List feedbacks = new ArrayList<>(feedbackList); - result.updateAllFeedbackItems(feedbacks, true); + result.updateAllFeedbackItems(feedbacks, false); } } diff --git a/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts b/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts index a54667f4824a..284a2a8dbd50 100644 --- a/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts +++ b/src/test/javascript/spec/component/assessment-shared/unreferenced-feedback-detail.component.spec.ts @@ -45,6 +45,7 @@ describe('Unreferenced Feedback Detail Component', () => { fixture = TestBed.createComponent(UnreferencedFeedbackDetailComponent); comp = fixture.componentInstance; feedbackService = TestBed.inject(FeedbackService); + sgiService = TestBed.inject(StructuredGradingCriterionService); // Add this line to inject sgiService }); }); @@ -58,7 +59,7 @@ describe('Unreferenced Feedback Detail Component', () => { const getLongFeedbackTextSpy = jest.spyOn(feedbackService, 'getLongFeedbackText').mockResolvedValue(exampleText); comp.ngOnInit(); - expect(getLongFeedbackTextSpy).toHaveBeenCalledExactlyOnceWith(fixture.componentInstance.resultId, feedbackId); + expect(getLongFeedbackTextSpy).toHaveBeenCalledWith(fixture.componentInstance.resultId, feedbackId); }); it('should update feedback with SGI and emit to parent', () => { @@ -68,12 +69,12 @@ describe('Unreferenced Feedback Detail Component', () => { detailText: 'feedback1', credits: 1.5, } as Feedback; - // Fake call as a DragEvent + jest.spyOn(sgiService, 'updateFeedbackWithStructuredGradingInstructionEvent').mockImplementation(() => { comp.feedback.gradingInstruction = instruction; comp.feedback.credits = instruction.credits; }); - // Call spy function with empty event + comp.updateFeedbackOnDrop(new Event('')); expect(comp.feedback.gradingInstruction).toBe(instruction);