Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Programming exercises: Enhance filtering for error analysis #9315

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package de.tum.cit.aet.artemis.assessment.dto;

import com.fasterxml.jackson.annotation.JsonInclude;

import de.tum.cit.aet.artemis.core.dto.SearchResultPageDTO;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
public record FeedbackAnalysisResponseDTO(SearchResultPageDTO<FeedbackDetailDTO> feedbackDetails, long totalItems) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import jakarta.annotation.Nullable;
Expand All @@ -28,6 +29,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.FeedbackAnalysisResponseDTO;
import de.tum.cit.aet.artemis.assessment.dto.FeedbackDetailDTO;
import de.tum.cit.aet.artemis.assessment.repository.ComplaintRepository;
import de.tum.cit.aet.artemis.assessment.repository.ComplaintResponseRepository;
Expand All @@ -40,6 +42,9 @@
import de.tum.cit.aet.artemis.buildagent.dto.ResultBuildJob;
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.SortingOrder;
import de.tum.cit.aet.artemis.core.dto.pageablesearch.SearchTermPageableSearchDTO;
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;
Expand Down Expand Up @@ -530,31 +535,66 @@ private Result shouldSaveResult(@NotNull Result result, boolean shouldSave) {
}

/**
* Retrieves aggregated feedback details for a given exercise, calculating relative counts based on the total number of distinct results.
* Retrieves paginated and filtered aggregated feedback details for a given exercise, calculating relative counts based on the total number of distinct results.
* The task numbers are assigned based on the associated test case names, using the set of tasks fetched from the database.
* <br>
* For each feedback detail:
* 1. The relative count is calculated as a percentage of the total number of distinct results for the exercise.
* 2. The task number is determined by matching the test case name with the tasks.
* <br>
* The method supports filtering by a search term across feedback details, test case names, counts, task numbers, and relative counts.
* Sorting is applied based on the specified column and order (ascending or descending).
* The result is paginated based on the provided page number and page size.
*
* @param exerciseId The ID of the exercise for which feedback details should be retrieved.
* @return A list of FeedbackDetailDTO objects, each containing:
* - feedback count,
* - relative count (as a percentage of distinct results),
* - detail text,
* - test case name,
* - determined task number (based on the test case name).
* @param search The pageable search DTO containing page number, page size, sorting options, and a search term for filtering results.
* @return A {@link FeedbackAnalysisResponseDTO} object containing:
* - a {@link SearchResultPageDTO} of paginated feedback details, and
* - the total number of distinct results (distinctResultCount) for the exercise.
*/
public List<FeedbackDetailDTO> findAggregatedFeedbackByExerciseId(long exerciseId) {
public FeedbackAnalysisResponseDTO getFeedbackDetailsOnPage(long exerciseId, SearchTermPageableSearchDTO<String> search) {
long distinctResultCount = studentParticipationRepository.countDistinctResultsByExerciseId(exerciseId);
Set<ProgrammingExerciseTask> tasks = programmingExerciseTaskService.getTasksWithUnassignedTestCases(exerciseId);
List<ProgrammingExerciseTask> tasks = programmingExerciseTaskService.getTasksWithUnassignedTestCases(exerciseId);

List<FeedbackDetailDTO> feedbackDetails = studentParticipationRepository.findAggregatedFeedbackByExerciseId(exerciseId);
String searchTerm = search.getSearchTerm() != null ? search.getSearchTerm().toLowerCase() : "";

return feedbackDetails.stream().map(detail -> {
feedbackDetails = feedbackDetails.stream().map(detail -> {
double relativeCount = (detail.count() * 100.0) / distinctResultCount;
int taskNumber = tasks.stream().filter(task -> task.getTestCases().stream().anyMatch(tc -> tc.getTestName().equals(detail.testCaseName()))).findFirst()
.map(task -> tasks.stream().toList().indexOf(task) + 1).orElse(0);
int taskNumber = determineTaskNumberOfTestCase(detail.testCaseName(), tasks);

return new FeedbackDetailDTO(detail.count(), relativeCount, detail.detailText(), detail.testCaseName(), taskNumber);
}).toList();
}).filter(matchesSearchTerm(searchTerm)).sorted(getComparatorForFeedbackDetails(search)).toList();
az108 marked this conversation as resolved.
Show resolved Hide resolved

return paginateFeedbackDetails(feedbackDetails, search.getPage(), search.getPageSize());
az108 marked this conversation as resolved.
Show resolved Hide resolved
}

private Predicate<FeedbackDetailDTO> matchesSearchTerm(String searchTerm) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up: As soon as filtering is added, remove:

  • detail.testCaseName().toLowerCase().contains(searchTerm)
  • String.valueOf(detail.count()).contains(searchTerm)
  • String.valueOf(detail.taskNumber()).contains(searchTerm)
  • String.valueOf(detail.relativeCount()).contains(searchTerm)

Additionally, as soon as we only have one colume to sort for, please try to include that into the DB query.

return detail -> searchTerm.isEmpty() || detail.detailText().toLowerCase().contains(searchTerm) || detail.testCaseName().toLowerCase().contains(searchTerm)
|| String.valueOf(detail.count()).contains(searchTerm) || String.valueOf(detail.taskNumber()).contains(searchTerm)
|| String.valueOf(detail.relativeCount()).contains(searchTerm);
}

private Comparator<FeedbackDetailDTO> getComparatorForFeedbackDetails(SearchTermPageableSearchDTO<String> search) {
Map<String, Comparator<FeedbackDetailDTO>> comparators = Map.of("count", Comparator.comparingLong(FeedbackDetailDTO::count), "detailText",
Comparator.comparing(FeedbackDetailDTO::detailText, String.CASE_INSENSITIVE_ORDER), "testCaseName",
Comparator.comparing(FeedbackDetailDTO::testCaseName, String.CASE_INSENSITIVE_ORDER), "taskNumber", Comparator.comparingInt(FeedbackDetailDTO::taskNumber),
"relativeCount", Comparator.comparingDouble(FeedbackDetailDTO::relativeCount));
Comparator<FeedbackDetailDTO> comparator = comparators.getOrDefault(search.getSortedColumn(), (a, b) -> 0);
return search.getSortingOrder() == SortingOrder.ASCENDING ? comparator : comparator.reversed();
}

private int determineTaskNumberOfTestCase(String testCaseName, List<ProgrammingExerciseTask> tasks) {
return tasks.stream().filter(task -> task.getTestCases().stream().anyMatch(tc -> tc.getTestName().equals(testCaseName))).findFirst()
.map(task -> tasks.stream().toList().indexOf(task) + 1).orElse(0);
}

private FeedbackAnalysisResponseDTO paginateFeedbackDetails(List<FeedbackDetailDTO> feedbackDetails, int page, int pageSize) {
az108 marked this conversation as resolved.
Show resolved Hide resolved
int start = (page - 1) * pageSize;
az108 marked this conversation as resolved.
Show resolved Hide resolved
int end = Math.min(start + pageSize, feedbackDetails.size());
List<FeedbackDetailDTO> paginatedFeedbackDetails = feedbackDetails.subList(start, end);

int totalPages = (feedbackDetails.size() + pageSize - 1) / pageSize;
az108 marked this conversation as resolved.
Show resolved Hide resolved
return new FeedbackAnalysisResponseDTO(new SearchResultPageDTO<>(paginatedFeedbackDetails, totalPages), feedbackDetails.size());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@

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.FeedbackDetailDTO;
import de.tum.cit.aet.artemis.assessment.dto.FeedbackAnalysisResponseDTO;
import de.tum.cit.aet.artemis.assessment.dto.ResultWithPointsPerGradingCriterionDTO;
import de.tum.cit.aet.artemis.assessment.repository.ResultRepository;
import de.tum.cit.aet.artemis.assessment.service.ResultService;
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.pageablesearch.SearchTermPageableSearchDTO;
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;
Expand Down Expand Up @@ -280,16 +281,21 @@ public ResponseEntity<Result> createResultForExternalSubmission(@PathVariable Lo
}

/**
* GET /exercises/:exerciseId/feedback-details : Retrieves all aggregated feedback details for a given exercise.
* The feedback details include counts and relative counts of feedback occurrences, along with associated test case names and task numbers.
* POST /exercises/{exerciseId}/feedback-details-paged : Retrieves paginated and filtered aggregated feedback details for a given exercise.
* The feedback details include counts and relative counts of feedback occurrences, test case names, and task numbers.
* The method allows filtering by search term and sorting by various fields.
* <br>
* Pagination is applied based on the provided {@link SearchTermPageableSearchDTO}, including page number, page size, sorting order, and search term.
* The response contains both the paginated feedback details and the total count of distinct results for the exercise.
*
* @param exerciseId The ID of the exercise for which feedback details should be retrieved.
* @return A ResponseEntity containing a list of {@link FeedbackDetailDTO}s
* @param search The pageable search DTO containing page number, page size, sorting options, and a search term for filtering results.
* @return A {@link ResponseEntity} containing a {@link FeedbackAnalysisResponseDTO}, which includes the paginated feedback details and the total count of distinct results.
*/
@GetMapping("exercises/{exerciseId}/feedback-details")
@PostMapping("exercises/{exerciseId}/feedback-details-paged")
az108 marked this conversation as resolved.
Show resolved Hide resolved
@EnforceAtLeastEditorInExercise
public ResponseEntity<List<FeedbackDetailDTO>> getAllFeedbackDetailsForExercise(@PathVariable Long exerciseId) {
log.debug("REST request to get all Feedback details for Exercise {}", exerciseId);
return ResponseEntity.ok(resultService.findAggregatedFeedbackByExerciseId(exerciseId));
public ResponseEntity<FeedbackAnalysisResponseDTO> getFeedbackDetailsPaged(@PathVariable long exerciseId, @RequestBody SearchTermPageableSearchDTO<String> search) {
FeedbackAnalysisResponseDTO response = resultService.getFeedbackDetailsOnPage(exerciseId, search);
return ResponseEntity.ok(response);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package de.tum.cit.aet.artemis.programming.repository.hestia;

import java.util.List;
import java.util.Optional;
import java.util.Set;

Expand Down Expand Up @@ -54,7 +55,7 @@ default ProgrammingExerciseTask findByIdWithTestCaseAndSolutionEntriesElseThrow(
* @throws EntityNotFoundException If the exercise with exerciseId does not exist
*/
@NotNull
default Set<ProgrammingExerciseTask> findByExerciseIdWithTestCaseAndSolutionEntriesElseThrow(long exerciseId) throws EntityNotFoundException {
default List<ProgrammingExerciseTask> findByExerciseIdWithTestCaseAndSolutionEntriesElseThrow(long exerciseId) throws EntityNotFoundException {
return getArbitraryValueElseThrow(findByExerciseIdWithTestCaseAndSolutionEntries(exerciseId), Long.toString(exerciseId));
}

Expand All @@ -72,7 +73,7 @@ default Set<ProgrammingExerciseTask> findByExerciseIdWithTestCaseAndSolutionEntr
WHERE t.exercise.id = :exerciseId
AND tc.exercise.id = :exerciseId
""")
Optional<Set<ProgrammingExerciseTask>> findByExerciseIdWithTestCaseAndSolutionEntries(@Param("exerciseId") long exerciseId);
Optional<List<ProgrammingExerciseTask>> findByExerciseIdWithTestCaseAndSolutionEntries(@Param("exerciseId") long exerciseId);

/**
* Gets all tasks with its test cases for a programming exercise
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ public boolean preCheckProjectExistsOnVCSOrCI(ProgrammingExercise programmingExe
* @param exerciseId of the exercise
*/
public void deleteTasksWithSolutionEntries(Long exerciseId) {
Set<ProgrammingExerciseTask> tasks = programmingExerciseTaskRepository.findByExerciseIdWithTestCaseAndSolutionEntriesElseThrow(exerciseId);
List<ProgrammingExerciseTask> tasks = programmingExerciseTaskRepository.findByExerciseIdWithTestCaseAndSolutionEntriesElseThrow(exerciseId);
az108 marked this conversation as resolved.
Show resolved Hide resolved
Set<ProgrammingExerciseSolutionEntry> solutionEntries = tasks.stream().map(ProgrammingExerciseTask::getTestCases).flatMap(Collection::stream)
.map(ProgrammingExerciseTestCase::getSolutionEntries).flatMap(Collection::stream).collect(Collectors.toSet());
programmingExerciseTaskRepository.deleteAll(tasks);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ public Set<ProgrammingExerciseTask> getTasksWithoutInactiveTestCases(long exerci
* @param exerciseId of the programming exercise
* @return Set of all tasks including one for not manually assigned tests
*/
public Set<ProgrammingExerciseTask> getTasksWithUnassignedTestCases(long exerciseId) {
Set<ProgrammingExerciseTask> tasks = programmingExerciseTaskRepository.findByExerciseIdWithTestCaseAndSolutionEntriesElseThrow(exerciseId);
public List<ProgrammingExerciseTask> getTasksWithUnassignedTestCases(long exerciseId) {
List<ProgrammingExerciseTask> tasks = programmingExerciseTaskRepository.findByExerciseIdWithTestCaseAndSolutionEntriesElseThrow(exerciseId);

Set<ProgrammingExerciseTestCase> testsWithTasks = tasks.stream().flatMap(task -> task.getTestCases().stream()).collect(Collectors.toSet());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;

import java.util.List;
import java.util.Set;

import org.slf4j.Logger;
Expand Down Expand Up @@ -74,13 +75,13 @@ public ResponseEntity<Set<ProgrammingExerciseTask>> getTasks(@PathVariable Long
*/
@GetMapping("programming-exercises/{exerciseId}/tasks-with-unassigned-test-cases")
@EnforceAtLeastTutor
public ResponseEntity<Set<ProgrammingExerciseTask>> getTasksWithUnassignedTask(@PathVariable Long exerciseId) {
public ResponseEntity<List<ProgrammingExerciseTask>> getTasksWithUnassignedTask(@PathVariable Long exerciseId) {
log.debug("REST request to retrieve ProgrammingExerciseTasks for ProgrammingExercise with id : {}", exerciseId);
// Reload the exercise from the database as we can't trust data from the client
ProgrammingExercise exercise = programmingExerciseRepository.findByIdElseThrow(exerciseId);
authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.TEACHING_ASSISTANT, exercise, null);

Set<ProgrammingExerciseTask> tasks = programmingExerciseTaskService.getTasksWithUnassignedTestCases(exerciseId);
List<ProgrammingExerciseTask> tasks = programmingExerciseTaskService.getTasksWithUnassignedTestCases(exerciseId);
return ResponseEntity.ok(tasks);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<div class="modal-header">
<h4 class="m-0" jhiTranslate="artemisApp.programmingExercise.configureGrading.feedbackAnalysis.feedbackModal.header"></h4>
<button type="button" class="btn-close" aria-label="Close" (click)="activeModal.close()"></button>
</div>
<hr class="my-0" />
<div class="modal-body">
<div class="d-flex justify-content-between mb-3">
<div class="border">
<span class="modal-label" jhiTranslate="artemisApp.programmingExercise.configureGrading.feedbackAnalysis.task"></span>
<p>{{ feedbackDetail().taskNumber }}</p>
</div>
<div class="border">
<span class="modal-label" jhiTranslate="artemisApp.programmingExercise.configureGrading.feedbackAnalysis.occurrence"></span>
<p>{{ feedbackDetail().relativeCount }}% ({{ feedbackDetail().count }})</p>
</div>
<div class="border">
<span class="modal-label" jhiTranslate="artemisApp.programmingExercise.configureGrading.feedbackAnalysis.testcase"></span>
<p>{{ feedbackDetail().testCaseName }}</p>
</div>
<div class="border">
<span class="modal-label" jhiTranslate="artemisApp.programmingExercise.configureGrading.feedbackAnalysis.errorCategory"></span>
<p>Student Error</p>
az108 marked this conversation as resolved.
Show resolved Hide resolved
</div>
</div>

<h5 class="mb-3" jhiTranslate="artemisApp.programmingExercise.configureGrading.feedbackAnalysis.feedbackModal.feedbackTitle"></h5>
<p>{{ feedbackDetail().detailText }}</p>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
.modal-body {
padding: 1rem;
}

.modal-label {
font-weight: bold;
}

.border {
padding: 5px;
border: var(--border-color) 1px solid;
border-radius: 10px;
}
az108 marked this conversation as resolved.
Show resolved Hide resolved

.modal-header {
border-bottom: none;
padding-left: 1rem;
padding-top: 1.5rem;
}

.modal-body p {
margin-bottom: 0;
}

.modal-footer {
border-top: none;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Component, InputSignal, inject, input } from '@angular/core';
import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
import { FeedbackDetail } from 'app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service';
import { ArtemisSharedCommonModule } from 'app/shared/shared-common.module';

@Component({
selector: 'jhi-feedback-modal',
templateUrl: './feedback-modal.component.html',
styleUrls: ['./feedback-modal.component.scss'],
imports: [ArtemisSharedCommonModule],
standalone: true,
})
export class FeedbackModalComponent {
feedbackDetail: InputSignal<FeedbackDetail> = input.required<FeedbackDetail>();

activeModal: NgbActiveModal = inject(NgbActiveModal);
}
Loading
Loading