Bugzilla – Attachment 64449 Details for
Bug 18651
Move of checkouts is still not correctly handled
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18651: Do not charge if the checkin failed
Bug-18651-Do-not-charge-if-the-checkin-failed.patch (text/plain), 4.84 KB, created by
Marcel de Rooy
on 2017-06-20 13:15:25 UTC
(
hide
)
Description:
Bug 18651: Do not charge if the checkin failed
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2017-06-20 13:15:25 UTC
Size:
4.84 KB
patch
obsolete
>From ef970b56fc3e596393c91e7f7caa27dfa7f2f366 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Mon, 22 May 2017 14:26:25 -0300 >Subject: [PATCH] Bug 18651: Do not charge if the checkin failed >Content-Type: text/plain; charset=utf-8 > >2. If the move fails for whatever reason (see >https://lists.katipo.co.nz/pipermail/koha/2017-May/048045.html for an >example), fines can be charged. It should not > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > C4/Circulation.pm | 10 +++++----- > t/db_dependent/Circulation/Returns.t | 27 ++++++++++++++++++++------- > 2 files changed, 25 insertions(+), 12 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 41f6c70..7e142ef 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1934,16 +1934,16 @@ sub AddReturn { > } > > if ($borrowernumber) { >- if ( ( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'} ) || $return_date ) { >- _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $borrower, return_date => $return_date } ); >- } >- > eval { > my $issue_id = MarkIssueReturned( $borrowernumber, $item->{'itemnumber'}, > $circControlBranch, $return_date, $borrower->{'privacy'} ); > $issue->{issue_id} = $issue_id; > }; >- if ( $@ ) { >+ unless ( $@ ) { >+ if ( ( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'} ) || $return_date ) { >+ _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $borrower, return_date => $return_date } ); >+ } >+ } else { > $messages->{'Wrongbranch'} = { > Wrongbranch => $branch, > Rightbranch => $message >diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t >index fe98452..9d295e5 100644 >--- a/t/db_dependent/Circulation/Returns.t >+++ b/t/db_dependent/Circulation/Returns.t >@@ -29,6 +29,7 @@ use C4::Circulation; > use C4::Items; > use C4::Members; > use Koha::Database; >+use Koha::Account::Lines; > use Koha::DateUtils; > use Koha::Items; > >@@ -263,9 +264,15 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub { > }; > > subtest 'Handle ids duplication' => sub { >- plan tests => 2; >+ plan tests => 4; >+ >+ t::lib::Mocks::mock_preference( 'item-level_itypes', 1 ); >+ t::lib::Mocks::mock_preference( 'CalculateFinesOnReturn', 1 ); >+ t::lib::Mocks::mock_preference( 'finesMode', 'production' ); >+ Koha::IssuingRules->search->update({ chargeperiod => 1, fine => 1, firstremind => 1, }); > > my $biblio = $builder->build( { source => 'Biblio' } ); >+ my $itemtype = $builder->build( { source => 'Itemtype', value => { rentalcharge => 5 } } ); > my $item = $builder->build( > { > source => 'Item', >@@ -274,19 +281,25 @@ subtest 'Handle ids duplication' => sub { > notforloan => 0, > itemlost => 0, > withdrawn => 0, >- >+ itype => $itemtype->{itemtype}, > } > } > ); > my $patron = $builder->build({source => 'Borrower'}); >+ $patron = Koha::Patrons->find( $patron->{borrowernumber} ); >+ >+ my $original_checkout = AddIssue( $patron->unblessed, $item->{barcode}, dt_from_string->subtract( days => 50 ) ); >+ $builder->build({ source => 'OldIssue', value => { issue_id => $original_checkout->issue_id } }); >+ my $old_checkout = Koha::Old::Checkouts->find( $original_checkout->issue_id ); >+ >+ AddRenewal( $patron->borrowernumber, $item->{itemnumber} ); > >- my $checkout = AddIssue( $patron, $item->{barcode} ); >- $builder->build({ source => 'OldIssue', value => { issue_id => $checkout->issue_id } }); >+ my ($doreturn, $messages, $new_checkout, $borrower) = AddReturn( $item->{barcode}, undef, undef, undef, dt_from_string ); > >- my ($doreturn, $messages, $issue, $borrower) = AddReturn( $item->{barcode} ); >- my $old_checkout = Koha::Old::Checkouts->find( $checkout->issue_id ); >+ my $account_lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber, issue_id => $new_checkout->{issue_id} }); >+ is( $account_lines->count, 1, 'One account line should exist on new issue_id' ); > >- isnt( $checkout->issue_id, $issue->{issue_id}, 'AddReturn should return the issue with the new issue_id' ); >+ isnt( $original_checkout->issue_id, $new_checkout->{issue_id}, 'AddReturn should return the issue with the new issue_id' ); > isnt( $old_checkout->itemnumber, $item->{itemnumber}, 'If an item is checked-in, it should be moved to old_issues even if the issue_id already existed in the table' ); > }; > >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18651
:
63629
|
63630
|
63631
|
63632
|
63633
|
63634
|
63635
|
63636
|
63637
|
63654
|
63659
|
63706
|
64352
|
64353
|
64354
|
64355
|
64356
|
64357
|
64394
|
64395
|
64396
|
64397
|
64398
|
64399
|
64448
| 64449 |
64450
|
64451
|
64452
|
64453
|
64454
|
64455
|
64489
|
64490
|
64495
|
65134