97f6d9a087
This fixes regression caused by "Bug 12362: Cancel transfer with hold cancelation" where cancelled hold's transfer didn't show up in intranet and opac because it create a new transfer that was not yet put in in-transit state. The original idea of bug 12362 was to be able to trigger transfer back home if a hold was cancelled (a regression caused by bug 26078). However, we can do it more simply by setting the $validTransfer variable true in the item check-in code when we are dealing with Reserve transfers. More down in the AddReturn() code there is also a check "and !$resfound" to make sure we only try to trigger the transfer back home automatically if there is no hold waiting at the current location the item arrived in. It should be noted however that now we only display generic message for the automatic transfer reason. Bug 12362 made the return display as the reason "Transfer was cancelled whilst in transit". However, since this fixes the original regressions caused by bug 26078 and restores similar behaviour to that I think giving a more descriptive message for example regarding a hold being cancelled can be considered a further enhancement. To test: 1) Apply patch 1) Have biblio with item in branch A 2) Create a new hold with a pickup library to branch B 3) Check-in the item at branch A and confirm the hold and transfer 4) Cancel the hold 5) Check-in the hold at branch B and notice it prompt to return it to branch A Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
329 lines
11 KiB
Perl
Executable file
329 lines
11 KiB
Perl
Executable file
#!/usr/bin/perl
|
|
|
|
# This file is part of Koha.
|
|
#
|
|
# Koha is free software; you can redistribute it and/or modify it
|
|
# under the terms of the GNU General Public License as published by
|
|
# the Free Software Foundation; either version 3 of the License, or
|
|
# (at your option) any later version.
|
|
#
|
|
# Koha is distributed in the hope that it will be useful, but
|
|
# WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
# GNU General Public License for more details.
|
|
#
|
|
# You should have received a copy of the GNU General Public License
|
|
# along with Koha; if not, see <http://www.gnu.org/licenses>.
|
|
|
|
use Modern::Perl;
|
|
|
|
use C4::Circulation;
|
|
use C4::Items;
|
|
use C4::Biblio;
|
|
use C4::Context;
|
|
use Koha::CirculationRules;
|
|
|
|
use Koha::Patrons;
|
|
|
|
use Test::More tests => 16;
|
|
use t::lib::Mocks;
|
|
use t::lib::TestBuilder;
|
|
|
|
BEGIN {
|
|
use_ok('C4::Circulation');
|
|
}
|
|
|
|
can_ok( 'C4::Circulation', qw(
|
|
AddIssue
|
|
AddReturn
|
|
GetBranchBorrowerCircRule
|
|
GetBranchItemRule
|
|
)
|
|
);
|
|
|
|
my $schema = Koha::Database->schema;
|
|
$schema->storage->txn_begin;
|
|
my $dbh = C4::Context->dbh;
|
|
my $query;
|
|
|
|
$dbh->do(q|DELETE FROM issues|);
|
|
$dbh->do(q|DELETE FROM items|);
|
|
$dbh->do(q|DELETE FROM borrowers|);
|
|
$dbh->do(q|DELETE FROM clubs|);
|
|
$dbh->do(q|DELETE FROM branches|);
|
|
$dbh->do(q|DELETE FROM categories|);
|
|
$dbh->do(q|DELETE FROM accountlines|);
|
|
$dbh->do(q|DELETE FROM itemtypes|);
|
|
$dbh->do(q|DELETE FROM circulation_rules|);
|
|
|
|
my $builder = t::lib::TestBuilder->new();
|
|
|
|
# Add branch
|
|
my $samplebranch1 = $builder->build({ source => 'Branch' });
|
|
my $samplebranch2 = $builder->build({ source => 'Branch' });
|
|
# Add itemtypes
|
|
my $no_circ_itemtype = $builder->build({
|
|
source => 'Itemtype',
|
|
value => {
|
|
rentalcharge => '0',
|
|
notforloan => 0
|
|
}
|
|
});
|
|
my $sampleitemtype1 = $builder->build({
|
|
source => 'Itemtype',
|
|
value => {
|
|
rentalcharge => '10.0',
|
|
notforloan => 1
|
|
}
|
|
});
|
|
my $sampleitemtype2 = $builder->build({
|
|
source => 'Itemtype',
|
|
value => {
|
|
rentalcharge => '5.0',
|
|
notforloan => 0
|
|
}
|
|
});
|
|
# Add Category
|
|
my $samplecat = $builder->build({
|
|
source => 'Category',
|
|
value => {
|
|
hidelostitems => 0
|
|
}
|
|
});
|
|
|
|
#Add biblio and item
|
|
my $record = MARC::Record->new();
|
|
$record->append_fields(
|
|
MARC::Field->new( '952', '0', '0', a => $samplebranch1->{branchcode} ) );
|
|
my ( $biblionumber, $biblioitemnumber ) = C4::Biblio::AddBiblio( $record, '' );
|
|
|
|
# item 1 has home branch and holding branch samplebranch1
|
|
my $item_id1 = Koha::Item->new(
|
|
{
|
|
biblionumber => $biblionumber,
|
|
barcode => 'barcode_1',
|
|
itemcallnumber => 'callnumber1',
|
|
homebranch => $samplebranch1->{branchcode},
|
|
holdingbranch => $samplebranch1->{branchcode},
|
|
itype => $no_circ_itemtype->{ itemtype }
|
|
}
|
|
)->store->itemnumber;
|
|
|
|
# item 2 has holding branch samplebranch2
|
|
my $item_id2 = Koha::Item->new(
|
|
{
|
|
biblionumber => $biblionumber,
|
|
barcode => 'barcode_2',
|
|
itemcallnumber => 'callnumber2',
|
|
homebranch => $samplebranch2->{branchcode},
|
|
holdingbranch => $samplebranch1->{branchcode},
|
|
itype => $no_circ_itemtype->{ itemtype }
|
|
},
|
|
)->store->itemnumber;
|
|
|
|
# item 3 has item type sampleitemtype2 with noreturn policy
|
|
my $item_id3 = Koha::Item->new(
|
|
{
|
|
biblionumber => $biblionumber,
|
|
barcode => 'barcode_3',
|
|
itemcallnumber => 'callnumber3',
|
|
homebranch => $samplebranch2->{branchcode},
|
|
holdingbranch => $samplebranch2->{branchcode},
|
|
itype => $sampleitemtype2->{itemtype}
|
|
}
|
|
)->store->itemnumber;
|
|
|
|
#Add borrower
|
|
my $borrower_id1 = Koha::Patron->new({
|
|
firstname => 'firstname1',
|
|
surname => 'surname1 ',
|
|
categorycode => $samplecat->{categorycode},
|
|
branchcode => $samplebranch1->{branchcode},
|
|
})->store->borrowernumber;
|
|
|
|
is_deeply(
|
|
GetBranchBorrowerCircRule(),
|
|
{ patron_maxissueqty => undef, patron_maxonsiteissueqty => undef },
|
|
"Without parameter, GetBranchBorrower returns undef (unilimited) for patron_maxissueqty and patron_maxonsiteissueqty if no rules defined"
|
|
);
|
|
|
|
Koha::CirculationRules->set_rules(
|
|
{
|
|
branchcode => $samplebranch1->{branchcode},
|
|
categorycode => $samplecat->{categorycode},
|
|
rules => {
|
|
patron_maxissueqty => 5,
|
|
patron_maxonsiteissueqty => 6,
|
|
}
|
|
}
|
|
);
|
|
|
|
|
|
Koha::CirculationRules->set_rules(
|
|
{
|
|
branchcode => $samplebranch2->{branchcode},
|
|
categorycode => undef,
|
|
rules => {
|
|
patron_maxissueqty => 3,
|
|
patron_maxonsiteissueqty => 2,
|
|
}
|
|
}
|
|
);
|
|
Koha::CirculationRules->set_rules(
|
|
{
|
|
branchcode => $samplebranch2->{branchcode},
|
|
itemtype => undef,
|
|
rules => {
|
|
holdallowed => 'from_home_library',
|
|
returnbranch => 'holdingbranch',
|
|
}
|
|
}
|
|
);
|
|
|
|
Koha::CirculationRules->set_rules(
|
|
{
|
|
branchcode => undef,
|
|
categorycode => undef,
|
|
rules => {
|
|
patron_maxissueqty => 4,
|
|
patron_maxonsiteissueqty => 5,
|
|
}
|
|
}
|
|
);
|
|
Koha::CirculationRules->set_rules(
|
|
{
|
|
branchcode => undef,
|
|
itemtype => undef,
|
|
rules => {
|
|
holdallowed => 'from_local_hold_group',
|
|
returnbranch => 'homebranch',
|
|
}
|
|
}
|
|
);
|
|
|
|
Koha::CirculationRules->set_rules(
|
|
{
|
|
branchcode => $samplebranch1->{branchcode},
|
|
itemtype => $sampleitemtype1->{itemtype},
|
|
rules => {
|
|
holdallowed => 'invalid_value',
|
|
returnbranch => 'homebranch',
|
|
}
|
|
}
|
|
);
|
|
Koha::CirculationRules->set_rules(
|
|
{
|
|
branchcode => $samplebranch2->{branchcode},
|
|
itemtype => $sampleitemtype1->{itemtype},
|
|
rules => {
|
|
holdallowed => 'invalid_value',
|
|
returnbranch => 'holdingbranch',
|
|
}
|
|
}
|
|
);
|
|
Koha::CirculationRules->set_rules(
|
|
{
|
|
branchcode => $samplebranch2->{branchcode},
|
|
itemtype => $sampleitemtype2->{itemtype},
|
|
rules => {
|
|
holdallowed => 'invalid_value',
|
|
returnbranch => 'noreturn',
|
|
}
|
|
}
|
|
);
|
|
|
|
#Test GetBranchBorrowerCircRule
|
|
is_deeply(
|
|
GetBranchBorrowerCircRule(),
|
|
{ patron_maxissueqty => 4, patron_maxonsiteissueqty => 5 },
|
|
"Without parameter, GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty of default_circ_rules"
|
|
);
|
|
is_deeply(
|
|
GetBranchBorrowerCircRule( $samplebranch2->{branchcode} ),
|
|
{ patron_maxissueqty => 3, patron_maxonsiteissueqty => 2 },
|
|
"Without only the branchcode specified, GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty corresponding"
|
|
);
|
|
is_deeply(
|
|
GetBranchBorrowerCircRule(
|
|
$samplebranch1->{branchcode},
|
|
$samplecat->{categorycode}
|
|
),
|
|
{ patron_maxissueqty => 5, patron_maxonsiteissueqty => 6 },
|
|
"GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty of the branch1 and the category1"
|
|
);
|
|
is_deeply(
|
|
GetBranchBorrowerCircRule( -1, -1 ),
|
|
{ patron_maxissueqty => 4, patron_maxonsiteissueqty => 5 },
|
|
"GetBranchBorrower with wrong parameters returns the patron_maxissueqty and patron_maxonsiteissueqty of default_circ_rules"
|
|
);
|
|
|
|
#Test GetBranchItemRule
|
|
my @lazy_any = ( 'hold_fulfillment_policy' => 'any' );
|
|
is_deeply(
|
|
GetBranchItemRule(
|
|
$samplebranch1->{branchcode},
|
|
$sampleitemtype1->{itemtype},
|
|
),
|
|
{ returnbranch => 'homebranch', holdallowed => 'invalid_value', @lazy_any },
|
|
"GetBranchitem returns holdallowed and return branch"
|
|
);
|
|
is_deeply(
|
|
GetBranchItemRule(),
|
|
{ returnbranch => 'homebranch', holdallowed => 'from_local_hold_group', @lazy_any },
|
|
"Without parameters GetBranchItemRule returns the values in default_circ_rules"
|
|
);
|
|
is_deeply(
|
|
GetBranchItemRule( $samplebranch2->{branchcode} ),
|
|
{ returnbranch => 'holdingbranch', holdallowed => 'from_home_library', @lazy_any },
|
|
"With only a branchcode GetBranchItemRule returns values in default_branch_circ_rules"
|
|
);
|
|
is_deeply(
|
|
GetBranchItemRule( -1, -1 ),
|
|
{ returnbranch => 'homebranch', holdallowed => 'from_local_hold_group', @lazy_any },
|
|
"With only one parametern GetBranchItemRule returns default values"
|
|
);
|
|
|
|
# Test return policies
|
|
t::lib::Mocks::mock_preference('AutomaticItemReturn','0');
|
|
|
|
# item1 returned at branch2 should trigger transfer to homebranch
|
|
$query =
|
|
"INSERT INTO issues (borrowernumber,itemnumber,branchcode) VALUES( ?,?,? )";
|
|
$dbh->do( $query, {}, $borrower_id1, $item_id1, $samplebranch1->{branchcode} );
|
|
|
|
my ($doreturn, $messages, $iteminformation, $borrower) = AddReturn('barcode_1',
|
|
$samplebranch2->{branchcode});
|
|
is( $messages->{NeedsTransfer}, $samplebranch1->{branchcode}, "AddReturn respects default return policy - return to homebranch" );
|
|
|
|
# item2 returned at branch2 should trigger transfer to holding branch
|
|
$query =
|
|
"INSERT INTO issues (borrowernumber,itemnumber,branchcode) VALUES( ?,?,? )";
|
|
$dbh->do( $query, {}, $borrower_id1, $item_id2, $samplebranch2->{branchcode} );
|
|
($doreturn, $messages, $iteminformation, $borrower) = AddReturn('barcode_2',
|
|
$samplebranch2->{branchcode});
|
|
is( $messages->{NeedsTransfer}, $samplebranch1->{branchcode}, "AddReturn respects branch return policy - item2->homebranch policy = 'holdingbranch'" );
|
|
|
|
# Generate the transfer from above
|
|
ModItemTransfer($item_id2, $samplebranch2->{branchcode}, $samplebranch1->{branchcode}, "ReturnToHolding");
|
|
# Fulfill it
|
|
($doreturn, $messages, $iteminformation, $borrower) = AddReturn('barcode_2',$samplebranch1->{branchcode});
|
|
is( $messages->{NeedsTransfer}, undef, "AddReturn does not generate a new transfer for return policy when resolving an existing non-Reserve transfer" );
|
|
|
|
# Generate a hold caused transfer which doesn't have a hold i.e. is the hold is cancelled
|
|
ModItemTransfer($item_id2, $samplebranch2->{branchcode}, $samplebranch1->{branchcode}, "Reserve");
|
|
# Fulfill it
|
|
($doreturn, $messages, $iteminformation, $borrower) = AddReturn('barcode_2',$samplebranch1->{branchcode});
|
|
is( $messages->{NeedsTransfer}, $samplebranch2->{branchcode}, "AddReturn generates a new transfer for hold transfer if the hold was cancelled" );
|
|
|
|
# item3 should not trigger transfer - floating collection
|
|
$query =
|
|
"INSERT INTO issues (borrowernumber,itemnumber,branchcode) VALUES( ?,?,? )";
|
|
$dbh->do( $query, {}, $borrower_id1, $item_id3, $samplebranch1->{branchcode} );
|
|
t::lib::Mocks::mock_preference( 'item-level_itypes', 1 );
|
|
($doreturn, $messages, $iteminformation, $borrower) = AddReturn('barcode_3',
|
|
$samplebranch1->{branchcode});
|
|
is($messages->{NeedsTransfer},undef,"AddReturn respects branch item return policy - noreturn");
|
|
t::lib::Mocks::mock_preference( 'item-level_itypes', 0 );
|
|
|
|
$schema->storage->txn_rollback;
|
|
|