Page MenuHomePhabricator

Create upload log entries for all imported file revisions
Closed, ResolvedPublic8 Story Points

Description

Motivation
By default, every new file revision has an entry in Commons' upload log. FileImporter files should behave the same way.

Acceptance Criteria

  • Create upload log entries for (old) imported file revisions with the date and user of the original upload.
  • Create an upload log entry for the import. This log entry should be associated to the already created null revision

Examples of current situation
Right now, we create an upload log for the latest uploaded file revision, but not for the import, and not for anything older.
https://commons.wikimedia.org/wiki/Special:Log?page=File%3AAnnular+(PSF)+EST.svg vs
https://et.wikipedia.org/wiki/Eri:Logid?type=upload&page=Fail%3AAnnular+(PSF)+EST.svg

https://commons.wikimedia.org/wiki/Special:Log?page=File%3APappmach%C3%A9dynastie_Adt.svg
https://de.wikipedia.org/wiki/Spezial:Logbuch?type=upload&page=Datei%3APappmach%C3%A9dynastie_Adt.svg

Notes

  • First research showed that this is an issue of file import handler vs old file handler, deep down in core. Usually the old file handler expects all old upload logs to exist already, but in this case they don't. Hopefully this can be fixed in our extension.
  • This should include tests again

Related Objects

Mentioned In
rEFLI9780da7250c3: Use Status value instead of isLatestFileRevision bool flag
rEFLI012f7abde460: Adding tests for the null revisions upload log entry
rEFLI73254b817dd6: Test for upload log entries for imported file revisions
rEFLIceeea130ec9b: Use Status value instead of isLatestFileRevision bool flag
rEFLIaebbec212e5c: Create upload log entry for import null revision
rEFLI88259e9491de: Document and refactor log creation code in Importer and related
rEFLIae48724474c2: Let ImporterComponentTest support multiple file revisions
rEFLIc4eaefe6e931: Test for upload log entries for imported file revisions
rEFLI306316480e86: Test for upload log entries for imported file revisions
rEFLIbc01a70639f6: Adding tests for the null revisions upload log entry
rEFLIc8bae85d8297: Use Status value instead of isLatestFileRevision bool flag
rEFLI7b956e7b24b1: Document and refactor log creation code in Importer and related
rEFLI9d2df07c7506: Document and refactor log creation code in Importer and related
rEFLI1124a98d3402: Document and refactor log creation code in Importer and related
rEFLIc982a3295d29: Create upload log entry for import null revision
rEFLIb1717428e902: Let ImporterComponentTest support multiple file revisions
rEFLIf764fcc3d69a: Use Status value instead of isLatestFileRevision bool flag
rEFLIce1253b80652: Use Status value instead of isLatestFileRevision bool flag
rEFLIa1f379b54122: Fix broken ImporterComponentTest
rEFLI4a3522f2047e: Use Status value instead of isLatestFileRevision bool flag
rEFLId62638595128: Document and refactor log creation code in Importer and related
rEFLI27236dc5dd5b: Create upload log entry for import null revision
rEFLIc357a30bf7f1: Document and refactor log creation code in Importer and related
rEFLI90ec935c7b49: Create upload log entries for archived file revisions
rEFLI6065707c9c91: Create upload log entry for import null revision
rEFLI324861292389: Create all necessary upload log entries
rEFLI78803f9bbd13: Create all necessary upload log entries
rEFLIfa1da83d9311: Create all necessary upload log entries
rEFLI763713601d42: Create all necessary upload log entries
rEFLIe17590ad1578: [WIP] Create all necessary upload log entries
rEFLI36518ada8034: [WIP] Create all necessary upload log entries
rEFLIca09968e2714: [WIP] Create all necessary upload log entries
rEFLI19ded0b60870: [WIP] Create all necessary upload log entries
rEFLI39db043691f7: [WIP] Create all necessary upload log entries
T198136: Insert a log entry on import
T209942: FileImporter should add entries to the import log
Mentioned Here
T209942: FileImporter should add entries to the import log

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
WMDE-Fisch renamed this task from Not all upload log entries are created on import to Create upload log entries for all imported file revisions.Dec 3 2018, 11:20 AM
WMDE-Fisch updated the task description. (Show Details)
WMDE-Fisch updated the task description. (Show Details)Dec 3 2018, 11:25 AM
Lea_WMDE triaged this task as Normal priority.Dec 4 2018, 1:23 PM
Lea_WMDE updated the task description. (Show Details)
Lea_WMDE set the point value for this task to 8.
Pikne added a subscriber: Pikne.Dec 4 2018, 4:33 PM

So, if I import a file then log will say that I uploaded a file, eventhough particular file versions will be attributed to other users? I wonder why is that.

So, if I import a file then log will say that I uploaded a file, eventhough particular file versions will be attributed to other users? I wonder why is that.

Maybe ther's a misunderstanding here. The idea is to create upload log entries to all the imported file version that reflect exactly what's there on the source. So these are attributed to the "original users" and each file version will have this log entry with the original user.

Then, for the step of the import, we will have an import log entry associated to the importer and another upload log entry also associated to the importing user.

So do you think that additiona" upload log entry should also rather be associated to the "last user" of the "last file version" from the source?

Pikne added a comment.Dec 5 2018, 11:13 AM

So do you think that additiona" upload log entry should also rather be associated to the "last user" of the "last file version" from the source?

I don't think I understand why there should be an additional upload log entry. Isn't an import log entry associated to the importer enough? Currently upload log entries seem to be associated to file versions and their uploaders. Different approach confuses me a little. Upload log entry associated to the "last user" of the "last file version" is already generated.

Maybe the problem is that import log doesn't clearly indicate that file versions were also imported in addition to page revisions? If it's needed to indicate that in logs then maybe it'd clearer to do that in import log (not in upload log) using new import subtype (earlier I commented about it at T209942#4796371).

Then again, don't pay too much attention to my comments as you probably know all the technicalites on what and why needs to be done much better than I do. :)

should we create the logs as we import each revision or should we create all the logs once at the end of the import process?

@Andrew-WMDE, I'm not sure. I would say it does not matter so much. Whatever is easier to implement. What's relevant is in which order all these database entries appear in the end.

thiemowmde removed Andrew-WMDE as the assignee of this task.Dec 17 2018, 3:48 PM
thiemowmde added a subscriber: Andrew-WMDE.

Change 480719 had a related patch set uploaded (by Andrew-WMDE; owner: Andrew-WMDE):
[mediawiki/extensions/FileImporter@master] [WIP] Create all necessary upload log entries

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

Change 481845 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/FileImporter@master] Create upload log entry for import null revision

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

Change 481853 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Document and refactor log creation code in Importer and related

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

Change 480719 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Create upload log entries for archived file revisions

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

Change 481860 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/core@master] Do not create new archive file names for old files

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

Change 481988 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Use Status value instead of isLatestFileRevision bool flag

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

Change 482040 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Fix broken ImporterComponentTest

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

Change 482040 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Let ImporterComponentTest support multiple file revisions

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

Change 481845 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Create upload log entry for import null revision

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

Can be seen on beta now e.g. here:

https://commons.wikimedia.beta.wmflabs.org/wiki/Special:Log?type=upload&page=File%3AGreen_Image_Test.png

Two "old" upload log entries imported with the original file revisions and one new to make the import step transparent.

Change 481853 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Document and refactor log creation code in Importer and related

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

Change 483115 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/FileImporter@master] Adding tests for the null revisions upload log entry

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

Change 483115 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Adding tests for the null revisions upload log entry

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

Lea_WMDE closed this task as Resolved.Jan 10 2019, 11:32 AM
Lea_WMDE moved this task from Demo to Done on the WMDE-QWERTY-X-Mas-Sprint-2018-12-18 board.

Change 484617 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/FileImporter@master] Test for upload log entries for imported file revisions

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

Change 484617 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Test for upload log entries for imported file revisions

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

Change 481860 had a related patch set uploaded (by Marostegui; owner: Thiemo Kreuz (WMDE)):
[mediawiki/core@master] Do not create new archive file names for old files

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

Change 481860 merged by jenkins-bot:
[mediawiki/core@master] Do not create new archive file names for old files

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

Change 481988 had a related patch set uploaded (by WMDE-Fisch; owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Use Status value instead of isLatestFileRevision bool flag

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

Change 481988 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Use Status value instead of isLatestFileRevision bool flag

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