Refactor UW statements code
Closed, ResolvedPublic
Actions

Description

We have this:

Statement widgets are currently being constructed in 2 separate files:

resources/controller/uw.controller.Metadata.js for the default statements (depicts)
resources/metadata/uw.MetadataContent.js for the others ("add statement" manually)

They're being constructed with different config.
This thing is currently pretty brittle.
See: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/UploadWizard/+/513631/2/resources/metadata/uw.MetadataContent.js#59
See: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/UploadWizard/+/513631/2/resources/controller/uw.controller.Metadata.js#110

We want this:

This should be refactored so that creation of statements doesn't happen in different ways in different places.

Details

Project	Branch	Lines +/-	Subject
mediawiki/extensions/WikibaseMediaInfo	wmf/1.34.0-wmf.20	+4 -0	No need to deal with formatvalue caches if they've already been cleaned up
mediawiki/extensions/WikibaseMediaInfo	master	+4 -0	No need to deal with formatvalue caches if they've already been cleaned up
mediawiki/extensions/UploadWizard	master	+324 -272	Refactor UW Statements Code

Customize query in gerrit

Related Objects

Mentioned Here: T135963: Add support for Content-Security-Policy (CSP) headers in MediaWiki

Event Timeline

matthiasmullie created this task.Jun 5 2019, 2:33 PM

Restricted Application added a project: Multimedia. · View Herald TranscriptJun 5 2019, 2:33 PM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

matthiasmullie added a project: SDC-Statements (Other-statements-running).Jun 6 2019, 9:46 AM

Ramsey-WMF assigned this task to matthiasmullie.Jun 17 2019, 4:29 PM

Ramsey-WMF moved this task from Untriaged to Next up on the Multimedia board.

Ramsey-WMF moved this task from To Do to Doing on the Structured-Data-Team-Current-Work board.

Ramsey-WMF moved this task from To Do to Doing on the Structured Data Engineering board.

matthiasmullie removed matthiasmullie as the assignee of this task.Jul 9 2019, 9:11 AM

matthiasmullie moved this task from Doing to To Do on the Structured-Data-Team-Current-Work board.

egardner added a subscriber: egardner.Aug 7 2019, 5:59 PM

Aklapper edited projects, added Structured-Data-Backlog; removed Multimedia.Aug 10 2019, 11:54 PM

Aklapper edited projects, added Structured-Data-Backlog (Current Work); removed Structured-Data-Backlog.Aug 10 2019, 11:57 PM

Change 529840 had a related patch set uploaded (by Eric Gardner; owner: Eric Gardner):
[mediawiki/extensions/UploadWizard@master] WIP: Refactor UW Statements Code

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

gerritbot added a project: Patch-For-Review.Aug 12 2019, 11:38 PM

egardner claimed this task.Aug 12 2019, 11:39 PM

I'm going to take a stab at this. I spent a large chunk of today walking through the UW code in a JS debugger to get a better sense of the flow.

Would it make sense to move all the statement-widget-creation logic into the controller class? That seems like the best place for it, but OOUI makes it hard to separate logic and presentation; moving logic there from the MetadataContent file means that setting up event listeners on user-generated StatementWidgets becomes more of a hassle, for example. I'm going to attempt to do this anyway to see how far I can get.

I'm also fighting with some kind of OOUI default, where new StatementWidget input elements get an inline display: none style for some reason.

WIP patch is here, I'll continue working on it tomorrow: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/UploadWizard/+/529840

Ramsey-WMF moved this task from Incoming to Doing on the Structured-Data-Backlog (Current Work) board.Aug 14 2019, 2:04 AM

egardner moved this task from Doing to Code Review on the Structured-Data-Backlog (Current Work) board.Aug 21 2019, 2:16 PM

matthiasmullie moved this task from Doing to Verify on Production on the Structured Data Engineering board.Aug 23 2019, 8:36 AM

matthiasmullie moved this task from To Do to Needs QA on the Structured-Data-Team-Current-Work board.Aug 23 2019, 8:42 AM

Change 529840 merged by jenkins-bot:
[mediawiki/extensions/UploadWizard@master] Refactor UW Statements Code

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

ReleaseTaggerBot added a project: MW-1.34-notes (1.34.0-wmf.20; 2019-08-27).Aug 23 2019, 10:00 AM

Maintenance_bot removed a project: Patch-For-Review.Aug 23 2019, 10:10 AM

This patch has been merged. Regarding QA needs, we've done extensive testing locally but it would be good to go through the basic UW tasks (both within and outside of campaigns) on Beta just to ensure everything is ok. Then this ticket can probably be closed.

Something I've seen on UW today (autosuggest box getting stuck in "loading" mode), CORS JavaScript errors in the console:

That's on beta, I suspect?
What steps did you do to trigger this? Just added the new property, and that's how it then appeared? Or did you start typing an item? Or did it only appear once you added the other statements or edited anything else?

Yes, on Beta.

Steps were simple: I just tried to add the new property (AErBWxVT) and type in letters to trigger the autosuggest. That property got stuck in the loading/wait state, so I tried adding another. I was able to add it but got nothing in the autosuggest for those subsequent properties (not even No Results Found).

The JavaScript console listed CORS errors in trying to hit beta wikidata, and also a "abortable.abort is not a function" error.

Oh, forgot to add that this seems to be intermittent. I did a lot of UW testing and it mostly worked fine. Every now and then something weird like this would happen though.

Ok cool, thanks for the info!
I suspect the CORS (CSP) errors in console have nothing to do with it (see T135963), but the "abort is not a function" might.

Change 532549 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@master] No need to deal with formatvalue caches if they've already been cleaned up

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

gerritbot added a project: Patch-For-Review.Aug 27 2019, 7:42 AM

Change 532549 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] No need to deal with formatvalue caches if they've already been cleaned up

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

Maintenance_bot removed a project: Patch-For-Review.Aug 27 2019, 9:10 PM

ReleaseTaggerBot edited projects, added MW-1.34-notes (1.34.0-wmf.21; 2019-09-03); removed MW-1.34-notes (1.34.0-wmf.20; 2019-08-27).Aug 27 2019, 10:00 PM

Change 532956 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@wmf/1.34.0-wmf.20] No need to deal with formatvalue caches if they've already been cleaned up

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

gerritbot added a project: Patch-For-Review.Aug 28 2019, 10:02 AM

Change 532956 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@wmf/1.34.0-wmf.20] No need to deal with formatvalue caches if they've already been cleaned up

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

ReleaseTaggerBot edited projects, added MW-1.34-notes (1.34.0-wmf.20; 2019-08-27); removed MW-1.34-notes (1.34.0-wmf.21; 2019-09-03).Aug 28 2019, 12:00 PM

Maintenance_bot removed a project: Patch-For-Review.Aug 28 2019, 12:10 PM

I backported the patch that I believe should fix this - are you still able to get into this stuck state now?

I have tried to reproduce the bug on Beta for a while now, and I haven't seen it! Good sign that it's no longer an issue 👍🏼

Ramsey-WMF moved this task from Needs QA to Verify on Production on the Structured-Data-Backlog (Current Work) board.Sep 9 2019, 4:37 PM

As discussed in today's backlog grooming, if UW works, then the refactor worked :) Closing.