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

Deprecate @payment_sources ivar in checkout controller #3128

Merged

Conversation

kennyadsl
Copy link
Member

Description

This method returns payment sources of the current user. It is no more used in our frontend. We used to assign the content of this method into an ivar (@payment_sources) into the checkout payment step. The new deprecated_payment_sources method is here only to be able to deprecate this ivar and will be removed with the ivar itself.

Trying to call this method in a view (for example @payment_sources.empty?) will produce this warning:

DEPRECATION WARNING: Please, do not use @payment_sources anymore, use @wallet_payment_sources instead. Args: [] (called from ...) 

Note

I had to introduce a custom class to deprecate instance variables passing a custom message otherwise we would have a misleading deprecation message:

DEPRECATION WARNING: @payment_sources is deprecated! Call deprecated_payment_sources.empty? instead of @payment_sources.empty?. Args: [] (called from ...)

I also opened a PR in Rails to improve this: rails/rails#35442

Checklist:

The default Rails class does not allow to pass a custom message to the
deprecation. This custom class extends its behavior to make it possible.

Note that I also opened a PR in Rails for fixing this: rails/rails#35442
This method returns payment sources of the current user. It is no more
used into our frontend. We used to assign the content of this method
into an ivar (@payment_sources) into the checkout payment step. This
method is here only to be able to deprecate this ivar and will be removed.
@kennyadsl kennyadsl self-assigned this Mar 1, 2019
@kennyadsl kennyadsl changed the title Kennyadsl/deprecate payment sources ivar Deprecate @payment_sources ivar in checkout controller Mar 1, 2019
Copy link
Contributor

@ericsaupe ericsaupe left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

Looks great Alberto. Good idea!

@kennyadsl kennyadsl merged commit 6578c9d into solidusio:master Mar 11, 2019
@kennyadsl kennyadsl deleted the kennyadsl/deprecate-payment_sources-ivar branch March 11, 2019 16:09
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.

3 participants