Fix outstanding balance sum in payment report #7334
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7334 +/- ##
=======================================
Coverage 89.70% 89.70%
=======================================
Files 649 649
Lines 18865 18865
=======================================
Hits 16923 16923
Misses 1942 1942
Continue to review full report at Codecov.
|
|
Thanks so much for the quick fix on this @Matt-Yorkley 👍 The reports responsible for the two bugsnag errors (and snails) are rendering now, after this PR. I figured it might be useful to test this under two scenarios, in which customer balances feature is toggled on and off. Found no difference whatsoever in the outcome:
Itemized totals:
Payment totals:
Notice the strangely long decimal. Looks a bit weird - I'm guessing they were the root cause? I've seen this recently... (under products somehow...?) Placed an order and checked that the customer balance is updated. The weird looking zeroes disappear if one selects "all" from the dropdown, and re-appear if a hub is selected:
I'm not sure this is related. Just felt it's worth pointing out since the Bugsnag does mention It's ready to go. |
| @@ -100,7 +100,7 @@ def columns | |||
| proc { |orders| orders.first.distributor.name }, | |||
| proc { |orders| orders.to_a.sum(&:item_total) }, | |||
| proc { |orders| orders.sum(&:ship_total) }, | |||
| proc { |orders| orders.sum(&:outstanding_balance) }, | |||
There was a problem hiding this comment.
I think we were just missing https://www.rubydoc.info/stdlib/bigdecimal/BigDecimal#coerce-instance_method. I didn't know this was possible 😍
I wonder if we didn't spot this due to the Ruby upgrade that after this 🤔
There was a problem hiding this comment.
It's really weird. I tried 5 or 6 times to write a test for this, in feature specs, controller specs, model specs... it passed without error every time! And we actually do have a test for this already, and it was passing. Really easy to replicate in the browser or console though! 💥
#WTF 🤷♂️ 🤷♂️ 🤷♂️
There was a problem hiding this comment.
🤔 🤔 I was probably stubbing things I shouldn't?
There was a problem hiding this comment.
I would have expected the tests here to have failed:
Just met this again (for the record,
This value gets actually gets stored and is even displayed in the shopfront:
I wonder if this can mess anything throughout the app... |
|
This definitely needs an issue. We're missing rounding somewhere. |
|
Thanks @sauloperez , opened #7504. |
What? Why?
Closes #7333. See notes in issue.
What should we test?
Payment reports should load without error
Release notes
Fixed outstanding balance sum in payment report
Changelog Category: User facing changes