From f882e3f09ba32cbf70f49c2226d13168febadb9f Mon Sep 17 00:00:00 2001 From: Anshul Sirur Date: Fri, 11 Oct 2019 14:28:17 +0100 Subject: [PATCH 1/2] Allow overriding tech-docs config in spec tests I've changed the `rebuild_site!` function to take an `overrides` argument which is a hash of options that will be added to the tech-docs config. Now you don't need to copy/paste the entire YAML config just to change a single option! --- example/config/hide-expiry.yml | 51 --------------------------- spec/features/page_expiration_spec.rb | 2 +- spec/spec_helper.rb | 29 ++++++++++----- 3 files changed, 21 insertions(+), 61 deletions(-) delete mode 100644 example/config/hide-expiry.yml diff --git a/example/config/hide-expiry.yml b/example/config/hide-expiry.yml deleted file mode 100644 index fd8e0f10..00000000 --- a/example/config/hide-expiry.yml +++ /dev/null @@ -1,51 +0,0 @@ -# Host to use for canonical URL generation (without trailing slash) -host: https://docs.example.com - -# Header-related options -show_govuk_logo: true -service_name: My First Service -service_link: / -phase: Beta - -# Links to show on right-hand-side of header -header_links: - Documentation: / - Expired page: /expired-page.html - Expired with owner: /expired-page-with-owner.html - Not expired page: /not-expired-page.html - -# Tracking ID from Google Analytics (e.g. UA-XXXX-Y) -ga_tracking_id: - -# Enable multipage navigation in the sidebar -multipage_nav: true - -# Enable collapsible navigation in the sidebar -collapsible_nav: true - -# Table of contents depth – how many levels to include in the table of contents. -# If your ToC is too long, reduce this number and we'll only show higher-level -# headings. -max_toc_heading_level: 6 - -# Prevent robots from indexing (e.g. whilst in development) -prevent_indexing: false - -google_site_verification: dstbao8TVS^DRVDS&rv76 - -enable_search: true - -show_contribution_banner: true - -github_repo: alphagov/example-repo -github_branch: source - -redirects: - /something/old.html: /index.html - -api_path: source/pets.yml - -# Optional global settings for the page review process -owner_slack_workspace: gds -default_owner_slack: '#2nd-line' -show_expiry: false diff --git a/spec/features/page_expiration_spec.rb b/spec/features/page_expiration_spec.rb index 84daa729..5859cc56 100644 --- a/spec/features/page_expiration_spec.rb +++ b/spec/features/page_expiration_spec.rb @@ -25,7 +25,7 @@ def when_the_site_is_created end def when_the_site_is_created_hiding_expiry - rebuild_site!(config: "config/hide-expiry.yml") + rebuild_site!(overrides: { 'show_expiry' => false }) end def and_i_visit_an_expired_page diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7da99321..08c9489d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,8 @@ $LOAD_PATH.unshift(File.expand_path('../lib', __dir__)) require 'govuk_tech_docs' +require 'tempfile' +require 'yaml' # This file was generated by the `rspec --init` command. Conventionally, all # specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`. @@ -21,16 +23,25 @@ # # See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration RSpec.configure do |config| - def rebuild_site!(config: "config/tech-docs.yml") - command = [ - "cd example", - "rm -rf build", - "bundle install --quiet", - "CONFIG_FILE=#{config} NO_CONTRACTS=true middleman build --bail --show-exceptions" - ].join(" && ") + def rebuild_site!(config: "config/tech-docs.yml", overrides: {}) + config_file = Tempfile.new('tech_docs_config') - unless system(command) - raise "`middleman build` failed, see the log for more info" + begin + Dir.chdir('example') do + new_config = YAML.load_file(config).merge(overrides) + config_file.write(YAML.dump(new_config)) + config_file.close + command = [ + "rm -rf build", + "bundle install --quiet", + "CONFIG_FILE=#{config_file.path} NO_CONTRACTS=true middleman build --bail --show-exceptions" + ].join(" && ") + unless system(command) + raise "`middleman build` failed, see the log for more info" + end + end + ensure + config_file.unlink end end From d0083555963170da2a17e9e214a5729e1dfb59fd Mon Sep 17 00:00:00 2001 From: Anshul Sirur Date: Fri, 11 Oct 2019 14:40:00 +0100 Subject: [PATCH 2/2] Add show_review_banner configuration option Previously, disabling the review banner that appears at the bottom of pages required a weird JavaScript hack. I've added a new config option that will allow users to disable the banner globally. --- CHANGELOG.md | 2 ++ docs/configuration.md | 12 ++++++++++++ docs/page-expiry.md | 9 ++++----- lib/govuk_tech_docs/page_review.rb | 4 ++++ lib/source/layouts/_page_review.erb | 2 +- spec/features/page_expiration_spec.rb | 16 ++++++++++++++++ 6 files changed, 39 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dc03ce4..ef91fe01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ Selectively include only the CSS for the GOV.UK Frontend components that we are Fix service name to link to the configured `service_link`, rather than being hardcoded to `/` ([#119](https://github.com/alphagov/tech-docs-gem/issues/119)) +Add the `show_review_banner` global configuration option allowing users to hide the page review banner. + ## 2.0.4 Adds `footer_links` option for displaying links in the footer and ability to hide pages from left hand navigation. diff --git a/docs/configuration.md b/docs/configuration.md index 437aa54f..4884277c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -214,3 +214,15 @@ If set to `false`, the red banner will not appear when the page needs to be revi See the separate [documentation for page expiry][expiry] for more details. [expiry]: https://tdt-documentation.london.cloudapps.digital/page-expiry.html#page-expiry-and-review + +## `show_review_banner` + +Decides whether or not to display the page review banner, regardless of whether the page needs to be reviewed or not. + +If not present or set to `true`, the banner will be displayed on the page. This is the default behaviour. + +If set to `false`, the banner will not be displayed. + +See the separate [documentation for page expiry][expiry] for more details. + +[expiry]: https://tdt-documentation.london.cloudapps.digital/page-expiry.html#page-expiry-and-review diff --git a/docs/page-expiry.md b/docs/page-expiry.md index f02fb664..ffe5e37f 100644 --- a/docs/page-expiry.md +++ b/docs/page-expiry.md @@ -24,14 +24,13 @@ Setting `show_expiry: false` generates a blue banner with the last reviewed date This feature relies on JavaScript being enabled on the user's browser to display the relevant notices. -If you want to disable the banners, but keep the review dates in the frontmatter, add the following to `source/javascripts/application.js` +If you want to disable the banners, but keep the review dates in the frontmatter, add the following to your global configuration file: -```js -// Disable page expiry banner -window.GOVUK.Modules.PageExpiry = null; +```yaml +show_review_banner: false ``` -For example if you do not want any page expiry banner at the bottom of the page, but want to have the review dates in the frontmatter for Daniel the Manual Spaniel to pick up. +The page review banner will no longer appear at the bottom of the page, but Daniel the Manual Spaniel will still notify you about expired pages. ## Frontmatter configuration diff --git a/lib/govuk_tech_docs/page_review.rb b/lib/govuk_tech_docs/page_review.rb index e7b4cacc..637a63ce 100644 --- a/lib/govuk_tech_docs/page_review.rb +++ b/lib/govuk_tech_docs/page_review.rb @@ -40,6 +40,10 @@ def show_expiry? @config[:tech_docs].fetch(:show_expiry, true) end + def show_review_banner? + @config[:tech_docs].fetch(:show_review_banner, true) + end + private def default_owner_slack diff --git a/lib/source/layouts/_page_review.erb b/lib/source/layouts/_page_review.erb index ffa9770b..3327eee4 100644 --- a/lib/source/layouts/_page_review.erb +++ b/lib/source/layouts/_page_review.erb @@ -1,4 +1,4 @@ -<% if current_page_review.under_review? %> +<% if current_page_review.under_review? && current_page_review.show_review_banner? %>
This page was last reviewed on <%= format_date current_page_review.last_reviewed_on %>. diff --git a/spec/features/page_expiration_spec.rb b/spec/features/page_expiration_spec.rb index 5859cc56..87841356 100644 --- a/spec/features/page_expiration_spec.rb +++ b/spec/features/page_expiration_spec.rb @@ -20,6 +20,14 @@ then_i_dont_see_review_date end + it "hides the review banner when show_review_banner is false" do + when_the_site_is_created_hiding_review_banner + and_i_visit_an_expired_page + then_i_dont_see_the_review_banner + and_i_visit_a_not_expired_page + then_i_dont_see_the_review_banner + end + def when_the_site_is_created rebuild_site! end @@ -28,6 +36,10 @@ def when_the_site_is_created_hiding_expiry rebuild_site!(overrides: { 'show_expiry' => false }) end + def when_the_site_is_created_hiding_review_banner + rebuild_site!(overrides: { 'show_review_banner' => false }) + end + def and_i_visit_an_expired_page visit "/expired-page.html" end @@ -47,4 +59,8 @@ def then_i_dont_see_the_page_has_expired def then_i_dont_see_review_date expect(page).to have_no_content "It needs to be reviewed again on" end + + def then_i_dont_see_the_review_banner + expect(page).to have_no_content "This page was last reviewed" + end end