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.

Sign up for GitHub

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

Jump to bottom

Don't auto-subscribe the current user when notification settings are changed #540

Merged
merged 4 commits into from
Nov 1, 2019
Merged

Don't auto-subscribe the current user when notification settings are changed #540

merged 4 commits into from
Nov 1, 2019

Conversation

dchymko
Copy link
Contributor

@dchymko dchymko commented Oct 31, 2019

Description

Anytime a post is saved, the current user is subscribed to the notifications. This includes when trying to remove themselves from the notification list resulting in it being impossible to unsubscribe from a post..

This PR skips auto-subscribing the current user whenever the notifications settings are updated.All other auto-subscribe functionality is retained. The current user can still subscribe to the post and if they explicitly save it, they will be resubscribed.

Related to #519 #520

Steps to Test

  1. Edit to a post not owned by you
  2. Unsubscribe from notifications.
  3. Refresh the page and note that you were not unsubscribed
  4. Apply patch
  5. Unsubscribe from the post
  6. Refresh the page, and note that you are now unsubscribed
  7. Save the post
  8. Refresh the page and confirm you are resubscribed

@dchymko dchymko changed the title Don't auto-susbscrbe the current user when notification settings are changed Don't auto-subscirbe the current user when notification settings are changed Oct 31, 2019
@dchymko dchymko requested a review from mjangda October 31, 2019 22:44
@dchymko dchymko changed the title Don't auto-subscirbe the current user when notification settings are changed Don't auto-subscribe the current user when notification settings are changed Oct 31, 2019
This was referenced Oct 31, 2019
Closed
Closed
mjangda
mjangda requested changes Nov 1, 2019
View reviewed changes
Daryl Chymko and others added 2 commits November 1, 2019 11:09
mjangda
mjangda approved these changes Nov 1, 2019
View reviewed changes
Copy link
Member

@mjangda mjangda left a comment

Choose a reason for hiding this comment

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

:shipit:

@mjangda mjangda merged commit 883f834 into master Nov 1, 2019
@mjangda mjangda deleted the fix/remove-yourself-from-notifications branch November 4, 2019 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjangda mjangda mjangda approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dchymko @mjangda