Feature/exam mode/seating distribution client #11492
Conversation
…gatable to without explicit URL
… transient fields to ExamUser
…e exam_user table
…es/ways to reference one
… needs to be an AbstractAuditingEntity, still need to figure out what to do with capacity and if I even need it but might store capacity with the layouts directly
…l need to return a DTO, still need helper functions
… code cleaned up, returns DTO now
…exam_user, various code cleanups
…unctionality for deletion of the DB
…INCT implicitly compares each field)
…eating-distribution-algorithm' into feature/exam-mode/seating-distribution-algorithm
…into feature/exam-mode/seating-distribution-algorithm
github-actions
bot
added
tests
server
WalkthroughAdds exam room distribution capability. Introduces new DTOs, repositories, services, and a REST controller on the backend; updates existing ExamRoom DTO and service. On the client, adds UI components, service, and i18n to select rooms and trigger distribution; extends admin room overview with id and default capacity. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Instructor
participant UI as Angular UI (Students page)
participant Modal as Room Distribution Dialog
participant FE as StudentsRoomDistributionService
participant API as ExamRoomDistributionResource
participant SVC as ExamRoomDistributionService
participant RepoE as ExamRepository
participant RepoR as ExamRoomRepository
participant RepoA as ExamRoomExamAssignmentRepository
participant RepoU as ExamUserRepository
participant RoomS as ExamRoomService
Instructor->>UI: Click "Distribute"
UI->>Modal: Open dialog
Modal->>FE: GET /api/exam/rooms/distribution-data
FE->>API: GET distribution-data
API->>SVC: getRoomDataForDistribution()
SVC->>RepoR: findAllCurrentExamRoomsForDistribution()
RepoR-->>SVC: Set<ExamRoomForDistributionDTO>
SVC-->>API: Set<...>
API-->>FE: 200 OK + data
FE-->>Modal: Rooms list
Instructor->>Modal: Select room(s), Finish
Modal->>FE: POST distribute-registered-students(courseId, examId, roomIds)
FE->>API: POST distribute-registered-students
API->>SVC: distributeRegisteredStudents(examId, roomIds)
SVC->>RepoE: findByIdWithRegisteredUsers(examId) (eager users)
SVC->>RepoR: findAllWithEagerLayoutStrategiesByIdIn(ids)
SVC->>RoomS: getDefaultUsableSeats(room) for each room
RoomS-->>SVC: Usable seats per room
SVC->>RepoA: deleteAllByExamId(examId)
SVC->>RepoA: save assignments for selected rooms
SVC->>RepoU: persist planned room/seat for users
SVC-->>API: void
API-->>FE: 200 OK
FE-->>Modal: Success
Modal-->>UI: Emit distributionDone
UI->>UI: Reload exam users
sequenceDiagram
autonumber
actor Instructor
participant UI as Angular UI (Typeahead)
participant FE as StudentsRoomDistributionService
participant API as ExamRoomDistributionResource
participant SVC as ExamRoomDistributionService
participant RepoR as ExamRoomRepository
Instructor->>UI: Focus and type in room search
UI->>FE: GET distribution-data
FE->>API: GET /api/exam/rooms/distribution-data
API->>SVC: getRoomDataForDistribution()
SVC->>RepoR: findAllCurrentExamRoomsForDistribution()
RepoR-->>SVC: Set<ExamRoomForDistributionDTO>
SVC-->>API: Set<...>
API-->>FE: 200 OK
FE-->>UI: HTTP response with rooms
UI->>UI: Filter and show suggestions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PMD (7.17.0)src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomDTO.java[ERROR] Error at ruleset.xml:58:5 59| ... [truncated 11412 characters] ... prone.xml/InaccurateNumericLiteral instead of the deprecated Rule name category/ecmascript/errorprone.xml/InnaccurateNumericLiteral. PMD 8.0.0 will remove support for this deprecated Rule name usage. 185| src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomForDistributionDTO.java[0.001s][warning][perf,memops] Cannot use file /tmp/hsperfdata_jailuser/1 because it is locked by another process (errno = 11) 59| ... [truncated 11539 characters] ... prone.xml/InaccurateNumericLiteral instead of the deprecated Rule name category/ecmascript/errorprone.xml/InnaccurateNumericLiteral. PMD 8.0.0 will remove support for this deprecated Rule name usage. 185| src/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.java[ERROR] Error at ruleset.xml:58:5 59| ... [truncated 11412 characters] ... prone.xml/InaccurateNumericLiteral instead of the deprecated Rule name category/ecmascript/errorprone.xml/InnaccurateNumericLiteral. PMD 8.0.0 will remove support for this deprecated Rule name usage. 185|
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
src/main/webapp/i18n/en/exam.json (1)
34-41: Clarify room column labelsThe new labels “Number 2” and “Name 2” read like placeholders and will look odd in the UI. Consider wording them as “Alternative Number” / “Alternative Name” (or similar) so users immediately understand what the second column represents.
src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-dialog.component.ts (1)
50-60: Consider using ngOnInit for initialization instead of effect().While
effect()works here, usingngOnInit()for one-time initialization is more conventional and clearer. Theeffect()API is primarily designed for reactive computations that should re-run when dependencies change.Apply this diff to refactor to ngOnInit:
- initEffect = effect(() => { + ngOnInit(): void { this.studentsRoomDistributionService.getRoomData().subscribe({ next: (result: HttpResponse<RoomForDistributionDTO[]>) => { this.availableRooms.set(result.body as RoomForDistributionDTO[]); }, error: (error: HttpErrorResponse) => { this.showErrorNotification('examRoomDataLoadFailed', {}, error.message); this.availableRooms.set(undefined); }, }); - }); + }Also update the class declaration:
-export class StudentsRoomDistributionDialogComponent implements OnDestroy { +export class StudentsRoomDistributionDialogComponent implements OnInit, OnDestroy {src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.ts (1)
228-238: Add explicit return type for calculateExamRoomData.
Improves readability and type safety.- private calculateExamRoomData() { + private calculateExamRoomData(): ExamRoomDTOExtended[] | undefined { return this.overview()?.newestUniqueExamRooms?.map( (examRoomDTO) => ({ ...examRoomDTO, defaultCapacity: this.getDefaultCapacityOfExamRoom(examRoomDTO), maxCapacity: this.getMaxCapacityOfExamRoom(examRoomDTO), layoutStrategyNames: this.getLayoutStrategyNames(examRoomDTO), }) as ExamRoomDTOExtended, ); }src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-button.component.ts (2)
41-49: Passing InputSignals into modal: verify behavior.
You assign InputSignals to the dialog’s inputs (also InputSignals). This is unconventional and may bypass Angular’s input update flow. Ensure the dialog can call courseId() and exam() safely at runtime.Prefer passing snapshot values and using plain @input in the dialog (or setInput on the component ref if accessible):
// here: modalRef.componentInstance.courseId = this.courseId(); // pass number modalRef.componentInstance.exam = this.exam(); // pass ExamThen in StudentsRoomDistributionDialogComponent, switch to classic inputs:
@Input() courseId!: number; @Input() exam!: Exam;This avoids cross-component signal lifetimes and surprises.
5-5: Minor cleanup: remove unused icons and fix comment.-import { faPlus, faThLarge, faUpload } from '@fortawesome/free-solid-svg-icons'; +import { faThLarge } from '@fortawesome/free-solid-svg-icons'; @@ - readonly faPlus = faPlus; - readonly faUpload = faUpload; readonly faThLarge = faThLarge; @@ - * Open up upload dialog for exam users image upload + * Open the room distribution dialog for exam usersAlso applies to: 33-35, 38-40
src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.java (4)
56-62: Unused dependency in constructorExamUserRepository is injected but not used in this service. Remove it to reduce coupling and improve testability, unless it’s needed imminently.
Would you like me to scan the codebase for any pending usages before removing?
258-263: Support both 'useable_seats' and 'usable_seats' layout keysJSON commonly spells it “usable”. Consider accepting both to be lenient with inputs.
- case "useable_seats" -> layoutStrategy.setType(LayoutStrategyType.FIXED_SELECTION); + case "useable_seats", "usable_seats" -> layoutStrategy.setType(LayoutStrategyType.FIXED_SELECTION);
315-318: Null-safe newest-version selectionIf createdDate can be null (e.g., pre-audited data), comparator will throw. Make it null-safe:
- Map<String, ExamRoom> newestRoomByRoomNumberAndName = examRooms.stream().collect(Collectors.toMap( - // Use null character as a separator, as it is not allowed in room numbers or names - examRoom -> examRoom.getRoomNumber() + "\u0000" + examRoom.getName(), Function.identity(), BinaryOperator.maxBy(Comparator.comparing(ExamRoom::getCreatedDate)))); + Map<String, ExamRoom> newestRoomByRoomNumberAndName = examRooms.stream().collect(Collectors.toMap( + examRoom -> examRoom.getRoomNumber() + "\u0000" + examRoom.getName(), + Function.identity(), + BinaryOperator.maxBy(Comparator.comparing(ExamRoom::getCreatedDate, Comparator.nullsLast(Comparator.naturalOrder())))));
423-426: Professionalize error messageAvoid whimsical wording in API errors.
- throw new BadRequestAlertException("Sire, the selected seat " + seatInput + " does not exist in room " + roomNumber, ENTITY_NAME, "room.seatNotFoundFixedSelection", + throw new BadRequestAlertException("Selected seat " + seatInput + " does not exist in room " + roomNumber, ENTITY_NAME, "room.seatNotFoundFixedSelection", Map.of("rowIndex", rowIndex, "seatIndex", seatIndex, "roomNumber", roomNumber));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/test/resources/test-data/exam-room/rooms-master.zipis excluded by!**/*.zip,!**/*.zip
📒 Files selected for processing (29)
src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomForDistributionDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomExamAssignmentRepository.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomRepository.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.java(10 hunks)src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.java(0 hunks)src/main/java/de/tum/cit/aet/artemis/exam/web/ExamRoomDistributionResource.java(1 hunks)src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.html(1 hunks)src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.spec.ts(2 hunks)src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.ts(4 hunks)src/main/webapp/app/core/admin/exam-rooms/exam-rooms.model.ts(2 hunks)src/main/webapp/app/exam/manage/students/exam-students.component.html(1 hunks)src/main/webapp/app/exam/manage/students/exam-students.component.ts(2 hunks)src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-button.component.ts(1 hunks)src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-dialog.component.html(1 hunks)src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-dialog.component.ts(1 hunks)src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.model.ts(1 hunks)src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.service.ts(1 hunks)src/main/webapp/i18n/de/error.json(1 hunks)src/main/webapp/i18n/de/examRooms.json(1 hunks)src/main/webapp/i18n/en/error.json(1 hunks)src/main/webapp/i18n/en/exam.json(1 hunks)src/main/webapp/i18n/en/examRooms.json(1 hunks)src/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.java(9 hunks)src/test/java/de/tum/cit/aet/artemis/exam/service/ExamRoomServiceTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/exam/test_repository/ExamRoomTestRepository.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/exam/util/ExamRoomZipFiles.java(1 hunks)supporting_scripts/code-coverage/module-coverage-client/check-client-module-coverage.mjs(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.java
🧰 Additional context used
📓 Path-based instructions (5)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/exam/manage/students/exam-students.component.tssrc/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.model.tssrc/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.spec.tssrc/main/webapp/app/core/admin/exam-rooms/exam-rooms.model.tssrc/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-dialog.component.tssrc/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.tssrc/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.service.tssrc/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-button.component.ts
src/main/webapp/**/*.html
⚙️ CodeRabbit configuration file
@if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
Files:
src/main/webapp/app/exam/manage/students/exam-students.component.htmlsrc/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.htmlsrc/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-dialog.component.html
src/test/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/test/java/de/tum/cit/aet/artemis/exam/test_repository/ExamRoomTestRepository.javasrc/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/exam/util/ExamRoomZipFiles.javasrc/test/java/de/tum/cit/aet/artemis/exam/service/ExamRoomServiceTest.java
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomForDistributionDTO.javasrc/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomExamAssignmentRepository.javasrc/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.javasrc/main/java/de/tum/cit/aet/artemis/exam/web/ExamRoomDistributionResource.javasrc/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomDTO.javasrc/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.javasrc/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomRepository.java
src/main/webapp/i18n/de/**/*.json
⚙️ CodeRabbit configuration file
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".
Files:
src/main/webapp/i18n/de/error.jsonsrc/main/webapp/i18n/de/examRooms.json
🧠 Learnings (10)
📓 Common learnings
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11419
File: src/main/java/de/tum/cit/aet/artemis/exam/web/ExamRoomDistributionResource.java:62-78
Timestamp: 2025-09-25T20:43:41.712Z
Learning: In the ExamRoomDistributionService.distributeRegisteredStudents method in src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java, ExamRoom entities are shared resources that are dynamically assigned to exams through the ExamRoomExamAssignment join table. Rooms don't "belong" to exams until the distribution process creates these assignments. The method first clears existing assignments for the exam, then creates new ones for the provided room IDs.
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11378
File: src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java:13-16
Timestamp: 2025-09-15T11:18:26.439Z
Learning: In ExamRoomDistributionService.distributeRegisteredStudents method in src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java, the team has decided not to use Transactional annotation despite multiple repository operations, based on human reviewer consultation.
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11378
File: src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java:72-79
Timestamp: 2025-09-15T11:21:15.983Z
Learning: SamuelRoettgermann prefers iterative development approach - initial implementations should be working starting points rather than full-fledged, finished implementations. Optimization and robustness improvements can be added later.
📚 Learning: 2025-09-25T20:48:13.391Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11419
File: src/test/java/de/tum/cit/aet/artemis/exam/test_repository/ExamRoomTestRepository.java:18-34
Timestamp: 2025-09-25T20:48:13.391Z
Learning: In ExamRoomTestRepository JPQL queries in src/test/java/de/tum/cit/aet/artemis/exam/test_repository/ExamRoomTestRepository.java, CTE (Common Table Expression) syntax with bare attribute names like `roomNumber`, `name`, `createdDate` inside the WITH clause works correctly without requiring entity aliases, confirmed working by SamuelRoettgermann in their Artemis/Hibernate setup.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/exam/test_repository/ExamRoomTestRepository.javasrc/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomRepository.java
📚 Learning: 2025-09-06T19:03:45.260Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11111
File: src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.ts:71-76
Timestamp: 2025-09-06T19:03:45.260Z
Learning: In the ExamRoomsComponent (src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.ts), the numberOfUniqueLayoutStrategies computed signal is intentionally designed to sum the total number of layout strategies across all unique exam rooms, not to count distinct layout strategies by name. The author prefers the more descriptive name "numberOfLayoutStrategiesOfUniqueRooms" to better reflect this intention.
Applied to files:
src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.spec.tssrc/main/webapp/app/core/admin/exam-rooms/exam-rooms.model.tssrc/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.ts
📚 Learning: 2025-08-30T20:20:17.236Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11111
File: src/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.java:0-0
Timestamp: 2025-08-30T20:20:17.236Z
Learning: In ExamRoomIntegrationTest.validateAdminOverview(), the first assertion should use subset containment (contains) rather than exact equality because the admin overview shows all newest unique exam rooms in the system including those from previous uploads, not just rooms from the current upload being tested. The test only needs to verify that the expected rooms from the current upload are present in the response.
Applied to files:
src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.spec.tssrc/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/exam/service/ExamRoomServiceTest.java
📚 Learning: 2025-09-25T20:43:41.712Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11419
File: src/main/java/de/tum/cit/aet/artemis/exam/web/ExamRoomDistributionResource.java:62-78
Timestamp: 2025-09-25T20:43:41.712Z
Learning: In the ExamRoomDistributionService.distributeRegisteredStudents method in src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java, ExamRoom entities are shared resources that are dynamically assigned to exams through the ExamRoomExamAssignment join table. Rooms don't "belong" to exams until the distribution process creates these assignments. The method first clears existing assignments for the exam, then creates new ones for the provided room IDs.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomForDistributionDTO.javasrc/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.javasrc/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.javasrc/main/java/de/tum/cit/aet/artemis/exam/web/ExamRoomDistributionResource.javasrc/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.javasrc/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomRepository.javasrc/test/java/de/tum/cit/aet/artemis/exam/service/ExamRoomServiceTest.javasrc/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.service.ts
📚 Learning: 2025-09-15T11:18:26.439Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11378
File: src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java:13-16
Timestamp: 2025-09-15T11:18:26.439Z
Learning: In ExamRoomDistributionService.distributeRegisteredStudents method in src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java, the team has decided not to use Transactional annotation despite multiple repository operations, based on human reviewer consultation.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomExamAssignmentRepository.javasrc/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.javasrc/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.javasrc/main/java/de/tum/cit/aet/artemis/exam/web/ExamRoomDistributionResource.javasrc/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.javasrc/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomRepository.javasrc/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.service.ts
📚 Learning: 2025-09-25T20:28:36.905Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11419
File: src/main/java/de/tum/cit/aet/artemis/exam/domain/ExamUser.java:16-17
Timestamp: 2025-09-25T20:28:36.905Z
Learning: In the Artemis codebase, ExamUser entity uses ExamSeatDTO as a transient field for performance reasons. SamuelRoettgermann tested domain value objects but they caused 60x slower performance. This architectural exception is approved by maintainers due to significant performance benefits and Artemis naming convention requirements.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.java
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: jakubriegel
PR: ls1intum/Artemis#8050
File: src/test/java/de/tum/in/www1/artemis/plagiarism/PlagiarismUtilService.java:62-66
Timestamp: 2024-06-10T19:44:09.116Z
Learning: The `createCourseWithUsers` method in `PlagiarismUtilService.java` uses fixed inputs as it is designed to be a test helper method for simplifying the setup of courses and users in tests.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.java
📚 Learning: 2025-08-14T21:24:50.201Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11111
File: src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.java:150-152
Timestamp: 2025-08-14T21:24:50.201Z
Learning: In the ExamRoomService.parseAndStoreExamRoomDataFromZipFile method in src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.java, IllegalArgumentException cannot be thrown in the try block, so only IOException needs to be caught when parsing ZIP files containing exam room JSON data.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/exam/util/ExamRoomZipFiles.javasrc/test/java/de/tum/cit/aet/artemis/exam/service/ExamRoomServiceTest.java
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: valentin-boehm
PR: ls1intum/Artemis#7384
File: src/test/java/de/tum/in/www1/artemis/exam/StudentExamIntegrationTest.java:988-993
Timestamp: 2024-06-10T19:44:09.116Z
Learning: The `testSubmitStudentExam_differentUser` method does not require additional checks to verify the state of `studentExam1` after receiving a `HttpStatus.FORBIDDEN` because the control flow in the `StudentExamResource` is straightforward and ensures no state change occurs.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.java
🧬 Code graph analysis (11)
src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomForDistributionDTO.java (2)
src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomDeletionSummaryDTO.java (1)
JsonInclude(5-7)src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomAdminOverviewDTO.java (1)
JsonInclude(9-12)
src/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomExamAssignmentRepository.java (1)
src/main/java/de/tum/cit/aet/artemis/exam/config/ExamEnabled.java (1)
ExamEnabled(13-25)
src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java (2)
src/main/java/de/tum/cit/aet/artemis/exam/web/ExamRoomDistributionResource.java (1)
Conditional(31-96)src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.java (1)
Conditional(45-524)
src/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.java (2)
src/main/webapp/app/core/admin/exam-rooms/exam-rooms.model.ts (3)
ExamRoomAdminOverviewDTO(38-43)ExamRoomDTO(8-15)ExamRoomLayoutStrategyDTO(23-27)src/test/java/de/tum/cit/aet/artemis/exam/util/ExamRoomZipFiles.java (1)
ExamRoomZipFiles(12-93)
src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-dialog.component.ts (2)
src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-button.component.ts (1)
Component(9-51)src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.model.ts (1)
RoomForDistributionDTO(1-8)
src/main/java/de/tum/cit/aet/artemis/exam/web/ExamRoomDistributionResource.java (3)
src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java (1)
Conditional(32-126)src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.java (1)
Conditional(45-524)src/main/java/de/tum/cit/aet/artemis/exam/config/ExamEnabled.java (1)
ExamEnabled(13-25)
src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomDTO.java (1)
src/main/webapp/app/core/admin/exam-rooms/exam-rooms.model.ts (1)
ExamRoomDTO(8-15)
src/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomRepository.java (1)
src/main/java/de/tum/cit/aet/artemis/exam/config/ExamEnabled.java (1)
ExamEnabled(13-25)
src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.ts (1)
src/main/webapp/app/core/admin/exam-rooms/exam-rooms.model.ts (2)
ExamRoomDTOExtended(17-21)ExamRoomDTO(8-15)
src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.service.ts (1)
src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.model.ts (1)
RoomForDistributionDTO(1-8)
src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-button.component.ts (2)
src/main/webapp/app/exam/manage/students/exam-students.component.ts (1)
Component(35-323)src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-dialog.component.ts (1)
Component(17-171)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: server-tests
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
- GitHub Check: Mend Security Check
🔇 Additional comments (21)
supporting_scripts/code-coverage/module-coverage-client/check-client-module-coverage.mjs (1)
115-115: Verify the programming module branch coverage threshold update.The PR touches multiple files under
src/main/webapp/app/programming/, leading to a branch coverage drop from 76.30% to 76.26%.
- If this reduction stems from unavoidable untested logic, document the rationale in the PR description.
- Otherwise, please add tests to restore the previous 76.30% threshold.
src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.model.ts (1)
1-8: LGTM!The interface definition is clean and correctly typed. It properly aligns with the backend DTO structure and follows TypeScript/Angular naming conventions.
src/main/webapp/i18n/de/examRooms.json (1)
37-37: LGTM!The German translations are appropriate and clear. "ID" is correctly left as-is (technical term), and "Standardkapazität" is proper German for "default capacity".
Also applies to: 41-41
src/main/webapp/app/exam/manage/students/exam-students.component.ts (1)
28-28: LGTM!The import and component registration follow Angular standalone component patterns correctly. The integration is minimal and non-invasive.
Also applies to: 44-44
src/main/webapp/app/core/admin/exam-rooms/exam-rooms.model.ts (2)
9-9: LGTM!The addition of
idas a readonly property anddefaultCapacityas a required field properly aligns with the backend DTO changes and supports the new admin UI features.Also applies to: 18-18
26-26: Capacity is always provided for ExamRoomLayoutStrategyDTO; making it mandatory is safe. All front-end object literals include a numericcapacity.src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.html (2)
69-73: LGTM!The new sortable columns for
idanddefaultCapacityare correctly implemented with proper sorting directives, icons, and i18n bindings.Also applies to: 89-93
107-112: LGTM!The data cells correctly display the new
idanddefaultCapacityfields. Tracking byexamRoom.idinstead ofexamRoom.roomNumberis a better practice for stable row identity.Also applies to: 125-128
src/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomExamAssignmentRepository.java (1)
13-24: LGTM!The repository interface follows Spring Data JPA best practices:
- Proper annotations including
@Conditional(ExamEnabled.class)for feature gating@Lazyinitialization appropriate for optional featuresdeleteAllByExamIdmethod correctly annotated with@Transactionaland@Modifying- Method name follows Spring Data naming conventions
Based on learnings
src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution-dialog.component.ts (1)
62-64: LGTM!The component implementation is solid:
- Proper signal usage with
WritableSignalandcomputed()- Memory cleanup correctly handles Subject unsubscription
- Search and filtering logic with subsequence matching provides good UX
- Room selection management maintains immutability
- Helper methods are well-structured and focused
Also applies to: 89-91, 93-110, 117-123, 130-141, 155-170
src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomForDistributionDTO.java (1)
8-11: LGTM!The DTO record is well-designed:
- Modern Java record pattern for immutable DTOs
- Proper Jakarta validation with
@NotBlankon required fields and@Nullableon optional fields@JsonInclude(JsonInclude.Include.NON_EMPTY)appropriately excludes null/empty values from JSON- Aligns perfectly with the TypeScript
RoomForDistributionDTOinterface- Follows coding guidelines for naming, records, and minimal data transfer
As per coding guidelines
src/main/webapp/app/core/admin/exam-rooms/exam-rooms.component.ts (3)
90-91: LGTM: computed data extraction is clear.
93-93: Sort keys extended; confirm numeric sorting for id.
If SortService compares as strings, numeric ids might sort incorrectly (e.g., 2 > 10). Please verify sortByProperty handles numbers.
107-107: Good UX: sort immediately after loading overview.src/main/java/de/tum/cit/aet/artemis/exam/dto/room/ExamRoomDTO.java (1)
10-11: LGTM: id added to DTO record.
Breaking API acknowledged; TS model appears aligned.Please confirm all server mappers/tests were updated to populate id to avoid serialization gaps.
src/main/webapp/app/exam/manage/students/room-distribution/students-room-distribution.service.ts (1)
10-17: LGTM: endpoints and typings look correct.
Paths align with backend; using observe: 'response' is fine.Also applies to: 26-28
src/main/java/de/tum/cit/aet/artemis/exam/web/ExamRoomDistributionResource.java (2)
63-80: LGTM: access check, validation, and delegation are sound.
Custom error keys for empty/invalid room IDs are helpful.
88-95: LGTM: distribution data endpoint is minimal and clear.src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java (2)
78-88: Consistency risk: delete → re-create without a transactional boundaryBetween deleteAllByExamId, saveAll(assignments), and saving users, a failure can leave partial state (e.g., assignments updated but users not saved). I know the team opted to avoid @transactional, but please at least:
- Compute and validate seat allocation before performing any writes (fail fast).
- Ensure idempotency via unique constraints on (examId, examRoomId) in ExamRoomExamAssignment.
- Consider wrapping the three write steps in a best-effort retry or compensating logic.
Would you like me to propose a fail-fast write ordering that keeps the current non-transactional approach?
Based on learnings
70-76: Edge-case: capacity computed from default layout onlyYou rely on default layout capacity; distribution then uses getDefaultUsableSeats again. Ensure capacities and actual usable seats cannot diverge (e.g., after data updates). If divergence is possible, compute usable seats once and reuse for both the check and assignment.
src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.java (1)
380-395: Should fixed-selection ignore SeatCondition?getUsableSeatsFixedSelection uses all seats (onlyUsableSeats=false). If a seat is flagged non-usable, do we still allow selection via layout JSON? If not, validate and reject such selections.
| SELECT id | ||
| FROM ( | ||
| SELECT er.id AS id, er.roomNumber AS roomNumber, er.name AS name, er.createdDate AS createdDate, ROW_NUMBER() OVER ( | ||
| PARTITION BY er.roomNumber, er.name | ||
| ORDER BY er.createdDate DESC, er.id DESC | ||
| ) AS rowNumber | ||
| FROM ExamRoom er | ||
| ) | ||
| WHERE rowNumber = 1 | ||
| """) |
There was a problem hiding this comment.
Alias missing on windowed subquery causes runtime failure
The derived table needs an alias in JPQL/SQL. As written, Hibernate will emit ... FROM (SELECT …) WHERE ..., which is illegal SQL and fails at runtime. Please alias the subselect and reference that alias in the SELECT/WHERE clause.
- @Query("""
- SELECT id
- FROM (
+ @Query("""
+ SELECT latest.id
+ FROM (
SELECT er.id AS id, er.roomNumber AS roomNumber, er.name AS name, er.createdDate AS createdDate, ROW_NUMBER() OVER (
PARTITION BY er.roomNumber, er.name
ORDER BY er.createdDate DESC, er.id DESC
) AS rowNumber
FROM ExamRoom er
- )
- WHERE rowNumber = 1
+ ) latest
+ WHERE latest.rowNumber = 1
""")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SELECT id | |
| FROM ( | |
| SELECT er.id AS id, er.roomNumber AS roomNumber, er.name AS name, er.createdDate AS createdDate, ROW_NUMBER() OVER ( | |
| PARTITION BY er.roomNumber, er.name | |
| ORDER BY er.createdDate DESC, er.id DESC | |
| ) AS rowNumber | |
| FROM ExamRoom er | |
| ) | |
| WHERE rowNumber = 1 | |
| """) | |
| @Query(""" | |
| SELECT latest.id | |
| FROM ( | |
| SELECT er.id AS id, er.roomNumber AS roomNumber, er.name AS name, er.createdDate AS createdDate, ROW_NUMBER() OVER ( | |
| PARTITION BY er.roomNumber, er.name | |
| ORDER BY er.createdDate DESC, er.id DESC | |
| ) AS rowNumber | |
| FROM ExamRoom er | |
| ) latest | |
| WHERE latest.rowNumber = 1 | |
| """) |
🤖 Prompt for AI Agents
In src/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRoomRepository.java
around lines 68 to 77, the windowed subquery is missing an alias which produces
invalid SQL; fix it by giving the derived table an alias (e.g., "t") immediately
after the closing parenthesis and update the outer query to reference that alias
(e.g., SELECT t.id FROM ( ... ) t WHERE t.rowNumber = 1) so the generated
JPQL/SQL is valid.
| private void distributeExamUsersToUsableSeatsInRooms(Exam exam, Set<ExamRoom> examRoomsForExam) { | ||
| Map<String, List<ExamSeatDTO>> roomNumberToUsableSeatsDefaultLayout = new HashMap<>(); | ||
| for (ExamRoom examRoom : examRoomsForExam) { | ||
| roomNumberToUsableSeatsDefaultLayout.put(examRoom.getRoomNumber(), examRoomService.getDefaultUsableSeats(examRoom)); | ||
| } | ||
|
|
||
| setPlannedRoomAndPlannedSeatForExamUsersRandomly(exam, roomNumberToUsableSeatsDefaultLayout); | ||
|
|
||
| examUserRepository.saveAll(exam.getExamUsers()); | ||
| examRepository.save(exam); | ||
| } |
There was a problem hiding this comment.
Uneven and non-deterministic distribution; remove redundant add and balance across rooms
Current logic fills rooms in arbitrary HashMap/Set order and may pack early rooms completely while leaving others unused. Also, re-adding an already-associated ExamUser to exam may be redundant and risky during iteration.
Apply a balanced, deterministic round‑robin assignment and drop the extra add:
- Map<String, List<ExamSeatDTO>> roomNumberToUsableSeatsDefaultLayout = new HashMap<>();
+ Map<String, List<ExamSeatDTO>> roomNumberToUsableSeatsDefaultLayout = new HashMap<>();
for (ExamRoom examRoom : examRoomsForExam) {
roomNumberToUsableSeatsDefaultLayout.put(examRoom.getRoomNumber(), examRoomService.getDefaultUsableSeats(examRoom));
}
- setPlannedRoomAndPlannedSeatForExamUsersRandomly(exam, roomNumberToUsableSeatsDefaultLayout);
+ assignExamUsersToSeatsBalanced(exam, roomNumberToUsableSeatsDefaultLayout);
examUserRepository.saveAll(exam.getExamUsers());
examRepository.save(exam);
}
- private void setPlannedRoomAndPlannedSeatForExamUsersRandomly(Exam exam, Map<String, List<ExamSeatDTO>> roomNumberToUsableSeats) {
- Iterator<ExamUser> examUsersIterator = exam.getExamUsers().iterator();
-
- for (var roomNumberToUsableSeatsEntry : roomNumberToUsableSeats.entrySet()) {
- final String roomNumber = roomNumberToUsableSeatsEntry.getKey();
- final List<ExamSeatDTO> usableSeatsForThisRoom = roomNumberToUsableSeatsEntry.getValue();
-
- for (ExamSeatDTO seat : usableSeatsForThisRoom) {
- if (!examUsersIterator.hasNext()) {
- return;
- }
-
- ExamUser nextExamUser = examUsersIterator.next();
- nextExamUser.setPlannedRoom(roomNumber);
- nextExamUser.setPlannedSeat(seat.name());
- exam.addExamUser(nextExamUser);
- }
- }
- }
+ private void assignExamUsersToSeatsBalanced(Exam exam, Map<String, List<ExamSeatDTO>> roomNumberToUsableSeats) {
+ // Deterministic room order
+ var sortedRoomNumbers = new java.util.ArrayList<>(roomNumberToUsableSeats.keySet());
+ java.util.Collections.sort(sortedRoomNumbers);
+ var seatsByRoom = new java.util.LinkedHashMap<String, List<ExamSeatDTO>>();
+ for (var rn : sortedRoomNumbers) {
+ seatsByRoom.put(rn, roomNumberToUsableSeats.get(rn));
+ }
+
+ // Round-robin seat order across rooms
+ int maxPerRoom = seatsByRoom.values().stream().mapToInt(List::size).max().orElse(0);
+ List<Map.Entry<String, ExamSeatDTO>> assignmentOrder = new ArrayList<>();
+ for (int i = 0; i < maxPerRoom; i++) {
+ for (var e : seatsByRoom.entrySet()) {
+ var seats = e.getValue();
+ if (i < seats.size()) {
+ assignmentOrder.add(Map.entry(e.getKey(), seats.get(i)));
+ }
+ }
+ }
+
+ Iterator<ExamUser> it = exam.getExamUsers().iterator();
+ for (var seatEntry : assignmentOrder) {
+ if (!it.hasNext()) {
+ break;
+ }
+ ExamUser user = it.next();
+ user.setPlannedRoom(seatEntry.getKey());
+ user.setPlannedSeat(seatEntry.getValue().name());
+ // no re-adding to exam collection
+ }
+ }As per coding guidelines
Also applies to: 103-121
| private static List<List<ExamSeatDTO>> getSortedRowsWithSortedSeats(ExamRoom examRoom, boolean onlyUsableSeats) { | ||
| List<ExamSeatDTO> examSeats = examRoom.getSeats(); | ||
| if (onlyUsableSeats) { | ||
| examSeats = examSeats.stream().filter(examSeatDTO -> examSeatDTO.seatCondition() == SeatCondition.USABLE).toList(); | ||
| } | ||
| Map<Double, List<ExamSeatDTO>> rowsByRowHeight = examSeats.stream().collect(Collectors.groupingBy(ExamSeatDTO::yCoordinate)); | ||
|
|
||
| // Sort rows by y ascending, and seats within row by x ascending | ||
| return rowsByRowHeight.entrySet().stream().sorted(Map.Entry.comparingByKey()) | ||
| .map(entry -> entry.getValue().stream().sorted(Comparator.comparingDouble(ExamSeatDTO::xCoordinate)).toList()).toList(); | ||
| } |
There was a problem hiding this comment.
Row grouping by exact double equality is brittle; use epsilon clustering
Grouping rows by exact y doubles can split a visual row into multiple due to tiny floating-point differences. Cluster by y within a tolerance and sort seats by x:
- private static List<List<ExamSeatDTO>> getSortedRowsWithSortedSeats(ExamRoom examRoom, boolean onlyUsableSeats) {
- List<ExamSeatDTO> examSeats = examRoom.getSeats();
- if (onlyUsableSeats) {
- examSeats = examSeats.stream().filter(examSeatDTO -> examSeatDTO.seatCondition() == SeatCondition.USABLE).toList();
- }
- Map<Double, List<ExamSeatDTO>> rowsByRowHeight = examSeats.stream().collect(Collectors.groupingBy(ExamSeatDTO::yCoordinate));
-
- // Sort rows by y ascending, and seats within row by x ascending
- return rowsByRowHeight.entrySet().stream().sorted(Map.Entry.comparingByKey())
- .map(entry -> entry.getValue().stream().sorted(Comparator.comparingDouble(ExamSeatDTO::xCoordinate)).toList()).toList();
- }
+ private static List<List<ExamSeatDTO>> getSortedRowsWithSortedSeats(ExamRoom examRoom, boolean onlyUsableSeats) {
+ List<ExamSeatDTO> seats = examRoom.getSeats();
+ if (onlyUsableSeats) {
+ seats = seats.stream().filter(s -> s.seatCondition() == SeatCondition.USABLE).toList();
+ }
+ // Sort all seats first by y, then by x
+ seats = seats.stream().sorted(Comparator.comparingDouble(ExamSeatDTO::yCoordinate).thenComparingDouble(ExamSeatDTO::xCoordinate)).toList();
+ List<List<ExamSeatDTO>> rows = new ArrayList<>();
+ final double Y_EPS = 0.5d; // tolerance for grouping seats into the same row
+ for (ExamSeatDTO seat : seats) {
+ if (rows.isEmpty() || Math.abs(rows.get(rows.size() - 1).get(0).yCoordinate() - seat.yCoordinate()) > Y_EPS) {
+ rows.add(new ArrayList<>());
+ }
+ rows.get(rows.size() - 1).add(seat);
+ }
+ return rows;
+ }As per coding guidelines
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static List<List<ExamSeatDTO>> getSortedRowsWithSortedSeats(ExamRoom examRoom, boolean onlyUsableSeats) { | |
| List<ExamSeatDTO> examSeats = examRoom.getSeats(); | |
| if (onlyUsableSeats) { | |
| examSeats = examSeats.stream().filter(examSeatDTO -> examSeatDTO.seatCondition() == SeatCondition.USABLE).toList(); | |
| } | |
| Map<Double, List<ExamSeatDTO>> rowsByRowHeight = examSeats.stream().collect(Collectors.groupingBy(ExamSeatDTO::yCoordinate)); | |
| // Sort rows by y ascending, and seats within row by x ascending | |
| return rowsByRowHeight.entrySet().stream().sorted(Map.Entry.comparingByKey()) | |
| .map(entry -> entry.getValue().stream().sorted(Comparator.comparingDouble(ExamSeatDTO::xCoordinate)).toList()).toList(); | |
| } | |
| private static List<List<ExamSeatDTO>> getSortedRowsWithSortedSeats(ExamRoom examRoom, boolean onlyUsableSeats) { | |
| List<ExamSeatDTO> seats = examRoom.getSeats(); | |
| if (onlyUsableSeats) { | |
| seats = seats.stream() | |
| .filter(s -> s.seatCondition() == SeatCondition.USABLE) | |
| .toList(); | |
| } | |
| // Sort all seats first by y, then by x | |
| seats = seats.stream() | |
| .sorted(Comparator.comparingDouble(ExamSeatDTO::yCoordinate) | |
| .thenComparingDouble(ExamSeatDTO::xCoordinate)) | |
| .toList(); | |
| List<List<ExamSeatDTO>> rows = new ArrayList<>(); | |
| final double Y_EPS = 0.5d; // tolerance for grouping seats into the same row | |
| for (ExamSeatDTO seat : seats) { | |
| if (rows.isEmpty() || | |
| Math.abs(rows.get(rows.size() - 1).get(0).yCoordinate() - seat.yCoordinate()) > Y_EPS) { | |
| rows.add(new ArrayList<>()); | |
| } | |
| rows.get(rows.size() - 1).add(seat); | |
| } | |
| return rows; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomService.java around
lines 405 to 415, grouping rows by exact double equality on yCoordinate is
brittle and can split a visual row due to tiny floating-point differences;
change the grouping to cluster seats whose yCoordinate values are within a small
epsilon (e.g., 1e-3) instead of using Collectors.groupingBy on the raw double.
Implementation: optionally filter by usable seats first, sort all seats by
yCoordinate ascending, iterate and create clusters by starting a new cluster
when the next seat's y differs from the current cluster's representative by more
than EPS (use cluster mean or first seat y as representative), then for each
cluster sort its seats by xCoordinate ascending and return the list of clusters
sorted by their representative y. Ensure EPS is a named constant and ordering is
stable.
| private getMaxCapacityOfExamRoom(examRoom: ExamRoomDTO): number { | ||
| return examRoom.layoutStrategies?.map((layoutStrategy) => layoutStrategy.capacity ?? 0).reduce((max, curr) => Math.max(max, curr), 0) ?? 0; | ||
| return examRoom.layoutStrategies?.map((layoutStrategy) => layoutStrategy.capacity).reduce((max, curr) => Math.max(max, curr), 0) ?? 0; | ||
| } |
There was a problem hiding this comment.
Critical: optional chaining misuse can throw at runtime.
Chains like layoutStrategies?.map(...).reduce(...) still call reduce on undefined. Same for filter → map → at. Use nullish fallback to [] or chain ?. consistently.
Apply this diff:
- private getMaxCapacityOfExamRoom(examRoom: ExamRoomDTO): number {
- return examRoom.layoutStrategies?.map((layoutStrategy) => layoutStrategy.capacity).reduce((max, curr) => Math.max(max, curr), 0) ?? 0;
- }
+ private getMaxCapacityOfExamRoom(examRoom: ExamRoomDTO): number {
+ return (examRoom.layoutStrategies ?? []).reduce((max, s) => Math.max(max, s.capacity), 0);
+ }
- private getDefaultCapacityOfExamRoom(examRoom: ExamRoomDTO): number {
- return (
- examRoom.layoutStrategies
- ?.filter((layoutStrategy) => layoutStrategy.name.toLowerCase() === 'default')
- .map((layoutStrategy) => layoutStrategy.capacity)
- .at(0) ?? 0
- );
- }
+ private getDefaultCapacityOfExamRoom(examRoom: ExamRoomDTO): number {
+ return (examRoom.layoutStrategies ?? []).find((ls) => ls.name.toLowerCase() === 'default')?.capacity ?? 0;
+ }Also update getLayoutStrategyNames to avoid a similar issue:
- return (
- examRoom.layoutStrategies
- ?.map((layoutStrategy) => layoutStrategy.name)
- .sort()
- .join(', ') ?? ''
- );
+ return (examRoom.layoutStrategies ?? []).map((ls) => ls.name).sort().join(', ');Also applies to: 210-217
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
1 participant
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
Description
Steps for Testing
Prerequisites:
Exam Mode Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
Screenshots
Summary by CodeRabbit
New Features
Improvements
Localization