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

Bug 25393: (QA follow-up) Tidy

Browse source 
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Nick Clemens 2023-10-11 14:40:37 +00:00 committed by Tomas Cohen Arazi
parent 448aec3fc9
commit 5b18fa1fc2
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
5 changed files with 143 additions and 134 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

80
admin/smart-rules.pl
View file

@ -266,8 +266,8 @@ elsif ($op eq 'add') {
my $maxonsiteissueqty = strip_non_numeric( scalar $input->param('maxonsiteissueqty') );
my $renewalsallowed = $input->param('renewalsallowed');
my $unseen_renewals_allowed = defined $input->param('unseen_renewals_allowed') ? strip_non_numeric( scalar $input->param('unseen_renewals_allowed') ) : q{};
my $renewalperiod = $input->param('renewalperiod');
my $norenewalbefore = $input->param('norenewalbefore');
my $renewalperiod = $input->param('renewalperiod');
my $norenewalbefore = $input->param('norenewalbefore');
$norenewalbefore = q{} if $norenewalbefore =~ /^\s*$/;
my $noautorenewalbefore = $input->param('noautorenewalbefore');
$noautorenewalbefore = q{} if $noautorenewalbefore =~ /^\s*$/;
@ -300,45 +300,45 @@ elsif ($op eq 'add') {
my $recall_shelf_time = $input->param('recall_shelf_time');
my $rules = {
maxissueqty => $maxissueqty,
maxonsiteissueqty => $maxonsiteissueqty,
rentaldiscount => $rentaldiscount,
fine => $fine,
finedays => $finedays,
maxsuspensiondays => $maxsuspensiondays,
suspension_chargeperiod => $suspension_chargeperiod,
firstremind => $firstremind,
chargeperiod => $chargeperiod,
chargeperiod_charge_at => $chargeperiod_charge_at,
issuelength => $issuelength,
daysmode => $daysmode,
lengthunit => $lengthunit,
hardduedate => $hardduedate,
hardduedatecompare => $hardduedatecompare,
renewalsallowed => $renewalsallowed,
unseen_renewals_allowed => $unseen_renewals_allowed,
renewalperiod => $renewalperiod,
norenewalbefore => $norenewalbefore,
noautorenewalbefore => $noautorenewalbefore,
auto_renew => $auto_renew,
no_auto_renewal_after => $no_auto_renewal_after,
maxissueqty => $maxissueqty,
maxonsiteissueqty => $maxonsiteissueqty,
rentaldiscount => $rentaldiscount,
fine => $fine,
finedays => $finedays,
maxsuspensiondays => $maxsuspensiondays,
suspension_chargeperiod => $suspension_chargeperiod,
firstremind => $firstremind,
chargeperiod => $chargeperiod,
chargeperiod_charge_at => $chargeperiod_charge_at,
issuelength => $issuelength,
daysmode => $daysmode,
lengthunit => $lengthunit,
hardduedate => $hardduedate,
hardduedatecompare => $hardduedatecompare,
renewalsallowed => $renewalsallowed,
unseen_renewals_allowed => $unseen_renewals_allowed,
renewalperiod => $renewalperiod,
norenewalbefore => $norenewalbefore,
noautorenewalbefore => $noautorenewalbefore,
auto_renew => $auto_renew,
no_auto_renewal_after => $no_auto_renewal_after,
no_auto_renewal_after_hard_limit => $no_auto_renewal_after_hard_limit,
reservesallowed => $reservesallowed,
holds_per_record => $holds_per_record,
holds_per_day => $holds_per_day,
onshelfholds => $onshelfholds,
opacitemholds => $opacitemholds,
overduefinescap => $overduefinescap,
cap_fine_to_replacement_price => $cap_fine_to_replacement_price,
article_requests => $article_requests,
note => $note,
decreaseloanholds => $decreaseloanholds,
recalls_allowed => $recalls_allowed,
recalls_per_record => $recalls_per_record,
on_shelf_recalls => $on_shelf_recalls,
recall_due_date_interval => $recall_due_date_interval,
recall_overdue_fine => $recall_overdue_fine,
recall_shelf_time => $recall_shelf_time,
reservesallowed => $reservesallowed,
holds_per_record => $holds_per_record,
holds_per_day => $holds_per_day,
onshelfholds => $onshelfholds,
opacitemholds => $opacitemholds,
overduefinescap => $overduefinescap,
cap_fine_to_replacement_price => $cap_fine_to_replacement_price,
article_requests => $article_requests,
note => $note,
decreaseloanholds => $decreaseloanholds,
recalls_allowed => $recalls_allowed,
recalls_per_record => $recalls_per_record,
on_shelf_recalls => $on_shelf_recalls,
recall_due_date_interval => $recall_due_date_interval,
recall_overdue_fine => $recall_overdue_fine,
recall_shelf_time => $recall_shelf_time,
};
Koha::CirculationRules->set_rules(

11
installer/data/mysql/atomicupdate/bug_25393.pl
View file

@ -3,7 +3,7 @@ use Modern::Perl;
return {
bug_number => "25393",
description => "Create separate 'no automatic renewal before' rule",
up => sub {
up => sub {
my ($args) = @_;
my ( $dbh, $out ) = @$args{qw(dbh out)};
@ -12,7 +12,7 @@ return {
{ Slice => {} }
);
if(!scalar @{$rules}){
if ( !scalar @{$rules} ) {
my $existing_rules = $dbh->selectall_arrayref(
q|SELECT * FROM circulation_rules WHERE rule_name = "norenewalbefore"|,
{ Slice => {} }
@ -31,10 +31,11 @@ return {
$existing_rule->{rule_value}
);
}
say $out "Bug 25939: New circulation rule 'noautorenewalbefore' has been added. Defaulting value to 'norenewalbefore'.";
}else{
say $out
"Bug 25939: New circulation rule 'noautorenewalbefore' has been added. Defaulting value to 'norenewalbefore'.";
} else {
say $out "Bug 25939: Circulation rule 'noautorenewalbefore' found. Skipping update.";
}
},
},
};

55
misc/cronjobs/automatic_renewals.pl
View file

@ -178,46 +178,47 @@ while ( my $auto_renew = $auto_renews->next ) {
$updated = 1;
if ($verbose) {
say sprintf "Issue id: %s for borrower: %s and item: %s %s be renewed.",
$auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, $confirm ? 'will' : 'would';
$auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber,
$confirm ? 'will' : 'would';
}
if ($confirm){
if ($confirm) {
my $date_due = AddRenewal(
{
borrowernumber => $auto_renew->borrowernumber,
itemnumber => $auto_renew->itemnumber,
branch => $auto_renew->branchcode,
seen => 0,
automatic => 1,
borrowernumber => $auto_renew->borrowernumber,
itemnumber => $auto_renew->itemnumber,
branch => $auto_renew->branchcode,
seen => 0,
automatic => 1,
}
);
push @item_renewal_ids, $auto_renew->itemnumber;
$auto_renew->auto_renew_error(undef)->store;
}
push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew
if ( $wants_messages ) && !$wants_digest;
} elsif (
$error eq 'too_many' ||
$error eq 'on_reserve' ||
$error eq 'restriction' ||
$error eq 'overdue' ||
$error eq 'too_unseen' ||
$error eq 'auto_account_expired' ||
$error eq 'auto_too_late' ||
$error eq 'auto_too_much_oweing' ||
$error eq 'auto_too_soon' ||
$error eq 'too_soon' ||
$error eq 'item_denied_renewal' ||
$error eq 'item_issued_to_other_patron'
) {
if ( $verbose ) {
if ($wants_messages) && !$wants_digest;
} elsif ( $error eq 'too_many'
|| $error eq 'on_reserve'
|| $error eq 'restriction'
|| $error eq 'overdue'
|| $error eq 'too_unseen'
|| $error eq 'auto_account_expired'
|| $error eq 'auto_too_late'
|| $error eq 'auto_too_much_oweing'
|| $error eq 'auto_too_soon'
|| $error eq 'too_soon'
|| $error eq 'item_denied_renewal'
|| $error eq 'item_issued_to_other_patron' )
{
if ($verbose) {
say sprintf "Issue id: %s for borrower: %s and item: %s %s not be renewed. (%s)",
$auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, $confirm ? 'will' : 'would', $error;
$auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber,
$confirm ? 'will' : 'would', $error;
}
$updated = 1 if (!$auto_renew->auto_renew_error || $error ne $auto_renew->auto_renew_error);
if ( $updated ) {
$updated = 1 if ( !$auto_renew->auto_renew_error || $error ne $auto_renew->auto_renew_error );
if ($updated) {
$auto_renew->auto_renew_error($error)->store if $confirm;
push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew
if $error ne 'auto_too_soon' && ( $wants_messages && !$wants_digest ); # Do not notify if it's too soon
if $error ne 'auto_too_soon' && ( $wants_messages && !$wants_digest ); # Do not notify if it's too soon
}
}

104
svc/checkouts
View file

@ -240,67 +240,71 @@ while ( my $c = $sth->fetchrow_hashref() ) {
}
my $checkout = {
DT_RowId => $c->{itemnumber} . '-' . $c->{borrowernumber},
title => $c->{title},
subtitle => \@subtitles,
medium => $c->{medium} // '',
part_number => $c->{part_number} // '',
part_name => $c->{part_name} // '',
author => $c->{author},
barcode => $c->{barcode},
DT_RowId => $c->{itemnumber} . '-' . $c->{borrowernumber},
title => $c->{title},
subtitle => \@subtitles,
medium => $c->{medium} // '',
part_number => $c->{part_number} // '',
part_name => $c->{part_name} // '',
author => $c->{author},
barcode => $c->{barcode},
type_for_stat => $type_for_stat || q{},
itemtype_description => $itemtype || q{},
recordtype_description => $recordtype || q{},
collection => $collection,
location => $location,
homebranch => $c->{homebranch},
itemnotes => $c->{itemnotes},
itemnotes_nonpublic => $c->{itemnotes_nonpublic},
branchcode => $c->{branchcode},
branchname => $c->{branchname},
itemcallnumber => $c->{itemcallnumber} || q{},
copynumber => $c->{copynumber} || q{},
charge => $charge,
fine => $fine,
price => $c->{replacementprice} || q{},
can_renew => $can_renew,
can_renew_error => $can_renew_error,
can_renew_date => $can_renew_date,
itemnumber => $c->{itemnumber},
borrowernumber => $c->{borrowernumber},
biblionumber => $c->{biblionumber},
issuedate => $c->{issuedate},
date_due => $c->{date_due},
date_due_overdue => $c->{date_due_overdue} ? JSON::true : JSON::false,
timestamp => $c->{timestamp},
auto_renew => $c->{auto_renew},
onsite_checkout => $c->{onsite_checkout},
enumchron => $c->{enumchron},
renewals_count => $renewals_count,
renewals_allowed => $renewals_allowed || 0,
renewals_remaining => $renewals_remaining,
unseen_count => $unseen_count,
unseen_allowed => $unseen_allowed,
unseen_remaining => $unseen_remaining,
collection => $collection,
location => $location,
homebranch => $c->{homebranch},
itemnotes => $c->{itemnotes},
itemnotes_nonpublic => $c->{itemnotes_nonpublic},
branchcode => $c->{branchcode},
branchname => $c->{branchname},
itemcallnumber => $c->{itemcallnumber} || q{},
copynumber => $c->{copynumber} || q{},
charge => $charge,
fine => $fine,
price => $c->{replacementprice} || q{},
can_renew => $can_renew,
can_renew_error => $can_renew_error,
can_renew_date => $can_renew_date,
itemnumber => $c->{itemnumber},
borrowernumber => $c->{borrowernumber},
biblionumber => $c->{biblionumber},
issuedate => $c->{issuedate},
date_due => $c->{date_due},
date_due_overdue => $c->{date_due_overdue} ? JSON::true : JSON::false,
timestamp => $c->{timestamp},
auto_renew => $c->{auto_renew},
onsite_checkout => $c->{onsite_checkout},
enumchron => $c->{enumchron},
renewals_count => $renewals_count,
renewals_allowed => $renewals_allowed || 0,
renewals_remaining => $renewals_remaining,
unseen_count => $unseen_count,
unseen_allowed => $unseen_allowed,
unseen_remaining => $unseen_remaining,
return_claim_id => $c->{return_claim_id},
return_claim_notes => $c->{return_claim_notes},
return_claim_created_on => $c->{return_claim_created_on},
return_claim_updated_on => $c->{return_claim_updated_on},
return_claim_created_on_formatted => $c->{return_claim_created_on} ? output_pref({ dt => dt_from_string( $c->{return_claim_created_on} ) }) : undef,
return_claim_updated_on_formatted => $c->{return_claim_updated_on} ? output_pref({ dt => dt_from_string( $c->{return_claim_updated_on} ) }) : undef,
return_claim_id => $c->{return_claim_id},
return_claim_notes => $c->{return_claim_notes},
return_claim_created_on => $c->{return_claim_created_on},
return_claim_updated_on => $c->{return_claim_updated_on},
return_claim_created_on_formatted => $c->{return_claim_created_on}
? output_pref( { dt => dt_from_string( $c->{return_claim_created_on} ) } )
: undef,
return_claim_updated_on_formatted => $c->{return_claim_updated_on}
? output_pref( { dt => dt_from_string( $c->{return_claim_updated_on} ) } )
: undef,
lost => $lost,
lost => $lost,
claims_returned => $claims_returned,
damaged => $damaged,
materials => $materials,
borrower => {
damaged => $damaged,
materials => $materials,
borrower => {
surname => $c->{surname},
firstname => $c->{firstname},
cardnumber => $c->{cardnumber},
},
issued_today => !$c->{not_issued_today},
recalled => $recalled,
recalled => $recalled,
};
if ( $c->{not_issued_today} ) {

27
t/db_dependent/Circulation.t
View file

@ -278,16 +278,16 @@ Koha::CirculationRules->set_rules(
branchcode => undef,
itemtype => undef,
rules => {
reservesallowed => 25,
issuelength => 14,
lengthunit => 'days',
renewalsallowed => 1,
renewalperiod => 7,
reservesallowed => 25,
issuelength => 14,
lengthunit => 'days',
renewalsallowed => 1,
renewalperiod => 7,
norenewalbefore => undef,
noautorenewalbefore => undef,
auto_renew => 0,
fine => .10,
chargeperiod => 1,
auto_renew => 0,
fine => .10,
chargeperiod => 1,
}
}
);
@ -845,10 +845,10 @@ subtest "CanBookBeRenewed tests" => sub {
}
);
my $auto_renew_issue = AddIssue( $renewing_borrower_obj, $item_4->barcode, undef, undef, undef, undef, { auto_renew => 1 } );
my $auto_renew_issue =
AddIssue( $renewing_borrower_obj, $item_4->barcode, undef, undef, undef, undef, { auto_renew => 1 } );
my $info;
( $renewokay, $error, $info ) =
CanBookBeRenewed( $renewing_borrower_obj, $auto_renew_issue );
( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrower_obj, $auto_renew_issue );
is( $info->{soonest_renew_date}, undef, "soonest_renew_date is not returned because this issue can be renewed" );
is( $renewokay, 1, 'Bug 25393: Can do a manual renew, even if renewal is automatic and premature' );
is(
@ -939,7 +939,10 @@ subtest "CanBookBeRenewed tests" => sub {
is( $error, 'too_soon',
'Bug 25393: Cannot renew, renewal is premature (returned code is too_soon)'
);
is( $info->{soonest_renew_date}, dt_from_string($auto_renew_issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'auto_too_soon'");
is(
$info->{soonest_renew_date}, dt_from_string( $auto_renew_issue->date_due )->subtract( days => 7 ),
"Soonest renew date returned when error is 'auto_too_soon'"
);
( $renewokay_cron, $error_cron, $info_cron ) =
CanBookBeRenewed( $renewing_borrower_obj, $auto_renew_issue, undef, 1 );