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

[jerclarke]

trafficstars

Expected behavior:

• When unticking the post author in Notifications metabox, they get unsubscribed
• OR If they aren't actually unsubscribed, we get a red flash instead of green after AJAX returns, and their box is still ticked.

Current behavior:

• Unticking the post author causes a green flash, and the user seems to be unticked
• Reloading the page shows they have been re-ticked, it is impossible to unsubscribe the post's author

Thoughts:

• This is really confusing to users, and likely to make them misunderstand the situation if they don't go back and look at the Notifications metabox after reloading
• I understand the reasons why the post author would remain always subscribed, and I don't think we need to change that aspect of this, it's understandable and users can work around it.
• BUT: if it's impossible to unsubscribe the author, we should make it clear when someone tries to untick them that it didn't work.

Proposed UX: AJAX returns red flash and leaves the user ticked

• Unticking the current author causes a red flash, and when AJAX returns, the user is still ticked.
• This seems the easiest to do by editing how the AJAX response is handled.
• Downside is that if the author select menu on screen has been changed, but not saved, it is the "old" author that will be un-unsubscribeable, rather than the "new" one, since our AJAX can only see the saved author on the backend.

Alternate UX: Disable the checkbox for current author based on a CSS class

• When rendering the user list, add a CSS class to the current post author: post_following_list-author
• CSS class matches the ones used to filer the notified list display,like post_following_list-current_user
• Use this to disable the checkbox for the author so they can't be unticked.
• Downside is the same as the AJAX solution: Doesn't account for the author having been changed during this pageload.
• Upside is that this seems really easy to implement. Adding a disabled property to the input that just lasts until the page is reloaded wouldn't involve much code at all.

Alternate UX 2: Disable checkbox based on currently selected author on screen

• Same as previous UX, but we also have JS that updates the "disabled" user in Notifications based on who is currently selected as the author in the author metabox
• This seems a bit harder because now we need to update the checkbox whenever the author UI is changed, and involves external (changing) JS, unlike the first two solutions which only rely on get_the_author()-type unchanging API calls.
• Upside is that this will reflect the state AFTER saving better, because if you change the author AND unsubscribe the old one, then the old one will indeed remain unsubscribed
• FWIW I think that whole scenario of changing the author and unsubscribing the old one is VERY niche, and won't come up very often. Fixing the core problem of making it clear you can't unsubscribe the current author is a much higher priority.

Can you replicate the key problem?

Maybe I'm missing something. Would be great just to hear if I'm right about the key premise, that it's impossible to unsubscribe the current author, and the UI lies and says that it is possible.

[mikeyarce]

Thanks for the report, @jerclarke !

Wanted to add a note that there is a filter, ef_notification_auto_subscribe_post_author which can be set to false so the author isn't automatically subscribed to notificaitons. https://github.com/Automattic/Edit-Flow/blob/19a9680518a9fbcca40495c041529cd6ff461dd5/modules/notifications/notifications.php#L522

I think one thing we could do is check this filter, and if it's not false, we disable the checkbox so you are not able to unsubscribe the post author.

[jerclarke]

I think one thing we could do is check this filter, and if it's not false, we disable the checkbox so you are not able to unsubscribe the post author.

Okay yes, this makes sense. We should check this filter before having these effects on the UI otherwise it would just create new confusion.

[mikeyarce]

@jerclarke curious for your thoughts on this proposal:

1. Let's disable the checkbox for the post_author and add a notice:

If the author is changed, it will auto-subscribe the new author and then disable that new author, but it won't be visible until after the author has been changed.

However, if the post_author is NOT Subscribed (they can click Following from the Post List, we actually enable the checkbox so they can subscribe.

2. Remove the logic entirely that always subscribes the Current User. Do you see any potential reason to keep this? To me, it seems to add more confusion and complexity and unexpected results.

[jerclarke]

I like that ef_notification_auto_subscribe_post_author is the default. It seems logical to my users and it saves a lot of time that everyone can take it for granted. I don't want to disable ef_notification_auto_subscribe_post_author though I of course can imagine why people would. I think having both options, and the defaults as-is is great.

I just want the box to be disabled if unticking the author isn't going to work.

If they have ef_notification_auto_subscribe_post_author disabled, then the box disablement is disabled too.

Here's a lightweight UI that I think would solve it and match the No Access flag. Getting the "[Post Author]" part to visually match a disabled checkbox is my favorite part:

[mikeyarce]

@jerclarke I like your lightweight suggestion for the UI - good idea!

[WPprodigy]

Linking to the new PR for this: https://github.com/Automattic/Edit-Flow/pull/563