Assessment: Introduce presentation assessments - #13288
Conversation
End-to-End Test Results
❌ Failed Tests
Test Strategy: Running all tests (configuration or infrastructure changes detected) Overall: ❌ E2E: real (non-flaky) test failure 🔗 Workflow Run · 📊 Test Report |
Assessment: Introduce presentation assessments
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPresentation assessments are introduced end-to-end, including a course feature toggle, persisted assessment and student-assignment data, instructor REST endpoints, Angular management views, validation, localization, and integration/unit tests. ChangesPresentation assessments
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested labels: Sequence Diagram(s)sequenceDiagram
participant Instructor
participant AngularManagement
participant PresentationAssessmentResource
participant PresentationAssessmentService
participant Database
Instructor->>AngularManagement: create or edit assessment
AngularManagement->>PresentationAssessmentResource: send assessment request
PresentationAssessmentResource->>PresentationAssessmentService: authorize and delegate
PresentationAssessmentService->>Database: save assessment and student assignments
Database-->>PresentationAssessmentService: return persisted data
PresentationAssessmentService-->>PresentationAssessmentResource: return result
PresentationAssessmentResource-->>AngularManagement: return HTTP response
AngularManagement-->>Instructor: update assessment table
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/main/java/de/tum/cit/aet/artemis/presentation/dto/PresentationAssessmentDTO.java (1)
15-16: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd
@Sizebounds totitle/descriptionfor defense-in-depth.Relying only on the client's
Validators.maxLengthand the entity's implicit column length leaves the API vulnerable to an unhandled DB-level failure for over-long input submitted outside the UI (e.g., via direct API calls).Based on learnings, "Always validate and sanitize inputs in server-side Java code. Do not trust client-side UI controls to enforce data integrity."🛡️ Proposed validation addition
-public record PresentationAssessmentDTO(Long id, `@NotBlank` String title, String description, `@NotNull` `@Positive` Double maxPoints, `@PositiveOrZero` Double resultPoints, +public record PresentationAssessmentDTO(Long id, `@NotBlank` `@Size`(max = 255) String title, `@Size`(max = 1000) String description, `@NotNull` `@Positive` Double maxPoints, `@PositiveOrZero` Double resultPoints, ZonedDateTime presentationDate, Long courseId) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/de/tum/cit/aet/artemis/presentation/dto/PresentationAssessmentDTO.java` around lines 15 - 16, Update the PresentationAssessmentDTO record by adding explicit `@Size` maximum bounds to both title and description, matching the corresponding persistence/API length constraints; keep the existing validation annotations and field types unchanged.Source: Learnings
src/main/webapp/app/presentation/manage/presentation-assessment-management.component.ts (1)
102-108: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a cross-field validator for
resultPoints <= maxPoints.The server rejects
resultPoints > maxPoints(BadRequestAlertException, "resultPointsExceedMaxPoints"), but the reactive form has no equivalent client-side check, so users only learn about the violation after a failed save round-trip.♻️ Proposed cross-field validator
editForm = this.formBuilder.group({ title: ['', [Validators.required, Validators.maxLength(255)]], description: ['', [Validators.maxLength(1000)]], maxPoints: [0, [Validators.required, Validators.min(0.01)]], resultPoints: [undefined as number | undefined, [Validators.min(0)]], presentationDate: [undefined as dayjs.Dayjs | undefined], -}); +}, { validators: resultPointsNotExceedingMaxPointsValidator });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/app/presentation/manage/presentation-assessment-management.component.ts` around lines 102 - 108, Update the editForm validation around maxPoints and resultPoints to add a cross-field validator enforcing resultPoints <= maxPoints. Ensure the validator handles the optional or undefined resultPoints value without error, and surfaces a form validation error when it exceeds maxPoints so submission is blocked before the server request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/de/tum/cit/aet/artemis/presentation/web/PresentationAssessmentResource.java`:
- Around line 154-160: Update getPresentationAssessmentStudents in
PresentationAssessmentResource to return a minimal Java record DTO rather than
exposing User entities. Map the students returned by
presentationAssessmentService.findStudents to the DTO, expose only the required
student fields, and update the corresponding client type to match the new
response shape.
- Around line 66-73: Presentation assessment endpoints must reject requests when
the presentation feature is disabled. In PresentationAssessmentResource.java at
lines 66-73, 83-90, 100-110, 120-128, 137-145, 154-161, 171-179, and 189-196,
resolve the course through the shared enabled-feature guard before listing,
retrieving, creating, updating, deleting, or managing assignments, while
preserving the existing role checks and endpoint behavior.
In `@src/main/webapp/app/course/manage/course-management.route.ts`:
- Around line 255-263: Update the presentations route around
PresentationAssessmentManagementComponent to require the
presentationAssessmentsEnabled feature flag in addition to instructor authority,
including direct navigation protection. Apply the same feature-flag enforcement
to the presentation management REST endpoints so API access is denied when the
flag is disabled, rather than relying on sidebar visibility.
In `@src/main/webapp/app/course/manage/update/course-update.component.html`:
- Around line 374-386: Replace the newly added presentationAssessmentsEnabled
control in the course update template with the project’s PrimeNG checkbox and
tooltip components, removing the Bootstrap classes and ngbTooltip usage.
Preserve the existing formControlName, translation keys, label association, and
question-circle icon while applying the established semantic tokens.
In `@src/main/webapp/app/course/shared/services/sidebar-item.service.ts`:
- Around line 255-264: Update getPresentationAssessmentsItem to remove the
primitive `#14b8a6` iconColor value and use an existing semantic/PrimeNG color
token instead, or omit iconColor if no suitable token exists.
In
`@src/main/webapp/app/presentation/manage/presentation-assessment-management.component.html`:
- Around line 2-6: Replace the newly added Bootstrap/ng-bootstrap UI with
PrimeNG throughout presentation-assessment-management.component.html: use
PrimeNG action controls, dialog/form components, layout and footer buttons,
table, and alert equivalents, while localizing the dialog close button’s
accessible label. In presentation-assessment-management.component.spec.ts,
replace the NgbModal dependency and its test provider with mocks for the
selected PrimeNG dialog API; apply these changes at the specified HTML and spec
sites.
In
`@src/main/webapp/app/presentation/manage/presentation-assessment-management.component.ts`:
- Line 29: Replace NgbModal and NgbModalRef usage in the new
presentation-assessment management component with PrimeNG DialogService,
following the inject(DialogService) and dialogService.open(...) pattern used by
course-update.component.ts. Update the affected imports and all modal creation,
reference, and close-result handling at the identified usages without retaining
any `@ng-bootstrap/ng-bootstrap` APIs.
---
Nitpick comments:
In
`@src/main/java/de/tum/cit/aet/artemis/presentation/dto/PresentationAssessmentDTO.java`:
- Around line 15-16: Update the PresentationAssessmentDTO record by adding
explicit `@Size` maximum bounds to both title and description, matching the
corresponding persistence/API length constraints; keep the existing validation
annotations and field types unchanged.
In
`@src/main/webapp/app/presentation/manage/presentation-assessment-management.component.ts`:
- Around line 102-108: Update the editForm validation around maxPoints and
resultPoints to add a cross-field validator enforcing resultPoints <= maxPoints.
Ensure the validator handles the optional or undefined resultPoints value
without error, and surfaces a form validation error when it exceeds maxPoints so
submission is blocked before the server request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fb18da36-9814-4032-ab5c-ceb683bbc7e7
⛔ Files ignored due to path filters (2)
src/main/resources/config/liquibase/changelog/20260706120000_changelog.xmlis excluded by!**/*.xmlsrc/main/resources/config/liquibase/master.xmlis excluded by!**/*.xml
📒 Files selected for processing (30)
src/main/java/de/tum/cit/aet/artemis/core/config/DatabaseConfiguration.javasrc/main/java/de/tum/cit/aet/artemis/course/domain/Course.javasrc/main/java/de/tum/cit/aet/artemis/course/dto/CourseCreateDTO.javasrc/main/java/de/tum/cit/aet/artemis/course/dto/CourseUpdateDTO.javasrc/main/java/de/tum/cit/aet/artemis/presentation/domain/PresentationAssessment.javasrc/main/java/de/tum/cit/aet/artemis/presentation/dto/PresentationAssessmentDTO.javasrc/main/java/de/tum/cit/aet/artemis/presentation/repository/PresentationAssessmentRepository.javasrc/main/java/de/tum/cit/aet/artemis/presentation/service/PresentationAssessmentService.javasrc/main/java/de/tum/cit/aet/artemis/presentation/web/PresentationAssessmentResource.javasrc/main/webapp/app/course/manage/course-management-container/course-management-container.component.tssrc/main/webapp/app/course/manage/course-management.route.tssrc/main/webapp/app/course/manage/update/course-update.component.htmlsrc/main/webapp/app/course/manage/update/course-update.component.spec.tssrc/main/webapp/app/course/manage/update/course-update.component.tssrc/main/webapp/app/course/shared/entities/course-update-dto.model.tssrc/main/webapp/app/course/shared/entities/course.model.tssrc/main/webapp/app/course/shared/services/sidebar-item.service.tssrc/main/webapp/app/presentation/manage/presentation-assessment-management.component.htmlsrc/main/webapp/app/presentation/manage/presentation-assessment-management.component.scsssrc/main/webapp/app/presentation/manage/presentation-assessment-management.component.spec.tssrc/main/webapp/app/presentation/manage/presentation-assessment-management.component.tssrc/main/webapp/app/presentation/manage/presentation-assessment.service.spec.tssrc/main/webapp/app/presentation/manage/presentation-assessment.service.tssrc/main/webapp/app/presentation/shared/entities/presentation-assessment.model.tssrc/main/webapp/i18n/de/course.jsonsrc/main/webapp/i18n/de/presentationAssessment.jsonsrc/main/webapp/i18n/en/course.jsonsrc/main/webapp/i18n/en/presentationAssessment.jsonsrc/test/java/de/tum/cit/aet/artemis/core/util/CourseTestService.javasrc/test/java/de/tum/cit/aet/artemis/presentation/PresentationAssessmentIntegrationTest.java
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jsuckart The presentation assessment foundation is promising, but the migration opts every existing course in, assignment changes survive Cancel/save failures, and the new server test is deterministically red; see inline comments. The unrelated E2E failures do not appear caused by this PR, while the targeted client tests pass. Our browser run was inconclusive because the PR image did not retain authentication after login.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.spec.ts (1)
14-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding coverage for create-mode and header-title reactivity.
All four tests use the same edit-mode
presentationAssessmentfixture. A test assertingstudentSectionTitle()reflects the currently-typed title in create mode (presentationAssessment: undefined) would have caught thecomputed()reactivity bug flagged inpresentation-assessment-form-dialog.component.ts(Line 99).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.spec.ts` around lines 14 - 104, Add a create-mode test for PresentationAssessmentFormDialogComponent by configuring the dialog with presentationAssessment undefined, then verify studentSectionTitle() updates when the form title changes. Ensure the test exercises the computed title after patching the title control, while preserving the existing edit-mode coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.ts`:
- Line 99: Replace the computed studentSectionTitle property with a plain
studentSectionTitle() method that preserves the existing fallback order and
trimming behavior, allowing the header to reflect title edits during change
detection; keep the existing template call unchanged.
In
`@src/main/webapp/app/presentation/manage/presentation-assessment-management.component.ts`:
- Around line 136-176: Update the error handling in save so loadAll() runs when
the create/update succeeds but persistAssignedStudents fails, while retaining
onError(alertService, res). Ensure the successful assessment is reloaded before
or alongside reporting the assignment-sync error, without changing the existing
success flow.
---
Nitpick comments:
In
`@src/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.spec.ts`:
- Around line 14-104: Add a create-mode test for
PresentationAssessmentFormDialogComponent by configuring the dialog with
presentationAssessment undefined, then verify studentSectionTitle() updates when
the form title changes. Ensure the test exercises the computed title after
patching the title control, while preserving the existing edit-mode coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 48f14309-3c0b-417c-92d8-3e585feb088e
⛔ Files ignored due to path filters (1)
src/main/resources/config/liquibase/changelog/20260706120000_changelog.xmlis excluded by!**/*.xml
📒 Files selected for processing (14)
src/main/java/de/tum/cit/aet/artemis/presentation/dto/PresentationAssessmentStudentDTO.javasrc/main/java/de/tum/cit/aet/artemis/presentation/web/PresentationAssessmentResource.javasrc/main/webapp/app/course/manage/course-management.route.tssrc/main/webapp/app/course/manage/update/course-update.component.htmlsrc/main/webapp/app/course/manage/update/course-update.component.tssrc/main/webapp/app/course/shared/services/sidebar-item.service.tssrc/main/webapp/app/presentation/manage/presentation-assessment-feature.guard.tssrc/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.htmlsrc/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.spec.tssrc/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.tssrc/main/webapp/app/presentation/manage/presentation-assessment-management.component.htmlsrc/main/webapp/app/presentation/manage/presentation-assessment-management.component.spec.tssrc/main/webapp/app/presentation/manage/presentation-assessment-management.component.tssrc/test/java/de/tum/cit/aet/artemis/presentation/PresentationAssessmentIntegrationTest.java
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/webapp/app/course/manage/update/course-update.component.ts
- src/main/webapp/app/course/shared/services/sidebar-item.service.ts
- src/main/webapp/app/course/manage/course-management.route.ts
- src/main/webapp/app/course/manage/update/course-update.component.html
- src/main/webapp/app/presentation/manage/presentation-assessment-management.component.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/webapp/app/presentation/manage/presentation-assessment-management.component.spec.ts`:
- Line 178: Update the imports in
presentationAssessmentManagement.component.spec.ts to include HttpErrorResponse
from the existing `@angular/common/http` import before its use in the addStudent
error test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 065936a0-41e1-4109-9745-22c0a55fbd07
📒 Files selected for processing (6)
src/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.htmlsrc/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.scsssrc/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.spec.tssrc/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.tssrc/main/webapp/app/presentation/manage/presentation-assessment-management.component.spec.tssrc/main/webapp/app/presentation/manage/presentation-assessment-management.component.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.spec.ts
- src/main/webapp/app/presentation/manage/presentation-assessment-management.component.ts
- src/main/webapp/app/presentation/manage/presentation-assessment-form-dialog.component.ts
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jsuckart [medium] The save-lifecycle issue remains on this head: isSaving rejects duplicate submissions, but Cancel and the dialog's default close/Escape controls can still dismiss the form during an in-flight create/update. The continuing request can then close a newly opened dialog and discard its input; disable every dismissal and reopen path while saving and add a regression test. The snapshot also still reports Codacy Static Code Analysis as ACTION_REQUIRED, with no repository evidence that it is unrelated.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jsuckart The current head fixes the previously reported dialog-dismissal lifecycle, and the relevant client, style, and build checks pass. However, Test / Server Tests (PostgreSQL) remains FAILURE and Codacy Static Code Analysis is ACTION_REQUIRED; the available evidence does not establish either completed failure as unrelated, so they must be fixed and rerun before approval.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jsuckart The dialog lifecycle and stale-response fixes are present on this exact head. However, the snapshot still records Codacy Static Code Analysis as a completed ACTION_REQUIRED failure, with no evidence establishing it as unrelated, so the existing changes-requested state must remain until that check passes or is shown to be unrelated.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jsuckart The current head resolves the previously reported dialog lifecycle, stale-response, assignment atomicity, and enrollment-query issues. However, the required All required CI Passed gate still fails because E2E / Report E2E Overall Status completed with FAILURE; the green Run All E2E Tests (PR) wrapper is intentionally continue-on-error, while the aggregator classifies the actual E2E verdict. The snapshot contains no test-level evidence establishing that failure as unrelated, so the E2E failure must be fixed or rerun before approval.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jsuckart The exact-head snapshot still reports E2E / Report E2E Overall Status as FAILURE, causing the required All required CI Passed gate to fail despite the green E2E wrapper job. The available evidence does not establish this completed failure as unrelated, so it must be fixed or successfully rerun before approval.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jsuckart No new static findings remain, and all retained review threads are resolved. However, E2E / Report E2E Overall Status is still FAILURE, causing the required All required CI Passed gate to fail; the available evidence does not establish that failure as unrelated, so it must be fixed or successfully rerun before approval.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jsuckart One medium save-state issue remains: the dialog blocks dismissal while saving but leaves its fields and student-assignment controls editable, so subsequent changes are silently discarded when the successful request closes it. The snapshot also reports E2E / Report E2E Overall Status as FAILURE, causing the required All required CI Passed gate to fail. The available evidence does not establish that CI failure as unrelated, so both issues must be addressed before approval.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jsuckart The previously reported save-state issue is resolved, and no new static defects remain. However, the exact-head snapshot still records Codacy Static Code Analysis as completed with ACTION_REQUIRED, with no repository evidence establishing it as unrelated; that failed check must be cleared before approval.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@jsuckart The current head resolves the prior save-state, stale-dialog, assignment-atomicity, enrollment-query, feature-gate, and course-scope issues. All retained threads are resolved, no new blocking defects remain, and the required CI gate passes.
MarkusPaulsen
left a comment
There was a problem hiding this comment.
Tested on TS 5. Works as expected, except the following follow-ups:
- Max. points and Result do not have any sensible upper bound (e.g. 1e+308 is possible)
- When creating a new presentation, all fields (except Presentation date) are missing a placeholder
- Max. points has a random value of 20 for no reason when creating a new presentation
- "New presentation" allows to add tutors, instructors and admins
jaylann
left a comment
There was a problem hiding this comment.
Tested on TS5. works as expected
jaylann
left a comment
There was a problem hiding this comment.
Overall clean PR. Just a few minor things i noticed






Summary
This PR introduces the first and very basic version of the presentation assessment feature.
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
This PR adds the basis for presentation assessment.
This is intended as a minimal first iteration that establishes the backend domain, REST API, a prototype UI and a feature toggle, while keeping the implementation scoped and independent from the existing exercise assessment flow.
Description
The PR adds:
presentationserver package with domain, DTO, repository, service, and resource classes.presentation_assessmentdatabase table for course-level presentation assessments./api/courses/{courseId}/presentation-assessments.presentationAssessmentsEnabledPresentationssidebar entry belowLectures, only visible to instructors when enabled for the course.Steps for Testing
Prerequisites:
Presentations Enabledtoggle is disabled by default.Presentationsentry appears in the course sidebar belowLectures.Presentations.Create presentation.Presentations Enabled, save, and verify that the sidebar entry disappears.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
Test Coverage
Client
Server
Last updated: 2026-08-26 11:13:07 UTC
Screenshots
Summary by CodeRabbit
Automated review screenshot