Enable rails-specific rules for rubocop-govuk

Mostly just running rubocop --auto-correct; however, a few special
cases:

- Some checks have been disabled. This is intended to be temporary;
  making them pass will require some kind of refactoring (`OutputSafety`
  and `HelperInstanceVariable`) or decisions about the behaviour of the
  code (`HasManyOrHasOneDependent` and `InverseOf`) that I felt were out
  of scope for this PR. I'll be raising them as future tasks.

Author: benjamineskola

.rubocop.yml

@@ -1,7 +1,13 @@
 inherit_gem:
   rubocop-govuk:
     - "config/default.yml"
+    - "config/rails.yml"
 
 Metrics/BlockLength:
   Exclude:
     - "test/**/*_test.rb"
+
+Rails/OutputSafety:
+  Enabled: false
+Rails/HelperInstanceVariable:
+  Enabled: false

app/controllers/content_items_controller.rb

@@ -144,19 +144,19 @@ def with_locale
   end
 
   def error_403(exception)
-    render plain: exception.message, status: 403
+    render plain: exception.message, status: :forbidden
   end
 
   def error_notfound
     render plain: "Not found", status: :not_found
   end
 
   def error_406
-    render plain: "Not acceptable", status: 406
+    render plain: "Not acceptable", status: :not_acceptable
   end
 
   def error_410
-    render plain: "Gone", status: 410
+    render plain: "Gone", status: :gone
   end
 
   def error_redirect(exception)

app/helpers/webchat_availability_helper.rb

@@ -6,8 +6,8 @@ module WebchatAvailabilityHelper
     /government/organisations/hm-revenue-customs/contact/self-assessment
     /government/organisations/hm-revenue-customs/contact/tax-credits-enquiries
   ).freeze
-  UNAVAILABILITY_START = Time.parse("2018-05-12 16:00 BST").freeze
-  UNAVAILABILITY_END = Time.parse("2018-05-14 09:00 BST").freeze
+  UNAVAILABILITY_START = Time.zone.parse("2018-05-12 16:00 BST").freeze
+  UNAVAILABILITY_END = Time.zone.parse("2018-05-14 09:00 BST").freeze
 
   def webchat_unavailable?(now = Time.zone.now)
     show_unavailability = now >= UNAVAILABILITY_START && now < UNAVAILABILITY_END

app/presenters/coming_soon_presenter.rb

@@ -15,7 +15,7 @@ def formatted_publish_date
   end
 
   def formatted_publish_time
-    display_time(Time.parse(@publish_time))
+    display_time(Time.zone.parse(@publish_time))
   end
 
   def page_title

app/presenters/content_item/updatable.rb

@@ -38,7 +38,7 @@ def change_history
     # The direction of change history isn’t guaranteed
     # https://github.com/alphagov/govuk-content-schemas/issues/545
     def reverse_chronological_change_history
-      change_history.sort_by { |item| Time.parse(item[:timestamp]) }.reverse
+      change_history.sort_by { |item| Time.zone.parse(item[:timestamp]) }.reverse
     end
 
     def any_updates?

app/presenters/travel_advice_presenter.rb

@@ -89,7 +89,7 @@ def atom_change_description
   end
 
   def atom_public_updated_at
-    Time.parse(content_item["public_updated_at"])
+    Time.zone.parse(content_item["public_updated_at"])
   end
 
   def cache_control_max_age(format)

lib/tasks/wraith.rake

@@ -1,10 +1,10 @@
 require "rest-client"
-require "#{Rails.root}/lib/helpers/wraith_config_helper.rb"
-require "#{Rails.root}/lib/helpers/document_types_helper.rb"
+require Rails.root.join("lib/helpers/wraith_config_helper.rb")
+require Rails.root.join("lib/helpers/document_types_helper.rb")
 
 namespace :wraith do
   desc "check top 10 content items for document_type using wraith"
-  task :document_type, [:document_type] do |_t, args|
+  task :document_type, [:document_type] => :environment do |_t, args|
     document_type = args[:document_type]
     document_type_paths = DocumentTypesHelper.new.type_paths(document_type)
     wraith_config_file = WraithConfigHelper.new(document_type, document_type_paths).create_config
@@ -13,7 +13,7 @@ namespace :wraith do
   end
 
   desc "check top 10 content items for all known document types"
-  task :all_document_types, [:sample_size] do |_t, args|
+  task :all_document_types, [:sample_size] => :environment do |_t, args|
     args.with_defaults(sample_size: 10)
     # Make sure an up to date document_types file exists
     Rake::Task["wraith:update_document_types"].invoke args[:sample_size]
@@ -23,7 +23,7 @@ namespace :wraith do
   end
 
   desc "creates a wraith config of document type examples from the search api"
-  task :update_document_types, [:sample_size] do |_t, args|
+  task :update_document_types, [:sample_size] => :environment do |_t, args|
     args.with_defaults(sample_size: 10)
     document_type_paths = DocumentTypesHelper.new(args[:sample_size]).all_type_paths
     document_types = { "document_types" => document_type_paths.keys }

test/controllers/content_items_controller_test.rb

@@ -146,8 +146,8 @@ class ContentItemsControllerTest < ActionController::TestCase
 
     get :show, params: { path: path_for(content_item) }
     assert_response :success
-    refute_empty content_item["links"]["ordered_related_items"], "Content item should have existing related links"
-    refute_empty content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links"
+    assert_not_empty content_item["links"]["ordered_related_items"], "Content item should have existing related links"
+    assert_not_empty content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links"
     assert_equal content_item["links"]["ordered_related_items"], assigns[:content_item].content_item["links"]["ordered_related_items"]
   end
 
@@ -160,8 +160,8 @@ class ContentItemsControllerTest < ActionController::TestCase
     get :show, params: { path: path_for(content_item) }
     assert_response :success
     assert_nil content_item["links"]["ordered_related_items"], "Content item should not have existing related links"
-    refute_empty content_item["links"]["ordered_related_items_overrides"], "Content item should have existing related link overrides"
-    refute_empty content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links"
+    assert_not_empty content_item["links"]["ordered_related_items_overrides"], "Content item should have existing related link overrides"
+    assert_not_empty content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links"
     assert_nil content_item["links"]["ordered_related_items"]
   end
 
@@ -174,7 +174,7 @@ class ContentItemsControllerTest < ActionController::TestCase
     get :show, params: { path: path_for(content_item) }
     assert_response :success
     assert_empty content_item["links"]["ordered_related_items"], "Content item should have existing related links"
-    refute_empty content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links"
+    assert_not_empty content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links"
     assert_equal [], assigns[:content_item].content_item["links"]["ordered_related_items"]
   end
 
@@ -187,7 +187,7 @@ class ContentItemsControllerTest < ActionController::TestCase
     get :show, params: { path: path_for(content_item) }
     assert_response :success
     assert_empty content_item["links"]["ordered_related_items"], "Content item should not have existing related links"
-    refute_empty content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links"
+    assert_not_empty content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links"
     assert_equal assigns[:content_item].content_item["links"]["ordered_related_items"], content_item["links"]["suggested_ordered_related_items"]
   end
 

test/controllers/step_navigation_controller_test.rb

@@ -31,7 +31,7 @@ class ContentItemsControllerTest < ActionController::TestCase
       get :show, params: { path: path }
 
       assert_response 200
-      refute response.body.include?("Learn to drive a car: step by step")
+      assert_not response.body.include?("Learn to drive a car: step by step")
     end
   end
 end

test/helpers/webchat_availability_helper_test.rb

@@ -9,9 +9,9 @@ class WebchatAvailabilityHelperTest < ActionView::TestCase
   end
 
   test "webchat_unavailable? indicates unavailability" do
-    refute webchat_unavailable?(Time.parse("2018-05-12 15:59 BST"))
-    assert webchat_unavailable?(Time.parse("2018-05-12 16:00 BST"))
-    assert webchat_unavailable?(Time.parse("2018-05-14 08:59 BST"))
-    refute webchat_unavailable?(Time.parse("2018-05-14 09:00 BST"))
+    assert_not webchat_unavailable?(Time.zone.parse("2018-05-12 15:59 BST"))
+    assert webchat_unavailable?(Time.zone.parse("2018-05-12 16:00 BST"))
+    assert webchat_unavailable?(Time.zone.parse("2018-05-14 08:59 BST"))
+    assert_not webchat_unavailable?(Time.zone.parse("2018-05-14 09:00 BST"))
   end
 end

test/integration/contact_test.rb

@@ -39,7 +39,7 @@ class ContactTest < ActionDispatch::IntegrationTest
 
     assert_equal 1, @content_item["details"]["phone_numbers"].size
     first_phone = @content_item["details"]["phone_numbers"].first
-    refute page.has_css?("h3", text: first_phone["title"])
+    assert_not page.has_css?("h3", text: first_phone["title"])
   end
 
   test "posts are rendered" do

test/integration/corporate_information_page_test.rb

@@ -27,7 +27,7 @@ class CorporateInformationPageTest < ActionDispatch::IntegrationTest
     content_store_has_item(item["base_path"], item.to_json)
     visit_with_cachebust(item["base_path"])
 
-    refute page.has_css?(".gem-c-contents-list")
+    assert_not page.has_css?(".gem-c-contents-list")
   end
 
   test "renders corporate information with body when present" do

test/integration/detailed_guide_test.rb

@@ -77,7 +77,7 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest
 
   test "renders without contents list if it has fewer than 3 items" do
     setup_and_visit_content_item("national_applicability_alternative_url_detailed_guide")
-    refute page.has_css?(".gem-c-contents-list")
+    assert_not page.has_css?(".gem-c-contents-list")
   end
 
   test "conditionally renders a logo" do

test/integration/document_collection_test.rb

@@ -56,7 +56,7 @@ class DocumentCollectionTest < ActionDispatch::IntegrationTest
     content_store_has_item(item["base_path"], item.to_json)
     visit_with_cachebust(item["base_path"])
 
-    refute page.has_css?(".gem-c-contents-list")
+    assert_not page.has_css?(".gem-c-contents-list")
   end
 
   test "renders each collection group" do

test/integration/fatality_notice_test.rb

@@ -18,7 +18,7 @@ class FatalityNoticeTest < ActionDispatch::IntegrationTest
     assert page.has_content?("Operations in Zululand")
     assert_has_component_title("Sir George Pomeroy Colley killed in Boer War")
 
-    refute page.has_css?(".gem-c-notice")
+    assert_not page.has_css?(".gem-c-notice")
 
     assert_has_publisher_metadata(
       published: "Published 27 February 1881",

test/integration/guide_test.rb

@@ -25,16 +25,16 @@ class GuideTest < ActionDispatch::IntegrationTest
   test "does not show part navigation, print link or part title when only one part" do
     setup_and_visit_content_item("single-page-guide")
 
-    refute page.has_css?("h1", text: @content_item["details"]["parts"].first["title"])
-    refute page.has_css?(".app-c-print-link")
+    assert_not page.has_css?("h1", text: @content_item["details"]["parts"].first["title"])
+    assert_not page.has_css?(".app-c-print-link")
   end
 
   test "replaces guide title with part title if in a step by step and hide_chapter_navigation is true" do
     setup_and_visit_content_item("guide-with-step-navs-and-hide-navigation")
     title = @content_item["title"]
     part_title = @content_item["details"]["parts"][0]["title"]
 
-    refute page.has_css?("h1", text: title)
+    assert_not page.has_css?("h1", text: title)
     assert_has_component_title(part_title)
   end
 
@@ -52,15 +52,15 @@ class GuideTest < ActionDispatch::IntegrationTest
     title = @content_item["title"]
     part_title = @content_item["details"]["parts"][0]["title"]
 
-    refute page.has_css?("h1", text: title)
+    assert_not page.has_css?("h1", text: title)
     assert_has_component_title(part_title)
   end
 
   test "does not show guide navigation and print link if in a step by step and hide_chapter_navigation is true" do
     setup_and_visit_content_item("guide-with-step-navs-and-hide-navigation")
 
-    refute page.has_css?(".gem-c-pagination")
-    refute page.has_css?(".app-c-print-link")
+    assert_not page.has_css?(".gem-c-pagination")
+    assert_not page.has_css?(".app-c-print-link")
   end
 
   test "shows guide navigation and print link if not in a step by step and hide_chapter_navigation is true" do

test/integration/html_publication_test.rb

@@ -64,7 +64,7 @@ class HtmlPublicationTest < ActionDispatch::IntegrationTest
     setup_and_visit_content_item("prime_ministers_office")
 
     within ".gem-c-inverse-header" do
-      refute page.has_text?("Contents")
+      assert_not page.has_text?("Contents")
     end
   end
 

test/integration/phase_label_test.rb

@@ -19,6 +19,6 @@ class PhaseLabelTest < ActionDispatch::IntegrationTest
 
     visit_with_cachebust "/government/case-studies/get-britain-building-carlisle-park"
 
-    refute page.has_text?("alpha")
+    assert_not page.has_text?("alpha")
   end
 end

test/integration/service_sign_in/choose_sign_in_test.rb

@@ -17,7 +17,7 @@ class ChooseSignInTest < ActionDispatch::IntegrationTest
 
       assert page.has_css?("title", text: "Prove your identity to continue - GOV.UK", visible: false)
       assert page.has_css?('meta[name="robots"][content="noindex, nofollow"]', visible: false)
-      refute page.has_css?("#proposition-menu")
+      assert_not page.has_css?("#proposition-menu")
 
       assert page.has_css?('.gem-c-back-link[href="/log-in-file-self-assessment-tax-return"]', text: "Back")
       assert page.has_css?('form[data-module="track-radio-group"]')
@@ -73,7 +73,7 @@ class ChooseSignInTest < ActionDispatch::IntegrationTest
       assert page.has_css?(".gem-c-radio .govuk-label", text: "Use GOV.UK Verify")
       assert page.has_css?(".gem-c-radio .govuk-hint", text: "You can use an existing identity account or create a new one. It usually takes about 5 minutes to create an account.")
 
-      refute page.has_css?(".govuk-radios__divider", text: "or")
+      assert_not page.has_css?(".govuk-radios__divider", text: "or")
     end
 
     test "page renders welsh correctly" do

test/integration/service_sign_in/create_new_account_test.rb

@@ -26,7 +26,7 @@ class CreateNewAccount < ActionDispatch::IntegrationTest
 
       assert page.has_text?("To use this service, you need to create either a Government Gateway or GOV.UK Verify account. These are used to help fight identity theft.")
       assert page.has_css?('meta[name="robots"][content="noindex, nofollow"]', visible: false)
-      refute page.has_css?("#proposition-menu")
+      assert_not page.has_css?("#proposition-menu")
 
       assert page.has_css?(
         '.gem-c-back-link[href="/log-in-file-self-assessment-tax-return/sign-in/choose-sign-in"]',

test/integration/specialist_document_test.rb

@@ -141,7 +141,7 @@ def assert_nested_content_item(heading)
       assert page.has_css?(selector), "Failed to find an element matching: #{selector}"
       assert page.has_css?(selector, text: text), "Failed to find an element matching #{selector} with text: #{text}"
     else
-      refute page.has_css?(selector), "Found a nested heading too deep, there should be no element matching: #{selector}"
+      assert_not page.has_css?(selector), "Found a nested heading too deep, there should be no element matching: #{selector}"
     end
 
     if heading["headers"].present?
@@ -154,7 +154,7 @@ def assert_nested_content_item(heading)
   test "renders no start button when not set" do
     setup_and_visit_content_item("aaib-reports")
 
-    refute page.has_css?(".gem-c-button", text: "Find out more")
+    assert_not page.has_css?(".gem-c-button", text: "Find out more")
   end
 
   test "renders start button" do
@@ -167,7 +167,7 @@ def assert_nested_content_item(heading)
   test "does not render a contents list if there are fewer than three items in the contents list" do
     setup_and_visit_content_item("aaib-reports")
 
-    refute page.has_css?("#contents .gem-c-contents-list")
+    assert_not page.has_css?("#contents .gem-c-contents-list")
   end
 
   test "renders a link to statutory instruments finder" do

test/integration/statistical_data_set_test.rb

@@ -65,6 +65,6 @@ class StatisticalDataSetTest < ActionDispatch::IntegrationTest
     content_store_has_item(item["base_path"], item.to_json)
     visit_with_cachebust(item["base_path"])
 
-    refute page.has_css?(".gem-c-contents-list")
+    assert_not page.has_css?(".gem-c-contents-list")
   end
 end

test/integration/statistics_announcement_test.rb

@@ -65,6 +65,6 @@ class StatisticsAnnouncementTest < ActionDispatch::IntegrationTest
   test "cancelled statistics announcements do not display the forthcoming notice" do
     setup_and_visit_content_item("cancelled_official_statistics")
 
-    refute page.has_text?(StatisticsAnnouncementPresenter::FORTHCOMING_NOTICE)
+    assert_not page.has_text?(StatisticsAnnouncementPresenter::FORTHCOMING_NOTICE)
   end
 end

test/integration/topical_event_about_page_test.rb

@@ -19,7 +19,7 @@ class TopicalEventAboutPageTest < ActionDispatch::IntegrationTest
 
   test "slim topical event about pages have no contents" do
     setup_and_visit_content_item("slim")
-    refute page.has_css?(".contents-list.contents-list-dashed")
+    assert_not page.has_css?(".contents-list.contents-list-dashed")
   end
 
   test "contents list not displayed when fewer than three items" do
@@ -29,7 +29,7 @@ class TopicalEventAboutPageTest < ActionDispatch::IntegrationTest
     content_store_has_item(@content_item["base_path"], @content_item.to_json)
 
     visit_with_cachebust @content_item["base_path"]
-    refute page.has_css?(".gem-c-contents-list")
+    assert_not page.has_css?(".gem-c-contents-list")
   end
 
   test "contents list displayed when fewer than three items and first item word count is greater than 100" do

test/integration/travel_advice_atom_feed_test.rb

@@ -8,7 +8,7 @@ class TravelAdviceAtomFeed < ActionDispatch::IntegrationTest
   setup do
     setup_and_parse_travel_advice_atom_feed("full-country")
     @base_path = @content_item["base_path"]
-    @updated_at = Time.parse(@content_item["public_updated_at"])
+    @updated_at = Time.zone.parse(@content_item["public_updated_at"])
   end
 
   test "it sets the alternative link correctly" do

test/integration/travel_advice_print_test.rb

@@ -15,7 +15,7 @@ class TravelAdvicePrint < ActionDispatch::IntegrationTest
     setup_and_visit_travel_advice_print("full-country")
     parts = @content_item["details"]["parts"]
 
-    assert_has_component_metadata_pair("Still current at", Date.today.strftime("%-d %B %Y"))
+    assert_has_component_metadata_pair("Still current at", Time.zone.today.strftime("%-d %B %Y"))
     assert_has_component_metadata_pair("Updated", Date.parse(@content_item["details"]["reviewed_at"]).strftime("%-d %B %Y"))
 
     within ".gem-c-metadata" do