Page MenuHomePhabricator
Create Task
Maniphest T175856

Implement changes to article concatenation based on books requirements
Closed, ResolvedPublic5 Estimated Story Points
Actions

Assigned To
ovasileva
Authored By
ovasileva
Sep 13 2017, 6:53 PM
Tags
Referenced Files
F10389088: image.png
Oct 23 2017, 8:07 PM
F10389144: image.png
Oct 23 2017, 8:07 PM
F10389169: image.png
Oct 23 2017, 8:07 PM
F10389199: image.png
Oct 23 2017, 8:07 PM
F10263289: book (2).pdf
Oct 16 2017, 5:38 PM
F10262718: image.png
Oct 16 2017, 4:57 PM
F10180217: Screen Shot 2017-10-13 at 2.08.36 PM.png
Oct 13 2017, 12:09 PM
F9507166: book.pdf
Sep 13 2017, 6:53 PM
Subscribers
ABorbaWMF
Aklapper
• bmansurov
gerritbot
Jdlrobson
• Niedzielski
• Nirzar
View All 10 Subscribers
Tokens
"Mountain of Wealth" token, awarded by phuedx.

Description

Background

In T171838: Build out article concatenation according to requirements for books, we implemented the first run-through for concatenation, which was missing certain portions of concatenation. We split up these portions into this task so we can move forward with deploying the first portion

Acceptance criteria:

Add the following portions to book concatenation:

  • Title page: title, subtitle, author (if available)
  • A page break between the toc and the beginning of the first article
  • List of images used and licensing info
  • List of text contributors
  • Remove underline and dotted line from toc

example of book from current concatenation

book.pdf2 MBDownload

Testing criteria

Details

SubjectRepoBranchLines +/-
Add tests for the fixTemplateData methodmediawiki/extensions/Collectionmaster+50 -5
Hygiene: One single master template simplifies renderingmediawiki/extensions/Collectionmaster+56 -97
Deal with extreme of an empty bookmediawiki/extensions/Collectionmaster+29 -8
Restore table of content generation testsmediawiki/extensions/Collectionmaster+26 -2
Add images sectionmediawiki/extensions/Collectionmaster+115 -8
Improve print stylesmediawiki/extensions/Collectionmaster+86 -59
Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 Resolved• JKatzWMFT150871 [EPIC] (Proposal) Replicate core OCG features and sunset OCG service
 InvalidNoneT186740 [EPIC] It should be possible to print a book using the Proton service
 StalledNoneT174670 Remove banner from saved books
 InvalidNoneT171832 Deploy new book renderer to all projects
 DuplicateNoneT171833 Deploy new book renderer to all projects side by side with OCG
 DeclinedNoneT173018 Add an option in Special:Book to download PDFs generated by ElectronPdfService
 InvalidNoneT173015 Use PDF post-processing service to generate final PDF
 InvalidNoneT173579 Expose PDF post-processing scripts as a stateless web service
 Resolved• bmansurovT171965 [Spike - 8 hours] How should the PDF post-processing script be exposed for use by Extension:Collection
 ResolvedpmiazgaT171838 Build out article concatenation according to requirements for books
 ResolvedovasilevaT175856 Implement changes to article concatenation based on books requirements
 InvalidovasilevaT177805 [Spike] How do we render contributors and images section of books accurately?
 ResolvedphuedxT177672 Collection tests do not run properly
 ResolvedphuedxT177801 Collection phpunit tests are failing for table of contents when run locally
 ResolvedJdlrobsonT177892 PDF table of contents styling font-size is inconsistent
 InvalidNoneT177993 Article concatenation fails on large books
 ResolvedovasilevaT177994 Book generation fails for articles with '/' character in title
 InvalidNoneT177996 Article concatenation not resilient to curl errors
 Resolved• dpatrickT173014 Security review of pdfrw

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
ovasileva moved this task from Current Sprint to Backlog - Q2 on the Proton board.Sep 15 2017, 2:30 PM
ovasileva set the point value for this task to 5.Sep 19 2017, 4:18 PM
ovasileva moved this task from To Do to Needs Design Review on the Readers-Web-Kanbanana-Board-Old board.Sep 20 2017, 5:23 PM
• bmansurov claimed this task.Sep 26 2017, 1:28 PM
• bmansurov moved this task from Needs Design Review to Doing on the Readers-Web-Kanbanana-Board-Old board.
gerritbot subscribed.Sep 26 2017, 6:50 PM

Change 380815 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/extensions/Collection@master] Print styles: vertically align the cover page content

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

gerritbot added a project: Patch-For-Review.Sep 26 2017, 6:50 PM
• bmansurov moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Sep 27 2017, 6:27 PM

I'd appreciate reviews on the above patch while I work on the remaining bits.

gerritbot added a comment.Sep 27 2017, 9:33 PM

Change 381133 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/extensions/Collection@master] WIP: add image info

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

Jdlrobson moved this task from Needs Code Review to Doing on the Readers-Web-Kanbanana-Board-Old board.Sep 28 2017, 4:13 PM
gerritbot added a comment.Sep 28 2017, 4:15 PM

Change 380815 merged by jenkins-bot:
[mediawiki/extensions/Collection@master] Improve print styles

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2017-10-03 (1.31.0-wmf.2)).Sep 28 2017, 5:00 PM
• bmansurov moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Sep 28 2017, 6:20 PM

The above two patches are ready for review.

Jdlrobson moved this task from Needs Code Review to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.Oct 2 2017, 10:19 PM
Jdlrobson subscribed.

Lots of talking points came out of this task. Seems like it might benefit from a post-standup sync on how to approach it.

• Niedzielski added subscribers: pmiazga, • Niedzielski, • Jhernandez.Oct 3 2017, 6:24 PM

@pmiazga requested feedback from @Jhernandez and @Niedzielski on the following:

https://gerrit.wikimedia.org/r/#/c/381133/
https://gerrit.wikimedia.org/r/#/c/381277/

pmiazga mentioned this in T175868: Deploy and test new book rendering (Remex + Electron).Oct 3 2017, 9:06 PM
• Jhernandez unsubscribed.Oct 4 2017, 11:42 AM
• bmansurov reassigned this task from • bmansurov to Jdlrobson.Oct 4 2017, 6:27 PM
• bmansurov subscribed.
Jdlrobson moved this task from Needs More Work to Doing on the Readers-Web-Kanbanana-Board-Old board.Oct 4 2017, 6:28 PM
Jdlrobson added a comment.Oct 4 2017, 6:52 PM

We had a good chat today. We've agreed to keep the render code at a high quality, but improving technical debt of any other functionality of the collection extension should be considered out of scope for reading web.

I've agreed to take over the remaining work here to wrap this up.

Jdlrobson added a comment.Oct 5 2017, 12:45 AM

@pmiazga Experienced a lot of pain but I think I've got this somewhere where it's a little demystifying and have detangled the responsibilities of some of the methods here. Function names can probably be improved and I'd like to add some unit tests at some point but my Vagrant has been a pain in the ass today.

https://gerrit.wikimedia.org/r/382332
https://gerrit.wikimedia.org/r/382331
https://gerrit.wikimedia.org/r/382323

Jdlrobson moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Oct 5 2017, 12:45 AM
phuedx awarded a token.Oct 5 2017, 1:36 PM
Jdlrobson moved this task from Needs Code Review to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.Oct 6 2017, 11:30 PM
gerritbot added a comment.Oct 6 2017, 11:54 PM

Change 381133 merged by jenkins-bot:
[mediawiki/extensions/Collection@master] Add images section

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

Jdlrobson added a comment.Oct 6 2017, 11:59 PM

@pmiazga and myself have merged a lot of code today but we've found lots of problems with the renderer while doing this. Contributors and images sections are not displaying properly and it seems we are limited in the number of contributors we can show. For a book with 5 articles where the 1st article has over 500 contributors, no other contributors for any other articles show. Doesn't seem like we factored this into the design.... Let's resume this conversation Monday.

T177672 is making progress difficult so I've thrown that into the sprint.

Jdlrobson moved this task from Needs More Work to Blocked on Others on the Readers-Web-Kanbanana-Board-Old board.Oct 6 2017, 11:59 PM
ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2017-10-10 (1.31.0-wmf.3)); removed MW-1.31-release-notes (WMF-deploy-2017-10-03 (1.31.0-wmf.2)).Oct 7 2017, 12:00 AM
phuedx subscribed.Oct 9 2017, 5:20 PM

List of text contributors

@Jdlrobson to add detail as to how we can retrieve the complete list of contributors and their associated tradeoffs.

phuedx added a comment.Oct 9 2017, 5:30 PM

@Jdlrobson: Are there any of the AC that don't have a performance impact? Is fetching the list of contributors the most expensive AC?

MBinder_WMF mentioned this in T177672: Collection tests do not run properly.Oct 9 2017, 6:47 PM
Jdlrobson added a comment.Oct 9 2017, 11:42 PM

I've explained the issue in T177805

Jdlrobson moved this task from Blocked on Others to Doing on the Readers-Web-Kanbanana-Board-Old board.Oct 10 2017, 6:07 PM
ovasileva added a subtask: T177805: [Spike] How do we render contributors and images section of books accurately?.Oct 10 2017, 6:50 PM
ovasileva added a subtask: T177672: Collection tests do not run properly.
MBinder_WMF moved this task from 2017-18 Q1 to 2017-18 Q2 on the Web-Team-Backlog board.Oct 10 2017, 7:01 PM
gerritbot added a comment.Oct 10 2017, 8:25 PM

Change 383411 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Collection@master] Restore table of content generation tests

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

gerritbot added a comment.Oct 10 2017, 8:25 PM

Change 383412 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Collection@master] Deal with extreme of an empty book

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

gerritbot added a comment.Oct 10 2017, 9:29 PM

Change 382752 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Collection@master] Hygiene: One single master template simplifies rendering

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

Jdlrobson moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Oct 10 2017, 9:42 PM
gerritbot added a comment.Oct 10 2017, 10:43 PM

Change 383476 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Collection@master] Add tests for the fixTemplateData method

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

Jdlrobson updated the task description. (Show Details)Oct 10 2017, 10:48 PM
Jdlrobson created subtask T177892: PDF table of contents styling font-size is inconsistent.
Jdlrobson updated the task description. (Show Details)Oct 10 2017, 10:53 PM
gerritbot added a comment.Oct 11 2017, 1:02 PM

Change 383411 merged by jenkins-bot:
[mediawiki/extensions/Collection@master] Restore table of content generation tests

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

phuedx closed subtask T177672: Collection tests do not run properly as Resolved.Oct 11 2017, 2:45 PM
gerritbot added a comment.Oct 11 2017, 6:54 PM

Change 383412 merged by jenkins-bot:
[mediawiki/extensions/Collection@master] Deal with extreme of an empty book

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

gerritbot added a comment.Oct 11 2017, 6:54 PM

Change 382752 merged by jenkins-bot:
[mediawiki/extensions/Collection@master] Hygiene: One single master template simplifies rendering

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

ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2017-10-17 (1.31.0-wmf.4)); removed MW-1.31-release-notes (WMF-deploy-2017-10-10 (1.31.0-wmf.3)).Oct 11 2017, 7:01 PM
gerritbot added a comment.Oct 11 2017, 7:20 PM

Change 383476 merged by jenkins-bot:
[mediawiki/extensions/Collection@master] Add tests for the fixTemplateData method

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

• bmansurov moved this task from Needs Code Review to Needs QA on the Readers-Web-Kanbanana-Board-Old board.Oct 11 2017, 7:30 PM
Jdlrobson moved this task from Needs QA to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.Oct 11 2017, 7:40 PM
Jdlrobson created subtask T177993: Article concatenation fails on large books.Oct 11 2017, 7:54 PM
Jdlrobson created subtask T177994: Book generation fails for articles with '/' character in title.Oct 11 2017, 8:01 PM
Jdlrobson mentioned this in T177988: Warning: Invalid argument supplied for foreach() in /srv/mediawiki/php-1.31.0-wmf.2/extensions/Collection/includes/DataProvider.php on line 152.Oct 11 2017, 8:09 PM
Jdlrobson created subtask T177996: Article concatenation not resilient to curl errors.Oct 11 2017, 8:15 PM
Jdlrobson added a comment.Oct 11 2017, 8:32 PM

The remaining work is captured in subtasks.

• bmansurov added a comment.Oct 12 2017, 11:31 AM

I think the task's done as far as the A/C is concerned, no?

Jdlrobson moved this task from Needs More Work to Needs QA on the Readers-Web-Kanbanana-Board-Old board.Oct 12 2017, 5:16 PM
Jdlrobson reassigned this task from Jdlrobson to • Nirzar.Oct 12 2017, 5:20 PM
Jdlrobson added a subscriber: • Nirzar.

@Nirzar and @ovasileva to review testing criteria and output for HTML for books and capture any issues.

ovasileva added a comment.Oct 13 2017, 12:09 PM

@Jdlrobson - I'm having some trouble following the testing criteria. When I try to render, I get the following:

Screen Shot 2017-10-13 at 2.08.36 PM.png (267×771 px, 70 KB)

Jdlrobson added a comment.Oct 13 2017, 5:23 PM

Congrats you've discovered T177996 :)

There are lots of bugs unfortunately which stop rendering (what I've been trying to explain). Try rearranging the book or clicking "Clear cache" on the test page before rendering. It worked for me just now but yes this is very temperamental.

ovasileva closed subtask T177892: PDF table of contents styling font-size is inconsistent as Resolved.Oct 16 2017, 11:16 AM
ovasileva added a comment.Oct 16 2017, 1:04 PM

Got it to work when signing off for T177892: PDF table of contents styling font-size is inconsistent - looks good from my side.

• Nirzar added a comment.Oct 16 2017, 4:57 PM

Tried rearranging, clearing cache, and incognito... still

image.png (496×1 px, 145 KB)

can someone attach the pdf here?

ovasileva added a comment.Oct 16 2017, 5:38 PM

@Nirzar -

book (2).pdf1 MBDownload

ABorbaWMF moved this task from Needs QA to Ready for Signoff on the Readers-Web-Kanbanana-Board-Old board.Oct 23 2017, 8:07 PM
ABorbaWMF subscribed.

Looks good to me on desktop. I did notice some weirdness on mobile for the example site in the phab description.

Desktop:

image.png (768×1 px, 117 KB)

image.png (768×1 px, 117 KB)

image.png (768×1 px, 171 KB)

Mobile:

image.png (667×376 px, 97 KB)

Jdlrobson changed the status of subtask T177994: Book generation fails for articles with '/' character in title from Open to Stalled.Oct 30 2017, 11:26 PM
Jdlrobson changed the status of subtask T177993: Article concatenation fails on large books from Open to Stalled.
Jdlrobson changed the status of subtask T177996: Article concatenation not resilient to curl errors from Open to Stalled.
• Niedzielski reassigned this task from • Nirzar to ovasileva.Oct 31 2017, 5:01 PM

@ovasileva, is this good for sign off? Piotr says this is due to known templates issues.

Jdlrobson added a comment.Oct 31 2017, 5:01 PM

Despite all the bugs we have identified are we good to resolve this task or do you need more?

ovasileva closed this task as Resolved.Nov 1 2017, 4:53 PM

I think we're okay for now, especially given that we have paused work on books. Resolving.

ovasileva closed subtask T177993: Article concatenation fails on large books as Invalid.Apr 9 2018, 2:42 PM
ovasileva closed subtask T177994: Book generation fails for articles with '/' character in title as Resolved.
ovasileva closed subtask T177996: Article concatenation not resilient to curl errors as Invalid.Apr 9 2018, 2:44 PM
ovasileva closed subtask T177805: [Spike] How do we render contributors and images section of books accurately? as Invalid.Apr 9 2018, 2:48 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL