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.

Sign up for GitHub

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

Jump to bottom

Support rendering of service manual document types #2583

Merged
merged 13 commits into from
Feb 8, 2023
Merged

Support rendering of service manual document types #2583

merged 13 commits into from
Feb 8, 2023

Conversation

Tetrino
Copy link
Contributor

@Tetrino Tetrino commented Oct 27, 2022 •
edited by hannako
Loading

Update this application so that it can render the following content types:

  • service_manual_guide
  • service_manual_homepage
  • service_manual_service_standard
  • service_manual_topic
  • service_toolkit

Trello: https://trello.com/c/1gPCngv0/1136-migrate-service-manual-frontend-templates-into-government-frontend-m

This PR is a precursor to the retirement of service manual frontend. It can safely be merged, as all published documents still have service_manual_frontend as their rendering_app at this time.

Next steps:

  1. update service manual publisher to set rendering app as government frontend for new publications
  2. update and republish existing content items.

Outstanding issue: #2679

Pages to compare

service_manual_guide

service_manual_homepage

service_manual_service_standard

service_manual_topic

service_toolkit

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 October 27, 2022 15:57 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from 89b9e37 to b8e770d Compare October 27, 2022 16:00
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 October 27, 2022 16:00 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from b8e770d to b635798 Compare October 27, 2022 16:03
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 October 27, 2022 16:03 Inactive
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 October 28, 2022 13:28 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from 482ff08 to 5c6e3f0 Compare October 31, 2022 11:56
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 October 31, 2022 11:56 Inactive
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 21, 2022 13:53 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from 94b5d3a to 7b360ec Compare November 21, 2022 14:54
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 21, 2022 14:55 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from 7b360ec to c27f072 Compare November 21, 2022 15:34
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 21, 2022 15:34 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from c27f072 to 2ea563e Compare November 21, 2022 15:46
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 21, 2022 15:47 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from 2ea563e to e2d1b40 Compare November 21, 2022 15:52
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 21, 2022 15:53 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from e2d1b40 to a6a4688 Compare November 23, 2022 15:04
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 23, 2022 15:04 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from a6a4688 to 360bda8 Compare November 23, 2022 15:05
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 23, 2022 15:05 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from 360bda8 to c4f45f1 Compare November 23, 2022 15:10
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 23, 2022 15:11 Inactive
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 23, 2022 15:31 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from 1210298 to 042560b Compare November 23, 2022 15:32
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 23, 2022 15:32 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from 042560b to 9857eb2 Compare November 23, 2022 15:42
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 23, 2022 15:42 Inactive
@Tetrino Tetrino force-pushed the migrate-service-manual-frontends branch from 9857eb2 to e6d05bf Compare November 23, 2022 15:48
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 November 23, 2022 15:48 Inactive
@hannako
Consolidate controllers
73d6f54 
- There is some logic needed to support a scoped search from the search
bar on service manual pages. Bring in the route, controller logic and specs
for that.
- A couple of service manual doc types require different slimmer templates.

Other than that, the code in the controllers was the same so we can consolidate.
@hannako
Support custom breadcrumbs for subset of service manual doc types
08b2802 
Service manual guides, topics and service standards have custom breadcrumbs defined
in their presenter classes. Service manual homepage has no breadcrumbs. The setting of
custom breadcrumbs is already supported by setting the var @do_not_show_breadcrumbs.
So follow that pattern here.
@hannako hannako force-pushed the migrate-service-manual-frontends branch from 740c012 to a1769ab Compare January 26, 2023 20:31
@hannako hannako changed the title Migrate Service Manuals from Service Manual Frontend Support rendering of service manual document types Jan 26, 2023
@hannako hannako marked this pull request as ready for review January 26, 2023 20:40
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 January 26, 2023 20:57 Inactive
@hannako hannako mentioned this pull request Jan 27, 2023
Open
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 January 27, 2023 13:00 Inactive
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 January 30, 2023 16:30 Inactive
@chao-xian
Copy link
Contributor

Can we swap out:

Screenshot 2023-01-30 at 5 04 35 pm

with https://components.publishing.service.gov.uk/component-guide/signup_link and delete the email icon?

@hannako
Copy link
Contributor

hannako commented Jan 30, 2023

Can we swap out:

Screenshot 2023-01-30 at 5 04 35 pm

with https://components.publishing.service.gov.uk/component-guide/signup_link and delete the email icon?

I guess we own the app now! Should we check with Monica?

@chao-xian
Copy link
Contributor

@hannako nah, given we're not really changing anything drastic and we're adding in a component. Give it a go and show them if you feel it needs to be checked.

@Tetrino
Copy link
Contributor Author

Tetrino commented Jan 31, 2023

I've taken a good look and nothing further from me - I'm good to approve this after the above is put in, if @chao-xian doesn't have any further notes?

@chao-xian
Copy link
Contributor

Sorry to be a pain, but could we split this commit up? f90b068 maybe per bullet point?

chao-xian
chao-xian approved these changes Feb 2, 2023
View reviewed changes
Copy link
Contributor

@chao-xian chao-xian left a comment

Choose a reason for hiding this comment

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

Great work folks! Just a couple of minor comments (using an existing component but I'm ok for that to be a separate PR) and splitting one of the PRs up (which is a nitpick). Thanks!

hannako and others added 7 commits February 6, 2023 12:56
@hannako @chao-xian
Import presenter and helper specs
7e0d59f 
- Source files: https://github.com/alphagov/service-manual-frontend/tree/main/test
- Update file names and class names.
@chao-xian
Remove examples
2b7a767 
Remove simulate_example_as_first_edition_on_draft_stack method and do
  not import https://github.com/alphagov/service-manual-frontend/blob/main/test/support/draft_stack_examples.rb
@chao-xian
De-dupe code provided by content item presenter
e5ff7aa 
Also refactor.
@hannako @chao-xian
Import integration tests
24da42f 
- https://github.com/alphagov/service-manual-frontend/tree/main/test/integration
@hannako @chao-xian
Fix integration tests
9a73883
@hannako @chao-xian
Support custom phase labels for service manuals
e1bc001 
- import the phase label helper and fix specs
- update application layout to toggle to custom phase label
for service manual doc types
@patrickpatrickpatrick @chao-xian
Add wrapper to phase banner on service manual pages
e05b691
@chao-xian chao-xian force-pushed the migrate-service-manual-frontends branch from 7fc4bab to e05b691 Compare February 6, 2023 13:01
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2583 February 6, 2023 13:01 Inactive
AgaDufrat
AgaDufrat approved these changes Feb 6, 2023
View reviewed changes
Copy link
Contributor

@AgaDufrat AgaDufrat left a comment

Choose a reason for hiding this comment

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

Looks good to me. Left a minor comment.
I couldn't see where the "icons-plus-minus" are used, which makes me wonder if we actually need them.

test/integration/service_manual_guide_test.rb Show resolved Hide resolved
@hannako hannako merged commit a36eb33 into main Feb 8, 2023
@hannako hannako deleted the migrate-service-manual-frontends branch February 8, 2023 12:15
hannako added a commit that referenced this pull request Feb 8, 2023
@hannako
Remove unwanted home breadcrumb from /service-toolkit
6a59a12 
- missing from #2583
hannako added a commit that referenced this pull request Feb 8, 2023
@hannako
Add "Service Manual" to the page title
e02c3e0 
- missing from #2583
@hannako hannako mentioned this pull request Feb 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AgaDufrat AgaDufrat AgaDufrat approved these changes

@chao-xian chao-xian chao-xian approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Tetrino @chao-xian @hannako @AgaDufrat @govuk-ci @patrickpatrickpatrick