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

Use the RuboCop Rails config #4471

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
inherit_gem:
rubocop-govuk:
- config/default.yml
- config/rails.yml

inherit_mode:
merge:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* Remove shared helper from inset text component ([PR #4571](https://github.com/alphagov/govuk_publishing_components/pull/4571))
* Use component wrapper on contextual footer ([PR #4562](https://github.com/alphagov/govuk_publishing_components/pull/4562))
* Update Govspeak "Warning Text" component styles ([PR #4487](https://github.com/alphagov/govuk_publishing_components/pull/4487))
* Use the RuboCop Rails config ([PR #4471](https://github.com/alphagov/govuk_publishing_components/pull/4471))

## 49.1.0

Expand Down
10 changes: 5 additions & 5 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,23 @@ RSpec::Core::RakeTask.new

namespace :assets do
desc "Test precompiling assets through dummy application"
task :precompile do
task precompile: :environment do
Rake::Task["app:assets:precompile"].invoke
end

desc "Test cleaning assets through dummy application"
task :clean do
task clean: :environment do
Rake::Task["app:assets:clean"].invoke
end

desc "Test clobbering assets through dummy application"
task :clobber do
task clobber: :environment do
Rake::Task["app:assets:clobber"].invoke
end
end

desc "Build the Sass files"
task :dartsass do
task dartsass: :environment do
Rake::Task["app:dartsass:build"].invoke
end

Expand All @@ -43,7 +43,7 @@ task lint: %i[rubocop environment] do
end

desc "Jasmine"
task :jasmine do
task jasmine: :environment do
sh "yarn run jasmine:ci"
end

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module GovukPublishingComponents
class ComponentGuideController < GovukPublishingComponents::ApplicationController
append_view_path File.join(Rails.root, "app", "views", GovukPublishingComponents::Config.component_directory_name)
append_view_path Rails.root.join("app", "views", GovukPublishingComponents::Config.component_directory_name).to_s

MATCH_COMPONENTS = /(?<=govuk_publishing_components\/components\/)[\/a-zA-Z_-]+(?=['"])/

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def find_components(files, find, type, keep_locations)
@gem_style_references << gem_style_references if gem_style_references
end
rescue StandardError => e
puts e.message
Rails.logger.debug e.message
end

components_found.flatten.uniq.sort
Expand Down
2 changes: 1 addition & 1 deletion app/models/govuk_publishing_components/audit_comparer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ def get_helpers_used_by_applications
results = []

@applications_data.each do |application|
next unless application[:application_found] && !application[:helper_references].blank?
next unless application[:application_found] && application[:helper_references].present?

application[:helper_references].each do |key, value|
location = {
Expand Down
2 changes: 1 addition & 1 deletion app/models/govuk_publishing_components/audit_components.rb
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ def get_helper_usage(components)
next unless File.file?(file)

helpers.each do |helper|
next unless File.foreach(file).grep(helper[:match]).present?
next if File.foreach(file).grep(helper[:match]).blank?

helper[:used_by] << {
name: component[:name],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,9 @@ def content_type
)
end

def content_type_abbr
content_type.abbr
end
delegate :abbr, to: :content_type, prefix: true

def content_type_name
content_type.name
end
delegate :name, to: :content_type, prefix: true

def file_size
attachment_data[:file_size]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def size
end

def heading_markup
return unless @heading.present?
return if @heading.blank?

if @is_page_heading
content_tag(
Expand All @@ -87,7 +87,7 @@ def checkbox_markup(checkbox, index)
checkbox_id = checkbox[:id] || "#{@id}-#{index}"
controls = checkbox[:controls]
aria_controls = "#{checkbox_id}-conditional-#{index || rand(1..100)}" if checkbox[:conditional].present?
checkbox_name = checkbox[:name].present? ? checkbox[:name] : @name
checkbox_name = checkbox[:name].presence || @name
checked = true if checkbox[:checked].present?
data = checkbox[:data_attributes] || {}
data[:controls] = controls
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,24 @@ def initialize(options)
def all_attributes
attributes = {}

attributes[:id] = @options[:id] unless @options[:id].blank?
attributes[:data] = @options[:data_attributes] unless @options[:data_attributes].blank?
attributes[:aria] = @options[:aria] unless @options[:aria].blank?
attributes[:id] = @options[:id] if @options[:id].present?
attributes[:data] = @options[:data_attributes] if @options[:data_attributes].present?
attributes[:aria] = @options[:aria] if @options[:aria].present?

((@options[:classes] ||= "") << " govuk-!-margin-bottom-#{@options[:margin_bottom]}").strip! if @options[:margin_bottom]
attributes[:class] = @options[:classes] unless @options[:classes].blank?
attributes[:class] = @options[:classes] if @options[:classes].present?

attributes[:role] = @options[:role] unless @options[:role].blank?
attributes[:lang] = @options[:lang] unless @options[:lang].blank?
attributes[:open] = @options[:open] unless @options[:open].blank?
attributes[:role] = @options[:role] if @options[:role].present?
attributes[:lang] = @options[:lang] if @options[:lang].present?
attributes[:open] = @options[:open] if @options[:open].present?
attributes[:hidden] = @options[:hidden] unless @options[:hidden].nil?
attributes[:tabindex] = @options[:tabindex] unless @options[:tabindex].blank?
attributes[:dir] = @options[:dir] unless @options[:dir].blank?
attributes[:type] = @options[:type] unless @options[:type].blank?
attributes[:draggable] = @options[:draggable] unless @options[:draggable].blank?
attributes[:rel] = @options[:rel] unless @options[:rel].blank?
attributes[:target] = @options[:target] unless @options[:target].blank?
attributes[:title] = @options[:title] unless @options[:title].blank?
attributes[:tabindex] = @options[:tabindex] if @options[:tabindex].present?
attributes[:dir] = @options[:dir] if @options[:dir].present?
attributes[:type] = @options[:type] if @options[:type].present?
attributes[:draggable] = @options[:draggable] if @options[:draggable].present?
attributes[:rel] = @options[:rel] if @options[:rel].present?
attributes[:target] = @options[:target] if @options[:target].present?
attributes[:title] = @options[:title] if @options[:title].present?

attributes
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def breadcrumbs

def organisation_breadcrumbs_items
first_related_organisation = ContentItem.new(content_item.related_organisations.first)
return [] unless first_related_organisation.present?
return [] if first_related_organisation.blank?

[{
title: first_related_organisation.title,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def breadcrumbs
ordered_parents = all_parents.map.with_index do |parent, index|
{
title: parent.title,
url: parent.url_override.present? ? parent.url_override : parent.base_path,
url: parent.url_override.presence || parent.base_path,
is_page_parent: index.zero?,
}
end
Expand Down
4 changes: 1 addition & 3 deletions lib/govuk_publishing_components/presenters/content_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,7 @@ def ==(other)
content_id == other.content_id
end

def hash
content_id.hash
end
delegate :hash, to: :content_id

def eql?(other)
self == other
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def head_line
# Not all formats have a `body` - some have their content split over
# multiple fields. In this case we'll skip the `articleBody` field
def body
return {} unless page.body.present?
return {} if page.body.blank?

{
"articleBody" => page.body,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def structured_data
def related_services
related_links = page.content_item.dig("links", "ordered_related_items")

return {} unless related_links.present?
return {} if related_links.blank?

{
"isRelatedTo" => related_links.each_with_object([]) do |link, items|
Expand All @@ -43,7 +43,7 @@ def related_services
def provider
organisations = page.content_item.dig("links", "organisations")

return {} unless organisations.present?
return {} if organisations.blank?

providers = organisations.map do |organisation|
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def structured_data
private

def members
return {} unless ministers.present?
return {} if ministers.blank?

members = ministers_with_grouped_roles.map do |person, roles|
person.merge("hasOccupation" => roles)
Expand Down Expand Up @@ -67,7 +67,7 @@ def person_schema_for_minister(minister)
end

def role_for_minister(minister)
return {} unless minister["role"].present?
return {} if minister["role"].blank?

{
"@type" => "Role",
Expand Down Expand Up @@ -108,7 +108,7 @@ def sub_organisations
def related_organisations(link_type, schema_name)
organisations = page.content_item.dig("links", link_type)

return {} unless organisations.present?
return {} if organisations.blank?

related_orgs = organisations.map do |org|
page = Page.new(content_item: org)
Expand Down
4 changes: 2 additions & 2 deletions lib/govuk_publishing_components/presenters/meta_tags.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ def add_core_tags(meta_tags)
meta_tags["govuk:updated-at"] = content_item[:updated_at] if content_item[:updated_at]
meta_tags["govuk:public-updated-at"] = content_item[:public_updated_at] if content_item[:public_updated_at]
primary_publisher = content_item.dig(:links, :primary_publishing_organisation)
primary_publisher = primary_publisher.first[:title] unless primary_publisher.blank?
meta_tags["govuk:primary-publishing-organisation"] = primary_publisher unless primary_publisher.blank?
primary_publisher = primary_publisher.first[:title] if primary_publisher.present?
meta_tags["govuk:primary-publishing-organisation"] = primary_publisher if primary_publisher.present?

# Some browse topics are nested in the content item, we want to grab these for GA4 tracking.
ga4_browse_topic = content_item.dig(:links, :ordered_related_items, 0, :links, :mainstream_browse_pages, 0, :links, :parent, 0, :title)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def current_step_nav
end

def active_step_nav_content_id
@active_step_nav_content_id ||= @query_parameters["step-by-step-nav"].present? ? @query_parameters["step-by-step-nav"] : nil
@active_step_nav_content_id ||= @query_parameters["step-by-step-nav"].presence
end

def steps
Expand Down
6 changes: 3 additions & 3 deletions spec/components/meta_tags_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,11 @@ def example_document_for(schema_name, example_name)

it "renders the has-content-history tag as true when the content has history" do
content_item = {
public_updated_at: Time.parse("2017-01-01"),
public_updated_at: Time.zone.parse("2017-01-01"),
details: {
first_public_at: Time.parse("2016-01-01"),
first_public_at: Time.zone.parse("2016-01-01"),
change_history: [
{ note: "test", public_timestamp: Time.parse("2016-01-01") },
{ note: "test", public_timestamp: Time.zone.parse("2016-01-01") },
],
},
}
Expand Down
Loading