From 211a026271704746f393bc5cc036d76eec568626 Mon Sep 17 00:00:00 2001 From: charles Date: Wed, 22 Apr 2015 15:54:04 -0400 Subject: [PATCH] Bug 12768 - Replacement cost and processing fee management --- C4/Accounts.pm | 72 ++++++++--------- C4/Circulation.pm | 5 +- admin/itemtypes.pl | 16 +++- ...-replacement_cost_processing_fee_management.sql | 2 + installer/data/mysql/sysprefs.sql | 2 + .../intranet-tmpl/prog/en/css/staff-global.css | 4 + .../prog/en/modules/admin/itemtypes.tt | 16 +++- .../en/modules/admin/preferences/circulation.pref | 12 +++ t/db_dependent/Circulation/Chargelostitem.t | 89 ++++++++++++++++++++++ 9 files changed, 172 insertions(+), 46 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_12768-replacement_cost_processing_fee_management.sql create mode 100644 t/db_dependent/Circulation/Chargelostitem.t diff --git a/C4/Accounts.pm b/C4/Accounts.pm index d0709d0..614bf8e 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -353,42 +353,35 @@ sub chargelostitem{ # a charge has been added # FIXME : if no replacement price, borrower just doesn't get charged? my $dbh = C4::Context->dbh(); - my ($borrowernumber, $itemnumber, $amount, $description) = @_; - + my ($issues, $description) = @_; + my $borrowernumber = $issues->{'borrowernumber'}; + my $itemnumber = $issues->{'itemnumber'}; + my $replacementprice = $issues->{'replacementprice'}; + my $defaultreplacecost = $issues->{'defaultreplacecost'}; + my $processfee = $issues->{'processfee'}; + my $usedefaultreplacementcost = C4::Context->preference("useDefaultReplacementCost"); + my $processingfeenote = C4::Context->preference("ProcessingFeeNote"); + + if ($usedefaultreplacementcost && $replacementprice == 0){ + $replacementprice = $defaultreplacecost; + } # first make sure the borrower hasn't already been charged for this item my $sth1=$dbh->prepare("SELECT * from accountlines WHERE borrowernumber=? AND itemnumber=? and accounttype='L'"); $sth1->execute($borrowernumber,$itemnumber); my $existing_charge_hashref=$sth1->fetchrow_hashref(); - # OK, they haven't + my $accountline; unless ($existing_charge_hashref) { - my $manager_id = 0; - $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; - # This item is on issue ... add replacement cost to the borrower's record and mark it returned - # Note that we add this to the account even if there's no replacement price, allowing some other - # process (or person) to update it, since we don't handle any defaults for replacement prices. - my $accountno = getnextacctno($borrowernumber); - my $sth2=$dbh->prepare("INSERT INTO accountlines - (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding,itemnumber,manager_id) - VALUES (?,?,now(),?,?,'L',?,?,?)"); - $sth2->execute($borrowernumber,$accountno,$amount, - $description,$amount,$itemnumber,$manager_id); - if ( C4::Context->preference("FinesLog") ) { - logaction("FINES", 'CREATE', $borrowernumber, Dumper({ - action => 'create_fee', - borrowernumber => $borrowernumber, - accountno => $accountno, - amount => $amount, - amountoutstanding => $amount, - description => $description, - accounttype => 'L', - itemnumber => $itemnumber, - manager_id => $manager_id, - })); + #add processing fee + if ($processfee > 0){ + manualinvoice($borrowernumber, $itemnumber, $description, 'PF', $processfee, $processingfeenote, 1); + } + #add replace cost + if ($replacementprice > 0){ + manualinvoice($borrowernumber, $itemnumber, $description, 'L', $replacementprice, undef, 1); } - } } @@ -408,18 +401,19 @@ should be the empty string. #' # FIXME: In Koha 3.0 , the only account adjustment 'types' passed to this function -# are : -# 'C' = CREDIT -# 'FOR' = FORGIVEN (Formerly 'F', but 'F' is taken to mean 'FINE' elsewhere) -# 'N' = New Card fee -# 'F' = Fine -# 'A' = Account Management fee -# 'M' = Sundry -# 'L' = Lost Item +# are : +# 'C' = CREDIT +# 'FOR' = FORGIVEN (Formerly 'F', but 'F' is taken to mean 'FINE' elsewhere) +# 'N' = New Card fee +# 'F' = Fine +# 'A' = Account Management fee +# 'PF' = Processing fee +# 'M' = Sundry +# 'L' = Lost Item # sub manualinvoice { - my ( $borrowernumber, $itemnum, $desc, $type, $amount, $note ) = @_; + my ( $borrowernumber, $itemnum, $desc, $type, $amount, $note, $skip_notify ) = @_; my $manager_id = 0; $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; my $dbh = C4::Context->dbh; @@ -427,6 +421,7 @@ sub manualinvoice { my $insert; my $accountno = getnextacctno($borrowernumber); my $amountleft = $amount; + $skip_notify //= 0; if ( ( $type eq 'L' ) or ( $type eq 'F' ) @@ -434,7 +429,7 @@ sub manualinvoice { or ( $type eq 'N' ) or ( $type eq 'M' ) ) { - $notifyid = 1; + $notifyid = 1 unless $skip_notify; } if ( $itemnum ) { @@ -473,9 +468,8 @@ sub manualinvoice { } sub getcharges { - my ( $borrowerno, $timestamp, $accountno ) = @_; + my ( $borrowerno, $accountno ) = @_; my $dbh = C4::Context->dbh; - my $timestamp2 = $timestamp - 1; my $query = ""; my $sth = $dbh->prepare( "SELECT * FROM accountlines WHERE borrowernumber=? AND accountno = ?" diff --git a/C4/Circulation.pm b/C4/Circulation.pm index f991680..62c13a4 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3629,10 +3629,11 @@ sub LostItem{ my ($itemnumber, $mark_returned) = @_; my $dbh = C4::Context->dbh(); - my $sth=$dbh->prepare("SELECT issues.*,items.*,biblio.title + my $sth=$dbh->prepare("SELECT issues.*,items.*,biblio.title,itemtypes.defaultreplacecost, itemtypes.processfee FROM issues JOIN items USING (itemnumber) JOIN biblio USING (biblionumber) + JOIN itemtypes on items.itype = itemtypes.itemtype WHERE issues.itemnumber=?"); $sth->execute($itemnumber); my $issues=$sth->fetchrow_hashref(); @@ -3646,7 +3647,7 @@ sub LostItem{ defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $itemnumber...) failed!"; # zero is OK, check defined } if (C4::Context->preference('WhenLostChargeReplacementFee')){ - C4::Accounts::chargelostitem($borrowernumber, $itemnumber, $issues->{'replacementprice'}, "Lost Item $issues->{'title'} $issues->{'barcode'}"); + C4::Accounts::chargelostitem($issues, "Lost Item $issues->{'title'} $issues->{'barcode'}"); #FIXME : Should probably have a way to distinguish this from an item that really was returned. #warn " $issues->{'borrowernumber'} / $itemnumber "; } diff --git a/admin/itemtypes.pl b/admin/itemtypes.pl index 9effa7e..69aeb5f 100755 --- a/admin/itemtypes.pl +++ b/admin/itemtypes.pl @@ -116,6 +116,8 @@ if ( $op eq 'add_form' ) { itemtype => $itemtype, description => $data->{'description'}, rentalcharge => sprintf( "%.2f", $data->{'rentalcharge'} ), + defaultreplacecost => sprintf( "%.2f", $data->{'defaultreplacecost'} ), + processfee => sprintf( "%.2f", $data->{'processfee'} ), notforloan => $data->{'notforloan'}, imageurl => $data->{'imageurl'}, template => C4::Context->preference('template'), @@ -144,6 +146,8 @@ elsif ( $op eq 'add_validate' ) { UPDATE itemtypes SET description = ? , rentalcharge = ? + , defaultreplacecost = ? + , processfee = ? , notforloan = ? , imageurl = ? , summary = ? @@ -156,6 +160,8 @@ elsif ( $op eq 'add_validate' ) { $sth->execute( $input->param('description'), $input->param('rentalcharge'), + $input->param('defaultreplacecost'), + $input->param('processfee'), ( $input->param('notforloan') ? 1 : 0 ), ( $input->param('image') eq 'removeImage' ? '' : ( @@ -168,15 +174,15 @@ elsif ( $op eq 'add_validate' ) { $input->param('checkinmsg'), $input->param('checkinmsgtype'), $sip_media_type, - $input->param('itemtype') + $input->param('itemtype'), ); } else { # add a new itemtype & not modif an old my $query = " INSERT INTO itemtypes - (itemtype,description,rentalcharge, notforloan, imageurl, summary, checkinmsg, checkinmsgtype, sip_media_type) + (itemtype,description, rentalcharge, defaultreplacecost, processfee, notforloan, imageurl, summary, checkinmsg, checkinmsgtype, sip_media_type) VALUES - (?,?,?,?,?,?,?,?,?); + (?,?,?,?,?,?,?,?,?,?,?); "; my $sth = $dbh->prepare($query); my $image = $input->param('image'); @@ -184,6 +190,8 @@ elsif ( $op eq 'add_validate' ) { $input->param('itemtype'), $input->param('description'), $input->param('rentalcharge'), + $input->param('defaultreplacecost'), + $input->param('processfee'), $input->param('notforloan') ? 1 : 0, $image eq 'removeImage' ? '' : $image eq 'remoteImage' ? $input->param('remoteImage') : @@ -250,6 +258,8 @@ else { # DEFAULT foreach my $itemtype ( @{$results} ) { $itemtype->{imageurl} = getitemtypeimagelocation( 'intranet', $itemtype->{imageurl} ); $itemtype->{rentalcharge} = sprintf( '%.2f', $itemtype->{rentalcharge} ); + $itemtype->{defaultreplacecost} = sprintf( '%.2f', $itemtype->{defaultreplacecost} ); + $itemtype->{processfee} = sprintf( '%.2f', $itemtype->{processfee} ); push( @loop, $itemtype ); } diff --git a/installer/data/mysql/atomicupdate/bug_12768-replacement_cost_processing_fee_management.sql b/installer/data/mysql/atomicupdate/bug_12768-replacement_cost_processing_fee_management.sql new file mode 100644 index 0000000..9b27966 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_12768-replacement_cost_processing_fee_management.sql @@ -0,0 +1,2 @@ +INSERT INTO systempreferences (variable,value,explanation,type) VALUES ('useDefaultReplacementCost',0,'default replacement cost defined in item type','YesNo'); +INSERT INTO systempreferences (variable,value,explanation,type) VALUES ('ProcessingFeeNote','','Set the text to be recorded in the column note, table accountlines when the processing fee (defined in item type) is applied','textarea'); diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index f371193..43673ad 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -344,6 +344,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('previousIssuesDefaultSortOrder','asc','asc|desc','Specify the sort order of Previous Issues on the circulation page','Choice'), ('printcirculationslips','1','','If ON, enable printing circulation receipts','YesNo'), ('PrintNoticesMaxLines','0','','If greater than 0, sets the maximum number of lines an overdue notice will print. If the number of items is greater than this number, the notice will end with a warning asking the borrower to check their online account for a full list of overdue items.','Integer'), +('ProcessingFeeNote', '', NULL, 'Set the text to be recorded in the column note, table accountlines when the processing fee (defined in item type) is applied', 'textarea'), ('QueryAutoTruncate','1',NULL,'If ON, query truncation is enabled by default','YesNo'), ('QueryFuzzy','1',NULL,'If ON, enables fuzzy option for searches','YesNo'), ('QueryStemming','1',NULL,'If ON, enables query stemming','YesNo'), @@ -461,6 +462,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('UseControlNumber','0','','If ON, record control number (w subfields) and control number (001) are used for linking of bibliographic records.','YesNo'), ('UseCourseReserves','0',NULL,'Enable the course reserves feature.','YesNo'), ('useDaysMode','Calendar','Calendar|Days|Datedue','Choose the method for calculating due date: select Calendar to use the holidays module, and Days to ignore the holidays module','Choice'), +('useDefaultReplacementCost', '0', NULL, 'default replacement cost defined in item type', 'YesNo'), ('useDischarge','','','Allows librarians to discharge borrowers and borrowers to request a discharge','YesNo'), ('UseICU','0','1','Tell Koha if ICU indexing is in use for Zebra or not.','YesNo'), ('UseKohaPlugins','0','','Enable or disable the ability to use Koha Plugins.','YesNo'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css b/koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css index 2c4011f..09d69ad 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css +++ b/koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css @@ -604,6 +604,10 @@ fieldset.rows fieldset { width: 9em; } +.yui-b fieldset.rows ol.oladditemtype label, .yui-b fieldset.rows ol.oladditemtype span.label { + width: 13em; +} + .yui-b fieldset.rows div.hint { margin-left : 10.5em; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt index 2e61a16..f26a0c9 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt @@ -162,7 +162,7 @@ Item types administration [% END %] -
    +
    1. [% IF ( notforloan ) %] @@ -175,7 +175,15 @@ Item types administration
    2. -
      3. + +
    3. + + +
      4. +
    4. + + +
    5. @@ -262,6 +270,8 @@ Item types administration Description Not for loan Charge + Default replacement cost + Processing fee (when lost) Checkin message Actions @@ -280,6 +290,8 @@ Item types administration [% loo.rentalcharge %] [% END %] + [% loo.defaultreplacecost %] + [% loo.processfee %] [% loo.checkinmsg | html_line_break %] Edit diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index a16f231..6d85069 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -626,6 +626,18 @@ Circulation: yes: Charge no: "Don't Charge" - the replacement price when a patron loses an item. + - + - pref: useDefaultReplacementCost + choices: + yes: use + no: "Don't use" + - the default replacement cost defined in item type. + - + - "Set the text to be recorded in the column 'note', table 'accountlines' when the processing fee (defined in item type) is applied." + - pref: ProcessingFeeNote + type: textarea + class: code + Self Checkout: - - "Include the following JavaScript on all pages in the web-based self checkout:" diff --git a/t/db_dependent/Circulation/Chargelostitem.t b/t/db_dependent/Circulation/Chargelostitem.t new file mode 100644 index 0000000..e9d9694 --- /dev/null +++ b/t/db_dependent/Circulation/Chargelostitem.t @@ -0,0 +1,89 @@ +#!/usr/bin/perl +# +# This Koha test module is a stub! +# Add more tests here!!! + +use strict; +use warnings; +use Modern::Perl; + +use Test::MockModule; +use C4::Biblio; +use C4::Items; +use C4::Members; +use C4::Branch; +use C4::Category; +use C4::Circulation; +use MARC::Record; +use Test::More tests => 7; + +BEGIN { + use_ok('C4::Accounts'); +} + +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; +$dbh->do(q|DELETE FROM accountlines|); + +my $branchcode; +my $branch_created; +my @branches = keys %{ GetBranches() }; +if (@branches) { + $branchcode = $branches[0]; +} else { + $branchcode = 'B'; + ModBranch({ add => 1, branchcode => $branchcode, branchname => 'Branch' }); + $branch_created = 1; +} + +my %item_branch_infos = ( + homebranch => $branchcode, + holdingbranch => $branchcode, +); + +my ($biblionumber1) = AddBiblio(MARC::Record->new, ''); +my $itemnumber1 = AddItem({ barcode => '0101', %item_branch_infos }, $biblionumber1); +my $itemnumber2 = AddItem({ barcode => '0102', %item_branch_infos }, $biblionumber1); + +my ($biblionumber2) = AddBiblio(MARC::Record->new, ''); +my $itemnumber3 = AddItem({ barcode => '0203', %item_branch_infos }, $biblionumber2); + +my $categorycode; +my $category_created; +my @categories = C4::Category->all; +if (@categories) { + $categorycode = $categories[0]->{categorycode} +} else { + $categorycode = 'C'; + C4::Context->dbh->do( + "INSERT INTO categories(categorycode) VALUES(?)", undef, $categorycode); + $category_created = 1; +} + +my $borrowernumber = AddMember(categorycode => $categorycode, branchcode => $branchcode); +my $borrower = GetMember(borrowernumber => $borrowernumber); + +# Need to mock userenv for AddIssue +my $module = new Test::MockModule('C4::Context'); +$module->mock('userenv', sub { { branch => $branchcode } }); +AddIssue($borrower, '0101'); +AddIssue($borrower, '0203'); + +# Begin tests... +my $processfee = 10; +my $issues; +$issues = C4::Circulation::GetIssues({biblionumber => $biblionumber1}); +my $issue=$issues->[0]; +$issue->{'processfee'} = $processfee; +my $accountlineId = C4::Accounts::chargelostitem($issue, 'test'); + +my @accountline = C4::Accounts::getcharges($borrowernumber, $accountlineId); + +is( scalar(@accountline), 1, 'accountline should have 1 row' ); +is( int($accountline[0]->{amount}), $processfee, "The accountline amount should be precessfee value " ); +is( $accountline[0]->{accounttype}, 'PF', "The accountline accounttype should be PF " ); +is( $accountline[0]->{borrowernumber}, $borrowernumber, "The accountline borrowernumber should be the exemple borrownumber" ); +my $itemnumber = C4::Items::GetItemnumberFromBarcode('0101'); +is( $accountline[0]->{itemnumber}, $itemnumber, "The accountline itemnumber should the linked with barcode '0101'" ); +is( $accountline[0]->{description}, 'test', "The accountline description should be 'test'" ); -- 2.1.4