@@ -, +, @@ on it. It should now be marked as cancelled that it still works --- Koha/Account/Line.pm | 99 +++++++++---- installer/data/mysql/account_credit_types.sql | 3 +- installer/data/mysql/account_offset_types.sql | 3 +- .../data/mysql/atomicupdate/bug_24603.perl | 20 +++ .../prog/en/modules/members/boraccount.tt | 4 +- members/cancel-charge.pl | 9 +- t/db_dependent/Koha/Account/Line.t | 131 +++++++++++++----- 7 files changed, 202 insertions(+), 67 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_24603.perl --- a/Koha/Account/Line.pm +++ a/Koha/Account/Line.pm @@ -291,46 +291,90 @@ sub void { Cancel a charge. It will mark the debit as 'cancelled' by updating its status to 'CANCELLED'. + Charges that have been fully or partially paid cannot be cancelled. -Return self in case of success, undef otherwise +Returns the cancellation accountline. =cut sub cancel { - my ($self) = @_; + my ( $self, $params ) = @_; - # Make sure it is a charge we are cancelling - return unless $self->is_debit; + # Make sure it is a charge we are reducing + unless ( $self->is_debit ) { + Koha::Exceptions::Account::IsNotDebit->throw( + error => 'Account line ' . $self->id . 'is not a debit' ); + } + if ( $self->debit_type_code eq 'PAYOUT' ) { + Koha::Exceptions::Account::IsNotDebit->throw( + error => 'Account line ' . $self->id . 'is a payout' ); + } # Make sure it is not already cancelled - return if $self->status && $self->status eq 'CANCELLED'; + if ( $self->status && $self->status eq 'CANCELLED' ) { + Koha::Exceptions::Account->throw( + error => 'Account line ' . $self->id . 'is already cancelled' ); + } # Make sure it has not be paid yet - return if $self->amount != $self->amountoutstanding; - - if ( C4::Context->preference("FinesLog") ) { - logaction('FINES', 'CANCEL', $self->borrowernumber, Dumper({ - action => 'cancel_charge', - borrowernumber => $self->borrowernumber, - amount => $self->amount, - amountoutstanding => $self->amountoutstanding, - description => $self->description, - debit_type_code => $self->debit_type_code, - note => $self->note, - itemnumber => $self->itemnumber, - manager_id => $self->manager_id, - })); + if ( $self->amount != $self->amountoutstanding ) { + Koha::Exceptions::Account->throw( + error => 'Account line ' . $self->id . 'is already offset' ); } - $self->set({ - status => 'CANCELLED', - amountoutstanding => 0, - amount => 0, - }); - $self->store(); + # Check for mandatory parameters + my @mandatory = ( 'staff_id', 'branch' ); + for my $param (@mandatory) { + unless ( defined( $params->{$param} ) ) { + Koha::Exceptions::MissingParameter->throw( + error => "The $param parameter is mandatory" ); + } + } - return $self; + my $cancellation; + $self->_result->result_source->schema->txn_do( + sub { + + # A 'cancellation' is a 'credit' + $cancellation = Koha::Account::Line->new( + { + date => \'NOW()', + amount => 0 - $self->amount, + credit_type_code => 'CANCELLATION', + status => 'ADDED', + amountoutstanding => 0 - $self->amount, + manager_id => $params->{staff_id}, + borrowernumber => $self->borrowernumber, + interface => 'intranet', + branchcode => $params->{branch}, + } + )->store(); + + my $cancellation_offset = Koha::Account::Offset->new( + { + credit_id => $cancellation->accountlines_id, + type => 'CANCELLATION', + amount => $self->amount + } + )->store(); + + # Link cancellation to charge + $cancellation->apply( + { + debits => [$self], + offset_type => 'CANCELLATION' + } + ); + $cancellation->status('APPLIED')->store(); + + # Update status of original debit + $self->status('CANCELLED')->store; + } + ); + + $cancellation->discard_changes; + return $cancellation; } =head3 reduce @@ -453,7 +497,8 @@ sub reduce { } else { - # Zero amount offset used to link original 'debit' to reduction 'credit' + # Zero amount offset used to link original 'debit' to + # reduction 'credit' my $link_reduction_offset = Koha::Account::Offset->new( { credit_id => $reduction->accountlines_id, --- a/installer/data/mysql/account_credit_types.sql +++ a/installer/data/mysql/account_credit_types.sql @@ -6,6 +6,7 @@ INSERT INTO account_credit_types ( code, description, can_be_added_manually, is_ ('DISCOUNT', 'A discount applied to a patrons fine', 0, 1), ('REFUND', 'A refund applied to a patrons fine', 0, 1), ('LOST_FOUND', 'Lost item fee refund', 0, 1), -('PURCHASE', 'Purchase', 0, 1); +('PURCHASE', 'Purchase', 0, 1), +('CANCELLATION', 'Cancellation', 0, 1); INSERT INTO authorised_values (category,authorised_value,lib) VALUES ('PAYMENT_TYPE','CASH','Cash'); --- a/installer/data/mysql/account_offset_types.sql +++ a/installer/data/mysql/account_offset_types.sql @@ -21,4 +21,5 @@ INSERT INTO account_offset_types ( type ) VALUES ('Credit Applied'), ('PAYOUT'), ('DISCOUNT'), -('REFUND'); +('REFUND'), +('CANCELLATION'); --- a/installer/data/mysql/atomicupdate/bug_24603.perl +++ a/installer/data/mysql/atomicupdate/bug_24603.perl @@ -0,0 +1,20 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if ( CheckVersion($DBversion) ) { + + $dbh->do( + qq{ + INSERT IGNORE INTO account_credit_types (code, description, can_be_added_manually, is_system) + VALUES + ('CANCELLATION', 'A cancellation applied to a patron charge', 0, 1) + } + ); + + $dbh->do( + qq{ + INSERT IGNORE INTO account_offset_types ( type ) VALUES ('CANCELLATION'); + } + ); + + # Always end with this (adjust the bug info) + NewVersion( $DBversion, 24603, "Add CANCELLATION credit_type_code" ); +} --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt @@ -87,12 +87,12 @@ [% END %] Details [% IF account.is_debit && account.amountoutstanding > 0 %] - Pay + Pay [% END %] [% IF account.is_credit && account.status != 'VOID' %] Void payment [% END %] - [% IF account.is_debit && account.amount == account.amountoutstanding && account.status != 'CANCELLED' %] + [% IF account.is_debit && account.amount == account.amountoutstanding && account.status != 'CANCELLED' && !(account.debit_type_code == 'PAYOUT') %]
--- a/members/cancel-charge.pl +++ a/members/cancel-charge.pl @@ -42,7 +42,12 @@ unless ($csrf_token_is_valid) { my $borrowernumber = $cgi->param('borrowernumber'); my $accountlines_id = $cgi->param('accountlines_id'); -my $line = Koha::Account::Lines->find($accountlines_id); -$line->cancel(); +my $charge = Koha::Account::Lines->find($accountlines_id); +$charge->cancel( + { + branch => C4::Context->userenv->{'branch'}, + staff_id => $user + } +); print $cgi->redirect('/cgi-bin/koha/members/boraccount.pl?borrowernumber=' . $borrowernumber); --- a/t/db_dependent/Koha/Account/Line.t +++ a/t/db_dependent/Koha/Account/Line.t @@ -901,7 +901,7 @@ subtest "payout() tests" => sub { subtest "reduce() tests" => sub { - plan tests => 27; + plan tests => 29; $schema->storage->txn_begin; @@ -961,7 +961,7 @@ subtest "reduce() tests" => sub { my $reduce_params = { interface => 'commandline', - reduction_type => 'REFUND', + reduction_type => 'DISCOUNT', amount => 5, staff_id => $staff->borrowernumber, branch => $branchcode @@ -1009,7 +1009,7 @@ subtest "reduce() tests" => sub { '->reduce() cannot reduce more than original amount'; # Partial Reduction - # (Refund 5 on debt of 20) + # (Discount 5 on debt of 20) my $reduction = $debit1->reduce($reduce_params); is( ref($reduction), 'Koha::Account::Line', @@ -1019,6 +1019,7 @@ subtest "reduce() tests" => sub { 0, "Reduce amountoutstanding is 0" ); is( $debit1->amountoutstanding() * 1, 15, "Debit amountoutstanding reduced by 5 to 15" ); + is( $debit1->status(), 'DISCOUNTED', "Debit status updated to DISCOUNTED"); is( $account->balance() * 1, -5, "Account balance is -5" ); is( $reduction->status(), 'APPLIED', "Reduction status is 'APPLIED'" ); @@ -1028,17 +1029,19 @@ subtest "reduce() tests" => sub { my $THE_offset = $offsets->next; is( $THE_offset->amount * 1, -5, 'Correct amount was applied against debit' ); - is( $THE_offset->type, 'REFUND', "Offset type set to 'REFUND'" ); + is( $THE_offset->type, 'DISCOUNT', "Offset type set to 'DISCOUNT'" ); # Zero offset created when zero outstanding # (Refund another 5 on paid debt of 20) $credit1->apply( { debits => [$debit1] } ); is( $debit1->amountoutstanding + 0, 0, 'Debit1 amountoutstanding reduced to 0' ); + $reduce_params->{reduction_type} = 'REFUND'; $reduction = $debit1->reduce($reduce_params); is( $reduction->amount() * 1, -5, "Reduce amount is -5" ); is( $reduction->amountoutstanding() * 1, -5, "Reduce amountoutstanding is -5" ); + is( $debit1->status(), 'REFUNDED', "Debit status updated to REFUNDED"); $offsets = Koha::Account::Offsets->search( { credit_id => $reduction->id, debit_id => $debit1->id } ); @@ -1077,28 +1080,47 @@ subtest "reduce() tests" => sub { }; subtest "cancel() tests" => sub { - plan tests => 9; + plan tests => 16; $schema->storage->txn_begin; # Create a borrower - my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; - my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; + my $categorycode = + $builder->build( { source => 'Category' } )->{categorycode}; + my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; - my $borrower = Koha::Patron->new( { - cardnumber => 'dariahall', - surname => 'Hall', - firstname => 'Daria', - } ); - $borrower->categorycode( $categorycode ); - $borrower->branchcode( $branchcode ); + my $borrower = Koha::Patron->new( + { + cardnumber => 'dariahall', + surname => 'Hall', + firstname => 'Daria', + } + ); + $borrower->categorycode($categorycode); + $borrower->branchcode($branchcode); $borrower->store; - t::lib::Mocks::mock_userenv({ branchcode => $branchcode, borrowernumber => $borrower->borrowernumber }); + my $staff = Koha::Patron->new( + { + cardnumber => 'bobby', + surname => 'Bloggs', + firstname => 'Bobby', + } + ); + $staff->categorycode($categorycode); + $staff->branchcode($branchcode); + $staff->store; + + t::lib::Mocks::mock_userenv( + { + branchcode => $branchcode, + borrowernumber => $borrower->borrowernumber + } + ); - my $account = Koha::Account->new({ patron_id => $borrower->id }); + my $account = Koha::Account->new( { patron_id => $borrower->id } ); - my $line1 = Koha::Account::Line->new( + my $debit1 = Koha::Account::Line->new( { borrowernumber => $borrower->borrowernumber, amount => 10, @@ -1107,7 +1129,7 @@ subtest "cancel() tests" => sub { debit_type_code => 'OVERDUE', } )->store(); - my $line2 = Koha::Account::Line->new( + my $debit2 = Koha::Account::Line->new( { borrowernumber => $borrower->borrowernumber, amount => 20, @@ -1117,27 +1139,68 @@ subtest "cancel() tests" => sub { } )->store(); - my $id = $account->pay({ - lines => [$line2], - amount => 5, - }); + my $ret = $account->pay( + { + lines => [$debit2], + amount => 5, + } + ); + my $credit = Koha::Account::Lines->find({ accountlines_id => $ret->{payment_id} }); is( $account->balance(), 25, "Account balance is 25" ); - is( $line1->amountoutstanding+0, 10, 'First fee has amount outstanding of 10' ); - is( $line2->amountoutstanding+0, 15, 'Second fee has amount outstanding of 15' ); + is( $debit1->amountoutstanding + 0, + 10, 'First fee has amount outstanding of 10' ); + is( $debit2->amountoutstanding + 0, + 15, 'Second fee has amount outstanding of 15' ); + throws_ok { + $credit->cancel( + { staff_id => $staff->borrowernumber, branch => $branchcode } ); + } + 'Koha::Exceptions::Account::IsNotDebit', + '->cancel() can only be used with debits'; - my $ret = $line1->cancel(); - is( ref($ret), 'Koha::Account::Line', 'Cancel returns the account line' ); - is( $account->balance(), 15, "Account balance is 15" ); - is( $line1->amount+0, 0, 'First fee has amount of 0' ); - is( $line1->amountoutstanding+0, 0, 'First fee has amount outstanding of 0' ); + throws_ok { + $debit1->reduce( { staff_id => $staff->borrowernumber } ); + } + 'Koha::Exceptions::MissingParameter', + "->cancel() requires the `branch` parameter is passed"; + throws_ok { + $debit1->reduce( { branch => $branchcode } ); + } + 'Koha::Exceptions::MissingParameter', + "->cancel() requires the `staff_id` parameter is passed"; + + throws_ok { + $debit2->cancel( + { staff_id => $staff->borrowernumber, branch => $branchcode } ); + } + 'Koha::Exceptions::Account', + '->cancel() can only be used with debits that have not been offset'; - $ret = $line2->cancel(); - is ($ret, undef, 'cancel returns undef when line cannot be cancelled'); + my $cancellation = $debit1->cancel( + { staff_id => $staff->borrowernumber, branch => $branchcode } ); + is( ref($cancellation), 'Koha::Account::Line', + 'Cancel returns an account line' ); + is( + $cancellation->amount() * 1, + $debit1->amount * -1, + "Cancellation amount is " . $debit1->amount + ); + is( $cancellation->amountoutstanding() * 1, + 0, "Cancellation amountoutstanding is 0" ); + is( $debit1->amountoutstanding() * 1, + 0, "Debit amountoutstanding reduced to 0" ); + is( $debit1->status(), 'CANCELLED', "Debit status updated to CANCELLED" ); + is( $account->balance() * 1, 15, "Account balance is 15" ); - my $account_payment = Koha::Account::Lines->find($id); - $ret = $account_payment->cancel(); - is ($ret, undef, 'payment cannot be cancelled'); + my $offsets = Koha::Account::Offsets->search( + { credit_id => $cancellation->id, debit_id => $debit1->id } ); + is( $offsets->count, 1, 'Only one offset is generated' ); + my $THE_offset = $offsets->next; + is( $THE_offset->amount * 1, + -10, 'Correct amount was applied against debit' ); + is( $THE_offset->type, 'CANCELLATION', + "Offset type set to 'CANCELLATION'" ); $schema->storage->txn_rollback; }; --