From 290d6a8a16b1a7629468f57bef6cb8365b08df7b Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Mon, 26 Jan 2009 18:24:10 -0600 Subject: [PATCH] Branchoverdues circ report reworking. branchoverdues.pl ~ Removed unused variables. ~ Use elsif where applicable. ~ Added many FIXMEs. ~ Added help description. ~ Changed link to more accurate description. ~ REFACTORED branchoverdues-specific function in C4 for obvious consolidation. This report is still of questionable value, since it's dataset has such strange hardcoded limitations. It is not clear that "FU" type fines and notifys=0 are reliable or useful indicators to query on, in hardcoded form. Signed-off-by: Galen Charlton --- C4/Overdues.pm | 166 +++++++----------- circ/branchoverdues.pl | 30 ++-- .../en/modules/circ/circulation-home.tmpl | 2 +- .../en/modules/help/circ/branchoverdues.tmpl | 7 + 4 files changed, 84 insertions(+), 121 deletions(-) diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 06cf37ae65..70ed23e8e4 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -1178,17 +1178,17 @@ this function is not exported, only used with GetOverduesForBranch =cut sub CheckItemNotify { - my ($notify_id,$notify_level,$itemnumber) = @_; - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare(" - SELECT COUNT(*) FROM notifys - WHERE notify_id = ? - AND notify_level = ? - AND itemnumber = ? "); - $sth->execute($notify_id,$notify_level,$itemnumber); - my $notified = $sth->fetchrow; -$sth->finish; -return ($notified); + my ($notify_id,$notify_level,$itemnumber) = @_; + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare(" + SELECT COUNT(*) + FROM notifys + WHERE notify_id = ? + AND notify_level = ? + AND itemnumber = ? "); + $sth->execute($notify_id,$notify_level,$itemnumber); + my $notified = $sth->fetchrow; + return ($notified); } =head2 GetOverduesForBranch @@ -1197,112 +1197,68 @@ Sql request for display all information for branchoverdues.pl 2 possibilities : with or without location . display is filtered by branch +FIXME: This function should be renamed. + =cut sub GetOverduesForBranch { my ( $branch, $location) = @_; my $itype_link = (C4::Context->preference('item-level_itypes')) ? " items.itype " : " biblioitems.itemtype "; - if ( not $location ) { - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare(" - SELECT - borrowers.surname, - borrowers.firstname, - biblio.title, - itemtypes.description, - issues.date_due, - issues.returndate, - branches.branchname, + my $dbh = C4::Context->dbh; + my $select = " + SELECT + borrowers.borrowernumber, + borrowers.surname, + borrowers.firstname, + borrowers.phone, + borrowers.email, + biblio.title, + biblio.biblionumber, + issues.date_due, + issues.returndate, + issues.branchcode, + branches.branchname, items.barcode, - borrowers.phone, - borrowers.email, items.itemcallnumber, - borrowers.borrowernumber, - items.itemnumber, - biblio.biblionumber, - issues.branchcode, - accountlines.notify_id, - accountlines.notify_level, items.location, - accountlines.amountoutstanding - FROM accountlines - LEFT JOIN issues ON issues.itemnumber = accountlines.itemnumber AND issues.borrowernumber = accountlines.borrowernumber - LEFT JOIN borrowers ON borrowers.borrowernumber = accountlines.borrowernumber - LEFT JOIN items ON items.itemnumber = issues.itemnumber - LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber - LEFT JOIN biblioitems ON biblioitems.biblioitemnumber=items.biblioitemnumber - LEFT JOIN itemtypes ON itemtypes.itemtype = $itype_link - LEFT JOIN branches ON branches.branchcode = issues.branchcode - WHERE ( accountlines.amountoutstanding != '0.000000') - AND ( accountlines.accounttype = 'FU') - AND (issues.branchcode = ?) - AND (issues.date_due <= NOW()) - ORDER BY borrowers.surname - "); - $sth->execute($branch); - my @getoverdues; - my $i = 0; - while ( my $data = $sth->fetchrow_hashref ) { - #check if the document has already been notified - my $countnotify = CheckItemNotify($data->{'notify_id'},$data->{'notify_level'},$data->{'itemnumber'}); - if ($countnotify eq '0'){ + items.itemnumber, + itemtypes.description, + accountlines.notify_id, + accountlines.notify_level, + accountlines.amountoutstanding + FROM accountlines + LEFT JOIN issues ON issues.itemnumber = accountlines.itemnumber + AND issues.borrowernumber = accountlines.borrowernumber + LEFT JOIN borrowers ON borrowers.borrowernumber = accountlines.borrowernumber + LEFT JOIN items ON items.itemnumber = issues.itemnumber + LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber + LEFT JOIN biblioitems ON biblioitems.biblioitemnumber = items.biblioitemnumber + LEFT JOIN itemtypes ON itemtypes.itemtype = $itype_link + LEFT JOIN branches ON branches.branchcode = issues.branchcode + WHERE (accountlines.amountoutstanding != '0.000000') + AND (accountlines.accounttype = 'FU' ) + AND (issues.branchcode = ? ) + AND (issues.date_due <= NOW()) + "; + my @getoverdues; + my $i = 0; + my $sth; + if ($location) { + $sth = $dbh->prepare("$select AND items.location = ? ORDER BY borrowers.surname, borrowers.firstname"); + $sth->execute($branch, $location); + } else { + $sth = $dbh->prepare("$select ORDER BY borrowers.surname, borrowers.firstname"); + $sth->execute($branch); + } + while ( my $data = $sth->fetchrow_hashref ) { + #check if the document has already been notified + my $countnotify = CheckItemNotify($data->{'notify_id'}, $data->{'notify_level'}, $data->{'itemnumber'}); + if ($countnotify eq '0') { $getoverdues[$i] = $data; $i++; - } } - return (@getoverdues); - $sth->finish; - } - else { - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare( " - SELECT borrowers.surname, - borrowers.firstname, - biblio.title, - itemtypes.description, - issues.date_due, - issues.returndate, - branches.branchname, - items.barcode, - borrowers.phone, - borrowers.email, - items.itemcallnumber, - borrowers.borrowernumber, - items.itemnumber, - biblio.biblionumber, - issues.branchcode, - accountlines.notify_id, - accountlines.notify_level, - items.location, - accountlines.amountoutstanding - FROM accountlines - LEFT JOIN issues ON issues.itemnumber = accountlines.itemnumber AND issues.borrowernumber = accountlines.borrowernumber - LEFT JOIN borrowers ON borrowers.borrowernumber = accountlines.borrowernumber - LEFT JOIN items ON items.itemnumber = issues.itemnumber - LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber - LEFT JOIN biblioitems ON biblioitems.biblioitemnumber=items.biblioitemnumber - LEFT JOIN itemtypes ON itemtypes.itemtype = $itype_link - LEFT JOIN branches ON branches.branchcode = issues.branchcode - WHERE ( accountlines.amountoutstanding != '0.000000') - AND ( accountlines.accounttype = 'FU') - AND (issues.branchcode = ? AND items.location = ?) - AND (issues.date_due <= NOW()) - ORDER BY borrowers.surname - " ); - $sth->execute( $branch, $location); - my @getoverdues; - my $i = 0; - while ( my $data = $sth->fetchrow_hashref ) { - #check if the document has already been notified - my $countnotify = CheckItemNotify($data->{'notify_id'},$data->{'notify_level'},$data->{'itemnumber'}); - if ($countnotify eq '0'){ - $getoverdues[$i] = $data; - $i++; - } - } - $sth->finish; - return (@getoverdues); } + return (@getoverdues); } diff --git a/circ/branchoverdues.pl b/circ/branchoverdues.pl index a09f5d3f76..a33d836226 100755 --- a/circ/branchoverdues.pl +++ b/circ/branchoverdues.pl @@ -17,6 +17,7 @@ # Suite 330, Boston, MA 02111-1307 USA use strict; +# use warnings; # FIXME use C4::Context; use CGI; use C4::Output; @@ -50,23 +51,23 @@ use C4::Debug; - 2) item issues is not returned - 3) this item as not been already notify + FIXME: who is the author? + FIXME: No privisions (i.e. "actions") for handling notices are implemented. + FIXME: This is linked as "Overdue Fines" but the relationship to fines in GetOverduesForBranch is more complicated than that. + =cut my $input = new CGI; my $dbh = C4::Context->dbh; -my $theme = $input->param('theme'); # only used if allowthemeoverride is set - -my ( $template, $loggedinuser, $cookie ) = get_template_and_user( - { +my ( $template, $loggedinuser, $cookie ) = get_template_and_user({ template_name => "circ/branchoverdues.tmpl", query => $input, type => "intranet", authnotrequired => 0, flagsrequired => { circulate => "circulate_remaining_permissions" }, debug => 1, - } -); +}); my $default = C4::Context->userenv->{'branch'}; @@ -78,22 +79,22 @@ my $overduelevel = $input->param('overduelevel'); my $notifyId = $input->param('notifyId'); my $location = $input->param('location'); +# FIXME: better check that borrowernumber is defined and valid. +# FIXME: same for itemnumber and other variables passed in here. + # now create the line in bdd (notifys) if ( $input->param('action') eq 'add' ) { my $addnotify = AddNotifyLine( $borrowernumber, $itemnumber, $overduelevel, $method, - $notifyId ); + $notifyId ) # FIXME: useless variable, no TMPL code for "action" exists.; } elsif ( $input->param('action') eq 'remove' ) { my $notify_date = $input->param('notify_date'); my $removenotify = - RemoveNotifyLine( $borrowernumber, $itemnumber, $notify_date ); + RemoveNotifyLine( $borrowernumber, $itemnumber, $notify_date ); # FIXME: useless variable, no TMPL code for "action" exists. } my @overduesloop; -my @todayoverduesloop; -my $counter = 0; - my @getoverdues = GetOverduesForBranch( $default, $location ); use Data::Dumper; $debug and warn "HERE : $default / $location" . Dumper(@getoverdues); @@ -122,17 +123,16 @@ foreach my $num (@getoverdues) { $overdueforbranch{'itemnumber'} = $num->{'itemnumber'}; # now we add on the template, the differents values of notify_level + # FIXME: numerical comparison, not string eq. if ( $num->{'notify_level'} eq '1' ) { $overdueforbranch{'overdue1'} = 1; $overdueforbranch{'overdueLevel'} = 1; } - - if ( $num->{'notify_level'} eq '2' ) { + elsif ( $num->{'notify_level'} eq '2' ) { $overdueforbranch{'overdue2'} = 1; $overdueforbranch{'overdueLevel'} = 2; } - - if ( $num->{'notify_level'} eq '3' ) { + elsif ( $num->{'notify_level'} eq '3' ) { $overdueforbranch{'overdue3'} = 1; $overdueforbranch{'overdueLevel'} = 3; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tmpl index ecf58ffe5d..2de61d8cd6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tmpl +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tmpl @@ -36,7 +36,7 @@
  • Overdues - Warning: This report is very resource intensive on systems with large numbers of overdue items.
    • -
  • Overdue fines
    • +
  • Overdues with fines - Limited to your library. See report help for other details.
    • diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/help/circ/branchoverdues.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/help/circ/branchoverdues.tmpl index 5111e0d913..b5ae00ab03 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/help/circ/branchoverdues.tmpl +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/help/circ/branchoverdues.tmpl @@ -2,6 +2,13 @@

    Overdue fines

    +

    This report shows items that: +

    +

    This report will not show items that are so long overdue that the system has marked them 'Lost'

    Once open the report can be filtered by the shelving location.

    -- 2.20.1