Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare for dated newspaper page requests #11150

Merged
merged 19 commits into from
Nov 24, 2015
Merged

Conversation

JustinPinner
Copy link
Member

This PR adds handling of newspaper book and book section requests so that we no longer have to hand these off to R2.

Tests are misbehaving locally, so I'm using the PR to trigger a server-side build and test 😎

@@ -0,0 +1,26 @@
@(index: services.IndexPage, nav: PreviousAndNext)(implicit request: RequestHeader)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This view isn't used so can be removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, thanks. That was left over from the earliest days of this thing's existence. Should have been binned a while back. Will do that now.


}

private def tagExists(publication: String, tag: String) = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be worth renaming to bookSectionTagExists

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. Will do.

@jennysivapalan
Copy link

👍

JustinPinner added a commit that referenced this pull request Nov 24, 2015
Prepare for dated newspaper page requests
@JustinPinner JustinPinner merged commit a9e5757 into master Nov 24, 2015
@JustinPinner JustinPinner deleted the jp-newspaper-pages-2 branch November 24, 2015 10:45
@rtyley rtyley mentioned this pull request Jul 1, 2022
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants