Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable rails-specific rules for rubocop-govuk #1563

Merged
merged 1 commit into from
Dec 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions app/controllers/content_items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/webchat_availability_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/coming_soon_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/content_item/updatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/travel_advice_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions lib/tasks/wraith.rake
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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]
Expand All @@ -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 }
Expand Down
12 changes: 6 additions & 6 deletions test/controllers/content_items_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion test/controllers/step_navigation_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 4 additions & 4 deletions test/helpers/webchat_availability_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/integration/contact_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/corporate_information_page_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/detailed_guide_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/document_collection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/fatality_notice_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 6 additions & 6 deletions test/integration/guide_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/html_publication_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion test/integration/phase_label_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions test/integration/service_sign_in/choose_sign_in_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"]')
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"]',
Expand Down
6 changes: 3 additions & 3 deletions test/integration/specialist_document_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/statistical_data_set_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/integration/statistics_announcement_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions test/integration/topical_event_about_page_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/travel_advice_atom_feed_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/travel_advice_print_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading