Skip to content

Commit

Permalink
Bug 30016: Remove GetOpenIssue subroutine
Browse files Browse the repository at this point in the history
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 <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
  • Loading branch information
kidclamp authored and tomascohen committed Aug 31, 2022
1 parent 839eb02 commit 02326eb
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 44 deletions.
33 changes: 5 additions & 28 deletions C4/Circulation.pm
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ BEGIN {
GetBranchBorrowerCircRule
GetBranchItemRule
GetBiblioIssues
GetOpenIssue
GetUpcomingDueIssues
CheckIfIssuedToPatron
IsItemIssued
Expand Down Expand Up @@ -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 } );
Expand Down Expand Up @@ -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},
);
Expand All @@ -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},
);
Expand Down
4 changes: 2 additions & 2 deletions C4/CourseReserves.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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'} });
}
}

Expand Down
11 changes: 5 additions & 6 deletions offline_circ/process_koc.pl
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
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;
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
10 changes: 2 additions & 8 deletions t/db_dependent/Circulation/issue.t
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -49,7 +49,6 @@ can_ok(
AddRenewal
AddReturn
GetIssuingCharges
GetOpenIssue
GetRenewCount
GetUpcomingDueIssues
)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 02326eb

Please sign in to comment.