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

Fix deprecation of active payment methods #4414

Merged

Conversation

waiting-for-dev
Copy link
Contributor

@waiting-for-dev waiting-for-dev commented Jun 14, 2022

PR #3837 added a rake task to deactivate payment method records that are
no longer valid because of removing a payment method class.

The task iterates over all payment method records and tries to
constantize from its polymorphic type column. When that process returns
a NameError, it assumes that the class is no longer present and
proceeds to deactivate the record.

Before this commit, ActiveSupport::Dependencies.constantize was used.
However, the method was removed on Rails 7 [1], so our code raised a
NoMethodError. As NoMethodError is a subclass of NameError [2],
the deactivation logic was called for every record.

As Rails recommends, we update the code to use String#constantize
instead.

[1] - https://edgeguides.rubyonrails.org/upgrading_ruby_on_rails.html#activesupport-dependencies-private-api-has-been-deleted
[2] - https://ruby-doc.org/core-3.1.0/NoMethodError.html

This fixes CI on master.

cc @luca-landa.

PR solidusio#3837 added a rake task to deactivate payment method records that are
no longer valid because of removing a payment method class.

The task iterates over all payment method records and tries to
constantize from its polymorphic type column. When that process returns
a `NameError`, it assumes that the class is no longer present and
proceeds to deactivate the record.

Before this commit, `ActiveSupport::Dependencies.constantize` was used.
However, the method was removed on Rails 7 [1], so our code raised a
`NoMethodError`. As `NoMethodError` is a subclass of `NameError` [2],
the deactivation logic was called for every record.

As Rails recommends, we update the code to use `String#constantize`
instead.

[1] - https://edgeguides.rubyonrails.org/upgrading_ruby_on_rails.html#activesupport-dependencies-private-api-has-been-deleted
[2] - https://ruby-doc.org/core-3.1.0/NoMethodError.html
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Marc!

Copy link
Member

@jarednorman jarednorman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏🏻

@waiting-for-dev
Copy link
Contributor Author

I'm going to merge this one even if CI is failing. The failure is not because of a failing test but because of some obscure problem. It's unrelated to this issue. In fact, I pushed another branch reverting the deprecation of payment methods, and we got the same error. As this PR fixes an actual test failure, it'll be easier to debug the other one if this is merged.

@waiting-for-dev waiting-for-dev merged commit aa77140 into solidusio:master Jun 17, 2022
@waiting-for-dev waiting-for-dev deleted the waiting-for-dev/fix_master branch June 17, 2022 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants