Bugzilla – Attachment 96892 Details for
Bug 24361
Fix warnings (or failing tests) from bug 24217
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24361: (bug 24217 follow-up) Fix several warnings in C4 modules
Bug-24361-bug-24217-follow-up-Fix-several-warnings.patch (text/plain), 19.58 KB, created by
Jonathan Druart
on 2020-01-07 13:20:33 UTC
(
hide
)
Description:
Bug 24361: (bug 24217 follow-up) Fix several warnings in C4 modules
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-01-07 13:20:33 UTC
Size:
19.58 KB
patch
obsolete
>From a7296b6325b18f3e154e1b029b9c87fe541d468f Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 7 Jan 2020 12:26:01 +0100 >Subject: [PATCH] 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 >--- > C4/Budgets.pm | 36 +++++++++++++++++++----------------- > C4/Circulation.pm | 21 +++++++++++---------- > C4/Items.pm | 16 +++++++++------- > C4/Koha.pm | 2 +- > C4/Overdues.pm | 6 +++--- > C4/Record.pm | 6 +++--- > C4/Reserves.pm | 16 ++++++++++------ > C4/Serials.pm | 2 +- > Koha/Plugins/Base.pm | 2 +- > 9 files changed, 58 insertions(+), 49 deletions(-) > >diff --git a/C4/Budgets.pm b/C4/Budgets.pm >index 387985ca76..16eb5a22a0 100644 >--- a/C4/Budgets.pm >+++ b/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; >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index a62d60e4e6..d2506275e3 100644 >--- a/C4/Circulation.pm >+++ b/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 >diff --git a/C4/Items.pm b/C4/Items.pm >index 862e5e2d90..bd335d5563 100644 >--- a/C4/Items.pm >+++ b/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 >diff --git a/C4/Koha.pm b/C4/Koha.pm >index 518c75776a..d1c5771440 100644 >--- a/C4/Koha.pm >+++ b/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; >diff --git a/C4/Overdues.pm b/C4/Overdues.pm >index 2bf280cd06..26ce74850d 100644 >--- a/C4/Overdues.pm >+++ b/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} ); >diff --git a/C4/Record.pm b/C4/Record.pm >index f54e368449..c15c2076f7 100644 >--- a/C4/Record.pm >+++ b/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; >@@ -427,12 +427,12 @@ sub marc2csv { > for my $itemnumber ( @$itemnumbers) { > my $item = Koha::Items->find( $itemnumber ); > my $biblionumber = $item->biblio->biblionumber; >- $output .= marcrecord2csv( $biblionumber, $id, $firstpass, $csv, $fieldprocessing, [$itemnumber] ); >+ $output .= marcrecord2csv( $biblionumber, $id, $firstpass, $csv, $fieldprocessing, [$itemnumber] ) // ''; > $firstpass = 0; > } > } else { > foreach my $biblio (@$biblios) { >- $output .= marcrecord2csv( $biblio, $id, $firstpass, $csv, $fieldprocessing ); >+ $output .= marcrecord2csv( $biblio, $id, $firstpass, $csv, $fieldprocessing ) // ''; > $firstpass = 0; > } > } >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index b953cf8961..503771e914 100644 >--- a/C4/Reserves.pm >+++ b/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; > } >diff --git a/C4/Serials.pm b/C4/Serials.pm >index e7e9e3aa13..2b9e1a62a5 100644 >--- a/C4/Serials.pm >+++ b/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); > } > >diff --git a/Koha/Plugins/Base.pm b/Koha/Plugins/Base.pm >index 9751b194a2..6a723c0c15 100644 >--- a/Koha/Plugins/Base.pm >+++ b/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; > } > >-- >2.11.0
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 24361
:
96873
| 96892 |
96969
|
97315
|
97991
|
98796