Programming exercises: Improve usability in the feedback analysis table

src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.java

@@ -6,11 +6,14 @@
 import com.fasterxml.jackson.annotation.JsonInclude;
 
 @JsonInclude(JsonInclude.Include.NON_EMPTY)
-public record FeedbackDetailDTO(List<Long> feedbackIds, long count, double relativeCount, List<String> detailTexts, String testCaseName, String taskName, String errorCategory) {
+public record FeedbackDetailDTO(List<Long> feedbackIds, long count, double relativeCount, List<String> detailTexts, String testCaseName, String taskName, String errorCategory,
+        boolean hasLongFeedbackText) {
 
-    public FeedbackDetailDTO(String feedbackId, long count, double relativeCount, String detailText, String testCaseName, String taskName, String errorCategory) {
+    public FeedbackDetailDTO(String feedbackId, long count, double relativeCount, String detailText, String testCaseName, String taskName, String errorCategory,
+            boolean hasLongFeedbackText) {
         // Feedback IDs are gathered in the query using a comma separator, and the detail texts are stored in a list because, in case aggregation is applied, the detail texts are
         // grouped together
-        this(Arrays.stream(feedbackId.split(",")).map(Long::valueOf).toList(), count, relativeCount, List.of(detailText), testCaseName, taskName, errorCategory);
+        this(Arrays.stream(feedbackId.split(",")).map(Long::valueOf).toList(), count, relativeCount, List.of(detailText), testCaseName, taskName, errorCategory,
+                hasLongFeedbackText);
     }
 }

src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java

@@ -39,7 +39,7 @@ public interface LongFeedbackTextRepository extends ArtemisJpaRepository<LongFee
             FROM LongFeedbackText longFeedback
                 LEFT JOIN FETCH longFeedback.feedback feedback
                 LEFT JOIN FETCH feedback.result result
-                LEFT JOIN FETCH result.participation
+                LEFT JOIN FETCH result.submission
             WHERE longFeedback.feedback.id = :feedbackId
             """)
     Optional<LongFeedbackText> findWithFeedbackAndResultAndParticipationByFeedbackId(@Param("feedbackId") final Long feedbackId);

src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java

@@ -638,7 +638,8 @@ public FeedbackAnalysisResponseDTO getFeedbackDetailsOnPage(long exerciseId, Fee
             // Process and map feedback details, calculating relative count and assigning task names
             processedDetails = feedbackDetailPage.getContent().stream()
                     .map(detail -> new FeedbackDetailDTO(detail.feedbackIds().subList(0, Math.min(detail.feedbackIds().size(), MAX_FEEDBACK_IDS)), detail.count(),
-                            (detail.count() * 100.00) / distinctResultCount, detail.detailTexts(), detail.testCaseName(), detail.taskName(), detail.errorCategory()))
+                            (detail.count() * 100.00) / distinctResultCount, detail.detailTexts(), detail.testCaseName(), detail.taskName(), detail.errorCategory(),
+                            detail.hasLongFeedbackText()))
                     .toList();
             totalPages = feedbackDetailPage.getTotalPages();
             totalCount = feedbackDetailPage.getTotalElements();
@@ -661,8 +662,10 @@ public FeedbackAnalysisResponseDTO getFeedbackDetailsOnPage(long exerciseId, Fee
             int start = Math.max(0, (page - 1) * pageSize);
             int end = Math.min(start + pageSize, processedDetailsPreSort.size());
             processedDetails = processedDetailsPreSort.subList(start, end);
-            processedDetails = processedDetails.stream().map(detail -> new FeedbackDetailDTO(detail.feedbackIds().subList(0, Math.min(detail.feedbackIds().size(), 5)),
-                    detail.count(), (detail.count() * 100.00) / distinctResultCount, detail.detailTexts(), detail.testCaseName(), detail.taskName(), detail.errorCategory()))
+            processedDetails = processedDetails.stream()
+                    .map(detail -> new FeedbackDetailDTO(detail.feedbackIds().subList(0, Math.min(detail.feedbackIds().size(), 5)), detail.count(),
+                            (detail.count() * 100.00) / distinctResultCount, detail.detailTexts(), detail.testCaseName(), detail.taskName(), detail.errorCategory(),
+                            detail.hasLongFeedbackText()))
                     .toList();
             totalPages = (int) Math.ceil((double) processedDetailsPreSort.size() / pageSize);
             totalCount = aggregatedFeedbackDetails.size();
@@ -713,7 +716,7 @@ private List<FeedbackDetailDTO> aggregateFeedback(List<FeedbackDetailDTO> feedba
                         // Replace the processed entry with the updated one
                         processedDetails.remove(processed);
                         FeedbackDetailDTO updatedProcessed = new FeedbackDetailDTO(mergedFeedbackIds, mergedCount, 0, mergedTexts, processed.testCaseName(), processed.taskName(),
-                                processed.errorCategory());
+                                processed.errorCategory(), processed.hasLongFeedbackText());
                         processedDetails.add(updatedProcessed); // Add the updated entry
                         isMerged = true;
                         break; // No need to check further
@@ -724,7 +727,7 @@ private List<FeedbackDetailDTO> aggregateFeedback(List<FeedbackDetailDTO> feedba
             if (!isMerged) {
                 // If not merged, add it as a new entry in processedDetails
                 FeedbackDetailDTO newEntry = new FeedbackDetailDTO(base.feedbackIds(), base.count(), 0, List.of(base.detailTexts().getFirst()), base.testCaseName(),
-                        base.taskName(), base.errorCategory());
+                        base.taskName(), base.errorCategory(), base.hasLongFeedbackText());
                 processedDetails.add(newEntry);
             }
         }

src/main/java/de/tum/cit/aet/artemis/assessment/web/LongFeedbackTextResource.java

@@ -15,7 +15,6 @@
 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.repository.LongFeedbackTextRepository;
-import de.tum.cit.aet.artemis.core.exception.BadRequestAlertException;
 import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastStudent;
 import de.tum.cit.aet.artemis.exercise.domain.participation.Participation;
 import de.tum.cit.aet.artemis.exercise.service.ParticipationAuthorizationCheckService;
@@ -39,28 +38,23 @@ public LongFeedbackTextResource(LongFeedbackTextRepository longFeedbackTextRepos
     /**
      * Gets the long feedback associated with the specified feedback.
      *
-     * @param resultId   The result the feedback belongs to.
      * @param feedbackId The feedback for which the long feedback should be fetched.
      * @return The long feedback text belonging to the feedback with id {@code feedbackId}.
      */
-    @GetMapping("results/{resultId}/feedbacks/{feedbackId}/long-feedback")
+    @GetMapping("feedbacks/{feedbackId}/long-feedback")
     @EnforceAtLeastStudent
-    public ResponseEntity<String> getLongFeedback(@PathVariable Long resultId, @PathVariable Long feedbackId) {
-        log.debug("REST request to get long feedback: {} (result: {})", feedbackId, resultId);
+    public ResponseEntity<String> getLongFeedback(@PathVariable Long feedbackId) {
+        log.debug("REST request to get long feedback: {}", feedbackId);
 
         final LongFeedbackText longFeedbackText = longFeedbackTextRepository.findByFeedbackIdWithFeedbackAndResultAndParticipationElseThrow(feedbackId);
-        checkCanAccessResultElseThrow(resultId, longFeedbackText);
+        checkCanAccessResultElseThrow(longFeedbackText);
 
         return ResponseEntity.ok().contentType(MediaType.TEXT_PLAIN).body(longFeedbackText.getText());
     }
 
-    private void checkCanAccessResultElseThrow(final Long resultId, final LongFeedbackText longFeedbackText) {
+    private void checkCanAccessResultElseThrow(final LongFeedbackText longFeedbackText) {
         final Result result = longFeedbackText.getFeedback().getResult();
-        if (!result.getId().equals(resultId)) {
-            throw new BadRequestAlertException("resultId of the path does not correspond to feedbackId", "result", "invalidResultId");
-        }
-
-        final Participation participation = result.getParticipation();
+        final Participation participation = result.getSubmission().getParticipation();
         participationAuthorizationCheckService.checkCanAccessParticipationElseThrow(participation);
     }
 }

src/main/java/de/tum/cit/aet/artemis/communication/domain/conversation/ChannelSubType.java

@@ -13,5 +13,8 @@ public enum ChannelSubType {
     LECTURE,
 
     @JsonProperty("exam")
-    EXAM
+    EXAM,
+
+    @JsonProperty("feedbackDiscussion")
+    FEEDBACK_DISCUSSION
 }

src/main/java/de/tum/cit/aet/artemis/communication/dto/ChannelDTO.java

@@ -192,6 +192,9 @@ else if (channel.getExam() != null) {
             this.subType = ChannelSubType.EXAM;
             this.subTypeReferenceId = channel.getExam().getId();
         }
+        else if (channel.getTopic() != null && channel.getTopic().contains("FeedbackDiscussion")) {
+            this.subType = ChannelSubType.FEEDBACK_DISCUSSION;
+        }
         else {
             this.subType = ChannelSubType.GENERAL;
         }

src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java

@@ -489,7 +489,6 @@ public ResponseEntity<ChannelDTO> createFeedbackChannel(@PathVariable Long cours
         Course course = courseRepository.findByIdElseThrow(courseId);
         checkCommunicationEnabledElseThrow(course);
         Channel createdChannel = channelService.createFeedbackChannel(course, exerciseId, channelDTO, feedbackDetailTexts, testCaseName, requestingUser);
-
         return ResponseEntity.created(new URI("/api/channels/" + createdChannel.getId())).body(conversationDTOService.convertChannelToDTO(requestingUser, createdChannel));
     }
 

src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java

@@ -1261,7 +1261,8 @@ SELECT MAX(t.taskName)
                         WHEN f.detailText LIKE 'ARES Security Error%' THEN 'Ares Error'
                         WHEN f.detailText LIKE 'Unwanted Statement found%' THEN 'AST Error'
                         ELSE 'Student Error'
-                    END
+                    END,
+                    f.hasLongFeedbackText
                 )
                 FROM ProgrammingExerciseStudentParticipation p
                 INNER JOIN p.results r ON r.id = (
@@ -1286,7 +1287,7 @@ WHERE t.taskName IN (:filterTaskNames)
                                 WHEN f.detailText LIKE 'Unwanted Statement found%' THEN 'AST Error'
                                 ELSE 'Student Error'
                             END IN (:filterErrorCategories))
-                GROUP BY f.detailText, f.testCase.testName
+                GROUP BY f.detailText, f.testCase.testName, f.hasLongFeedbackText
                 HAVING COUNT(f.id) BETWEEN :minOccurrence AND :maxOccurrence
             """)
     Page<FeedbackDetailDTO> findFilteredFeedbackByExerciseId(@Param("exerciseId") long exerciseId, @Param("searchTerm") String searchTerm,

src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java

@@ -74,6 +74,7 @@ Optional<ProgrammingExerciseStudentParticipation> findByIdWithAllResultsAndRelat
             """)
     List<ProgrammingExerciseStudentParticipation> findAllWithBuildPlanIdWithResults();
 
+    @EntityGraph(type = LOAD, attributePaths = { "submissions" })
     Optional<ProgrammingExerciseStudentParticipation> findByExerciseIdAndStudentLogin(long exerciseId, String username);
 
     List<ProgrammingExerciseStudentParticipation> findAllByExerciseIdAndStudentLogin(long exerciseId, String username);

src/main/webapp/app/assessment/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts

@@ -75,8 +75,8 @@ export class UnreferencedFeedbackDetailComponent implements OnInit {
      * 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!);
+        if (this.feedback.id && this.feedback.hasLongFeedbackText) {
+            this.feedback.detailText = await this.feedbackService.getLongFeedbackText(this.feedback.id);
             this.onFeedbackChange.emit(this.feedback);
         }
     }

src/main/webapp/app/entities/metis/conversation/channel.model.ts

@@ -10,6 +10,7 @@ export enum ChannelSubType {
     EXERCISE = 'exercise',
     LECTURE = 'lecture',
     EXAM = 'exam',
+    FEEDBACK_DISCUSSION = 'feedbackDiscussion',
 }
 
 /**

src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts

@@ -21,6 +21,7 @@ export interface FeedbackDetail {
     testCaseName: string;
     taskName: string;
     errorCategory: string;
+    hasLongFeedbackText: boolean;
 }
 export interface FeedbackAffectedStudentDTO {
     participationId: number;

src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/modal/feedback-detail-channel-modal.component.html

@@ -48,9 +48,9 @@ <h4 class="modal-title">
         <div class="form-group">
             <label [jhiTranslate]="TRANSLATION_BASE + '.channelVisibility'"></label>
             <div class="btn-group" role="group">
-                <input type="radio" class="btn-check" id="public" formControlName="isPublic" [value]="true" />
+                <input type="radio" class="btn-check" id="public" formControlName="isPrivate" [value]="false" />
                 <label for="public" class="btn btn-outline-secondary" [jhiTranslate]="TRANSLATION_BASE + '.visibilityPublic'"></label>
-                <input type="radio" class="btn-check" id="private" formControlName="isPublic" [value]="false" />
+                <input type="radio" class="btn-check" id="private" formControlName="isPrivate" [value]="true" />
                 <label for="private" class="btn btn-outline-secondary" [jhiTranslate]="TRANSLATION_BASE + '.visibilityPrivate'"></label>
             </div>
         </div>

src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/modal/feedback-detail-channel-modal.component.ts

@@ -30,7 +30,7 @@ export class FeedbackDetailChannelModalComponent {
     form: FormGroup = this.formBuilder.group({
         name: ['', [Validators.required, Validators.maxLength(30), Validators.pattern('^[a-z0-9-]{1}[a-z0-9-]{0,30}$')]],
         description: ['', [Validators.required, Validators.maxLength(250)]],
-        isPublic: [true, Validators.required],
+        isPrivate: [true, Validators.required],
         isAnnouncementChannel: [false, Validators.required],
     });
 
@@ -42,7 +42,8 @@ export class FeedbackDetailChannelModalComponent {
                 const channelDTO = new ChannelDTO();
                 channelDTO.name = this.form.get('name')?.value;
                 channelDTO.description = this.form.get('description')?.value;
-                channelDTO.isPublic = this.form.get('isPublic')?.value || false;
+                channelDTO.topic = 'FeedbackDiscussion';
+                channelDTO.isPublic = !this.form.get('isPrivate')?.value || false;
                 channelDTO.isAnnouncementChannel = this.form.get('isAnnouncementChannel')?.value || false;
 
                 this.formSubmitted.emit({ channelDto: channelDTO, navigate });

src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/modal/feedback-modal.component.html

@@ -20,7 +20,11 @@ <h4 class="m-0" [jhiTranslate]="TRANSLATION_BASE + '.feedbackModal.header'"></h4
     </div>
 
     <h5 class="mb-3" [jhiTranslate]="TRANSLATION_BASE + '.feedbackModal.feedbackTitle'"></h5>
-    <p>{{ feedbackDetail().detailTexts[0] }}</p>
+    @if (longFeedbackText()) {
+        <p>{{ longFeedbackText() }}</p>
+    } @else {
+        <p>{{ feedbackDetail().detailTexts[0] }}</p>
+    }
 </div>
 
 <ng-template #detailTemplate let-label="label" let-value="value">

src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/modal/feedback-modal.component.ts

@@ -1,16 +1,28 @@
+import { Component, OnInit, inject, input, signal } from '@angular/core';
 import { CommonModule } from '@angular/common';
-import { Component, inject, input } from '@angular/core';
 import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
 import { FeedbackDetail } from 'app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service';
+import { LongFeedbackTextService } from 'app/exercises/shared/feedback/long-feedback-text.service';
 import { TranslateDirective } from 'app/shared/language/translate.directive';
 
 @Component({
     selector: 'jhi-feedback-modal',
     templateUrl: './feedback-modal.component.html',
     imports: [TranslateDirective, CommonModule],
 })
-export class FeedbackModalComponent {
+export class FeedbackModalComponent implements OnInit {
     feedbackDetail = input.required<FeedbackDetail>();
+    longFeedbackText = signal<string>('');
+
     activeModal = inject(NgbActiveModal);
+    longFeedbackTextService = inject(LongFeedbackTextService);
     readonly TRANSLATION_BASE = 'artemisApp.programmingExercise.configureGrading.feedbackAnalysis';
+
+    ngOnInit(): void {
+        if (this.feedbackDetail().hasLongFeedbackText) {
+            this.longFeedbackTextService.find(this.feedbackDetail().feedbackIds[0]).subscribe((response) => {
+                this.longFeedbackText.set(response.body || this.feedbackDetail().detailTexts[0]);
+            });
+        }
+    }
 }

src/main/webapp/app/exercises/shared/feedback/feedback.service.ts

@@ -1,4 +1,4 @@
-import { Injectable, Signal } from '@angular/core';
+import { Injectable } from '@angular/core';
 import { Feedback } from 'app/entities/feedback.model';
 import { BaseApiHttpService } from 'app/course/learning-paths/services/base-api-http.service';
 
@@ -17,9 +17,8 @@ export class FeedbackService extends BaseApiHttpService {
         return feedbacks.filter((feedback) => feedback.testCase?.id && filter.includes(feedback.testCase.id));
     };
 
-    public async getLongFeedbackText(resultId: Signal<number>, feedbackId: number): Promise<string> {
-        const resultIdValue = resultId();
-        const url = `results/${resultIdValue}/feedbacks/${feedbackId}/long-feedback`;
+    public async getLongFeedbackText(feedbackId: number): Promise<string> {
+        const url = `feedbacks/${feedbackId}/long-feedback`;
         return await this.get<string>(url, { responseType: 'text' });
     }
 }

src/main/webapp/app/exercises/shared/feedback/long-feedback-text.service.ts

@@ -8,7 +8,7 @@ export type LongFeedbackResponse = HttpResponse<string>;
 export class LongFeedbackTextService {
     private http = inject(HttpClient);
 
-    find(resultId: number, feedbackId: number): Observable<LongFeedbackResponse> {
-        return this.http.get(`api/results/${resultId}/feedbacks/${feedbackId}/long-feedback`, { observe: 'response', responseType: 'text' });
+    find(feedbackId: number): Observable<LongFeedbackResponse> {
+        return this.http.get(`api/feedbacks/${feedbackId}/long-feedback`, { observe: 'response', responseType: 'text' });
     }
 }

src/main/webapp/app/exercises/shared/feedback/text/feedback-text.component.ts

@@ -30,19 +30,20 @@ export class FeedbackTextComponent implements OnInit {
     }
 
     private loadLongFeedback() {
-        const resultId = this.feedback.feedbackReference.result!.id!;
-        const feedbackId = this.feedback.feedbackReference.id!;
-
-        this.longFeedbackService.find(resultId, feedbackId).subscribe((longFeedbackResponse) => {
-            const longFeedback = longFeedbackResponse.body!;
-            const textLength = longFeedback.length ?? 0;
-
-            if (textLength > this.MAX_DISPLAYABLE_LENGTH) {
-                this.setDownloadInfo(longFeedback);
-            } else {
-                this.text = longFeedback;
-            }
-        });
+        if (this.feedback.feedbackReference.id) {
+            const feedbackId = this.feedback.feedbackReference.id;
+
+            this.longFeedbackService.find(feedbackId).subscribe((longFeedbackResponse) => {
+                const longFeedback = longFeedbackResponse.body!;
+                const textLength = longFeedback.length ?? 0;
+
+                if (textLength > this.MAX_DISPLAYABLE_LENGTH) {
+                    this.setDownloadInfo(longFeedback);
+                } else {
+                    this.text = longFeedback;
+                }
+            });
+        }
     }
 
     private setDownloadInfo(longFeedback: string) {

src/main/webapp/app/overview/course-conversations/course-conversations.component.ts

@@ -14,6 +14,7 @@ import {
     faHeart,
     faList,
     faMessage,
+    faPersonChalkboard,
     faPlus,
     faSearch,
     faTimes,
@@ -64,6 +65,7 @@ const DEFAULT_CHANNEL_GROUPS: AccordionGroups = {
     exerciseChannels: { entityData: [] },
     lectureChannels: { entityData: [] },
     examChannels: { entityData: [] },
+    feedbackDiscussion: { entityData: [] },
     hiddenChannels: { entityData: [] },
     savedPosts: { entityData: [] },
 };
@@ -77,6 +79,7 @@ const CHANNEL_TYPE_ICON: ChannelTypeIcons = {
     favoriteChannels: faHeart,
     lectureChannels: faFile,
     hiddenChannels: faBan,
+    feedbackDiscussion: faPersonChalkboard,
     savedPosts: faBookmark,
     recents: faClock,
 };
@@ -90,6 +93,7 @@ const DEFAULT_COLLAPSE_STATE: CollapseState = {
     favoriteChannels: false,
     lectureChannels: true,
     hiddenChannels: true,
+    feedbackDiscussion: true,
     savedPosts: true,
     recents: true,
 };
@@ -103,6 +107,7 @@ const DEFAULT_SHOW_ALWAYS: SidebarItemShowAlways = {
     favoriteChannels: true,
     lectureChannels: false,
     hiddenChannels: false,
+    feedbackDiscussion: false,
     savedPosts: true,
     recents: true,
 };