Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 34153: (QA follow-up) Tidy code

Browse source 
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Kyle Hall 2023-09-20 06:23:36 -04:00 committed by Tomas Cohen Arazi
parent 7f2d38d85e
commit 643e38e835
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
2 changed files with 99 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

65
C4/SIP/ILS/Transaction/Checkout.pm
View file

@ -40,17 +40,17 @@ sub new {
}
sub do_checkout {
my $self = shift;
my $account = shift;
my $no_block_due_date = shift;
siplog('LOG_DEBUG', "ILS::Transaction::Checkout performing checkout...");
my $shelf = $self->{item}->hold_attached;
my $barcode = $self->{item}->id;
my $patron = Koha::Patrons->find($self->{patron}->{borrowernumber});
my $overridden_duedate; # usually passed as undef to AddIssue
my $prevcheckout_block_checkout = $account->{prevcheckout_block_checkout};
my $allow_additional_materials_checkout = $account->{allow_additional_materials_checkout};
my ($issuingimpossible, $needsconfirmation, $messages) = _can_we_issue($patron, $barcode, 0);
my $self = shift;
my $account = shift;
my $no_block_due_date = shift;
siplog( 'LOG_DEBUG', "ILS::Transaction::Checkout performing checkout..." );
my $shelf = $self->{item}->hold_attached;
my $barcode = $self->{item}->id;
my $patron = Koha::Patrons->find( $self->{patron}->{borrowernumber} );
my $overridden_duedate; # usually passed as undef to AddIssue
my $prevcheckout_block_checkout = $account->{prevcheckout_block_checkout};
my $allow_additional_materials_checkout = $account->{allow_additional_materials_checkout};
my ( $issuingimpossible, $needsconfirmation, $messages ) = _can_we_issue( $patron, $barcode, 0 );
if ( $no_block_due_date ) {
my $year = substr($no_block_due_date,0,4);
@ -65,50 +65,52 @@ sub do_checkout {
}
my $noerror=1; # If set to zero we block the issue
if (keys %{$issuingimpossible}) {
foreach (keys %{$issuingimpossible}) {
if ( keys %{$issuingimpossible} ) {
foreach ( keys %{$issuingimpossible} ) {
# do something here so we pass these errors
$self->screen_msg("Issue failed : $_");
$noerror = 0;
last;
}
} else {
foreach my $confirmation (keys %{$needsconfirmation}) {
if ($confirmation eq 'RENEW_ISSUE'){
foreach my $confirmation ( keys %{$needsconfirmation} ) {
if ( $confirmation eq 'RENEW_ISSUE' ) {
$self->screen_msg("Item already checked out to you: renewing item.");
} elsif ($confirmation eq 'RESERVED' and !C4::Context->preference("AllowItemsOnHoldCheckoutSIP")) {
} elsif ( $confirmation eq 'RESERVED' and !C4::Context->preference("AllowItemsOnHoldCheckoutSIP") ) {
$self->screen_msg("Item is reserved for another patron upon return.");
$noerror = 0;
} elsif ($confirmation eq 'RESERVED' and C4::Context->preference("AllowItemsOnHoldCheckoutSIP")) {
} elsif ( $confirmation eq 'RESERVED' and C4::Context->preference("AllowItemsOnHoldCheckoutSIP") ) {
next;
} elsif ($confirmation eq 'RESERVE_WAITING'
or $confirmation eq 'TRANSFERRED'
or $confirmation eq 'PROCESSING') {
$self->screen_msg("Item is on hold for another patron.");
$noerror = 0;
} elsif ($confirmation eq 'ISSUED_TO_ANOTHER') {
} elsif ( $confirmation eq 'RESERVE_WAITING'
or $confirmation eq 'TRANSFERRED'
or $confirmation eq 'PROCESSING' )
{
$self->screen_msg("Item is on hold for another patron.");
$noerror = 0;
} elsif ( $confirmation eq 'ISSUED_TO_ANOTHER' ) {
$self->screen_msg("Item already checked out to another patron. Please return item for check-in.");
$noerror = 0;
last;
} elsif ($confirmation eq 'DEBT') {
} elsif ( $confirmation eq 'DEBT' ) {
$self->screen_msg('Outstanding Fines block issue');
$noerror = 0;
last;
} elsif ($confirmation eq 'HIGHHOLDS') {
} elsif ( $confirmation eq 'HIGHHOLDS' ) {
$overridden_duedate = $needsconfirmation->{$confirmation}->{returndate};
$self->screen_msg('Loan period reduced for high-demand item');
} elsif ($confirmation eq 'RENTALCHARGE') {
if ($self->{fee_ack} ne 'Y') {
} elsif ( $confirmation eq 'RENTALCHARGE' ) {
if ( $self->{fee_ack} ne 'Y' ) {
$noerror = 0;
last;
}
} elsif ($confirmation eq 'PREVISSUE') {
} elsif ( $confirmation eq 'PREVISSUE' ) {
$self->screen_msg("This item was previously checked out by you");
$noerror = 0 if ($prevcheckout_block_checkout);
last;
} elsif ( $confirmation eq 'ADDITIONAL_MATERIALS' ) {
if ( $allow_additional_materials_checkout ) {
my $item = Koha::Items->find({ barcode => $barcode });
if ($allow_additional_materials_checkout) {
my $item = Koha::Items->find( { barcode => $barcode } );
$self->screen_msg( 'Item has additional materials: ' . $item->materials );
} else {
$self->screen_msg('Item must be checked out at a circulation desk');
@ -120,10 +122,11 @@ sub do_checkout {
$noerror = 0;
last;
} else {
# We've been returned a case other than those above
$self->screen_msg("Item cannot be issued: $confirmation");
$noerror = 0;
siplog('LOG_DEBUG', "Blocking checkout Reason:$confirmation");
siplog( 'LOG_DEBUG', "Blocking checkout Reason:$confirmation" );
last;
}
}

105
t/db_dependent/SIP/Message.t
View file

@ -599,64 +599,89 @@ subtest 'test_allow_additional_materials_checkout' => sub {
plan tests => 4;
my $builder = t::lib::TestBuilder->new();
my $branchcode = $builder->build({ source => 'Branch' })->{branchcode};
my $branchcode2 = $builder->build({ source => 'Branch' })->{branchcode};
my $builder = t::lib::TestBuilder->new();
my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode};
my $branchcode2 = $builder->build( { source => 'Branch' } )->{branchcode};
my ( $response, $findpatron );
my $mocks = create_mocks( \$response, \$findpatron, \$branchcode );
# create some data
my $patron1 = $builder->build({
source => 'Borrower',
value => {
password => hash_password( PATRON_PW ),
},
});
my $card1 = $patron1->{cardnumber};
my $sip_patron1 = C4::SIP::ILS::Patron->new( $card1 );
my $patron1 = $builder->build(
{
source => 'Borrower',
value => {
password => hash_password(PATRON_PW),
},
}
);
my $card1 = $patron1->{cardnumber};
my $sip_patron1 = C4::SIP::ILS::Patron->new($card1);
$findpatron = $sip_patron1;
my $item_object = $builder->build_sample_item({
damaged => 0,
withdrawn => 0,
itemlost => 0,
restricted => 0,
homebranch => $branchcode,
holdingbranch => $branchcode,
materials => "This is a materials note",
});
my $item_object = $builder->build_sample_item(
{
damaged => 0,
withdrawn => 0,
itemlost => 0,
restricted => 0,
homebranch => $branchcode,
holdingbranch => $branchcode,
materials => "This is a materials note",
}
);
my $mockILS = $mocks->{ils};
my $server = { ils => $mockILS, account => {} };
my $server = { ils => $mockILS, account => {} };
$mockILS->mock( 'institution', sub { $branchcode; } );
$mockILS->mock( 'supports', sub { return; } );
$mockILS->mock( 'checkout', sub {
shift;
return C4::SIP::ILS->checkout(@_);
});
$mockILS->mock( 'supports', sub { return; } );
$mockILS->mock(
'checkout',
sub {
shift;
return C4::SIP::ILS->checkout(@_);
}
);
my $today = dt_from_string;
t::lib::Mocks::mock_userenv({ branchcode => $branchcode, flags => 1 });
t::lib::Mocks::mock_preference( 'CircConfirmItemParts', '1' );
t::lib::Mocks::mock_userenv( { branchcode => $branchcode, flags => 1 } );
t::lib::Mocks::mock_preference( 'CircConfirmItemParts', '1' );
my $siprequest = CHECKOUT . 'YN' . siprequestdate($today) .
siprequestdate( $today->clone->add( days => 1) ) .
FID_INST_ID . $branchcode . '|'.
FID_PATRON_ID . $sip_patron1->id . '|' .
FID_ITEM_ID . $item_object->barcode . '|' .
FID_TERMINAL_PWD . 'ignored' . '|';
my $siprequest =
CHECKOUT . 'YN'
. siprequestdate($today)
. siprequestdate( $today->clone->add( days => 1 ) )
. FID_INST_ID
. $branchcode . '|'
. FID_PATRON_ID
. $sip_patron1->id . '|'
. FID_ITEM_ID
. $item_object->barcode . '|'
. FID_TERMINAL_PWD
. 'ignored' . '|';
undef $response;
my $msg = C4::SIP::Sip::MsgType->new( $siprequest, 0 );
$server->{account}->{allow_additional_materials_checkout} = 0;
$msg->handle_checkout( $server );
$msg->handle_checkout($server);
my $respcode = substr( $response, 0, 2 );
check_field( $respcode, $response, FID_SCREEN_MSG, 'Item must be checked out at a circulation desk', 'Check screen msg', 'equals' );
is( Koha::Checkouts->search({ itemnumber => $item_object->id })->count, 0, "Item was not checked out (allow_additional_materials_checkout disabled)");
check_field(
$respcode, $response, FID_SCREEN_MSG, 'Item must be checked out at a circulation desk',
'Check screen msg', 'equals'
);
is(
Koha::Checkouts->search( { itemnumber => $item_object->id } )->count, 0,
"Item was not checked out (allow_additional_materials_checkout disabled)"
);
$server->{account}->{allow_additional_materials_checkout} = 1;
$msg->handle_checkout( $server );
$msg->handle_checkout($server);
$respcode = substr( $response, 0, 2 );
check_field( $respcode, $response, FID_SCREEN_MSG, 'Item has additional materials: This is a materials note', 'Check screen msg', 'equals' );
is( Koha::Checkouts->search({ itemnumber => $item_object->id })->count, 1, "Item was checked out (allow_additional_materials_checkout enabled");
check_field(
$respcode, $response, FID_SCREEN_MSG, 'Item has additional materials: This is a materials note',
'Check screen msg', 'equals'
);
is(
Koha::Checkouts->search( { itemnumber => $item_object->id } )->count, 1,
"Item was checked out (allow_additional_materials_checkout enabled"
);
};
# Here is room for some more subtests