-
-
Notifications
You must be signed in to change notification settings - Fork 250
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 to delete user when unregister for notifications failed #3454
Fix to delete user when unregister for notifications failed #3454
Conversation
@@ -129,6 +129,7 @@ public void onNext(@io.reactivex.annotations.NonNull GenericOverall genericOvera | |||
@Override | |||
public void onError(@io.reactivex.annotations.NonNull Throwable e) { | |||
Log.e(TAG, "error while trying to unregister Device For Notifications", e); | |||
initiateUserDeletion(user); |
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.
especially this missing deletion command caused bugs..
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 code wise 👍
Nextcloud Server? Invalid auth because the token was removed, ... |
Whenever there was an error when unregistering from notifications, the user was not deleted. This could lead to multiple bugs. Furthermore, external signaling server connection and arbitrary storage is now always handled for user deletion, which was not the case before. Signed-off-by: Marcel Hibbe <[email protected]>
021af62
to
423ce97
Compare
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/3454-talk.apk |
it's not yet tested, but from my pov this should...
fix #3243
fix #3105
Not sure for which cases unregister for notifications might fail, maybe @nickvergessen knows best.
Further improvements for user deletions will come with #3436
🚧 TODO
🏁 Checklist
/backport to stable-xx.x