Bug 14486 - C4/Accountlines.pm needs better documentation and removal of stale code
Summary: C4/Accountlines.pm needs better documentation and removal of stale code
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-02 12:29 UTC by Barton Chittenden
Modified: 2019-10-14 19:58 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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