From a9b4f1220b77b6f0897f4747c2c5456ba317690f Mon Sep 17 00:00:00 2001
From: Mark Taylor <138604938+mtaylorgds@users.noreply.github.com>
Date: Fri, 26 Jan 2024 14:32:53 +0000
Subject: [PATCH] Migrate the Reports page to GOV.UK Design System
- Adds use of the
[Page Title](https://components.publishing.service.gov.uk/component-guide/title)
component to `app/views/layouts/design_system.html.erb` to ensure
transitioned pages use consistent formatting for the page title.
- Updates the reports index view to use the
[Document List](https://components.publishing.service.gov.uk/component-guide/document_list)
component.
- Improves the test coverage around the reports controller.
Since the transitioned design splits the generated message into two
separate pieces of metadata (one for the time and one for the date),
there will now be twice the calls to S3 to fetch that data. This could
be optimised if it becomes a problem, but this page is not used much so
that is left as a potential future optimisation.
---
app/controllers/reports_controller.rb | 14 ++-
app/views/layouts/design_system.html.erb | 17 ++-
app/views/reports/index.html.erb | 117 +++++++++++++++------
test/functional/reports_controller_test.rb | 80 ++++++++++----
4 files changed, 171 insertions(+), 57 deletions(-)
diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb
index 64c44955f..3e93c8679 100644
--- a/app/controllers/reports_controller.rb
+++ b/app/controllers/reports_controller.rb
@@ -38,12 +38,18 @@ def all_urls
private
def report_last_updated(report_name)
- last_updated = ::Report.new(report_name).last_updated
+ ::Report.new(report_name).last_updated
+ end
+ helper_method :report_last_updated
+
+ def report_generated_time_message(report_name)
+ last_updated = report_last_updated(report_name)
if last_updated
- tag.span "Generated #{last_updated.to_fs(:govuk_date)}", class: "text-muted"
+ "Generated #{last_updated.strftime('%-l:%M%#p')}".strip
else
- tag.span "Report currently unavailable", class: "text-muted"
+ "Report currently unavailable"
end
end
- helper_method :report_last_updated
+ helper_method :report_generated_time_message
+
end
diff --git a/app/views/layouts/design_system.html.erb b/app/views/layouts/design_system.html.erb
index 918c46d57..782f2453d 100644
--- a/app/views/layouts/design_system.html.erb
+++ b/app/views/layouts/design_system.html.erb
@@ -18,7 +18,22 @@
- <%= yield %>
+
+ <% if yield(:title).present? %>
+
+
+ <%= render "govuk_publishing_components/components/title", {
+ title: yield(:title),
+ margin_top: 0,
+ margin_bottom: 6,
+ } %>
+
+
+ <% end %>
+
+
+ <%= yield %>
+
diff --git a/app/views/reports/index.html.erb b/app/views/reports/index.html.erb
index d54f206f0..356ac6c94 100644
--- a/app/views/reports/index.html.erb
+++ b/app/views/reports/index.html.erb
@@ -1,35 +1,86 @@
-
+<% content_for :page_title, "Reports" %>
+<% content_for :title, "CSV Reports" %>
-
- <%= link_to 'All documents for departmental distribution', organisation_content_report_path(format: :csv) %>
- <%= report_last_updated("organisation_content")%>
-
-
- <%= link_to 'Churn in non-archived editions', edition_churn_report_path(format: :csv) %>
- <%= report_last_updated("edition_churn")%>
-
-
- <%= link_to 'Churn in all editions', all_edition_churn_report_path(format: :csv) %>
- <%= report_last_updated("all_edition_churn")%>
-
-
- <%= link_to 'Progress on all non-archived editions', progress_report_path(format: :csv) %>
- <%= report_last_updated("editorial_progress")%>
-
-
- <%= link_to 'Content summary and workflow history for all published editions', content_workflow_report_path(format: :csv) %>
- <%= report_last_updated("content_workflow")%>
-
-
- <%= link_to 'Content summary and workflow history for all editions', all_content_workflow_report_path(format: :csv) %>
- <%= report_last_updated("all_content_workflow")%>
-
-
- <%= link_to 'All URLs', all_urls_report_path(format: :csv) %>
- <%= report_last_updated("all_urls")%>
-
+
+ <%= render "govuk_publishing_components/components/lead_paragraph", {
+ text: "These reports are updated every hour.",
+ margin_bottom: 6
+ } %>
-<% content_for :page_title, "Reports" %>
+ <%= render "govuk_publishing_components/components/document_list", {
+ margin_bottom: 6,
+ items: [
+ {
+ link: {
+ text: "All documents for departmental distribution",
+ path: organisation_content_report_path(format: :csv)
+ },
+ metadata: {
+ updated_at_time: report_generated_time_message("organisation_content"),
+ public_updated_at: report_last_updated("organisation_content"),
+ }
+ },
+ {
+ link: {
+ text: "Churn in non-archived editions",
+ path: edition_churn_report_path(format: :csv)
+ },
+ metadata: {
+ updated_at_time: report_generated_time_message("edition_churn"),
+ public_updated_at: report_last_updated("edition_churn"),
+ }
+ },
+ {
+ link: {
+ text: "Churn in all editions",
+ path: all_edition_churn_report_path(format: :csv)
+ },
+ metadata: {
+ updated_at_time: report_generated_time_message("all_edition_churn"),
+ public_updated_at: report_last_updated("all_edition_churn"),
+ }
+ },
+ {
+ link: {
+ text: "Progress on all non-archived editions",
+ path: progress_report_path(format: :csv)
+ },
+ metadata: {
+ updated_at_time: report_generated_time_message("editorial_progress"),
+ public_updated_at: report_last_updated("editorial_progress"),
+ }
+ },
+ {
+ link: {
+ text: "Content summary and workflow history for all published editions",
+ path: content_workflow_report_path(format: :csv)
+ },
+ metadata: {
+ updated_at_time: report_generated_time_message("content_workflow"),
+ public_updated_at: report_last_updated("content_workflow"),
+ }
+ },
+ {
+ link: {
+ text: "Content summary and workflow history for all editions",
+ path: all_content_workflow_report_path(format: :csv)
+ },
+ metadata: {
+ updated_at_time: report_generated_time_message("all_content_workflow"),
+ public_updated_at: report_last_updated("all_content_workflow"),
+ }
+ },
+ {
+ link: {
+ text: "All URLs",
+ path: all_urls_report_path(format: :csv)
+ },
+ metadata: {
+ updated_at_time: report_generated_time_message("all_urls"),
+ public_updated_at: report_last_updated("all_urls"),
+ }
+ },
+ ]
+ } %>
+
+
diff --git a/test/functional/reports_controller_test.rb b/test/functional/reports_controller_test.rb
index 1bbac2d4f..b7e1a3f06 100644
--- a/test/functional/reports_controller_test.rb
+++ b/test/functional/reports_controller_test.rb
@@ -1,33 +1,75 @@
require "test_helper"
class ReportsControllerTest < ActionController::TestCase
- setup do
- login_as_stub_user
- last_modified = Time.zone.local(2023, 12, 12, 1, 1, 1)
+ context "When reports are available" do
+ setup do
+ login_as_stub_user
- Aws.config[:s3] = {
- stub_responses: {
- head_object: { last_modified: },
- },
- }
+ last_modified = Time.zone.local(2023, 12, 12, 1, 2, 3)
- ENV["REPORTS_S3_BUCKET_NAME"] = "example"
- end
+ Aws.config[:s3] = {
+ stub_responses: {
+ head_object: { last_modified: },
+ },
+ }
- teardown do
- ENV["REPORTS_S3_BUCKET_NAME"] = nil
- end
+ ENV["REPORTS_S3_BUCKET_NAME"] = "example"
+ end
+
+ teardown do
+ ENV["REPORTS_S3_BUCKET_NAME"] = nil
+ end
+
+ should "redirect the user to S3 when following report links" do
+ [
+ :progress,
+ :organisation_content,
+ :edition_churn,
+ :all_edition_churn,
+ :content_workflow,
+ :all_content_workflow,
+ :all_urls,
+ ].each do |action|
+ get action
+
+ assert_equal 302, response.status
+ end
+ end
- test "it redirects the user to S3" do
- get :progress
+ should "show the last updated time on the index page" do
+ get :index
- assert_equal 302, response.status
+ assert_select "ul.gem-c-document-list__item-metadata" do
+ assert_select "li.gem-c-document-list__attribute", { count: 7, text: "Generated 1:02am" }
+ assert_select "li.gem-c-document-list__attribute", { count: 7, text: "12 December 2023" }
+ end
+ end
end
- test "shows the last updated time on the index page" do
- get :index
+ context "When reports are not available" do
+ setup do
+ login_as_stub_user
+
+ Aws.config[:s3] = {
+ stub_responses: {
+ head_object: 'NotFound',
+ },
+ }
+
+ ENV["REPORTS_S3_BUCKET_NAME"] = "example"
+ end
+
+ teardown do
+ ENV["REPORTS_S3_BUCKET_NAME"] = nil
+ end
+
+ should "indicate that reports are not available on the index page" do
+ get :index
- assert_match(/Generated 1:01am, 12 December 2023/, response.body)
+ assert_select "ul.gem-c-document-list__item-metadata" do
+ assert_select "li.gem-c-document-list__attribute", { count: 7, text: "Report currently unavailable" }
+ end
+ end
end
end