Attachment 57783 Details for Bug 17679 – Bug 17679: C4::Circulation - Remove unused GetItemIssues

[patch] Bug 17679: C4::Circulation - Remove unused GetItemIssues

Bug-17679-C4Circulation---Remove-unused-GetItemIss.patch (text/plain), 5.61 KB, created by Jonathan Druart on 2016-11-28 08:33:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2016-11-28 08:33:39 UTC

Size: 5.61 KB

patch

obsolete

>From cea2091735bef7d39f739cc13369070c960aa070 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Fri, 25 Nov 2016 11:52:11 +0100
>Subject: [PATCH] Bug 17679: C4::Circulation - Remove unused GetItemIssues
>
>Ready for an archaeology course?
>
>C4::Circulation::GetItemIssues is only used once, from
>catalogue/issuehistory.pl
>This call has been added by
>  commit 95d6452462a560ba0c0ac859a2cfb7783c25c925
>    Adding some more information on issuehistory.
>which says "Adding itemnumber to issuehistory.pl API so that one could search
>for issuehistory of a specific item."
>So it added the ability to see the item issue history but did not
>provide a way to access it via the interface.
>It's ok so far but this subroutine is broken since
>  commit aa114f53499b9cffde0571fe7e08622f9c9a332a
>    Bug 5549 : Only use DateTime for issues table
>because of this change:
>-    my $today = C4::Dates->today('iso');
>+    my $today = DateTime->now( time_zome => C4::Context->tz);
>
>I let you catch the typo ;)
>And since this commit the subroutine explodes with "The following
>parameter was passed in the call to DateTime::from_epoch but was not
>listed in the validation options: time_zome"
>
>Since it has never been raised by someone and that the feature is
>hidden, I'd recommend to simply remove it.
>
>Note that the "Checked out from" column would have been wrong even if we
>fixed all the previous issue.
>
>Test plan:
>Just dig into the code and confirm what this commit message tells
>---
> C4/Circulation.pm                  | 48 --------------------------------------
> catalogue/issuehistory.pl          | 22 +++--------------
> t/db_dependent/Circulation/issue.t |  5 ----
> 3 files changed, 3 insertions(+), 72 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index b505031..8abc9c2 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -88,7 +88,6 @@ BEGIN {
>         &GetSoonestRenewDate
>         &GetLatestAutoRenewDate
> 		&GetItemIssue
>-		&GetItemIssues
> 		&GetIssuingCharges
>         &GetBranchBorrowerCircRule
>         &GetBranchItemRule
>@@ -2573,53 +2572,6 @@ sub GetIssues {
>     return $rv ? $sth->fetchall_arrayref({}) : undef;
> }
> 
>-=head2 GetItemIssues
>-
>-  $issues = &GetItemIssues($itemnumber, $history);
>-
>-Returns patrons that have issued a book
>-
>-C<$itemnumber> is the itemnumber
>-C<$history> is false if you just want the current "issuer" (if any)
>-and true if you want issues history from old_issues also.
>-
>-Returns reference to an array of hashes
>-
>-=cut
>-
>-sub GetItemIssues {
>-    my ( $itemnumber, $history ) = @_;
>-    
>-    my $today = DateTime->now( time_zome => C4::Context->tz);  # get today date
>-    $today->truncate( to => 'minute' );
>-    my $sql = "SELECT * FROM issues
>-              JOIN borrowers USING (borrowernumber)
>-              JOIN items     USING (itemnumber)
>-              WHERE issues.itemnumber = ? ";
>-    if ($history) {
>-        $sql .= "UNION ALL
>-                 SELECT * FROM old_issues
>-                 LEFT JOIN borrowers USING (borrowernumber)
>-                 JOIN items USING (itemnumber)
>-                 WHERE old_issues.itemnumber = ? ";
>-    }
>-    $sql .= "ORDER BY date_due DESC";
>-    my $sth = C4::Context->dbh->prepare($sql);
>-    if ($history) {
>-        $sth->execute($itemnumber, $itemnumber);
>-    } else {
>-        $sth->execute($itemnumber);
>-    }
>-    my $results = $sth->fetchall_arrayref({});
>-    foreach (@$results) {
>-        my $date_due = dt_from_string($_->{date_due},'sql');
>-        $date_due->truncate( to => 'minute' );
>-
>-        $_->{overdue} = (DateTime->compare($date_due, $today) == -1) ? 1 : 0;
>-    }
>-    return $results;
>-}
>-
> =head2 GetBiblioIssues
> 
>   $issues = GetBiblioIssues($biblionumber);
>diff --git a/catalogue/issuehistory.pl b/catalogue/issuehistory.pl
>index b106143..a3e349d 100755
>--- a/catalogue/issuehistory.pl
>+++ b/catalogue/issuehistory.pl
>@@ -42,30 +42,14 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
> my $params = $query->Vars;
> 
> my $biblionumber = $params->{'biblionumber'};
>-my $itemnumber   = $params->{'itemnumber'};
> 
> if (C4::Context->preference("HidePatronName")) {
>    $template->param(HidePatronName => 1);
> }
> 
>-my ($issues,$biblio,$barcode);
>-if ($itemnumber){
>-	$issues=GetItemIssues($itemnumber);
>-	$biblio=GetBiblioFromItemNumber($itemnumber);
>-	$biblionumber=$biblio->{biblionumber};
>-	$barcode=$issues->[0]->{barcode};
>-	$template->param(
>-		%$biblio,
>-		barcode=> $barcode,
>-	);
>-} else {
>-	$issues = GetBiblioIssues($biblionumber);
>-        my $biblio = GetBiblio($biblionumber);
>-	my $total  = scalar @$issues;
>-	$template->param(
>-               %{$biblio},
>-	);
>-} 
>+my $issues = GetBiblioIssues($biblionumber);
>+my $biblio = GetBiblio($biblionumber);
>+$template->param(%$biblio);
> 
> $template->param(
>     total        => scalar @$issues,
>diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t
>index 1de08bb..c1a157d 100644
>--- a/t/db_dependent/Circulation/issue.t
>+++ b/t/db_dependent/Circulation/issue.t
>@@ -46,7 +46,6 @@ can_ok(
>       GetBiblioIssues
>       GetIssuingCharges
>       GetItemIssue
>-      GetItemIssues
>       GetOpenIssue
>       GetRenewCount
>       GetUpcomingDueIssues
>@@ -224,10 +223,6 @@ is( GetBiblioIssues(), undef, "GetBiblio Issues without parameters" );
> is(GetItemIssue,undef,"Without parameter GetItemIssue returns undef");
> #is(GetItemIssue($item_id1),{},"Item1's issues");
> 
>-#Test GetItemIssues
>-#FIXME: this routine currently doesn't work be
>-#is_deeply (GetItemIssues,{},"Without parameter, GetItemIssue returns all the issues");
>-
> #Test GetOpenIssue
> is( GetOpenIssue(), undef, "Without parameter GetOpenIssue returns undef" );
> is( GetOpenIssue(-1), undef,
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 17679: 57770 | 57783 | 57784 | 58413