From 7896ff92ba441632cef661120423a6306a6dac6c Mon Sep 17 00:00:00 2001 From: Aniruddh Zaveri <92953467+az108@users.noreply.github.com> Date: Sat, 16 Nov 2024 10:25:48 +0100 Subject: [PATCH] Programming exercises: Add affected students to feedback analysis table (#9728) --- .../dto/FeedbackAffectedStudentDTO.java | 4 + .../assessment/dto/FeedbackDetailDTO.java | 11 ++- .../assessment/service/ResultService.java | 26 ++++++- .../assessment/web/ResultResource.java | 31 +++++++- .../cit/aet/artemis/core/util/PageUtil.java | 3 + .../StudentParticipationRepository.java | 47 ++++++++++-- ...ack-affected-students-modal.component.html | 49 ++++++++++++ ...dback-affected-students-modal.component.ts | 59 +++++++++++++++ .../feedback-analysis.component.html | 34 +++------ .../feedback-analysis.component.ts | 10 ++- .../feedback-analysis.service.ts | 27 ++++++- .../webapp/app/shared/table/pageable-table.ts | 6 ++ .../webapp/i18n/de/programmingExercise.json | 10 ++- .../webapp/i18n/en/programmingExercise.json | 10 ++- .../ResultServiceIntegrationTest.java | 42 +++++++++++ .../feedback-analysis.component.spec.ts | 18 ++++- .../feedback-analysis.service.spec.ts | 72 +++++++++++++++++- ...-affected-students-modal.component.spec.ts | 75 +++++++++++++++++++ .../modals/feedback-modal.component.spec.ts | 1 + 19 files changed, 486 insertions(+), 49 deletions(-) create mode 100644 src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAffectedStudentDTO.java create mode 100644 src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.html create mode 100644 src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.ts create mode 100644 src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-affected-students-modal.component.spec.ts diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAffectedStudentDTO.java b/src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAffectedStudentDTO.java new file mode 100644 index 000000000000..71c6b73a208f --- /dev/null +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAffectedStudentDTO.java @@ -0,0 +1,4 @@ +package de.tum.cit.aet.artemis.assessment.dto; + +public record FeedbackAffectedStudentDTO(long courseId, long participationId, String firstName, String lastName, String login, String repositoryURI) { +} diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.java b/src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.java index 6d31e250f5d5..d22a036e7489 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.java @@ -1,7 +1,16 @@ package de.tum.cit.aet.artemis.assessment.dto; +import java.util.Arrays; +import java.util.List; + import com.fasterxml.jackson.annotation.JsonInclude; @JsonInclude(JsonInclude.Include.NON_EMPTY) -public record FeedbackDetailDTO(long count, double relativeCount, String detailText, String testCaseName, String taskName, String errorCategory) { +public record FeedbackDetailDTO(List concatenatedFeedbackIds, long count, double relativeCount, String detailText, String testCaseName, String taskName, + String errorCategory) { + + public FeedbackDetailDTO(String concatenatedFeedbackIds, long count, double relativeCount, String detailText, String testCaseName, String taskName, String errorCategory) { + this(Arrays.stream(concatenatedFeedbackIds.split(",")).map(Long::valueOf).toList(), count, relativeCount, detailText, testCaseName, taskName, errorCategory); + } + } 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 d0c25898b6a3..50576953916b 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 @@ -4,6 +4,7 @@ import java.time.ZonedDateTime; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Comparator; import java.util.HashMap; @@ -24,6 +25,7 @@ import org.slf4j.LoggerFactory; import org.springframework.context.annotation.Profile; import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageRequest; import org.springframework.stereotype.Service; import de.tum.cit.aet.artemis.assessment.domain.AssessmentType; @@ -31,6 +33,7 @@ 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.FeedbackAffectedStudentDTO; import de.tum.cit.aet.artemis.assessment.dto.FeedbackAnalysisResponseDTO; import de.tum.cit.aet.artemis.assessment.dto.FeedbackDetailDTO; import de.tum.cit.aet.artemis.assessment.dto.FeedbackPageableDTO; @@ -46,6 +49,7 @@ import de.tum.cit.aet.artemis.core.domain.Course; import de.tum.cit.aet.artemis.core.domain.User; import de.tum.cit.aet.artemis.core.dto.SearchResultPageDTO; +import de.tum.cit.aet.artemis.core.dto.pageablesearch.PageableSearchDTO; import de.tum.cit.aet.artemis.core.exception.BadRequestAlertException; import de.tum.cit.aet.artemis.core.repository.UserRepository; import de.tum.cit.aet.artemis.core.security.Role; @@ -618,9 +622,8 @@ public FeedbackAnalysisResponseDTO getFeedbackDetailsOnPage(long exerciseId, Fee maxOccurrence, filterErrorCategories, pageable); // 10. Process and map feedback details, calculating relative count and assigning task names - List processedDetails = feedbackDetailPage.getContent().stream().map(detail -> new FeedbackDetailDTO(detail.count(), + List processedDetails = feedbackDetailPage.getContent().stream().map(detail -> new FeedbackDetailDTO(detail.concatenatedFeedbackIds(), detail.count(), (detail.count() * 100.00) / distinctResultCount, detail.detailText(), detail.testCaseName(), detail.taskName(), detail.errorCategory())).toList(); - // 11. Predefined error categories available for filtering on the client side final List ERROR_CATEGORIES = List.of("Student Error", "Ares Error", "AST Error"); @@ -642,6 +645,25 @@ public long getMaxCountForExercise(long exerciseId) { return studentParticipationRepository.findMaxCountForExercise(exerciseId); } + /** + * Retrieves a paginated list of students affected by specific feedback entries for a given exercise. + *
+ * This method filters students based on feedback IDs and returns participation details for each affected student. It uses + * pagination and sorting (order based on the {@link PageUtil.ColumnMapping#AFFECTED_STUDENTS}) to allow efficient retrieval and sorting of the results, thus supporting large + * datasets. + *
+ * + * @param exerciseId for which the affected student participation data is requested. + * @param feedbackIds used to filter the participation to only those affected by specific feedback entries. + * @param data A {@link PageableSearchDTO} object containing pagination and sorting parameters. + * @return A {@link Page} of {@link FeedbackAffectedStudentDTO} objects, each representing a student affected by the feedback. + */ + public Page getAffectedStudentsWithFeedbackId(long exerciseId, String feedbackIds, PageableSearchDTO data) { + List feedbackIdLongs = Arrays.stream(feedbackIds.split(",")).map(Long::valueOf).toList(); + PageRequest pageRequest = PageUtil.createDefaultPageRequest(data, PageUtil.ColumnMapping.AFFECTED_STUDENTS); + return studentParticipationRepository.findAffectedStudentsByFeedbackId(exerciseId, feedbackIdLongs, pageRequest); + } + /** * Deletes long feedback texts for the provided list of feedback items to prevent duplicate entries in the {@link LongFeedbackTextRepository}. *
diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java b/src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java index 2a235145a04b..431fb66373e8 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java @@ -14,6 +14,7 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Profile; +import org.springframework.data.domain.Page; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; @@ -22,12 +23,14 @@ import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; import de.tum.cit.aet.artemis.assessment.domain.Feedback; import de.tum.cit.aet.artemis.assessment.domain.Result; +import de.tum.cit.aet.artemis.assessment.dto.FeedbackAffectedStudentDTO; import de.tum.cit.aet.artemis.assessment.dto.FeedbackAnalysisResponseDTO; import de.tum.cit.aet.artemis.assessment.dto.FeedbackPageableDTO; import de.tum.cit.aet.artemis.assessment.dto.ResultWithPointsPerGradingCriterionDTO; @@ -36,13 +39,14 @@ import de.tum.cit.aet.artemis.core.domain.Course; import de.tum.cit.aet.artemis.core.domain.User; import de.tum.cit.aet.artemis.core.dto.SearchResultPageDTO; +import de.tum.cit.aet.artemis.core.dto.pageablesearch.PageableSearchDTO; import de.tum.cit.aet.artemis.core.exception.BadRequestAlertException; import de.tum.cit.aet.artemis.core.repository.UserRepository; import de.tum.cit.aet.artemis.core.security.Role; import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastInstructor; import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastStudent; import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastTutor; -import de.tum.cit.aet.artemis.core.security.annotations.enforceRoleInExercise.EnforceAtLeastInstructorInExercise; +import de.tum.cit.aet.artemis.core.security.annotations.enforceRoleInExercise.EnforceAtLeastEditorInExercise; import de.tum.cit.aet.artemis.core.service.AuthorizationCheckService; import de.tum.cit.aet.artemis.core.util.HeaderUtil; import de.tum.cit.aet.artemis.exam.domain.Exam; @@ -328,7 +332,7 @@ public ResponseEntity createResultForExternalSubmission(@PathVariable Lo * */ @GetMapping("exercises/{exerciseId}/feedback-details") - @EnforceAtLeastInstructorInExercise + @EnforceAtLeastEditorInExercise public ResponseEntity getFeedbackDetailsPaged(@PathVariable long exerciseId, @ModelAttribute FeedbackPageableDTO data) { FeedbackAnalysisResponseDTO response = resultService.getFeedbackDetailsOnPage(exerciseId, data); return ResponseEntity.ok(response); @@ -343,9 +347,30 @@ public ResponseEntity getFeedbackDetailsPaged(@Path * @return A {@link ResponseEntity} containing the maximum count of feedback occurrences (long). */ @GetMapping("exercises/{exerciseId}/feedback-details-max-count") - @EnforceAtLeastInstructorInExercise + @EnforceAtLeastEditorInExercise public ResponseEntity getMaxCount(@PathVariable long exerciseId) { long maxCount = resultService.getMaxCountForExercise(exerciseId); return ResponseEntity.ok(maxCount); } + + /** + * GET /exercises/{exerciseId}/feedback-details-participation : Retrieves paginated details of students affected by specific feedback entries for a specified exercise. + * This endpoint returns details of students whose submissions were impacted by specified feedback entries, including student information + * and participation details. + *
+ * + * @param exerciseId for which the participation data is requested. + * @param feedbackIdsHeader to filter affected students by specific feedback entries. + * @param data A {@link PageableSearchDTO} object containing pagination and sorting parameters. + * @return A {@link ResponseEntity} containing a {@link Page} of {@link FeedbackAffectedStudentDTO}, each representing a student affected by the feedback entries. + */ + @GetMapping("exercises/{exerciseId}/feedback-details-participation") + @EnforceAtLeastEditorInExercise + public ResponseEntity> getAffectedStudentsWithFeedback(@PathVariable long exerciseId, @RequestHeader("feedbackIds") String feedbackIdsHeader, + @ModelAttribute PageableSearchDTO data) { + + Page participation = resultService.getAffectedStudentsWithFeedbackId(exerciseId, feedbackIdsHeader, data); + + return ResponseEntity.ok(participation); + } } diff --git a/src/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.java b/src/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.java index 7417fe060e93..88f7bb7302e6 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.java @@ -82,6 +82,9 @@ SELECT MAX(t.taskName) JOIN t.testCases tct WHERE t.exercise.id = :exerciseId AND tct.testName = f.testCase.testName ), '')""" + )), + AFFECTED_STUDENTS(Map.of( + "participationId", "p.id" )); // @formatter:on diff --git a/src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java index b617ae44948f..1101f94d4708 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java @@ -29,6 +29,7 @@ import de.tum.cit.aet.artemis.assessment.domain.AssessmentType; import de.tum.cit.aet.artemis.assessment.domain.Result; +import de.tum.cit.aet.artemis.assessment.dto.FeedbackAffectedStudentDTO; import de.tum.cit.aet.artemis.assessment.dto.FeedbackDetailDTO; import de.tum.cit.aet.artemis.core.domain.User; import de.tum.cit.aet.artemis.core.repository.base.ArtemisJpaRepository; @@ -1217,7 +1218,7 @@ SELECT COALESCE(AVG(p.presentationScore), 0) * - Occurrence range: Filters feedback where the number of occurrences (COUNT) is between the specified minimum and maximum values (inclusive). * - Error categories: Filters feedback based on error categories, which can be "Student Error", "Ares Error", or "AST Error". *
- * Grouping is done by feedback detail text, test case name, and error category. The occurrence count is filtered using the HAVING clause. + * Grouping is done by feedback detail text, test case name and error category. The occurrence count is filtered using the HAVING clause. * * @param exerciseId The ID of the exercise for which feedback details should be retrieved. * @param searchTerm The search term used for filtering the feedback detail text (optional). @@ -1233,6 +1234,7 @@ SELECT COALESCE(AVG(p.presentationScore), 0) */ @Query(""" SELECT new de.tum.cit.aet.artemis.assessment.dto.FeedbackDetailDTO( + LISTAGG(CAST(f.id AS string), ',') WITHIN GROUP (ORDER BY f.id), COUNT(f.id), 0, f.detailText, @@ -1240,7 +1242,7 @@ SELECT COALESCE(AVG(p.presentationScore), 0) COALESCE(( SELECT MAX(t.taskName) FROM ProgrammingExerciseTask t - JOIN t.testCases tct + LEFT JOIN t.testCases tct WHERE t.exercise.id = :exerciseId AND tct.testName = f.testCase.testName ), 'Not assigned to task'), CASE @@ -1250,12 +1252,12 @@ SELECT MAX(t.taskName) END ) FROM StudentParticipation p - JOIN p.results r ON r.id = ( + LEFT JOIN p.results r ON r.id = ( SELECT MAX(pr.id) FROM p.results pr WHERE pr.participation.id = p.id ) - JOIN r.feedbacks f + LEFT JOIN r.feedbacks f WHERE p.exercise.id = :exerciseId AND p.testRun = FALSE AND f.positive = FALSE @@ -1264,7 +1266,7 @@ SELECT MAX(pr.id) AND (:#{#filterTaskNames != NULL && #filterTaskNames.size() < 1} = TRUE OR f.testCase.testName NOT IN ( SELECT tct.testName FROM ProgrammingExerciseTask t - JOIN t.testCases tct + LEFT JOIN t.testCases tct WHERE t.taskName IN (:filterTaskNames) )) AND (:#{#filterErrorCategories != NULL && #filterErrorCategories.size() < 1} = TRUE OR CASE @@ -1290,7 +1292,7 @@ Page findFilteredFeedbackByExerciseId(@Param("exerciseId") lo @Query(""" SELECT COUNT(DISTINCT r.id) FROM StudentParticipation p - JOIN p.results r ON r.id = ( + LEFT JOIN p.results r ON r.id = ( SELECT MAX(pr.id) FROM p.results pr WHERE pr.participation.id = p.id @@ -1317,12 +1319,12 @@ SELECT MAX(feedbackCounts.feedbackCount) FROM ( SELECT COUNT(f.id) AS feedbackCount FROM StudentParticipation p - JOIN p.results r ON r.id = ( + LEFT JOIN p.results r ON r.id = ( SELECT MAX(pr.id) FROM p.results pr WHERE pr.participation.id = p.id ) - JOIN r.feedbacks f + LEFT JOIN r.feedbacks f WHERE p.exercise.id = :exerciseId AND p.testRun = FALSE AND f.positive = FALSE @@ -1330,4 +1332,33 @@ SELECT MAX(pr.id) ) AS feedbackCounts """) long findMaxCountForExercise(@Param("exerciseId") long exerciseId); + + /** + * Retrieves a paginated list of students affected by specific feedback entries for a given programming exercise. + *
+ * + * @param exerciseId for which the affected student participation data is requested. + * @param feedbackIds used to filter the participation to only those affected by specific feedback entries. + * @param pageable A {@link Pageable} object to control pagination and sorting of the results, specifying page number, page size, and sort order. + * @return A {@link Page} of {@link FeedbackAffectedStudentDTO} objects, each representing a student affected by the feedback. + */ + @Query(""" + SELECT new de.tum.cit.aet.artemis.assessment.dto.FeedbackAffectedStudentDTO( + p.exercise.course.id, + p.id, + p.student.firstName, + p.student.lastName, + p.student.login, + p.repositoryUri + ) + FROM ProgrammingExerciseStudentParticipation p + LEFT JOIN p.submissions s + LEFT JOIN s.results r + LEFT JOIN r.feedbacks f + WHERE p.exercise.id = :exerciseId + AND f.id IN :feedbackIds + AND p.testRun = FALSE + ORDER BY p.student.firstName ASC + """) + Page findAffectedStudentsByFeedbackId(@Param("exerciseId") long exerciseId, @Param("feedbackIds") List feedbackIds, Pageable pageable); } diff --git a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.html b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.html new file mode 100644 index 000000000000..4f53792a04e5 --- /dev/null +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.html @@ -0,0 +1,49 @@ + + + + + + + diff --git a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.ts b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.ts new file mode 100644 index 000000000000..947f3f3d44cd --- /dev/null +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.ts @@ -0,0 +1,59 @@ +import { Component, computed, effect, inject, input, signal, untracked } from '@angular/core'; +import { ArtemisSharedCommonModule } from 'app/shared/shared-common.module'; +import { ArtemisSharedComponentModule } from 'app/shared/components/shared-component.module'; +import { FeedbackAffectedStudentDTO, FeedbackAnalysisService, FeedbackDetail } from 'app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service'; +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { AlertService } from 'app/core/util/alert.service'; +import { PageableResult, PageableSearch, SortingOrder } from 'app/shared/table/pageable-table'; + +@Component({ + selector: 'jhi-affected-students-modal', + templateUrl: './feedback-affected-students-modal.component.html', + imports: [ArtemisSharedCommonModule, ArtemisSharedComponentModule], + providers: [FeedbackAnalysisService], + standalone: true, +}) +export class AffectedStudentsModalComponent { + exerciseId = input.required(); + feedbackDetail = input.required(); + readonly participation = signal>({ content: [], totalPages: 0, totalElements: 0 }); + readonly TRANSLATION_BASE = 'artemisApp.programmingExercise.configureGrading.feedbackAnalysis.affectedStudentsModal'; + + page = signal(1); + pageSize = signal(10); + readonly collectionsSize = computed(() => this.participation().totalPages * this.pageSize()); + + activeModal = inject(NgbActiveModal); + feedbackService = inject(FeedbackAnalysisService); + alertService = inject(AlertService); + + constructor() { + effect(() => { + untracked(async () => { + await this.loadAffected(); + }); + }); + } + + private async loadAffected() { + const feedbackDetail = this.feedbackDetail(); + const pageable: PageableSearch = { + page: this.page(), + pageSize: this.pageSize(), + sortedColumn: 'participationId', + sortingOrder: SortingOrder.ASCENDING, + }; + + try { + const response = await this.feedbackService.getParticipationForFeedbackIds(this.exerciseId(), feedbackDetail.concatenatedFeedbackIds, pageable); + this.participation.set(response); + } catch (error) { + this.alertService.error(this.TRANSLATION_BASE + '.error'); + } + } + + setPage(newPage: number): void { + this.page.set(newPage); + this.loadAffected(); + } +} diff --git a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html index 9a17640e3d72..20206a2c4ae3 100644 --- a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html @@ -1,5 +1,5 @@ - - + +
@if (column !== 'errorCategory') { @@ -33,21 +33,13 @@

- - - - - - - - - - - - - + + + + + @@ -69,6 +61,7 @@

{{ item.errorCategory }}

} @@ -76,16 +69,7 @@

+
- - +
diff --git a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts index b36b453109fd..9026d7cbb1ec 100644 --- a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts @@ -2,7 +2,7 @@ import { Component, computed, effect, inject, input, signal, untracked } from '@ import { FeedbackAnalysisService, FeedbackDetail } from './feedback-analysis.service'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { AlertService } from 'app/core/util/alert.service'; -import { faFilter, faSort, faSortDown, faSortUp, faUpRightAndDownLeftFromCenter } from '@fortawesome/free-solid-svg-icons'; +import { faFilter, faSort, faSortDown, faSortUp, faUpRightAndDownLeftFromCenter, faUsers } from '@fortawesome/free-solid-svg-icons'; import { SearchResult, SortingOrder } from 'app/shared/table/pageable-table'; import { ArtemisSharedCommonModule } from 'app/shared/shared-common.module'; import { FeedbackModalComponent } from 'app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-modal.component'; @@ -10,6 +10,7 @@ import { FeedbackFilterModalComponent, FilterData } from 'app/exercises/programm import { LocalStorageService } from 'ngx-webstorage'; import { BaseApiHttpService } from 'app/course/learning-paths/services/base-api-http.service'; import { SortIconComponent } from 'app/shared/sort/sort-icon.component'; +import { AffectedStudentsModalComponent } from 'app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component'; @Component({ selector: 'jhi-feedback-analysis', @@ -44,6 +45,7 @@ export class FeedbackAnalysisComponent { readonly faSortDown = faSortDown; readonly faFilter = faFilter; readonly faUpRightAndDownLeftFromCenter = faUpRightAndDownLeftFromCenter; + readonly faUsers = faUsers; readonly SortingOrder = SortingOrder; readonly MAX_FEEDBACK_DETAIL_TEXT_LENGTH = 200; @@ -183,4 +185,10 @@ export class FeedbackAnalysisComponent { } return count; } + + async openAffectedStudentsModal(feedbackDetail: FeedbackDetail): Promise { + const modalRef = this.modalService.open(AffectedStudentsModalComponent, { centered: true, size: 'lg' }); + modalRef.componentInstance.exerciseId = this.exerciseId; + modalRef.componentInstance.feedbackDetail = signal(feedbackDetail); + } } diff --git a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts index 4228e50c329a..214c9a4e4f4c 100644 --- a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@angular/core'; -import { SearchResult, SearchTermPageableSearch } from 'app/shared/table/pageable-table'; +import { PageableResult, PageableSearch, SearchResult, SearchTermPageableSearch } from 'app/shared/table/pageable-table'; import { BaseApiHttpService } from 'app/course/learning-paths/services/base-api-http.service'; -import { HttpParams } from '@angular/common/http'; +import { HttpHeaders, HttpParams } from '@angular/common/http'; import { FilterData } from 'app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-filter-modal.component'; export interface FeedbackAnalysisResponse { @@ -12,6 +12,7 @@ export interface FeedbackAnalysisResponse { errorCategories: string[]; } export interface FeedbackDetail { + concatenatedFeedbackIds: number[]; count: number; relativeCount: number; detailText: string; @@ -19,6 +20,14 @@ export interface FeedbackDetail { taskName: string; errorCategory: string; } +export interface FeedbackAffectedStudentDTO { + courseId: number; + participationId: number; + firstName: string; + lastName: string; + login: string; + repositoryURI: string; +} @Injectable() export class FeedbackAnalysisService extends BaseApiHttpService { search(pageable: SearchTermPageableSearch, options: { exerciseId: number; filters: FilterData }): Promise { @@ -39,4 +48,18 @@ export class FeedbackAnalysisService extends BaseApiHttpService { getMaxCount(exerciseId: number): Promise { return this.get(`exercises/${exerciseId}/feedback-details-max-count`); } + + async getParticipationForFeedbackIds(exerciseId: number, feedbackIds: number[], pageable: PageableSearch): Promise> { + const feedbackIdsHeader = feedbackIds.join(','); + + const params = new HttpParams() + .set('page', pageable.page.toString()) + .set('pageSize', pageable.pageSize.toString()) + .set('sortedColumn', pageable.sortedColumn) + .set('sortingOrder', pageable.sortingOrder); + + const headers = new HttpHeaders().set('feedbackIds', feedbackIdsHeader); + + return this.get>(`exercises/${exerciseId}/feedback-details-participation`, { params, headers }); + } } diff --git a/src/main/webapp/app/shared/table/pageable-table.ts b/src/main/webapp/app/shared/table/pageable-table.ts index 82c9d4f0ef5e..131648d7737b 100644 --- a/src/main/webapp/app/shared/table/pageable-table.ts +++ b/src/main/webapp/app/shared/table/pageable-table.ts @@ -1,3 +1,9 @@ +export interface PageableResult { + content: T[]; + totalElements: number; + totalPages: number; +} + export interface SearchResult { resultsOnPage: T[]; numberOfPages: number; diff --git a/src/main/webapp/i18n/de/programmingExercise.json b/src/main/webapp/i18n/de/programmingExercise.json index 79c6a18a57c8..41533417cf1e 100644 --- a/src/main/webapp/i18n/de/programmingExercise.json +++ b/src/main/webapp/i18n/de/programmingExercise.json @@ -351,7 +351,7 @@ "task": "Aufgabe", "testcase": "Testfall", "errorCategory": "Fehlerkategorie", - "totalItems": "Insgesamt {{count}} Elemente", + "totalItems": "Insgesamt {{count}} Element(e)", "error": "Beim Laden des Feedbacks ist ein Fehler aufgetreten.", "search": "Suche ...", "filter": "Filter", @@ -371,6 +371,14 @@ "clear": "Filter zurücksetzen", "cancel": "Abbrechen", "apply": "Filter anwenden" + }, + "affectedStudentsModal": { + "header": "Betroffene Studierende", + "name": "Name", + "login": "Login", + "repository": "Repository", + "totalItems": "Insgesamt {{count}} Element(e)", + "error": "Beim Laden der betroffenen Studierenden ist ein Fehler aufgetreten." } }, "help": { diff --git a/src/main/webapp/i18n/en/programmingExercise.json b/src/main/webapp/i18n/en/programmingExercise.json index 244334cb61c8..d3a53f20f180 100644 --- a/src/main/webapp/i18n/en/programmingExercise.json +++ b/src/main/webapp/i18n/en/programmingExercise.json @@ -351,7 +351,7 @@ "task": "Task", "testcase": "Test Case", "errorCategory": "Error Category", - "totalItems": "In total {{count}} items", + "totalItems": "In total {{count}} item(s)", "error": "An error occurred while loading the feedback.", "search": "Search ...", "filter": "Filters", @@ -371,6 +371,14 @@ "clear": "Clear Filter", "cancel": "Cancel", "apply": "Apply Filter" + }, + "affectedStudentsModal": { + "header": "Affected Students", + "name": "Name", + "login": "Login", + "repository": "Repository", + "totalItems": "In total {{count}} item(s)", + "error": "There was an error while loading the affected Students for this feedback." } }, "help": { diff --git a/src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java index 2413dcab2078..6b02b3e4d247 100644 --- a/src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java @@ -22,9 +22,13 @@ import org.junit.jupiter.params.provider.MethodSource; import org.mockito.ArgumentMatchers; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.security.test.context.support.WithMockUser; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + import de.tum.cit.aet.artemis.assessment.domain.AssessmentType; import de.tum.cit.aet.artemis.assessment.domain.Feedback; import de.tum.cit.aet.artemis.assessment.domain.GradingCriterion; @@ -861,4 +865,42 @@ void testGetMaxCountForExerciseWithMultipleFeedback() throws Exception { assertThat(maxCount).isEqualTo(2); } + + @Test + @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") + void testGetParticipationForFeedbackId() throws Exception { + ProgrammingExercise programmingExercise = programmingExerciseUtilService.addProgrammingExerciseToCourse(course); + StudentParticipation participation = participationUtilService.createAndSaveParticipationForExercise(programmingExercise, TEST_PREFIX + "student1"); + ProgrammingExerciseTestCase testCase = programmingExerciseUtilService.addTestCaseToProgrammingExercise(programmingExercise, "test1"); + testCase.setId(1L); + + Feedback feedback = new Feedback(); + feedback.setPositive(false); + feedback.setDetailText("Feedback for student"); + feedback.setTestCase(testCase); + feedback = feedbackRepository.saveAndFlush(feedback); + + Result result = participationUtilService.addResultToParticipation(AssessmentType.AUTOMATIC, null, participation); + participationUtilService.addFeedbackToResult(feedback, result); + + String url = "/api/exercises/" + programmingExercise.getId() + "/feedback-details-participation?page=1&pageSize=10&sortedColumn=participationId&sortingOrder=ASCENDING"; + HttpHeaders headers = new HttpHeaders(); + headers.add("feedbackIds", String.valueOf(feedback.getId())); + + String jsonResponse = request.get(url, HttpStatus.OK, String.class, headers); + + JsonNode jsonNode = new ObjectMapper().readTree(jsonResponse); + assertThat(jsonNode.has("content")).isTrue(); + assertThat(jsonNode.has("pageable")).isTrue(); + assertThat(jsonNode.has("last")).isTrue(); + assertThat(jsonNode.has("totalElements")).isTrue(); + assertThat(jsonNode.has("totalPages")).isTrue(); + assertThat(jsonNode.has("first")).isTrue(); + assertThat(jsonNode.has("size")).isTrue(); + assertThat(jsonNode.has("number")).isTrue(); + assertThat(jsonNode.has("sort")).isTrue(); + assertThat(jsonNode.has("numberOfElements")).isTrue(); + assertThat(jsonNode.has("empty")).isTrue(); + } + } diff --git a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts index 37e7821fe502..09d22ab65dd6 100644 --- a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts +++ b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts @@ -8,6 +8,7 @@ import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { LocalStorageService } from 'ngx-webstorage'; import '@angular/localize/init'; import { FeedbackFilterModalComponent } from 'app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-filter-modal.component'; +import { AffectedStudentsModalComponent } from 'app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component'; describe('FeedbackAnalysisComponent', () => { let fixture: ComponentFixture; @@ -18,6 +19,7 @@ describe('FeedbackAnalysisComponent', () => { const feedbackMock: FeedbackDetail[] = [ { + concatenatedFeedbackIds: [1, 2], detailText: 'Test feedback 1 detail', testCaseName: 'test1', count: 10, @@ -26,6 +28,7 @@ describe('FeedbackAnalysisComponent', () => { errorCategory: 'Student Error', }, { + concatenatedFeedbackIds: [3, 4], detailText: 'Test feedback 2 detail', testCaseName: 'test2', count: 5, @@ -62,9 +65,9 @@ describe('FeedbackAnalysisComponent', () => { localStorageService = fixture.debugElement.injector.get(LocalStorageService); jest.spyOn(localStorageService, 'retrieve').mockReturnValue([]); - searchSpy = jest.spyOn(feedbackAnalysisService, 'search').mockResolvedValue(feedbackResponseMock); + // Initial input setup fixture.componentRef.setInput('exerciseId', 1); fixture.componentRef.setInput('exerciseTitle', 'Sample Exercise Title'); @@ -238,4 +241,17 @@ describe('FeedbackAnalysisComponent', () => { expect(modalSpy).toHaveBeenCalledOnce(); }); }); + + describe('openAffectedStudentsModal', () => { + it('should open affected students modal with the correct feedback detail', () => { + const modalService = fixture.debugElement.injector.get(NgbModal); + const modalSpy = jest.spyOn(modalService, 'open').mockReturnValue({ componentInstance: {} } as any); + + const feedbackDetail = feedbackMock[1]; + component.openAffectedStudentsModal(feedbackDetail); + + expect(modalSpy).toHaveBeenCalledWith(AffectedStudentsModalComponent, { centered: true, size: 'lg' }); + expect(modalSpy).toHaveBeenCalledOnce(); + }); + }); }); diff --git a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts index 60b280966bb1..23b9bfaf0f49 100644 --- a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts +++ b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts @@ -9,15 +9,32 @@ describe('FeedbackAnalysisService', () => { let httpMock: HttpTestingController; const feedbackDetailsMock: FeedbackDetail[] = [ - { detailText: 'Feedback 1', testCaseName: 'test1', count: 5, relativeCount: 25.0, taskName: '1', errorCategory: 'StudentError' }, - { detailText: 'Feedback 2', testCaseName: 'test2', count: 3, relativeCount: 15.0, taskName: '2', errorCategory: 'StudentError' }, + { + concatenatedFeedbackIds: [1, 2], + detailText: 'Feedback 1', + testCaseName: 'test1', + count: 5, + relativeCount: 25.0, + taskName: '1', + errorCategory: 'StudentError', + }, + { + concatenatedFeedbackIds: [3, 4], + detailText: 'Feedback 2', + testCaseName: 'test2', + count: 3, + relativeCount: 15.0, + taskName: '2', + errorCategory: 'StudentError', + }, ]; const feedbackAnalysisResponseMock = { feedbackDetails: { resultsOnPage: feedbackDetailsMock, numberOfPages: 1 }, totalItems: 2, - totalAmountOfTasks: 2, + taskNames: ['task1', 'task2'], testCaseNames: ['test1', 'test2'], + errorCategories: ['StudentError'], }; beforeEach(() => { @@ -34,7 +51,7 @@ describe('FeedbackAnalysisService', () => { }); describe('search', () => { - it('should retrieve feedback details for a given exercise', async () => { + it('should retrieve feedback details for a given exercise with concatenatedFeedbackIds', async () => { const pageable = { page: 1, pageSize: 10, @@ -53,6 +70,8 @@ describe('FeedbackAnalysisService', () => { const result = await responsePromise; expect(result).toEqual(feedbackAnalysisResponseMock); + expect(result.feedbackDetails.resultsOnPage[0].concatenatedFeedbackIds).toStrictEqual([1, 2]); + expect(result.feedbackDetails.resultsOnPage[1].concatenatedFeedbackIds).toStrictEqual([3, 4]); }); }); @@ -68,4 +87,49 @@ describe('FeedbackAnalysisService', () => { expect(result).toBe(10); }); }); + + describe('getParticipationForFeedbackIds', () => { + it('should retrieve paginated participation details for specified feedback IDs', async () => { + const feedbackIds = [1, 2]; + const pageable = { + page: 1, + pageSize: 10, + sortedColumn: 'participationId', + sortingOrder: SortingOrder.ASCENDING, + }; + const participationResponseMock = { + content: [ + { + courseId: 1, + participationId: 101, + firstName: 'John', + lastName: 'Doe', + login: 'johndoe', + repositoryName: 'repo1', + }, + { + courseId: 1, + participationId: 102, + firstName: 'Jane', + lastName: 'Smith', + login: 'janesmith', + repositoryName: 'repo2', + }, + ], + numberOfPages: 1, + totalItems: 2, + }; + + const responsePromise = service.getParticipationForFeedbackIds(1, feedbackIds, pageable); + + const req = httpMock.expectOne('api/exercises/1/feedback-details-participation?page=1&pageSize=10&sortedColumn=participationId&sortingOrder=ASCENDING'); + expect(req.request.method).toBe('GET'); + req.flush(participationResponseMock); + + const result = await responsePromise; + expect(result).toEqual(participationResponseMock); + expect(result.content[0].firstName).toBe('John'); + expect(result.content[1].firstName).toBe('Jane'); + }); + }); }); diff --git a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-affected-students-modal.component.spec.ts b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-affected-students-modal.component.spec.ts new file mode 100644 index 000000000000..bf8b1b77141a --- /dev/null +++ b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-affected-students-modal.component.spec.ts @@ -0,0 +1,75 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { AffectedStudentsModalComponent } from 'app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component'; +import { FeedbackAffectedStudentDTO, FeedbackAnalysisService, FeedbackDetail } from 'app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service'; +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { TranslateModule } from '@ngx-translate/core'; +import { of } from 'rxjs'; +import { provideHttpClient } from '@angular/common/http'; +import '@angular/localize/init'; + +describe('AffectedStudentsModalComponent', () => { + let fixture: ComponentFixture; + let component: AffectedStudentsModalComponent; + let feedbackService: FeedbackAnalysisService; + + const feedbackDetailMock: FeedbackDetail = { + concatenatedFeedbackIds: [1, 2], + count: 5, + relativeCount: 25.0, + detailText: 'Some feedback detail', + testCaseName: 'testCase1', + taskName: '1', + errorCategory: 'StudentError', + }; + + const participationMock: FeedbackAffectedStudentDTO[] = [ + { courseId: 1, participationId: 101, firstName: 'John', lastName: 'Doe', login: 'johndoe', repositoryURI: 'repo1' }, + { courseId: 1, participationId: 102, firstName: 'Jane', lastName: 'Smith', login: 'janesmith', repositoryURI: 'repo2' }, + ]; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [TranslateModule.forRoot(), AffectedStudentsModalComponent], + providers: [ + provideHttpClient(), + NgbActiveModal, + { + provide: FeedbackAnalysisService, + useValue: { + getParticipationForFeedbackIds: jest.fn().mockReturnValue(of({ content: participationMock, totalPages: 1, totalElements: 2 })), + }, + }, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(AffectedStudentsModalComponent); + component = fixture.componentInstance; + feedbackService = TestBed.inject(FeedbackAnalysisService); + + fixture.componentRef.setInput('exerciseId', 1); + fixture.componentRef.setInput('feedbackDetail', feedbackDetailMock); + fixture.detectChanges(); + }); + + it('should update page and reload data when setPage is called', async () => { + const loadAffectedSpy = jest.spyOn(component as any, 'loadAffected'); + + component.setPage(2); + expect(component.page()).toBe(2); + expect(loadAffectedSpy).toHaveBeenCalledOnce(); + }); + + it('should handle error when loadAffected fails', async () => { + jest.spyOn(feedbackService, 'getParticipationForFeedbackIds').mockReturnValueOnce(Promise.reject(new Error('Error loading data'))); + const alertServiceSpy = jest.spyOn(component.alertService, 'error'); + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + await component['loadAffected'](); + + expect(component.participation().content).toEqual([]); + expect(alertServiceSpy).toHaveBeenCalled(); + expect(consoleErrorSpy).toHaveBeenCalled(); + + consoleErrorSpy.mockRestore(); + }); +}); diff --git a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-modal.component.spec.ts b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-modal.component.spec.ts index f3a6c0361e9f..cf31d3b2b9e0 100644 --- a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-modal.component.spec.ts +++ b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-modal.component.spec.ts @@ -10,6 +10,7 @@ describe('FeedbackModalComponent', () => { let activeModal: NgbActiveModal; const mockFeedbackDetail: FeedbackDetail = { + concatenatedFeedbackIds: [1, 2], count: 5, relativeCount: 25.0, detailText: 'Some feedback detail',