Looking into this

Pull Request here: https://github.com/wikimedia/InteractionTimeline/pull/117

@Niharika @dbarratt should we split this up into an server/client task? I'm happy to take on the server side of things

@Jdforrester-WMF Since CommonSettings is currently setting these two configs to true there shouldn't be any problems I assume.

I'd advocate for fixing all at once if there's a clean way to do it. @dmaza did you have a way in mind?

I do not.

I think that's a bit gross. I would honestly rather just leave that English-only than do that.

Do you care to expand on this? I don't think "gross" is a valid argument

In T185785#5414327, @dbarratt wrote:

would we rather:

1. Enable server-rendering which will allow us to display the message in the user’s language. This will require splitting the tool into two tools (which is trivial, imho, and can remain in a single repo if we want): https://lists.wikimedia.org/pipermail/cloud/2019-August/000774.html
2. Leave the app as “static” and display the message in all languages?

@Milimetric @nettrom_WMF correct me if I'm wrong but other than annotating that the schema is active[1] there isn't anything else I need to do to start logging events other than deploying the code[2] that would make use of it, right?

Here is the schema for our data capture. Feel free to make changes and I'll fix up the patches
https://meta.wikimedia.org/wiki/Schema:SpecialMuteSubmit

@dom_walden can you confirm that the issue is fixed tho? (Block list and log incorrectly report "cannot edit own talk page" for some blocks) I'll take a look at how the form displays the errors anyway

In T224468#5393469, @dom_walden wrote:

@dmaza Would it be better if the error message appeared underneath the "Editing their own talk page" checkbox?
This would make it immediately obvious what it refers to and would make it consistent with the location of other error messages (see below).

Makes sense, I'll fix it

In T224468#5375145, @dbarratt wrote:

Works for me.

Great

In T224468#5339366, @dbarratt wrote:

I'm saying you can show the error message, but the field may be disabled, so they may not be able to correct the error.

Sure, lets have another task and we can investigate there

Assuming $wgEnableSpecialMute = true. Neither $wgEnableUserEmailBlacklist nor $wgEchoPerUserBlacklist has to be true, which means you can see this link even if there is nothing you can do with it. I don't know why you would configure your wiki like this, though.

@dmaza I don't think we should show the link if the page is going to throw an error anyways. It's effectively an access denied (and perhaps we could use that mechanism?).

It shows an error message saying "Mute features are unavailable. This might be because: you haven't confirmed your email address or the wiki administrator has disabled email features and/or email blacklist for this wiki. "
We could check for $wgEnableUserEmailBlacklist but we can't check for $wgEchoPerUserBlacklist. I don't think there is anything we can do really other than what's already in place.

In T224468#5329582, @dbarratt wrote:

If a user tries to save ipb_allow_usertalk in a way that does not conform with those rules, an error message should be shown and the block should not be saved.

Or maybe just do the thing that is expected, rather than throwing an error (because if the form field is in a disabled state, you wont be able to resolve the error).
In other words, DWIM.

In T220163#5326499, @Niharika wrote:

@dmaza @dbarratt I agree it seems repetitive. We can make the user's name bold in the sentence above to make it a bit more prominent but let's go with ...from this user for now.

In T220163#5322863, @dbarratt wrote:

Instead of saying "Mute emails from this user" and "Mute emails from this user" would it be better to say: "Mute emails from user ABC" and "Mute notifications from user ABC"?
I was just wondering because when I first come to the page, my brain skips the first line entirely and I read the options, which use this and then I have to figure out what this is.

If the issue is that after unchecking "Editing", "Editing their own [...]" remains checked (although disabled), that will/should be fixed on T221117: Special:Block checkboxes should remember checked state. I personally don't see a problem other than what's described on T221117

I'm a bit confused as to what is the issue here

Expected Result:
Either:
Grey 'Editing their own talk page' when editing blocks are disabled

Isn't this what's happening right now?

In T218265#5313664, @dbarratt wrote:

@dmaza I prefer the non-prefixed version as I think User: is unnecessary context.

@Niharika Just to confirm. Should the form header include User: as part of the text or just the username?
E.g
"Please select your mute preferences for ‪User:Vandal‬."
vs
"Please select your mute preferences for Vandal‬."

In T226853#5305428, @Niharika wrote:

In T226853#5301929, @nettrom_WMF wrote:

@Niharika : Does AHT have any EventLogging schemas that are whitelisted?

I'm not sure. @dbarratt, @dmaza - do you know?

@Niharika this task introduces a few changes that conflict with the error messages we added on T218265: Mute: Add links to disable email and mute specific user to emails sent via Special:EmailUser

In T218265#5288275, @dom_walden wrote:

@dmaza The email footer I see now (in my MUA at least) is: To manage email preferences for Steve please visit <http://192.168.122.72/T218265/index.php/Special:Mute/Steve>.

@Niharika emails from Special:EmailUser are in plain/text. Currently the default email footer looks as follow if I use the plain msg:

This email was sent by Admin to Vandal by the "Email this user" function at devwiki. If you reply to this email, your email will be sent directly to the original sender, revealing your email address to them.
[http://dev.wiki.local.wmftest.net:8080/wiki/Special:Mute/Admin Manage email preferences for ‪Admin‬.]

+1 for CompositeBlock. IMO the name fits/describes how the object behaves (https://en.wikipedia.org/wiki/Composite_pattern)

In T218265#5227205, @Niharika wrote:

@dmaza I updated the task description. The things in bold are new. Did I manage to answer everything?
Sorry that the scope on this ticket keeps changing. :(

@dbarratt @Niharika @aezell Can any of you update the task description and summarize how this should look/behave/text in the page and so on? I'm not 100% sure what's the consensus here.

No. My point is that we need to make them checkboxes now, and avoid using verbs when arriving at this page.

This is what I was doing initially and we talked and agreed on not using a checkbox because there is only one option and it was bad UX (merging the lists or no)

In T218265#5221553, @Niharika wrote:

@dmaza I agree with David that "mute" by itself is ambiguous here. We should indicate that they are attempting to mute emails. So, for the first case, it should be like: "You have requested to mute emails from User:Vandal" and similarly for others.

Sure

https://github.com/wikimedia/InteractionTimeline/pull/100

Her are all the screens for this feature (so far)

@Niharika After discussing with @dbarratt we agreed that for this task it is not necessary to have a checkbox since the only available option will be to mute or unmute emails (for now). We propose to have a message indicating what action would happen depending if the user is already muted or not.

@dbarratt I meant to check on GlobalPreferences not on core. Same thing @Mooeypoo is suggesting but checking against what we already have on the db instead of a config

I think I already asked this but it escapes me if I did. Why was it that we can't check if a given option is already a global preference or a local exception?
In other words, when saving "whatever-option" on onUserSaveOptions we could determine if it is a global preference and/or if it has a local override and make the changes wherever they need to be done.
Considering that the choice to make something a global or a local exception is made by the user I would say that it will be expected that any changes to those options will be stored based on the user's choice.

You could also create a new topic on https://www.mediawiki.org/wiki/Manual_talk:Coding_conventions/PHP. It might add more visibility.

I think that supporting the definition to be either a callable or a string would be ideal (so a combination of your suggestions). It will make it backward compatible and we can start migrating into DI in incremental changes. SpecialPageFactory::getPage() already supports a string or a callable so a "quick" change would be to make $coreList non-static and adjust the SpecialPageFactory accordingly.
I'm not sure about Api Modules 'cause I haven't seen how that works

i did say feature flag originally ;)

You did, sorry I missed it :)

Copying over the "in-patch" comment for better discussion

It might also be a good idea to provide more info about mute (https://www.mediawiki.org/wiki/Help:Notifications#mute) and/or more information on how they can see their current email/mute list on Special:Preferences