Exam mode: Notify instructors about changes in the test repository for exam exercise
#9242
Conversation
github-actions
bot
added
server
coolchock
changed the title
Exam mode: Notify instructors about changes in the test repository
coolchock
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
coolchock
changed the title
Exam mode: Notify instructors about changes in the test repositoryExam mode: Notify instructors about changes in the test repository for exam exercise
WalkthroughThe changes introduce a new notification type, Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code looks good 👍 One test is failing: https://bamboo.ase.in.tum.de/browse/ARTEMIS-TESTS7024-JAVATEST-4/test/case/769040033
Reading its java docs in the code looks a bit concerning, as the changes here might affect other clients (like the iOS app) too.
Is this an issue for this PR?
Its java docs
/**
* If this test fails, you have changed the notification placeholder files. This may have dramatic consequences on the mobile applications (iOS & Android) and the database.
* Changing the notification placeholders files changes the following things:
* 1. Other key value pairs are sent to the clients. This may mean that the native clients no longer understand them and notifications are no longer shown to users.
* 2. The database becomes inconsistent. The placeholders are stored as JSON in the database.
* You must now do the following:
* 1. Check if you really need to change these placeholders. If not, revert your changes.
* 2. Write a database migration for the old placeholder JSON strings, such that they match your new signature.
* 3. Increment the {{@link de.tum.in.www1.artemis.config.Constants#PUSH_NOTIFICATION_VERSION}}. This ensures that old versions of the native apps discard your new
* notifications.
* 4. Update both the Android and iOS app. Only merge this server PR after they have been updated and released to the stores. Otherwise, notifications no longer work for
* end users.
* 5. Execute the Gradle task generatePlaceholderRevisionSignatures to make this test pass again.
*/
| public static String[] createPlaceholderExerciseNotification(String courseTitle, String exerciseTitle) { | ||
| return new String[] { courseTitle, exerciseTitle }; | ||
| } | ||
|
|
||
| @NotificationPlaceholderCreator(values = { EXERCISE_RELEASED, EXERCISE_PRACTICE, QUIZ_EXERCISE_STARTED, EXERCISE_UPDATED, PROGRAMMING_TEST_CASES_CHANGED, |
There was a problem hiding this comment.
Is this list and the one above the same? And will they also stay the same?
If so, please move them into a common array and use it for these two occurrences
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
Checklist
General
Server
Client
Motivation and Context
Instructors need to be notified about changes in the test repository for programming exercises in exams. This is crucial because such changes can affect student results. However, the rebuilding of submissions does not happen automatically, as it is a very expensive operation, especially for exams with around 2000 students. Rebuilding must be triggered manually, and instructors should do this only after they have completed their changes in the test repository.
Description
A new notification type has been added. This notification is enabled by default. Instructors will receive a notification in the navigation bar as well as a message pop-up to ensure they do not miss it. The notification is triggered when changes to the test repository affect the results (e.g., changing the test logic), but not for non-impactful changes (e.g., modifying a comment). The notification is only sent if there are already existing results for the exam programming exercise.
Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Summary by CodeRabbit
New Features
Localization
Bug Fixes
Tests