From 02326ebf245e6b2f8c542b39aa111bffa357b64d Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 3 Feb 2022 13:14:33 +0000 Subject: [PATCH] Bug 30016: Remove GetOpenIssue subroutine This patch adjusts the code that uses GetOpenIssue to use/find a Koha::Checkout object instead To test: 1 - Add a course to course reserves 2 - Create an item with barcode TESTKOC 3 - Add the item to a course 4 - Checkout the item 5 - View course details on stff and opac and confirm item shows as checked out and due date displays 6 - prove t/db_dependent/Circulation/issue.t t/db_dependent/Circulation.t t/db_dependent/CourseReserves.t 7 - Browse to Circulation->Upload offline circulation 8 - Upload a file to return the item: https://wiki.koha-community.org/wiki/Koha_offline_circulation_file_format 9 - Confirm item is returned Signed-off-by: David Nind Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi --- C4/Circulation.pm | 33 +++++------------------------- C4/CourseReserves.pm | 4 ++-- offline_circ/process_koc.pl | 11 +++++----- t/db_dependent/Circulation/issue.t | 10 ++------- 4 files changed, 14 insertions(+), 44 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 87f892ab4b..58ceeae3d1 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -95,7 +95,6 @@ BEGIN { GetBranchBorrowerCircRule GetBranchItemRule GetBiblioIssues - GetOpenIssue GetUpcomingDueIssues CheckIfIssuedToPatron IsItemIssued @@ -2790,28 +2789,6 @@ sub _GetCircControlBranch { return $branch; } -=head2 GetOpenIssue - - $issue = GetOpenIssue( $itemnumber ); - -Returns the row from the issues table if the item is currently issued, undef if the item is not currently issued - -C<$itemnumber> is the item's itemnumber - -Returns a hashref - -=cut - -sub GetOpenIssue { - my ( $itemnumber ) = @_; - return unless $itemnumber; - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare( "SELECT * FROM issues WHERE itemnumber = ? AND returndate IS NULL" ); - $sth->execute( $itemnumber ); - return $sth->fetchrow_hashref(); - -} - =head2 GetUpcomingDueIssues my $upcoming_dues = GetUpcomingDueIssues( { days_in_advance => 4 } ); @@ -4011,12 +3988,12 @@ sub ProcessOfflineReturn { if ( $item ) { my $itemnumber = $item->itemnumber; - my $issue = GetOpenIssue( $itemnumber ); + my $issue = $item->checkout; if ( $issue ) { my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0; ModDateLastSeen( $itemnumber, $leave_item_lost ); MarkIssueReturned( - $issue->{borrowernumber}, + $issue->borrowernumber, $itemnumber, $operation->{timestamp}, ); @@ -4043,11 +4020,11 @@ sub ProcessOfflineIssue { return "Barcode not found."; } my $itemnumber = $item->itemnumber; - my $issue = GetOpenIssue( $itemnumber ); + my $issue = $item->checkout; - if ( $issue and ( $issue->{borrowernumber} ne $patron->borrowernumber ) ) { # Item already issued to another patron mark it returned + if ( $issue and ( $issue->borrowernumber ne $patron->borrowernumber ) ) { # Item already issued to another patron mark it returned MarkIssueReturned( - $issue->{borrowernumber}, + $issue->borrowernumber, $itemnumber, $operation->{timestamp}, ); diff --git a/C4/CourseReserves.pm b/C4/CourseReserves.pm index 7f9a4aa7f6..fac8b849e2 100644 --- a/C4/CourseReserves.pm +++ b/C4/CourseReserves.pm @@ -20,12 +20,12 @@ use Modern::Perl; use List::MoreUtils qw( any ); use C4::Context; -use C4::Circulation qw( GetOpenIssue ); use Koha::Courses; use Koha::Course::Instructors; use Koha::Course::Items; use Koha::Course::Reserves; +use Koha::Checkouts; use vars qw(@FIELDS); our (@ISA, @EXPORT_OK); @@ -873,7 +873,7 @@ sub GetCourseReserves { $cr->{'item'} = $item; $cr->{'biblio'} = $biblio; $cr->{'biblioitem'} = $biblioitem; - $cr->{'issue'} = GetOpenIssue( $cr->{'itemnumber'} ); + $cr->{'issue'} = Koha::Checkouts->find({ itemnumber => $cr->{'itemnumber'} }); } } diff --git a/offline_circ/process_koc.pl b/offline_circ/process_koc.pl index b3dc48a54c..b99f5b29ee 100755 --- a/offline_circ/process_koc.pl +++ b/offline_circ/process_koc.pl @@ -26,7 +26,7 @@ use C4::Output qw( output_html_with_http_headers ); use C4::Auth qw( get_template_and_user ); use C4::Context; use C4::Accounts; -use C4::Circulation qw( barcodedecode GetOpenIssue AddRenewal AddIssue MarkIssueReturned ); +use C4::Circulation qw( barcodedecode AddRenewal AddIssue MarkIssueReturned ); use C4::Items qw( ModDateLastSeen ); use C4::Members; use C4::Stats; @@ -253,13 +253,12 @@ sub kocIssueItem { if ( $issue ) { ## Item is currently checked out to another person. #warn "Item Currently Issued."; - my $issue = GetOpenIssue( $item->itemnumber ); # FIXME Hum? That does not make sense, if it's in the issue table, the issue is open (i.e. returndate is null) - if ( $issue->{'borrowernumber'} eq $borrower->{'borrowernumber'} ) { ## Issued to this person already, renew it. + if ( $issue->borrowernumber eq $borrower->{'borrowernumber'} ) { ## Issued to this person already, renew it. #warn "Item issued to this member already, renewing."; C4::Circulation::AddRenewal( - $issue->{'borrowernumber'}, # borrowernumber + $issue->borrowernumber, # borrowernumber $item->itemnumber, # itemnumber undef, # branch undef, # datedue - let AddRenewal calculate it automatically @@ -280,9 +279,9 @@ sub kocIssueItem { } else { #warn "Item issued to a different member."; - #warn "Date of previous issue: $issue->{'issuedate'}"; + #warn "Date of previous issue: $issue->issuedate"; #warn "Date of this issue: $circ->{'date'}"; - my ( $i_y, $i_m, $i_d ) = split( /-/, $issue->{'issuedate'} ); + my ( $i_y, $i_m, $i_d ) = split( /-/, $issue->issuedate ); my ( $c_y, $c_m, $c_d ) = split( /-/, $circ->{'date'} ); if ( Date_to_Days( $i_y, $i_m, $i_d ) < Date_to_Days( $c_y, $c_m, $c_d ) ) { ## Current issue to a different persion is older than this issue, return and issue. diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t index 81d5e6742c..5fc658b57f 100755 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -17,14 +17,14 @@ use Modern::Perl; -use Test::More tests => 50; +use Test::More tests => 48; use DateTime::Duration; use t::lib::Mocks; use t::lib::TestBuilder; use C4::Biblio qw( AddBiblio ); -use C4::Circulation qw( AddIssue AddIssuingCharge AddRenewal AddReturn GetIssuingCharges GetOpenIssue GetRenewCount GetUpcomingDueIssues ); +use C4::Circulation qw( AddIssue AddIssuingCharge AddRenewal AddReturn GetIssuingCharges GetRenewCount GetUpcomingDueIssues ); use C4::Context; use C4::Items; use C4::Reserves qw( AddReserve ); @@ -49,7 +49,6 @@ can_ok( AddRenewal AddReturn GetIssuingCharges - GetOpenIssue GetRenewCount GetUpcomingDueIssues ) @@ -296,11 +295,6 @@ subtest 'Show that AddRenewal respects OpacRenewalBranch and interface' => sub { } }; -#Test GetOpenIssue -is( GetOpenIssue(), undef, "Without parameter GetOpenIssue returns undef" ); -is( GetOpenIssue(-1), undef, - "With wrong parameter GetOpenIssue returns undef" ); -my $openissue = GetOpenIssue($borrower_id1, $item_id1); my @renewcount; #Test GetRenewCount -- 2.39.5