Bug 14486

Summary: C4/Accountlines.pm needs better documentation and removal of stale code
Product: Koha Reporter: Barton Chittenden <barton>
Component: CirculationAssignee: Bugs List <koha-bugs>
Status: CLOSED INVALID QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, josef.moravec, kyle.m.hall
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12194
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:

Description Barton Chittenden 2015-07-02 12:29:52 UTC
The documentation in C4/Accountlines.pm is confusing. There is no clear statement of what the module is for, and some poor programmer has sprinkled the code with 'FIXME' comments to the effect of "I don't understand what this function does."

The intent and proper use of these functions should be documented, and the FIXME comments should be removed.

fixaccounts() is entirely commented out. This should be removed.

The following functions are not documented:

chargelostitem()

getcharges()

ModNote()

getcredits()

getrefunds()

ReversePayment()

makepartialpayment()

These should have at least minimal documentation saying "It does what you think it does", and any functions which modify the accountlines need a complete explanation of what they're changing in the database and why. 

The weakness of the accountlines table is that there is no link between fines and issues/old_issues which is a perennial source of bugs. This needs to be explicitly documented as a gotcha.

Obviously work on documentation will be obsoleted by Bug 6427 - Rewrite of the accounts system, but in some sense, because the re-write is supposed to work with legacy code, I feel that the lack of clear documentation in the old code is an impediment to the testing of the new.
Comment 1 Josef Moravec 2018-10-26 09:25:03 UTC
As the C4/Accounts.pm is almost whole rewritten to Koha::Account this does not make sense anymore. Closing