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

Add support for rendering guides #338

Merged
merged 12 commits into from May 4, 2017
Merged

Add support for rendering guides #338

merged 12 commits into from May 4, 2017

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented May 2, 2017

  • Refactors part logic from travel advice to make it re-usable with guides (and other formats)
  • Add guides styles, template and tests
  • Include a guides print view for rendering all pages
  • Accept guides with no parts, but send airbrake notifications when detected
  • Include support for guides with only one part (ie no part navigation, print link, etc)

Omissions

PR excludes important features which will block switching the rendering app. These will be in later PRs:

Requesting content item from part rather than parent

Details in 1767b1c

The content store used to return a 404 when requesting the content item for a part (eg /foreign-travel-advice/albania/money). We needed custom routing for travel advice to request the content via the parent content item (/foreign-travel-advice/albania/). Content store has been updated to return a 301 when requesting a document's part, so a request to /foreign-travel-advice/albania/money will return the content we need. See: https://trello.com/c/w8HN3M4A/

The returned content item is the same for all parts and performs a 301 for any part slug, the frontend must determine which part was requested and whether it was valid.

  • Guides do not have a common prefix so cannot be determined to be
    content with parts before making a request to the content store
  • Guides and travel advice use mostly the same pattern for parts – however travel advice bundles up the summary (ie the default part) differently, whereas the default part for a guide is always the first one in details.parts
  • Remove special case travel advice controller, route and tests – any
    format could have parts
  • Pass the requested content item path to each presenter to determine
    what part is requested, if any
  • Check whether a part is being requested in the
    content_item_controller and redirect to base_path if part is invalid
  • Use part_slug to determine which part to show, if part is a valid

Screenshots

Old New
guides-old guides-new
@fofr fofr requested review from andrewgarner and gpeng May 2, 2017
@fofr
Copy link
Contributor Author

@fofr fofr commented May 2, 2017

@andrewgarner @gpeng I've requested your review on this PR as this refactors some of the routing logic I discussed with you when building travel advice.

fofr added 11 commits Apr 20, 2017
Run `bundle exec rails generate format guide`
The content store used to return a 404 when requesting the content item
for a part (eg /foreign-travel-advice/albania/money). We needed custom
routing for travel advice to request the parent content item
(/foreign-travel-advice/albania/). Content store has been updated to
return a 301 when requesting a document's part, so a request to
/foreign-travel-advice/albania/money will return the content we need.
See: https://trello.com/c/w8HN3M4A/

The returned content item is the same for all parts and performs a 301
for _any part slug_, the frontend must determine which part was
requested and whether it was valid.

* Guides do not have a common prefix so cannot be determined to be
content with parts before making a request to the content store
* Guides and travel advice use the same pattern for parts
* Remove special case travel advice controller, route and tests – any
format could have parts
* Pass the requested content item path to each presenter to determine
what part is requested, if any
* Check whether a part is being requested in the
content_item_controller and redirect to base_path if part is invalid
* Use part_slug to determine which part to show, if part is a valid
Allow guides to reuse existing parts logic:

* Extract parts navigation styles into reusable mixin
* Extract presenter logic into parts module
* Extract navigation template into partial
* Add parts navigation
* Include related items from govuk_navigation_helper
* Include link to print guide
* Guides currently use numbers
* Hide bullet points for all other parts (travel advice)
* Add unit test for parts module
A guide without parts is technically valid according to the schema. The
Publisher also currently allows it, although no guides have no parts.

We expect this to never be the case as a guide without parts shouldn’t
ever be approved and published. However, it could be true of the draft
content – ie a new content item without any parts _yet_.

Protect against no parts and send Airbrake notification if detected.
* The part title isn’t relevant when only one part, in this case the
document title is all that’s needed
* Also ensure that any part navigation is omitted

Live example of single page guide: https://www.gov.uk/corporation-tax
* Parts use ‘title’ not ‘name’
* The text part of this test was being omitted
@fofr fofr force-pushed the guides branch from 3428e92 to e3c570c May 2, 2017
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-338 May 2, 2017 Inactive
@gpeng
gpeng approved these changes May 3, 2017
@mcgoooo mcgoooo added the enhancement label May 4, 2017
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-338 May 4, 2017 Inactive
* Stub the method needed by the controller for redirecting
* Overwrite it using the `Parts` module on formats that have parts
Copy link
Contributor

@nickcolley nickcolley left a comment

💯

@nickcolley nickcolley merged commit 355724f into master May 4, 2017
2 checks passed
2 checks passed
continuous-integration/jenkins/branch This commit looks good
Details
security/snyk No new vulnerabilities
Details
@nickcolley nickcolley deleted the guides branch May 4, 2017
fofr added a commit to alphagov/frontend that referenced this pull request May 9, 2017
* Guides got moved to government-frontend:
alphagov/government-frontend#338
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.