Skip to content

Conversation

@mikeyarce
Copy link
Contributor

This PR will disable the notifications checkbox for post_authors if:

  • The ef_notification_auto_subscribe_post_author filter is true (default behaviour)
  • If the post_author is currently subscribed (there is an edge case where they might not be)

We also add a visual indication that the user is the Post Author:
Screen Shot 2019-08-22 at 2 33 50 PM

Thoughts on implementation:
My first attempt was to change how the user list is populated in class-module.php, but I quickly realized that looping through each user to programmatically add details is resource-intensive when you have many users. I opted for JavaScript because this way we don't have to loop through each user, we can just target one specific user and modify the elements that way.

Fixes #508

Add  inline JS variables
`post_author_id` - the ID of the post author
`post_author_is_follower` - if the post_author is subscribed to post notifications
`post_author_auto_subscribe` - if post authors are set to be automatically subscribed
`maybe_disable_post_author_checkbox` will disable the post_author checkbox from the Notifications Meta Box if:
1. Post authors are automatically subscribed, and
2. If the post author is currently subscribed

`display_post_author_warning` will add a visual indicator that this user is the Post Author.
@mikeyarce mikeyarce requested a review from anigeluk August 22, 2019 21:38
@dchymko
Copy link
Contributor

dchymko commented Oct 31, 2019

Tested successfully in conjunction with #540. It makes sense to merge these together, as that will allow users to unsubscribe from their own posts which I don't think we want.

@dchymko dchymko requested a review from mjangda October 31, 2019 22:59
@cojennin
Copy link
Contributor

cojennin commented Nov 5, 2019

Looking good. Couple thoughts:

  1. When using the Block Editor, the disabled checkbox won't appear after the post is saved until after the page is reloaded. Can we observe a post save and trigger the disabled checkbox?
  2. It can also be the case that the current user will be auto-subscribed to the post if the filter ef_notification_auto_subscribe_current_user returns true. Can we add a similar UI treatment for the current user (i.e., show checkbox as disabled when post is saved and add a nice box with something like "Auto-Subscribed")?
  3. I really like this UI treatment, does anyone have any reasons as to why we can't just show it all the time instead of under certain conditions? It adds nice context to the notifications list and it'd be easier to maintain without logical checks

@mjangda mjangda added this to the 0.9.3 milestone Dec 2, 2019
@cojennin cojennin mentioned this pull request Dec 4, 2019
@mjangda
Copy link
Member

mjangda commented Jan 6, 2020

Closing; continued in #563

@mjangda mjangda closed this Jan 6, 2020
@mjangda mjangda deleted the fix-author-notification branch January 6, 2020 16:33
@GaryJones GaryJones removed this from the 0.9.3 milestone Dec 19, 2025
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.

Unsubscribing author from Notifications is impossible: Show error or disable unsubscribe checkbox

6 participants