From 56417b1f1f2b85e88e571fd71c0750057ec51086 Mon Sep 17 00:00:00 2001
From: Mark Taylor <138604938+mtaylorgds@users.noreply.github.com>
Date: Wed, 6 Sep 2023 08:37:51 +0100
Subject: [PATCH 1/5] Rename link_check_reports to legacy_link_check_reports
In preparation for migration to the design system, where we will migrate
a page at a time to use a link checker that uses design system
components.
---
...ller.rb => legacy_link_check_reports_controller.rb} | 6 +++---
.../_form.html.erb | 2 +-
.../_link_check_report.html.erb | 8 ++++----
.../admin/legacy_link_check_reports/create.js.erb | 1 +
app/views/admin/legacy_link_check_reports/show.js.erb | 3 +++
app/views/admin/link_check_reports/create.js.erb | 1 -
app/views/admin/link_check_reports/show.js.erb | 3 ---
app/views/manuals/show.html.erb | 2 +-
app/views/sections/show.html.erb | 2 +-
config/routes.rb | 2 +-
...rb => legacy_link_check_reports_controller_spec.rb} | 10 +++++-----
11 files changed, 20 insertions(+), 20 deletions(-)
rename app/controllers/{link_check_reports_controller.rb => legacy_link_check_reports_controller.rb} (89%)
rename app/views/admin/{link_check_reports => legacy_link_check_reports}/_form.html.erb (52%)
rename app/views/admin/{link_check_reports => legacy_link_check_reports}/_link_check_report.html.erb (81%)
create mode 100644 app/views/admin/legacy_link_check_reports/create.js.erb
create mode 100644 app/views/admin/legacy_link_check_reports/show.js.erb
delete mode 100644 app/views/admin/link_check_reports/create.js.erb
delete mode 100644 app/views/admin/link_check_reports/show.js.erb
rename spec/controllers/{link_check_reports_controller_spec.rb => legacy_link_check_reports_controller_spec.rb} (91%)
diff --git a/app/controllers/link_check_reports_controller.rb b/app/controllers/legacy_link_check_reports_controller.rb
similarity index 89%
rename from app/controllers/link_check_reports_controller.rb
rename to app/controllers/legacy_link_check_reports_controller.rb
index 7ebfde520..03b41a900 100644
--- a/app/controllers/link_check_reports_controller.rb
+++ b/app/controllers/legacy_link_check_reports_controller.rb
@@ -1,4 +1,4 @@
-class LinkCheckReportsController < ApplicationController
+class LegacyLinkCheckReportsController < ApplicationController
def create
service = LinkCheckReport::CreateService.new(
user: current_user,
@@ -13,7 +13,7 @@ def create
@reportable = reportable_hash
respond_to do |format|
- format.js { render "admin/link_check_reports/create" }
+ format.js { render "admin/legacy_link_check_reports/create" }
format.html { redirect_to_reportable_path }
end
end
@@ -26,7 +26,7 @@ def show
@reportable = reportable_hash
respond_to do |format|
- format.js { render "admin/link_check_reports/show" }
+ format.js { render "admin/legacy_link_check_reports/show" }
format.html { redirect_to_reportable_path }
end
end
diff --git a/app/views/admin/link_check_reports/_form.html.erb b/app/views/admin/legacy_link_check_reports/_form.html.erb
similarity index 52%
rename from app/views/admin/link_check_reports/_form.html.erb
rename to app/views/admin/legacy_link_check_reports/_form.html.erb
index 557c47571..969c5e987 100644
--- a/app/views/admin/link_check_reports/_form.html.erb
+++ b/app/views/admin/legacy_link_check_reports/_form.html.erb
@@ -1,3 +1,3 @@
-<%= form_tag link_check_reports_path(link_reportable: reportable), remote: true do %>
+<%= form_tag legacy_link_check_reports_path(link_reportable: reportable), remote: true do %>
<%= submit_tag button_text, class: "btn btn-default add-top-margin remove-bottom-margin" %>
<% end %>
diff --git a/app/views/admin/link_check_reports/_link_check_report.html.erb b/app/views/admin/legacy_link_check_reports/_link_check_report.html.erb
similarity index 81%
rename from app/views/admin/link_check_reports/_link_check_report.html.erb
rename to app/views/admin/legacy_link_check_reports/_link_check_report.html.erb
index 095a37cb6..11a8a491a 100644
--- a/app/views/admin/link_check_reports/_link_check_report.html.erb
+++ b/app/views/admin/legacy_link_check_reports/_link_check_report.html.erb
@@ -2,13 +2,13 @@
<% if !report.present? %>
Check this document for broken links. The report will take a few moments to complete. This document contains no broken links.
Please wait.
<%= link_to "Refresh",
- link_check_report_path(report.id),
+ legacy_link_check_report_path(report.id),
class: 'js-broken-links-refresh js-hidden',
remote: true %>
Check this document for broken links. The report will take a few moments to complete.
+ <%= render 'admin/link_check_reports/form', reportable: reportable, button_text: 'Check for broken links' %> +<%= t "broken_links.#{status}.subheading" %>
+<%= link.problem_summary %>
+ <% if link.suggested_fix %> +Suggested fix: <%= link.suggested_fix %>
+ <% end %> +This document contains no broken links.
+ <%= render 'admin/link_check_reports/form', reportable: reportable, button_text: 'Check again' %> +Check this document for broken links. The report will take a few moments to complete.
+Check this document for broken links. The report will take a few moments to complete.
<%= render 'admin/link_check_reports/form', reportable: reportable, button_text: 'Check for broken links' %>Broken link report in progress.
+Please wait. <%= link_to "Refresh", link_check_report_path(report.id), class: 'js-broken-links-refresh js-hidden', - remote: true %> + remote: true %>
<%= t "broken_links.#{status}.subheading" %>
-<%= t "broken_links.#{status}.subheading" %>
+<%= link.problem_summary %>
+ <%= link_to link.uri.truncate(50), link.uri, title: link.uri, class: 'govuk-link' %> + <%= render "govuk_publishing_components/components/details", { + title: "See more details about this link" + } do %> +<%= link.problem_summary %>
<% if link.suggested_fix %> -Suggested fix: <%= link.suggested_fix %>
- <% end %> -Suggested fix: <%= link.suggested_fix %>
+ <% end %> + <% end %>This document contains no broken links.
+This document contains no broken links.
<%= render 'admin/link_check_reports/form', reportable: reportable, button_text: 'Check again' %>Warning: There are duplicate section slugs in this manual.
-<%= manual.summary %>
-<%= manual.body %>-
+ <%= link_to "Preview on website (opens in new tab)", content_preview_url(manual), target: "_blank" %>
+This is a new draft of a document that has already been published.
++ <%= link_to "Go to published edition", url_for_public_manual(manual), target: "_blank", class: "govuk-link" %>
+a manual body
" }, + ) + end + + it "does not return a row for the body when it is empty" do + manual = FactoryBot.build_stubbed(:manual, body: "") + + rows = manual_front_page_rows(manual) + + expect(rows).to_not include(include(key: "Body")) + end + end + + describe "#manual_sidebar_action_items" do + before do + allow_any_instance_of(ApplicationHelper).to receive(:allow_publish?).and_return(true) + end + + it "returns a 'publish' button when the manual is allowed to be published" do + manual = FactoryBot.build_stubbed(:manual) + confirm_publish_path = "/manual/blah/confirm_publish" + allow_any_instance_of(ApplicationHelper).to receive(:allow_publish?).and_return(true) + allow_any_instance_of(ApplicationHelper).to receive(:confirm_publish_manual_path).and_return(confirm_publish_path) + + items = manual_sidebar_action_items(manual, true) + + expect(items).to include(include("Publish")) + expect(items).to include(include(confirm_publish_path)) + end + + it "does not return a 'publish' button when the manual is not allowed to be published" do + manual = FactoryBot.build_stubbed(:manual) + allow_any_instance_of(ApplicationHelper).to receive(:allow_publish?).and_return(false) + + items = manual_sidebar_action_items(manual, true) + + expect(items).not_to include(include("Publish")) + end + + it "returns a 'discard' button when the manual has never been published" do + manual = FactoryBot.build_stubbed(:manual, ever_been_published: false) + + items = manual_sidebar_action_items(manual, true) + + expect(items).to include(include("Discard")) + end + + it "does not return a 'discard' button when the manual has been published" do + manual = FactoryBot.build_stubbed(:manual, ever_been_published: true) + + items = manual_sidebar_action_items(manual, true) + + expect(items).not_to include(include("Discard")) + end + end + + describe "#manual_section_rows" do + before do + allow_any_instance_of(ApplicationHelper).to receive(:last_updated_text).and_return("") + end + + it "returns a row for each section in the manual" do + manual = FactoryBot.build_stubbed(:manual) + manual.build_section({}) + manual.build_section({}) + + rows = manual_section_rows(manual) + + expect(rows.length).to be(2) + end + + it "adds a 'DRAFT' tag to the key when the section is in the 'draft' state" do + manual = FactoryBot.build_stubbed(:manual) + manual.build_section({ state: "draft", title: "test title" }) + + rows = manual_section_rows(manual) + + expect(rows).to include(include(key: include(">DRAFT"))) + expect(rows).to include(include(key: include(">test title"))) + end + + it "does not add a 'DRAFT' tag to the key when the section is in the 'published' state" do + manual = FactoryBot.build_stubbed(:manual) + manual.build_section({ state: "published", title: "test title" }) + + rows = manual_section_rows(manual) + + expect(rows).not_to include(include(key: include(">DRAFT"))) + expect(rows).to include(include(key: include(">test title"))) + end + + it "uses the section's title for the key when the section is in the 'published state" do + manual = FactoryBot.build_stubbed(:manual) + manual.build_section({ state: "published", title: "test title" }) + + rows = manual_section_rows(manual) + + expect(rows).to include(include(key: "test title")) + end + end end diff --git a/spec/views/admin/link_check_reports/_form.html.erb_spec.rb b/spec/views/admin/link_check_reports/_form.html.erb_spec.rb new file mode 100644 index 000000000..c6c584147 --- /dev/null +++ b/spec/views/admin/link_check_reports/_form.html.erb_spec.rb @@ -0,0 +1,12 @@ +require "spec_helper" + +describe "admin/link_check_reports/_form", type: :view do + it "renders a button with the specified text" do + allow(view).to receive(:reportable).and_return({}) + allow(view).to receive(:button_text).and_return("Do the thing") + + render + + expect(rendered).to have_css("button", exact_text: "Do the thing") + end +end diff --git a/spec/views/manuals/show.html.erb_spec.rb b/spec/views/manuals/show.html.erb_spec.rb index c35a8c597..9923e41c5 100644 --- a/spec/views/manuals/show.html.erb_spec.rb +++ b/spec/views/manuals/show.html.erb_spec.rb @@ -23,4 +23,25 @@ expect(rendered).to match(/Discard draft/) end + + [ + [:notice, ".govuk-notification-banner .govuk-notification-banner__content"], + [:success, ".govuk-notification-banner .govuk-notification-banner__content"], + [:error, ".gem-c-error-alert .gem-c-error-alert__message"], + ].each do |flash_type, msg_css_selector| + it "renders a #{flash_type} flash message" do + manual = FactoryBot.build_stubbed(:manual) + manual.publish_tasks = [] + msg = "A flash message" + flash[flash_type] = msg + allow(view).to receive(:flash).and_return(flash) + allow(view).to receive(:current_user).and_return(FactoryBot.build_stubbed(:user)) + + render template: "manuals/show", + layout: "layouts/design_system", + locals: { manual:, slug_unique: true, clashing_sections: [] } + + expect(rendered).to have_css(msg_css_selector, text: msg) + end + end end From a6d412ca754b0d0accee089d8d59406d8fad1c95 Mon Sep 17 00:00:00 2001 From: Mark Taylor <138604938+mtaylorgds@users.noreply.github.com> Date: Thu, 7 Sep 2023 09:25:24 +0100 Subject: [PATCH 4/5] Replace heading tag elements Replace HTML tag elements with use of the `heading` component from the design system. Visually they should appear the same. --- app/assets/stylesheets/application.scss | 1 + app/views/manuals/show.html.erb | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index d7ec13d2c..6e5f0ed9f 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -8,6 +8,7 @@ $govuk-page-width: 1140px; @import "govuk_publishing_components/components/details"; @import "govuk_publishing_components/components/error-alert"; @import "govuk_publishing_components/components/error-message"; +@import "govuk_publishing_components/components/heading"; @import "govuk_publishing_components/components/hint"; @import "govuk_publishing_components/components/input"; @import "govuk_publishing_components/components/label"; diff --git a/app/views/manuals/show.html.erb b/app/views/manuals/show.html.erb index a4197d275..25ed621c9 100644 --- a/app/views/manuals/show.html.erb +++ b/app/views/manuals/show.html.erb @@ -18,7 +18,12 @@<%= link_to "Preview on website (opens in new tab)", content_preview_url(manual), target: "_blank" %>