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

Review if we haven't missed any place where customer balances are used #6908

Closed
RachL opened this issue Feb 17, 2021 · 3 comments · Fixed by #7150
Closed

Review if we haven't missed any place where customer balances are used #6908

RachL opened this issue Feb 17, 2021 · 3 comments · Fixed by #7150
Assignees

Comments

@RachL
Copy link
Contributor

RachL commented Feb 17, 2021

Description

Balances are also shown on each orders payments tabs (admin side). Do we need to refactor that bit as well?

Also:

we'll need to review b9d72ce and its PR: #1468. Looks like we stopped using that file? 🔥 🔥 🔥

Acceptance Criteria & Tests

@sauloperez
Copy link
Contributor

sauloperez commented Feb 25, 2021

Places I found we use oustanding_balance and need to be feature-toggled:

  • shared/_payment.html.haml (used by most of the files below)
  • Spree::Payment::Processing#calculate_refund_amount
  • Api::Admin::OrderSerializer#display_outstanding_balance
  • spree/admin/payments/_form.html.haml
  • spree/admin/payments/index.html.haml
  • spree/admin/payments/new.html.haml
  • app/mailers/subscription_mailer.rb
  • app/mailers/spree/order_mailer.rb
  • lib/open_food_network/payments_report.rb
  • engines/order_management/app/services/order_management/order/updater.rb

@mkllnk
Copy link
Member

mkllnk commented Mar 21, 2021

@sauloperez This issue is in Code Review. Is there anything you would like us to look at, double-check?

@sauloperez
Copy link
Contributor

I'd appreciate it if you could thoroughly check the tests on that open PR.

I updated the list of occurrences above for the records. I believe I didn't forget any file.

Re the Payments report, @RachL and I thought it'd be best if we could fix it separately as we did with the bulk coop report so we don't make that PR too broad. It's also the report itself that is broken (it doesn't consider canceled orders) and not the balance calculation. This, however, will lead to inconsistent balances in UK and FR; the report will show a different amount compared to all other pages. We think it's an affordable trade-off given it's not a wildly used report (we still need to confirm this hypothesis with Matomo's data).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants