Render Get Involved page on Government Frontend #1939

Tetrino · 2020-12-11T10:29:05Z

The summation of a bunch of work to migrate the Get Involved static page from Whitehall to Government Frontend.

This took more time than it really should of, in part due to modernising elements in Whitehall that we do not support in Government Frontend, and in part due to other unrelated concerns.

Important that while this repo is continuously deployed, this work will not be visible until enabled by a related rake task. With this in mind it is safe to deploy.

Testing is being handled primarily through an upcoming Smokey test, as the actual search-api calls are not ideal for a contract test nor can they be relied on for consistent unit testing.

https://trello.com/c/EZt2QYbz/2717-migrate-rendering-of-government-get-involved-from-whitehall-frontend-to-government-frontend

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

ChrisBAshton

First of all, congrats on getting through this very chunky piece of work - I know it wasn't easy!

I have no major concerns about the overall approach in the code, but it does seem to be missing quite a few tests which ought to be added before merging. The commit history is also quite confusing to follow. I've made some suggestions below, hopefully it's not too painful to rebase!

app/controllers/get_involved_controller.rb

app/assets/stylesheets/views/_get-involved.scss

app/views/content_items/get_involved.html.erb

app/controllers/content_items_controller.rb

app/assets/stylesheets/application.scss

app/controllers/get_involved_controller.rb

brucebolt

Thanks for this @Tetrino. It's great to see such a substantial piece of work come together nicely.

I've done a quick first pass review and left some comments, that shouldn't be too much effort to resolve.

I can't say I'm content with reviewing the frontend code as there's quite a lot of moving parts, so maybe a frontend developer could be sought to look over this?

app/views/content_items/get_involved.html.erb

app/controllers/get_involved_controller.rb

test/controllers/get_involved_controller_test.rb

app/helpers/get_involved_helper.rb

app/assets/stylesheets/helpers/_featured_news.scss

app/controllers/get_involved_controller.rb

app/lib/services.rb

app/views/content_items/get_involved.html.erb

app/helpers/get_involved_helper.rb

brucebolt · 2021-08-05T09:19:43Z

Important that while this repo is continuously deployed, this work will not be visible until enabled by a related rake task. With this in mind it is safe to deploy.

Have you got some instructions for running this locally in govuk-docker? The content item contains the existing Whitehall version of the page, so I think I'll need to update something in this content item to get this page to render in government-frontend.

brucebolt

This is looking great @Tetrino.

I've done a side-by-side comparison of the page rendered by Whitehall and Government Frontend, which identified a few minor things. They should all be easy to change.

app/views/content_items/get_involved.html.erb

app/controllers/get_involved_controller.rb

app/views/content_items/get_involved.html.erb

config/locales/yi.yml

Tetrino · 2021-10-28T14:32:32Z

Also fixed, but I lost the comment @brucebolt, was the lack of rel attribute and class name here:

government-frontend/app/views/content_items/get_involved.html.erb

Line 95 in 0ffe74e

    
           create_a_petition: link_to(t('get_involved.petition_paragraph.create_a_petition', class: "govuk-link", rel: "external"), "https://petition.parliament.uk/")) %>

This commit is focused on moving the required assets from Whitehall to government frontend with little in the way of changes to make them compatible. Controllers and helpers from Whitehall were left behind as they are not useful in the GF framework.

The core logic of the controller is a migration of a set of function calls in whitehall/app/controllers/home_controller.rb for populating the Get Involved page with relevant data. Main bulk of controlling calls from Whitehall were taken from here and applied into this new controller: https://github.com/alphagov/whitehall/blob/bcda05ec43d87b3bec1541e644c1373bafa2a3e8/app/controllers/home_controller.rb#L17-L25 In this case the function calls are replaced with calls to the publishing-api and respective filtering of those results to give us what we need in Government Frontend.

brucebolt

Great work and thank you for persevering with all the review comments, @Tetrino.

This looks good to be merged now.

ChrisBAshton

Nice one, Jon 👍 happy for this to be merged. I did leave a few comments that I ended up deleting as they were resolved in later commits, and I appreciate that a PR like this would be difficult and risky to unpick in a rebase.

Nothing blocking below, though would be nice to see a couple of tests for the helper (which can be added in a new commit). I'll leave it up to you.

ChrisBAshton · 2021-10-29T08:50:06Z

app/helpers/get_involved_helper.rb

@@ -0,0 +1,34 @@
+module GetInvolvedHelper
+  def date_microformat(attribute_name)
+    attribute_name.to_date.strftime("%d %B %Y")


I think this helper is missing a test file. Don't need to test every method (e.g. page_title is fine!) but probably worth testing the method above, as well as time_until_closure (which I see no reason to be kept private).

config/routes.rb

app/controllers/get_involved_controller.rb

config/locales/en.yml

The helper in this case handles a selection of time to string filtering/ labelling and a single routing function to allow us to link to the search page for a given set of consultations (feature match to Whitehall original version). The presenter is empty and is included for consistency with other content.

When Get Involved existed in Whitehall it relied on an outdated set of styling frameworks that we no longer support. Of course, none of these were present in Government Frontend, so a large chunk of the related work was in modernising this style system. Also included in the Get Involved page specifically are calls to the correct functions in the controller such that we have the needed data to display.

Previously we were retrieving all data for the get involved page from the Publishing API. This was particularly heavy on our system with extra unneeded pings that were not correctly cached. Moving to the Search API where possible allows us to use cached results and saves a lot of overhead.

The Frontend team removed many functions being used in the original port of this page from Whitehall. This commit updates the page to fit the component ecosystem from govuk_publishing_components with the added benefit of losing a lot of now unnecessary CSS content for the same end result.

Tetrino force-pushed the render-getinvolved-frontend branch 2 times, most recently from f5592c1 to f8ff4f9 Compare December 11, 2020 11:16

bevanloon temporarily deployed to government-f-render-get-hspyn6 December 11, 2020 12:56 Inactive

Tetrino force-pushed the render-getinvolved-frontend branch from 091a0b7 to f8ff4f9 Compare December 14, 2020 07:04

bevanloon temporarily deployed to government-f-render-get-hspyn6 December 14, 2020 07:04 Inactive

bevanloon temporarily deployed to government-f-render-get-hspyn6 December 15, 2020 15:46 Inactive

Tetrino force-pushed the render-getinvolved-frontend branch from facb3c2 to cbe2ae1 Compare January 4, 2021 08:07

ChrisBAshton reviewed Jan 4, 2021

View reviewed changes

thomasleese reviewed Jan 6, 2021

View reviewed changes

app/controllers/get_involved_controller.rb Outdated Show resolved Hide resolved

Tetrino force-pushed the render-getinvolved-frontend branch from cbe2ae1 to 9f7bb72 Compare January 12, 2021 15:38

Tetrino force-pushed the render-getinvolved-frontend branch 4 times, most recently from b2b53bf to 826309a Compare March 29, 2021 15:11

Tetrino force-pushed the render-getinvolved-frontend branch 5 times, most recently from 0dfe35e to 6ffee78 Compare May 10, 2021 12:50

Tetrino force-pushed the render-getinvolved-frontend branch 2 times, most recently from 3ed7333 to 9c8f23b Compare July 5, 2021 14:37

Tetrino marked this pull request as ready for review July 5, 2021 14:37

Tetrino force-pushed the render-getinvolved-frontend branch from 9c8f23b to 808dc5b Compare July 5, 2021 14:40

Tetrino force-pushed the render-getinvolved-frontend branch from 808dc5b to 3c84fde Compare July 20, 2021 10:04

Tetrino force-pushed the render-getinvolved-frontend branch 2 times, most recently from 0d63dcb to 06ef82e Compare July 29, 2021 11:49

Tetrino requested a review from huwd July 29, 2021 11:58

Tetrino force-pushed the render-getinvolved-frontend branch from 06ef82e to 0567be7 Compare July 29, 2021 13:21

brucebolt reviewed Jul 29, 2021

View reviewed changes

Tetrino force-pushed the render-getinvolved-frontend branch 4 times, most recently from f5a7135 to 7b0cda1 Compare October 4, 2021 08:47

Tetrino force-pushed the render-getinvolved-frontend branch 2 times, most recently from b53225f to 0e2e3aa Compare October 15, 2021 09:26

Tetrino mentioned this pull request Oct 15, 2021

Add big number component to the get involved page alphagov/whitehall#6329

Closed

Tetrino force-pushed the render-getinvolved-frontend branch 3 times, most recently from 6ddd460 to 6c0c2f6 Compare October 18, 2021 15:09

Tetrino force-pushed the render-getinvolved-frontend branch from 6c0c2f6 to 6cff742 Compare October 27, 2021 11:03

brucebolt reviewed Oct 27, 2021

View reviewed changes

Tetrino force-pushed the render-getinvolved-frontend branch 3 times, most recently from 8d68a10 to 0ffe74e Compare October 28, 2021 14:22

Tetrino force-pushed the render-getinvolved-frontend branch 2 times, most recently from a574c7d to 1dab0a3 Compare October 28, 2021 14:36

Tetrino added 2 commits October 28, 2021 15:36

brucebolt approved these changes Oct 28, 2021

View reviewed changes

ChrisBAshton approved these changes Oct 29, 2021

View reviewed changes

Tetrino force-pushed the render-getinvolved-frontend branch from 1dab0a3 to 8dba7fc Compare October 29, 2021 10:01

Tetrino added 5 commits October 29, 2021 11:01

Add i18n support to Get Involved page

cc03e8f

Tetrino merged commit ee97631 into main Oct 29, 2021

Tetrino deleted the render-getinvolved-frontend branch October 29, 2021 10:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Render Get Involved page on Government Frontend #1939

Render Get Involved page on Government Frontend #1939

Tetrino commented Dec 11, 2020 •

edited

Loading

ChrisBAshton left a comment

brucebolt left a comment

brucebolt commented Aug 5, 2021

brucebolt left a comment

Tetrino commented Oct 28, 2021

brucebolt left a comment

ChrisBAshton left a comment

ChrisBAshton Oct 29, 2021

Render Get Involved page on Government Frontend #1939

Render Get Involved page on Government Frontend #1939

Conversation

Tetrino commented Dec 11, 2020 • edited Loading

ChrisBAshton left a comment

Choose a reason for hiding this comment

brucebolt left a comment

Choose a reason for hiding this comment

brucebolt commented Aug 5, 2021

brucebolt left a comment

Choose a reason for hiding this comment

Tetrino commented Oct 28, 2021

brucebolt left a comment

Choose a reason for hiding this comment

ChrisBAshton left a comment

Choose a reason for hiding this comment

ChrisBAshton Oct 29, 2021

Choose a reason for hiding this comment

Tetrino commented Dec 11, 2020 •

edited

Loading