Programming exercises: Add affected students to feedback analysis table
#9728
Conversation
github-actions
bot
added
tests
server
WalkthroughThis pull request introduces several updates to the feedback handling system within the application. A new record class Changes
Suggested labels
Suggested reviewers
Possibly related PRs
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (20)
src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.java (1)
6-6: Rename field to better reflect its purpose.The name
concatenatedFeedbackIdsimplies string concatenation, which might not accurately represent its intended use. Consider using a more precise name based on the actual type and purpose.If it's a collection:
-public record FeedbackDetailDTO(Object concatenatedFeedbackIds, long count, double relativeCount, String detailText, String testCaseName, String taskName, String errorCategory) { +public record FeedbackDetailDTO(List<Long> associatedFeedbackIds, long count, double relativeCount, String detailText, String testCaseName, String taskName, String errorCategory) {src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-modal.component.spec.ts (4)
Line range hint
1-5: Use NgMocks instead of direct component importAccording to our testing guidelines, we should use NgMocks for component testing to ensure proper isolation.
-import { FeedbackModalComponent } from 'app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-modal.component'; +import { MockBuilder, MockRender } from '@ng-mock/core';
Line range hint
33-38: Update assertions to follow testing guidelinesThe test assertions should follow our recommended patterns for better specificity and consistency.
it('should initialize with the provided feedback detail', () => { - expect(component.feedbackDetail()).toEqual(mockFeedbackDetail); - expect(component.feedbackDetail().detailText).toBe('Some feedback detail'); - expect(component.feedbackDetail().testCaseName).toBe('testCase1'); + expect(component.feedbackDetail()).toContainEntries(Object.entries(mockFeedbackDetail)); + expect(component.feedbackDetail().detailText).toEqual('Some feedback detail'); + expect(component.feedbackDetail().testCaseName).toEqual('testCase1'); });
Line range hint
40-44: Update spy assertion to follow testing guidelinesThe spy assertion should use the recommended pattern for verifying calls.
it('should call close on activeModal when close is triggered', () => { const closeSpy = jest.spyOn(activeModal, 'close'); component.activeModal.close(); - expect(closeSpy).toHaveBeenCalledOnce(); + expect(closeSpy).toHaveBeenCalledExactlyOnceWith(); });
Line range hint
1-45: Add missing test coverage for affected students functionalityBased on the PR objectives, this modal should display affected students and provide links to their repositories. However, the current test suite doesn't cover these critical features.
Please add the following test cases:
- Test for loading affected students when the modal opens
- Test for displaying student repository links
- Test for pagination of affected students (if implemented)
- Test for error handling when loading students fails
Example test structure:
it('should load affected students when modal opens', async () => { // Setup mock service response // Verify students are loaded using concatenatedFeedbackIds // Verify display of student data }); it('should display correct repository links for each student', () => { // Verify link generation // Verify link accessibility }); it('should handle pagination of affected students', async () => { // Test page navigation // Verify correct data loading }); it('should handle errors when loading affected students fails', async () => { // Mock service error // Verify error handling // Verify user feedback });src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.html (1)
36-41: Consider adding aria-label to pagination controls.While the pagination implementation is functional, consider enhancing accessibility by adding an aria-label to the pagination component.
- <ngb-pagination [collectionSize]="collectionsSize()" [pageSize]="pageSize()" [page]="page()" (pageChange)="setPage($event)" size="sm" [maxSize]="5"></ngb-pagination> + <ngb-pagination + [collectionSize]="collectionsSize()" + [pageSize]="pageSize()" + [page]="page()" + (pageChange)="setPage($event)" + size="sm" + [maxSize]="5" + aria-label="Navigate affected students pages"> + </ngb-pagination>src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.ts (3)
1-15: LGTM! Consider organizing imports.The component setup follows Angular best practices with standalone configuration and proper module imports. The use of signals and computed properties aligns with modern Angular patterns.
Consider organizing imports by category (Angular core, shared modules, services) with a blank line between categories:
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 { AlertService } from 'app/core/util/alert.service'; import { PageableResult, PageableSearch, SortingOrder } from 'app/shared/table/pageable-table'; import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
17-29: LGTM! Consider memory leak prevention.The component properly uses signals for state management and follows Angular's dependency injection patterns. The required inputs and state management are well-structured.
Consider implementing
OnDestroyto clean up any potential subscriptions or signal effects:export class AffectedStudentsModalComponent implements OnDestroy { private readonly destroy = new Subject<void>(); ngOnDestroy(): void { this.destroy.next(); this.destroy.complete(); } }
55-58: Consider pagination improvements.The pagination handling could be enhanced with validation and debouncing.
Consider these improvements:
private readonly pageChange = new Subject<number>(); constructor() { // Setup page change subscription this.pageChange.pipe( takeUntil(this.destroy), debounceTime(300), filter(page => page > 0 && page <= this.participation().totalPages) ).subscribe(page => { this.page.set(page); this.loadAffected(); }); } setPage(newPage: number): void { this.pageChange.next(newPage); }src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts (2)
23-30: Consider adding JSDoc documentation for the DTOWhile the interface is well-structured, adding JSDoc documentation would improve code maintainability by describing the purpose of each field and any constraints.
Here's a suggested documentation:
+/** + * Data Transfer Object representing a student affected by specific feedback + * @property courseId - The ID of the course the student is enrolled in + * @property participationId - The ID of the student's participation + * @property firstName - Student's first name + * @property lastName - Student's last name + * @property login - Student's login username + * @property repositoryName - Name of the student's repository + */ export interface FeedbackAffectedStudentDTO { courseId: number; participationId: number; firstName: string; lastName: string; login: string; repositoryName: string; }
52-61: Consider adding input validation and error handlingThe implementation is solid but could be enhanced with:
- Input validation for feedbackIds array (empty check)
- Error handling for failed HTTP requests
Consider this enhancement:
async getParticipationForFeedbackIds(exerciseId: number, feedbackIds: string[], pageable: PageableSearch): Promise<PageableResult<FeedbackAffectedStudentDTO>> { + if (!feedbackIds?.length) { + throw new Error('feedbackIds array cannot be empty'); + } + 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<PageableResult<FeedbackAffectedStudentDTO>>(`exercises/${exerciseId}/feedback-details-participation`, { params }); + try { + return await this.get<PageableResult<FeedbackAffectedStudentDTO>>(`exercises/${exerciseId}/feedback-details-participation`, { params }); + } catch (error) { + console.error('Failed to fetch participation details:', error); + throw error; + } }src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html (2)
38-42: Consider adding a comment about column size distributionThe column sizing is well-structured, but it would be helpful to add a comment explaining the column size distribution (11 columns total) for future maintenance.
+<!-- Column distribution: count(1) + feedback(6) + task(1) + testcase(2) + errorCategory(1) = 11 columns, leaving space for actions --> <ng-container *ngTemplateOutlet="headerTemplate; context: { column: 'count', label: TRANSLATION_BASE + '.occurrence', colSize: 'col-1' }"></ng-container>
64-64: Verify accessibility for the new affected students iconThe icon implementation looks good and aligns with the PR objectives. However, consider adding an aria-label for screen readers.
-<fa-icon [icon]="faUsers" class="ms-2 clickable" (click)="openAffectedStudentsModal(item)"></fa-icon> +<fa-icon [icon]="faUsers" class="ms-2 clickable" (click)="openAffectedStudentsModal(item)" aria-label="Show affected students"></fa-icon>src/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.java (1)
86-87: Consider adding documentation for the AFFECTED_STUDENTS mapping.While the mapping follows the established patterns, it would be beneficial to add a brief comment explaining its purpose in the feedback analysis context. Additionally, the SQL alias 'p' in "p.id" could be ambiguous - consider using a more descriptive alias like "participation.id".
Apply this diff to improve clarity:
AFFECTED_STUDENTS(Map.of( - "participationId", "p.id" + // Maps the participation ID for querying students affected by feedback + "participationId", "participation.id" ));src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts (1)
Line range hint
54-74: Enhance test coverage and description.While the test correctly verifies the basic functionality, consider these improvements:
- Make the test description more specific, e.g., "should return feedback details with concatenated IDs in the response"
- Add edge cases:
- Empty feedback IDs array
- Single feedback ID
- Large number of feedback IDs
Example implementation:
it('should return feedback details with concatenated IDs in the response', async () => { // ... existing test code ... }); it('should handle empty feedback IDs array', async () => { const emptyFeedbackDetailsMock = { ...feedbackAnalysisResponseMock, feedbackDetails: { resultsOnPage: [], numberOfPages: 0 } }; // ... test implementation });src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts (1)
189-193: Consider adding error handling to modal opening.The implementation is good but could benefit from some defensive programming practices.
Consider enhancing the implementation with error handling:
async openAffectedStudentsModal(feedbackDetail: FeedbackDetail): Promise<void> { - const modalRef = this.modalService.open(AffectedStudentsModalComponent, { centered: true, size: 'lg' }); - modalRef.componentInstance.exerciseId = this.exerciseId; - modalRef.componentInstance.feedbackDetail = signal(feedbackDetail); + try { + const modalRef = this.modalService.open(AffectedStudentsModalComponent, { centered: true, size: 'lg' }); + const instance = modalRef.componentInstance as AffectedStudentsModalComponent; + instance.exerciseId = this.exerciseId; + instance.feedbackDetail = signal(feedbackDetail); + } catch (error) { + this.alertService.error(this.TRANSLATION_BASE + '.modal.error'); + } }This enhancement:
- Adds error handling for modal opening
- Improves type safety with explicit casting
- Uses the existing alert service for consistency
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (1)
382-384: Consider adding input validation for feedbackIds.The endpoint could benefit from explicit validation of the feedbackIds parameter to handle edge cases.
public ResponseEntity<Page<FeedbackAffectedStudentDTO>> getParticipationWithFeedback(@PathVariable long exerciseId, @RequestParam List<String> feedbackIds, @ModelAttribute PageableSearchDTO<String> data) { + if (feedbackIds == null || feedbackIds.isEmpty()) { + throw new BadRequestAlertException("Feedback IDs must not be null or empty", "result", "feedbackIdsRequired"); + } Page<FeedbackAffectedStudentDTO> participation = resultService.getParticipationWithFeedbackId(exerciseId, feedbackIds, data);src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java (1)
1334-1375: Implementation looks good with minor optimization opportunities.The implementation follows good practices with proper DTO projection, pagination, and clear join statements. However, consider the following optimizations:
- Add index hints for better query performance
- Consider using LEFT JOIN FETCH for
submissionsto avoid N+1 queriesConsider this optimization:
- FROM ProgrammingExerciseStudentParticipation p - LEFT JOIN p.submissions s + FROM ProgrammingExerciseStudentParticipation p + LEFT JOIN FETCH p.submissions ssrc/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java (2)
500-533: Simplify and Enhance Null Handling in Feedback ProcessingIn the
saveFeedbackWithHibernateWorkaroundmethod, consider improving null checks and simplifying the logic for better readability and robustness.Suggested refactor:
// Avoid Hibernate exception List<Feedback> savedFeedbacks = new ArrayList<>(); // Collect IDs of feedbacks that have long feedback. List<Long> feedbackIdsWithLongFeedback = feedbackList.stream() - .filter(feedback -> feedback.getId() != null && feedback.getHasLongFeedbackText()) + .filter(feedback -> feedback != null && feedback.getId() != null && feedback.getHasLongFeedbackText()) .map(Feedback::getId) .toList(); // Get long feedback list from the database. List<LongFeedbackText> longFeedbackTextList = longFeedbackTextRepository.findByFeedbackIds(feedbackIdsWithLongFeedback); // Convert list to map for later access. Map<Long, LongFeedbackText> longFeedbackTextMap = longFeedbackTextList.stream() .collect(Collectors.toMap(longFeedbackText -> longFeedbackText.getFeedback().getId(), Function.identity())); feedbackList.forEach(feedback -> { + if (feedback == null) { + return; + } // Cut association to parent object feedback.setResult(null); LongFeedbackText longFeedback = null; // Look for long feedback that the parent feedback has and cut the association. if (feedback.getId() != null && feedback.getHasLongFeedbackText()) { longFeedback = longFeedbackTextMap.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); // Restore the association of the long feedback to the parent feedback if (longFeedback != null) { feedback.setDetailText(longFeedback.getText()); } savedFeedbacks.add(feedback); }); return savedFeedbacks;
646-655: Minor Corrections in Javadoc CommentsConsider rephrasing for clarity and conciseness in the Javadoc documentation.
Suggested changes:
-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. +This method filters students based on feedback IDs and returns participation details for each affected student. It utilizes pagination and sorting to efficiently handle large datasets.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (18)
src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAffectedStudentDTO.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/service/ResultService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java(5 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(4 hunks)src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.html(1 hunks)src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html(3 hunks)src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts(3 hunks)src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts(3 hunks)src/main/webapp/app/shared/table/pageable-table.ts(1 hunks)src/main/webapp/i18n/de/programmingExercise.json(1 hunks)src/main/webapp/i18n/en/programmingExercise.json(1 hunks)src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts(5 hunks)src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts(4 hunks)src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-affected-students-modal.component.spec.ts(1 hunks)src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-modal.component.spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAffectedStudentDTO.java
🧰 Additional context used
📓 Path-based instructions (16)
src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.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
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
src/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_imports
src/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_imports
src/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_imports
src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-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-affected-students-modal.component.ts (1)
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/table/pageable-table.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/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-affected-students-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/component/programming-exercise/feedback-analysis/modals/feedback-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}}
🔇 Additional comments (28)
src/main/webapp/app/shared/table/pageable-table.ts (1)
1-5: Add JSDoc documentation and consider property naming consistency.
The new interface looks good but could benefit from some improvements:
- Add JSDoc documentation to describe the interface purpose and generic type parameter
- Consider aligning property names with the existing
SearchResultinterface for consistency (e.g.,resultsOnPagevscontent)
+/**
+ * Represents a paginated result set.
+ * @template T The type of elements in the result set
+ */
export interface PageableResult<T> {
+ /** The current page's content */
content: T[];
+ /** The total number of elements across all pages */
totalElements: number;
+ /** The total number of available pages */
totalPages: number;
}Let's check if there are other similar pagination interfaces that we should align with:
src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-modal.component.spec.ts (1)
13-13: LGTM: New concatenatedFeedbackIds property added correctly
The mock data structure correctly includes the new property with appropriate test values.
src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.html (3)
1-4: LGTM! Modal header follows best practices.
The header implementation includes proper translation support and accessibility attributes.
6-19: LGTM! Table structure is well-organized.
The implementation uses appropriate Bootstrap classes and includes proper internationalization for headers.
20-33: LGTM! Table content iteration follows new Angular syntax.
The implementation correctly uses the new @for syntax and properly handles repository access through the code button component.
src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts (2)
2-2: LGTM: Import statement is properly structured
The new imports for pagination components are correctly added from the shared module, following Angular's best practices for code organization.
15-15: LGTM: FeedbackDetail interface extension
The addition of concatenatedFeedbackIds array aligns with the PR's objective to track feedback IDs for affected students analysis.
src/test/javascript/spec/component/programming-exercise/feedback-analysis/modals/feedback-affected-students-modal.component.spec.ts (2)
10-29: LGTM! Well-structured mock data setup.
The mock data is well-organized, follows the DTO structure, and provides realistic test scenarios.
54-61: 🛠️ Refactor suggestion
Enhance test expectations following guidelines.
The test expectations should follow our guidelines for specificity.
Apply these changes:
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();
+ expect(component.page()).toEqual(2);
+ expect(loadAffectedSpy).toHaveBeenCalledExactlyOnceWith();
});Likely invalid or redundant comment.
src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.html (2)
1-2: LGTM: Header template enhancement with dynamic column sizing
The addition of the colSize parameter and dynamic class binding improves the template's flexibility and maintainability.
72-72: LGTM: Clean pagination implementation
The pagination component is well-structured with all necessary bindings and improved formatting.
src/main/java/de/tum/cit/aet/artemis/core/util/PageUtil.java (1)
85-87: Verify integration with feedback analysis components.
The new mapping appears to be related to the feedback analysis feature. Let's verify the integration:
✅ Verification successful
Let me gather more context about the feedback analysis integration by checking the complete implementation and usage.
Let me check one final aspect to verify the complete integration chain.
Let me check the complete query implementation in the repository.
Integration with feedback analysis components is properly implemented
The verification confirms proper integration:
- The
participationIdfield inPageUtil.AFFECTED_STUDENTSmapping correctly corresponds to the DTO field inFeedbackAffectedStudentDTO - The mapping is correctly used in
ResultService.getParticipationWithFeedbackId()for pagination and sorting - The repository query in
StudentParticipationRepositoryproperly joins the necessary tables (participations, submissions, results, feedbacks) and uses the mapped field in the result projection
The implementation maintains consistency across all components of the feedback analysis feature.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the relationship between PageUtil and feedback analysis components
# Test 1: Check if FeedbackAffectedStudentDTO uses consistent field names
echo "Checking FeedbackAffectedStudentDTO field names..."
rg -t java "participationId" --glob "**/*FeedbackAffectedStudentDTO.java"
# Test 2: Check if the mapping is used in feedback-related queries
echo "Checking usage in feedback queries..."
rg -t java "AFFECTED_STUDENTS" --glob "**/*Service.java" --glob "**/*Repository.java"
Length of output: 1022
Script:
#!/bin/bash
# Check the complete implementation of PageUtil.AFFECTED_STUDENTS
echo "Checking PageUtil.AFFECTED_STUDENTS implementation..."
rg -t java "AFFECTED_STUDENTS" -B 5 -A 5
# Check the complete FeedbackAffectedStudentDTO implementation
echo "Checking FeedbackAffectedStudentDTO implementation..."
cat src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackAffectedStudentDTO.java
# Check for any related repository methods
echo "Checking repository methods..."
ast-grep --pattern 'interface $_Repository {
$$$
findAffectedStudents($$$) {
$$$
}
$$$
}'
Length of output: 4845
Script:
#!/bin/bash
# Check the repository implementation for the findParticipationByFeedbackId method
echo "Checking repository implementation..."
rg -t java "findParticipationByFeedbackId" -B 2 -A 10
# Check for any SQL queries or JPA annotations related to this feature
echo "Checking SQL/JPA queries..."
rg -t java "@Query.*AFFECTED_STUDENTS" -B 2 -A 10
Length of output: 1575
Script:
#!/bin/bash
# Check the complete query implementation in StudentParticipationRepository
echo "Checking complete repository query..."
rg -t java -B 10 -A 2 "Page<FeedbackAffectedStudentDTO> findParticipationByFeedbackId"
# Check for any related named queries or SQL fragments
echo "Checking for related SQL fragments..."
rg -t java "@Query.*participationId.*feedback" -B 5 -A 10
Length of output: 2021
src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts (1)
12-37: LGTM: Mock data structure is well-defined and comprehensive.
The mock data accurately represents the feedback analysis structure with concatenatedFeedbackIds, taskNames, and errorCategories, aligning well with the PR's objective of enhancing feedback analysis capabilities.
src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.component.ts (3)
5-5: LGTM: Icon import follows Angular style guide.
The faUsers icon is appropriately imported and grouped with other FontAwesome icons.
13-13: LGTM: Modal component import follows naming conventions.
The import statement and component naming follow Angular style guide and coding guidelines (PascalCase for types).
48-48: LGTM: Icon property follows class property conventions.
The faUsers property is correctly declared as readonly and follows camelCase naming convention.
src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts (2)
11-11: LGTM!
The import for AffectedStudentsModalComponent follows the project's conventions and is appropriately placed with other modal-related imports.
22-22: LGTM!
The mock data has been appropriately updated with concatenatedFeedbackIds to match the DTO changes. The test data values are representative and maintain consistency across mock objects.
Also applies to: 31-31
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (3)
17-17: LGTM: Import changes are appropriate.
The new imports support the added functionality while following coding guidelines (avoiding star imports).
Also applies to: 32-32, 41-41, 48-48
334-334: LGTM: Security annotation changes follow principle of least privilege.
The change from instructor to editor role is appropriate for feedback analysis functionality, providing better granularity in access control.
Also applies to: 349-349
355-386: LGTM: Well-implemented endpoint with comprehensive documentation.
The new endpoint follows REST principles, has appropriate security controls, and includes detailed JavaDoc documentation.
src/main/webapp/i18n/en/programmingExercise.json (1)
374-380: LGTM! The translations for the affected students modal are well-structured.
The added translations provide clear and consistent labels for the new modal component that displays affected students, aligning with the PR objectives and existing UI terminology.
src/main/webapp/i18n/de/programmingExercise.json (1)
374-380: LGTM! Translations follow guidelines.
The new translations for the affected students modal are properly formatted, use appropriate German terms, and correctly follow the informal language guideline.
src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java (2)
1221-1221: Documentation looks good!
The Javadoc is comprehensive and well-structured, providing clear explanations of the method's functionality and parameters.
1237-1237: Consider database-specific GROUP_CONCAT limitations.
The use of GROUP_CONCAT for feedback IDs might have limitations:
- Some databases have a maximum length limit for GROUP_CONCAT results
- Could impact performance with large datasets
Consider:
- Adding a database-specific length configuration if needed
- Implementing pagination or batching for large result sets
src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java (3)
26-26: Import Statement Added Correctly
The addition of import org.springframework.data.domain.PageRequest; is appropriate for pagination functionality.
34-34: New DTO Import for FeedbackAffectedStudentDTO
Importing FeedbackAffectedStudentDTO is necessary for the new method handling affected students.
50-50: Import PageableSearchDTO for Enhanced Pagination
Including PageableSearchDTO improves pagination and search capabilities within the service.
src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.java
Outdated
Show resolved
Hide resolved
...ramming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.ts
Show resolved
Hide resolved
...ramming/manage/grading/feedback-analysis/Modal/feedback-affected-students-modal.component.ts
Outdated
Show resolved
Hide resolved
...ramming-exercise/feedback-analysis/modals/feedback-affected-students-modal.component.spec.ts
Outdated
Show resolved
Hide resolved
...ramming-exercise/feedback-analysis/modals/feedback-affected-students-modal.component.spec.ts
Show resolved
Hide resolved
...ramming-exercise/feedback-analysis/modals/feedback-affected-students-modal.component.spec.ts
Show resolved
Hide resolved
...ript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts
Outdated
Show resolved
Hide resolved
...pt/spec/component/programming-exercise/feedback-analysis/feedback-analysis.component.spec.ts
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java
Outdated
Show resolved
Hide resolved
HawKhiem
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
zagemello
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
src/main/java/de/tum/cit/aet/artemis/assessment/dto/FeedbackDetailDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java
Outdated
Show resolved
Hide resolved
…g-exercises/add-affected-students-to-analysis-table
flbrgit
temporarily deployed
to
artemis-test5.artemis.cit.tum.de
GitHub Actions
Inactive
…d-students-to-analysis-table
…d-affected-students-to-analysis-table' into feature/programming-exercises/add-affected-students-to-analysis-table
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts (1)
Line range hint
54-74: Consider adding edge cases to strengthen test coverage.While the happy path test is well-implemented, consider adding these test cases:
- Empty concatenatedFeedbackIds array
- Error response handling
- Pagination with multiple pages
Example test case:
it('should handle empty concatenatedFeedbackIds', async () => { const mockResponse = { ...feedbackAnalysisResponseMock, feedbackDetails: { resultsOnPage: [{ ...feedbackDetailsMock[0], concatenatedFeedbackIds: [] }], numberOfPages: 1 } }; // ... test implementation });src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java (2)
Line range hint
1295-1299: Consider using window functions for better performance.The current implementation uses a correlated subquery to get the latest result. This could be optimized using window functions.
Consider refactoring the query to use ROW_NUMBER():
WITH RankedResults AS ( SELECT r.id, ROW_NUMBER() OVER (PARTITION BY p.id ORDER BY r.id DESC) as rn FROM participation p JOIN result r ON r.participation_id = p.id ) SELECT COUNT(DISTINCT r.id) FROM participation p JOIN result r ON r.id = ( SELECT rr.id FROM RankedResults rr WHERE rr.rn = 1 ) WHERE p.exercise_id = :exerciseId AND p.test_run = FALSE;
1336-1344: Documentation improvement needed.The method documentation could be more precise about:
- The order of results (currently only mentioned in the query)
- The handling of NULL cases
- The behavior when no students are found
Consider updating the documentation:
/** * Retrieves a paginated list of students affected by specific feedback entries for a given programming exercise. * <br> + * The results are ordered by student's first name in ascending order. If no students are found, returns an empty page. + * Note: Only considers participations that are not test runs and have valid 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. */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java(7 hunks)src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts(3 hunks)src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java
- src/main/webapp/app/exercises/programming/manage/grading/feedback-analysis/feedback-analysis.service.ts
🧰 Additional context used
📓 Path-based instructions (2)
src/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_imports
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}}
🔇 Additional comments (3)
src/test/javascript/spec/component/programming-exercise/feedback-analysis/feedback-analysis.service.spec.ts (1)
12-37: LGTM! Mock data structure is well-defined.
The mock data structure accurately reflects the DTO changes with proper typing and organization.
src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java (2)
32-32: LGTM!
The import statement for the new DTO is correctly placed and follows the existing import organization pattern.
1335-1363: Verify the completeness of affected students retrieval.
The query for finding affected students might miss some cases:
- It uses LEFT JOINs but doesn't handle NULL cases explicitly
- It doesn't consider the result completion status
Let's verify the query behavior:
✅ Verification successful
The LEFT JOIN query implementation is correct and complete
The query implementation for finding affected students is properly designed:
-
The LEFT JOINs are correctly used to handle NULL cases implicitly - they preserve all participations that match the WHERE clause regardless of whether they have submissions, results, or feedbacks.
-
The WHERE clause with
f.id IN :feedbackIdseffectively filters only participations that have the specified feedbacks, making NULL cases irrelevant. -
The completion status of results is not relevant for this query as we only care about finding students who received specific feedback, regardless of the result status.
-
The DTO structure (
FeedbackAffectedStudentDTO) captures all necessary student information without any dependency on result completion status.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the completeness of affected students retrieval
# Check if there are any participations with NULL results that should be included
ast-grep --pattern 'SELECT $_ FROM StudentParticipation WHERE exercise_id = $_ AND result IS NULL'
# Check if there are any results without completion date
rg -A 5 "completionDate IS NULL.*feedback.*exercise_id"
Length of output: 158
Script:
#!/bin/bash
# Let's analyze the query structure and related entities more thoroughly
# Check for other similar queries in the repository to understand the pattern
rg -l "FROM ProgrammingExerciseStudentParticipation.*JOIN.*results.*feedbacks"
# Look for Result entity definition to understand its structure
fd "Result.java|Results.java" --exec cat {}
# Look for related repository methods that might handle similar cases
ast-grep --pattern 'Page<$_> find$_ByFeedback$_'
# Check for test cases related to feedback queries
fd -e java -e groovy --full-path ".*[Tt]est.*" --exec rg -l "findAffectedStudentsByFeedbackId" {}
Length of output: 43091
Script:
#!/bin/bash
# Let's analyze the query structure and related entities more specifically
# Check for Result entity structure and its relationship with feedbacks
ast-grep --pattern 'class Result {
$$$
List<Feedback> $_ = $_
$$$
}'
# Look for similar queries with LEFT JOIN that handle NULL cases
rg -A 3 "LEFT JOIN.*feedbacks.*WHERE.*IS NULL"
# Check for any test cases or service methods that handle feedback-related queries
rg -A 5 "findAffectedStudentsByFeedbackId|findByFeedbackId"
# Look for any validation or handling of result completion status in services
rg -A 3 "completionDate.*result.*feedback"
Length of output: 6592
Script:
#!/bin/bash
# Let's examine the query implementation and related entities more closely
# Check for the FeedbackAffectedStudentDTO structure
fd "FeedbackAffectedStudentDTO.java" --exec cat {}
# Look for similar repository methods that handle student participations
ast-grep --pattern 'FROM ProgrammingExerciseStudentParticipation p
$$$
WHERE p.exercise.id = $_'
# Check for any service methods that use this repository method
rg -A 5 "studentParticipationRepository.*findAffectedStudentsByFeedbackId"
# Look for any test cases that verify this functionality
rg -A 5 "test.*findAffectedStudentsByFeedbackId"
Length of output: 1230
...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
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java (5)
886-886: Consider using constants for URL parameters.The URL string contains hardcoded values. Consider extracting these as constants for better maintainability:
-String url = "/api/exercises/" + programmingExercise.getId() + "/feedback-details-participation?page=1&pageSize=10&sortedColumn=participationId&sortingOrder=ASCENDING"; +private static final String DEFAULT_PAGE = "1"; +private static final String DEFAULT_PAGE_SIZE = "10"; +private static final String DEFAULT_SORT_COLUMN = "participationId"; +private static final String DEFAULT_SORT_ORDER = "ASCENDING"; +String url = String.format("/api/exercises/%d/feedback-details-participation?page=%s&pageSize=%s&sortedColumn=%s&sortingOrder=%s", + programmingExercise.getId(), DEFAULT_PAGE, DEFAULT_PAGE_SIZE, DEFAULT_SORT_COLUMN, DEFAULT_SORT_ORDER);
887-889: Use builder pattern for headers construction.Consider using the builder pattern for headers construction to improve readability:
-HttpHeaders headers = new HttpHeaders(); -headers.add("feedbackIds", String.valueOf(feedback.getId())); +HttpHeaders headers = HttpHeaders.builder() + .add("feedbackIds", String.valueOf(feedback.getId())) + .build();
877-882: Consider using test data builders for feedback creation.The feedback setup could be simplified using a builder pattern or utility method:
-Feedback feedback = new Feedback(); -feedback.setPositive(false); -feedback.setDetailText("Feedback for student"); -feedback.setTestCase(testCase); -feedback = feedbackRepository.saveAndFlush(feedback); +Feedback feedback = TestDataBuilder.buildFeedback() + .positive(false) + .detailText("Feedback for student") + .testCase(testCase) + .build(); +feedback = feedbackRepository.saveAndFlush(feedback);
869-871: Test method name could be more descriptive.The current test method name
testGetParticipationForFeedbackIdcould be more descriptive about what it's testing:-void testGetParticipationForFeedbackId() +void shouldReturnPaginatedParticipationsForGivenFeedbackId()
25-25: Group related imports together.The imports could be better organized by grouping related imports together:
import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.security.test.context.support.WithMockUser; + import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper;Also applies to: 29-31
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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: true
src/test/java/de/tum/cit/aet/artemis/assessment/ResultServiceIntegrationTest.java
Show resolved
Hide resolved
…d-students-to-analysis-table
krusche
deleted the
feature/programming-exercises/add-affected-students-to-analysis-table
branch
github-actions
bot
added
the
deployment-error
Checklist
General
Server
Client
authoritiesto 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: #9622. The goal is to provide instructors with clearer insights into the feedback given to students. In this PR, an “Affected Students” modal was added, allowing instructors to see which students encountered a specific feedback error. This feature is crucial for analyzing unusual feedback outputs to students, enabling instructors to review and reproduce issues using student repositories to further improve test case feedback.
Description
A new server-side query was added that takes feedbackIds as input and returns the student participations matching those feedbackIds. On the client side, instructors can now view students encountering a specific error via a modal accessible on the far right of each feedback entry. This modal also allows instructors to navigate directly to the student’s repository, enabling further investigation into why a student might be receiving a particular feedback response.
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
Enhancements
Localization
Bug Fixes