Conversation
| def test_get_delete_notifications(self): | ||
| # get all notifications | ||
| res = self.nxc.get_notifications() | ||
| all_data = res['ocs']['data'] |
There was a problem hiding this comment.
This is interesting, the test looks like that it may end up either way - there are or aren't any notifications. I would appreciate more control - what about deleting, asserting that there are none, and then adding and rediscovering them. Does it make sense?
There was a problem hiding this comment.
Yes, you right. As far as I researched, right now there is no possibility to create actions using api. It will be possible if we have WebDAV api implemented, which will let us make files related changes, writing comments, or Federated files sharing api, which also creates activity objects when sharing with another cloud. Also the same related to activites api (#15) tests.
Noticed it in code below as well:
# TODO: add more tests if WebDAV api will be implemented and there will be ability to make actions
# using api which creates notifications (mentions in comments)
# or when Federated file sharings api will be implemented (harder way)
|
This looks good so far, good work! However, I took a look at the notification-related commit, and it has no relationship to #11. Next time, you can make the PR based on master, it will be cleaner, and you don't have to worry about merge errors, there won't be any, or they will be trivial to resolve. |
danil-topchiy
force-pushed
the
notifications
branch
from
44f35a6 to
d61c858
Compare
|
Thank you for your pull request! Please rebase to resolve the conflict - there is no other merge blocker here. |
danil-topchiy
force-pushed
the
notifications
branch
from
d61c858 to
383fb8f
Compare
|
@matejak resolved and rebased |
|
The inspection completed: 1 new issues, 6 updated code elements |
|
Great, merging! |
3 participants
Add class to work with nextcloud notifications api
Nextcloud notifications api docs:
https://github.com/nextcloud/notifications/blob/master/docs/ocs-endpoint-v2.md
Based on #11, will rebase to master after #11 will be merged