MBS-13364 / MBS-13377 / MBS-13378: Fixes / improvements for editor mailing preferences #3109
Conversation
reosarevok
added
Bug
| <FormRowCheckbox | ||
| field={field.skip_email_on_abstain} | ||
| label={l( | ||
| `When I vote Abstain on an edit, | ||
| do not mail me all future notes for that edit, even if | ||
| I usually get mailed on votes.`, | ||
| )} | ||
| uncontrolled | ||
| /> |
There was a problem hiding this comment.
I wonder if it'd be less confusing to rephrase the settings like
When I vote Yes or No on an edit, mail me all future notes for that edit.
When I vote Abstain on an edit, mail me all future notes for that edit.
And have the second one default to the value of the first one? (So it will be enabled by default, but if they previously disabled the first option, it'll be disabled.)
There was a problem hiding this comment.
I considered this, but then I remembered Approve is also a vote and it started to feel silly 🤔
There was a problem hiding this comment.
A little, but I guess we could have a separate string with "Approve" listed only for auto-editors. For normal users it might be easier to understand than the current negated option. :) Though I am fine with either if you prefer to keep it as-is.
There was a problem hiding this comment.
Hmm, I think I prefer it split out. It's quite a confusing concept, and bitmap's suggestion definitely helped me grok it faster. I don't think it's that bad if option A includes 'Approve'.
I looked at some other subscription thingies online and this seems to be pretty standard, wording wise:
Email me a notification when:
[ ] There is activity on an edit I have voted on
[ ] There is activity on an edit I have abstained on
There was a problem hiding this comment.
I did basically as aerozol suggested, including shortening the lines in a separate commit:
I also made it default to email_on_vote - I hope correctly, tested a bit and it did seem right at least.
email_on_notes is supposed to only determine whether you get mailed for notes added to other people's edits you left notes on. Notes left on edits you voted on are supposed to be governed by email_on_votes (currently useless, will fix in a subsequent commit). Notes left on your own edits are supposed to *always* be mailed.
We were not using this at all. This stops using email_on_notes for the vote case, and checks email_on_vote instead.
This gets rid of the weird hardcoded list of user IDs who do not get emails for notes left on edits they abstained on. Instead, it adds an actual preference, so that editors can choose whether they want to use abstain to wait and see what else the editors have to say (send emails), or to get rid of the edit on their unvoted list and not hear about it anymore (send no emails). If the editor hasn't yet picked an option here, the default will be based on their "email_on_vote" preference.
aerozol suggested that having a shared start per section and a shorter line for each checkbox makes this easier to follow and it does feel that way to me too.
If `$default` is a subroutine, it has to be called to get the default value. But we can't call it on the existing preferences object, because it may depend on the value of a preference which was modified (for example, `email_on_abstain` defaults to the value of `email_on_vote`). So we first construct the new preferences object to invoke `$default` on.
reosarevok
force-pushed
the
MBS-13364
branch
from
ed1edf6
to
84fd781
Compare
Fix MBS-13364 / MBS-13377, implement MBS-13378
Problem
Our entire preference system for mailing editors is a horrible buggy mess.
email_on_noteswrongly determines whether the original editor gets mailed,email_on_voteis completely unused and votes also depend onemail_on_notes, and we have an undocumented hardcoded feature changing the way the email works on abstain for three specific editor IDs.Solution
The first commit ensures the original editor is always emailed notes on their edits.
The second commit splits the checks for
email_on_notesandemail_on_voteso they actually affect notes and votes respectively.The third commit gets rid of the hardcoded IDs "preference" for abstain notifications and adds an actual preference that anyone can actually use, so that editors can choose whether they want to use abstain to wait and see what else the editors have to say (send emails), or to get rid of the edit on their unvoted list and not hear about it anymore (send no emails).
Testing
Added extra tests that take into consideration every one of these conditions.