From 2aef101f04abafb3a3002bb88809e84e907feef9 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Sun, 10 Nov 2024 21:43:39 +0100 Subject: [PATCH 01/64] added tests and documentation --- .../dto/FeedbackAffectedStudentDTO.java | 4 + .../assessment/dto/FeedbackDetailDTO.java | 2 +- .../assessment/service/ResultService.java | 118 +++++++++--------- .../assessment/web/ResultResource.java | 42 ++++++- .../cit/aet/artemis/core/util/PageUtil.java | 3 + .../StudentParticipationRepository.java | 46 ++++++- ...ack-affected-students-modal.component.html | 42 +++++++ ...dback-affected-students-modal.component.ts | 59 +++++++++ .../feedback-analysis.component.html | 34 ++--- .../feedback-analysis.component.ts | 10 +- .../feedback-analysis.service.ts | 22 +++- .../webapp/app/shared/table/pageable-table.ts | 6 + .../webapp/i18n/de/programmingExercise.json | 7 ++ .../webapp/i18n/en/programmingExercise.json | 7 ++ .../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 + 18 files changed, 473 insertions(+), 95 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..76dc1e2deb72 --- /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 repositoryName) { +} 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..4b785d3d6b6a 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 @@ -3,5 +3,5 @@ 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(Object concatenatedFeedbackIds, long count, double relativeCount, String detailText, String testCaseName, String taskName, String 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..f9640a69ee06 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java @@ -12,7 +12,6 @@ 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; @@ -24,6 +23,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 +31,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 +47,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; @@ -495,49 +497,45 @@ public Map getLogsAvailabilityForResults(List results, Par @NotNull private List saveFeedbackWithHibernateWorkaround(@NotNull Result result, List feedbackList) { + // Avoid hibernate exception List savedFeedbacks = new ArrayList<>(); + // Collect ids of feedbacks that have long feedback. + List feedbackIdsWithLongFeedback = feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId) + .toList(); + // Get long feedback list from the database. + List longFeedbackTextList = longFeedbackTextRepository.findByFeedbackIds(feedbackIdsWithLongFeedback); - // 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())); - + // Convert list to map for accessing later. + Map longLongFeedbackTextMap = longFeedbackTextList.stream() + .collect(Collectors.toMap(longFeedbackText -> longFeedbackText.getFeedback().getId(), longFeedbackText -> longFeedbackText)); 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); + // 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); - // Clear the long feedback if it exists in the map - if (feedback.getId() != null && feedback.getHasLongFeedbackText()) { - LongFeedbackText longFeedback = longFeedbackTextMap.get(feedback.getId()); + // restore the association of the long feedback to the parent feedback if (longFeedback != null) { - feedback.clearLongFeedback(); + feedback.setDetailText(longFeedback.getText()); } - } - - // 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()); - } + savedFeedbacks.add(feedback); + }); + return savedFeedbacks; } @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); } @@ -618,8 +616,10 @@ 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(), - (detail.count() * 100.00) / distinctResultCount, detail.detailText(), detail.testCaseName(), detail.taskName(), detail.errorCategory())).toList(); + List processedDetails = feedbackDetailPage.getContent().stream() + .map(detail -> new FeedbackDetailDTO(List.of(String.valueOf(detail.concatenatedFeedbackIds()).split(",")), 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"); @@ -643,31 +643,33 @@ public long getMaxCountForExercise(long exerciseId) { } /** - * Deletes long feedback texts for the provided list of feedback items to prevent duplicate entries in the {@link LongFeedbackTextRepository}. + * Retrieves a paginated list of students affected by specific feedback entries for a given exercise. *
- * 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. - *

- * **Note:** This method should only be used for manually assessed submissions, not for fully automatic assessments, due to its dependency on the - * {@link Result#updateAllFeedbackItems} method, which is designed for manual feedback management. Using this method with automatic assessments could - * lead to unintended behavior or data inconsistencies. + * This method filters students based on feedback IDs and returns participation details for each affected student. It uses + * pagination and sorting to allow efficient retrieval and sorting of the results, thus supporting large datasets. + *
+ * Supports: + *

    + *
  • Pagination: Controls the page number and page size for the returned results.
  • + *
  • Sorting: Applies sorting by specified columns and sorting order based on the {@link PageUtil.ColumnMapping#AFFECTED_STUDENTS} configuration.
  • + *
* - * @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. + * @param exerciseId The ID of the exercise for which the affected student participation data is requested. + * @param feedbackIds A list of feedback IDs used to filter the participation to only those affected by specific feedback entries. + * @param data A {@link PageableSearchDTO} object containing pagination and sorting parameters: + *
    + *
  • Page number and page size for pagination
  • + *
  • Sorting order and column for sorting (e.g., "participationId")
  • + *
+ * @return A {@link Page} of {@link FeedbackAffectedStudentDTO} objects, each representing a student affected by the feedback, with: + *
    + *
  • Details about the affected students, including participation data and student information.
  • + *
  • Total count of affected students, allowing for pagination on the client side.
  • + *
*/ - 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, true); + public Page getParticipationWithFeedbackId(long exerciseId, List feedbackIds, PageableSearchDTO data) { + List feedbackIdLongs = feedbackIds.stream().map(Long::valueOf).toList(); + PageRequest pageRequest = PageUtil.createDefaultPageRequest(data, PageUtil.ColumnMapping.AFFECTED_STUDENTS); + return studentParticipationRepository.findParticipationByFeedbackId(exerciseId, feedbackIdLongs, pageRequest); } } 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..6bc8bb2bc160 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; @@ -28,6 +29,7 @@ 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 +38,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 +331,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 +346,42 @@ 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. This supports efficient loading of affected students' data by returning only the required information. + *
+ * Supports pagination and sorting, allowing the client to control the amount and order of returned data: + *
    + *
  • Pagination: Controls page number and page size for the response.
  • + *
  • Sorting: Allows sorting by specified columns (e.g., "participationId") in ascending or descending order.
  • + *
+ * + * @param exerciseId The ID of the exercise for which the participation data is requested. + * @param feedbackIds A list of feedback IDs to filter affected students by specific feedback entries. + * @param data A {@link PageableSearchDTO} object containing pagination and sorting parameters: + *
    + *
  • Page number and page size
  • + *
  • Sorted column and sorting order
  • + *
+ * @return A {@link ResponseEntity} containing a {@link Page} of {@link FeedbackAffectedStudentDTO}, each representing a student affected by the feedback entries, including: + *
    + *
  • List of affected students with participation details.
  • + *
  • Total number of students affected (for pagination).
  • + *
+ */ + @GetMapping("exercises/{exerciseId}/feedback-details-participation") + @EnforceAtLeastEditorInExercise + public ResponseEntity> getParticipationWithFeedback(@PathVariable long exerciseId, @RequestParam List feedbackIds, + @ModelAttribute PageableSearchDTO data) { + Page participation = resultService.getParticipationWithFeedbackId(exerciseId, feedbackIds, 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 3681b3a94221..977433d27f4c 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( + GROUP_CONCAT(f.id), COUNT(f.id), 0, f.detailText, @@ -1329,4 +1331,46 @@ 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. + *
+ * This query joins `ProgrammingExerciseStudentParticipation`, `Submission`, `Result`, and `Feedback` entities to filter + * participation records based on feedback IDs and exercise ID. The results include information about each affected student, + * such as their course ID, participation ID, student details, and repository URI. + *
+ * Supports: + *
    + *
  • Pagination: The results are paginated using a {@link Pageable} parameter, allowing control over the page number and size.
  • + *
  • Sorting: The query sorts results by the student's first name in ascending order.
  • + *
+ * + * @param exerciseId The ID of the exercise for which the affected student participation data is requested. + * @param feedbackIds A list of feedback IDs 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, containing: + *
    + *
  • Course ID, participation ID, and student information (first name, last name, login).
  • + *
  • Repository URI, linking the affected student's repository.
  • + *
  • Total count of affected students to facilitate pagination on the client side.
  • + *
+ */ + @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 + JOIN s.results r + JOIN r.feedbacks f + WHERE p.exercise.id = :exerciseId + AND f.id IN :feedbackIds + ORDER BY p.student.firstName ASC + """) + Page findParticipationByFeedbackId(@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..89cf9c63ff83 --- /dev/null +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.html @@ -0,0 +1,42 @@ + + 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..cbf558d928d6 --- /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('There was an error while loading the affected Students for this feedback'); + } + } + + 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..2dc7abab2a8d 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,5 +1,5 @@ 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 { FilterData } from 'app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-filter-modal.component'; @@ -12,6 +12,7 @@ export interface FeedbackAnalysisResponse { errorCategories: string[]; } export interface FeedbackDetail { + concatenatedFeedbackIds: string[]; 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; + repositoryName: string; +} @Injectable() export class FeedbackAnalysisService extends BaseApiHttpService { search(pageable: SearchTermPageableSearch, options: { exerciseId: number; filters: FilterData }): Promise { @@ -39,4 +48,15 @@ export class FeedbackAnalysisService extends BaseApiHttpService { getMaxCount(exerciseId: number): Promise { return this.get(`exercises/${exerciseId}/feedback-details-max-count`); } + + async getParticipationForFeedbackIds(exerciseId: number, feedbackIds: string[], pageable: PageableSearch): Promise> { + const params = new HttpParams() + .set('feedbackIds', feedbackIds.join(',')) + .set('page', pageable.page.toString()) + .set('pageSize', pageable.pageSize.toString()) + .set('sortedColumn', pageable.sortedColumn) + .set('sortingOrder', pageable.sortingOrder); + + return this.get>(`exercises/${exerciseId}/feedback-details-participation`, { params }); + } } 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..1f1852efc9dc 100644 --- a/src/main/webapp/i18n/de/programmingExercise.json +++ b/src/main/webapp/i18n/de/programmingExercise.json @@ -371,6 +371,13 @@ "clear": "Filter zurücksetzen", "cancel": "Abbrechen", "apply": "Filter anwenden" + }, + "affectedStudentsModal": { + "header": "Betroffene Studierende", + "name": "Name", + "login": "Login", + "repository": "Repository", + "totalItems": "Insgesamt {{count}} Elemente" } }, "help": { diff --git a/src/main/webapp/i18n/en/programmingExercise.json b/src/main/webapp/i18n/en/programmingExercise.json index 244334cb61c8..0ce0db4fb6f5 100644 --- a/src/main/webapp/i18n/en/programmingExercise.json +++ b/src/main/webapp/i18n/en/programmingExercise.json @@ -371,6 +371,13 @@ "clear": "Clear Filter", "cancel": "Cancel", "apply": "Apply Filter" + }, + "affectedStudentsModal": { + "header": "Affected Students", + "name": "Name", + "login": "Login", + "repository": "Repository", + "totalItems": "In total {{count}} items" } }, "help": { 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..d20bddb3a978 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..87ca7272cb2c 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).toEqual(['1', '2']); + expect(result.feedbackDetails.resultsOnPage[1].concatenatedFeedbackIds).toEqual(['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?feedbackIds=1,2&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..16af4ecb8aec --- /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', repositoryName: 'repo1' }, + { courseId: 1, participationId: 102, firstName: 'Jane', lastName: 'Smith', login: 'janesmith', repositoryName: '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(alertServiceSpy).toHaveBeenCalledWith('There was an error while loading the affected Students for this feedback'); + expect(component.participation().content).toEqual([]); + 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..4de1fe16d168 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', From 862e9b0919e414faac59dbe553e952e70537897d Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Sun, 10 Nov 2024 22:01:10 +0100 Subject: [PATCH 02/64] fix --- .../assessment/service/ResultService.java | 91 +++++++++++++------ 1 file changed, 62 insertions(+), 29 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index f9640a69ee06..68fb02fbb821 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java @@ -12,6 +12,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import jakarta.annotation.Nullable; @@ -497,45 +498,49 @@ public Map getLogsAvailabilityForResults(List results, Par @NotNull private List saveFeedbackWithHibernateWorkaround(@NotNull Result result, List feedbackList) { - // Avoid hibernate exception List savedFeedbacks = new ArrayList<>(); - // Collect ids of feedbacks that have long feedback. - List feedbackIdsWithLongFeedback = feedbackList.stream().filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()).map(Feedback::getId) - .toList(); - // Get long feedback list from the database. - List longFeedbackTextList = longFeedbackTextRepository.findByFeedbackIds(feedbackIdsWithLongFeedback); - // Convert list to map for accessing later. - Map longLongFeedbackTextMap = longFeedbackTextList.stream() - .collect(Collectors.toMap(longFeedbackText -> longFeedbackText.getFeedback().getId(), longFeedbackText -> longFeedbackText)); - feedbackList.forEach(feedback -> { - // cut association to parent object - feedback.setResult(null); - LongFeedbackText longFeedback = null; - // look for long feedback that parent feedback has and cut the association between them. - if (feedback.getId() != null && feedback.getHasLongFeedbackText()) { - longFeedback = longLongFeedbackTextMap.get(feedback.getId()); - if (longFeedback != null) { - feedback.clearLongFeedback(); - } - } - // persist the child object without an association to the parent object. - feedback = feedbackRepository.saveAndFlush(feedback); - // restore the association to the parent object - feedback.setResult(result); + // Fetch long feedback texts associated with 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); } @@ -620,7 +625,6 @@ public FeedbackAnalysisResponseDTO getFeedbackDetailsOnPage(long exerciseId, Fee .map(detail -> new FeedbackDetailDTO(List.of(String.valueOf(detail.concatenatedFeedbackIds()).split(",")), 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"); @@ -672,4 +676,33 @@ public Page getParticipationWithFeedbackId(long exer PageRequest pageRequest = PageUtil.createDefaultPageRequest(data, PageUtil.ColumnMapping.AFFECTED_STUDENTS); return studentParticipationRepository.findParticipationByFeedbackId(exerciseId, feedbackIdLongs, pageRequest); } + + /** + * 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. + *

+ * **Note:** This method should only be used for manually assessed submissions, not for fully automatic assessments, due to its dependency on the + * {@link Result#updateAllFeedbackItems} method, which is designed for manual feedback management. Using this method with automatic assessments could + * lead to unintended behavior or data inconsistencies. + * + * @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, true); + } } From 8fb7be185785c5d7d2b12c9c826ff5e90f16cb8a Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Mon, 11 Nov 2024 17:02:20 +0100 Subject: [PATCH 03/64] johannes feedback --- .../artemis/assessment/dto/FeedbackDetailDTO.java | 2 +- .../artemis/assessment/service/ResultService.java | 12 ++++++------ .../aet/artemis/assessment/web/ResultResource.java | 2 +- .../repository/StudentParticipationRepository.java | 2 +- .../feedback-analysis/feedback-analysis.service.ts | 6 +++--- .../assessment/ResultServiceIntegrationTest.java | 1 + 6 files changed, 13 insertions(+), 12 deletions(-) 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 4b785d3d6b6a..312cbf66e876 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 @@ -3,5 +3,5 @@ import com.fasterxml.jackson.annotation.JsonInclude; @JsonInclude(JsonInclude.Include.NON_EMPTY) -public record FeedbackDetailDTO(Object concatenatedFeedbackIds, long count, double relativeCount, String detailText, String testCaseName, String taskName, String errorCategory) { +public record FeedbackDetailDTO(String concatenatedFeedbackIds, long count, double relativeCount, String detailText, String testCaseName, String taskName, String 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 68fb02fbb821..56c8b85b98ad 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; @@ -621,10 +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(List.of(String.valueOf(detail.concatenatedFeedbackIds()).split(",")), detail.count(), - (detail.count() * 100.00) / distinctResultCount, detail.detailText(), detail.testCaseName(), detail.taskName(), detail.errorCategory())) - .toList(); + 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"); @@ -671,8 +670,9 @@ public long getMaxCountForExercise(long exerciseId) { *

  • Total count of affected students, allowing for pagination on the client side.
  • * */ - public Page getParticipationWithFeedbackId(long exerciseId, List feedbackIds, PageableSearchDTO data) { - List feedbackIdLongs = feedbackIds.stream().map(Long::valueOf).toList(); + public Page getParticipationWithFeedbackId(long exerciseId, String feedbackIds, PageableSearchDTO data) { + List feedbackIdsList = Arrays.asList(feedbackIds.split(",")); + List feedbackIdLongs = feedbackIdsList.stream().map(Long::valueOf).toList(); PageRequest pageRequest = PageUtil.createDefaultPageRequest(data, PageUtil.ColumnMapping.AFFECTED_STUDENTS); return studentParticipationRepository.findParticipationByFeedbackId(exerciseId, feedbackIdLongs, pageRequest); } 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 6bc8bb2bc160..408b0abd16e0 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 @@ -379,7 +379,7 @@ public ResponseEntity getMaxCount(@PathVariable long exerciseId) { */ @GetMapping("exercises/{exerciseId}/feedback-details-participation") @EnforceAtLeastEditorInExercise - public ResponseEntity> getParticipationWithFeedback(@PathVariable long exerciseId, @RequestParam List feedbackIds, + public ResponseEntity> getParticipationWithFeedback(@PathVariable long exerciseId, @RequestParam String feedbackIds, @ModelAttribute PageableSearchDTO data) { Page participation = resultService.getParticipationWithFeedbackId(exerciseId, feedbackIds, data); return ResponseEntity.ok(participation); 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 977433d27f4c..cc87e3231af8 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 @@ -1234,7 +1234,7 @@ SELECT COALESCE(AVG(p.presentationScore), 0) */ @Query(""" SELECT new de.tum.cit.aet.artemis.assessment.dto.FeedbackDetailDTO( - GROUP_CONCAT(f.id), + CAST(GROUP_CONCAT(f.id) AS string), COUNT(f.id), 0, f.detailText, 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 2dc7abab2a8d..2742b4d811e1 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 @@ -12,7 +12,7 @@ export interface FeedbackAnalysisResponse { errorCategories: string[]; } export interface FeedbackDetail { - concatenatedFeedbackIds: string[]; + concatenatedFeedbackIds: string; count: number; relativeCount: number; detailText: string; @@ -49,9 +49,9 @@ export class FeedbackAnalysisService extends BaseApiHttpService { return this.get(`exercises/${exerciseId}/feedback-details-max-count`); } - async getParticipationForFeedbackIds(exerciseId: number, feedbackIds: string[], pageable: PageableSearch): Promise> { + async getParticipationForFeedbackIds(exerciseId: number, feedbackIds: string, pageable: PageableSearch): Promise> { const params = new HttpParams() - .set('feedbackIds', feedbackIds.join(',')) + .set('feedbackIds', feedbackIds) .set('page', pageable.page.toString()) .set('pageSize', pageable.pageSize.toString()) .set('sortedColumn', pageable.sortedColumn) 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..445bc7999f76 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 @@ -861,4 +861,5 @@ void testGetMaxCountForExerciseWithMultipleFeedback() throws Exception { assertThat(maxCount).isEqualTo(2); } + } From ae13615143ce4edf3cfa1a23c48556729eccecbc Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Mon, 11 Nov 2024 17:26:47 +0100 Subject: [PATCH 04/64] johannes feedback --- .../ResultServiceIntegrationTest.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) 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 445bc7999f76..da14e5a928ad 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 @@ -25,6 +25,9 @@ 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; @@ -862,4 +865,43 @@ 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); + + List feedbacks = feedbackRepository.findAll(); + System.out.println("Feedbacks in DB: " + feedbacks); + System.out.println("Using Feedback ID: " + feedback.getId()); + + String url = "/api/exercises/" + programmingExercise.getId() + + "/feedback-details-participation?page=1&pageSize=10&sortedColumn=participationId&sortingOrder=ASCENDING&feedbackIds=" + feedback.getId(); + String jsonResponse = request.get(url, HttpStatus.OK, String.class); + + 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(); + } + } From 15d3fba3fad76f3f8a77f507a3576cf29f6a1ab1 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Mon, 11 Nov 2024 17:44:01 +0100 Subject: [PATCH 05/64] johannes feedback --- .../exercise/repository/StudentParticipationRepository.java | 1 + 1 file changed, 1 insertion(+) 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 cc87e3231af8..c9682d8c9c58 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 @@ -1370,6 +1370,7 @@ SELECT MAX(pr.id) 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 findParticipationByFeedbackId(@Param("exerciseId") long exerciseId, @Param("feedbackIds") List feedbackIds, Pageable pageable); From 7f32c6f2636f02a05f687b02ba5609268759fad1 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Mon, 11 Nov 2024 18:11:12 +0100 Subject: [PATCH 06/64] tests fix --- .../artemis/assessment/ResultServiceIntegrationTest.java | 4 ---- .../feedback-analysis/feedback-analysis.component.spec.ts | 4 ++-- .../feedback-analysis/feedback-analysis.service.spec.ts | 8 ++++---- .../feedback-affected-students-modal.component.spec.ts | 2 +- .../modals/feedback-modal.component.spec.ts | 2 +- 5 files changed, 8 insertions(+), 12 deletions(-) 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 da14e5a928ad..4113841c7673 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 @@ -882,10 +882,6 @@ void testGetParticipationForFeedbackId() throws Exception { Result result = participationUtilService.addResultToParticipation(AssessmentType.AUTOMATIC, null, participation); participationUtilService.addFeedbackToResult(feedback, result); - List feedbacks = feedbackRepository.findAll(); - System.out.println("Feedbacks in DB: " + feedbacks); - System.out.println("Using Feedback ID: " + feedback.getId()); - String url = "/api/exercises/" + programmingExercise.getId() + "/feedback-details-participation?page=1&pageSize=10&sortedColumn=participationId&sortingOrder=ASCENDING&feedbackIds=" + feedback.getId(); String jsonResponse = request.get(url, HttpStatus.OK, String.class); 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 d20bddb3a978..f46725d0d394 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 @@ -19,7 +19,7 @@ describe('FeedbackAnalysisComponent', () => { const feedbackMock: FeedbackDetail[] = [ { - concatenatedFeedbackIds: ['1', '2'], + concatenatedFeedbackIds: '1,2', detailText: 'Test feedback 1 detail', testCaseName: 'test1', count: 10, @@ -28,7 +28,7 @@ describe('FeedbackAnalysisComponent', () => { errorCategory: 'Student Error', }, { - concatenatedFeedbackIds: ['3', '4'], + concatenatedFeedbackIds: '3,4', detailText: 'Test feedback 2 detail', testCaseName: 'test2', count: 5, 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 87ca7272cb2c..870e6437f4e3 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 @@ -10,7 +10,7 @@ describe('FeedbackAnalysisService', () => { const feedbackDetailsMock: FeedbackDetail[] = [ { - concatenatedFeedbackIds: ['1', '2'], + concatenatedFeedbackIds: '1, 2', detailText: 'Feedback 1', testCaseName: 'test1', count: 5, @@ -19,7 +19,7 @@ describe('FeedbackAnalysisService', () => { errorCategory: 'StudentError', }, { - concatenatedFeedbackIds: ['3', '4'], + concatenatedFeedbackIds: '3, 4', detailText: 'Feedback 2', testCaseName: 'test2', count: 3, @@ -70,8 +70,8 @@ describe('FeedbackAnalysisService', () => { const result = await responsePromise; expect(result).toEqual(feedbackAnalysisResponseMock); - expect(result.feedbackDetails.resultsOnPage[0].concatenatedFeedbackIds).toEqual(['1', '2']); - expect(result.feedbackDetails.resultsOnPage[1].concatenatedFeedbackIds).toEqual(['3', '4']); + expect(result.feedbackDetails.resultsOnPage[0].concatenatedFeedbackIds).toBe('1, 2'); + expect(result.feedbackDetails.resultsOnPage[1].concatenatedFeedbackIds).toBe('3, 4'); }); }); 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 index 16af4ecb8aec..b0e52c74866f 100644 --- 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 @@ -13,7 +13,7 @@ describe('AffectedStudentsModalComponent', () => { let feedbackService: FeedbackAnalysisService; const feedbackDetailMock: FeedbackDetail = { - concatenatedFeedbackIds: ['1', '2'], + concatenatedFeedbackIds: '1,2', count: 5, relativeCount: 25.0, detailText: 'Some feedback detail', 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 4de1fe16d168..fd07d43fb1f1 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,7 +10,7 @@ describe('FeedbackModalComponent', () => { let activeModal: NgbActiveModal; const mockFeedbackDetail: FeedbackDetail = { - concatenatedFeedbackIds: ['1', '2'], + concatenatedFeedbackIds: '1,2', count: 5, relativeCount: 25.0, detailText: 'Some feedback detail', From 3703ca00e4f5ea4094758d4642defebeb0e5085b Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Mon, 11 Nov 2024 18:29:12 +0100 Subject: [PATCH 07/64] johannes feedback --- .../cit/aet/artemis/assessment/service/ResultService.java | 3 +-- .../feedback-analysis/feedback-analysis.service.spec.ts | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index 56c8b85b98ad..90f9fabe77bd 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 @@ -671,8 +671,7 @@ public long getMaxCountForExercise(long exerciseId) { * */ public Page getParticipationWithFeedbackId(long exerciseId, String feedbackIds, PageableSearchDTO data) { - List feedbackIdsList = Arrays.asList(feedbackIds.split(",")); - List feedbackIdLongs = feedbackIdsList.stream().map(Long::valueOf).toList(); + List feedbackIdLongs = Arrays.stream(feedbackIds.split(",")).map(Long::valueOf).toList(); PageRequest pageRequest = PageUtil.createDefaultPageRequest(data, PageUtil.ColumnMapping.AFFECTED_STUDENTS); return studentParticipationRepository.findParticipationByFeedbackId(exerciseId, feedbackIdLongs, pageRequest); } 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 870e6437f4e3..e29b4737a6f4 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 @@ -90,7 +90,7 @@ describe('FeedbackAnalysisService', () => { describe('getParticipationForFeedbackIds', () => { it('should retrieve paginated participation details for specified feedback IDs', async () => { - const feedbackIds = ['1', '2']; + const feedbackIds = '1, 2'; const pageable = { page: 1, pageSize: 10, @@ -122,7 +122,9 @@ describe('FeedbackAnalysisService', () => { const responsePromise = service.getParticipationForFeedbackIds(1, feedbackIds, pageable); - const req = httpMock.expectOne('api/exercises/1/feedback-details-participation?feedbackIds=1,2&page=1&pageSize=10&sortedColumn=participationId&sortingOrder=ASCENDING'); + const req = httpMock.expectOne( + 'api/exercises/1/feedback-details-participation?feedbackIds=1,%202&page=1&pageSize=10&sortedColumn=participationId&sortingOrder=ASCENDING', + ); expect(req.request.method).toBe('GET'); req.flush(participationResponseMock); From 3b32032198b8292f0189bb945d48c12b4fe6c983 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 12 Nov 2024 08:21:34 +0100 Subject: [PATCH 08/64] bilel feedback --- .../exercise/repository/StudentParticipationRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c9682d8c9c58..2d15d50cad15 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 @@ -1234,7 +1234,7 @@ SELECT COALESCE(AVG(p.presentationScore), 0) */ @Query(""" SELECT new de.tum.cit.aet.artemis.assessment.dto.FeedbackDetailDTO( - CAST(GROUP_CONCAT(f.id) AS string), + LISTAGG(CAST(f.id AS string), ',') WITHIN GROUP (ORDER BY f.id), COUNT(f.id), 0, f.detailText, From f0b99a34aa3eb8f8258869f96c1d0bb68c6515c4 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 12 Nov 2024 13:38:57 +0100 Subject: [PATCH 09/64] florian feedback --- .../dto/FeedbackAffectedStudentDTO.java | 2 +- .../assessment/dto/FeedbackDetailDTO.java | 11 ++++- .../assessment/service/ResultService.java | 28 ++++--------- .../assessment/web/ResultResource.java | 28 ++++--------- .../StudentParticipationRepository.java | 40 ++++++------------- ...ack-affected-students-modal.component.html | 2 +- ...dback-affected-students-modal.component.ts | 2 +- .../feedback-analysis.service.ts | 8 ++-- .../webapp/i18n/de/programmingExercise.json | 3 +- .../webapp/i18n/en/programmingExercise.json | 3 +- .../feedback-analysis.component.spec.ts | 4 +- ...-affected-students-modal.component.spec.ts | 6 +-- .../modals/feedback-modal.component.spec.ts | 2 +- 13 files changed, 55 insertions(+), 84 deletions(-) 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 index 76dc1e2deb72..71c6b73a208f 100644 --- 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 @@ -1,4 +1,4 @@ package de.tum.cit.aet.artemis.assessment.dto; -public record FeedbackAffectedStudentDTO(long courseId, long participationId, String firstName, String lastName, String login, String repositoryName) { +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 312cbf66e876..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(String concatenatedFeedbackIds, 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 90f9fabe77bd..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 @@ -649,31 +649,19 @@ public long getMaxCountForExercise(long 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 to allow efficient retrieval and sorting of the results, thus supporting large datasets. + * 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. *
    - * Supports: - *
      - *
    • Pagination: Controls the page number and page size for the returned results.
    • - *
    • Sorting: Applies sorting by specified columns and sorting order based on the {@link PageUtil.ColumnMapping#AFFECTED_STUDENTS} configuration.
    • - *
    * - * @param exerciseId The ID of the exercise for which the affected student participation data is requested. - * @param feedbackIds A list of feedback IDs used to filter the participation to only those affected by specific feedback entries. - * @param data A {@link PageableSearchDTO} object containing pagination and sorting parameters: - *
      - *
    • Page number and page size for pagination
    • - *
    • Sorting order and column for sorting (e.g., "participationId")
    • - *
    - * @return A {@link Page} of {@link FeedbackAffectedStudentDTO} objects, each representing a student affected by the feedback, with: - *
      - *
    • Details about the affected students, including participation data and student information.
    • - *
    • Total count of affected students, allowing for pagination on the client side.
    • - *
    + * @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 getParticipationWithFeedbackId(long exerciseId, String feedbackIds, PageableSearchDTO data) { + 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.findParticipationByFeedbackId(exerciseId, feedbackIdLongs, pageRequest); + return studentParticipationRepository.findAffectedStudentsByFeedbackId(exerciseId, feedbackIdLongs, pageRequest); } /** 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 408b0abd16e0..d4cf9b20f684 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 @@ -354,34 +354,20 @@ public ResponseEntity getMaxCount(@PathVariable long exerciseId) { /** * 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. This supports efficient loading of affected students' data by returning only the required information. + * and participation details. *
    - * Supports pagination and sorting, allowing the client to control the amount and order of returned data: - *
      - *
    • Pagination: Controls page number and page size for the response.
    • - *
    • Sorting: Allows sorting by specified columns (e.g., "participationId") in ascending or descending order.
    • - *
    * - * @param exerciseId The ID of the exercise for which the participation data is requested. - * @param feedbackIds A list of feedback IDs to filter affected students by specific feedback entries. - * @param data A {@link PageableSearchDTO} object containing pagination and sorting parameters: - *
      - *
    • Page number and page size
    • - *
    • Sorted column and sorting order
    • - *
    - * @return A {@link ResponseEntity} containing a {@link Page} of {@link FeedbackAffectedStudentDTO}, each representing a student affected by the feedback entries, including: - *
      - *
    • List of affected students with participation details.
    • - *
    • Total number of students affected (for pagination).
    • - *
    + * @param exerciseId for which the participation data is requested. + * @param feedbackIds 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> getParticipationWithFeedback(@PathVariable long exerciseId, @RequestParam String feedbackIds, + public ResponseEntity> getAffectedStudentsWithFeedback(@PathVariable long exerciseId, @RequestParam String feedbackIds, @ModelAttribute PageableSearchDTO data) { - Page participation = resultService.getParticipationWithFeedbackId(exerciseId, feedbackIds, data); + Page participation = resultService.getAffectedStudentsWithFeedbackId(exerciseId, feedbackIds, data); return ResponseEntity.ok(participation); } } 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 2d15d50cad15..66728627e4fc 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 @@ -1242,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 @@ -1252,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 @@ -1266,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 @@ -1292,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 @@ -1318,12 +1318,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 @@ -1335,25 +1335,11 @@ SELECT MAX(pr.id) /** * Retrieves a paginated list of students affected by specific feedback entries for a given programming exercise. *
    - * This query joins `ProgrammingExerciseStudentParticipation`, `Submission`, `Result`, and `Feedback` entities to filter - * participation records based on feedback IDs and exercise ID. The results include information about each affected student, - * such as their course ID, participation ID, student details, and repository URI. - *
    - * Supports: - *
      - *
    • Pagination: The results are paginated using a {@link Pageable} parameter, allowing control over the page number and size.
    • - *
    • Sorting: The query sorts results by the student's first name in ascending order.
    • - *
    * - * @param exerciseId The ID of the exercise for which the affected student participation data is requested. - * @param feedbackIds A list of feedback IDs used to filter the participation to only those affected by specific feedback entries. + * @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, containing: - *
      - *
    • Course ID, participation ID, and student information (first name, last name, login).
    • - *
    • Repository URI, linking the affected student's repository.
    • - *
    • Total count of affected students to facilitate pagination on the client side.
    • - *
    + * @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( @@ -1366,12 +1352,12 @@ SELECT MAX(pr.id) ) FROM ProgrammingExerciseStudentParticipation p LEFT JOIN p.submissions s - JOIN s.results r - JOIN r.feedbacks f + 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 findParticipationByFeedbackId(@Param("exerciseId") long exerciseId, @Param("feedbackIds") List feedbackIds, Pageable pageable); + 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 index 89cf9c63ff83..d5b4da2e12ed 100644 --- 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 @@ -25,7 +25,7 @@ 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 index cbf558d928d6..947f3f3d44cd 100644 --- 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 @@ -48,7 +48,7 @@ export class AffectedStudentsModalComponent { const response = await this.feedbackService.getParticipationForFeedbackIds(this.exerciseId(), feedbackDetail.concatenatedFeedbackIds, pageable); this.participation.set(response); } catch (error) { - this.alertService.error('There was an error while loading the affected Students for this feedback'); + this.alertService.error(this.TRANSLATION_BASE + '.error'); } } 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 2742b4d811e1..a52e259d5b03 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 @@ -12,7 +12,7 @@ export interface FeedbackAnalysisResponse { errorCategories: string[]; } export interface FeedbackDetail { - concatenatedFeedbackIds: string; + concatenatedFeedbackIds: number[]; count: number; relativeCount: number; detailText: string; @@ -26,7 +26,7 @@ export interface FeedbackAffectedStudentDTO { firstName: string; lastName: string; login: string; - repositoryName: string; + repositoryURI: string; } @Injectable() export class FeedbackAnalysisService extends BaseApiHttpService { @@ -49,9 +49,9 @@ export class FeedbackAnalysisService extends BaseApiHttpService { return this.get(`exercises/${exerciseId}/feedback-details-max-count`); } - async getParticipationForFeedbackIds(exerciseId: number, feedbackIds: string, pageable: PageableSearch): Promise> { + async getParticipationForFeedbackIds(exerciseId: number, feedbackIds: number[], pageable: PageableSearch): Promise> { const params = new HttpParams() - .set('feedbackIds', feedbackIds) + .set('feedbackIds', feedbackIds.join(',')) .set('page', pageable.page.toString()) .set('pageSize', pageable.pageSize.toString()) .set('sortedColumn', pageable.sortedColumn) diff --git a/src/main/webapp/i18n/de/programmingExercise.json b/src/main/webapp/i18n/de/programmingExercise.json index 1f1852efc9dc..ae9d8d88ce7f 100644 --- a/src/main/webapp/i18n/de/programmingExercise.json +++ b/src/main/webapp/i18n/de/programmingExercise.json @@ -377,7 +377,8 @@ "name": "Name", "login": "Login", "repository": "Repository", - "totalItems": "Insgesamt {{count}} Elemente" + "totalItems": "Insgesamt {{count}} Elemente", + "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 0ce0db4fb6f5..fc73759dd829 100644 --- a/src/main/webapp/i18n/en/programmingExercise.json +++ b/src/main/webapp/i18n/en/programmingExercise.json @@ -377,7 +377,8 @@ "name": "Name", "login": "Login", "repository": "Repository", - "totalItems": "In total {{count}} items" + "totalItems": "In total {{count}} items", + "error": "There was an error while loading the affected Students for this feedback." } }, "help": { 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 f46725d0d394..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 @@ -19,7 +19,7 @@ describe('FeedbackAnalysisComponent', () => { const feedbackMock: FeedbackDetail[] = [ { - concatenatedFeedbackIds: '1,2', + concatenatedFeedbackIds: [1, 2], detailText: 'Test feedback 1 detail', testCaseName: 'test1', count: 10, @@ -28,7 +28,7 @@ describe('FeedbackAnalysisComponent', () => { errorCategory: 'Student Error', }, { - concatenatedFeedbackIds: '3,4', + concatenatedFeedbackIds: [3, 4], detailText: 'Test feedback 2 detail', testCaseName: 'test2', count: 5, 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 index b0e52c74866f..457bc8be7085 100644 --- 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 @@ -13,7 +13,7 @@ describe('AffectedStudentsModalComponent', () => { let feedbackService: FeedbackAnalysisService; const feedbackDetailMock: FeedbackDetail = { - concatenatedFeedbackIds: '1,2', + concatenatedFeedbackIds: [1, 2], count: 5, relativeCount: 25.0, detailText: 'Some feedback detail', @@ -23,8 +23,8 @@ describe('AffectedStudentsModalComponent', () => { }; const participationMock: FeedbackAffectedStudentDTO[] = [ - { courseId: 1, participationId: 101, firstName: 'John', lastName: 'Doe', login: 'johndoe', repositoryName: 'repo1' }, - { courseId: 1, participationId: 102, firstName: 'Jane', lastName: 'Smith', login: 'janesmith', repositoryName: 'repo2' }, + { 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 () => { 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 fd07d43fb1f1..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,7 +10,7 @@ describe('FeedbackModalComponent', () => { let activeModal: NgbActiveModal; const mockFeedbackDetail: FeedbackDetail = { - concatenatedFeedbackIds: '1,2', + concatenatedFeedbackIds: [1, 2], count: 5, relativeCount: 25.0, detailText: 'Some feedback detail', From 4d5a609620ddc5b7b8e639c76289d59cd1951cfa Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 12 Nov 2024 13:56:26 +0100 Subject: [PATCH 10/64] client test feedback --- .../feedback-analysis.service.spec.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) 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 e29b4737a6f4..326c0de0eaa2 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 @@ -10,7 +10,7 @@ describe('FeedbackAnalysisService', () => { const feedbackDetailsMock: FeedbackDetail[] = [ { - concatenatedFeedbackIds: '1, 2', + concatenatedFeedbackIds: [1, 2], detailText: 'Feedback 1', testCaseName: 'test1', count: 5, @@ -19,7 +19,7 @@ describe('FeedbackAnalysisService', () => { errorCategory: 'StudentError', }, { - concatenatedFeedbackIds: '3, 4', + concatenatedFeedbackIds: [3, 4], detailText: 'Feedback 2', testCaseName: 'test2', count: 3, @@ -70,8 +70,8 @@ describe('FeedbackAnalysisService', () => { const result = await responsePromise; expect(result).toEqual(feedbackAnalysisResponseMock); - expect(result.feedbackDetails.resultsOnPage[0].concatenatedFeedbackIds).toBe('1, 2'); - expect(result.feedbackDetails.resultsOnPage[1].concatenatedFeedbackIds).toBe('3, 4'); + expect(result.feedbackDetails.resultsOnPage[0].concatenatedFeedbackIds).toStrictEqual([1, 2]); + expect(result.feedbackDetails.resultsOnPage[1].concatenatedFeedbackIds).toStrictEqual([3, 4]); }); }); @@ -90,7 +90,7 @@ describe('FeedbackAnalysisService', () => { describe('getParticipationForFeedbackIds', () => { it('should retrieve paginated participation details for specified feedback IDs', async () => { - const feedbackIds = '1, 2'; + const feedbackIds = [1, 2]; const pageable = { page: 1, pageSize: 10, @@ -122,9 +122,7 @@ describe('FeedbackAnalysisService', () => { const responsePromise = service.getParticipationForFeedbackIds(1, feedbackIds, pageable); - const req = httpMock.expectOne( - 'api/exercises/1/feedback-details-participation?feedbackIds=1,%202&page=1&pageSize=10&sortedColumn=participationId&sortingOrder=ASCENDING', - ); + const req = httpMock.expectOne('api/exercises/1/feedback-details-participation?feedbackIds=1,2&page=1&pageSize=10&sortedColumn=participationId&sortingOrder=ASCENDING'); expect(req.request.method).toBe('GET'); req.flush(participationResponseMock); From cfd94420c8851f2954159e11534bc4fbe94cbe73 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 12 Nov 2024 14:17:55 +0100 Subject: [PATCH 11/64] client test feedback --- .../modals/feedback-affected-students-modal.component.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 457bc8be7085..3b991c19ef34 100644 --- 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 @@ -66,7 +66,7 @@ describe('AffectedStudentsModalComponent', () => { await component['loadAffected'](); - expect(alertServiceSpy).toHaveBeenCalledWith('There was an error while loading the affected Students for this feedback'); + expect(alertServiceSpy).toHaveBeenCalledTimes(2); expect(component.participation().content).toEqual([]); expect(consoleErrorSpy).toHaveBeenCalled(); From 30ebcb7cfc1b5db24aa3693c715ee89f3cccee45 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 12 Nov 2024 15:33:52 +0100 Subject: [PATCH 12/64] client test feedback --- .../modals/feedback-affected-students-modal.component.spec.ts | 2 -- 1 file changed, 2 deletions(-) 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 index 3b991c19ef34..6f80ebac01e2 100644 --- 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 @@ -61,12 +61,10 @@ describe('AffectedStudentsModalComponent', () => { 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(alertServiceSpy).toHaveBeenCalledTimes(2); expect(component.participation().content).toEqual([]); expect(consoleErrorSpy).toHaveBeenCalled(); From 72b482e74a094e072c10f28aa0abbdb33df7be83 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 12 Nov 2024 16:14:50 +0100 Subject: [PATCH 13/64] client test --- .../modals/feedback-affected-students-modal.component.spec.ts | 2 ++ 1 file changed, 2 insertions(+) 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 index 6f80ebac01e2..bf8b1b77141a 100644 --- 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 @@ -61,11 +61,13 @@ describe('AffectedStudentsModalComponent', () => { 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(); From be2876929a65bd32514d4771c915b28f1ce4d19e Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Thu, 14 Nov 2024 19:01:41 +0100 Subject: [PATCH 14/64] ramona feedback --- ...eedback-affected-students-modal.component.html | 15 +++++++++++---- src/main/webapp/i18n/de/programmingExercise.json | 4 ++-- src/main/webapp/i18n/en/programmingExercise.json | 4 ++-- 3 files changed, 15 insertions(+), 8 deletions(-) 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 index d5b4da2e12ed..4f53792a04e5 100644 --- 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 @@ -1,3 +1,8 @@ + + + + +
    - + @if (form.get('description')?.invalid && (form.get('description')?.dirty || form.get('description')?.touched)) {
    diff --git a/src/main/webapp/i18n/de/programmingExercise.json b/src/main/webapp/i18n/de/programmingExercise.json index 4a2624f57f1e..331efb6f84cb 100644 --- a/src/main/webapp/i18n/de/programmingExercise.json +++ b/src/main/webapp/i18n/de/programmingExercise.json @@ -399,6 +399,7 @@ "createAndNavigateButton": "Erstellen und Navigieren", "createChannelButton": "Kanal erstellen", "placeholder": "Dieses Feld bitte nicht leer lassen", + "descriptionLabel": "Kanalbeschreibung", "confirmationModal": { "header": "Kanal Erstellen bestätigen", "confirmationMessage": "Sind Sie sicher, dass Sie diesen Kanal erstellen möchten? Dies wird einen Kanal mit {{count}} Student(en) erstellen.", diff --git a/src/main/webapp/i18n/en/programmingExercise.json b/src/main/webapp/i18n/en/programmingExercise.json index d493792af005..7828988a742b 100644 --- a/src/main/webapp/i18n/en/programmingExercise.json +++ b/src/main/webapp/i18n/en/programmingExercise.json @@ -399,6 +399,7 @@ "createAndNavigateButton": "Create and Navigate", "createChannelButton": "Create Channel", "placeholder": "This field should not be empty", + "descriptionLabel": "Description", "confirmationModal": { "header": "Confirm Channel Creation", "confirmationMessage": "Are you sure you want to create this channel? This will create a channel with {{count}} student(s).", diff --git a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/confirm-feedback-channel-creation-modal.component.spec.ts b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/confirm-feedback-channel-creation-modal.component.spec.ts index e3cc77fdbd09..5c59597134d6 100644 --- a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/confirm-feedback-channel-creation-modal.component.spec.ts +++ b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/confirm-feedback-channel-creation-modal.component.spec.ts @@ -28,12 +28,12 @@ describe('ConfirmFeedbackChannelCreationModalComponent', () => { it('should call close on activeModal with true when confirm is triggered', () => { const closeSpy = jest.spyOn(activeModal, 'close'); component.confirm(); - expect(closeSpy).toHaveBeenCalledWith(true); + expect(closeSpy).toHaveBeenCalledExactlyOnceWith(true); }); it('should call dismiss on activeModal when dismiss is triggered', () => { const dismissSpy = jest.spyOn(activeModal, 'dismiss'); component.dismiss(); - expect(dismissSpy).toHaveBeenCalled(); + expect(dismissSpy).toHaveBeenCalledOnce(); }); }); diff --git a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-detail-channel-modal.component.spec.ts b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-detail-channel-modal.component.spec.ts index 8842a307462b..74928d400dd3 100644 --- a/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-detail-channel-modal.component.spec.ts +++ b/src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-detail-channel-modal.component.spec.ts @@ -45,13 +45,13 @@ describe('FeedbackDetailChannelModalComponent', () => { it('should call activeModal.close when closeModal is triggered', () => { const closeSpy = jest.spyOn(activeModal, 'close'); component.closeModal(); - expect(closeSpy).toHaveBeenCalled(); + expect(closeSpy).toHaveBeenCalledOnce(); }); it('should call activeModal.dismiss when dismissModal is triggered', () => { const dismissSpy = jest.spyOn(activeModal, 'dismiss'); component.dismissModal(); - expect(dismissSpy).toHaveBeenCalled(); + expect(dismissSpy).toHaveBeenCalledOnce(); }); it('should open confirmation modal and emit formSubmitted on successful confirmation', async () => { @@ -68,7 +68,7 @@ describe('FeedbackDetailChannelModalComponent', () => { await component.submitForm(false); expect(component.isConfirmModalOpen()).toBeFalse(); - expect(formSubmittedSpy).toHaveBeenCalledWith({ + expect(formSubmittedSpy).toHaveBeenCalledExactlyOnceWith({ channelDto: expect.objectContaining({ creationDate: undefined, creator: undefined, @@ -115,8 +115,8 @@ describe('FeedbackDetailChannelModalComponent', () => { await component.submitForm(false); - expect(component.handleModal).toHaveBeenCalled(); - expect(formSubmittedSpy).toHaveBeenCalledWith({ + expect(component.handleModal).toHaveBeenCalledOnce(); + expect(formSubmittedSpy).toHaveBeenCalledExactlyOnceWith({ channelDto: expect.objectContaining({ name: 'channel', description: 'channelDescription', @@ -141,13 +141,13 @@ describe('FeedbackDetailChannelModalComponent', () => { await component.submitForm(false); - expect(component.handleModal).toHaveBeenCalled(); - expect(formSubmittedSpy).not.toHaveBeenCalled(); + expect(component.handleModal).toHaveBeenCalledOnce(); + expect(formSubmittedSpy).not.toHaveBeenCalledOnce(); }); it('should not open confirmation modal if form is invalid', async () => { const modalSpy = jest.spyOn(modalService, 'open'); await component.submitForm(true); - expect(modalSpy).not.toHaveBeenCalled(); + expect(modalSpy).not.toHaveBeenCalledOnce(); }); }); From 7233d3cdfc32c403a6c37f9d7f8881dc75e4fb45 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 19 Nov 2024 13:06:19 +0100 Subject: [PATCH 24/64] flo feedback --- .../assessment/service/ResultService.java | 8 ++--- .../service/conversation/ChannelService.java | 19 ++++------- .../web/conversation/ChannelResource.java | 19 ++++------- ...edback-channel-creation-modal.component.ts | 7 ++-- ...edback-detail-channel-modal.component.html | 13 ++++--- ...feedback-detail-channel-modal.component.ts | 26 ++++++-------- .../webapp/i18n/de/programmingExercise.json | 5 +++ .../webapp/i18n/en/programmingExercise.json | 5 +++ .../communication/ChannelIntegrationTest.java | 34 +++++++++++++++++++ 9 files changed, 82 insertions(+), 54 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java b/src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java index 56357dd9f850..1f62ef78665b 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 @@ -695,13 +695,9 @@ public void deleteLongFeedback(List feedbackList, Result result) { /** * Retrieves the number of students affected by a specific feedback detail text for a given exercise. - *

    - * This method queries the repository to count the distinct students whose submissions were impacted - * by feedback entries matching the provided detail text within the specified exercise. - *

    * - * @param exerciseId the ID of the exercise for which the affected student count is requested. - * @param detailText the feedback detail text used to filter affected students. + * @param exerciseId for which the affected student count is requested. + * @param detailText used to filter affected students. * @return the total number of distinct students affected by the feedback detail text. */ public long getAffectedStudentCountByFeedbackDetailText(long exerciseId, String detailText) { diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java index c637691d63a8..791847b75670 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/conversation/ChannelService.java @@ -417,22 +417,17 @@ private static String generateChannelNameFromTitle(@NotNull String prefix, Optio /** * Creates a feedback-specific channel for an exercise within a course. - *

    - * This method sets up a new channel and adds affected students based on the provided feedback detail text. - * It validates the channel name, creates the channel, and registers affected students as members. Additionally, - * it sends notifications to the added users about the new channel. - *

    * - * @param course the course in which the channel is being created. - * @param exerciseId the ID of the exercise associated with the feedback channel. - * @param channelDTO the DTO containing the properties of the channel to be created, such as name, description, and visibility. - * @param feedbackDetailText the feedback detail text used to identify the students affected by the feedback. - * @param requestingUser the user initiating the channel creation request. + * @param course in which the channel is being created. + * @param exerciseId of the exercise associated with the feedback channel. + * @param channelDTO containing the properties of the channel to be created, such as name, description, and visibility. + * @param feedbackDetailText used to identify the students affected by the feedback. + * @param requestingUser initiating the channel creation request. * @return the created {@link Channel} object with its properties. * @throws BadRequestAlertException if the channel name starts with an invalid prefix (e.g., "$"). */ public Channel createFeedbackChannel(Course course, Long exerciseId, ChannelDTO channelDTO, String feedbackDetailText, User requestingUser) { - var channelToCreate = new Channel(); + Channel channelToCreate = new Channel(); channelToCreate.setName(channelDTO.getName()); channelToCreate.setIsPublic(channelDTO.getIsPublic()); channelToCreate.setIsAnnouncementChannel(channelDTO.getIsAnnouncementChannel()); @@ -443,7 +438,7 @@ public Channel createFeedbackChannel(Course course, Long exerciseId, ChannelDTO throw new BadRequestAlertException("User generated channels cannot start with $", "channel", "channelNameInvalid"); } - var createdChannel = createChannel(course, channelToCreate, Optional.of(requestingUser)); + Channel createdChannel = createChannel(course, channelToCreate, Optional.of(requestingUser)); List userLogins = studentParticipationRepository.findAffectedLoginsByFeedbackDetailText(exerciseId, feedbackDetailText); diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java b/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java index b466ee39d3d1..6e5c1b310f9c 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java @@ -43,6 +43,7 @@ import de.tum.cit.aet.artemis.communication.service.conversation.ConversationService; import de.tum.cit.aet.artemis.communication.service.conversation.auth.ChannelAuthorizationService; import de.tum.cit.aet.artemis.communication.service.notifications.SingleUserNotificationService; +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.exception.AccessForbiddenAlertException; import de.tum.cit.aet.artemis.core.exception.BadRequestAlertException; @@ -469,14 +470,10 @@ public ResponseEntity deregisterUsers(@PathVariable Long courseId, @PathVa /** * POST /api/courses/:courseId/channels/: Creates a new feedback-specific channel in a course. - *

    - * This endpoint allows authorized users to create a new channel within a course that is specifically designed for discussions - * around a particular exercise's feedback. The channel is populated with all affected students based on the provided feedback detail text. - *

    * - * @param courseId the ID of the course where the channel is being created. - * @param exerciseId the ID of the exercise for which the feedback channel is being created. - * @param channelDTO the DTO containing the properties of the channel to be created, such as name, description, and visibility. + * @param courseId where the channel is being created. + * @param exerciseId for which the feedback channel is being created. + * @param channelDTO containing the properties of the channel to be created, such as name, description, and visibility. * @param feedbackDetailText a string representing the feedback detail text used to determine the affected students to be added to the channel. * @return ResponseEntity with status 201 (Created) and the body containing the details of the created channel. * @throws URISyntaxException if the URI for the created resource cannot be constructed. @@ -488,13 +485,11 @@ public ResponseEntity createFeedbackChannel(@PathVariable Long cours @RequestHeader("feedback-detail-text") String feedbackDetailText) throws URISyntaxException { log.debug("REST request to create feedback channel in course {} with properties: {}", courseId, channelDTO); - var requestingUser = userRepository.getUserWithGroupsAndAuthorities(); - var course = courseRepository.findByIdElseThrow(courseId); - + User requestingUser = userRepository.getUserWithGroupsAndAuthorities(); + Course course = courseRepository.findByIdElseThrow(courseId); checkCommunicationEnabledElseThrow(course); channelAuthorizationService.isAllowedToCreateChannel(course, requestingUser); - - var createdChannel = channelService.createFeedbackChannel(course, exerciseId, channelDTO, feedbackDetailText, requestingUser); + Channel createdChannel = channelService.createFeedbackChannel(course, exerciseId, channelDTO, feedbackDetailText, requestingUser); return ResponseEntity.created(new URI("/api/channels/" + createdChannel.getId())).body(conversationDTOService.convertChannelToDTO(requestingUser, createdChannel)); } diff --git a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/confirm-feedback-channel-creation-modal.component.ts b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/confirm-feedback-channel-creation-modal.component.ts index 58d0471a844f..352a0a657acc 100644 --- a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/confirm-feedback-channel-creation-modal.component.ts +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/confirm-feedback-channel-creation-modal.component.ts @@ -1,4 +1,4 @@ -import { Component, input } from '@angular/core'; +import { Component, inject, input } from '@angular/core'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; import { ArtemisSharedCommonModule } from 'app/shared/shared-common.module'; @@ -9,10 +9,9 @@ import { ArtemisSharedCommonModule } from 'app/shared/shared-common.module'; standalone: true, }) export class ConfirmFeedbackChannelCreationModalComponent { - affectedStudentsCount = input.required(); readonly TRANSLATION_BASE = 'artemisApp.programmingExercise.configureGrading.feedbackAnalysis.feedbackDetailChannel.confirmationModal'; - - constructor(private activeModal: NgbActiveModal) {} + affectedStudentsCount = input.required(); + private activeModal = inject(NgbActiveModal); confirm(): void { this.activeModal.close(true); diff --git a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html index a98d2b393eaf..e78a2b72e145 100644 --- a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html @@ -15,13 +15,13 @@
    - @if (programmingExercise.isAtLeastEditor && activeTab === 'feedback-analysis') { + @if (programmingExercise.isAtLeastEditor && activeTab === 'feedback-analysis' && programmingExercise.title && programmingExercise.id && course.id) { } diff --git a/src/main/webapp/app/exercises/programming/manage/grading/programming-exercise-configure-grading.component.ts b/src/main/webapp/app/exercises/programming/manage/grading/programming-exercise-configure-grading.component.ts index bb4afd12ab7c..13ce8237d382 100644 --- a/src/main/webapp/app/exercises/programming/manage/grading/programming-exercise-configure-grading.component.ts +++ b/src/main/webapp/app/exercises/programming/manage/grading/programming-exercise-configure-grading.component.ts @@ -92,6 +92,7 @@ export class ProgrammingExerciseConfigureGradingComponent implements OnInit, OnD readonly RESET_TABLE = ProgrammingGradingChartsDirective.RESET_TABLE; readonly chartFilterType = ChartFilterType; readonly ProgrammingLanguage = ProgrammingLanguage; + protected readonly isCommunicationEnabled = isCommunicationEnabled; // We have to separate these test cases in order to separate the table and chart presentation if the table is filtered by the chart staticCodeAnalysisCategoriesForTable: StaticCodeAnalysisCategory[] = []; @@ -788,6 +789,4 @@ export class ProgrammingExerciseConfigureGradingComponent implements OnInit, OnD this.staticCodeAnalysisCategoriesForCharts = this.staticCodeAnalysisCategoriesForTable; this.backupStaticCodeAnalysisCategories = this.staticCodeAnalysisCategoriesForTable; } - - protected readonly isCommunicationEnabled = isCommunicationEnabled; } diff --git a/src/main/webapp/i18n/de/programmingExercise.json b/src/main/webapp/i18n/de/programmingExercise.json index 047baac81211..feed824a7254 100644 --- a/src/main/webapp/i18n/de/programmingExercise.json +++ b/src/main/webapp/i18n/de/programmingExercise.json @@ -357,7 +357,7 @@ "filter": "Filter", "noData": "Es konnten keine Feedback Einträge für die Programmieraufgabe gefunden werden.", "noDataFilter": "Für den spezifizierten Filter oder Suchbegriff konnten keine Feedback Einträge gefunden werden.", - "channelSuccess": "Kanal erfolgreich erstellt.", + "channelSuccess": "Kanal {{channelName}} erfolgreich erstellt.", "feedbackModal": { "header": "Fehlerdetails", "feedbackTitle": "Feedback zu Testfällen", diff --git a/src/main/webapp/i18n/en/programmingExercise.json b/src/main/webapp/i18n/en/programmingExercise.json index 3d641b5c6f24..cba315f1ea34 100644 --- a/src/main/webapp/i18n/en/programmingExercise.json +++ b/src/main/webapp/i18n/en/programmingExercise.json @@ -357,7 +357,7 @@ "filter": "Filters", "noData": "No Feedback Entries could be found for the Programming Exercise.", "noDataFilter": "No Feedback Entries could be found for the specified filter or search Term.", - "channelSuccess": "Channel successfully created.", + "channelSuccess": "Channel {{channelName}} successfully created.", "feedbackModal": { "header": "Error Details", "feedbackTitle": "Test Case Feedback", diff --git a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java index 8f3b01dadb9f..c31d2fd4fdd4 100644 --- a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java @@ -937,9 +937,8 @@ void createFeedbackChannel_asStudent_shouldReturnForbidden() { @Test @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void createFeedbackChannel_asInstructor_shouldCreateChannel() { - // Given - Long courseId = 1L; // Existing course ID - Long exerciseId = 1L; // Existing exercise ID + Long courseId = 1L; + Long exerciseId = 1L; ChannelDTO channelDTO = new ChannelDTO(); channelDTO.setName("feedback-channel"); channelDTO.setDescription("Discussion channel for feedback"); @@ -950,18 +949,16 @@ void createFeedbackChannel_asInstructor_shouldCreateChannel() { headers.set("feedback-detail-text", "Sample feedback text"); String BASE_ENDPOINT = "/api/courses/{courseId}/{exerciseId}/feedback-channel"; - // When + ChannelDTO response = null; try { response = request.postWithResponseBody(BASE_ENDPOINT.replace("{courseId}", courseId.toString()).replace("{exerciseId}", exerciseId.toString()), channelDTO, - ChannelDTO.class, HttpStatus.OK, // Expecting 200 OK here instead of 201 Created - headers); + ChannelDTO.class, HttpStatus.CREATED, headers); } catch (Exception e) { Assertions.fail("Failed to create feedback channel", e); } - // Then assertThat(response).isNotNull(); assertThat(response.getName()).isEqualTo("feedback-channel"); assertThat(response.getDescription()).isEqualTo("Discussion channel for feedback"); From bf0611efbb18fc6f74597ebd90cedb5134864fef Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 19 Nov 2024 13:45:10 +0100 Subject: [PATCH 26/64] flo feedback --- .../confirm-feedback-channel-creation-modal.component.ts | 2 +- .../Modal/feedback-detail-channel-modal.component.html | 5 +---- .../feedback-analysis/feedback-analysis.component.ts | 7 ++++++- src/main/webapp/i18n/de/programmingExercise.json | 2 +- src/main/webapp/i18n/en/programmingExercise.json | 2 +- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/confirm-feedback-channel-creation-modal.component.ts b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/confirm-feedback-channel-creation-modal.component.ts index 352a0a657acc..4a31192b6299 100644 --- a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/confirm-feedback-channel-creation-modal.component.ts +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/confirm-feedback-channel-creation-modal.component.ts @@ -9,7 +9,7 @@ import { ArtemisSharedCommonModule } from 'app/shared/shared-common.module'; standalone: true, }) export class ConfirmFeedbackChannelCreationModalComponent { - readonly TRANSLATION_BASE = 'artemisApp.programmingExercise.configureGrading.feedbackAnalysis.feedbackDetailChannel.confirmationModal'; + protected readonly TRANSLATION_BASE = 'artemisApp.programmingExercise.configureGrading.feedbackAnalysis.feedbackDetailChannel.confirmationModal'; affectedStudentsCount = input.required(); private activeModal = inject(NgbActiveModal); diff --git a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html index e78a2b72e145..f36eeba33e0b 100644 --- a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html @@ -35,10 +35,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 4b8c9202bf79..335183139636 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 @@ -13,6 +13,7 @@ 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'; import { FeedbackDetailChannelModalComponent } from 'app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component'; import { ChannelDTO } from 'app/entities/metis/conversation/channel.model'; +import { Router } from '@angular/router'; @Component({ selector: 'jhi-feedback-analysis', @@ -32,6 +33,7 @@ export class FeedbackAnalysisComponent { private alertService = inject(AlertService); private modalService = inject(NgbModal); private localStorage = inject(LocalStorageService); + private router = inject(Router); readonly page = signal(1); readonly pageSize = signal(25); @@ -213,7 +215,10 @@ export class FeedbackAnalysisComponent { const name = createdChannel.name; this.alertService.success(this.TRANSLATION_BASE + '.channelSuccess', { name }); if (navigate) { - window.location.href = `/courses/${this.courseId()}/communication?conversationId=${createdChannel.id}`; + const urlTree = this.router.createUrlTree(['courses', this.courseId(), 'communication'], { + queryParams: { conversationId: createdChannel.id }, + }); + await this.router.navigateByUrl(urlTree); } } catch (error) { this.alertService.error(error); diff --git a/src/main/webapp/i18n/de/programmingExercise.json b/src/main/webapp/i18n/de/programmingExercise.json index feed824a7254..ac011d498daa 100644 --- a/src/main/webapp/i18n/de/programmingExercise.json +++ b/src/main/webapp/i18n/de/programmingExercise.json @@ -402,7 +402,7 @@ "descriptionLabel": "Kanalbeschreibung", "requiredValidationErrorName": "Der Kanalname ist ein Pflichtfeld.", "maxLengthValidationErrorName": "Kanalname kann max {{ max }} Zeichen lang sein!", - "regexValidationError": "Namen können nur Kleinbuchstaben, Zahlen und Striche enthalten. Nur Artemis kann Kanäle erstellen, die mit $ beginnen.", + "regexValidationErrorName": "Namen können nur Kleinbuchstaben, Zahlen und Striche enthalten. Nur Artemis kann Kanäle erstellen, die mit $ beginnen.", "requiredValidationErrorDescription": "Die Kanalbeschreibung ist ein Pflichtfeld.", "maxLengthValidationErrorDescription": "Kanalbeschreibung kann max {{ max }} Zeichen lang sein!", "confirmationModal": { diff --git a/src/main/webapp/i18n/en/programmingExercise.json b/src/main/webapp/i18n/en/programmingExercise.json index cba315f1ea34..f1dd00faa0b7 100644 --- a/src/main/webapp/i18n/en/programmingExercise.json +++ b/src/main/webapp/i18n/en/programmingExercise.json @@ -402,7 +402,7 @@ "descriptionLabel": "Description", "requiredValidationErrorName": "The channel name is required.", "maxLengthValidationErrorName": "Channel name can be max {{ max }} characters long!", - "regexValidationError": "Names can only contain lowercase letters, numbers, and dashes. Only Artemis can create channels that start with a $.", + "regexValidationErrorName": "Names can only contain lowercase letters, numbers, and dashes. Only Artemis can create channels that start with a $.", "requiredValidationErrorDescription": "The channel description is required.", "maxLengthValidationErrorDescription": "Channel description can be max {{ max }} characters long!", "confirmationModal": { From ec4dc71425cee46f67b68093b2ff47aa2e3c576f Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 19 Nov 2024 14:55:47 +0100 Subject: [PATCH 27/64] server style --- .../cit/aet/artemis/communication/ChannelIntegrationTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java index c31d2fd4fdd4..27fa2f02c36f 100644 --- a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java @@ -10,7 +10,6 @@ import java.util.Set; import java.util.stream.Collectors; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -956,7 +955,7 @@ void createFeedbackChannel_asInstructor_shouldCreateChannel() { ChannelDTO.class, HttpStatus.CREATED, headers); } catch (Exception e) { - Assertions.fail("Failed to create feedback channel", e); + fail("Failed to create feedback channel", e); } assertThat(response).isNotNull(); From 32cf02368777d3538509433b935d455d5db0b610 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Tue, 19 Nov 2024 23:20:45 +0100 Subject: [PATCH 28/64] markus feedback --- .../dto/FeedbackChannelRequestDTO.java | 7 ++++ .../web/conversation/ChannelResource.java | 20 +++++----- .../StudentParticipationRepository.java | 22 ++++++---- .../feedback-analysis.component.ts | 8 +++- .../feedback-analysis.service.ts | 10 +++-- .../communication/ChannelIntegrationTest.java | 15 ++++--- .../feedback-analysis.service.spec.ts | 40 +++++++++++++++++++ 7 files changed, 91 insertions(+), 31 deletions(-) create mode 100644 src/main/java/de/tum/cit/aet/artemis/communication/dto/FeedbackChannelRequestDTO.java diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/dto/FeedbackChannelRequestDTO.java b/src/main/java/de/tum/cit/aet/artemis/communication/dto/FeedbackChannelRequestDTO.java new file mode 100644 index 000000000000..d38b1c1d90f2 --- /dev/null +++ b/src/main/java/de/tum/cit/aet/artemis/communication/dto/FeedbackChannelRequestDTO.java @@ -0,0 +1,7 @@ +package de.tum.cit.aet.artemis.communication.dto; + +import com.fasterxml.jackson.annotation.JsonInclude; + +@JsonInclude(JsonInclude.Include.NON_EMPTY) +public record FeedbackChannelRequestDTO(ChannelDTO channel, String feedbackDetailText) { +} diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java b/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java index 6e5c1b310f9c..cbb59c4b7e46 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ChannelResource.java @@ -26,7 +26,6 @@ import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.PutMapping; 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; @@ -36,6 +35,7 @@ import de.tum.cit.aet.artemis.communication.domain.conversation.Channel; import de.tum.cit.aet.artemis.communication.dto.ChannelDTO; import de.tum.cit.aet.artemis.communication.dto.ChannelIdAndNameDTO; +import de.tum.cit.aet.artemis.communication.dto.FeedbackChannelRequestDTO; import de.tum.cit.aet.artemis.communication.repository.ConversationParticipantRepository; import de.tum.cit.aet.artemis.communication.repository.conversation.ChannelRepository; import de.tum.cit.aet.artemis.communication.service.conversation.ChannelService; @@ -471,24 +471,26 @@ public ResponseEntity deregisterUsers(@PathVariable Long courseId, @PathVa /** * POST /api/courses/:courseId/channels/: Creates a new feedback-specific channel in a course. * - * @param courseId where the channel is being created. - * @param exerciseId for which the feedback channel is being created. - * @param channelDTO containing the properties of the channel to be created, such as name, description, and visibility. - * @param feedbackDetailText a string representing the feedback detail text used to determine the affected students to be added to the channel. + * @param courseId where the channel is being created. + * @param exerciseId for which the feedback channel is being created. + * @param feedbackChannelRequest containing a DTO with the properties of the channel (e.g., name, description, visibility) + * and the feedback detail text used to determine the affected students to be added to the channel. * @return ResponseEntity with status 201 (Created) and the body containing the details of the created channel. * @throws URISyntaxException if the URI for the created resource cannot be constructed. * @throws BadRequestAlertException if the channel name starts with an invalid prefix (e.g., "$"). */ @PostMapping("{courseId}/{exerciseId}/feedback-channel") @EnforceAtLeastEditorInCourse - public ResponseEntity createFeedbackChannel(@PathVariable Long courseId, @PathVariable Long exerciseId, @RequestBody ChannelDTO channelDTO, - @RequestHeader("feedback-detail-text") String feedbackDetailText) throws URISyntaxException { - log.debug("REST request to create feedback channel in course {} with properties: {}", courseId, channelDTO); + public ResponseEntity createFeedbackChannel(@PathVariable Long courseId, @PathVariable Long exerciseId, + @RequestBody FeedbackChannelRequestDTO feedbackChannelRequest) throws URISyntaxException { + log.debug("REST request to create feedback channel for course {} and exercise {} with properties: {}", courseId, exerciseId, feedbackChannelRequest); + + ChannelDTO channelDTO = feedbackChannelRequest.channel(); + String feedbackDetailText = feedbackChannelRequest.feedbackDetailText(); User requestingUser = userRepository.getUserWithGroupsAndAuthorities(); Course course = courseRepository.findByIdElseThrow(courseId); checkCommunicationEnabledElseThrow(course); - channelAuthorizationService.isAllowedToCreateChannel(course, requestingUser); Channel createdChannel = channelService.createFeedbackChannel(course, exerciseId, channelDTO, feedbackDetailText, requestingUser); return ResponseEntity.created(new URI("/api/channels/" + createdChannel.getId())).body(conversationDTOService.convertChannelToDTO(requestingUser, createdChannel)); 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 c67ec302ff3c..03d2485f0f0a 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 @@ -1372,13 +1372,16 @@ SELECT MAX(pr.id) @Query(""" SELECT DISTINCT p.student.login FROM ProgrammingExerciseStudentParticipation p - LEFT JOIN p.submissions s - LEFT JOIN s.results r - LEFT JOIN r.feedbacks f + INNER JOIN p.submissions s + INNER JOIN s.results r ON r.id = ( + SELECT MAX(pr.id) + FROM s.results pr + WHERE pr.participation.id = p.id + ) + INNER JOIN r.feedbacks f WHERE p.exercise.id = :exerciseId AND f.detailText = :detailText AND p.testRun = FALSE - AND f.positive = FALSE """) List findAffectedLoginsByFeedbackDetailText(@Param("exerciseId") long exerciseId, @Param("detailText") String detailText); @@ -1396,13 +1399,16 @@ SELECT MAX(pr.id) @Query(""" SELECT COUNT(DISTINCT p.student.id) FROM ProgrammingExerciseStudentParticipation p - LEFT JOIN p.submissions s - LEFT JOIN s.results r - LEFT JOIN r.feedbacks f + INNER JOIN p.submissions s + INNER JOIN s.results r ON r.id = ( + SELECT MAX(pr.id) + FROM s.results pr + WHERE pr.participation.id = p.id + ) + INNER JOIN r.feedbacks f WHERE p.exercise.id = :exerciseId AND f.detailText = :detailText AND p.testRun = FALSE - AND f.positive = FALSE """) long countAffectedStudentsByFeedbackDetailText(@Param("exerciseId") long exerciseId, @Param("detailText") String detailText); } 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 335183139636..9c41949f3393 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 @@ -1,5 +1,5 @@ import { Component, computed, effect, inject, input, signal, untracked } from '@angular/core'; -import { FeedbackAnalysisService, FeedbackDetail } from './feedback-analysis.service'; +import { FeedbackAnalysisService, FeedbackChannelRequestDTO, FeedbackDetail } from './feedback-analysis.service'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { AlertService } from 'app/core/util/alert.service'; import { faFilter, faMessage, faSort, faSortDown, faSortUp, faUpRightAndDownLeftFromCenter, faUsers } from '@fortawesome/free-solid-svg-icons'; @@ -211,7 +211,11 @@ export class FeedbackAnalysisComponent { modalRef.componentInstance.feedbackDetail = signal(feedbackDetail); modalRef.componentInstance.formSubmitted.subscribe(async ({ channelDto, navigate }: { channelDto: ChannelDTO; navigate: boolean }) => { try { - const createdChannel = await this.feedbackAnalysisService.createChannel(this.courseId(), this.exerciseId(), channelDto, feedbackDetail.detailText); + const feedbackChannelRequest: FeedbackChannelRequestDTO = { + channel: channelDto, + feedbackDetailText: feedbackDetail.detailText, + }; + const createdChannel = await this.feedbackAnalysisService.createChannel(this.courseId(), this.exerciseId(), feedbackChannelRequest); const name = createdChannel.name; this.alertService.success(this.TRANSLATION_BASE + '.channelSuccess', { name }); if (navigate) { 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 906b699f19a5..d034cc56a506 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 @@ -29,6 +29,10 @@ export interface FeedbackAffectedStudentDTO { login: string; repositoryURI: string; } +export interface FeedbackChannelRequestDTO { + channel: ChannelDTO; + feedbackDetailText: string; +} @Injectable() export class FeedbackAnalysisService extends BaseApiHttpService { search(pageable: SearchTermPageableSearch, options: { exerciseId: number; filters: FilterData }): Promise { @@ -64,10 +68,8 @@ export class FeedbackAnalysisService extends BaseApiHttpService { return this.get>(`exercises/${exerciseId}/feedback-details-participation`, { params, headers }); } - createChannel(courseId: number, exerciseId: number, channelDTO: ChannelDTO, feedbackDetailText: string): Promise { - const headers = new HttpHeaders().set('feedback-detail-text', feedbackDetailText); - - return this.post(`courses/${courseId}/${exerciseId}/feedback-channel`, channelDTO, { headers }); + createChannel(courseId: number, exerciseId: number, feedbackChannelRequest: FeedbackChannelRequestDTO): Promise { + return this.post(`courses/${courseId}/${exerciseId}/feedback-channel`, feedbackChannelRequest); } getAffectedStudentCount(exerciseId: number, feedbackDetailText: string): Promise { diff --git a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java index 27fa2f02c36f..3bb88f191080 100644 --- a/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/communication/ChannelIntegrationTest.java @@ -16,7 +16,6 @@ import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.ValueSource; 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 org.springframework.util.LinkedMultiValueMap; @@ -24,6 +23,7 @@ import de.tum.cit.aet.artemis.communication.domain.conversation.Channel; import de.tum.cit.aet.artemis.communication.dto.ChannelDTO; import de.tum.cit.aet.artemis.communication.dto.ChannelIdAndNameDTO; +import de.tum.cit.aet.artemis.communication.dto.FeedbackChannelRequestDTO; import de.tum.cit.aet.artemis.communication.dto.MetisCrudAction; import de.tum.cit.aet.artemis.communication.util.ConversationUtilService; import de.tum.cit.aet.artemis.core.domain.Course; @@ -913,20 +913,20 @@ void getLectureChannel_asCourseStudent_shouldGetLectureChannel() throws Exceptio void createFeedbackChannel_asStudent_shouldReturnForbidden() { Course course = programmingExerciseUtilService.addCourseWithOneProgrammingExercise(); ProgrammingExercise programmingExercise = programmingExerciseUtilService.addProgrammingExerciseToCourse(course); + ChannelDTO channelDTO = new ChannelDTO(); channelDTO.setName("feedback-channel"); channelDTO.setDescription("Discussion channel for feedback"); channelDTO.setIsPublic(true); channelDTO.setIsAnnouncementChannel(false); - HttpHeaders headers = new HttpHeaders(); - headers.set("feedback-detail-text", "Sample feedback text"); + FeedbackChannelRequestDTO feedbackChannelRequest = new FeedbackChannelRequestDTO(channelDTO, "Sample feedback text"); String BASE_ENDPOINT = "api/courses/{courseId}/{exerciseId}/feedback-channel"; try { request.postWithoutResponseBody(BASE_ENDPOINT.replace("{courseId}", course.getId().toString()).replace("{exerciseId}", programmingExercise.getId().toString()), - channelDTO, HttpStatus.FORBIDDEN, headers); + feedbackChannelRequest, HttpStatus.FORBIDDEN); } catch (Exception e) { fail("There was an error executing the post request.", e); @@ -944,15 +944,14 @@ void createFeedbackChannel_asInstructor_shouldCreateChannel() { channelDTO.setIsPublic(true); channelDTO.setIsAnnouncementChannel(false); - HttpHeaders headers = new HttpHeaders(); - headers.set("feedback-detail-text", "Sample feedback text"); + FeedbackChannelRequestDTO feedbackChannelRequest = new FeedbackChannelRequestDTO(channelDTO, "Sample feedback text"); String BASE_ENDPOINT = "/api/courses/{courseId}/{exerciseId}/feedback-channel"; ChannelDTO response = null; try { - response = request.postWithResponseBody(BASE_ENDPOINT.replace("{courseId}", courseId.toString()).replace("{exerciseId}", exerciseId.toString()), channelDTO, - ChannelDTO.class, HttpStatus.CREATED, headers); + response = request.postWithResponseBody(BASE_ENDPOINT.replace("{courseId}", courseId.toString()).replace("{exerciseId}", exerciseId.toString()), feedbackChannelRequest, + ChannelDTO.class, HttpStatus.CREATED); } catch (Exception e) { fail("Failed to create feedback channel", e); 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 b735a2e0f796..45bb903fe421 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 @@ -149,4 +149,44 @@ describe('FeedbackAnalysisService', () => { expect(result).toBe(affectedStudentCountMock); }); }); + + describe('createChannel', () => { + it('should send a POST request to create a feedback-specific channel and return the created channel DTO', async () => { + const courseId = 1; + const exerciseId = 2; + + const channelDtoMock = { + name: 'feedback-channel', + description: 'Discussion channel for feedback', + isPublic: true, + isAnnouncementChannel: false, + }; + + const feedbackChannelRequestMock = { + channel: channelDtoMock, + feedbackDetailText: 'Sample feedback detail text', + }; + + const createdChannelMock = { + id: 1001, + name: 'feedback-channel', + description: 'Discussion channel for feedback', + isPublic: true, + isAnnouncementChannel: false, + }; + + const responsePromise = service.createChannel(courseId, exerciseId, feedbackChannelRequestMock); + + const req = httpMock.expectOne(`api/courses/${courseId}/${exerciseId}/feedback-channel`); + expect(req.request.method).toBe('POST'); + expect(req.request.body).toEqual(feedbackChannelRequestMock); + + req.flush(createdChannelMock); + + const result = await responsePromise; + expect(result).toEqual(createdChannelMock); + expect(result.name).toBe('feedback-channel'); + expect(result.description).toBe('Discussion channel for feedback'); + }); + }); }); From fdbc64cb30a08bcdcb64c1451764369a6d533c83 Mon Sep 17 00:00:00 2001 From: aniruddhzaveri Date: Mon, 25 Nov 2024 13:18:35 +0100 Subject: [PATCH 29/64] ramona feedback --- .../feedback-detail-channel-modal.component.html | 8 ++++---- .../feedback-analysis.component.html | 2 +- .../feedback-analysis.component.ts | 7 ++++--- src/main/webapp/content/icons/icons.ts | 15 +++++++++++++++ src/main/webapp/i18n/de/programmingExercise.json | 7 +++---- src/main/webapp/i18n/en/programmingExercise.json | 5 ++--- 6 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html index f36eeba33e0b..36d5b08ed5b9 100644 --- a/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html +++ b/src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-detail-channel-modal.component.html @@ -12,7 +12,7 @@