-
Notifications
You must be signed in to change notification settings - Fork 297
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
: Add error categories and categorize feedback in grading analysis
#9622
Programming exercises
: Add error categories and categorize feedback in grading analysis
#9622
Conversation
…feedback-in-table
…tegorize-feedback-in-table' into feature/programming-exercises/categorize-feedback-in-table
…feedback-in-table
WalkthroughThe pull request introduces significant modifications across multiple files, primarily focusing on enhancing the feedback analysis functionality in the application. Key changes include the restructuring of data transfer objects, particularly the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
🧹 Outside diff range and nitpick comments (23)
src/main/webapp/app/shared/sort/sort-icon.component.html (1)
1-4
: The sort icon implementation looks good but could be enhanced.The implementation is clean and functional. However, there are a few improvements that could make it more maintainable and accessible:
Consider applying these enhancements:
-<div class="sort-icon"> - <fa-icon [icon]="faSortUp" [style.color]="isAscending() ? 'black' : 'grey'"></fa-icon> - <fa-icon [icon]="faSortDown" [style.color]="isDescending() ? 'black' : 'grey'"></fa-icon> +<div class="sort-icon" role="img" aria-label="Sort direction indicator"> + <fa-icon + [icon]="faSortUp" + [class.active]="isAscending()" + aria-hidden="true"> + </fa-icon> + <fa-icon + [icon]="faSortDown" + [class.active]="isDescending()" + aria-hidden="true"> + </fa-icon> </div>Add these styles to your component's CSS:
.sort-icon { fa-icon { color: var(--inactive-sort-color, grey); &.active { color: var(--active-sort-color, black); } } }These changes:
- Move inline styles to CSS classes for better maintainability
- Use CSS variables for colors, making theme changes easier
- Add ARIA attributes for better accessibility
- Improve code readability with proper formatting
src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAnalysisResponseDTO.java (1)
11-12
: Consider adding JavaDoc documentation.The record structure looks good and follows DTO best practices. Consider adding JavaDoc to document the purpose of each field, especially the new
taskNames
anderrorCategories
fields, to improve maintainability.Example:
/** * DTO for feedback analysis response containing paginated feedback details and filtering options. * * @param feedbackDetails Paginated feedback details * @param totalItems Total number of items across all pages * @param taskNames Set of unique task names available for filtering * @param testCaseNames List of test case names available for filtering * @param errorCategories List of error categories available for filtering */src/main/webapp/app/shared/sort/sort-icon.component.ts (1)
13-21
: LGTM! Clean implementation with room for optimization.The component implementation follows best practices with proper typing and computed properties. Consider using signals for better change detection performance.
Optional optimization:
- isAscending = computed(() => this.direction() === 'asc'); - isDescending = computed(() => this.direction() === 'desc'); + readonly isAscending = computed(() => this.direction() === 'asc'); + readonly isDescending = computed(() => this.direction() === 'desc');src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackPageableDTO.java (2)
51-53
: Consider adding null check for consistency.The
getSearchTerm()
method includes null handling, butgetFilterErrorCategories()
doesn't. Consider adding similar null handling for consistency across the DTO.public List<String> getFilterErrorCategories() { - return filterErrorCategories; + return filterErrorCategories != null ? filterErrorCategories : List.of(); }
Line range hint
1-58
: Consider converting to a Java record and adding validation.Since this is a DTO class used for transferring search parameters:
- Consider converting it to a Java record for immutability and automatic getter generation:
public record FeedbackPageableDTO( List<String> filterTasks, List<String> filterTestCases, String[] filterOccurrence, String searchTerm, List<String> filterErrorCategories ) extends PageableSearchDTO<String> { public FeedbackPageableDTO { // Defensive copying for mutable fields filterTasks = filterTasks != null ? List.copyOf(filterTasks) : List.of(); filterTestCases = filterTestCases != null ? List.copyOf(filterTestCases) : List.of(); filterErrorCategories = filterErrorCategories != null ? List.copyOf(filterErrorCategories) : List.of(); filterOccurrence = filterOccurrence != null ? filterOccurrence.clone() : new String[0]; searchTerm = searchTerm != null ? searchTerm : ""; } }
- If keeping the current class structure, consider adding constructor validation and defensive copying to prevent potential NPEs and mutability issues.
src/test/javascript/spec/util/shared/sort-icon.component.spec.ts (1)
18-37
: Add test case for invalid direction value.While the existing test cases cover the valid scenarios well, we should also test how the component handles invalid direction values.
Add this test case:
it('should handle invalid direction value by setting both flags to false', () => { fixture.componentRef.setInput('direction', 'invalid'); fixture.detectChanges(); expect(component.isAscending()).toBeFalse(); expect(component.isDescending()).toBeFalse(); });src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts (1)
33-34
: Consider explicit empty array handling for filters.While the implementation is correct, consider adding explicit handling for empty filter arrays to improve clarity and prevent potential issues with backend interpretation.
Consider this enhancement:
- .set('filterErrorCategories', options.filters.errorCategories.join(',')); + .set('filterErrorCategories', options.filters.errorCategories?.length ? options.filters.errorCategories.join(',') : '');src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts (1)
12-13
: Consider adding more diverse test cases.While the current mock data is valid, consider adding test cases with:
- Different error categories to test filtering
- Edge cases like empty taskName or errorCategory
- Special characters in detailText
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' }, + { detailText: 'Feedback 3', testCaseName: 'test3', count: 1, relativeCount: 5.0, taskName: '3', errorCategory: 'CompilationError' }, + { detailText: '', testCaseName: 'test4', count: 0, relativeCount: 0.0, taskName: '', errorCategory: '' } ];src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-filter-modal.component.ts (1)
31-37
: Consider adding JSDoc comments for the new signalsThe new constant and signals are well-implemented, but could benefit from documentation explaining their purpose.
Consider adding JSDoc comments:
/** Key for storing error categories filter in local storage */ readonly FILTER_ERROR_CATEGORIES_KEY = 'feedbackAnalysis.errorCategories'; /** Array of available task names */ readonly taskArray = signal<string[]>([]); /** Available error categories for filtering */ readonly errorCategories = signal<string[]>([]);src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-filter-modal.component.html (1)
46-65
: LGTM! Consider adding aria-label for better accessibility.The implementation follows best practices, uses the new Angular syntax correctly, and maintains consistency with other filter sections.
Consider adding aria-label attributes to improve accessibility:
<input type="checkbox" class="form-check-input" id="errorCategory-{{ errorCategory }}" [value]="errorCategory" (change)="onCheckboxChange($event, 'errorCategories')" [checked]="filters.errorCategories.includes(errorCategory)" + [attr.aria-label]="'Filter by error category: ' + errorCategory" />
src/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.java (1)
78-84
: Consider optimizing the taskName query for performance and maintainability.The current implementation has several potential concerns:
- The subquery with JOIN and MAX could impact performance for large datasets
- Complex SQL logic embedded in an enum reduces maintainability
Consider these improvements:
- Move the SQL query to a dedicated repository method:
@Query(""" SELECT COALESCE(MAX(t.taskName), '') FROM ProgrammingExerciseTask t JOIN t.testCases tct WHERE t.exercise.id = :exerciseId AND tct.testName = :testName """) String findTaskNameByExerciseAndTestCase( @Param("exerciseId") Long exerciseId, @Param("testName") String testName );
- Add an index to improve JOIN performance:
CREATE INDEX idx_test_cases_test_name ON programming_exercise_test_cases(test_name);Would you like me to help create a separate issue for implementing these improvements?
🧰 Tools
🪛 ast-grep
[warning] 73-84: Detected a cookie where the
HttpOnly
flag is either missing or disabled. TheHttpOnly
cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnly
flag to true` in all other cases.
Context: (Map.of(
"count", "COUNT(f.id)",
"detailText", "f.detailText",
"testCaseName", "f.testCase.testName",
"taskName", """
COALESCE((
SELECT MAX(t.taskName)
FROM ProgrammingExerciseTask t
JOIN t.testCases tct
WHERE t.exercise.id = :exerciseId AND tct.testName = f.testCase.testName
), '')"""
))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 73-84: Detected a cookie where the
Secure
flag is either missing or disabled. TheSecure
cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecure
flag totrue
so the cookie will only be sent over HTTPS.
Context: (Map.of(
"count", "COUNT(f.id)",
"detailText", "f.detailText",
"testCaseName", "f.testCase.testName",
"taskName", """
COALESCE((
SELECT MAX(t.taskName)
FROM ProgrammingExerciseTask t
JOIN t.testCases tct
WHERE t.exercise.id = :exerciseId AND tct.testName = f.testCase.testName
), '')"""
))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfigurationsrc/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-filter-modal.component.spec.ts (2)
52-52
: Consider using consistent test data across test cases.The test correctly verifies error category storage, but consider extracting the test values 'Student Error' and 'Ares Error' into constants at the top of the file for reuse across test cases.
const TEST_ERROR_CATEGORIES = ['Student Error', 'Ares Error'];Also applies to: 58-58
85-110
: Consider improving event mocking and test structure.While the test cases provide good coverage, consider these improvements:
- Use proper TypeScript types for event mocking instead of
unknown
:interface MockCheckboxEvent { target: { checked: boolean; value: string; }; }
- Extract repeated test setup into beforeEach:
beforeEach(() => { component.filters = { tasks: [], testCases: [], occurrence: [0, 10], errorCategories: [] }; });
- Use
toHaveBeenCalledExactlyOnceWith
for more specific assertions as per coding guidelines.src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html (3)
3-8
: LGTM! Consider adding aria-sort for accessibility.The flex container and new @if syntax look good. The sort icon implementation is clean and modular.
Consider adding
aria-sort
attribute to improve accessibility:- <th scope="col" [class.clickable]="isSortableColumn(column)" (click)="isSortableColumn(column) && setSortedColumn(column)"> + <th scope="col" [class.clickable]="isSortableColumn(column)" (click)="isSortableColumn(column) && setSortedColumn(column)" + [attr.aria-sort]="isSortableColumn(column) ? getSortDirection(column) : null">
37-44
: LGTM! Consider min-width for better responsive behavior.The column width distribution looks good, but consider adding min-width to prevent content squishing on smaller screens.
Consider adding min-width to maintain readability:
- <col style="width: 62.5%" /> + <col style="width: 62.5%; min-width: 300px" />
68-88
: LGTM! Consider trackBy for better performance.The table body implementation is clean and follows the new Angular syntax guidelines.
Consider using a trackBy function instead of tracking the entire item for better performance:
- @for (item of content().resultsOnPage; track item) { + @for (item of content().resultsOnPage; track trackByFn(item)) {Add to your component:
trackByFn(item: any): string { return `${item.taskName}-${item.testCaseName}-${item.detailText}`; }src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts (2)
122-122
: LGTM: Sorting implementation is clean and type-safe.The sorting functionality is well-implemented. Consider extracting the sortable columns into a constant for better maintainability.
+ private readonly SORTABLE_COLUMNS = ['count', 'detailText', 'testCaseName', 'taskName'] as const; + type SortableColumn = typeof SORTABLE_COLUMNS[number]; + isSortableColumn(column: string): boolean { - return ['count', 'detailText', 'testCaseName', 'taskName'].includes(column); + return this.SORTABLE_COLUMNS.includes(column as SortableColumn); }Also applies to: 135-141
Line range hint
63-67
: Consider implementing memory leak prevention.While the component is well-structured, consider implementing the following improvements:
- Implement
ngOnDestroy
to clean up subscriptions- Use
takeUntilDestroyed
operator for the debounced load dataprivate readonly destroy$ = new Subject<void>(); constructor() { effect(() => { untracked(async () => { await this.loadData(); }); }, { injector: inject(DestroyRef) }); // Auto-cleanup of effect } ngOnDestroy() { this.destroy$.next(); this.destroy$.complete(); }src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts (2)
Line range hint
118-135
: Enhance test setup with explicit type annotations.While the test setup is correct, consider adding explicit type annotations for the filter object to improve code maintainability.
- const filters = { + const filters: FilterData = { tasks: ['task1'], testCases: ['testCase1'], occurrence: [component.minCount(), 5], errorCategories: ['Student Error'], };
Based on the gathered information, I can now provide a final response.
Test coverage needs improvement in specific areas
The test suite requires additional test cases for comprehensive coverage:
- No tests found for empty feedback table message display
- Pagination tests only cover basic page changes but miss edge cases and bounds handling
- Error category testing is limited to basic cases ('Student Error', 'AST Error') and lacks comprehensive filter combination tests
- Service tests only cover 'StudentError' category, missing other categories like 'AST Error' and 'Ares Error'
🔗 Analysis chain
Line range hint
1-240
: Verify test coverage for all PR objectives.The test suite covers the core functionality well, but consider using a coverage tool to ensure complete coverage of:
- Empty feedback table message display
- Pagination bounds handling
- All error category filter combinations
Let me gather more information about the test coverage by examining the test files found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check test coverage for feedback analysis component jest --coverage src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts # Verify all error categories are tested rg -l "errorCategory" src/test/javascript/spec/Length of output: 579
Script:
#!/bin/bash # Check the content of test files for error category testing echo "=== Feedback Analysis Component Tests ===" rg "errorCategory" -B 2 -A 2 src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts echo -e "\n=== Feedback Analysis Service Tests ===" rg "errorCategory" -B 2 -A 2 src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts echo -e "\n=== Feedback Modal Tests ===" rg "errorCategory" -B 2 -A 2 src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-modal.component.spec.ts # Check for empty table message tests echo -e "\n=== Empty Table Message Tests ===" rg -i "empty|no.*results" src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts # Check for pagination tests echo -e "\n=== Pagination Tests ===" rg "page|setPage" -B 2 -A 2 src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.tsLength of output: 2503
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (1)
286-328
: Documentation improvements suggested.The documentation is comprehensive and well-structured. However, consider these refinements for better consistency and maintainability:
- Use consistent HTML formatting (prefer
<ul>
over<br>
for lists).- Align response structure documentation with Java records pattern.
Apply these changes to improve the documentation:
* GET /exercises/{exerciseId}/feedback-details : Retrieves paginated and filtered aggregated feedback details for a specified exercise. - * <br> + * * This endpoint provides detailed feedback analytics, including: - * - The count and relative count (percentage) of each unique feedback entry. - * - Associated test case names. - * - Task names, mapped from test cases. - * <br> + * <ul> + * <li>The count and relative count (percentage) of each unique feedback entry</li> + * <li>Associated test case names</li> + * <li>Task names, mapped from test cases</li> + * </ul> * * Pagination, sorting, and filtering options allow flexible data retrieval: * <ul> * <li><b>Pagination:</b> Based on page number and page size, as specified in the request.</li> * <li><b>Sorting:</b> By column (e.g., "count" or "detailText") and sorting order (ASCENDING or DESCENDING). * If the specified column is not valid for sorting, the default sorting column is "count".</li> * <li><b>Filtering:</b> * <ul> * <li><b>Task names:</b> Filters feedback entries by specific task names, including "Not assigned to task" if unassigned feedback is requested.</li> * <li><b>Test case names:</b> Filters by specified test cases, using only active test cases from the exercise.</li> * <li><b>Occurrence range:</b> Filters by the minimum and maximum number of occurrences (inclusive).</li> * <li><b>Search term:</b> Case-insensitive filter applied to feedback detail text.</li> * <li><b>Error categories:</b> Filters feedback entries by specified error categories (e.g., "Student Error," "Ares Error," and "AST Error").</li> * </ul> * </li> * </ul> * * @param exerciseId The unique identifier of the exercise for which feedback details are requested. * @param data A {@link FeedbackPageableDTO} object containing pagination, sorting, and filtering parameters, including: - * <ul> - * <li>Page number and page size</li> - * <li>Search term (optional)</li> - * <li>Sorting order (ASCENDING or DESCENDING)</li> - * <li>Sorted column</li> - * <li>Filter task names (optional)</li> - * <li>Filter test case names (optional)</li> - * <li>Occurrence range (optional)</li> - * <li>Error categories (optional)</li> - * </ul> + * See {@link FeedbackPageableDTO} for detailed parameter descriptions. * @return A {@link ResponseEntity} containing a {@link FeedbackAnalysisResponseDTO}, which includes: * <ul> * <li>{@link SearchResultPageDTO < FeedbackDetailDTO >} feedbackDetails: Paginated and filtered feedback details for the exercise.</li> * <li>long totalItems: The total count of feedback entries (for pagination).</li> * <li>Set<String> taskNames: A set of task names relevant to the feedback items, including "Not assigned to task" if applicable.</li> * <li>List<String> testCaseNames: A list of active test case names used in the feedback.</li> * <li>List<String> errorCategories: The list of error categories included in the feedback details, such as "Student Error," "Ares Error," and "AST Error".</li> * </ul>src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java (1)
756-756
: Consider using constants for error categories.The error categories are hardcoded in the test. Consider defining these as constants to maintain consistency and make updates easier across tests.
+ private static final Set<String> EXPECTED_ERROR_CATEGORIES = Set.of("Student Error", "Ares Error", "AST Error"); ... - assertThat(response.errorCategories()).containsExactlyInAnyOrder("Student Error", "Ares Error", "AST Error"); + assertThat(response.errorCategories()).containsExactlyInAnyOrderElementsOf(EXPECTED_ERROR_CATEGORIES);src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java (1)
620-621
: Consider defining ERROR_CATEGORIES as a static final class constantTo improve reusability and maintainability, consider defining
ERROR_CATEGORIES
as astatic final
constant at the class level.Suggested change:
+private static final List<String> ERROR_CATEGORIES = List.of("Student Error", "Ares Error", "AST Error"); ... - final List<String> ERROR_CATEGORIES = List.of("Student Error", "Ares Error", "AST Error");
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (23)
- src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAnalysisResponseDTO.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackPageableDTO.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java (1 hunks)
- src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-filter-modal.component.html (1 hunks)
- src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-filter-modal.component.ts (4 hunks)
- src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-modal.component.html (1 hunks)
- src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html (4 hunks)
- src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts (8 hunks)
- src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts (2 hunks)
- src/main/webapp/app/shared/sort/sort-icon.component.html (1 hunks)
- src/main/webapp/app/shared/sort/sort-icon.component.scss (1 hunks)
- src/main/webapp/app/shared/sort/sort-icon.component.ts (1 hunks)
- src/main/webapp/i18n/de/programmingExercise.json (2 hunks)
- src/main/webapp/i18n/en/programmingExercise.json (2 hunks)
- src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java (4 hunks)
- src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts (7 hunks)
- src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts (2 hunks)
- src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-filter-modal.component.spec.ts (3 hunks)
- src/test/javascript/spec/util/shared/sort-icon.component.spec.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.java
- src/main/webapp/app/shared/sort/sort-icon.component.scss
🧰 Additional context used
📓 Path-based instructions (20)
src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAnalysisResponseDTO.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_importssrc/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackPageableDTO.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_importssrc/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_importssrc/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.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_importssrc/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.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_importssrc/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.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_importssrc/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-filter-modal.component.html (1)
Pattern
src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-filter-modal.component.ts (1)
src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-modal.component.html (1)
Pattern
src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html (1)
Pattern
src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts (1)
src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts (1)
src/main/webapp/app/shared/sort/sort-icon.component.html (1)
Pattern
src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.src/main/webapp/app/shared/sort/sort-icon.component.ts (1)
src/main/webapp/i18n/de/programmingExercise.json (1)
Pattern
src/main/webapp/i18n/de/**/*.json
: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java (1)
Pattern
src/test/java/**/*.java
: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: truesrc/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts (1)
Pattern
src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts (1)
Pattern
src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-filter-modal.component.spec.ts (1)
Pattern
src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}src/test/javascript/spec/util/shared/sort-icon.component.spec.ts (1)
Pattern
src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
🪛 ast-grep
src/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.java
[warning] 73-84: Detected a cookie where the
HttpOnly
flag is either missing or disabled. TheHttpOnly
cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnly
flag to true` in all other cases.
Context: (Map.of(
"count", "COUNT(f.id)",
"detailText", "f.detailText",
"testCaseName", "f.testCase.testName",
"taskName", """
COALESCE((
SELECT MAX(t.taskName)
FROM ProgrammingExerciseTask t
JOIN t.testCases tct
WHERE t.exercise.id = :exerciseId AND tct.testName = f.testCase.testName
), '')"""
))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 73-84: Detected a cookie where the
Secure
flag is either missing or disabled. TheSecure
cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecure
flag totrue
so the cookie will only be sent over HTTPS.
Context: (Map.of(
"count", "COUNT(f.id)",
"detailText", "f.detailText",
"testCaseName", "f.testCase.testName",
"taskName", """
COALESCE((
SELECT MAX(t.taskName)
FROM ProgrammingExerciseTask t
JOIN t.testCases tct
WHERE t.exercise.id = :exerciseId AND tct.testName = f.testCase.testName
), '')"""
))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
🪛 GitHub Check: client-style
src/main/webapp/app/shared/sort/sort-icon.component.ts
[warning] 7-7:
The selector should start with one of these prefixes: "jhi" (https://angular.dev/style-guide#style-02-07)
🔇 Additional comments (29)
src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAnalysisResponseDTO.java (1)
4-4
: LGTM! Clean import statement.The specific import for Set follows best practices by avoiding wildcard imports.
src/main/webapp/app/shared/sort/sort-icon.component.ts (2)
1-4
: LGTM! Clean and efficient imports.The imports are well-organized and follow best practices by importing only the necessary modules and specific icons.
1-21
: Verify integration with feedback analysis feature.Since this component is part of the feedback analysis enhancement, let's verify its integration.
✅ Verification successful
Integration with feedback analysis feature is properly implemented
The sort-icon component is correctly integrated within the feedback analysis feature:
- Used in table headers for sortable columns via
<app-sort-icon [direction]="getSortDirection(column)"></app-sort-icon>
- Properly imported and declared in the component's imports
- Sorting direction is correctly managed through the
getSortDirection()
method which returns the expected 'asc' | 'desc' | 'none' values- Sorting functionality is fully implemented with column sorting and order switching
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of the sort-icon component in feedback analysis rg -l "app-sort-icon" --type html | grep -i "feedback" # Verify sorting functionality implementation rg -A 5 "direction=" --type html | grep -i "feedback"Length of output: 212
Script:
#!/bin/bash # Let's check the actual implementation in the feedback analysis component cat src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html # Also check the corresponding TypeScript file for sorting logic cat src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.tsLength of output: 14478
🧰 Tools
🪛 GitHub Check: client-style
[warning] 7-7:
The selector should start with one of these prefixes: "jhi" (https://angular.dev/style-guide#style-02-07)src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackPageableDTO.java (2)
17-18
: LGTM! Field declaration follows established patterns.The new field follows consistent naming and type patterns with other filter fields in the class.
55-57
: LGTM! Setter follows established patterns.The setter method is consistent with other setters in the class and follows Java bean conventions.
src/test/javascript/spec/util/shared/sort-icon.component.spec.ts (1)
18-23
: LGTM! Test cases follow best practices.The test cases:
- Properly set input values using componentRef.setInput
- Call detectChanges at appropriate times
- Use correct assertion methods (toBeTrue/toBeFalse)
- Cover all valid scenarios
Also applies to: 25-30, 32-37
src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts (1)
45-45
: LGTM!The filters object correctly includes the new errorCategories array, maintaining consistency with the service implementation.
src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-filter-modal.component.ts (3)
12-12
: LGTM: Interface property addition follows conventionsThe new
errorCategories
property is well-typed and follows TypeScript naming conventions.
43-43
: LGTM: Consistent filter initializationThe
errorCategories
filter is properly initialized following the same pattern as other filters.
1-1
: Verify lazy loading configurationThe component is properly configured as standalone, but we should verify it's properly set up for lazy loading in the routing configuration.
src/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.java (1)
78-84
: Verify NULL handling in the taskName query.The COALESCE with empty string might mask NULL values, making it harder to distinguish between "no task name exists" and "task name is intentionally empty".
Let's verify the NULL handling requirements:
✅ Verification successful
Based on the search results, we can see that taskName is used in a consistent way across the codebase, particularly in test cases and repositories. The COALESCE with empty string in the query appears to be an intentional design choice to handle cases where no task name exists, as evidenced by:
- The same pattern is used in multiple repository queries (StudentParticipationRepository)
- Test cases show that taskName is a meaningful identifier that's explicitly set and checked
- No validation constraints or special NULL handling requirements were found in the codebase
The current NULL handling with COALESCE is appropriate.
The empty string fallback provides a consistent interface for UI/client code while maintaining the ability to distinguish between "no task exists" (query returns empty string) and "task exists but has no name" (task exists in join but has NULL name) through the original database records.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for NULL handling patterns in similar contexts rg -g '*.java' -A 5 'COALESCE.*taskName' # Check if empty strings are used elsewhere for task names rg -g '*.java' 'taskName\s*=\s*""'Length of output: 79
Script:
#!/bin/bash # Let's search for broader patterns related to taskName and NULL handling rg -g '*.java' -B 2 -A 2 'taskName.*NULL|NULL.*taskName' # Search for ProgrammingExerciseTask usage patterns ast-grep --pattern 'class $_ { $$$ taskName $$$ }' # Look for any SQL queries or JPA operations involving taskName rg -g '*.java' -B 2 -A 2 'SELECT.*taskName|FROM.*ProgrammingExerciseTask' # Check repository tests for taskName handling rg -g '*Test.java' 'taskName'Length of output: 8333
Script:
#!/bin/bash # Let's check the entity class to understand taskName's constraints and usage ast-grep --pattern 'class ProgrammingExerciseTask { $$$ }' # Look for service layer handling of taskName rg -g '*.java' -B 3 -A 3 'ProgrammingExerciseTask.*service|service.*ProgrammingExerciseTask' # Check for any validation or business logic around taskName rg -g '*.java' 'taskName.*validate|validate.*taskName'Length of output: 11427
🧰 Tools
🪛 ast-grep
[warning] 73-84: Detected a cookie where the
HttpOnly
flag is either missing or disabled. TheHttpOnly
cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnly
flag to true` in all other cases.
Context: (Map.of(
"count", "COUNT(f.id)",
"detailText", "f.detailText",
"testCaseName", "f.testCase.testName",
"taskName", """
COALESCE((
SELECT MAX(t.taskName)
FROM ProgrammingExerciseTask t
JOIN t.testCases tct
WHERE t.exercise.id = :exerciseId AND tct.testName = f.testCase.testName
), '')"""
))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 73-84: Detected a cookie where the
Secure
flag is either missing or disabled. TheSecure
cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecure
flag totrue
so the cookie will only be sent over HTTPS.
Context: (Map.of(
"count", "COUNT(f.id)",
"detailText", "f.detailText",
"testCaseName", "f.testCase.testName",
"taskName", """
COALESCE((
SELECT MAX(t.taskName)
FROM ProgrammingExerciseTask t
JOIN t.testCases tct
WHERE t.exercise.id = :exerciseId AND tct.testName = f.testCase.testName
), '')"""
))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfigurationsrc/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-filter-modal.component.spec.ts (3)
35-35
: LGTM: Filter initialization properly includes error categories.The addition of
errorCategories
array to the filters object follows the existing pattern and matches the PR objectives.Also applies to: 42-42
73-73
: LGTM: Filter clearing properly handles error categories.The test correctly verifies both the localStorage clearing and filter reset for error categories.
Also applies to: 79-79
Line range hint
1-116
: Verify test coverage for edge cases.While the test coverage is good for the main functionality, consider adding these test cases:
- Handling of invalid error category values
- Behavior when maximum number of filters is selected
- Case sensitivity in error category matching
src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html (3)
11-17
: LGTM! Well-structured empty state handling.The empty state template provides clear user feedback with appropriate messages for different scenarios (filtered vs. no data).
54-54
: LGTM! Column headers align with PR objectives.The replacement of taskNumber with taskName and addition of errorCategory align with the PR's goal of enhancing feedback analysis.
Also applies to: 62-62
99-100
: LGTM! Pagination limit improves UI clarity.Setting maxSize to 5 helps maintain a clean pagination interface without overwhelming the user.
src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts (5)
12-12
: LGTM: Import and component configuration changes are correct.The addition of
SortIconComponent
aligns with the PR objective of making columns sortable and follows Angular's style guide.Also applies to: 19-19
74-74
: LGTM: Data loading implementation is robust.The integration of error categories into the data loading flow is well-implemented with proper error handling and state management.
Also applies to: 82-82, 92-92, 97-97, 99-99
146-146
: LGTM: Filter modal integration is complete.The error categories are properly integrated into the filter modal with consistent patterns for state management and data passing.
Also applies to: 153-153, 156-156, 161-161
181-183
: LGTM: Filter counting logic is consistent.The error categories are properly integrated into the filter counting mechanism.
32-32
: Verify the impact of increased page size and text length.While the changes support the new features, please verify:
- The increased page size (25 items) doesn't impact performance or load times
- The increased feedback text length (200 chars) doesn't cause UI layout issues
Also applies to: 47-47, 52-52, 54-58
✅ Verification successful
Let me gather more information about the pagination and performance aspects.
Based on the analysis of the search results, I can now provide a final response:
The page size and text length changes are safe to implement
The increased page size (25) is well within normal range:
- Other components in the codebase use similar or larger page sizes (e.g., conversation components use 50, learning paths table uses 100)
- The feedback analysis component handles pagination through standard mechanisms used across the application
The text length handling (200 chars) is properly managed:
- The template (feedback-analysis.component.html) already implements text truncation with ellipsis
- The UI safely handles longer texts by showing only first 100 characters with "..." when exceeding the limit
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for pagination-related performance tests rg -l "pageSize.*performance|load.*test.*page" --type ts # Search for feedback text length handling in templates rg "MAX_FEEDBACK_DETAIL_TEXT_LENGTH|feedbackDetail.*text" --type htmlLength of output: 392
Script:
#!/bin/bash # Search for pageSize usage in components and services rg "pageSize" -A 3 --type ts # Look for any performance or load testing related files fd "performance|load" --type f --extension ts # Search for feedback text display related code ast-grep --pattern 'detailText.length > $_'Length of output: 45139
src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts (1)
20-43
: LGTM! Mock data structure aligns with the new requirements.The mock data correctly implements the new error categories and task name changes, providing good test coverage for different error types.
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (1)
Line range hint
331-334
: LGTM! Clean implementation following best practices.The method implementation:
- Uses proper REST and security annotations
- Follows single responsibility principle by delegating to service layer
- Uses appropriate parameter binding
src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java (2)
746-746
: LGTM: URL parameter addition for error categories.The addition of
filterErrorCategories
parameter aligns with the PR objectives and follows the existing URL parameter pattern.
812-812
: Duplicate error categories assertion found.This assertion duplicates the error categories from the previous test. Using the suggested constants would help avoid this duplication and ensure consistency.
src/main/webapp/i18n/en/programmingExercise.json (2)
357-358
: LGTM! Clear and helpful empty state messages.The messages effectively communicate two distinct scenarios to users:
- When no feedback entries exist for the exercise
- When no entries match the current filter/search criteria
368-368
: LGTM! Error category filter option added.The new filter option aligns with the PR objectives to enhance feedback analysis with error categories.
src/main/webapp/i18n/de/programmingExercise.json (1)
368-368
: LGTM!The translation "Fehlerkategorien" is correct and appropriate for this context.
src/test/javascript/spec/util/shared/sort-icon.component.spec.ts
Outdated
Show resolved
Hide resolved
...bapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts
Show resolved
Hide resolved
...p/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-modal.component.html
Outdated
Show resolved
Hide resolved
...ript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code LGTM
…tegorize-feedback-in-table' into feature/programming-exercises/categorize-feedback-in-table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on TS1 and everything worked as expected. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We talked about a few small things that will be implemented/improved in the next PR.
Programming exercises
: Add Error Categories and Categorize FeedbackProgramming exercises
: Add error categories and categorize feedback in grading analysis
Checklist
General
Server
Client
authorities
to all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
This is a follow-up to the feature I’m working on: #9315. The goal is to provide instructors with clearer insights into the feedback given to students. In this PR, error categories have been introduced, and a message is displayed when the table is empty. Additionally, minor adjustments were made: pagination now remains within bounds, and the previously introduced taskNumber was replaced with taskName, as it is more useful.
Description
The query was updated to accept filterErrorCategorie for more accurate filtering. Additionally, the taskNumber logic was completely removed and replaced by taskName, which is now stored directly in the database. All columns, except for Error Category, are now sortable.
Additionally a new shared component was introduced which allows sorting columns with a double arrow icon and depending on how we sort the column the remaining column arrows will be put as inactive and set to grey.
Steps for Testing
Prerequisites:
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
Code Review
Manual Tests
Performance Tests
Test Coverage
Screenshots
Before:
After:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Localization
Tests