{"action":"created","issue":{"url":"https://api.github.com/repos/solidusio/solidus/issues/1254","repository_url":"https://api.github.com/repos/solidusio/solidus","labels_url":"https://api.github.com/repos/solidusio/solidus/issues/1254/labels{/name}","comments_url":"https://api.github.com/repos/solidusio/solidus/issues/1254/comments","events_url":"https://api.github.com/repos/solidusio/solidus/issues/1254/events","html_url":"https://github.com/solidusio/solidus/issues/1254","id":160648968,"number":1254,"title":"Order#outstanding_balance implementation limits Refund usage","user":{"login":"jordan-brough","id":23050,"avatar_url":"https://avatars.githubusercontent.com/u/23050?v=3","gravatar_id":"","url":"https://api.github.com/users/jordan-brough","html_url":"https://github.com/jordan-brough","followers_url":"https://api.github.com/users/jordan-brough/followers","following_url":"https://api.github.com/users/jordan-brough/following{/other_user}","gists_url":"https://api.github.com/users/jordan-brough/gists{/gist_id}","starred_url":"https://api.github.com/users/jordan-brough/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jordan-brough/subscriptions","organizations_url":"https://api.github.com/users/jordan-brough/orgs","repos_url":"https://api.github.com/users/jordan-brough/repos","events_url":"https://api.github.com/users/jordan-brough/events{/privacy}","received_events_url":"https://api.github.com/users/jordan-brough/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-06-16T12:25:44Z","updated_at":"2016-07-20T17:02:46Z","closed_at":null,"body":"### Executive Summary\r\n\r\nThe current implementation of `Order#outstanding_balance` limits `Refund` usage to certain use cases.  This is mostly the fault of one person who shall remain nameless, but his initials are J.B. and [it is me](https://github.com/solidusio/solidus/commit/aa47046).\r\n\r\nIn a nutshell, Refunds are being treated as \"Order adjustment + Return of money on a payment\", and they probably should only be \"Return of money on a payment\".  That was the original intention of `Refund`.\r\n\r\n### Long-winded explanation\r\n\r\nCurrently `outstanding_balance` [is defined as](https://github.com/solidusio/solidus/blob/v1.2.2/core/app/models/spree/order.rb#L333-L343):\r\n```ruby\r\ntotal - payment_total - refund_total\r\n```\r\nwhere `payment_total` [is defined as](https://github.com/solidusio/solidus/blob/v1.2.2/core/app/models/spree/order_updater.rb#L63) `payments.completed.sum(:amount) - refund_total`.\r\n\r\nSo `outstanding_balance` expands to:\r\n```ruby\r\ntotal - (payments.completed.sum(:amount) - refund_total) - refund_total\r\n```\r\nwhich reduces to:\r\n```ruby\r\ntotal - payments.completed.sum(:amount)\r\n```\r\nThis means that \"outstanding balance\" is \"the order total minus your completed payment total, regardless of whether those payments have been refunded or not\".\r\n\r\nAnother way to think of it is that a `Refund` effectively adjusts the order total.  It is saying that you got returned X amount of money *and* the order should consider it as counting toward the payment total.\r\n\r\n##### This works fine for common scenarios like:\r\n- User completes an order for widget A ($10) and widget B ($20) and submits payment\r\n- User receives their shipment and reports that widget B is broken\r\n- Admin provides a refund for the broken widget B\r\n\r\nIn this case `outstanding_balance` is correctly $0.\r\n\r\n##### It does *not* work for scenarios where the order is modified after payment is complete:\r\n- User completes an order for widget A ($10) and widget B ($20) and submits payment\r\n- User asks to have widget B cancelled, and admin deletes widget B's line item from the order and grants a refund for widget B.\r\n\r\nIn this case the outstanding balance will end up being -$20 (it appears that we owe them money) because the raw payment total is $30 but the order total has gone down to $10.\r\n\r\n##### It also does *not* work for scenarios where a refund is issued for some other reason:\r\n- User completes an order for widget A ($10) and submits payment\r\n- User says \"oops, I need to pay using a different account. Can you refund me and let me pay with a different card?\"\r\n- Admin grants a refund and the user submits a new payment\r\n\r\nIn this case the outstanding balance will end up being -$10 (it appears that we owe them money) because the raw payment total is $20 (the sum of both completed payments) but the order total is still only $10.\r\n\r\n### Potential solution\r\n\r\nIt feels like the right fix could be:\r\n\r\n1. Change `outstanding_balance` to be simply `total - payment_total`\r\n2. When generating a refund for scenarios like \"widget is broken\", we also generate a negative adjustment on the order (or line item) to represent the order total .\r\n\r\nThat would allow `Refund` to *just* represent the return of money on a payment, and allow it to be used in various scenarios.  That might be a hard thing to do and maintain backward compatibility, however. :(\r\n\r\nThoughts?  Have others run into this and solved it in different ways?"},"comment":{"url":"https://api.github.com/repos/solidusio/solidus/issues/comments/234013288","html_url":"https://github.com/solidusio/solidus/issues/1254#issuecomment-234013288","issue_url":"https://api.github.com/repos/solidusio/solidus/issues/1254","id":234013288,"user":{"login":"Senjai","id":1831235,"avatar_url":"https://avatars.githubusercontent.com/u/1831235?v=3","gravatar_id":"","url":"https://api.github.com/users/Senjai","html_url":"https://github.com/Senjai","followers_url":"https://api.github.com/users/Senjai/followers","following_url":"https://api.github.com/users/Senjai/following{/other_user}","gists_url":"https://api.github.com/users/Senjai/gists{/gist_id}","starred_url":"https://api.github.com/users/Senjai/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Senjai/subscriptions","organizations_url":"https://api.github.com/users/Senjai/orgs","repos_url":"https://api.github.com/users/Senjai/repos","events_url":"https://api.github.com/users/Senjai/events{/privacy}","received_events_url":"https://api.github.com/users/Senjai/received_events","type":"User","site_admin":false},"created_at":"2016-07-20T17:02:45Z","updated_at":"2016-07-20T17:02:45Z","body":"Can we go with #1 here? I don't see an immediate problem if payment_total also takes into account the refund total. We currently fix this with an adjustment at the moment because refunds don't put the order into the payment state we expect it to be in."}}
{"actor":{"display_login":"Senjai"}}