Need help with my pull request(first) on sales/purchase invoice rounding

Frappe Framework
1

Hi,

Following recent momentum on community contribution, I have spent few days trying to fix rounding issue in Sales/Purchase invoice(Invoice round off does not get reflected in accounts #8816 by sathishpy · Pull Request #11042 · frappe/erpnext · GitHub). I have made all the changes, tested the changes and also ran unit-tests related to sales/purchase invoice and fixed the failing ones. However travis build is still complaining about one more test which I don’t know how to reproduce as it passes when run locally. I also need some help with review/merging. If someone who knows this module well can spend couple of minutes helping me, it would be really great. Perhaps I will try to document them and put it in wiki so that others can also benefit.

Regards
Sathish

2 Likes
2

I have fixed those test cases in the latest release. Can you please rebase and push again?

And regarding review and merge, I will surely work on it next week.

1 Like
3

Please do document this procedure if you can as it will help others. Also, thank you for your work. And keep it going.

4

Thanks @nabinhait. Just wondering why the failures were seen only on travis build but not on my repo. I ran 'bench run-tests --doctype ‘Sales Invoice’ and 'bench run-tests --doctype ‘PurchaseInvoice’. They both passed far total of 60 test cases. I tried running ‘bench run-tests’, but that failed even without any of my changes. Ideally I would have loved to fix those things on my own and use your time in review. Perhaps I can attempt that next time, but really need to know how could I replicated those failures in my setup. If you think a new install would have caught that issue, I will give it another try.

5

@nabinhait, sorry to bring this again. Without rounding option in purchase invoice, I generally use ‘discount’ option which used to work. However today I observed that adding ‘discount’ for small amount erroneously resulted in a ‘rounding adjustment’ of 1 paise. This isn’t good. I didn’t troubleshoot the issue further as the fix I have done is sufficient to resolve it. All I need is some assistance in fixing the last ‘client test’ failure and a review.

6

I will review the PR today for sure. There was some bottleneck at our end, which caused delay in reviewing PRs. I will try to review all open PRs in next 7 days.

2 Likes
7

Great, thank you for the update.