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

Refunding Doesn't Update Credit Owed #1475

Closed
gmacdougall opened this issue Sep 27, 2016 · 3 comments
Closed

Refunding Doesn't Update Credit Owed #1475

gmacdougall opened this issue Sep 27, 2016 · 3 comments
Labels
type:bug Error, flaw or fault

Comments

@gmacdougall
Copy link
Member

gmacdougall commented Sep 27, 2016

When a refund is given, the "Credit Owed" in the admin and order payment state are not correctly updated.

Steps to Reproduce

Start up a Solidus Sandbox

  1. Purchase 3 items
  2. Go to the admin page for that order
  3. Capture the payment for the order
  4. Cancel one of the three items ordered
  5. Go to the payment page and see that credit is owed
  6. Refund the amount owed to the customer

Expected Results

The order should move into a paid state and balance due/credit owed should be $0

Actual Results

The order's payment state is still credit owed, with -$20.99 showing as credit owed at the top of the page.

Example

image

@gmacdougall gmacdougall added the type:bug Error, flaw or fault label Sep 27, 2016
@DanielePalombo
Copy link
Contributor

DanielePalombo commented Oct 10, 2016

My and @mtylty started to investigate this issue and we found these related issues: #1254, #1107, spree/spree#6229 , spree/spree#5907

DanielePalombo added a commit to DanielePalombo/solidus that referenced this issue Oct 19, 2016
In Spree::OrderUpdater the `update_payment_total` was incorrectly
removing  the refunds total from each order payment. This seems
incorrect because they are separate things. Refund total has to be
taken away into the outstanding_balance calculation instead.

rel solidusio#1475
DanielePalombo added a commit to DanielePalombo/solidus that referenced this issue Oct 28, 2016
In Spree::OrderUpdater the `update_payment_total` was incorrectly
removing  the refunds total from each order payment. This seems
incorrect because they are separate things. Refund total has to be
taken away into the outstanding_balance calculation instead.

Replace payment_total column with method that subtract refunds from
incoming_payment.

To keep backward compatibility update_payment_total call
update_incoming_payment, and put in incoming_payment the sum of
completed payments.

rel solidusio#1475
@skukx
Copy link
Contributor

skukx commented Apr 7, 2017

The reason this is happening is because of this line:
https://github.com/solidusio/solidus/blob/v2.1/core/app/models/spree/order.rb#L355

    def outstanding_balance
      # If reimbursement has happened add it back to total to prevent balance_due payment state
      # See: https://github.com/spree/spree/issues/6229
      adjusted_payment_total = payment_total + refund_total

      if state == 'canceled'
        -1 * adjusted_payment_total
      else
        total - adjusted_payment_total
      end
    end

After a Spree::Refund is created perform! is called
https://github.com/solidusio/solidus/blob/master/core/app/models/spree/refund.rb#L16

In turn, this method calls payment.order.updater.update Which updates the payment_total column on the Spree::Order. This update already accounts for refunds when calculating payment_total.

See https://github.com/solidusio/solidus/blob/master/core/app/models/spree/order_updater.rb#L141

def update_payment_total
    order.payment_total = payments.completed.includes(:refunds).map { |payment| payment.amount - payment.refunds.sum(:amount) }.sum
end

So TLDR; Get rid of adjusted_payment_total because payment_total is already adjusted.

Edit

I'm realizing that this method may be used at times when the payment total has not been updated. So the question stands on what is the best way to fix this?

skukx referenced this issue in jhawthorn/solidus May 22, 2017
ericsaupe added a commit to deseretbook/solidus that referenced this issue Jun 5, 2017
Created a spec to simulate the behavior listed in solidusio#1475 in order
to properly fix the issue.
jhawthorn pushed a commit to jhawthorn/solidus that referenced this issue Jun 7, 2017
Created a spec to simulate the behavior listed in solidusio#1475 in order
to properly fix the issue.
jhawthorn pushed a commit to jhawthorn/solidus that referenced this issue Jun 22, 2017
Created a spec to simulate the behavior listed in solidusio#1475 in order
to properly fix the issue.
@jhawthorn
Copy link
Contributor

I believe this is fixed by #2002

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error, flaw or fault
Projects
None yet
Development

No branches or pull requests

4 participants