From a3066ed75e1f5fbd4e25aa9eca0961c96f69ad04 Mon Sep 17 00:00:00 2001 From: Massimiliano Lattanzio Date: Thu, 2 Nov 2023 09:26:32 +0100 Subject: [PATCH 1/5] Create new cop to check existing_card_id --- ...reate_new_cop_to_check_existing_card_id.md | 1 + config/default.yml | 6 +++ .../solidus/existing_card_id_deprecated.rb | 40 +++++++++++++++++++ lib/rubocop/cop/solidus_cops.rb | 1 + .../existing_card_id_deprecated_spec.rb | 16 ++++++++ 5 files changed, 64 insertions(+) create mode 100644 changelog/new_create_new_cop_to_check_existing_card_id.md create mode 100644 lib/rubocop/cop/solidus/existing_card_id_deprecated.rb create mode 100644 spec/rubocop/cop/solidus/existing_card_id_deprecated_spec.rb diff --git a/changelog/new_create_new_cop_to_check_existing_card_id.md b/changelog/new_create_new_cop_to_check_existing_card_id.md new file mode 100644 index 0000000..793ed3a --- /dev/null +++ b/changelog/new_create_new_cop_to_check_existing_card_id.md @@ -0,0 +1 @@ +* [#60](https://github.com/solidusio/rubocop-solidus/issues/60): Create new cop to check existing_card_id. ([@MassimilianoLattanzio][]) diff --git a/config/default.yml b/config/default.yml index 205d9df..1916184 100644 --- a/config/default.yml +++ b/config/default.yml @@ -14,6 +14,12 @@ Solidus/ClassEvalDecorator: VersionAdded: '0.1.0' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/21' +Solidus/ExistingCardIdDeprecated: + Description: 'Checks if existing_card_id is being used and suggest using wallet_payment_source_id instead' + Enabled: true + VersionAdded: '<>' + Reference: 'https://github.com/solidusio/rubocop-solidus/issues/60' + Solidus/ReimbursementHookDeprecated: Description: 'Checks if reimbursement_success_hooks and reimbursement_failed_hooks is being used' Enabled: true diff --git a/lib/rubocop/cop/solidus/existing_card_id_deprecated.rb b/lib/rubocop/cop/solidus/existing_card_id_deprecated.rb new file mode 100644 index 0000000..24d4412 --- /dev/null +++ b/lib/rubocop/cop/solidus/existing_card_id_deprecated.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Solidus + # This cop finds existing_card_id occurrences and suggest using wallet_payment_source_id instead. + # + # @example + # + # # bad + # { + # name: payment_method.name, + # existing_card_id: payment_source.id + # } + # + # # good + # { + # name: payment_method.name, + # wallet_payment_source_id: payment_source.wallet.wallet_payment_sources.first.id + # } + # + class ExistingCardIdDeprecated < Base + include TargetSolidusVersion + minimum_solidus_version 2.2 + + MSG = 'Use `wallet_payment_source_id` instead of `existing_card_id`.' + + def_node_matcher :existing_card_id?, <<~PATTERN + (send ... :existing_card_id) + PATTERN + + def on_send(node) + return unless existing_card_id?(node) + + add_offense(node, severity: :warning) + end + end + end + end +end diff --git a/lib/rubocop/cop/solidus_cops.rb b/lib/rubocop/cop/solidus_cops.rb index 36d5e75..e9b7a70 100644 --- a/lib/rubocop/cop/solidus_cops.rb +++ b/lib/rubocop/cop/solidus_cops.rb @@ -3,6 +3,7 @@ require_relative 'mixin/target_solidus_version' require_relative 'solidus/class_eval_decorator' +require_relative 'solidus/existing_card_id_deprecated' require_relative 'solidus/reimbursement_hook_deprecated' require_relative 'solidus/spree_calculator_free_shipping_deprecated' require_relative 'solidus/spree_calculator_percent_per_item_deprecated' diff --git a/spec/rubocop/cop/solidus/existing_card_id_deprecated_spec.rb b/spec/rubocop/cop/solidus/existing_card_id_deprecated_spec.rb new file mode 100644 index 0000000..aa2e16f --- /dev/null +++ b/spec/rubocop/cop/solidus/existing_card_id_deprecated_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Solidus::ExistingCardIdDeprecated, :config do + it 'registers an offense when using `existing_card_id`' do + expect_offense(<<~RUBY, severity: :warning) + existing_card_id + ^^^^^^^^^^^^^^^^ Use `wallet_payment_source_id` instead of `existing_card_id`. + RUBY + end + + it 'does not register an offense when using `wallet_payment_source_id`' do + expect_no_offenses(<<~RUBY) + wallet_payment_source_id + RUBY + end +end From b8400cc871d3cc2b42d202752c2139554af9acd7 Mon Sep 17 00:00:00 2001 From: Massimiliano Lattanzio Date: Thu, 2 Nov 2023 10:10:10 +0100 Subject: [PATCH 2/5] Fix overridden add_offense method In d525445, we updated the `add_offense` method to let it work with older ruby versions. Everything worked fine until we started passing other params to the method, such as the severity, which doesn't work in Ruby 2.7 because the behavior is incorrect. Our way of calling the super method was introduced in Ruby 2.7, so we need to start using the new way from the correct version. --- changelog/fix_fix_overridden_add_offense_method.md | 1 + lib/rubocop/cop/mixin/target_solidus_version.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog/fix_fix_overridden_add_offense_method.md diff --git a/changelog/fix_fix_overridden_add_offense_method.md b/changelog/fix_fix_overridden_add_offense_method.md new file mode 100644 index 0000000..8a6dea1 --- /dev/null +++ b/changelog/fix_fix_overridden_add_offense_method.md @@ -0,0 +1 @@ +* [#60](https://github.com/solidusio/rubocop-solidus/issues/60): Fix overridden add_offense method. ([@MassimilianoLattanzio][]) diff --git a/lib/rubocop/cop/mixin/target_solidus_version.rb b/lib/rubocop/cop/mixin/target_solidus_version.rb index fa3966f..7fcb8f9 100644 --- a/lib/rubocop/cop/mixin/target_solidus_version.rb +++ b/lib/rubocop/cop/mixin/target_solidus_version.rb @@ -36,7 +36,7 @@ def required_minimum_solidus_version def add_offense(*args, **kwargs, &block) return unless affected_solidus_version? - if Gem::Version.new(RUBY_VERSION) > Gem::Version.new('3') + if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.7') super(*args, **kwargs, &block) else super(*args, &block) From 4fd6238af27417f35c66917bd002f3c448e2a5f9 Mon Sep 17 00:00:00 2001 From: Massimiliano Lattanzio Date: Thu, 2 Nov 2023 10:24:27 +0100 Subject: [PATCH 3/5] Update Changelog --- CHANGELOG.md | 8 ++++++++ changelog/fix_fix_overridden_add_offense_method.md | 1 - changelog/new_create_new_cop_to_check_existing_card_id.md | 1 - 3 files changed, 8 insertions(+), 2 deletions(-) delete mode 100644 changelog/fix_fix_overridden_add_offense_method.md delete mode 100644 changelog/new_create_new_cop_to_check_existing_card_id.md diff --git a/CHANGELOG.md b/CHANGELOG.md index bf043b4..bcb04e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ ## main (unreleased) +### New features + +* [#60](https://github.com/solidusio/rubocop-solidus/issues/60): Create new cop to check existing_card_id. ([@MassimilianoLattanzio][]) + +### Bug fixes + +* [#60](https://github.com/solidusio/rubocop-solidus/issues/60): Fix overridden add_offense method. ([@MassimilianoLattanzio][]) + ## 0.1.4 (2023-08-04) ### New features diff --git a/changelog/fix_fix_overridden_add_offense_method.md b/changelog/fix_fix_overridden_add_offense_method.md deleted file mode 100644 index 8a6dea1..0000000 --- a/changelog/fix_fix_overridden_add_offense_method.md +++ /dev/null @@ -1 +0,0 @@ -* [#60](https://github.com/solidusio/rubocop-solidus/issues/60): Fix overridden add_offense method. ([@MassimilianoLattanzio][]) diff --git a/changelog/new_create_new_cop_to_check_existing_card_id.md b/changelog/new_create_new_cop_to_check_existing_card_id.md deleted file mode 100644 index 793ed3a..0000000 --- a/changelog/new_create_new_cop_to_check_existing_card_id.md +++ /dev/null @@ -1 +0,0 @@ -* [#60](https://github.com/solidusio/rubocop-solidus/issues/60): Create new cop to check existing_card_id. ([@MassimilianoLattanzio][]) From 6504154debf95809d3b98e041407482c4ac61b24 Mon Sep 17 00:00:00 2001 From: Massimiliano Lattanzio Date: Thu, 2 Nov 2023 10:27:59 +0100 Subject: [PATCH 4/5] Create new version --- CHANGELOG.md | 2 ++ Gemfile.lock | 2 +- config/default.yml | 22 +++++++++++----------- lib/rubocop/solidus/version.rb | 2 +- relnotes/v0.2.0.md | 9 +++++++++ 5 files changed, 24 insertions(+), 13 deletions(-) create mode 100644 relnotes/v0.2.0.md diff --git a/CHANGELOG.md b/CHANGELOG.md index bcb04e1..7fb916c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## main (unreleased) +## 0.2.0 (2023-11-02) + ### New features * [#60](https://github.com/solidusio/rubocop-solidus/issues/60): Create new cop to check existing_card_id. ([@MassimilianoLattanzio][]) diff --git a/Gemfile.lock b/Gemfile.lock index e328b4e..e6883b0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - rubocop-solidus (0.1.4) + rubocop-solidus (0.2.0) rubocop GEM diff --git a/config/default.yml b/config/default.yml index 1916184..fa486bd 100644 --- a/config/default.yml +++ b/config/default.yml @@ -11,65 +11,65 @@ AllCops: Solidus/ClassEvalDecorator: Description: 'Checks if Class.class_eval is being used in the code' Enabled: true - VersionAdded: '0.1.0' + VersionAdded: '0.1' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/21' Solidus/ExistingCardIdDeprecated: Description: 'Checks if existing_card_id is being used and suggest using wallet_payment_source_id instead' Enabled: true - VersionAdded: '<>' + VersionAdded: '0.2' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/60' Solidus/ReimbursementHookDeprecated: Description: 'Checks if reimbursement_success_hooks and reimbursement_failed_hooks is being used' Enabled: true - VersionAdded: '0.1.0' + VersionAdded: '0.1' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/27' Solidus/SpreeCalculatorFreeShippingDeprecated: Description: 'Checks if Spree::Calculator::FreeShipping is being used and add deprecation message' Enabled: true - VersionAdded: '0.1.0' + VersionAdded: '0.1' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/29' Solidus/SpreeCalculatorPercentPerItemDeprecated: Description: 'Checks if Spree::Calculator::PercentPerItem is being used and add deprecation message' Enabled: true - VersionAdded: '0.1.0' + VersionAdded: '0.1' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/29' Solidus/SpreeCalculatorPriceSackDeprecated: Description: 'Checks if Spree::Calculator::PriceSack is being used and add deprecation message' Enabled: true - VersionAdded: '0.1.0' + VersionAdded: '0.1' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/29' Solidus/SpreeDefaultCreditCardDeprecated: Description: 'Checks if user.default_credit_card is used and suggest using user.wallet.default_wallet_payment_source' Enabled: true - VersionAdded: '0.1.0' + VersionAdded: '0.1' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/33' Solidus/SpreeGatewayBogusDeprecated: Description: 'Checks if SpreeGatewayBogus is being used and replaces it with Spree::PaymentMethod::BogusCreditCard' Enabled: true - VersionAdded: '0.1.0' + VersionAdded: '0.1' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/26' Solidus/SpreeIconDeprecated: Description: 'Checks if icon helper is being used and suggest `solidus_icon`' Enabled: true - VersionAdded: '0.1.0' + VersionAdded: '0.1' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/32' Solidus/SpreeRefundCallPerform: Description: 'Checks if Spree::Refund.create is being used and require calling .perform!' Enabled: true - VersionAdded: '0.1.0' + VersionAdded: '0.1' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/28' Solidus/SpreeTDeprecated: Description: 'Checks if Spree.t is being used and replaces it with I18n.t.' Enabled: true - VersionAdded: '0.1.0' + VersionAdded: '0.1' Reference: 'https://github.com/solidusio/rubocop-solidus/issues/22' diff --git a/lib/rubocop/solidus/version.rb b/lib/rubocop/solidus/version.rb index c9930a9..b172f02 100644 --- a/lib/rubocop/solidus/version.rb +++ b/lib/rubocop/solidus/version.rb @@ -2,6 +2,6 @@ module RuboCop module Solidus - VERSION = '0.1.4' + VERSION = '0.2.0' end end diff --git a/relnotes/v0.2.0.md b/relnotes/v0.2.0.md new file mode 100644 index 0000000..bcde3b2 --- /dev/null +++ b/relnotes/v0.2.0.md @@ -0,0 +1,9 @@ +### New features + +* [#60](https://github.com/solidusio/rubocop-solidus/issues/60): Create new cop to check existing_card_id. ([@MassimilianoLattanzio][]) + +### Bug fixes + +* [#60](https://github.com/solidusio/rubocop-solidus/issues/60): Fix overridden add_offense method. ([@MassimilianoLattanzio][]) + +[@MassimilianoLattanzio]: https://github.com/MassimilianoLattanzio From 50a2e02c08eade141c159709035154677cc6c232 Mon Sep 17 00:00:00 2001 From: Massimiliano Lattanzio Date: Thu, 2 Nov 2023 10:29:52 +0100 Subject: [PATCH 5/5] Update docs --- docs/cops.md | 1 + docs/cops_solidus.md | 48 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/docs/cops.md b/docs/cops.md index c7b14ed..b29b3d9 100644 --- a/docs/cops.md +++ b/docs/cops.md @@ -6,6 +6,7 @@ In the following section you find all available cops: #### Department [Solidus](cops_solidus.md) * [Solidus/ClassEvalDecorator](cops_solidus.md#solidusclassevaldecorator) +* [Solidus/ExistingCardIdDeprecated](cops_solidus.md#solidusexistingcardiddeprecated) * [Solidus/ReimbursementHookDeprecated](cops_solidus.md#solidusreimbursementhookdeprecated) * [Solidus/SpreeCalculatorFreeShippingDeprecated](cops_solidus.md#solidusspreecalculatorfreeshippingdeprecated) * [Solidus/SpreeCalculatorPercentPerItemDeprecated](cops_solidus.md#solidusspreecalculatorpercentperitemdeprecated) diff --git a/docs/cops_solidus.md b/docs/cops_solidus.md index d191eb8..3a836fe 100644 --- a/docs/cops_solidus.md +++ b/docs/cops_solidus.md @@ -4,7 +4,7 @@ Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version --- | --- | --- | --- | --- | --- -Enabled | Yes | No | 0.1.0 | - | - +Enabled | Yes | No | 0.1 | - | - Solidus suggests a decorator module instead of `class_eval` when overriding some features. This cop finds any `class_eval` and asks to use a decorator module instead. @@ -32,11 +32,39 @@ end * [https://github.com/solidusio/rubocop-solidus/issues/21](https://github.com/solidusio/rubocop-solidus/issues/21) +## Solidus/ExistingCardIdDeprecated + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version +--- | --- | --- | --- | --- | --- +Enabled | Yes | No | 0.2 | - | 2.2 + +This cop finds existing_card_id occurrences and suggest using wallet_payment_source_id instead. + +### Examples + +```ruby +# bad +{ + name: payment_method.name, + existing_card_id: payment_source.id +} + +# good +{ + name: payment_method.name, + wallet_payment_source_id: payment_source.wallet.wallet_payment_sources.first.id +} +``` + +### References + +* [https://github.com/solidusio/rubocop-solidus/issues/60](https://github.com/solidusio/rubocop-solidus/issues/60) + ## Solidus/ReimbursementHookDeprecated Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version --- | --- | --- | --- | --- | --- -Enabled | Yes | No | 0.1.0 | - | 2.11 +Enabled | Yes | No | 0.1 | - | 2.11 This cop finds reimbursement_success_hooks and reimbursement_failed_hooks calls and asks to remove them and subscribe to reimbursement_reimbursed event instead. @@ -66,7 +94,7 @@ reimbursement_failed_hooks.any? Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version --- | --- | --- | --- | --- | --- -Enabled | Yes | No | 0.1.0 | - | - +Enabled | Yes | No | 0.1 | - | - This cop finds Spree::Calculator::FreeShipping calls. This cop is needed as they have been deprecated in future version. @@ -88,7 +116,7 @@ Spree::Calculator::FreeShipping Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version --- | --- | --- | --- | --- | --- -Enabled | Yes | Yes | 0.1.0 | - | - +Enabled | Yes | Yes | 0.1 | - | - This cop finds Spree::Calculator::PercentPerItem calls. This cop is needed as they have been deprecated in future version. @@ -111,7 +139,7 @@ Spree::Calculator::PercentOnLineItem Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version --- | --- | --- | --- | --- | --- -Enabled | Yes | No | 0.1.0 | - | - +Enabled | Yes | No | 0.1 | - | - This cop finds Spree::Calculator::PriceSack calls. This cop is needed as they have been deprecated in future version. @@ -133,7 +161,7 @@ Spree::Calculator::PriceSack Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version --- | --- | --- | --- | --- | --- -Enabled | Yes | Yes | 0.1.0 | - | 2.2 +Enabled | Yes | Yes | 0.1 | - | 2.2 This cop finds user.default_credit_card suggest using user.wallet.default_wallet_payment_source. @@ -155,7 +183,7 @@ user.wallet.default_wallet_payment_source Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version --- | --- | --- | --- | --- | --- -Enabled | Yes | Yes | 0.1.0 | - | 2.1 +Enabled | Yes | Yes | 0.1 | - | 2.1 This cop finds Spree::Gateway::Bogus calls and replaces them with the Spree::PaymentMethod::BogusCreditCard. This cop is needed as the Spree::Gateway::Bogus has been deprecated in future version. @@ -182,7 +210,7 @@ Spree::PaymentMethod::BogusCreditCard.create! Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version --- | --- | --- | --- | --- | --- -Enabled | Yes | Yes | 0.1.0 | - | 2.3 +Enabled | Yes | Yes | 0.1 | - | 2.3 This cop finds icon helper calls and suggest using solidus_icon. @@ -204,7 +232,7 @@ helper.solidus_icon('example') Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version --- | --- | --- | --- | --- | --- -Enabled | Yes | No | 0.1.0 | - | 2.11 +Enabled | Yes | No | 0.1 | - | 2.11 This cop finds Spree::Refund.create(your: attributes) calls and replaces them with the Spree::Refund.create(your: attributes, perform_after_create: false).perform! call. @@ -227,7 +255,7 @@ Spree::Refund.create(your: attributes, perform_after_create: false).perform! Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Required Solidus Version --- | --- | --- | --- | --- | --- -Enabled | Yes | Yes | 0.1.0 | - | - +Enabled | Yes | Yes | 0.1 | - | - This cop finds Spree.t method calls and replaces them with the I18n,t method call. This cop is needed as the Spree.t version has been deprecated in future version.