Page MenuHomePhabricator

show error when editor can't edit the statement because of permission errors on the repository
Open, HighPublic13 Story Points

Description

As an editor on the client I want to be warned if I can't make an edit on the repository in order to not waste my time.

Problem:
The Item where the statement is coming from might be protected on the repository or the editor might be blocked. In this case we want to tell them that they can't edit the statement they are trying to edit.

We have 3 cases that can happen:

  • The item is fully protected
  • The item is semi-protected
  • The user is blocked on Wikidata

Screenshots/mockups:
more details in figma mock

Desktop - Fully protected

Desktop - Semi-protected

Desktop - Blocked on repository

Mobile version (please note that typography switches to 16px size/24px line-height on mobile)

(Blocked on client +) blocked on repo+ protected on repo

Links in mocks

  • $repoName: ???
  • $clientName: ???
  • templates and items: ???
  • fully protected: $repourl/wiki/Project:Page_protection_policy
  • administrator(s): $repourl/wiki/Project:Administrators
  • protected: $repourl/wiki/Project:Page_protection_policy
  • protection log: $repourl/wiki/Special:ProtectedPages
  • editing disputes: $repourl/wiki/Project:Edit_warring
  • discuss this item: link to discussion page of respective item
  • semi-protected: $repourl/wiki/Project:Page_protection_policy
  • autoconfirmed/confirmed users: $repourl/wiki/Project:Autoconfirmed_users
  • log in: link to the client's log in page
  • create an account on $repoName: link to the client's account creation page (side-note from charlie: not sure about this one since editors might think that this is where they'll need to make the edits to get autoconfirmed but then still won't be able to edit since the auto confirmation happened on the wrong wiki)

BDD
GIVEN a protected Item
WHEN a user without the necessary edit rights tries to edit it in the bridge
THEN an error message is shown when opening the bridge
AND the edit can't be performed

GIVEN an editor that is blocked on the repository
WHEN they try to edit in the bridge
THEN an error message is shown when opening the bridge
AND the edit can't be performed

Acceptance criteria:

  • loading bar is replaced by error message as soon as possible (but abiding by the constraints listed in this ticket T232468) due to any of the reasons listed
  • the editability status depends on the Item the statement is coming from (which is not necessarily the one connected to the article via the sitelink)
  • These screens do not have a save button
  • The green-blueish side margin outlined on the specs must always be respected: margin sizes will not change due to text size changes
  • In case of triple error, notifications should be displayed following the order stablished by the designs: blockage on client + blockage on repo + item protection
  • Error notifications are accompanied by an expandable link component (find specs in WiKit): the link displays the error details on click/press
  • In case only a single message is displayed, then the expandable link is activated by default (error details are displayed) (See example "Desktop - Fully protected")

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a project: Wikidata. · View Herald TranscriptOct 10 2019, 8:11 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Lydia_Pintscher added a subscriber: Charlie_WMDE.

@Charlie_WMDE could you have a look and add a mockup and text if you have it?

Charlie_WMDE updated the task description. (Show Details)Oct 15 2019, 1:25 PM

@Lydia_Pintscher ready for story writing from my side. okay from your side?

In MediaWiki core, uneditable pages display two messages: permissionserrorstext-withaction, the generic “you can’t edit this”, and then protectedpagetext or blockedtext (and cascadeprotected in case of cascade protection) with parameters detailing the cause. We should do something similar in Data Bridge, but with bridge-specific messages. The messages should also be read from the client wiki, not the repo wiki, so they can be customized via the MediaWiki namespace. @Charlie_WMDE and @Lydia_Pintscher will define the initial content of those messages, based on the MediaWiki versions, before the next story time (2019-10-22).

Charlie_WMDE updated the task description. (Show Details)Oct 22 2019, 8:21 AM
Charlie_WMDE updated the task description. (Show Details)Oct 22 2019, 8:42 AM

TODO @Charlie_WMDE: unify the copy to consistently say "items" not "statements"

Charlie_WMDE updated the task description. (Show Details)Oct 22 2019, 9:05 AM
WMDE-leszek set the point value for this task to 13.Oct 22 2019, 9:15 AM
WMDE-leszek moved this task from Ready to estimate to Ready to pick up on the Wikidata-Bridge board.
Charlie_WMDE updated the task description. (Show Details)Oct 23 2019, 8:58 AM
Lydia_Pintscher triaged this task as High priority.Nov 4 2019, 10:02 PM
Pablo-WMDE added a subscriber: Pablo-WMDE.EditedNov 6 2019, 11:35 AM

During the task break down a question came up which does not seem to be answered:
What if a user is not allowed to edit for multiple reasons? Do we show (and explain) all of them? Or is there a hierarchy and we only show the most severe one?

Here’s what MediaWiki does in that case:

@Pablo-WMDE @Lucas_Werkmeister_WMDE good point. I hadn't considered this case. Will add a mock for it after lunch. Is this gonna block your task breakdown until then?

I don’t think it blocks us, but if you already know whether your mock will show one or all reasons, that would be the most important information for us.

I will aim to show all reasons, I just don't know exactly how yet. And in case there's no good way to do it I might opt to show only the more pressing issue (block above page protection)

Partial breakdown for now. We will have to break it down further.

Another question: the links that are mentioned are apparently not supposed to run through Special:MyLanguage - is that intentional?

Another question: the links that are mentioned are apparently not supposed to run through Special:MyLanguage - is that intentional?

No, sorry. My oversight. They should use it.

In the task breakdown, we identified three general approaches to implement this (affecting both how we get the information and how we display it):

  1. Add a new API on the repo which checks whether the user is allowed to edit the page and directly returns the HTML which we want to show in the bridge window.
  2. Pre-render all the messages to HTML in PHP, leaving the $ placeholders as they are, and then in JS only replace those dollars with their contents (HTML-escaped where necessary). This is similar to how AdvancedSearch handles the content of their help messages, except they don’t need to handle any $ placeholders.
  3. Combine all the messages into one block of wikitext (having $.i18n process {{GENDER}} etc.), then send that to the action=parse API to turn it into HTML.

In the second and third approach, it’s not fully determined how we determine which message(s) to show – the client could get that information ready-made from the repo via a new API, or it could get the page’s protection, user’s groups, and user’s blocks from existing APIs and determine the message(s) from that. See T237522: Investigate options to get block and protection info from remote repository.

Another question: How does this error state affect the save/publish button? The mock-ups show it hidden, but the AC do not mention it.

Is following the mockups in this case a big issue? If not then let's follow Charlie's mockups. If it is then let's discuss it.

Another question: How does this error state affect the save/publish button? The mock-ups show it hidden, but the AC do not mention it.

Good point. I will update the ACs

Charlie_WMDE updated the task description. (Show Details)Tue, Nov 12, 9:16 AM

update:

waiting on feedback from hanna and volker on design suggestions for the mixed permission messages. will get back to you when i have a response.

Everything else is solved so far, right?

Charlie_WMDE updated the task description. (Show Details)

We discussed that the investigation (T237522) uncovered that we currently cannot get the intended blockee from the mediawiki API. We decided that for now, we drop that requirement, i.e. that bullet point in the error message, and create a ticket to follow up later. See T238828: Show intended blockee in blocked message in wikidata bridge

We haven’t fully decided on how to implement this yet, but some points from the task breakdown:

  • We’ll need the title of the item on the repo wiki, which may be distinct from the ID (if there’s an Item: namespace). We’ll get this from the edit link, i. e. the RegExp we use to match it will gain a third capturing group that matches the whole title, not just the item ID part.
  • There will be a PermissionRepo (name not final) which is instantiated with a (Foreign)Api object (side note: probably rename the current ForeignApi interface to Api) and which, given a title, returns a (possibly empty) list of permission errors why the user can’t edit that title on the wiki targeted by the given Api.
  • There will be two instances of this PermissionRepo, one for the repo wiki and one for the client wiki. There will be some component which translates the generic errors they return (“user blocked”) into specific ones (“user blocked on repo”).

We have not yet decided how the titles will be communicated between various components. They may be passed around as method arguments, or they may be injected in the constructor at service creation time. We would prefer to decide this in pair programming, probably on Monday.

With regard to T235154#5640545, we decided:

  • We will parse messages PHP-side, before replacing parameters, and add the result (HTML potentially containing $1, etc.) to the parser output – most likely in mw.config under a name like dataBridgeMessages (analogous to dataBridgeConfig, but not a part of that). The JS-side code will then substitute the parameters without any further processing (but HTML-escaping them where necessary, of course).
  • The init module will load those messages and add them to mw.messages (using mw.messages.set()), so that they are available to the standard mw.message()/mw.msg() function and no modification to our MessagesPlugin or MwMessagesRepository is necessary.
Sarai-WMDE updated the task description. (Show Details)Tue, Nov 26, 12:47 PM

Links in mocks
administrator(s): $repourl/wiki/Project:Administrators

Where should this link go for the “blocked on client” error? I don’t think it makes sense to link to the repo’s administrators in that case, but on the client, the Project:Administrators page may not exist.

Ah, MediaWiki has a “grouppage-sysop” message we can use for the client version, I think.

Sarai-WMDE updated the task description. (Show Details)Tue, Nov 26, 2:44 PM
Sarai-WMDE updated the task description. (Show Details)Tue, Nov 26, 2:59 PM
Sarai-WMDE updated the task description. (Show Details)

With regard to T235154#5640545, we decided:

  • We will parse messages PHP-side, before replacing parameters, and add the result (HTML potentially containing $1, etc.) to the parser output – most likely in mw.config under a name like dataBridgeMessages (analogous to dataBridgeConfig, but not a part of that). The JS-side code will then substitute the parameters without any further processing (but HTML-escaping them where necessary, of course).
  • The init module will load those messages and add them to mw.messages (using mw.messages.set()), so that they are available to the standard mw.message()/mw.msg() function and no modification to our MessagesPlugin or MwMessagesRepository is necessary.

So I started writing the messages for this, but ran into trouble when they have to link to the repository (which is common in the mockups). To do this directly in the wikitext, the interwiki prefix of the repository would have to be available as a parser function or similar (it can’t be a parameter, because we’re not substituting those in PHP); or the wikitext would have to produce some kind of fake URL that we could replace with the real one, which would be unreliable; or there would be a placeholder for the whole link (<a> element), but then we need a “lego message” for the link text, separate from the rest of the message.

And it turns out that all of this was based on a false premise. I had assumed that there’s no way to parse wikitext in JS, that we don’t want to implement it, and therefore at some point we have to do the parsing in PHP – similar to what AdvancedSearch does. But as @Pablo-WMDE found out, MediaWiki does in fact support limited wikitext in JS – internal (non-interwiki) and external links, a handful of magic words, and a set of whitelisted HTML tags and attributes. This means that, for example, a list is written as <ul><li>foo</li><li>bar</li></ul> rather than * foo\n* bar, but there are already some messages targeted for that client-side parser that work like this, and so we can probably assume that our translators will be able to cope with this (though we can link to the mw.org documentation in qqq.json, of course).

So this seems like the saner way to go: declare a dependency on the mediawiki.jqueryMsg module, ship our messages as-is through ResourceLoader (no PHP-side parsing), process them in JS (mw.message( key, ...params ).parse()), and take care to only use supported syntax in the English version of the message (HTML tags rather than wikitext syntax).

Sarai-WMDE updated the task description. (Show Details)Tue, Nov 26, 5:17 PM
Charlie_WMDE updated the task description. (Show Details)Tue, Dec 3, 1:53 PM

Questions about the messages:

  • Are $repoName and $clientName really links? If yes, to what URL?
  • What’s the URL for “templates and items”?
  • Why does “protection log” link to Special:ProtectedPages rather than Special:Log/protect?

Edit: “protection log” could also link to that particular item’s protection log, using Special:Log/protect?page=Q123

Pablo-WMDE added a comment.EditedFri, Dec 6, 1:22 PM

Another question that came up: "this value is currently semi-protected on $repoName and can be edited only be autoconfirmed/confirmed users", second from last mock-up.
This sounds like it iterates the groups that can. In practice we know about rights which need to be granted. Certainly we can try to deduce the groups from these but we'd put more effort into this than MediaWiki itself (uses fixed messages per "reason", e.g. "semi-protected"). Is that intended?

(Praise @Lucas_Werkmeister_WMDE for keeping our terminology consistent!)