[Koha-bugs] [Bug 12768] Replacement cost and processing fee management

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

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

Katrin Fischer <katrin.fischer at bsz-bw.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |katrin.fischer at bsz-bw.de
             Status|Signed Off                  |Failed QA
           Keywords|Academy                     |

--- Comment #16 from Katrin Fischer <katrin.fischer at bsz-bw.de> ---
Hi Simith,

starting with a code review here:

Results of the QA script:

 FAIL    C4/Accounts.pm
   OK      critic
   FAIL      forbidden patterns
        forbidden pattern: tab char (line 404)
        forbidden pattern: tab char (line 410)
        forbidden pattern: tab char (line 409)
        forbidden pattern: tab char (line 469)
        forbidden pattern: tab char (line 408)
        forbidden pattern: tab char (line 407)
        forbidden pattern: tab char (line 406)
   OK      pod
   OK      valid

Also some tests seem to be failing:

Test Summary Report
-------------------
t/db_dependent/Circulation.t (Wstat: 512 Tests: 60 Failed: 2)
  Failed tests:  43-44
  Non-zero exit status: 2


'Manual' code review:

1) Don't use double as the type, but decimal, as we are already using decimal
in all other places where amounts are stored and double should not be used to
store monetary values:

+  defaultreplacecost double(16,4) default NULL, -- default replacement cost
+  processfee double(16,4) default NULL, -- default text be recorded in the
column note when the processing fee is applied

+ updatedatabase.pl as well (blocker)

2) Please amend POD of manualinvoice with the new type PF and the new
parameter.
Can you explain what notifyid is used for? (normal)

3) Spotting a translation problem here:

 +            C4::Accounts::chargelostitem($issues, "Lost Item
$issues->{'title'} $issues->{'barcode'}");

... but it was there before your patch.

4) Please include the updated schema files in a separate patch file.

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