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.

Sign up for GitHub

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

Jump to bottom

Stop calling perform! as Spree::Refund after_create callback #3641

Merged
merged 5 commits into from
May 29, 2020
Merged

Stop calling perform! as Spree::Refund after_create callback #3641

merged 5 commits into from
May 29, 2020

Conversation

kennyadsl
Copy link
Member

@kennyadsl kennyadsl commented May 26, 2020 •
edited
Loading

Description

This PR allows creating Refunds without perform! being automatically called by a Rails callback.

This is a combination of two leftover PRs that we want to bring into Solidus v2.11:

They were both incomplete: the first one was doing the job but without taking care of adding a deprecation warning, the second one was only adding the deprecation warning without actually change the behavior. If this is an accepted approach we can close both of them.

I merged them together and added some missing pieces. In 3.0 we'll introduce a deprecation warning for that attribute so that people can remove that since it will be useless.

Some notes for the CHANGELOG entry:

From Solidus v3.0 onwards, #perform! will need to be explicitly called when creating new refunds. Please, change your code from:

Spree::Refund.create(your: attributes)

to:

Spree::Refund.create(your: attributes, perform_after_creation: false).perform!

The `perform_after_creation` attribute will be deprecated in Solidus 3.x

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • [ ] I have attached screenshots to this PR for visual changes (if needed)

@kennyadsl kennyadsl added type:enhancement Proposed or newly added feature changelog:solidus_core Changes to the solidus_core gem labels May 26, 2020
@kennyadsl kennyadsl added this to the 2.11 milestone May 26, 2020
@kennyadsl kennyadsl self-assigned this May 26, 2020
@kennyadsl kennyadsl force-pushed the kennyadsl/restore-1415 branch from 8372729 to b024a50 Compare May 27, 2020 16:35
@kennyadsl kennyadsl requested a review from aldesantis May 27, 2020 18:57
jarednorman
jarednorman requested changes May 28, 2020
View reviewed changes
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.

One small thing, but otherwise I think this looks good.

core/app/models/spree/reimbursement_type/reimbursement_helpers.rb Outdated Show resolved Hide resolved
swively added 4 commits May 29, 2020 13:43
@swively @kennyadsl
Remove after_create callbacks
b50c632 
To provide users more control over how they work with refunds, we want
to be able to create a refund withouth implicitly performing the
transaction.

This change moves perform! to a public method; users must now
explicitly call perform! on a refund.
@swively @kennyadsl
Call perform! on refund explicitly
68d5ce6
@swively @kennyadsl
Call #perform! from RefundsController#create
0755395 
We need to explicitly call #perform! on a refund for the transaction to
occur, now that the after_create callback has been removed.
@swively @kennyadsl
Perform refund during payment cancellation
2f7ec0f 
Previously the #cancel method relied on the after_create callback
performing the refund. Now it is explicitly called after the refund is
built.
@kennyadsl kennyadsl force-pushed the kennyadsl/restore-1415 branch from b024a50 to b04b2d1 Compare May 29, 2020 11:45
@kennyadsl @aitbw
Keep calling perform! after refund creation with a deprecation message
3d6f510 
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 <aitbw@users.noreply.github.com>
@kennyadsl kennyadsl force-pushed the kennyadsl/restore-1415 branch from b04b2d1 to 3d6f510 Compare May 29, 2020 12:02
@kennyadsl kennyadsl requested a review from jarednorman May 29, 2020 14:37
@kennyadsl kennyadsl merged commit 1aa554f into solidusio:master May 29, 2020
@kennyadsl kennyadsl deleted the kennyadsl/restore-1415 branch May 29, 2020 19:37
This was referenced May 29, 2020
Closed
Closed
@spaghetticode spaghetticode mentioned this pull request Jun 19, 2020
Merged
5 tasks
kennyadsl added a commit to solidusio/solidus_stripe that referenced this pull request Jan 22, 2021
@kennyadsl
Do not automatically perform! after redund creation
ee826d5 
After solidusio/solidus#3641 it's requested to create and
perform refunds in two different operations. This commit
aligns the code to the new standard.
kennyadsl added a commit to solidusio/solidus_stripe that referenced this pull request Jan 22, 2021
@kennyadsl @rainerdema
Do not automatically perform! after redund creation
76d4bc0 
After solidusio/solidus#3641 it's requested to create and
perform refunds in two different operations. This commit
aligns the code to the new standard.

Co-authored-by: Rainer Dema <rainerdema@users.noreply.github.com>
kennyadsl added a commit to solidusio/solidus_stripe that referenced this pull request Jan 26, 2021
@kennyadsl @rainerdema
Do not automatically perform! after redund creation
3c28582 
After solidusio/solidus#3641 it's requested to create and
perform refunds in two different operations. This commit
aligns the code to the new standard.

Co-authored-by: Rainer Dema <rainerdema@users.noreply.github.com>
kennyadsl added a commit to solidusio/solidus_stripe that referenced this pull request Jan 27, 2021
@kennyadsl @rainerdema
Do not automatically perform! after redund creation
07ca3ed 
After solidusio/solidus#3641 it's requested to create and
perform refunds in two different operations. This commit
aligns the code to the new standard.

Co-authored-by: Rainer Dema <rainerdema@users.noreply.github.com>
kennyadsl added a commit to solidusio/solidus_stripe that referenced this pull request Jan 28, 2021
@kennyadsl @rainerdema
Do not automatically perform! after redund creation
ad35ebf 
After solidusio/solidus#3641 it's requested to create and
perform refunds in two different operations. This commit
aligns the code to the new standard.

Co-authored-by: Rainer Dema <rainerdema@users.noreply.github.com>
kennyadsl added a commit to solidusio/solidus_stripe that referenced this pull request Jan 28, 2021
@kennyadsl @rainerdema
Do not automatically perform! after redund creation
60fd59e 
After solidusio/solidus#3641 it's requested to create and
perform refunds in two different operations. This commit
aligns the code to the new standard.

Co-authored-by: Rainer Dema <rainerdema@users.noreply.github.com>
kennyadsl added a commit to solidusio/solidus_stripe that referenced this pull request Jan 29, 2021
@kennyadsl @rainerdema
Do not automatically perform! after redund creation
ecbd45c 
After solidusio/solidus#3641 it's requested to create and
perform refunds in two different operations. This commit
aligns the code to the new standard.

Co-authored-by: Rainer Dema <rainerdema@users.noreply.github.com>
kennyadsl added a commit to solidusio/solidus_stripe that referenced this pull request Jan 29, 2021
@kennyadsl @rainerdema
Do not automatically perform! after redund creation
85f82e8 
After solidusio/solidus#3641 it's requested to create and
perform refunds in two different operations. This commit
aligns the code to the new standard.

Co-authored-by: Rainer Dema <rainerdema@users.noreply.github.com>
kennyadsl added a commit to solidusio/solidus_stripe that referenced this pull request Jan 29, 2021
@kennyadsl @rainerdema
Do not automatically perform! after redund creation
aad2ed6 
After solidusio/solidus#3641 it's requested to create and
perform refunds in two different operations. This commit
aligns the code to the new standard.

Co-authored-by: Rainer Dema <rainerdema@users.noreply.github.com>
@DanielePalombo DanielePalombo mentioned this pull request Jun 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@jarednorman jarednorman jarednorman approved these changes

@aldesantis aldesantis aldesantis approved these changes

Assignees

@kennyadsl kennyadsl

Labels
changelog:solidus_core Changes to the solidus_core gem type:enhancement Proposed or newly added feature
Projects
None yet
Milestone
2.11
Development

Successfully merging this pull request may close these issues.
6 participants
@kennyadsl @aldesantis @jarednorman @houndci-bot @spaghetticode @swively