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

2655 fix admin payments #2697

Merged
merged 3 commits into from
Sep 14, 2018
Merged

2655 fix admin payments #2697

merged 3 commits into from
Sep 14, 2018

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Sep 13, 2018

What? Why?

Closes #2655.

The new Stripe code is loading payment methods including their calculated fees. But it forgot to pass on the current order to calculate fees on. Trying to create a payment for an order failed if the enterprise had certain payment fees configured.

What should we test?

Creating a payment from the admin interface.

  • Admin -> Orders -> select order -> Payments -> New Payment

Creating a payment should work.

Release notes

Fixed charging customers from the admin interface which failed when certain payment fees where configured.

Changelog Category: Fixed

We can't always rely on other parts of the code been loaded first. We
need to declare dependencies so that they are always present.

I just ran into this problem in my dev environment.
@mkllnk mkllnk self-assigned this Sep 13, 2018
@mkllnk mkllnk requested a review from kristinalim September 13, 2018 06:33
@luisramos0
Copy link
Contributor

@mkllnk I am moving to Test Ready with two reviews done. But please move back if @kristinalim review is required.

@sstead
Copy link

sstead commented Sep 14, 2018

Testing Notes

One thing is causing an error - if I try to create a partial payment for a payment method that has a fee attached I get an error

Things working:

  • when I create an order I can create a payment for it now worries
  • I can 'capture' payments
  • I can void a payment
  • I can adjust the order and the balance adjusts correctly

https://docs.google.com/document/d/1c6Wy8O6aYmpGvmXkUb3UpNFhBVP4uYULTa8Pd-LPIGI/edit#

@mkllnk
Copy link
Member Author

mkllnk commented Sep 14, 2018

Looks like you discovered an old bug. I filed it as #2724 now.

@mkllnk mkllnk merged commit 6b56def into openfoodfoundation:master Sep 14, 2018
@mkllnk mkllnk deleted the 2655-fix-admin-payments branch September 14, 2018 01:26
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