Programming exercises: Fix an issue in which long manual feedback is not correctly displayed

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> 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<Feedb
         User user = userRepository.getUser();
         result.setAssessor(user);
 
+        // long feedback text is deleted as it otherwise causes duplicate entries errors and will be saved again with {@link resultRepository.save}
+        resultService.deleteLongFeedback(feedbackList, result);
+
         result.updateAllFeedbackItems(feedbackList, false);
         result.determineAssessmentType();
 

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;
@@ -494,45 +495,49 @@ public Map<Long, String> getLogsAvailabilityForResults(List<Result> results, Par
 
     @NotNull
     private List<Feedback> saveFeedbackWithHibernateWorkaround(@NotNull Result result, List<Feedback> feedbackList) {
-        // Avoid hibernate exception
         List<Feedback> savedFeedbacks = new ArrayList<>();
-        // Collect ids of feedbacks that have long feedback.
-        List<Long> feedbackIdsWithLongFeedback = feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId)
-                .toList();
-        // Get long feedback list from the database.
-        List<LongFeedbackText> longFeedbackTextList = longFeedbackTextRepository.findByFeedbackIds(feedbackIdsWithLongFeedback);
 
-        // Convert list to map for accessing later.
-        Map<Long, LongFeedbackText> 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<Long, LongFeedbackText> 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<Long, LongFeedbackText> 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,30 @@ 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.
+     * <br>
+     * 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.
+     * <p>
+     * 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<Feedback> feedbackList, Result result) {
+        if (feedbackList == null) {
+            return;
+        }
+        List<Feedback> feedbacks = new ArrayList<>(feedbackList);
+        result.updateAllFeedbackItems(feedbacks, true);
+        for (Feedback feedback : feedbackList) {
+            if (feedback.getHasLongFeedbackText() && feedback.getId() != null) {
+                Optional<LongFeedbackText> longFeedbackTextOpt = longFeedbackTextRepository.findByFeedbackId(feedback.getId());
+                longFeedbackTextOpt.ifPresent(longFeedbackTextRepository::delete);
+            }
+        }
+    }
 }

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> ltiNewResultService, SingleUserNotificationService singleUserNotificationService, ResultWebsocketService resultWebsocketService,
-            ProgrammingExerciseParticipationService programmingExerciseParticipationService, Optional<AthenaFeedbackSendingService> athenaFeedbackSendingService) {
+            ProgrammingExerciseParticipationService programmingExerciseParticipationService, Optional<AthenaFeedbackSendingService> 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();

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> 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;
     }
 

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<number> = input.required<number>();
     @Input() isSuggestion: boolean;
     @Input() public readOnly: boolean;
     @Input() highlightDifferences: boolean;
@@ -21,6 +23,7 @@ export class UnreferencedFeedbackDetailComponent {
     @Output() public onFeedbackDelete = new EventEmitter<Feedback>();
     @Output() onAcceptSuggestion = new EventEmitter<Feedback>();
     @Output() onDiscardSuggestion = new EventEmitter<Feedback>();
+    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
      */

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<string | number | boolean>;
                   };
-            responseType?: 'json';
+            responseType?: 'json' | 'text';
         },
     ): Promise<T> {
         try {
             const response = await lastValueFrom(
-                this.httpClient.request<T>(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<string | number | boolean>;
                   };
+            responseType?: 'json' | 'text';
         },
     ): Promise<T> {
         return await this.request<T>(HttpMethod.Get, url, options);
@@ -139,6 +141,7 @@ export abstract class BaseApiHttpService {
                 | {
                       [param: string]: string | number | boolean | ReadonlyArray<string | number | boolean>;
                   };
+            responseType?: 'json' | 'text';
         },
     ): Promise<T> {
         return await this.request<T>(HttpMethod.Post, url, { body: body, ...options });
@@ -168,6 +171,7 @@ export abstract class BaseApiHttpService {
                 | {
                       [param: string]: string | number | boolean | ReadonlyArray<string | number | boolean>;
                   };
+            responseType?: 'json' | 'text';
         },
     ): Promise<T> {
         return await this.request<T>(HttpMethod.Delete, url, options);
@@ -198,6 +202,7 @@ export abstract class BaseApiHttpService {
                 | {
                       [param: string]: string | number | boolean | ReadonlyArray<string | number | boolean>;
                   };
+            responseType?: 'json' | 'text';
         },
     ): Promise<T> {
         return await this.request<T>(HttpMethod.Patch, url, { body: body, ...options });
@@ -228,6 +233,7 @@ export abstract class BaseApiHttpService {
                 | {
                       [param: string]: string | number | boolean | ReadonlyArray<string | number | boolean>;
                   };
+            responseType?: 'json' | 'text';
         },
     ): Promise<T> {
         return await this.request<T>(HttpMethod.Put, url, { body: body, ...options });