Programming exercises: Add group feedback feature to feedback analysis table

[az108]

Checklist

General

• I tested all changes and their related features with all corresponding user types on a test server.
• This is a small issue that I tested locally and was confirmed by another developer on a test server.
• Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
• I chose a title conforming to the naming conventions for pull requests.

Server

• Important: I implemented the changes with a very good performance and prevented too many (unnecessary) and too complex database calls.
• I strictly followed the principle of data economy for all database calls.
• I strictly followed the server coding and design guidelines.
• I added multiple integration tests (Spring) related to the features (with a high test coverage).
• I added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).
• I documented the Java code using JavaDoc style.

Client

• Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data (e.g. using paging).
• I strictly followed the principle of data economy for all client-server REST calls.
• I strictly followed the client coding and design guidelines.
• Following the theming guidelines, I specified colors only in the theming variable files and checked that the changes look consistent in both the light and the dark theme.
• I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
• I added authorities to all new routes and checked the course groups for displaying navigation elements (links, buttons).
• I documented the TypeScript code using JSDoc style.
• I added multiple screenshots/screencasts of my UI changes.
• I translated all newly inserted strings into English and German.

Motivation and Context

This is a follow-up to the feature I’m working on: #9810. The goal is to provide instructors with clearer insights into the feedback given to students. In this PR, a “Group Feedback” feature has been introduced. This feature allows instructors to group similar feedback (similarity > 90%), ensuring that the table remains manageable without an overwhelming number of individual entries. This is particularly useful in scenarios where test cases utilize random values, leading to feedback that differs only slightly (e.g., by numbers) while the rest of the content remains identical.

Description

A new toggle was added along with a corresponding flag to indicate whether Levenshtein grouping should be applied. Depending on the toggle’s state (true or false), the server now either groups the feedback using Levenshtein or skips the grouping. Unfortunately, Levenshtein grouping is resource-intensive, which is why a loading indicator was added to enhance user experience during the processing.

Additionally, since Levenshtein grouping cannot be applied directly within a query, manual pagination is required when the toggle is activated.

The Affected Students Modal was also refactored. It no longer relies on feedback IDs but instead uses the feedback detail text. This approach was previously tested in the last PR with feedback channel creation, where a similar change was made, and it proved to be more efficient than working with IDs.

Steps for Testing

Prerequisites:

• 1 Instructor
• Student Sumbissions
• 1 Programming Exercise

1. Log in to Artemis
2. Navigate to a Course with Instructor rights
3. Create a Programming Exercise or use an existing one
4. Solve the Programming Exercise as Students, ideally with different kinds of Progress (You can use artemis test users for this)
5. Make sure you have two identical feedbacks in one testcase (similarity > 90 percent). You might need to adjust testcases by editing them for the programming exercises
6. Navigate as Instructor to the Grading of an Exercise
7. Navigate to the Feedback Analysis section
8. Verify that the table displays data correctly, and press the group feedback toggle
9. Verify that the two similar feedbacks were grouped into one feedback and count was updated correctly

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.

Review Progress

Performance Review

• I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance even for very large courses with more than 2000 students.
• I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance even for very large courses with more than 2000 students.

Code Review

• Code Review 1
• Code Review 2

Manual Tests

• Test 1
• Test 2

Performance Tests

• Test 1
• Test 2

Test Coverage

(will be updated after developer reviews!)

Class/File	Line Coverage	Confirmation (expect)
feedback-analysis	94.11%	✅
feedback-analysis-modal	92.59%	✅
Methods in ResultService	100%	✅
Methods in ResultResource	100%	✅
Methods in ChannelResource	100%	✅

Screenshots

Before:

After:

Summary by CodeRabbit

Release Notes

• New Features

  • Introduced a toggle for grouping feedback based on Levenshtein distance, enhancing feedback analysis.
  • Added support for multiple feedback detail texts in various components and services.
  • Enhanced user interface with dynamic loading indicators and improved feedback presentation.
  • Implemented a new parameter for feedback retrieval methods to support grouping by Levenshtein distance.
  • Updated feedback detail channel creation to accept multiple feedback texts and a test case name.
  • Added new localization keys for improved clarity regarding the grouping feature.

• Bug Fixes

  • Improved error handling and loading state management across feedback-related components.

• Documentation

  • Updated localization files to include new keys and descriptions for the grouping feature.

• Tests

  • Refined integration and unit tests to align with the new data structures and functionalities.

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between d363283 and b157de6.

📒 Files selected for processing (1)

• src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java (7 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

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

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

📓 Learnings (1)

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

Learnt from: julian-christl
PR: ls1intum/Artemis#9322
File: src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java:0-0
Timestamp: 2024-11-12T12:51:51.201Z
Learning: In Artemis, an `ExerciseGroup` always has an associated `Exam`, so `exerciseGroup.exam` is never null.

Learnt from: julian-christl
PR: ls1intum/Artemis#9322
File: src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java:170-172
Timestamp: 2024-11-12T12:51:46.554Z
Learning: In Artemis, `exercise.exerciseGroup` may be null, as not all exercises belong to an `ExerciseGroup`.

🔇 Additional comments (4)

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

10-10: LGTM: Import statements are appropriate.

The added imports are necessary for implementing the new Levenshtein grouping functionality.

Also applies to: 29-29, 53-53, 69-69

---

744-752: LGTM: Method refactor improves code clarity.

The change from using feedbackId to detailText and testCaseName parameters is well-implemented and aligns with the PR objectives.

---

694-695: ⚠️ Potential issue

Add null check before accessing first element of detailText list.

Calling getFirst() without checking if the list is empty could throw NoSuchElementException.

Apply this diff:

-aggregatedTexts.add(base.detailText().getFirst());
+if (!base.detailText().isEmpty()) {
+    aggregatedTexts.add(base.detailText().getFirst());
+}

Likely invalid or redundant comment.

---

629-632: ⚠️ Potential issue

Fix potential NullPointerException in levenshtein comparison.

The current implementation could throw NPE if levenshtein is null.

Apply this diff:

-if (!Boolean.parseBoolean(levenshtein)) {
+if (!enableLevenshteinGrouping) {

Likely invalid or redundant comment.

[SimonEntholzer]

Code lgtm to me now, with the improved aggregation algorithm

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between b157de6 and 9df1533.

📒 Files selected for processing (1)

• src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java (6 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

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

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

📓 Learnings (1)

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

Learnt from: julian-christl
PR: ls1intum/Artemis#9322
File: src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java:0-0
Timestamp: 2024-11-12T12:51:51.201Z
Learning: In Artemis, an `ExerciseGroup` always has an associated `Exam`, so `exerciseGroup.exam` is never null.

Learnt from: julian-christl
PR: ls1intum/Artemis#9322
File: src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java:170-172
Timestamp: 2024-11-12T12:51:46.554Z
Learning: In Artemis, `exercise.exerciseGroup` may be null, as not all exercises belong to an `ExerciseGroup`.

🔇 Additional comments (4)

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

744-752: LGTM! Method signature changes align with feedback grouping feature.

The changes to use detailText list and testCaseName instead of feedbackIds are well-documented and properly implemented.

---

673-675: ⚠️ Potential issue

Add null check for detailText list access.

Accessing detailText.getFirst() without checking if the list is empty could cause NoSuchElementException.

Apply this diff:

-                Comparator.comparing(detail -> detail.detailText().isEmpty() ? "" : detail.detailText().getFirst(), // Sort by the first element of the list
+                Comparator.comparing(detail -> detail.detailText().stream().findFirst().orElse(""), // Safely get first element

Likely invalid or redundant comment.

---

575-580: ⚠️ Potential issue

Convert levenshtein parameter to boolean type.

The levenshtein parameter is used as a boolean flag but declared as String. This is error-prone and less type-safe.

Apply this diff to improve type safety:

-public FeedbackAnalysisResponseDTO getFeedbackDetailsOnPage(long exerciseId, FeedbackPageableDTO data, String levenshtein) {
+public FeedbackAnalysisResponseDTO getFeedbackDetailsOnPage(long exerciseId, FeedbackPageableDTO data, boolean enableLevenshteinGrouping) {

Likely invalid or redundant comment.

---

628-631: ⚠️ Potential issue

Use safer boolean comparison and consider string constant.

The string comparison for levenshtein parameter is error-prone and could lead to NullPointerException.

Apply this diff to fix the comparison:

-if (!Boolean.parseBoolean(levenshtein)) {
+if (!enableLevenshteinGrouping) {

Likely invalid or redundant comment.

[raffifasaro]

re-approve

[sachmii]

Tested on TS2, grouping works just fine.

[ahbitaqu]

[Tested on TS2]
Works as described!

[HawKhiem]

Tested on TS2. Everything works as described