[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Page MenuHomePhabricator

Decouple Special:Investigate toollinks from Special:CheckUser toollinks
Closed, ResolvedPublic2 Estimated Story PointsJun 30 2020

Description

Problem
In T250290#6129101 Dom discovered that when the toollinks are similarly-named (link on enwiki), it becomes confusing in the UI.

special_checkuser_links.png (27×640 px, 5 KB)
special_investigate_links.png (480×234 px, 18 KB)

Acceptance criteria

  • Introduce a new toollinks JSON JsonConfig for all wikis and include some basic default tools. The default configuration should be in a MediaWiki config variable (expressed in extension.json)
  • Default tools should be the same as those currently in checkuser-toollinks and may be updated in T256280
  • Allow Special:Investigate toollinks to be set independently from Special:CheckUser toollinks

(Originally this task was for setting the toollinks via JSONConfig. See discussion in the comments for more details.)

Event Timeline

Change 605683 had a related patch set uploaded (by Dbarratt; owner: Dbarratt):
[mediawiki/extensions/CheckUser@master] Move tool link configuration to JsonConfig.

https://gerrit.wikimedia.org/r/605683

ARamirez_WMF set the point value for this task to 2.Jun 17 2020, 4:21 PM
ARamirez_WMF changed the subtype of this task from "Task" to "Deadline".
Aklapper renamed this task from Move toolink configuration to config variable and JsonConfig to Move toollink configuration to config variable and JsonConfig.Jun 17 2020, 5:56 PM

I hadn't realised that we would be the first to make use of JsonConfig in this way. Having looked into this more, it seems to be standard practice to use messages to build a list of links, e.g. sidebar (defines the nested list structure and uses other messages for the labels and URLs), loginlanguagelinks, gadgets-definition. These can be documented with the notranslate template to indicate that these messages should only be configured on wiki.

My main concern with being the first to use JSONConfig to do this is that adding NS_CONFIG to every wiki seems like a lot of overhead for this one small feature. If we do think these things should instead be handled by the JSONConfig extension, then I think we could make that case separately, and then have a wider discussion about enabling NS_CONFIG, whether it should be protected, etc.

For our current use-case, I think we should follow the existing practice, and use a new message.

Another idea, followup from conversations in the team:

I am not sure if enabling the Config: namespace in all wikis is the path of least resistance here, and I am slightly concerned about mixing the concepts of language and configuration. Since we should enable per-wiki config for the list of links, but we still want the titles to be translatable, I'd split the two apart, and follow the current practice (as @Tchanders pointed out above examples with gadgets or sidebar)

Here's what we can do:

  • Create a page in MediaWiki: namespace, ending with a .json suffix, that contains the configuration. Example: MediaWiki:Checkuser-toollinks-config.json - this will make the content type "json" which will also be EXTREMELY helpful for RTL languages on-wiki editing.
  • Mark this message in qqq as one that should not be translated, similarly to what exists in the toolbar message.

Now, if a wiki wants to add or change a tool, the can copy/paste the base definition into a local MediaWiki:Checkuser-toollinks-config.json page, and add an entry that has a language key. They can then locally set the translation of the keys, potentially with whatever translations on top of it.

Example, say I want to add the tool "foo" to Hebrew Wiki, I'll copy/paste the base definition and then add that line to the MediaWiki:Checkuser-toollinks-config.json page:

"CheckUserDefaultConfig": {
			"value": {
				"toollinks": [
					{
						"label": "checkuser-toollink-whois-rdns",
						"href": "https://www.robtex.com/whois/$1.html"
					},
					{
						"label": "checkuser-toollink-foo",
						"href": "https://foo.example.com"
					},

			}
}

And then I'll go to MediaWiki:checkuser-toollink-foo for my Hebrew label, and MediaWiki:checkuser-toollink-foo/en for an English label, keeping everything otherwise local.

After a long conversation today, we I learned today that a wiki can create new message strings and those message strings can be translated by creating subpages by language code (example). This wasn't immediately obvious, and I think MediaWiki should probably link to the translated subpages that are availble.

@Mooeypoo brings up an important point of separating the concerns (separating the config from the translations). However, it seems like it might be a slight abuse of the translation system to use it for config, even though, as @Tchanders point out, it's already being used that way. I wasn't able to find any other messages being used in a JSON format (they all seem to use a custom format).

I propose that we use JsonConfig, but remove the ability to provide a language map. This means that the config must consist of urls and message keys like @Mooeypoo has outlined in T255504#6265687.

There are some pros and cons to using JsonConfig. The pro is that it is a standardized system of handling configuration that exists outside of the translation system and provides some fairly good validation with proper error message. The con is that it will expose the Config namespace on our wikis which may open the flood gate for other extensions to start using it. It feels like config should exist in JsonConfig (and arguably, the functionality of that extension should be in core) and translatable strings should exist in the messages. However, if we really want to use a message for config, we certainly can, I'm not committed to using JsonConfig, it just feels like a slightly better option.

I was thinking MediaWiki would complain with a message key like checkuser-toollinks-config.json (with the . in there) in i18n/en.json, but it does work. This would only have to be put in en.json. Because of the language fallbacks, MediaWiki will use that value. I think it's technically fine to use a translated message with a string of JSON in it.

I guess the question is... which would we rather have? A new Config namespace or a string of JSON in a message key? what do you think @Tchanders?

If we use a message key, we will need to add a hook to ensure that an override remains valid and throws an appropriate error message (not a huge deal, but something to be aware of).

@Mooeypoo brings up an important point of separating the concerns (separating the config from the translations). However, it seems like it might be a slight abuse of the translation system to use it for config, even though, as @Tchanders point out, it's already being used that way. I wasn't able to find any other messages being used in a JSON format (they all seem to use a custom format).

We already have a bunch of tools that use pages in the MediaWiki: namespace for config, but in any case that's why I pointed out, in my proposal, that the separation is there -- the base config JSON can be used anywhere, really, but the "commonly used" place for this is most often the MediaWiki namespace. But then we separate the concern to have the actual translation messages themselves.

So I get what you're saying with "abusing" the message system in relation to the actual .json, but (a) we're doing that in other instances already, and (b) putting it anywhere else has a lot more implications (for example, enabling the Config namespace on all wiki is something that should be a broader conversation with the dev community, that probably also includes what existing config stuff should be moved into that namespace)

So, the straight forward solution is the one I outlined, and it's also one that follows the common pattern we've been using.

I propose that we use JsonConfig, but remove the ability to provide a language map. This means that the config must consist of urls and message keys like @Mooeypoo has outlined in T255504#6265687.

The problem with jsonconfig is adding a config namespace to all wikis. I don't think that's something we should do lightly just for one use-case. I think it should be a larger conversation that includes migration of current json/config elements in. I wouldn't get the team into this for this "quick" feature.

Using a message works for me, I don't have a strong feeling about JsonConfig. I'll make those changes on the patch (unless @Tchanders disagrees). :)

@dbarratt I'd be happy to use a .json message or a wikitext message. It sounds like there's an RTL-accessibility reason to use .json, so might be better to go with that.

I do agree that this kind of thing should probably be done via JSONConfig or something like it, so it might be worth starting a conversation with the developer community about this, separately from this work.

Thank you both for the detailed conversation.

I am leaning towards the quickest solution here. Let's use the .json message if that seems like the most straightforward way to accomplish this.

@dbarratt I'd be happy to use a .json message or a wikitext message. It sounds like there's an RTL-accessibility reason to use .json, so might be better to go with that.

I had a look locally into using a .json message, but it seems there's a problem with replacing the IP parameter if we do that. @Niharika I think the simplest solution is to use a wikitext message. That means we shouldn't need any code changes except for adding and using the new message.

We can use the existing checkuser-toollinks message as our default message. If wikis want to localise the labels, or add new tools with localised labels, they can create those messages locally and include them in their override of our new message.

(We could go one step further and add messages for the default labels.)

Change 609267 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/extensions/CheckUser@master] Add a new message for Special:Investigate tool links

https://gerrit.wikimedia.org/r/609267

Change 605683 abandoned by Dbarratt:
[mediawiki/extensions/CheckUser@master] Move tool link configuration to checkuser-investigate-toollinks.json.

Reason:
Went a different direction

https://gerrit.wikimedia.org/r/605683

Tchanders renamed this task from Move toollink configuration to config variable and JsonConfig to Decouple Special:Investigate toollinks from Special:CheckUser toollinks.Jul 6 2020, 4:57 PM
Tchanders updated the task description. (Show Details)

Change 609267 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Add a new message for Special:Investigate tool links

https://gerrit.wikimedia.org/r/609267

@dom_walden You might notice that some of the default links are outdated - I've filed T257230 (now merged into T256280) to address that.

Reassigning to @dbarratt, who did the majority of the work on this.

I haven't noticed any differences (compared to before) in the tool links while I have been testing CheckUser.

Test environment: local vagrant MediaWiki 1.35.0-alpha (0679746)