Introduce deprecation warning when using Spree::Refund#perform! callback
#3181
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit introduces a deprecation warning for Solidus-based stores expecting `Spree::Refund#perform!` to be automatically executed Neither Ruby nor Rails include a built-in way to define accessors with default values; here, we define a method and a writer accessor with the same name, so we can set a value or fallback to a default one Serves as starting point for solidusio#1415
|
Thanks for this change! I think we should submit this change with #1415 commits as well since we would otherwise ship a deprecation warning without the code that does the "new thing". WDYT? |
|
@kennyadsl, sounds good to me. Do you want me to rebase that PR and then add these commits or cherry pick them into this branch? |
|
I think we can use this PR cherry-picking and close that one, thanks! |
|
@kennyadsl, I noticed something while re-reading your first comment: if we include the changes proposed in #1415 alongside this PR, we'd render this warning useless as the callback would be removed prior to Solidus v3.0 (which I assume is not the intended) and we would not be giving the developers any previous notice that the What do you think? |
|
I think that what we have to do is:
In the next version released, we can safely deprecate Does it make sense? |
|
@kennyadsl, yes, that makes sense, but my concern is focused on the stores that rely on that callback to be performed automatically —would that change break their stores if we don't give them a previous warning? |
|
If they don't set |
|
No, the |
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
May 26, 2020
We can't just stop calling this callback or we are going to break applications when it's defined code like: Spree::Refund.create(attrs: ...) since it currently relies on also calling perform! as well. With this code, the old behavior is still there but we are asking users to update their code to transition to the new behavior, which is something like: Spree::Refund.create(attrs: ..., perform_after_creation: false).perform! The two extra callbacks are needed to: - set_perform_after_creation_default: prints the deprecation message only when creating the instance, otherwise it will be printed also when calling perform! on a good instance. Also, it sets the deafult to true when the attribute is not passed, which means that code has not been updated. - clear_perform_after_creation: this callback is needed to clean the instance, after this game ends, otherwise each call of perform! after the callbacks execution could not be executed if the instance was created with perform_after_creations: false. Specs needed some seriuos refactor to reflect this new architecture but they look cleaner now. Co-Authoring the commit since the initial code has been taken from solidusio#3181. Co-authored-by: Angel Perez <[email protected]>
4 tasks
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
May 27, 2020
We can't just stop calling this callback or we are going to break applications when it's defined code like: Spree::Refund.create(attrs: ...) since it currently relies on also calling perform! as well. With this code, the old behavior is still there but we are asking users to update their code to transition to the new behavior, which is something like: Spree::Refund.create(attrs: ..., perform_after_creation: false).perform! The two extra callbacks are needed to: - set_perform_after_creation_default: prints the deprecation message only when creating the instance, otherwise it will be printed also when calling perform! on a good instance. Also, it sets the deafult to true when the attribute is not passed, which means that code has not been updated. - clear_perform_after_creation: this callback is needed to clean the instance, after this game ends, otherwise each call of perform! after the callbacks execution could not be executed if the instance was created with perform_after_creations: false. Specs needed some seriuos refactor to reflect this new architecture but they look cleaner now. Co-Authoring the commit since the initial code has been taken from solidusio#3181. Co-authored-by: Angel Perez <[email protected]>
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
May 29, 2020
We can't just stop calling this callback or we are going to break applications when it's defined code like: Spree::Refund.create(attrs: ...) since it currently relies on also calling perform! as well. With this code, the old behavior is still there but we are asking users to update their code to transition to the new behavior, which is something like: Spree::Refund.create(attrs: ..., perform_after_create: false).perform! The two extra callbacks are needed to: - set_perform_after_create_default: prints the deprecation message only when creating the instance, otherwise it will be printed also when calling perform! on a good instance. Also, it sets the deafult to true when the attribute is not passed, which means that code has not been updated yet. - clear_perform_after_create: this callback is needed to clean the instance, after this process ends, otherwise each call of perform! after the callbacks execution could not be executed if the instance was created with perform_after_create: false. Specs needed some seriuos refactor to reflect this new architecture but they look cleaner now. Co-Authoring the commit since the initial code has been taken from solidusio#3181. Co-authored-by: Angel Perez <[email protected]>
kennyadsl
added a commit
to nebulab/solidus
that referenced
this pull request
May 29, 2020
We can't just stop calling this callback or we are going to break applications when it's defined code like: Spree::Refund.create(attrs: ...) since it currently relies on also calling perform! as well. With this code, the old behavior is still there but we are asking users to update their code to transition to the new behavior, which is something like: Spree::Refund.create(attrs: ..., perform_after_create: false).perform! The two extra callbacks are needed to: - set_perform_after_create_default: prints the deprecation message only when creating the instance, otherwise it will be printed also when calling perform! on a good instance. Also, it sets the deafult to true when the attribute is not passed, which means that code has not been updated yet. - clear_perform_after_create: this callback is needed to clean the instance, after this process ends, otherwise each call of perform! after the callbacks execution could not be executed if the instance was created with perform_after_create: false. Specs needed some seriuos refactor to reflect this new architecture but they look cleaner now. Co-Authoring the commit since the initial code has been taken from solidusio#3181. Co-authored-by: Angel Perez <[email protected]>
|
Closed in favor of #3641 |
aldesantis
pushed a commit
to aldesantis/solidus
that referenced
this pull request
Dec 28, 2020
We can't just stop calling this callback or we are going to break applications when it's defined code like: Spree::Refund.create(attrs: ...) since it currently relies on also calling perform! as well. With this code, the old behavior is still there but we are asking users to update their code to transition to the new behavior, which is something like: Spree::Refund.create(attrs: ..., perform_after_create: false).perform! The two extra callbacks are needed to: - set_perform_after_create_default: prints the deprecation message only when creating the instance, otherwise it will be printed also when calling perform! on a good instance. Also, it sets the deafult to true when the attribute is not passed, which means that code has not been updated yet. - clear_perform_after_create: this callback is needed to clean the instance, after this process ends, otherwise each call of perform! after the callbacks execution could not be executed if the instance was created with perform_after_create: false. Specs needed some seriuos refactor to reflect this new architecture but they look cleaner now. Co-Authoring the commit since the initial code has been taken from solidusio#3181. Co-authored-by: Angel Perez <[email protected]>
aldesantis
pushed a commit
to aldesantis/solidus
that referenced
this pull request
Dec 28, 2020
We can't just stop calling this callback or we are going to break applications when it's defined code like: Spree::Refund.create(attrs: ...) since it currently relies on also calling perform! as well. With this code, the old behavior is still there but we are asking users to update their code to transition to the new behavior, which is something like: Spree::Refund.create(attrs: ..., perform_after_create: false).perform! The two extra callbacks are needed to: - set_perform_after_create_default: prints the deprecation message only when creating the instance, otherwise it will be printed also when calling perform! on a good instance. Also, it sets the deafult to true when the attribute is not passed, which means that code has not been updated yet. - clear_perform_after_create: this callback is needed to clean the instance, after this process ends, otherwise each call of perform! after the callbacks execution could not be executed if the instance was created with perform_after_create: false. Specs needed some seriuos refactor to reflect this new architecture but they look cleaner now. Co-Authoring the commit since the initial code has been taken from solidusio#3181. Co-authored-by: Angel Perez <[email protected]>
mamhoff
pushed a commit
to mamhoff/solidus
that referenced
this pull request
Feb 1, 2021
We can't just stop calling this callback or we are going to break applications when it's defined code like: Spree::Refund.create(attrs: ...) since it currently relies on also calling perform! as well. With this code, the old behavior is still there but we are asking users to update their code to transition to the new behavior, which is something like: Spree::Refund.create(attrs: ..., perform_after_create: false).perform! The two extra callbacks are needed to: - set_perform_after_create_default: prints the deprecation message only when creating the instance, otherwise it will be printed also when calling perform! on a good instance. Also, it sets the deafult to true when the attribute is not passed, which means that code has not been updated yet. - clear_perform_after_create: this callback is needed to clean the instance, after this process ends, otherwise each call of perform! after the callbacks execution could not be executed if the instance was created with perform_after_create: false. Specs needed some seriuos refactor to reflect this new architecture but they look cleaner now. Co-Authoring the commit since the initial code has been taken from solidusio#3181. Co-authored-by: Angel Perez <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces a deprecation warning when relying on
Spree::Refund#perform!implicitly (i.e.: calling it as a callback rather than calling it explicitly); aims to serve as a starting point for #1415Checklist: