SEO: keyword and contributors index pages #5199
Conversation
|
@gklopper This build is failing because I've added a mandatory configuration property |
|
Sure, but just put it in the properties files in the project, its not a secret so no need to have it in the puppet files. |
|
Have you shown these to Peter Martin? |
|
@gklopper Yeah, I spoke to Peter yesterday. He's happy for all the contributor tags to be in the index, segmented by their first letter. As there's a lot of keywords, he only wants us to show the ones that are in public sections from the Content API (he's further curating this list of sections, as even some of them should not be in the index), and he wants the keyword pages to be segmented by their section. This pull request takes care of all of that. We'll just need to remove a couple entries from the list once Peter gets back to us. To clarify, so I don't do this in error, by just put it into the properties files in the project, do you mean the properties files in this repository (the Thanks! |
|
Yes, these properties files |
|
Consider paginating when displaying more than 100 links per page. |
|
@tudorraul Would probably help visually anyway. Given that maybe we need to subdivide further on keywords after section, to do it by the initial letters of the title (as you suggested), and also apply that to the initial letters of the contributors. We should talk to the SEO people first, but if they're happy with that, it might be a good thing for you to do some more server side stuff on. |
|
No reason not to ship this while we wait for design. It is effectively unreachable in PROD. |
|
Anything happening with this? |
|
I was just doing some last minute changes before shipping it, as there were requirements I wasn't aware of. I'll ship it today I PWOMISE! 🏇 |
|
@tudorraul Could you give the nav stuff I've done here a once-over and call me out if it's crazy-insane-crazy? |
|
@kaelig I've kind of copy-pasted some of the styles here from article and made a new stylesheet, as I imagine the styles here might diverge a lot once somebody with an actual eye for design looks at the pages. If this is a crazy thing to do let me know & I'll fix. |
| @@ -0,0 +1,11 @@ | |||
| package common | |||
|
|
|||
| object StopWatch { | |||
There was a problem hiding this comment.
The only client of this object sits in the admin project, should they be collocated?
There was a problem hiding this comment.
Maybe. I guess I thought it was general purpose enough other people might want to use my horrible code. I'm probably wrong though ;-) I'll move it.
SEO: keyword and contributors index pages
In June 2018 PR #262 introduced useful new auto-paginating methods like `paginate()` in the CAPI client, but it was effectively restricted to _only_ **Content** search queries (ie not for paginating through Tags or Atoms): https://github.com/guardian/content-api-scala-client/blob/49004e43f002812c8fc1d1b2629c665325785d04/client/src/main/scala/com.gu.contentapi.client/model/Queries.scala#L195-L200 This PR adds pagination for **Tag** queries, and it uses an approach that probably could be extended to paginate Atoms queries too. We're only interested in Tags right now because we're trying to replace the pagination code based on [`play-iteratees`](https://github.com/playframework/play-iteratees) in guardian/frontend#5199 which is blocking the Scala 2.13 upgrade for Frontend! In order to get this to work, we've done several refactors: * different query types paginate different ways - Content `SearchQuery`s use the `content/[capi-id]/next` & `content/[capi-id]/prev` endpoints, but `TagsQuery` don't have those, so we use basic page-number incrementing instead (tags are created/deleted less frequently than Content, probably ok!). We have to be able to choose the appropriate method based on the query type, and so queries that want to support pagination now must extend `PaginatedApiQuery`, implementing the methods that define what a 'following' query is. * rather than having the internal Response thrift `Decoder`s be parametrised by _Query_, they're now parametrised by _Response_ - this means: * there can be fewer of them - just **1** decoder for `AtomsResponse`, rather than **6**. * the typelevel machinery to find the appropriate Decoder for a Response is greatly simplified (ie, it's just the new `Response` type parameter of `ContentApiQuery`) - no need for the [`Aux` pattern](https://gigiigig.github.io/posts/2015/09/13/aux-pattern.html) to work out the `Response` from the `Query`, and so the `Aux` typeclass is deleted ☠️ * the functionality of `PaginatedApiResponse` was cleanly supported in the new Response `Decoder`s for `SearchResponse` & `TagsResponse`, so `PaginatedApiResponse` was deleted too ☠️ Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk>
The latest version of the CAPI client (v19) includes guardian/content-api-scala-client#359, which we can use to replace the pagination code based on `play-iteratees` (https://github.com/playframework/play-iteratees - introduced in July 2014 with #5199), currently blocking the Scala 2.13 upgrade for Frontend (see #24817). The updated CAPI client is binary-incompatible with previous versions of the CAPI client, meaning we also need to update the FAPI client as it also uses CAPI. The updates to the CAPI client also support `previous`/`next` search functionality, as used by the Lightbox, meaning that the small `ContentApiNavQuery` hack for the Lightbox (introduced here: #20462) can now be removed. A similar PR updating FAPI & CAPI in Ophan is at guardian/ophan#4719. Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk> We're adding a test that will break at build time rather than runtime if our versions of the FAPI & CAPI clients are incompatible. See also: * #25139 (comment) * https://github.com/guardian/ophan/pull/4719/files#diff-ee7f97c92065084bba37d70d043ad0daa0d7745f235d0ad3206b59f073829529
The latest version of the CAPI client (v19) includes guardian/content-api-scala-client#359, which we can use to replace the pagination code based on `play-iteratees` (https://github.com/playframework/play-iteratees - introduced in July 2014 with #5199), currently blocking the Scala 2.13 upgrade for Frontend (see #24817). The updated CAPI client is binary-incompatible with previous versions of the CAPI client, meaning we also need to update the FAPI client as it also uses CAPI. A similar PR updating FAPI & CAPI in Ophan is at guardian/ophan#4719. Like there, we're adding a test that will break at build time rather than runtime if our versions of the FAPI & CAPI clients are incompatible. The updates to the CAPI client also support `previous`/`next` search functionality, as used by the Lightbox, meaning that the small `ContentApiNavQuery` hack for the Lightbox (introduced here: #20462) can now be removed. See also: * #25139 (comment) * https://github.com/guardian/ophan/pull/4719/files#diff-ee7f97c92065084bba37d70d043ad0daa0d7745f235d0ad3206b59f073829529 Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk>
Initial A-Z indexes for keywords and contributors. I'm going to make a separate pull request for the keywords indexed by section.