Browse Source

Bug 24361: (bug 24217 follow-up) Fix several warnings in C4 modules

With bug 24217 pushed, lot of warnings appears during the tests are run.
Most of them are "Use of uninitialized value in "

Test plan:
Take a look at the output of run 1049
https://jenkins.koha-community.org/job/Koha_Master_D9/1049/consoleFull

Most of the warnings from this run will be removed by this patch

At least 2 are not fixed:
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/C4/Reserves.pm line 791.
t/db_dependent/Items_DelItemCheck.t => see 23463

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
20.05.x
Jonathan Druart 2 years ago
committed by Martin Renvoize
parent
commit
094f0623c2
Signed by: martin.renvoize GPG Key ID: 422B469130441A0F
  1. 36
      C4/Budgets.pm
  2. 21
      C4/Circulation.pm
  3. 16
      C4/Items.pm
  4. 2
      C4/Koha.pm
  5. 6
      C4/Overdues.pm
  6. 2
      C4/Record.pm
  7. 16
      C4/Reserves.pm
  8. 2
      C4/Serials.pm
  9. 2
      Koha/Plugins/Base.pm

36
C4/Budgets.pm

@ -342,7 +342,7 @@ sub GetBudgetSpent {
datecancellationprinted IS NULL
|);
$sth->execute($budget_id);
my $sum = 0 + $sth->fetchrow_array;
my $sum = ( $sth->fetchrow_array || 0 ) + 0;
$sth = $dbh->prepare(qq|
SELECT SUM(shipmentcost) AS sum
@ -352,7 +352,7 @@ sub GetBudgetSpent {
$sth->execute($budget_id);
my ($shipmentcost_sum) = $sth->fetchrow_array;
$sum += $shipmentcost_sum;
$sum += ( $shipmentcost_sum || 0 ) + 0;
my $adjustments = Koha::Acquisition::Invoice::Adjustments->search({budget_id => $budget_id, closedate => { '!=' => undef } },{ join => 'invoiceid' });
while ( my $adj = $adjustments->next ){
@ -373,7 +373,7 @@ sub GetBudgetOrdered {
datecancellationprinted IS NULL
|);
$sth->execute($budget_id);
my $sum = 0 + $sth->fetchrow_array;
my $sum = ( $sth->fetchrow_array || 0 ) + 0;
my $adjustments = Koha::Acquisition::Invoice::Adjustments->search({budget_id => $budget_id, encumber_open => 1, closedate => undef},{ join => 'invoiceid' });
while ( my $adj = $adjustments->next ){
@ -606,12 +606,12 @@ sub _recursiveAdd {
_recursiveAdd($child, $budget, $hr_budget_spent, $hr_budget_spent_shipment, $hr_budget_ordered, $hr_budget_ordered_shipment, $hr_budget_spent_adjustment, $hr_budget_ordered_adjustment );
}
$budget->{budget_spent} += $hr_budget_spent->{$budget->{budget_id}}->{budget_spent};
$budget->{budget_spent} += $hr_budget_spent_shipment->{$budget->{budget_id}}->{shipmentcost};
$budget->{budget_spent} += $hr_budget_spent_adjustment->{$budget->{budget_id}}->{adjustments};
$budget->{budget_ordered} += $hr_budget_ordered->{$budget->{budget_id}}->{budget_ordered};
$budget->{budget_ordered} += $hr_budget_ordered_shipment->{$budget->{budget_id}}->{shipmentcost};
$budget->{budget_ordered} += $hr_budget_ordered_adjustment->{$budget->{budget_id}}->{adjustments};
$budget->{budget_spent} += $hr_budget_spent->{$budget->{budget_id}}->{budget_spent} || 0;
$budget->{budget_spent} += $hr_budget_spent_shipment->{$budget->{budget_id}}->{shipmentcost} || 0;
$budget->{budget_spent} += $hr_budget_spent_adjustment->{$budget->{budget_id}}->{adjustments} || 0;
$budget->{budget_ordered} += $hr_budget_ordered->{$budget->{budget_id}}->{budget_ordered} || 0;
$budget->{budget_ordered} += $hr_budget_ordered_shipment->{$budget->{budget_id}}->{shipmentcost} || 0;
$budget->{budget_ordered} += $hr_budget_ordered_adjustment->{$budget->{budget_id}}->{adjustments} || 0;
$budget->{total_spent} += $budget->{budget_spent};
$budget->{total_ordered} += $budget->{budget_ordered};
@ -636,30 +636,32 @@ sub _add_budget_children {
}
# -------------------------------------------------------------------
# FIXME Must be replaced by Koha::Acquisition::Fund->store
sub AddBudget {
my ($budget) = @_;
return unless ($budget);
undef $budget->{budget_encumb} if $budget->{budget_encumb} eq '';
undef $budget->{budget_owner_id} if $budget->{budget_owner_id} eq '';
undef $budget->{budget_encumb} if defined $budget->{budget_encumb} && $budget->{budget_encumb} eq '';
undef $budget->{budget_owner_id} if defined $budget->{budget_owner_id} && $budget->{budget_owner_id} eq '';
my $resultset = Koha::Database->new()->schema->resultset('Aqbudget');
return $resultset->create($budget)->id;
}
# -------------------------------------------------------------------
# FIXME Must be replaced by Koha::Acquisition::Fund->store
sub ModBudget {
my ($budget) = @_;
my $result = Koha::Database->new()->schema->resultset('Aqbudget')->find($budget);
return unless($result);
undef $budget->{budget_encumb} if $budget->{budget_encumb} eq '';
undef $budget->{budget_owner_id} if $budget->{budget_owner_id} eq '';
undef $budget->{budget_encumb} if defined $budget->{budget_encumb} && $budget->{budget_encumb} eq '';
undef $budget->{budget_owner_id} if defined $budget->{budget_owner_id} && $budget->{budget_owner_id} eq '';
$result = $result->update($budget);
return $result->in_storage;
}
# -------------------------------------------------------------------
# FIXME Must be replaced by Koha::Acquisition::Fund->delete
sub DelBudget {
my ($budget_id) = @_;
my $dbh = C4::Context->dbh;
@ -809,14 +811,14 @@ sub GetBudgetsReport {
ON bp.budget_period_id = b.budget_period_id
INNER JOIN aqorders o
ON b.budget_id = o.budget_id ';
if($activity ne ''){
if ( $activity && $activity ne '' ) {
$query .= 'WHERE bp.budget_period_active=? ';
}
$query .= 'AND (o.orderstatus != "cancelled")
ORDER BY b.budget_name';
my $sth = $dbh->prepare($query);
if($activity ne ''){
if ( $activity && $activity ne '' ) {
$sth->execute($activity);
}
else{
@ -1221,7 +1223,7 @@ sub CloneBudgetHierarchy {
my @first_level_budgets =
( not defined $children_of )
? map { ( not $_->{budget_parent_id} ) ? $_ : () } @$budgets
: map { ( $_->{budget_parent_id} == $children_of ) ? $_ : () } @$budgets;
: map { ( defined $_->{budget_parent_id} && $_->{budget_parent_id} == $children_of ) ? $_ : () } @$budgets;
# get only the columns of aqbudgets
my @columns = Koha::Database->new()->schema->source('Aqbudget')->columns;

21
C4/Circulation.pm

@ -748,11 +748,11 @@ sub CanBookBeIssued {
return( { STATS => 1 }, {});
}
if ( $patron->gonenoaddress == 1 ) {
if ( $patron->gonenoaddress && $patron->gonenoaddress == 1 ) {
$issuingimpossible{GNA} = 1;
}
if ( $patron->lost == 1 ) {
if ( $patron->lost && $patron->lost == 1 ) {
$issuingimpossible{CARD_LOST} = 1;
}
if ( $patron->is_debarred ) {
@ -1430,7 +1430,8 @@ sub AddIssue {
}
)->store;
}
if ( $item_object->location eq 'CART' && $item_object->permanent_location ne 'CART' ) {
if ( $item_object->location && $item_object->location eq 'CART'
&& ( !$item_object->permanent_location || $item_object->permanent_location ne 'CART' ) ) {
## Item was moved to cart via UpdateItemLocationOnCheckin, anything issued should be taken off the cart.
CartToShelf( $item_object->itemnumber );
}
@ -1458,7 +1459,7 @@ sub AddIssue {
ModItem(
{
issues => $item_object->issues + 1,
issues => ( $item_object->issues || 0 ) + 1,
holdingbranch => C4::Context->userenv->{'branch'},
itemlost => 0,
onloan => $datedue->ymd(),
@ -1472,7 +1473,7 @@ sub AddIssue {
# If it costs to borrow this book, charge it to the patron's account.
my ( $charge, $itemtype ) = GetIssuingCharges( $item_object->itemnumber, $borrower->{'borrowernumber'} );
if ( $charge > 0 ) {
if ( $charge && $charge > 0 ) {
AddIssuingCharge( $issue, $charge, 'RENT' );
}
@ -2168,7 +2169,7 @@ sub MarkIssueReturned {
my $issue_id = $issue->issue_id;
my $anonymouspatron;
if ( $privacy == 2 ) {
if ( $privacy && $privacy == 2 ) {
# The default of 0 will not work due to foreign key constraints
# The anonymisation will fail if AnonymousPatron is not a valid entry
# We need to check if the anonymous patron exist, Koha will fail loudly if it does not
@ -2195,7 +2196,7 @@ sub MarkIssueReturned {
my $old_checkout = Koha::Old::Checkout->new($issue->unblessed)->store;
# anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
if ( $privacy == 2) {
if ( $privacy && $privacy == 2) {
$old_checkout->borrowernumber($anonymouspatron)->store;
}
@ -2327,7 +2328,7 @@ sub _debar_user_on_return {
# if borrower was already debarred but does not get an extra debarment
my $patron = Koha::Patrons->find( $borrower->{borrowernumber} );
my ($new_debarment_str, $is_a_reminder);
if ( $borrower->{debarred} eq $patron->is_debarred ) {
if ( $borrower->{debarred} && $borrower->{debarred} eq $patron->is_debarred ) {
$is_a_reminder = 1;
$new_debarment_str = $borrower->{debarred};
} else {
@ -2927,7 +2928,7 @@ sub AddRenewal {
# Update the issues record to have the new due date, and a new count
# of how many times it has been renewed.
my $renews = $issue->renewals + 1;
my $renews = ( $issue->renewals || 0 ) + 1;
my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
WHERE borrowernumber=?
AND itemnumber=?"
@ -2936,7 +2937,7 @@ sub AddRenewal {
$sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber );
# Update the renewal count on the item, and tell zebra to reindex
$renews = $item_object->renewals + 1;
$renews = ( $item_object->renewals || 0 ) + 1;
ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item_object->biblionumber, $itemnumber, { log_action => 0 } );
# Charge a new rental fee, if applicable

16
C4/Items.pm

@ -552,7 +552,7 @@ sub ModItemTransfer {
my $item = Koha::Items->find( $itemnumber );
# Remove the 'shelving cart' location status if it is being used.
CartToShelf( $itemnumber ) if ( $item->location eq 'CART' && $item->permanent_location ne 'CART' );
CartToShelf( $itemnumber ) if $item->location && $item->location eq 'CART' && ( !$item->permanent_location || $item->permanent_location ne 'CART' );
$dbh->do("UPDATE branchtransfers SET datearrived = NOW(), comments = ? WHERE itemnumber = ? AND datearrived IS NULL", undef, "Canceled, new transfer from $frombranch to $tobranch created", $itemnumber);
@ -1445,11 +1445,13 @@ sub _do_column_fixes_for_mod {
(not defined $item->{'withdrawn'} or $item->{'withdrawn'} eq '')) {
$item->{'withdrawn'} = 0;
}
if (exists $item->{location}
and $item->{location} ne 'CART'
and $item->{location} ne 'PROC'
if (
exists $item->{location}
and ( !defined $item->{location}
|| ( $item->{location} ne 'CART' and $item->{location} ne 'PROC' ) )
and not $item->{permanent_location}
) {
)
{
$item->{'permanent_location'} = $item->{'location'};
}
if (exists $item->{'timestamp'}) {
@ -2515,7 +2517,7 @@ sub PrepareItemrecordDisplay {
$subfield_data{marc_value} = {
type => 'select',
values => \@authorised_values,
default => "$defaultvalue",
default => $defaultvalue // q{},
labels => \%authorised_lib,
};
} elsif ( $tagslib->{$tag}->{$subfield}->{value_builder} ) {
@ -2528,7 +2530,7 @@ sub PrepareItemrecordDisplay {
my $pars = { dbh => $dbh, record => undef, tagslib =>$tagslib, id => $subfield_data{id}, tabloop => undef };
$plugin->build( $pars );
if ( $itemrecord and my $field = $itemrecord->field($tag) ) {
$defaultvalue = $field->subfield($subfield);
$defaultvalue = $field->subfield($subfield) || q{};
}
if( !$plugin->errstr ) {
#TODO Move html to template; see report 12176/13397

2
C4/Koha.pm

@ -846,7 +846,7 @@ sub NormalizeISBN {
my $string = $params->{isbn};
my $strip_hyphens = $params->{strip_hyphens};
my $format = $params->{format};
my $format = $params->{format} || q{};
my $return_invalid = $params->{return_invalid};
return unless $string;

6
C4/Overdues.pm

@ -247,7 +247,7 @@ sub CalcFine {
my $fine_unit = $issuing_rule->lengthunit || 'days';
my $chargeable_units = get_chargeable_units($fine_unit, $start_date, $end_date, $branchcode);
my $units_minus_grace = $chargeable_units - $issuing_rule->firstremind;
my $units_minus_grace = $chargeable_units - ($issuing_rule->firstremind || 0);
my $amount = 0;
if ( $issuing_rule->chargeperiod && ( $units_minus_grace > 0 ) ) {
my $units = C4::Context->preference('FinesIncludeGracePeriod') ? $chargeable_units : $units_minus_grace;
@ -261,9 +261,9 @@ sub CalcFine {
$amount = $issuing_rule->overduefinescap if $issuing_rule->overduefinescap && $amount > $issuing_rule->overduefinescap;
# This must be moved to Koha::Item (see also similar code in C4::Accounts::chargelostitem
$item->{replacementprice} ||= $itemtype->defaultreplacecost
$item->{replacementprice} //= $itemtype->defaultreplacecost
if $itemtype
&& $item->{replacementprice} == 0
&& ( ! defined $item->{replacementprice} || $item->{replacementprice} == 0 )
&& C4::Context->preference("useDefaultReplacementCost");
$amount = $item->{replacementprice} if ( $issuing_rule->cap_fine_to_replacement_price && $item->{replacementprice} && $amount > $item->{replacementprice} );

2
C4/Record.pm

@ -80,7 +80,7 @@ Returns an ISO-2709 scalar
sub marc2marc {
my ($marc,$to_flavour,$from_flavour,$encoding) = @_;
my $error;
if ($to_flavour =~ m/marcstd/) {
if ($to_flavour && $to_flavour =~ m/marcstd/) {
my $marc_record_obj;
if ($marc =~ /^MARC::Record/) { # it's already a MARC::Record object
$marc_record_obj = $marc;

16
C4/Reserves.pm

@ -204,7 +204,7 @@ sub AddReserve {
my $waitingdate;
# If the reserv had the waiting status, we had the value of the resdate
if ( $found eq 'W' ) {
if ( $found && $found eq 'W' ) {
$waitingdate = $resdate;
}
@ -228,7 +228,7 @@ sub AddReserve {
item_level_hold => $checkitem ? 1 : 0,
}
)->store();
$hold->set_waiting() if $found eq 'W';
$hold->set_waiting() if $found && $found eq 'W';
logaction( 'HOLDS', 'CREATE', $hold->id, Dumper($hold->unblessed) )
if C4::Context->preference('HoldsLog');
@ -1067,7 +1067,9 @@ sub ModReserveStatus {
$sth_set->execute( $newstatus, $itemnumber );
my $item = Koha::Items->find($itemnumber);
if ( ( $item->location eq 'CART' && $item->permanent_location ne 'CART' ) && $newstatus ) {
if ( $item->location && $item->location eq 'CART'
&& ( !$item->permanent_location || $item->permanent_location ne 'CART' )
&& $newstatus ) {
CartToShelf( $itemnumber );
}
}
@ -1120,7 +1122,8 @@ sub ModReserveAffect {
_FixPriority( { biblionumber => $biblionumber } );
my $item = Koha::Items->find($itemnumber);
if ( ( $item->location eq 'CART' && $item->permanent_location ne 'CART' ) ) {
if ( $item->location && $item->location eq 'CART'
&& ( !$item->permanent_location || $item->permanent_location ne 'CART' ) ) {
CartToShelf( $itemnumber );
}
@ -1472,7 +1475,7 @@ sub _FixPriority {
if ( $rank eq "del" ) { # FIXME will crash if called without $hold
$hold->cancel;
}
elsif ( $rank eq "W" || $rank eq "0" ) {
elsif ( $reserve_id && ( $rank eq "W" || $rank eq "0" ) ) {
# make sure priority for waiting or in-transit items is 0
my $query = "
@ -1500,11 +1503,12 @@ sub _FixPriority {
push( @priority, $line );
}
# FIXME This whole sub must be rewritten, especially to highlight what is done when reserve_id is not given
# To find the matching index
my $i;
my $key = -1; # to allow for 0 to be a valid result
for ( $i = 0 ; $i < @priority ; $i++ ) {
if ( $reserve_id == $priority[$i]->{'reserve_id'} ) {
if ( $reserve_id && $reserve_id == $priority[$i]->{'reserve_id'} ) {
$key = $i; # save the index
last;
}

2
C4/Serials.pm

@ -2144,7 +2144,7 @@ sub abouttoexpire {
}
}
} elsif ($subscription->{numberlength}>0) {
} elsif ( $subscription->{numberlength} && $subscription->{numberlength}>0) {
return (countissuesfrom($subscriptionid,$subscription->{'startdate'}) >=$subscription->{numberlength}-1);
}

2
Koha/Plugins/Base.pm

@ -179,7 +179,7 @@ sub get_metadata {
#FIXME: Why another encoding issue? For metadata containing non latin characters.
my $metadata = $self->{metadata};
utf8::decode($metadata->{$_}) for keys %$metadata;
$metadata->{$_} && utf8::decode($metadata->{$_}) for keys %$metadata;
return $metadata;
}

Loading…
Cancel
Save