Page MenuHomePhabricator

Handle statements with unsupported properties
Closed, ResolvedPublic

Description

User story
As a normal user
I want to be able go to the File page
And view existing Q-item values for a non-depicts property

We have this
<undefined current behavior>

We want this
At the very least we should make sure non-depicts statements don't break the page

Acceptance Criteria

Test case set up at https://commons.wikimedia.beta.wmflabs.org/wiki/File:T219381.png

On the File page

  • Existing non-depicts statements should be rendered so that we can spot vandalism
  • They should be rendered differently (e.g. lighter/grayed out) so that it's obvious they are non-standard statements (for now)
  • Non-depicts statements need to be rendered in a way that makes sense: i.e. no obvious issues
  • These statements must not affect captions or depicts statements, which should continue to work as expected

Original ticket description below:

If a user creates a statement using an unsupported wikidata property (i.e. anything except 'depicts') what should we do?

At the very least we should make sure it doesn't break the page - our js isn't really set up for handling non-depicts statements on the page, so I'm not sure what the js will do if a non-depicts statement appears in the DOM (the php will show it by default)

We may need to turn off display of non-depicts statements entirely (via php)

@Ramsey-WMF this needs to be considered for the crawling release (even if we decide not to do it we do need to think about it)

Event Timeline

Cparle created this task.Mar 27 2019, 1:16 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 27 2019, 1:16 PM
Cparle renamed this task from Handle unsupported properties to Handle statements with unsupported properties.Mar 27 2019, 1:16 PM

JS will not do anything right now: the depicts widget will only attach to the node with the depicts property id, everything else will be left alone.

So right now, other statements simply get rendered just fine in PHP, but there's no JS to enhance it.
They can't be edited, but there's also no failures, and they render just fine (well, minus the interactive stuff like "edit" buttons etc) and get pulled into the "structured data" tab just fine.

Ok great - move this into 'needs qa' then?

Here's an example of an additional (non-depicts) statement: https://commons.wikimedia.beta.wmflabs.org/wiki/File:Photo_on_08-02-2019_at_16.35.jpg

Yeah, let's move this to "needs QA" to get input on whether this is OK or not?

Cparle moved this task from Untriaged to Next up on the Multimedia board.Mar 27 2019, 3:29 PM

Hmm the display of that is a bit janky ... @PDrouin-WMF any thoughts?

Moved back into 'doing' pending feedback from @PDrouin-WMF and @Ramsey-WMF

Assigning to @Ramsey-WMF so he can specify visual changes that are required

Cparle moved this task from Blocked to To Do on the Multimedia-Current-Work board.

Added a patch that simply adds a little whitespace between multiple statement groups.

Do we need to do anything else?

Change 500736 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@master] Separate multiple statement groups

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

Adding a little whitespace between multiple statement groups (until they're fully supported) works for me.

I believe @Ramsey-WMF was going to reply with how unsupported statements should be handled (hidden, visible but grayed out, etc.), since any unsupported statements won't be editable until they are actually supported.

Yes. Pam and I chatted about this recently and threw around a few ideas. We could:

  1. Hide the unsupported elements in the File Page UI completely. This has some problems though, as it hides potential problems and vandalism from admins
  2. Show the unsupported data but show them as visually different (probably grayed out). Editing via the UI wouldn't be an option, but at least admins could address issues via API

Number 2 is the best option in my opinion, and I spoke with @Cparle last Thursday about the feasibility of doing it. He mentioned it might be a bit tricky to do, and probably need to be done on the JS side since the PHP isn't aware of the necessary styling, but doable.

Change 500736 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Separate multiple statement groups

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

@Ramsey-WMF

Agreed that #2 is the best option, and we can make that work. I guess we could lower the opacity of non-depicts statement blocks to indicate the difference?

OTOH... thought I'd look into T219382: Allow editing of non-depicts statement block on File page first, and: it's not too much work! (I whipped up a WIP patch that I can finish off in a couple of hours)

So the options are:

  1. hide non-depicts statement blocks: no-one is aware of them (but vandalism will mostly go unnoticed until we turn other statements on)
  2. keep non-depicts statement blocks: vandalism will be noticed, but they're a pain in the ass to remove (need manually crafted API calls)
  3. apply the exact same JS functionality to other statement blocks as we have for depicts: vandalism would be easy to edit & delete

#3 would provide (the exact same) convenient UI to edit other statement (that have been added via custom API calls - there still is no UI to add other statements)

matthiasmullie added a comment.EditedApr 8 2019, 1:27 PM

So basically, our options in screenshots, for how to deal with other statements should user manage to sneak them in by crafting their own API calls to create them:

(all screenshots are with qualifiers on - just ignore those for now, they won't show up in initial releases)

#0 (current status)

#1 (just hide other statements)

#2 (show, but greyed out)

#3 (same UI as depicts)

Cparle moved this task from To Do to Doing on the Multimedia-Current-Work board.Apr 8 2019, 3:39 PM

As discussed yesterday, we're going to distinguish them visually (#2) AND apply the JS widget to make it easy to edit/delete vandalism.

This is going to look something like this:

If/when they decide to edit an unsupported statement, the opacity won't reduced (to make it easier to see what's being worked on)

As soon as unsupported statements switch back to read mode, their opacity is again reduced (see first screenshot)

Sounds good?

Change 502210 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@master] Apply DepictsWidget (now StatementWidget) to all statement blocks

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

Change 502210 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Apply DepictsWidget (now StatementWidget) to all statement blocks

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

matthiasmullie reassigned this task from matthiasmullie to Ramsey-WMF.EditedApr 15 2019, 11:25 AM
matthiasmullie moved this task from Code Review to Needs QA on the Multimedia-Current-Work board.

@Ramsey: this one wasn't really fleshed out. I updated the original description to add more detail & acceptance criteria based on earlier conversations, and on what I felt makes sense. Can you please verify these acceptance criteria ok are?
If yes: this has already been merged and is currently on beta - please re-assign to @Edtadros!

The current implementation looks good but I wonder if we need some sort of messaging to explain why some things are gray. Then, of course, the question would be where to put such messaging. If this were going to be a forever feature, I'd say we definitely need some explanatory text somewhere. But since this is (hopefully) going to be both temporary and rare, I question how much effort we should put it into it. Also pinging @PDrouin-WMF for input.

What about a tooltip that appears on hover?

What about a tooltip that appears on hover?

I'd be okay with that, although would that be on-hover over the entire panel or just the edit button? (the latter I assume would be easier ^_^)

Since it's temporary, whatever is easier should be fine.

Cparle updated the task description. (Show Details)Apr 17 2019, 3:48 PM
Cparle added a comment.EditedApr 17 2019, 3:52 PM

@Ramsey-WMF @PDrouin-WMF could we get a new ticket for the tooltip, seeing as this will be deployed without it, and is working as originally described?

Tested on beta

Tested on test-commons

I've looked at this briefly but could use some more expert eyes on. Could you double check this, @matthiasmullie

I did during deployment - seemed to do what I'd expect it to :)

matthiasmullie closed this task as Resolved.Apr 25 2019, 4:01 PM