Skip to content

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Apr 10, 2019

Fix #285
Fix #250

Server requirement: nextcloud/server#15040

@nickvergessen
Copy link
Member Author

@skjnldsv mind to help?
On master it seems that the Vue stuff is not working anymore. I can see <!-- --> being added to the DOM, but the app itself does nto show up anymore :(

@skjnldsv
Copy link
Member

@nickvergessen what you see is not the notifications app, the script is not even on the page for me :)

@nickvergessen
Copy link
Member Author

Did you checkout nextcloud/server#15040 on the server too?

@skjnldsv
Copy link
Member

@nickvergessen right, I did not.
The notification app is here for me, but indeed, I do not receive anything.
Capture d’écran_2019-04-16_14-32-23

The get requests return an empty response for notifications

@nickvergessen nickvergessen force-pushed the feature/13980/push-for-deleted-notifications branch from ecfeb67 to 1cb3553 Compare April 30, 2019 12:27
@tobiasKaminsky
Copy link
Member

When I remove a notification in web ui, I do receive this in android app:

app = null
id = null
nid = 343
subject = null
type = null

I can then succesfully remove associated notification 343 from status bar.
Is this intended that all values are null? I would have expected to have e.g. "type = REMOVAL"?

@nickvergessen
Copy link
Member Author

https://github.com/nextcloud/notifications/pull/318/files#diff-1383a847921c5d9a7163f3de431d5165R301

there are new parameters delete and delete-all

Type and ID refer to an object, so abusing them is not a good idea.

@tobiasKaminsky
Copy link
Member

there are new parameters delete and delete-all

Ah, great.
When can delete-all be triggered?

Type and ID refer to an object, so abusing them is not a good idea.

Never wanted to do this :-)

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented May 4, 2019

Automatically json unboxing has a problem with delete-all, can we change this to deleteAll?

@tobiasKaminsky
Copy link
Member

Android part is in: nextcloud/android#3969

@MorrisJobke
Copy link
Member

@nickvergessen What is the status here? We are close to the beta 1. Should this go into 17 or 18?

@nickvergessen
Copy link
Member Author

Still on track and planned for today+tomorrow

@nickvergessen nickvergessen force-pushed the feature/13980/push-for-deleted-notifications branch from 1cb3553 to b833ed9 Compare July 16, 2019 09:37
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Code looks good and works 👍

@nickvergessen nickvergessen force-pushed the feature/13980/push-for-deleted-notifications branch from 4848c3c to d8452e4 Compare July 18, 2019 07:04
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the feature/13980/push-for-deleted-notifications branch from f4c8ff1 to 7a18325 Compare July 18, 2019 07:38
@nickvergessen nickvergessen merged commit 19ab364 into master Jul 18, 2019
@delete-merged-branch delete-merged-branch bot deleted the feature/13980/push-for-deleted-notifications branch July 18, 2019 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extending the public APIs Notifications should vanish when they are acted upon

5 participants