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 87df115afc0d..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 @@ -6,9 +6,11 @@ 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; 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; @@ -42,6 +44,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/AssessmentService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java index 1a451ad49aff..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 @@ -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; } /** @@ -283,6 +287,9 @@ public Result saveManualAssessment(final Submission submission, final List 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 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())); - // 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) { + // 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); } @@ -623,4 +628,32 @@ 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 to prevent duplicate entries in the {@link LongFeedbackTextRepository}. + *
+ * 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. + *

+ * 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 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).toList(); + + longFeedbackTextRepository.deleteByFeedbackIds(feedbackIdsWithLongText); + + List feedbacks = new ArrayList<>(feedbackList); + result.updateAllFeedbackItems(feedbacks, false); + } } 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..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 @@ -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; @@ -48,9 +49,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 +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) { + // 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/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; } 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..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,17 +1,19 @@ -import { Component, EventEmitter, Input, Output } 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'; 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; + resultId: InputSignal = input.required(); @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 acabef44c6b6..e93232471a3e 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,12 +49,15 @@ @if (invalidError) {

} - + @if (result && result.id) { + + } } @else { @if (!loadingInitialSubmission) { @@ -96,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 2bdfb5600177..e7152a1b94af 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 && result.id) { + + } @if ((hasAutomaticFeedback || highlightMissingFeedback) && !result?.completionDate) {

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

}

- + @if (result && 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..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 @@ -183,12 +183,15 @@

} @if (assessmentMode) { - + @if (result && result.id) { + + } }

@if (exercise) { 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 a36543eca9a3..4d6adec82308 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 && 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..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,8 +1,9 @@ -import { Injectable } 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'; @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,10 @@ export class FeedbackService { } return feedbacks.filter((feedback) => feedback.testCase?.id && filter.includes(feedback.testCase.id)); }; + + 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/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..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,6 +19,7 @@

- + @if (result && result.id) { + + }
} @else { @if (!loadingInitialSubmission) { @@ -101,12 +104,15 @@
- + @if (result && result.id) { + + }
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..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,7 +159,15 @@
+ @if (result && result.id) { + + } } @if (toComplete) {
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) { 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..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 @@ -15,10 +15,12 @@ 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; let fixture: ComponentFixture; + let feedbackService: FeedbackService; let sgiService: StructuredGradingCriterionService; beforeEach(() => { @@ -36,16 +38,30 @@ 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(() => { fixture = TestBed.createComponent(UnreferencedFeedbackDetailComponent); comp = fixture.componentInstance; - sgiService = fixture.debugElement.injector.get(StructuredGradingCriterionService); + feedbackService = TestBed.inject(FeedbackService); + sgiService = TestBed.inject(StructuredGradingCriterionService); // Add this line to inject sgiService }); }); + 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(); + expect(getLongFeedbackTextSpy).toHaveBeenCalledWith(fixture.componentInstance.resultId, feedbackId); + }); + 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 = { @@ -53,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); 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..10e19cd70630 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,25 @@ +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'; +import { createSignal } from '@angular/core/primitives/signals'; 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 +32,19 @@ describe('FeedbackService', () => { expect(service.filterFeedback(feedbacks, [25, 26])).toEqual(includedFeedbacks); }); + + it('should get long feedback text from server', async () => { + const feedbackId = 42; + const resultId = 1; + const resultIdSignal = createSignal(resultId); + const expectedResponse = 'This is a long feedback text.'; + const promise = service.getLongFeedbackText(resultIdSignal, 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); + }); });