Page MenuHomePhabricator

Explore simple, campaign-specific "sync" functionality for statements in UploadWizard
Closed, ResolvedPublic

Description

User story: As an uploader, I want a way to apply my structured data edits to all files, so that I can avoid having to add data to each file manually when I do a batch upload.

We have this:
We've explored this functionality previously, but haven't had the time or resources to properly implement it for all use cases. However, now that we have some campaign-specific features for structured data on UploadWizard, we may be able to implement something simple just for campaigns.

We want this:
A button on the Add Data step of UploadWizard that duplicates all statements on the currently viewed file to all other files in the batch

Acceptance Criteria:

  • This functionality is OFF by default and can be enabled via campaign configuration JSON
  • The button for this functionality is disabled until changes to the data are made. Once changes are done, button goes back to disabled again
  • The functionality here is a replication of statements, not addition. If a file in the batch has existing statements, they get overridden/replaced by this function.
  • An "Are you sure?" confirmation dialog should appear when the function is initiated. Text: "Data on other files in this batch will be replaced with this file's data. Are you sure?"

During development, please test the following:

  • Test this feature on at least one mobile browser

Details

Related Gerrit Patches:
mediawiki/extensions/WikibaseMediaInfo : masterImprove DOM merge logic - nodes with different props are not equal
mediawiki/extensions/WikibaseMediaInfo : wmf/1.34.0-wmf.20Allow passing in non-default data
mediawiki/extensions/UploadWizard : wmf/1.34.0-wmf.20Add "copy statements to all files" button
mediawiki/extensions/UploadWizard : masterAdd "copy statements to all files" button
mediawiki/extensions/WikibaseMediaInfo : masterAllow passing in non-default data

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 26 2019, 6:24 PM

Change 532484 had a related patch set uploaded (by Eric Gardner; owner: Eric Gardner):
[mediawiki/extensions/UploadWizard@master] Add "copy statements to all files" button

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

Change 532690 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@master] Allow passing in non-default data

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

Tested it this morning with two images, and adding statements to one image each, and copying to the other. Seems to work fine...

Left two small questions in code review. I can approve this tomorrow (which will also fix T228912).

It doesn't quite work for quantity fields. It seems to work if you start with the top item, but when I started with the 2nd of 3 uploads and set two quantity statements, when I did the copy the statements appeared on the 1st and 3rd files, but the value wasn't set (this seemed to only happen the first time; as I did more tests with the same files, I couldn't replicate the initial problem).

Also, after setting statements for all files via the copy, I tried deleting all the statements on one file and then copying that new "empty" state to the other files. But the copy button didn't re-enable after deleting the statement (I had to add another value to get it to re-enable).

Also, "remove all" seems to be missing entirely from the interface on labs

It's so close(!) but not quite there 😦

I believe all of those things have now been fixed - can you do another run on labs to verify? :)

Change 532690 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Allow passing in non-default data

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

Okay, so here are the results of my latest round of testing on labs:

a.) I'm getting some bad errors during upload. I'm going to chalk this up to Labs being its usual unstable self, but that it was worth mentioning just in case it is a real problem.

b.) For multiple uploads, I'm seeing a bug where UploadWizard changes all the filenames to the name of the first image and puts them in a sequence (eg, "Some file", "Some file1", "Some file2"). Again, I assume this is a labs-specific bug but if it's not please do fix 🙂

c.) the Remove All links still aren't showing on Labs. I don't really care about that as long as they show up on Beta and Test

d.) Good news: the quantity bug from my last testing round appears to be fixed!

e.) Good news: the Copy-empty-statements bug from the last round appears to be fixed!

f.) Bad news: just like quantity before, it looks like plain-text fields don't get copied (I tried it with relative position within images). But I'm okay with that! I think, for the purposes of campaigns, that will be an extremely rare use case and we can fix it in a subsequent release.

Overall, I'd want to see things on Beta (the patches don't appear to be merged there yet), but if issues A - C are just labs things, I think we're good to go forward with this.

a) is nothing to worry about: already fixed in prod T231488

b) I'm not sure exactly what you're seeing, but it sounds a lot like what you'd get after copying in the "describe" step, with "Copy title (with automatic numbering)" checked (which is the default case)
Any chance that's what happened? I couldn't find any other way to reproduce something like what was described.

c) should probably get resolved in time for deploy

d) cool!

e) cool!

f) IDK what's going on here exactly, or whether it'll be fixed in time for deploy (or even regress in next week's deploy - there's been changes in this area, so ongoing work parallel to this copy functionality), but I agree it's not the end of the world should we not manage to get this resolved properly. will start looking into right away

Ah yes, I might have forgotten to uncheck the title checkbox on a copy on the describe step. I'm not entirely sure if I did, but I just tried again and could not reproduce it, and if you can't either I'd say just forget it. I probably borked something on my side 😅

Change 533166 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@master] Improve DOM merge logic - nodes with different props are not equal

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

Change 533167 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/UploadWizard@wmf/1.34.0-wmf.20] Add "copy statements to all files" button

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

Change 533168 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@wmf/1.34.0-wmf.20] Allow passing in non-default data

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

Change 533168 abandoned by Matthias Mullie:
Allow passing in non-default data

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

Change 533167 abandoned by Matthias Mullie:
Add "copy statements to all files" button

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

Change 532484 merged by jenkins-bot:
[mediawiki/extensions/UploadWizard@master] Add "copy statements to all files" button

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

Change 533168 restored by Matthias Mullie:
Allow passing in non-default data

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

Change 533167 restored by Matthias Mullie:
Add "copy statements to all files" button

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

Change 533168 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@wmf/1.34.0-wmf.20] Allow passing in non-default data

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

Change 533167 merged by jenkins-bot:
[mediawiki/extensions/UploadWizard@wmf/1.34.0-wmf.20] Add "copy statements to all files" button

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

Ramsey-WMF closed this task as Resolved.Sep 9 2019, 3:44 PM

We may revisit this functionality later, but for now this works.

Change 533166 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Improve DOM merge logic - nodes with different props are not equal

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

DannyS712 added a subscriber: DannyS712.

[batch] remove patch for review tag from resolved tasks