Skip to content
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

fix: Fix notifications of type "submitted" not being read #1163

Merged

Conversation

GabrielCWT
Copy link
Contributor

@GabrielCWT GabrielCWT commented Aug 26, 2024

Currently the notifications are not marked as read for type: submitted. When the grading is published, be it manually or automatically, the notification will be marked as read. The grading notification should also be marked as read when the submission is unsubmitted.

I feel that notifications should signify that there are things to be done on the grading page. This solution resolves this bug of there being "things to do" even though there aren't any.
Screenshot 2024-08-26 at 10 31 20 PM

@GabrielCWT GabrielCWT self-assigned this Aug 26, 2024
@coveralls
Copy link

coveralls commented Aug 26, 2024

Coverage Status

coverage: 94.365% (+0.009%) from 94.356%
when pulling ff7183c on GabrielCWT:fix/publish-notifications
into 2d43f01 on source-academy:master.

@GabrielCWT GabrielCWT marked this pull request as draft August 26, 2024 15:10
@GabrielCWT GabrielCWT marked this pull request as ready for review August 27, 2024 05:23
@GabrielCWT GabrielCWT changed the title fix: Mark notifications for grading as read when publishing grading fix: Fix notifications of type "submitted" not being read Aug 27, 2024
@GabrielCWT
Copy link
Contributor Author

GabrielCWT commented Aug 28, 2024

One thing I have noticed today: Paths also create a notification. Would it be a good idea to add a check where if is_manually_graded is true then create a notification, else do not. This is to follow the idea that the todo count is meant to be for things the staff needs to do in the grading tab.

Furthermore, there is no way to resolve the todo for a path since it is autograded and autopublished

@RichDom2185
Copy link
Member

One thing I have noticed today: Paths also create a notification. Would it be a good idea to add a check where if is_manually_graded is true then create a notification, else do not. This is to follow the idea that the todo count is meant to be for things the staff needs to do in the grading tab.

Furthermore, there is no way to resolve the todo for a path since it is autograded and autopublished

This can be a separate issue + PR, thanks!

Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@RichDom2185 RichDom2185 merged commit 4e19a57 into source-academy:master Aug 29, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants