Programming exercises: Deletion summaries for all exercises
#12020
+692
−355
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
tests
server
matyasht
changed the title
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
matyasht
changed the title
Programming exercises: Deletion summaries for all exercises
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
matyasht
force-pushed
the
feature/exercises/non-programming-deletion-summaries
branch
from
9e50a5e to
c4fdd52
Compare
github-actions
bot
added
the
core
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
… dynamically leaves values empty based on exercise type
…ogramming exercise
…le non programming exercises
matyasht
force-pushed
the
feature/exercises/non-programming-deletion-summaries
branch
from
c460c9c to
af61da0
Compare
github-actions
bot
removed
the
core
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
Contributor
WalkthroughAdds a generic exercise-level deletion-summary DTO and endpoint, implements a service to aggregate deletion counts across repositories, removes programming-specific deletion-summary types/endpoints, updates frontend wiring to use the generic endpoint, and adds integration tests and translations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ExerciseResource
participant ExerciseDeletionService
participant StudentParticipationRepository
participant BuildJobRepository
participant ResultRepository
participant ChannelRepository
participant PostRepository
participant AnswerPostRepository
Client->>ExerciseResource: GET /exercises/{id}/deletion-summary
ExerciseResource->>ExerciseDeletionService: getDeletionSummary(exerciseId)
ExerciseDeletionService->>StudentParticipationRepository: countByExerciseId(id)
StudentParticipationRepository-->>ExerciseDeletionService: participationCount
alt isProgrammingExercise
ExerciseDeletionService->>BuildJobRepository: countByExerciseId(id)
BuildJobRepository-->>ExerciseDeletionService: buildCount
end
alt isNotProgrammingAndNotQuiz
ExerciseDeletionService->>ResultRepository: countFinishedByExerciseId(id)
ResultRepository-->>ExerciseDeletionService: assessmentCount
end
ExerciseDeletionService->>ChannelRepository: findExerciseChannelByExerciseId(id)
ChannelRepository-->>ExerciseDeletionService: channel?
alt channelExists
ExerciseDeletionService->>PostRepository: countByChannelId(channelId)
PostRepository-->>ExerciseDeletionService: communicationPosts
ExerciseDeletionService->>AnswerPostRepository: countByChannelId(channelId)
AnswerPostRepository-->>ExerciseDeletionService: answerPosts
end
ExerciseDeletionService-->>ExerciseResource: ExerciseDeletionSummaryDTO
ExerciseResource-->>Client: 200 OK + DTO
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/test/java/de/tum/cit/aet/artemis/shared/architecture/ArchitectureTest.java (1)
306-319: Fix JsonInclude condition/violation message text.
The condition still advertises onlyNON_EMPTY, and the violation message repeatsNON_EMPTYand misses a space before “or”, which makes diagnostics misleading.🛠️ Suggested fix
- return new ArchCondition<>("Use `@JsonInclude`(JsonInclude.Include.NON_EMPTY)") { + return new ArchCondition<>("Use `@JsonInclude`(JsonInclude.Include.NON_EMPTY) or `@JsonInclude`(JsonInclude.Include.NON_NULL)") { @@ - if (!value.name().equals("NON_EMPTY") && !value.name().equals("NON_NULL")) { - events.add(violated(item, item + " should be annotated with `@JsonInclude`(JsonInclude.Include.NON_EMPTY)" + "or `@JsonInclude`(JsonInclude.Include.NON_EMPTY)")); + if (!value.name().equals("NON_EMPTY") && !value.name().equals("NON_NULL")) { + events.add(violated(item, item + " should be annotated with `@JsonInclude`(JsonInclude.Include.NON_EMPTY) or `@JsonInclude`(JsonInclude.Include.NON_NULL)")); }src/main/webapp/app/modeling/manage/modeling-exercise/modeling-exercise.component.html (1)
149-165: Incorrect element ID for modeling exercises bulk delete button.The
idattribute is set to"delete-all-quiz"but this is the modeling exercises component. This appears to be a copy-paste error and should be"delete-all-modeling"for consistency and accurate element identification in tests.Proposed fix
(delete)="deleteMultipleExercises(selectedExercises, modelingExerciseService)" [dialogError]="dialogError$" - id="delete-all-quiz" + id="delete-all-modeling" class="mb-1"
🤖 Fix all issues with AI agents
In
`@src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java`:
- Around line 124-144: The participation count in getDeletionSummary is using
participationRepository.countByExerciseId(exerciseId) which counts all
participation types; replace it with a student-only count by either calling
studentParticipationRepository.findByExerciseId(exerciseId).size() or,
preferably, add and use a new StudentParticipationRepository method
countStudentParticipationsByExerciseId(exerciseId) and call that from
getDeletionSummary to populate numberOfStudentParticipations so only
StudentParticipation rows are counted.
In
`@src/main/webapp/app/exercise/exercise-detail-common-actions/non-programming-exercise-detail-common-actions.component.html`:
- Around line 85-88: The template accesses course.title and course.testCourse
inside the delete button block guarded only by exercise.isAtLeastInstructor,
which can cause runtime errors if course is undefined; update the delete-button
block to also guard for course (e.g., wrap the block with an `@if` (course)) or
change usages to optional chaining (course?.title and course?.testCourse) so
references in the translateValues object are safe; ensure you update the block
that contains exercise.isAtLeastInstructor to include this additional course
presence check or replace direct course property access with course?.property.
In `@src/main/webapp/app/quiz/manage/detail/quiz-exercise-detail.component.html`:
- Line 10: Replace the unsafe non-null assertion quizExercise.course! used in
the jhi-quiz-exercise-manage-buttons input with a safe resolution that handles
both course and exam exercises: either use nullish coalescing to pass
quizExercise.course ?? quizExercise.exerciseGroup?.exam?.course, or call the
existing utility getCourseFromExercise(quizExercise) and pass its result as
[course]; update the template binding for jhi-quiz-exercise-manage-buttons to
use that safe expression so the component always receives a valid Course object.
In
`@src/main/webapp/app/text/manage/text-exercise/exercise/text-exercise.component.html`:
- Line 82: Replace the incorrect id value "delete-all-quiz" in the text exercise
template with "delete-all-text" and update any code that references the old id
string (e.g., querySelector or test selectors referencing
"delete-all-quiz")—look for occurrences in the TextExerciseComponent class and
related unit/e2e tests and change them to "delete-all-text" so the template id
and all consumers match.
In `@src/main/webapp/i18n/de/exercise.json`:
- Around line 25-35: The JSON translation contains a typo in the "summary.title"
key of the German exercise translations; change the value for "summary.title"
from "Aufaben-Zusammenfassung" to the correct "Aufgaben‑Zusammenfassung" (note
the missing "g" and use of a non-breaking hyphen if preferred) so the
user-facing string displays correctly.
🧹 Nitpick comments (4)
src/main/webapp/app/exercise/exercise-detail-common-actions/non-programming-exercise-detail-common-actions.component.html (1)
84-84: Refactor to avoid creating new Observable instances on each change detection cycle.Calling
fetchExerciseDeletionSummary(exercise.id!)in the template binding creates a new Observable on every change detection cycle. While the actual subscription only occurs on button click, this pattern is inefficient and wastes memory. Instead, pre-compute the Observable as a component property or useshareReplay()to cache the Observable across multiple accesses:deletionSummary$ = this.exerciseService.getDeletionSummary(this.exercise.id!);Then bind to the pre-computed property:
[fetchEntitySummary]="deletionSummary$"src/test/java/de/tum/cit/aet/artemis/exercise/ExerciseDeletionSummaryIntegrationTest.java (3)
195-219: Consider adding assertions for communication posts.The test validates participations, assessments, and builds correctly, but unlike the programming and text exercise tests, it doesn't assert
numberOfCommunicationPostsandnumberOfAnswerPosts. Consider adding these assertions for completeness:assertThat(summary.numberOfStudentParticipations()).isEqualTo(2); assertThat(summary.numberOfAssessments()).isEqualTo(1); assertThat(summary.numberOfBuilds()).isNull(); + assertThat(summary.numberOfCommunicationPosts()).isEqualTo(0); + assertThat(summary.numberOfAnswerPosts()).isEqualTo(0);
221-247: Consider adding assertions for communication posts.Same as the modeling exercise test - consider adding assertions for
numberOfCommunicationPostsandnumberOfAnswerPostsfor completeness.
249-265: Consider improving variable naming for clarity.The test logic is correct, but
quizExercise2obtained fromsubmission.getParticipation().getExercise()could be named more descriptively to clarify that it's a different exercise created by the utility method:Submission submission = quizExerciseUtilService.addQuizExerciseToCourseWithParticipationAndSubmissionForUser(course, TEST_PREFIX + "student1", false); - Exercise quizExercise2 = submission.getParticipation().getExercise(); + Exercise quizExerciseWithParticipation = submission.getParticipation().getExercise();
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java
Show resolved
Hide resolved
...exercise-detail-common-actions/non-programming-exercise-detail-common-actions.component.html
Show resolved
Hide resolved
src/main/webapp/app/quiz/manage/detail/quiz-exercise-detail.component.html
Outdated
Show resolved
Hide resolved
src/main/webapp/app/text/manage/text-exercise/exercise/text-exercise.component.html
Outdated
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java`:
- Around line 132-133: In ExerciseDeletionService, rename the misspelled local
variable hasAssesments to hasAssessments and update all usages (e.g., in the
ternary that sets numberOfAssessments) to the new name; ensure the declaration
"final boolean hasAssessments = !(exercise instanceof ProgrammingExercise ||
exercise instanceof QuizExercise);" replaces the old one and any references
elsewhere in the method/class are adjusted accordingly.
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java
Outdated
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/webapp/app/quiz/manage/manage-buttons/quiz-exercise-manage-buttons.component.ts`:
- Around line 101-103: The navigation call uses this.course()?.id which can be
undefined and produce an invalid route; change the logic in isDetailPage()
handling to use the guaranteed route-derived this.courseId (or guard against
undefined): obtain courseId from this.courseId (or const courseId =
this.course()?.id and check it), then only call
this.router.navigate(['course-management', courseId, 'exercises']) when courseId
is truthy; update the block around isDetailPage(), this.course(), this.courseId
and router.navigate accordingly.
src/main/webapp/app/quiz/manage/manage-buttons/quiz-exercise-manage-buttons.component.ts
Show resolved
Hide resolved
matyasht
force-pushed
the
feature/exercises/non-programming-deletion-summaries
branch
from
51bad90 to
8fea9b6
Compare
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
Currently, only programming exercises show a deletion summary, other exercise types do not.
This PR makes it consistent so that all exercise types have a deletion summary, indicating activity, and potentially avoiding accidental deletions.
Description
This PR extends the exercise deletion summary feature to all exercise types:
GET /programming-exercises/{id}/deletion-summarytoGET /exercises/{id}/deletion-summarythat works for all exercise typesExerciseDeletionSummaryDTOnow includes:numberOfStudentParticipations(all exercises)numberOfBuilds(programming exercises only)numberOfAssessments(text, modeling, file upload exercises only - excludes programming and quiz)numberOfCommunicationPostsandnumberOfAnswerPosts(all exercises)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
Screenshots
Summary by CodeRabbit
New Features
Localization
UI
Tests
✏️ Tip: You can customize this high-level summary in your review settings.