[Koha-patches] [PATCH] Branchoverdues circ report reworking.
Joe Atzberger
joe.atzberger at liblime.com
Tue Jan 27 01:24:10 CET 2009
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.
---
C4/Overdues.pm | 166 +++++++------------
circ/branchoverdues.pl | 30 ++--
.../prog/en/modules/circ/circulation-home.tmpl | 2 +-
.../prog/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 63b0842..8c18294 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 a09f5d3..a33d836 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 e692fe7..884b0dc 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 @@
<li> <a href="/cgi-bin/koha/circ/overdue.pl">Overdues</a>
- <b>Warning:</b> This report is very resource intensive on
systems with large numbers of overdue items.</li>
- <li> <a href="/cgi-bin/koha/circ/branchoverdues.pl">Overdue fines</a></li>
+ <li> <a href="/cgi-bin/koha/circ/branchoverdues.pl">Overdues with fines</a> - Limited to your library. See report help for other details.</li>
<!-- <li> <a href="/cgi-bin/koha/circ/billing.pl">Billing</a></li> -->
<!-- <li> <a href="/cgi-bin/koha/circ/stats.pl?time=yesterday">Daily reconciliation</a></li> -->
</ul>
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 5111e0d..b5ae00a 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 @@
<h3>Overdue fines</h3>
+<p>This report shows items that:
+<ul>
+ <li>are still checked out,</li>
+ <li>have not had a notice sent, and</li>
+ <li>already have an associated fine (type "FU").</li>
+</ul>
+
<p>This report will not show items that are so long overdue that the system has marked them 'Lost'</p>
<p>Once open the report can be filtered by the shelving location.</p>
--
1.5.5.GIT
More information about the Koha-patches
mailing list