From e00c410f483f49f0a0359e2ba3d974a6b306429a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 2 Jun 2021 05:09:33 +0000 Subject: [PATCH 1/2] Bump rubocop-govuk from 3.17.2 to 4.0.0 Bumps [rubocop-govuk](https://github.com/alphagov/rubocop-govuk) from 3.17.2 to 4.0.0. - [Release notes](https://github.com/alphagov/rubocop-govuk/releases) - [Changelog](https://github.com/alphagov/rubocop-govuk/blob/main/CHANGELOG.md) - [Commits](https://github.com/alphagov/rubocop-govuk/compare/v3.17.2...v4.0.0) --- updated-dependencies: - dependency-name: rubocop-govuk dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- Gemfile.lock | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index debb5a335..19fe143b9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -64,7 +64,7 @@ GEM ansi (1.5.0) archive-zip (0.12.0) io-like (~> 0.3.0) - ast (2.4.1) + ast (2.4.2) better_errors (2.7.1) coderay (>= 1.0.0) erubi (>= 1.0.0) @@ -199,8 +199,8 @@ GEM mini_portile2 (~> 2.5.0) racc (~> 1.4) null_logger (0.0.1) - parallel (1.19.2) - parser (2.7.2.0) + parallel (1.20.1) + parser (3.0.1.1) ast (~> 2.4.1) phantomjs (2.1.1.0) plek (4.0.0) @@ -265,32 +265,33 @@ GEM rexml (3.2.5) robotex (1.0.0) rouge (3.26.0) - rubocop (0.87.1) + rubocop (1.15.0) parallel (~> 1.10) - parser (>= 2.7.1.1) + parser (>= 3.0.0.0) rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 1.7) + regexp_parser (>= 1.8, < 3.0) rexml - rubocop-ast (>= 0.1.0, < 1.0) + rubocop-ast (>= 1.5.0, < 2.0) ruby-progressbar (~> 1.7) - unicode-display_width (>= 1.4.0, < 2.0) - rubocop-ast (0.8.0) - parser (>= 2.7.1.5) - rubocop-govuk (3.17.2) - rubocop (= 0.87.1) - rubocop-ast (= 0.8.0) - rubocop-rails (= 2.8.1) + unicode-display_width (>= 1.4.0, < 3.0) + rubocop-ast (1.6.0) + parser (>= 3.0.1.1) + rubocop-govuk (4.0.0) + rubocop (~> 1.15.0) + rubocop-ast (~> 1.6.0) + rubocop-rails (~> 2.10.0) rubocop-rake (= 0.5.1) - rubocop-rspec (= 1.42.0) - rubocop-rails (2.8.1) + rubocop-rspec (~> 2.3.0) + rubocop-rails (2.10.1) activesupport (>= 4.2.0) rack (>= 1.1) - rubocop (>= 0.87.0) + rubocop (>= 1.7.0, < 2.0) rubocop-rake (0.5.1) rubocop - rubocop-rspec (1.42.0) - rubocop (>= 0.87.0) - ruby-progressbar (1.10.1) + rubocop-rspec (2.3.0) + rubocop (~> 1.0) + rubocop-ast (>= 1.1.0) + ruby-progressbar (1.11.0) ruby2_keywords (0.0.4) rubyzip (2.3.0) safe_yaml (1.0.5) @@ -338,7 +339,7 @@ GEM unf (0.1.4) unf_ext unf_ext (0.0.7.7) - unicode-display_width (1.7.0) + unicode-display_width (2.0.0) unicorn (5.8.0) kgio (~> 2.6) raindrops (~> 0.7) From d198c053639a82f3677be5561683ff55e61e4072 Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Fri, 4 Jun 2021 09:38:25 +0100 Subject: [PATCH 2/2] Correct linting failures --- app/controllers/application_controller.rb | 2 +- app/controllers/content_items_controller.rb | 4 ++-- app/helpers/typography_helper.rb | 4 ++-- app/presenters/consultation_presenter.rb | 8 ++++---- app/presenters/content_item/brexit_hub_page.rb | 2 +- app/presenters/content_item/news_image.rb | 2 +- app/presenters/content_item_presenter.rb | 2 +- app/presenters/guide_presenter.rb | 4 ++-- .../guide_faq_page_schema_presenter.rb | 2 +- .../machine_readable/yaml_faq_page_schema_presenter.rb | 2 +- app/presenters/service_sign_in/paths.rb | 2 +- app/services/presenter_builder.rb | 4 +++- config/environments/production.rb | 2 +- test/controllers/content_items_controller_test.rb | 6 +++--- test/controllers/step_navigation_controller_test.rb | 4 ++-- test/integration/answer_test.rb | 2 +- .../service_sign_in/create_new_account_test.rb | 2 +- test/integration/specialist_document_test.rb | 2 +- test/presenters/content_item/parts_test.rb | 8 ++++---- test/presenters/content_item/shareable_test.rb | 2 +- test/presenters/content_item_presenter_test.rb | 2 +- test/test_helper.rb | 10 +++++----- 22 files changed, 40 insertions(+), 38 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 41e929d77..3cee9a51a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -18,6 +18,6 @@ def content_item_path .compact .join(".") - "/" + URI.encode_www_form_component(path_and_optional_locale).gsub("%2F", "/") + "/#{URI.encode_www_form_component(path_and_optional_locale).gsub('%2F', '/')}" end end diff --git a/app/controllers/content_items_controller.rb b/app/controllers/content_items_controller.rb index e09d428d1..1e3bbc6d1 100644 --- a/app/controllers/content_items_controller.rb +++ b/app/controllers/content_items_controller.rb @@ -173,8 +173,8 @@ def service_url(original_url) url.to_s end - def with_locale - I18n.with_locale(@content_item.locale || I18n.default_locale) { yield } + def with_locale(&block) + I18n.with_locale(@content_item.locale || I18n.default_locale, &block) end def error_403(exception) diff --git a/app/helpers/typography_helper.rb b/app/helpers/typography_helper.rb index 31289b310..5ae481b6b 100644 --- a/app/helpers/typography_helper.rb +++ b/app/helpers/typography_helper.rb @@ -3,10 +3,10 @@ def nbsp_between_last_two_words(text) return text if text.nil? escaped_text = html_escape_once(text.strip) - escaped_text.sub(/\s([\w\.\?\!\:]+)$/, ' \1').html_safe + escaped_text.sub(/\s([\w.?!:]+)$/, ' \1').html_safe end def strip_trailing_colons(text) - text.gsub(/\:$/, "") + text.gsub(/:$/, "") end end diff --git a/app/presenters/consultation_presenter.rb b/app/presenters/consultation_presenter.rb index 577a1da0d..ff2085926 100644 --- a/app/presenters/consultation_presenter.rb +++ b/app/presenters/consultation_presenter.rb @@ -24,7 +24,7 @@ def opening_date_midnight? end def closing_date - display_date_and_time(closing_date_time, true) + display_date_and_time(closing_date_time, rollback_midnight: true) end def open? @@ -118,16 +118,16 @@ def add_margin? private - def display_date_and_time(date, rollback_midnight = false) + def display_date_and_time(date, rollback_midnight: false) time = Time.zone.parse(date) date_format = "%-e %B %Y" time_format = "%l:%M%P" - if rollback_midnight + if rollback_midnight && (time.strftime(time_format) == "12:00am") # 12am, 12:00am and "midnight on" can all be misinterpreted # Use 11:59pm on the day before to remove ambiguity # 12am on 10 January becomes 11:59pm on 9 January - time -= 1.second if time.strftime(time_format) == "12:00am" + time -= 1.second end I18n.l(time, format: "#{time_format} on #{date_format}").gsub(":00", "").gsub("12pm", "midday").gsub("12am on ", "").strip end diff --git a/app/presenters/content_item/brexit_hub_page.rb b/app/presenters/content_item/brexit_hub_page.rb index e52737b3b..e7592c02f 100644 --- a/app/presenters/content_item/brexit_hub_page.rb +++ b/app/presenters/content_item/brexit_hub_page.rb @@ -20,7 +20,7 @@ def brexit_links end def brexit_link - brexit_links[content_item.dig("content_id")] + brexit_links[content_item["content_id"]] end end end diff --git a/app/presenters/content_item/news_image.rb b/app/presenters/content_item/news_image.rb index 7aa3a6cbc..2c3d0069e 100644 --- a/app/presenters/content_item/news_image.rb +++ b/app/presenters/content_item/news_image.rb @@ -13,7 +13,7 @@ def default_news_image def placeholder_image # this image has been uploaded to asset-manager - if content_item.dig("document_type") == "world_news_story" + if content_item["document_type"] == "world_news_story" { "url" => "https://assets.publishing.service.gov.uk/media/5e985599d3bf7f3fc943bbd8/UK_government_logo.jpg" } else { "url" => "https://assets.publishing.service.gov.uk/media/5e59279b86650c53b2cefbfe/placeholder.jpg" } diff --git a/app/presenters/content_item_presenter.rb b/app/presenters/content_item_presenter.rb index f160748d0..547500e6e 100644 --- a/app/presenters/content_item_presenter.rb +++ b/app/presenters/content_item_presenter.rb @@ -66,7 +66,7 @@ def web_url def canonical_url if requesting_a_part? - web_url + "/" + part_slug + "#{web_url}/#{part_slug}" else web_url end diff --git a/app/presenters/guide_presenter.rb b/app/presenters/guide_presenter.rb index 3c32d03cc..dec1852e6 100644 --- a/app/presenters/guide_presenter.rb +++ b/app/presenters/guide_presenter.rb @@ -46,8 +46,8 @@ def show_guide_navigation? end def content_title - if parts.any? - return current_part_title if hide_chapter_navigation? + if parts.any? && hide_chapter_navigation? + return current_part_title end title diff --git a/app/presenters/machine_readable/guide_faq_page_schema_presenter.rb b/app/presenters/machine_readable/guide_faq_page_schema_presenter.rb index d740c3faf..7de8ecad6 100644 --- a/app/presenters/machine_readable/guide_faq_page_schema_presenter.rb +++ b/app/presenters/machine_readable/guide_faq_page_schema_presenter.rb @@ -56,7 +56,7 @@ def part_url(part, index) if index.zero? guide_url else - guide_url + "/" + part["slug"] + "#{guide_url}/#{part['slug']}" end end diff --git a/app/presenters/machine_readable/yaml_faq_page_schema_presenter.rb b/app/presenters/machine_readable/yaml_faq_page_schema_presenter.rb index 52e70ceb6..dd9c8592f 100644 --- a/app/presenters/machine_readable/yaml_faq_page_schema_presenter.rb +++ b/app/presenters/machine_readable/yaml_faq_page_schema_presenter.rb @@ -7,7 +7,7 @@ def self.configured?(content_item) end def self.config_path(content_item) - CONFIG_PATH.join(content_item.slug + ".yml").to_s + CONFIG_PATH.join("#{content_item.slug}.yml").to_s end def initialize(content_item) diff --git a/app/presenters/service_sign_in/paths.rb b/app/presenters/service_sign_in/paths.rb index 4a4b4b254..79f7d9de8 100644 --- a/app/presenters/service_sign_in/paths.rb +++ b/app/presenters/service_sign_in/paths.rb @@ -1,7 +1,7 @@ module ServiceSignIn module Paths def path - content_item["base_path"] + "/" + content_item["details"][page_type]["slug"] + "#{content_item['base_path']}/#{content_item['details'][page_type]['slug']}" end def has_valid_path? diff --git a/app/services/presenter_builder.rb b/app/services/presenter_builder.rb index d04934807..f4b755103 100644 --- a/app/services/presenter_builder.rb +++ b/app/services/presenter_builder.rb @@ -38,7 +38,7 @@ def presenter_name return service_sign_in_presenter_name end - content_item["schema_name"].classify + "Presenter" + "#{content_item['schema_name'].classify}Presenter" end def service_sign_in_format? @@ -66,6 +66,8 @@ def initialize(content_item) @content_item = content_item end end + class SpecialRouteReturned < StandardError; end + class GovernmentReturned < StandardError; end end diff --git a/config/environments/production.rb b/config/environments/production.rb index d12b86a27..525e36f7a 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -75,7 +75,7 @@ # config.logger = ActiveSupport::TaggedLogging.new(Syslog::Logger.new 'app-name') if ENV["RAILS_LOG_TO_STDOUT"].present? - logger = ActiveSupport::Logger.new(STDOUT) + logger = ActiveSupport::Logger.new($stdout) logger.formatter = config.log_formatter config.logger = ActiveSupport::TaggedLogging.new(logger) end diff --git a/test/controllers/content_items_controller_test.rb b/test/controllers/content_items_controller_test.rb index 73fe1b2ca..1261b4448 100644 --- a/test/controllers/content_items_controller_test.rb +++ b/test/controllers/content_items_controller_test.rb @@ -294,7 +294,7 @@ class ContentItemsControllerTest < ActionController::TestCase test "returns 404 for invalid url" do path = "foreign-travel-advice/egypt]" - stub_content_store_does_not_have_item("/" + path) + stub_content_store_does_not_have_item("/#{path}") get :show, params: { path: path } assert_response :not_found @@ -303,7 +303,7 @@ class ContentItemsControllerTest < ActionController::TestCase test "returns 404 for item not in content store" do path = "government/case-studies/boost-chocolate-production" - stub_content_store_does_not_have_item("/" + path) + stub_content_store_does_not_have_item("/#{path}") get :show, params: { path: path } assert_response :not_found @@ -323,7 +323,7 @@ class ContentItemsControllerTest < ActionController::TestCase test "returns 403 for access-limited item" do path = "government/case-studies/super-sekrit-document" - url = content_store_endpoint + "/content/" + path + url = "#{content_store_endpoint}/content/#{path}" stub_request(:get, url).to_return(status: 403, headers: {}) get :show, params: { path: path } diff --git a/test/controllers/step_navigation_controller_test.rb b/test/controllers/step_navigation_controller_test.rb index 65a63218b..2a821ee48 100644 --- a/test/controllers/step_navigation_controller_test.rb +++ b/test/controllers/step_navigation_controller_test.rb @@ -7,7 +7,7 @@ class ContentItemsControllerTest < ActionController::TestCase test "#{schema_name} shows step by step navigation where relevant" do content_item = content_store_has_schema_example(schema_name, "#{schema_name}-with-step-navs") content_item["base_path"] = "/pass-plus" - path = content_item["base_path"][1..-1] + path = content_item["base_path"][1..] stub_content_store_has_item(content_item["base_path"], content_item) @@ -22,7 +22,7 @@ class ContentItemsControllerTest < ActionController::TestCase test "#{schema_name} does not show step by step navigation where relevant" do content_item = content_store_has_schema_example(schema_name, schema_name) content_item["base_path"] = "/not-part-of-a-step-by-step" - path = content_item["base_path"][1..-1] + path = content_item["base_path"][1..] stub_content_store_has_item(content_item["base_path"], content_item) diff --git a/test/integration/answer_test.rb b/test/integration/answer_test.rb index 209e872f4..fa710e7ba 100644 --- a/test/integration/answer_test.rb +++ b/test/integration/answer_test.rb @@ -17,7 +17,7 @@ class AnswerTest < ActionDispatch::IntegrationTest first_related_link = @content_item["details"]["external_related_links"].first within(".gem-c-related-navigation") do - assert page.has_css?('.gem-c-related-navigation__section-link--other[href="' + first_related_link["url"] + '"]', text: first_related_link["title"]) + assert page.has_css?(".gem-c-related-navigation__section-link--other[href=\"#{first_related_link['url']}\"]", text: first_related_link["title"]) end end diff --git a/test/integration/service_sign_in/create_new_account_test.rb b/test/integration/service_sign_in/create_new_account_test.rb index 7d880b392..d84da637a 100644 --- a/test/integration/service_sign_in/create_new_account_test.rb +++ b/test/integration/service_sign_in/create_new_account_test.rb @@ -36,7 +36,7 @@ class CreateNewAccount < ActionDispatch::IntegrationTest def setup_and_visit_create_new_account_page content_item = get_content_example("service_sign_in") - path = content_item["base_path"] + "/create-new-account" + path = "#{content_item['base_path']}/create-new-account" stub_content_store_has_item(path, content_item.to_json) visit(path) end diff --git a/test/integration/specialist_document_test.rb b/test/integration/specialist_document_test.rb index f40cd8fe5..cb90c1346 100644 --- a/test/integration/specialist_document_test.rb +++ b/test/integration/specialist_document_test.rb @@ -135,7 +135,7 @@ class SpecialistDocumentTest < ActionDispatch::IntegrationTest def assert_nested_content_item(heading) heading_level = heading["level"] selector = "a[href=\"##{heading['id']}\"]" - text = heading["text"].gsub(/\:$/, "") + text = heading["text"].gsub(/:$/, "") if heading_level < 4 assert page.has_css?(selector), "Failed to find an element matching: #{selector}" diff --git a/test/presenters/content_item/parts_test.rb b/test/presenters/content_item/parts_test.rb index 2401bf7d3..19fb22e85 100644 --- a/test/presenters/content_item/parts_test.rb +++ b/test/presenters/content_item/parts_test.rb @@ -91,7 +91,7 @@ def part_slug end def requested_path - base_path + "/second-slug" + "#{base_path}/second-slug" end end @@ -105,7 +105,7 @@ def part_slug end def requested_path - base_path + "/" + part_slug + "#{base_path}/#{part_slug}" end end @@ -121,7 +121,7 @@ def part_slug end def requested_path - base_path + "/" + part_slug + "#{base_path}/#{part_slug}" end end @@ -136,7 +136,7 @@ def part_slug end def requested_path - base_path + "/" + part_slug + "#{base_path}/#{part_slug}" end end diff --git a/test/presenters/content_item/shareable_test.rb b/test/presenters/content_item/shareable_test.rb index e5ff02bab..229ea2e6e 100644 --- a/test/presenters/content_item/shareable_test.rb +++ b/test/presenters/content_item/shareable_test.rb @@ -15,7 +15,7 @@ def initialize end def expected_path - url_encode(Plek.current.website_root + "/a/base/path") + url_encode("#{Plek.current.website_root}/a/base/path") end test "presents the twitter share url" do diff --git a/test/presenters/content_item_presenter_test.rb b/test/presenters/content_item_presenter_test.rb index 305e20467..adba8e80f 100644 --- a/test/presenters/content_item_presenter_test.rb +++ b/test/presenters/content_item_presenter_test.rb @@ -33,7 +33,7 @@ class ContentItemPresenterTest < PresenterTestCase test "#canonical_url with a part" do example_with_parts = govuk_content_schema_example("travel_advice", "full-country") - request_path = example_with_parts["base_path"] + "/safety-and-security" + request_path = "#{example_with_parts['base_path']}/safety-and-security" presenter = create_presenter( TravelAdvicePresenter, content_item: example_with_parts, diff --git a/test/test_helper.rb b/test/test_helper.rb index 1b0f5f6ef..c2175c742 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -37,7 +37,7 @@ class ActiveSupport::TestCase include GovukContentSchemaExamples end -# Note: This is so that slimmer is skipped, preventing network requests for +# NOTE: This is so that slimmer is skipped, preventing network requests for # content from static (i.e. core_layout.html.erb). class ActionController::Base before_action :set_skip_slimmer_header @@ -85,7 +85,7 @@ def assert_has_contents_list(contents) end end - def assert_has_published_dates(first_published = nil, last_updated = nil, history_link = false) + def assert_has_published_dates(first_published = nil, last_updated = nil, history_link: false) text = [] text << first_published if first_published text << last_updated if last_updated @@ -135,7 +135,7 @@ def assert_has_metadata_local(metadata, term_selector, definition_selector) def assert_has_publisher_metadata(options) within(".app-c-publisher-metadata") do - assert_has_published_dates(options[:first_published], options[:last_updated], options[:history_link]) + assert_has_published_dates(options[:first_published], options[:last_updated], history_link: options[:history_link]) assert_has_publisher_metadata_other(options[:metadata]) end end @@ -148,8 +148,8 @@ def assert_has_important_metadata(metadata) end end - def assert_footer_has_published_dates(first_published = nil, last_updated = nil, history_link = false) - assert_has_published_dates(first_published, last_updated, history_link) + def assert_footer_has_published_dates(first_published = nil, last_updated = nil, history_link: false) + assert_has_published_dates(first_published, last_updated, history_link: history_link) end def setup_and_visit_content_item(name, parameter_string = "")