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

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

Testing criteria

Related Objects

StatusAssignedTask
OpenJKatzWMF
InvalidNone
StalledNone
InvalidNone
DuplicateNone
DeclinedNone
InvalidNone
InvalidNone
Resolvedbmansurov
Resolvedpmiazga
Resolvedovasileva
Invalidovasileva
Resolvedphuedx
Resolvedphuedx
ResolvedJdlrobson
InvalidNone
Resolvedovasileva
InvalidNone
Resolved dpatrick
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

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

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

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

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

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

The above two patches are ready for review.

Jdlrobson added a subscriber: Jdlrobson.

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

bmansurov reassigned this task from bmansurov to Jdlrobson.Oct 4 2017, 6:27 PM
bmansurov added a subscriber: bmansurov.

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.

@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

phuedx awarded a token.Oct 5 2017, 1:36 PM

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

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

@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.

phuedx added a subscriber: phuedx.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?

I've explained the issue in T177805

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

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

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

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:53 PM

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

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

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

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

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

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

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

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

The remaining work is captured in subtasks.

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

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.

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

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.

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

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

can someone attach the pdf here?

ABorbaWMF added a subscriber: ABorbaWMF.

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

Desktop:



Mobile:

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.

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.