From 446b9488898be157420e6899316c4d20409d2a34 Mon Sep 17 00:00:00 2001 From: cyrillefr Date: Mon, 3 Mar 2025 14:03:47 +0100 Subject: [PATCH 1/4] Fixes some rubocop linting offenses - part II --- .rubocop_todo.yml | 18 ------------------ .../payment_gateways/stripe_controller.rb | 10 +++++----- app/models/spree/user.rb | 1 + engines/catalog/spec/spec_helper.rb | 2 -- engines/dfc_provider/spec/spec_helper.rb | 2 +- spec/system_helper.rb | 2 +- 6 files changed, 8 insertions(+), 27 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 5196330f3f..28e32673a1 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -39,24 +39,6 @@ Lint/FloatComparison: Exclude: - 'app/models/spree/gateway/pay_pal_express.rb' -# Offense count: 1 -Lint/IneffectiveAccessModifier: - Exclude: - - 'app/models/spree/user.rb' - -# Offense count: 1 -Lint/NoReturnInBeginEndBlocks: - Exclude: - - 'app/controllers/payment_gateways/stripe_controller.rb' - -# Offense count: 3 -# This cop supports unsafe autocorrection (--autocorrect-all). -Lint/RedundantDirGlobSort: - Exclude: - - 'engines/catalog/spec/spec_helper.rb' - - 'engines/dfc_provider/spec/spec_helper.rb' - - 'spec/system_helper.rb' - # Offense count: 24 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes, Max. Metrics/AbcSize: diff --git a/app/controllers/payment_gateways/stripe_controller.rb b/app/controllers/payment_gateways/stripe_controller.rb index ff88831324..b49ac05e46 100644 --- a/app/controllers/payment_gateways/stripe_controller.rb +++ b/app/controllers/payment_gateways/stripe_controller.rb @@ -68,11 +68,11 @@ module PaymentGateways end def valid_payment_intent? - @valid_payment_intent ||= begin - return false unless params["payment_intent"]&.starts_with?("pi_") - - order_and_payment_valid? - end + @valid_payment_intent ||= if params["payment_intent"]&.starts_with?("pi_") + order_and_payment_valid? + else + false + end end def order_and_payment_valid? diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 72f2a3c80a..5b867bcd00 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -168,6 +168,7 @@ module Spree def self.friendly_token SecureRandom.base64(15).tr('+/=', '-_ ').strip.delete("\n") end + private_class_method :friendly_token def limit_owned_enterprises return unless owned_enterprises.size > enterprise_limit diff --git a/engines/catalog/spec/spec_helper.rb b/engines/catalog/spec/spec_helper.rb index f6b5d7710e..183f65d422 100644 --- a/engines/catalog/spec/spec_helper.rb +++ b/engines/catalog/spec/spec_helper.rb @@ -1,5 +1,3 @@ # frozen_string_literal: true require '../../spec/spec_helper' - -Dir["#{File.dirname(__FILE__)}/support/**/*.rb"].sort.each { |f| require f } diff --git a/engines/dfc_provider/spec/spec_helper.rb b/engines/dfc_provider/spec/spec_helper.rb index 3e363b392e..9ddcac95d1 100644 --- a/engines/dfc_provider/spec/spec_helper.rb +++ b/engines/dfc_provider/spec/spec_helper.rb @@ -2,7 +2,7 @@ require_relative '../../../spec/spec_helper' -Dir["#{File.dirname(__FILE__)}/support/**/*.rb"].sort.each { |f| require f } +Dir["#{File.dirname(__FILE__)}/support/**/*.rb"].each { |f| require f } RSpec.configure do |config| config.include AuthorizationHelper, type: :request diff --git a/spec/system_helper.rb b/spec/system_helper.rb index 4af20f4f56..2acb7f6534 100644 --- a/spec/system_helper.rb +++ b/spec/system_helper.rb @@ -3,4 +3,4 @@ require "base_spec_helper" # system/support/ files contain system tests configurations and helpers -Dir[File.join(__dir__, "system/support/**/*.rb")].sort.each { |file| require file } +Dir[File.join(__dir__, "system/support/**/*.rb")].each { |file| require file } From 633cdaca56671ae9880f4b9e6e002af57a142d57 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 6 Mar 2025 09:49:50 +1100 Subject: [PATCH 2/4] Remove unused method I believe it's unused since e4d307fe5efe88357b9a1e83796cf2941a7a3fdb --- app/models/spree/user.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 5b867bcd00..2b7cc79397 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -165,10 +165,6 @@ module Spree end # Generate a friendly string randomically to be used as token. - def self.friendly_token - SecureRandom.base64(15).tr('+/=', '-_ ').strip.delete("\n") - end - private_class_method :friendly_token def limit_owned_enterprises return unless owned_enterprises.size > enterprise_limit From 45eedd1e897eab315b95c6710413faf6d08c0211 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 6 Mar 2025 10:04:21 +1100 Subject: [PATCH 3/4] Simplify expresssion It's worth noting that a falsy value won't get cached (as before). But I didn't take the time to investigate if that makes a difference or not. --- app/controllers/payment_gateways/stripe_controller.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/app/controllers/payment_gateways/stripe_controller.rb b/app/controllers/payment_gateways/stripe_controller.rb index b49ac05e46..37167a636d 100644 --- a/app/controllers/payment_gateways/stripe_controller.rb +++ b/app/controllers/payment_gateways/stripe_controller.rb @@ -68,11 +68,8 @@ module PaymentGateways end def valid_payment_intent? - @valid_payment_intent ||= if params["payment_intent"]&.starts_with?("pi_") - order_and_payment_valid? - else - false - end + @valid_payment_intent ||= params["payment_intent"]&.starts_with?("pi_") && + order_and_payment_valid? end def order_and_payment_valid? From 2e4b59daaf5a395c909ef1160415e75b8957d4bd Mon Sep 17 00:00:00 2001 From: Maikel Date: Thu, 6 Mar 2025 11:28:25 +1100 Subject: [PATCH 4/4] Remove orphaned comment --- app/models/spree/user.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 2b7cc79397..0bbe9d60f2 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -164,8 +164,6 @@ module Spree self.login ||= email if email end - # Generate a friendly string randomically to be used as token. - def limit_owned_enterprises return unless owned_enterprises.size > enterprise_limit