Skip to content

Mark notification as read#932

Merged
sfdye merged 6 commits intoPyGithub:masterfrom
zer0tonin:mark_notification_as_read
Oct 18, 2018
Merged

Mark notification as read#932
sfdye merged 6 commits intoPyGithub:masterfrom
zer0tonin:mark_notification_as_read

Conversation

@zer0tonin
Copy link
Copy Markdown
Contributor

Wraps the parts of the notification API that allows the user to mark notifications as read.
New methods are :

  • Notification.mark_as_read : marks a single notification thread as read
  • Repository.mark_notifications_as_read : marks all the notifications for a given repository as read
  • AuthenticatedUser.mark_notifications_as_read : marks all the notifications as read

Aims to fix : #571 and simply uses the APIs described on this page : https://developer.github.com/enterprise/11.10.340/v3/activity/notifications/

A weird thing I noticed doing this is that Repository.notifications_url doesn't seem to be usable directly. I used Repository.url + "/notifications" instead.

@sfdye
Copy link
Copy Markdown
Member

sfdye commented Oct 16, 2018

Thanks for your PR. Very useful addition! Would you mind adding some unit tests for the new methods?

@sfdye
Copy link
Copy Markdown
Member

sfdye commented Oct 17, 2018

Also some conflicts need to be resolved 😝

.. code-block:: python

>>> repo = g.get_repo("PyGithub/PyGithub")
>>> repo.mark_notification_has_read()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
>>> repo.mark_notification_has_read()
>>> repo.mark_notification_as_read()

@zer0tonin zer0tonin force-pushed the mark_notification_as_read branch from 2f4fb82 to 7ff4ddc Compare October 18, 2018 17:12
@zer0tonin
Copy link
Copy Markdown
Contributor Author

@sfdye I think I took into account all your feedbacks now. Please let me know if I forgot something.

Copy link
Copy Markdown
Member

@sfdye sfdye left a comment

Choose a reason for hiding this comment

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

LGTM

@sfdye sfdye merged commit 0a10d7c into PyGithub:master Oct 18, 2018
@zer0tonin zer0tonin deleted the mark_notification_as_read branch October 19, 2018 07:54
candrikos pushed a commit to candrikos/PyGithub that referenced this pull request Sep 25, 2020
Wraps the parts of the notification API that allows the user to mark notifications as read.
New methods are : 
 
- Notification.mark_as_read : marks a single notification thread as read
- Repository.mark_notifications_as_read : marks all the notifications for a given repository as read
- AuthenticatedUser.mark_notifications_as_read : marks all the notifications as read

Aims to fix : PyGithub#571 and simply uses the APIs described on this page : https://developer.github.com/enterprise/11.10.340/v3/activity/notifications/

A weird thing I noticed doing this is that Repository.notifications_url doesn't seem to be usable directly. I used Repository.url + "/notifications" instead.
github-merge-queue bot pushed a commit that referenced this pull request Jul 30, 2024
This PR adds the ability to mark a notification as done, following the
already implemented `mark_as_read` by #932. Duplicated by #2976.

Fixes #2947

Co-authored-by: Hodei Navarro <hodei.navarro@outlook.com>
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.

Marking notification as read from PyGithub is either missing or undocumented

3 participants