[WIP] Add travel advice pages #4225
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leenagupte
force-pushed
the
add-travel-advice-pages
branch
from
05be61d
to
13d875b
Compare
leenagupte
force-pushed
the
add-travel-advice-pages
branch
from
aa8759c
to
182a438
Compare
leenagupte
force-pushed
the
add-travel-advice-pages
branch
from
db969d7
to
a9de3d9
Compare
leenagupte
force-pushed
the
add-travel-advice-pages
branch
from
abfcfd8
to
6539c30
Compare
leenagupte
force-pushed
the
add-travel-advice-pages
branch
from
6539c30
to
dbf66da
Compare
Adds the existing translation keys for `en` and `cy` to the other languages.
It replicates the logic from government-frontend: https://github.com/alphagov/government-frontend/blob/cbbbf70e5f2fd5a41b0208e9c01f9083bb6363d3/app/presenters/content_item_presenter.rb#L74-L80 It's only used in the show template, and passed to the machine_readable_metadata component. The travel advice index template calls `Frontend.govuk_website_root + root_path` directly in the template. Considered using this pattern, but having `Frontend.govuk_website_root` defined seems unnecessary, and calling `Plek.new.website_root` directly in the view feels wrong.
The tests check that the print variant renders, they don't test that the correct variant is being printed because that would be testing that setting `request.variant` does the right thing.
Replaces the need to create a body_for_metadata method: alphagov/government-frontend@74f6e4e
Travel Advice pages need to known the withdrawn status in able to construct the page title. See: https://github.com/alphagov/government-frontend/blob/main/app/presenters/content_item/withdrawable.rb#L8 and https://github.com/alphagov/government-frontend/blob/main/app/presenters/travel_advice_presenter.rb#L6-L12 However, some of the methods feel like model concerns rather than presentation concerns, so they have been moved the to model. There is only one example of the content item with a withdrawn notice in content schemas, so the shared example is being tested on the generic content item model rather than travel advice. The travel advice page title is a presentation concern and will be added in a later commit.
This presenter is badly named ContentItemModelPresenter because ContentItemPresenter already exists. ContentItemPresenter takes a hash of the results from content store and models them. That work is already being done by the ContentItem model, and shouldn't be repeated in the presenter. Some extra work needs to be done to evaluate the existing routes to see how they can be unstitched from the existing ContentItemPresenter and then this class can be removed. The page_title has been added to a generic presenter because other document types from government-frontend use it as the basis for their own page title methods.
And use it in the views
It's not possible to use `@presenter` as that is being used by the
ApplicationHelper to define the wrapper classes. If you do, and you
don't have a "publication" defined, you get the following error:
NoMethodError:
# private method `format' called for #<TravelAdvicePresenter
./app/helpers/application_helper.rb:15:in `wrapper_class'
It's worth looking into unstitching all of this, but outside of this PR.
Pinched from TakePart PR: 1a20adf The translations should be added in that PR.
This was in the travel advice presenter in government-frontend. However as it is a purely presentation concern, it did not feel appropriate to add it to the model. It'll be needed by some new presenter methods.
Adds methods to the TravelAdvice model. These information is only exists for Travel Advice, so it doesn't need to be added to the generic ContentItem model.
This has been added to the application helper because it seems generic enough. It will also be required for other document_types. The original location was in a presenter in government-frontend: https://github.com/alphagov/government-frontend/blob/b002d46b804d1b11e9fca12fff7b022cd4ae2960/app/presenters/content_item_presenter.rb#L128
This takes the metadata presenter method from government-frontend: https://github.com/alphagov/government-frontend/blob/main/app/presenters/travel_advice_presenter.rb#L14-L28 but rather than recreating it, builds the params directly in the view. The old presenter method need to use view_context to call the `simple_format` helper method, which feels unnecessary for a method that's only used once in this document type specific partial. This code could probably be refactored into a more elegant helper method in the future.
This is rather than replicating the web_url method from the ContentItemPresenter in government-frontend that is doing the same thing: https://github.com/alphagov/government-frontend/blob/b3cbb8d7b8ff3ff792aa70c58ca8272a384d1773/app/presenters/content_item_presenter.rb#L70
This method only had one line and was unnecessary: https://github.com/alphagov/government-frontend/blob/b3cbb8d7b8ff3ff792aa70c58ca8272a384d1773/app/presenters/travel_advice_presenter.rb#L97C5-L97C21 The public_updated_at field has been added to the ContentItem model as it feels generic enough to be there, and so that the json object doesn't need to be directly accessed from the view.
This is to make space for the "show" tests
TO DO: Fix title and atom link tests
This is needed for atom feeds, otherwise the content item is not set. Should probably make this a "before_action" in the content item controller.
leenagupte
force-pushed
the
add-travel-advice-pages
branch
from
0ab5675
to
20b8cd0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Why
Trello card
How
Screenshots?