-
Notifications
You must be signed in to change notification settings - Fork 302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exam mode
: Fix missing repository lock notifications
#7297
Conversation
…gifx/exam-update-notification
…gifx/exam-update-notification
…ls1intum/Artemis into bugifx/exam-update-notification
…gifx/exam-update-notification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good but I don't understand one client side change
...in/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.html
Outdated
Show resolved
Hide resolved
…gifx/exam-update-notification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Tested on LS11
|
@xHadie Thanks for testing.
I assume you mean the notification on the bottom right of the screen by that. This is intentional, as these notifications should only be used for stuff that needs immediate attention, whereas these types of notifications are more a confirmation that don't require any manual action.
Do you mean while working on an exam? This is also correct, as we don't want to distract users while using the exam mode.
This is unrelated to this PR, please open a GitHub issue to track this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Manual tested on legacy_ts11. Worked as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-tested locally, works as expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good.
Tested on legacy ts1, works as expected.
Checklist
General
Server
Client
Motivation and Context
The "The student repositories for this programming exercise were locked successfully." notification is missing for exam exercises, but it is an important piece of information for instructors to verify if the operation before and after the exam went well.
Description
The issue was that these type of notifications were "exercise updates" before. In exam exercises, these notifications get filtered out of from the notification bar (since the notification bar is not present during an exam and updates directly get highlighted in the exam viewer).
This PR introduces a new notification type for repository permission updates, which therefore won't be filtered out in the notification bar.
Steps for Testing
Prerequisites:
Exam Mode Testing
Prerequisites:
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Test Coverage
Screenshots