From 39b236cc33864d37966a63556c71959414ec39dc Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Wed, 19 Jun 2024 10:13:17 +0000 Subject: [PATCH 1/6] Remove unused `IconHelper` --- app/helpers/icon_helper.rb | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 app/helpers/icon_helper.rb diff --git a/app/helpers/icon_helper.rb b/app/helpers/icon_helper.rb deleted file mode 100644 index 60b848af..00000000 --- a/app/helpers/icon_helper.rb +++ /dev/null @@ -1,6 +0,0 @@ -module IconHelper - # http://getbootstrap.com/components/#glyphicons-glyphs - def icon(icon_name) - tag.i "", class: "glyphicon glyphicon-#{icon_name}" - end -end From 31380b4dea23178cc08eded43ca36a456dde7098 Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Wed, 19 Jun 2024 10:19:02 +0000 Subject: [PATCH 2/6] Simplify navigation check for current section --- app/helpers/application_helper.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 8b9922bc..3304a221 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -6,7 +6,7 @@ def navigation_items { text: "External links", href: recommended_links_path, - active: is_current?(recommended_links_path), + active: controller.controller_name == "recommended_links", }, { text: current_user.name, @@ -18,10 +18,4 @@ def navigation_items }, ] end - - def is_current?(link) - recognized = Rails.application.routes.recognize_path(link) - recognized[:controller] == params[:controller] && - recognized[:action] == params[:action] - end end From c64399a9b43c0b04e76375a0bdde8e2a7613052a Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Wed, 19 Jun 2024 10:19:34 +0000 Subject: [PATCH 3/6] Remove unused `BetsMailer` --- app/mailers/bets_mailer.rb | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 app/mailers/bets_mailer.rb diff --git a/app/mailers/bets_mailer.rb b/app/mailers/bets_mailer.rb deleted file mode 100644 index 69dc1e1e..00000000 --- a/app/mailers/bets_mailer.rb +++ /dev/null @@ -1,17 +0,0 @@ -class BetsMailer < ApplicationMailer - def expiring_bets_list(address, bets) - return if bets.empty? - - @when = bets.first.expiration_date - @grouped_bets = bets.each_with_object({}) do |bet, grouped| - grouped[bet.query.display_name] ||= [] - grouped[bet.query.display_name] << bet.link - end - - view_mail( - template_id, - to: address, - subject: "Best and worst bets expiring on #{@when.strftime('%d %b %Y')}", - ) - end -end From ebb30638ac117a55c6669c91604d669a07b9bdef Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Wed, 19 Jun 2024 13:20:42 +0000 Subject: [PATCH 4/6] Properly localise `FormsHelper` helpers These should use `full_messages` to get the field names from i18n, rather than just humanizing the field name. --- app/helpers/forms_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/forms_helper.rb b/app/helpers/forms_helper.rb index 7dac81df..fe62f90b 100644 --- a/app/helpers/forms_helper.rb +++ b/app/helpers/forms_helper.rb @@ -2,15 +2,15 @@ module FormsHelper def error_items(record, field) return unless record.errors[field].any? - record.errors[field].map do |error| - { text: "#{field.to_s.humanize} #{error}" } + record.errors.full_messages_for(field).map do |error| + { text: error } end end def error_summary_items(record) record.errors.map do |error| { - text: "#{error.attribute.to_s.humanize} #{error.message}", + text: error.full_message, href: "##{record.class.name.underscore}_#{error.attribute}", } end From f47fa92672215ab48efe110e51fc663afbaa28d1 Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Wed, 19 Jun 2024 13:59:52 +0000 Subject: [PATCH 5/6] Remove factories for removed models --- spec/factories.rb | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index ca78767c..169b70b0 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -1,39 +1,4 @@ FactoryBot.define do - sequence :numeric_position do |n| - n - end - - factory :bet do - link { "/death-and-taxes" } - expiration_date { Time.zone.now + 1.day } - - trait(:worst) do - is_best { false } - end - - trait(:best) do - is_best { true } - position { generate :numeric_position } - end - - trait(:permanent) do - permanent { true } - end - - user - end - - factory :query do - query { "tax" } - match_type { "exact" } - - trait :with_best_bet do - after(:create) do |query| - create(:bet, :best, query:) - end - end - end - factory :recommended_link do title { "Tax online" } link { "https://www.tax.service.gov.uk/" } From 919e0eb06e3f8ce9dcb7f7ac54bb6bb2ffb0db4f Mon Sep 17 00:00:00 2001 From: Christian Sutter Date: Wed, 19 Jun 2024 14:10:14 +0000 Subject: [PATCH 6/6] Move creation of test user to global before/after hook --- spec/spec_helper.rb | 9 +++++++++ spec/system/recommended_links_spec.rb | 4 ---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4a08dc51..976ac090 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -31,6 +31,15 @@ config.include FactoryBot::Syntax::Methods + config.before(:all, type: :system) do + @user = create(:user) + GDS::SSO.test_user = @user + end + + config.after(:all, type: :system) do + @user.destroy! + end + config.before(:each, type: :system) do driven_by :rack_test end diff --git a/spec/system/recommended_links_spec.rb b/spec/system/recommended_links_spec.rb index ade03e89..a2375cd1 100644 --- a/spec/system/recommended_links_spec.rb +++ b/spec/system/recommended_links_spec.rb @@ -1,9 +1,5 @@ RSpec.describe "Recommended links" do before do - # TODO: Refactor me out into a support file (once we have more system specs and old - # controller specs are gone that this might conflict with) - GDS::SSO.test_user = create(:user) - stub_external_content_publisher end