From 4e3608ed58f4eafb2b016077f8d9f5152e332b41 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      |  13 ++-
 app/views/layouts/design_system.html.erb   |  17 ++-
 app/views/reports/index.html.erb           | 117 +++++++++++++++------
 test/functional/reports_controller_test.rb |  79 ++++++++++----
 4 files changed, 169 insertions(+), 57 deletions(-)

diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb
index 64c44955f..3abefc452 100644
--- a/app/controllers/reports_controller.rb
+++ b/app/controllers/reports_controller.rb
@@ -38,12 +38,17 @@ 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 @@
 
   <div class="govuk-width-container">
     <main class="govuk-main-wrapper" id="main-content" role="main">
-      <%= yield %>
+
+      <% if yield(:title).present? %>
+        <div class="govuk-grid-row">
+          <div class="govuk-grid-column-two-thirds">
+            <%= render "govuk_publishing_components/components/title", {
+              title: yield(:title),
+              margin_top: 0,
+              margin_bottom: 6,
+            } %>
+          </div>
+        </div>
+      <% end %>
+
+      <div class="govuk-grid-row">
+        <%= yield %>
+      </div>
     </main>
   </div>
 
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 @@
-<div class="page-header">
-  <h1>CSV Reports</h1>
-  <p>These reports are updated every hour.</p>
-</div>
+<% content_for :page_title, "Reports" %>
+<% content_for :title, "CSV Reports" %>
 
-<p>
-  <strong><%= link_to 'All documents for departmental distribution', organisation_content_report_path(format: :csv) %></strong><br />
-  <%= report_last_updated("organisation_content")%>
-</p>
-<p>
-  <strong><%= link_to 'Churn in non-archived editions', edition_churn_report_path(format: :csv) %></strong><br />
-  <%= report_last_updated("edition_churn")%>
-</p>
-<p>
-  <strong><%= link_to 'Churn in all editions', all_edition_churn_report_path(format: :csv) %></strong><br />
-  <%= report_last_updated("all_edition_churn")%>
-</p>
-<p>
-  <strong><%= link_to 'Progress on all non-archived editions', progress_report_path(format: :csv) %></strong><br />
-  <%= report_last_updated("editorial_progress")%>
-</p>
-<p>
-  <strong><%= link_to 'Content summary and workflow history for all published editions', content_workflow_report_path(format: :csv) %></strong><br />
-  <%= report_last_updated("content_workflow")%>
-</p>
-<p>
-  <strong><%= link_to 'Content summary and workflow history for all editions', all_content_workflow_report_path(format: :csv) %></strong><br />
-  <%= report_last_updated("all_content_workflow")%>
-</p>
-<p>
-  <strong><%= link_to 'All URLs', all_urls_report_path(format: :csv) %></strong><br />
-  <%= report_last_updated("all_urls")%>
-</p>
+<div class="govuk-grid-column-two-thirds">
+  <%= 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"),
+        }
+      },
+    ]
+  } %>
+
+</div>
diff --git a/test/functional/reports_controller_test.rb b/test/functional/reports_controller_test.rb
index 1bbac2d4f..266a7db4e 100644
--- a/test/functional/reports_controller_test.rb
+++ b/test/functional/reports_controller_test.rb
@@ -1,33 +1,74 @@
 require "test_helper"
 
 class ReportsControllerTest < ActionController::TestCase
-  setup do
-    login_as_stub_user
+  context "When reports are available" do
+    setup do
+      login_as_stub_user
 
-    last_modified = Time.zone.local(2023, 12, 12, 1, 1, 1)
+      last_modified = Time.zone.local(2023, 12, 12, 1, 2, 3)
 
-    Aws.config[:s3] = {
-      stub_responses: {
-        head_object: { last_modified: },
-      },
-    }
+      Aws.config[:s3] = {
+        stub_responses: {
+          head_object: { last_modified: },
+        },
+      }
 
-    ENV["REPORTS_S3_BUCKET_NAME"] = "example"
-  end
+      ENV["REPORTS_S3_BUCKET_NAME"] = "example"
+    end
 
-  teardown do
-    ENV["REPORTS_S3_BUCKET_NAME"] = nil
-  end
+    teardown do
+      ENV["REPORTS_S3_BUCKET_NAME"] = nil
+    end
+
+    should "redirect the user to S3 when following report links" do
+      %i[
+        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