Tutorial groups: Improve student tutorial group detail page
#11350
Conversation
github-actions
bot
added
client
github-actions
bot
added
the
server
github-actions
bot
added
the
communication
marlonnienaber
changed the title
Tutorial groups: Refactor student tutorial group detail page
marlonnienaber
dismissed stale reviews from HawKhiem, tobias-lippert, and ekayandan
via
75a43e6
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/main/webapp/app/tutorialgroup/overview/course-tutorial-group-detail/course-tutorial-group-detail.component.spec.ts (2)
53-78: Reuse the spy variable instead of creating a duplicate spy.Line 54 calls
jest.spyOn(CourseModel, 'isMessagingEnabled')again, creating a second spy on the same method. This overrides the spy frombeforeEachand can lead to test pollution.Based on learnings
Apply this diff:
it('should display no conversation links if messaging disabled', () => { - jest.spyOn(CourseModel, 'isMessagingEnabled').mockReturnValue(false); + isMessagingEnabledSpy.mockReturnValue(false); const raw: RawTutorialGroupDetailGroupDTO = {
24-51: Declare a reusable spy variable for isMessagingEnabled to prevent spy conflicts.The test creates a spy for
CourseModel.isMessagingEnabledinbeforeEach(line 48) but doesn't store it in a variable. Later, line 54 creates a duplicate spy, which can cause test pollution and unpredictable behavior.Based on learnings
Apply this diff to declare and reuse a single spy:
describe('NewTutorialGroupDetail', () => { let component: CourseTutorialGroupDetailComponent; let fixture: ComponentFixture<CourseTutorialGroupDetailComponent>; + let isMessagingEnabledSpy: jest.SpyInstance; const mockTranslateService = new MockTranslateService(); mockTranslateService.use('en'); const mockAccountService = new MockAccountService(); mockAccountService.userIdentity = new User(undefined, 'artemis_admin'); beforeEach(async () => { await TestBed.configureTestingModule({ imports: [CourseTutorialGroupDetailComponent], providers: [ { provide: TranslateService, useValue: mockTranslateService }, { provide: AccountService, useValue: mockAccountService }, { provide: OneToOneChatService, useValue: MockService(OneToOneChatService) }, { provide: AlertService, useValue: MockService(AlertService) }, { provide: Router, useValue: MockService(Router) }, ], declarations: [MockDirective(TranslateDirective), MockDirective(RouterLink)], }).compileComponents(); fixture = TestBed.createComponent(CourseTutorialGroupDetailComponent); component = fixture.componentInstance; - jest.spyOn(CourseModel, 'isMessagingEnabled').mockReturnValue(true); + isMessagingEnabledSpy = jest.spyOn(CourseModel, 'isMessagingEnabled').mockReturnValue(true); fixture.componentRef.setInput('course', { id: 1 } as Course); });src/main/java/de/tum/cit/aet/artemis/tutorialgroup/service/TutorialGroupService.java (1)
642-669: Guard schedule time parsing against null values.Lines 658-660 parse
scheduleStartTime()andscheduleEndTime()without null checks. The repository query uses aLEFT JOINon schedule, so these fields can be null even whenscheduleDayOfWeek()is non-null.LocalTime.parse(null)will throw aNullPointerException.Apply this fix to guard against null values:
- if (rawGroupDTOs.scheduleDayOfWeek() != null) { + if (rawGroupDTOs.scheduleDayOfWeek() != null + && rawGroupDTOs.scheduleStartTime() != null + && rawGroupDTOs.scheduleEndTime() != null) { int scheduleDayOfWeek = rawGroupDTOs.scheduleDayOfWeek(); LocalTime scheduleStart = LocalTime.parse(rawGroupDTOs.scheduleStartTime()); LocalTime scheduleEnd = LocalTime.parse(rawGroupDTOs.scheduleEndTime()); String scheduleLocation = rawGroupDTOs.scheduleLocation(); sessionDTOs = rawSessionDTOs.stream() .map(data -> TutorialGroupDetailSessionDTO.from(data, scheduleDayOfWeek, scheduleStart, scheduleEnd, scheduleLocation, courseTimeZone)).toList(); } else { sessionDTOs = rawSessionDTOs.stream().map(TutorialGroupDetailSessionDTO::from).toList(); }This ensures all three time-related fields are non-null before parsing, preventing NPE when schedule data is incomplete.
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailSessionDTO.java (1)
1-1: Consider moving DTO to a dedicated package.Per coding guidelines, DTOs should follow single responsibility and be organized appropriately. This record is placed in the
utilpackage, but DTOs are typically placed in adtoorweb.dtopackage for better organization and separation of concerns.If the project convention is to place DTOs in a dedicated package (e.g.,
de.tum.cit.aet.artemis.tutorialgroup.dtoorde.tum.cit.aet.artemis.tutorialgroup.web.dto), consider moving this record there for consistency. Ifutilis the established convention for "raw" DTOs used in repository projections, this is acceptable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/service/TutorialGroupService.java(9 hunks)src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailSessionDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupResource.java(9 hunks)src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupSessionResource.java(10 hunks)src/main/webapp/app/tutorialgroup/overview/course-tutorial-group-detail-container/course-tutorial-group-detail-container.component.spec.ts(1 hunks)src/main/webapp/app/tutorialgroup/overview/course-tutorial-group-detail/course-tutorial-group-detail.component.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/app/tutorialgroup/overview/course-tutorial-group-detail-container/course-tutorial-group-detail-container.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
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/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.javasrc/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupResource.javasrc/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupSessionResource.javasrc/main/java/de/tum/cit/aet/artemis/tutorialgroup/service/TutorialGroupService.javasrc/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailSessionDTO.java
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/tutorialgroup/overview/course-tutorial-group-detail/course-tutorial-group-detail.component.spec.ts
🧠 Learnings (8)
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: JohannesStoehr
PR: ls1intum/Artemis#8679
File: src/main/java/de/tum/in/www1/artemis/web/rest/tutorialgroups/TutorialGroupSessionResource.java:37-37
Timestamp: 2024-06-10T19:44:09.116Z
Learning: The DTOs `CompetencyProgressForLearningPathDTO`, `ProgrammingExerciseResetOptionsDTO`, and `CourseWithIdDTO` do not contain nullable values or `Optional` types, making the `JsonInclude` annotation unnecessary for them.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8597
File: src/main/webapp/app/exercises/programming/manage/repositories-checkout-directories-dto.ts:8-8
Timestamp: 2024-06-10T19:44:09.116Z
Learning: DTOs are typically defined in the `src/main/webapp/app/entities` folder on the client side in the Artemis project.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java
📚 Learning: 2025-09-10T16:35:23.211Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11111
File: src/main/java/de/tum/cit/aet/artemis/exam/domain/room/ExamRoom.java:69-75
Timestamp: 2025-09-10T16:35:23.211Z
Learning: Artemis convention: The term “DTO” is used broadly (includes DAOs/value objects), not strictly client-transport models. Avoid assuming dto.* packages are always API-facing.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java
📚 Learning: 2024-10-08T15:35:42.972Z
Learnt from: Strohgelaender
PR: ls1intum/Artemis#8574
File: src/main/java/de/tum/in/www1/artemis/service/tutorialgroups/TutorialGroupService.java:0-0
Timestamp: 2024-10-08T15:35:42.972Z
Learning: The `tryToFindMatchingUsers` method in `TutorialGroupService.java` has been updated to skip registrations without a student, enhancing the method's robustness. This change was implemented in commit `bef30f9751de0913143e8cb28cc0088264052261`.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupResource.javasrc/main/java/de/tum/cit/aet/artemis/tutorialgroup/service/TutorialGroupService.java
📚 Learning: 2025-09-18T22:22:54.269Z
Learnt from: marlonnienaber
PR: ls1intum/Artemis#11350
File: src/main/java/de/tum/cit/aet/artemis/tutorialgroup/service/TutorialGroupService.java:0-0
Timestamp: 2025-09-18T22:22:54.269Z
Learning: In the tutorial group domain, TutorialGroup entities are guaranteed to have a non-null teachingAssistant due to database constraints. This means teachingAssistantLogin() will always return a non-null value, and null checks are not necessary when using this field in repository calls or other operations.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/service/TutorialGroupService.java
📚 Learning: 2025-09-18T20:55:38.115Z
Learnt from: marlonnienaber
PR: ls1intum/Artemis#11350
File: src/main/java/de/tum/cit/aet/artemis/tutorialgroup/repository/TutorialGroupRepository.java:145-168
Timestamp: 2025-09-18T20:55:38.115Z
Learning: In tutorial groups, every TutorialGroup is guaranteed to have a teachingAssistant (non-null constraint), so teaching assistant associations don't require LEFT JOINs in JPQL queries. Only optional associations like tutorialGroupSchedule and tutorialGroupChannel need LEFT JOINs.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/service/TutorialGroupService.java
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: JohannesStoehr
PR: ls1intum/Artemis#0
File: :0-0
Timestamp: 2024-06-10T19:44:09.116Z
Learning: Admins are included in the `isAtLeastTutor` check, and unauthenticated users cannot access the `CourseTutorialGroupsComponent`.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/service/TutorialGroupService.java
📚 Learning: 2025-09-01T13:47:02.624Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts:167-169
Timestamp: 2025-09-01T13:47:02.624Z
Learning: In Jest tests, prefer using jest.spyOn() over direct method assignment for mocking service methods, as spies are automatically restored by Jest's cleanup mechanisms (jest.restoreAllMocks()), while direct assignment bypasses this system and can lead to test pollution where mocked methods affect subsequent tests.
Applied to files:
src/main/webapp/app/tutorialgroup/overview/course-tutorial-group-detail/course-tutorial-group-detail.component.spec.ts
🧬 Code graph analysis (6)
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java (1)
src/main/webapp/app/tutorialgroup/shared/entities/tutorial-group.model.ts (1)
RawTutorialGroupDetailGroupDTO(79-92)
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupResource.java (5)
src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-181)src/main/webapp/app/core/user/user.model.ts (1)
User(5-65)src/main/webapp/app/tutorialgroup/shared/entities/tutorial-group.model.ts (1)
TutorialGroupDetailGroupDTO(37-77)src/main/webapp/app/tutorialgroup/shared/service/tutorial-groups.service.ts (3)
getOneOfCourse(41-45)getTutorialGroupDetailGroupDTO(47-51)exportTutorialGroupsToCSV(184-186)src/main/webapp/app/core/auth/account.service.ts (2)
isAdmin(228-230)isAtLeastInstructorInCourse(200-202)
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupSessionResource.java (5)
src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-181)src/main/webapp/app/core/user/user.model.ts (1)
User(5-65)src/main/webapp/app/tutorialgroup/shared/entities/tutorial-group.model.ts (1)
TutorialGroup(10-35)src/main/webapp/app/tutorialgroup/shared/service/tutorial-group-session.service.ts (1)
getOneOfTutorialGroup(27-31)src/main/webapp/app/core/auth/account.service.ts (2)
isAdmin(228-230)isAtLeastInstructorInCourse(200-202)
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/service/TutorialGroupService.java (3)
src/main/webapp/app/tutorialgroup/shared/entities/tutorial-group.model.ts (3)
TutorialGroup(10-35)TutorialGroupDetailGroupDTO(37-77)RawTutorialGroupDetailGroupDTO(79-92)src/main/webapp/app/tutorialgroup/shared/entities/tutorial-group-session.model.ts (3)
TutorialGroupSession(12-23)TutorialGroupDetailSessionDTO(25-36)RawTutorialGroupDetailSessionDTO(38-47)src/main/webapp/app/tutorialgroup/shared/service/tutorial-groups.service.ts (3)
getOneOfCourse(41-45)getTutorialGroupDetailGroupDTO(47-51)exportTutorialGroupsToCSV(184-186)
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailSessionDTO.java (1)
src/main/webapp/app/tutorialgroup/shared/entities/tutorial-group-session.model.ts (1)
RawTutorialGroupDetailSessionDTO(38-47)
src/main/webapp/app/tutorialgroup/overview/course-tutorial-group-detail/course-tutorial-group-detail.component.spec.ts (5)
src/test/javascript/spec/helpers/mocks/service/mock-account.service.ts (1)
MockAccountService(7-45)src/main/webapp/app/core/user/user.model.ts (1)
User(5-65)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-181)src/main/webapp/app/tutorialgroup/shared/entities/tutorial-group.model.ts (2)
RawTutorialGroupDetailGroupDTO(79-92)TutorialGroupDetailGroupDTO(37-77)src/main/webapp/app/tutorialgroup/shared/entities/tutorial-group-session.model.ts (1)
RawTutorialGroupDetailSessionDTO(38-47)
⏰ 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). (5)
- GitHub Check: server-tests
- GitHub Check: client-tests-selected
- 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: Analyse
🔇 Additional comments (27)
src/main/webapp/app/tutorialgroup/overview/course-tutorial-group-detail/course-tutorial-group-detail.component.spec.ts (2)
12-13: Import issue resolved.The missing
RawTutorialGroupDetailSessionDTOimport flagged in previous reviews is now present on line 13.
80-1144: Comprehensive test coverage with good edge case handling.The test suite thoroughly covers:
- Messaging enablement and conversation link visibility
- Computed properties (language, capacity, mode, campus, nextSession, average attendance)
- Pie chart data and color calculations across all attendance ranges
- UI disclaimer display for missing data
- Edge cases (no capacity, no attendance, no sessions, no future sessions)
The tests follow Angular best practices with input-driven testing and consistent assertion patterns.
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailSessionDTO.java (2)
12-14: Review@JsonInclude(NON_EMPTY)semantics with nullable field.The class-level
@JsonInclude(JsonInclude.Include.NON_EMPTY)annotation excludes "empty" values (null, empty collections, empty strings, etc.). However,attendanceCountis@Nullable Integerwhich will be excluded when null. Verify this is the intended behavior:
- If
attendanceCount == nullshould be explicitly serialized asnullin JSON (to distinguish "no data" from "0 attendees"), use@JsonInclude(JsonInclude.Include.ALWAYS)on that field specifically- If excluding null
attendanceCountfrom JSON is correct (client treats absence as "no data"), the current annotation is fineBased on the PR objectives mentioning "data economy" and the field being marked
@Nullable, the current approach likely matches intent. However, confirm the frontend correctly handles the absence of this field versus an explicitnullor0value.
1-15: Incorrect mismatch concern – no change needed
The frontend’s RawTutorialGroupDetailSessionDTO interface represents the JSON‐serialized TutorialGroupDetailSessionDTO (record inde.tum.cit.aet.artemis.tutorialgroup.dto), not the internal RawTutorialGroupDetailSessionDTO inutil. Their shapes align correctly: the JSON DTO supplies the boolean flags and optional attendanceCount that the TS model expects.Likely an incorrect or invalid review comment.
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupSessionResource.java (9)
90-103: LGTM: Constructor injection follows guidelines.The addition of
CourseRepositoryandUserRepositoryvia constructor injection aligns with the coding guideline to use constructor injection for DI.
114-126: LGTM: Authorization now uses session's course.The endpoint correctly derives the course from
session.getTutorialGroup().getCourse()and uses it for the instructor check, eliminating the previous authorization gap.
176-181: LGTM: Helper method reduces duplication.The new helper method encapsulates the common pattern of fetching user context and computing admin/instructor status, following DRY principles.
147-149: LGTM: Authorization refactored correctly.The update method now uses the helper and calls the appropriate throwing authorization check method.
199-203: LGTM: Consistent authorization pattern.The updateAttendanceCount method follows the same authorization pattern as update.
223-227: LGTM: Delete authorization uses correct method.The deleteSession method now calls
checkIfUserIsAllowedToDeleteTutorialGroupElseThrow, which is semantically appropriate for deletion operations.
246-251: LGTM: Create authorization follows pattern.The create method correctly fetches the tutorial group and performs authorization checks before creating the session.
296-300: LGTM: Cancel authorization consistent.The cancel method uses the same authorization pattern as other session-modifying operations.
326-330: LGTM: Activate authorization consistent.The activate method follows the same authorization pattern as other session modification endpoints.
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupResource.java (7)
655-664: LGTM: Helper methods reduce duplication.The two helper methods
isAdminOrInstructorOfCourseandgetUserAndCheckWhetherTheyAreAllowedToChangeRegistrationencapsulate common authorization patterns and follow DRY principles.
187-196: LGTM: Authorization context passed correctly.The getAllForCourse method now computes and passes the
isAdminOrInstructorflag to the service layer.
205-214: LGTM: Annotation and authorization updated correctly.The getOneOfCourse method now uses
EnforceAtLeastStudentInCourseand passes theisAdminOrInstructorcontext to the service.
450-460: LGTM: Authorization refactored to use helper.The deregisterStudent method now uses the helper to fetch user and validate authorization, reducing code duplication.
472-486: LGTM: Consistent authorization pattern.The registerStudent method follows the same refactored authorization pattern as deregisterStudent.
592-612: LGTM: Export method updated with authorization context.The exportTutorialGroupsToCSV method now computes and passes the
isAdminOrInstructorflag to the service layer.
216-238: Membership validation confirmed tutorialGroupService.getTutorialGroupDetailGroupDTO delegates to tutorialGroupRepository.getTutorialGroupDetailData(tutorialGroupId, courseId), enforcing the tutorial group belongs to the course by filtering on both IDs and throwing EntityNotFoundException otherwise.src/main/java/de/tum/cit/aet/artemis/tutorialgroup/service/TutorialGroupService.java (7)
90-102: LGTM: Constructor injection follows guidelines.The addition of
OneToOneChatRepositoryvia constructor injection is correct and supports the new tutorial-group detail DTO functionality.
598-610: LGTM: Signature updated with authorization context.The findAllForCourse method now accepts and uses the
isAdminOrInstructorflag, eliminating the dependency on AuthorizationCheckService in the service layer.
621-631: LGTM: Consistent signature update.The getOneOfCourse method follows the same pattern as findAllForCourse.
671-690: LGTM: Clear authorization helper method.The
userHasManagingRightsForTutorialGroupmethod provides a clear, reusable authorization check and correctly handles lazy-loaded teaching assistant associations.
692-731: LGTM: Throwing authorization methods follow consistent pattern.The three authorization check methods (
checkIfUserIsAllowedToChangeRegistrationsOfTutorialGroupElseThrow,checkIfUserIsAllowedToDeleteTutorialGroupElseThrow,checkIfUserIsAllowedToModifySessionsOfTutorialGroupElseThrow) follow a consistent pattern, use clear naming (per past review feedback), and have proper@NotNullannotations.
739-776: LGTM: Export method signature updated consistently.The exportTutorialGroupsToCSV method signature now accepts the
isAdminOrInstructorflag and passes it through tofindAllForCourse, maintaining consistency with other service methods.
564-588: LGTM: User lookup refactored to repository methods.The tryToFindMatchingUsers method now uses dedicated repository methods, improving clarity and leveraging database-level filtering. Based on learnings.
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java
Show resolved
Hide resolved
|
@tobias-lippert see slack chat with @rabeatwork. Lets go with the headings as is for now and generally improve heading use in a follow up. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java (1)
9-13: Don't dropisOnline=falsefrom serialization
@JsonInclude(Include.NON_EMPTY)suppressesBoolean.FALSE, so offline groups lose theisOnlineflag entirely and the student UI can no longer distinguish “on-site” from “missing data”. Switch toInclude.NON_NULL(or drop the annotation) so legitimatefalsevalues are sent.-@JsonInclude(Include.NON_EMPTY) +@JsonInclude(Include.NON_NULL) public record RawTutorialGroupDetailGroupDTO(...)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java(1 hunks)supporting_scripts/code-coverage/module-coverage-client/check-client-module-coverage.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java
🧠 Learnings (3)
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: JohannesStoehr
PR: ls1intum/Artemis#8679
File: src/main/java/de/tum/in/www1/artemis/web/rest/tutorialgroups/TutorialGroupSessionResource.java:37-37
Timestamp: 2024-06-10T19:44:09.116Z
Learning: The DTOs `CompetencyProgressForLearningPathDTO`, `ProgrammingExerciseResetOptionsDTO`, and `CourseWithIdDTO` do not contain nullable values or `Optional` types, making the `JsonInclude` annotation unnecessary for them.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8597
File: src/main/webapp/app/exercises/programming/manage/repositories-checkout-directories-dto.ts:8-8
Timestamp: 2024-06-10T19:44:09.116Z
Learning: DTOs are typically defined in the `src/main/webapp/app/entities` folder on the client side in the Artemis project.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java
📚 Learning: 2025-09-10T16:35:23.211Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11111
File: src/main/java/de/tum/cit/aet/artemis/exam/domain/room/ExamRoom.java:69-75
Timestamp: 2025-09-10T16:35:23.211Z
Learning: Artemis convention: The term “DTO” is used broadly (includes DAOs/value objects), not strictly client-transport models. Avoid assuming dto.* packages are always API-facing.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java
🧬 Code graph analysis (1)
src/main/java/de/tum/cit/aet/artemis/tutorialgroup/util/RawTutorialGroupDetailGroupDTO.java (1)
src/main/webapp/app/tutorialgroup/shared/entities/tutorial-group.model.ts (1)
RawTutorialGroupDetailGroupDTO(79-92)
⏰ 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). (11)
- GitHub Check: Codacy Static Code Analysis
- 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-tests
- GitHub Check: server-style
- GitHub Check: client-style
- GitHub Check: server-tests
- GitHub Check: client-tests-selected
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
supporting_scripts/code-coverage/module-coverage-client/check-client-module-coverage.mjs
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
There was a problem hiding this comment.
Re-tested on TS4.
Lgtm; I never said it before, but I especially like that the Attendance value can display values above 100% (the Utilization bar in the Manage View can't do that).
Fun fact, the attendance value text can leak into the Attendance circle, but that is such an unrealistic scenario that I think it's fine either way.
Seems like the inputs in the management view are not validated correctly. Such inputs should not be possible. Out of scope for this PR though. |
krusche
changed the title
Tutorial groups: Refactor student tutorial group detail pageTutorial groups: Improve student tutorial group detail page
github-project-automation
bot
moved this from Ready For Review
to Merged
in Artemis Development
Checklist
General
Server
Client
Motivation and Context
The old tutorial group detail page has room for improvement when it comes to UI/UX.
Description
In this PR we refactor the tutorial group detail page that can be accessed through the course overview (not the management page). I built a new version of the page. I fetch the data via a new DTO endpoint (the call of the old page loaded a lot of unnecessary data). The new endpoint (part of TutorialGroupResource )calls a method in TutorialGroupService. While implementing this method some architecture tests started to fail because it is a very large Service and with the new method some threshold regarding Service complexity and size were exceeded. So I looked at the Service and decided one way to improve the complexity a little is to remove the AuthorizationCheckService dependency. I hence moved all authorization related logic into the Resource layer (I personally think that is cleaner anyways).
Steps for 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
Code Review
Manual Tests
Test Coverage
Client
Server
Screenshots
Summary by CodeRabbit
New Features
Improvements
Tests