https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15896
Marcel de Rooy <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #42 from Marcel de Rooy <[email protected]> --- QA Comment: Since we are in a migration/consolidation process for Accounts routines, I will not block this patch set, although Jonathan had some concerns about incomplete test coverage and code quality. I agree with Jonathan that we should have some more tests here, but note that you are not doing complete new things here (just move code). You added two tests btw. We now have calls to ->pay with and without accountlines_id. The statistics test in Accounts.t is quite rudimentary. Could be extended. A test related to the call of ReturnLostItem could be added. And what about sip type? About the code quality. The code in sub pay is getting longer. That is true. If we first consolidate everything in one spot, we can still improve. But maybe this is just of matter of adding one or two private subroutines to have a more readable sub pay? Another note in the larger scope of making payments: The call to ReturnLostItem was and now is only when you pay on one line. But what about paying multiple lines including such a lost fee? Seems to be a point of attention. On the paycollect interface: If you pay more, the interface responds with a warn Pay less or equal. The interface should also protest against amount zero or 0.00 btw. And in connection to makepayment: There is still a call to makepayment in opac/opac-account-pay-paypal-return.pl. Glancing through that code, I am wondering if you really want to pay all accountlines without checking if the sum matches the amount. But I may be mistaken? In conclusion: I have no objection to pushing this code now. But in this migration project we should try to improve later on. Passed QA -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
