[Koha-bugs] [Bug 7243] Do not take rentals as fines

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7243

M. de Rooy <m.de.rooy at rijksmuseum.nl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Passed QA

--- Comment #18 from M. de Rooy <m.de.rooy at rijksmuseum.nl> ---
QA Comment:
Code looks good to me. Worked as described during limited testing. The above
warning seems to be false; the remark on spelling remains!

The enhancement is written such a way that it does not interfere with current
behavior. By default rentals and manual invoices are included in the patron
balance used when checking out. Only by explicitly changing the prefs behavior
changes. (Exception here could be the amounts marked with Res.) 

I only have a few non-blocker comments. Followups are always welcome!

You mention NoissueCharge in some descriptions. Please correct. Looking for
noissuescharge will help find the pref much easier ;) Note the additional s in
the middle. The description of the prefs could be improved (see below).

t/db_dependent/lib/KohaTest/Members.pm looks to me like an unused test module;
I once asked the RM already to remove such files

Not a blocker, but just mentioning: The names of the prefs and also the names
of the variables (returned values) by the new GetMemberAccountBallance are
confusing.
E.g. What means Do not include .. in No issues .. (double no)? Not intuitively
clear. Could you say something like Consider rentals as fines? And explain in
one sentence that they do or do not block checkout. And similarly Consider
other charges as fines? Note that this is just to stimulate the process of
finding more understandable names :)
E.g. what is $non_issue_ballance (note double l ..)? Actually you mean here
total fines. It would be easier if you say total amount= fines + other charges,
and work with such names in code. 

Passed QA

-- 
You are receiving this mail because:
You are watching all bug changes.