[Koha-cvs] CVS: koha/C4/Circulation Fines.pm,1.5,1.6

[Andrew Arensburger]

Update of /cvsroot/koha/koha/C4/Circulation
In directory usw-pr-cvs1:/tmp/cvs-serv21702

Modified Files:
        Fines.pm 
Log Message:
Added magic RCS comment.
Removed a lying comment.
Removed unused global variables.
Added POD.
Added some FIXME comments.
Added some explanatory comments.
Removed unused finalizer.


Index: Fines.pm
===================================================================
RCS file: /cvsroot/koha/koha/C4/Circulation/Fines.pm,v
retrieving revision 1.5
retrieving revision 1.6
diff -C2 -r1.5 -r1.6
*** Fines.pm    5 Oct 2002 09:54:21 -0000       1.5
--- Fines.pm    13 Oct 2002 06:18:42 -0000      1.6
***************
*** 1,7 ****
! package C4::Circulation::Fines; #asummes C4/Circulation/Fines
! 
! #requires DBI.pm to be installed
! #uses DBD:Pg
  
  
  # Copyright 2000-2002 Katipo Communications
--- 1,5 ----
! package C4::Circulation::Fines;
  
+ # $Id$
  
  # Copyright 2000-2002 Katipo Communications
***************
*** 26,73 ****
  use DBI;
  use C4::Context;
! use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
  
  # set the version for version checking
  $VERSION = 0.01;
  
! @ISA = qw(Exporter);
! @EXPORT = qw(&Getoverdues &CalcFine &BorType &UpdateFine &ReplacementCost);
! %EXPORT_TAGS = ( );     # eg: TAG => [ qw!name1 name2! ],
  
! # your exported package globals go here,
! # as well as any optionally exported functions
  
! @EXPORT_OK   = qw($Var1 %Hashit);
  
  
! # non-exported package globals go here
! use vars qw(@more $stuff);
  
! # initalize package globals, first exported ones
  
! my $Var1   = '';
! my %Hashit = ();
  
  
! # then the others (which are still accessible as $Some::Module::stuff)
! my $stuff  = '';
! my @more   = ();
  
! # all file-scoped lexicals must be created before
! # the functions below that use them.
  
! # file-private lexicals go here
! my $priv_var    = '';
! my %secret_hash = ();
  
! # here's a file-private function as a closure,
! # callable as &$priv_func;  it cannot be prototyped.
! my $priv_func = sub {
!   # stuff goes here.
!   };
!   
! # make all your functions, whether exported or not;
  
  
  sub Getoverdues{
    my $dbh = C4::Context->dbh;
--- 24,68 ----
  use DBI;
  use C4::Context;
! use vars qw($VERSION @ISA @EXPORT);
  
  # set the version for version checking
  $VERSION = 0.01;
  
! =head1 NAME
  
! C4::Circulation::Fines - Koha module dealing with fines
  
! =head1 SYNOPSIS
  
+   use C4::Circulation::Fines;
  
! =head1 DESCRIPTION
  
! This module contains several functions for dealing with fines for
! overdue items. It is primarily used by the 'misc/fines2.pl' script.
  
! =head1 FUNCTIONS
  
+ =over 2
+ 
+ =cut
+ 
+ @ISA = qw(Exporter);
+ @EXPORT = qw(&Getoverdues &CalcFine &BorType &UpdateFine &ReplacementCost);
  
! =item Getoverdues
  
!   ($count, $overdues) = &Getoverdues();
  
! Returns the list of all overdue books.
  
! C<$count> is the number of elements in C<@{$overdues}>.
  
+ C<$overdues> is a reference-to-array. Each element is a
+ reference-to-hash whose keys are the fields of the issues table in the
+ Koha database.
  
+ =cut
+ #'
  sub Getoverdues{
    my $dbh = C4::Context->dbh;
***************
*** 76,79 ****
--- 71,75 ----
    my $sth=$dbh->prepare($query);
    $sth->execute;
+   # FIXME - Use push @results
    my $i=0;
    my @results;
***************
*** 84,93 ****
--- 80,151 ----
    $sth->finish;
  #  print @results;
+   # FIXME - Bogus API.
    return($i,address@hidden);  
  }
  
+ =item CalcFine
+ 
+   ($amount, $chargename, $message) =
+       &CalcFine($itemnumber, $borrowercode, $days_overdue);
+ 
+ Calculates the fine for a book.
+ 
+ The categoryitems table in the Koha database is a fine matrix, listing
+ the penalties for each type of patron for each type of item (e.g., the
+ standard fine for books might be $0.50, but $1.50 for DVDs, or staff
+ members might get a longer grace period between the first and second
+ reminders that a book is overdue).
+ 
+ The fine is calculated as follows: if it is time for the first
+ reminder, the fine is the value listed for the given (item type,
+ borrower code) combination. If it is time for the second reminder, the
+ fine is doubled. Finally, if it is time to send the account to a
+ collection agency, the fine is set to 5 local monetary units (a really
+ good deal for the patron if the library is in Italy). Otherwise, the
+ fine is 0.
+ 
+ Note that the way this function is currently implemented, it only
+ returns a nonzero value on the notable days listed above. That is, if
+ the categoryitems entry says to send a first reminder 7 days after the
+ book is due, then if you call C<&CalcFine> 7 days after the book is
+ due, it will give a nonzero fine. If you call C<&CalcFine> the next
+ day, however, it will say that the fine is 0.
+ 
+ C<$itemnumber> is the book's item number.
+ 
+ C<$borrowercode> is the borrower code of the patron who currently has
+ the book.
+ 
+ C<$days_overdue> is the number of days elapsed since the book's due
+ date.
+ 
+ C<&CalcFine> returns a list of three values:
+ 
+ C<$amount> is the fine owed by the patron (see above).
+ 
+ C<$chargename> is the chargename field from the applicable record in
+ the categoryitem table, whatever that is.
+ 
+ C<$message> is a text message, either "First Notice", "Second Notice",
+ or "Final Notice".
+ 
+ =cut
+ #'
  sub CalcFine {
    my ($itemnumber,$bortype,$difference)address@hidden;
    my $dbh = C4::Context->dbh;
+ 
+   # Look up the categoryitem record for this book's item type and the
+   # given borrwer type.
+   # The reason this query is so messy is that it's a messy question:
+   # given the barcode, we can find the book's items record. This gives
+   # us the biblioitems record, which gives us a set of categoryitem
+   # records. Then we select the one that corresponds to the desired
+   # borrower type.
+ 
+   # FIXME - Is it really necessary to get absolutely everything from
+   # all four tables? It looks as if this code only wants
+   # firstremind, chargeperiod, accountsent, and chargename from the
+   # categoryitem table.
    my $query="Select * from items,biblioitems,itemtypes,categoryitem where 
items.itemnumber=$itemnumber
    and items.biblioitemnumber=biblioitems.biblioitemnumber and
***************
*** 95,115 ****
--- 153,199 ----
    categoryitem.itemtype=itemtypes.itemtype and
    categoryitem.categorycode='$bortype' and (items.itemlost <> 1 or 
items.itemlost is NULL)";
+ 
    my $sth=$dbh->prepare($query);
  #  print $query;
    $sth->execute;
    my $data=$sth->fetchrow_hashref;
+       # FIXME - Error-checking: the item might be lost, or there
+       # might not be an entry in 'categoryitem' for this item type
+       # or borrower type.
    $sth->finish;
    my $amount=0;
    my $printout;
+ 
+   # Is it time to send out the first reminder?
+   # FIXME - I'm not sure the "=="s are correct here. Let's say that
+   # $data->{firstremind} is today, but 'fines2.pl' doesn't run for
+   # some reason (the cron daemon died, the server crashed, the
+   # sysadmin had the machine down for maintenance, or whatever).
+   #
+   # Then the next day, the book is $data->{firstremind}+1 days
+   # overdue. But this function returns $amount == 0, $printout ==
+   # undef, on the assumption that 'fines2.pl' ran the previous day. So
+   # the first thing the patron gets is a second notice, but that's a
+   # week after the server crash, so people may not connect the two
+   # events.
    if ($difference == $data->{'firstremind'}){
+     # Yes. Set the fine as listed.
      $amount=$data->{'fine'};
      $printout="First Notice";
    }
+ 
+   # Is it time to send out a second reminder?
    my $second=$data->{'firstremind'}+$data->{'chargeperiod'};
    if ($difference == $second){
+     # Yes. The fine is double.
      $amount=$data->{'fine'}*2;
      $printout="Second Notice";
    }
+ 
+   # Is it time to send the account to a collection agency?
+   # FIXME - At least, I *think* that's what this code is doing.
    if ($difference == $data->{'accountsent'} && $data->{'fine'} > 0){
+     # Yes. Set the fine at 5 local monetary units.
+     # FIXME - This '5' shouldn't be hard-wired.
      $amount=5;
      $printout="Final Notice";
***************
*** 118,124 ****
--- 202,244 ----
  }
  
+ =item UpdateFine
+ 
+   &UpdateFine($itemnumber, $borrowernumber, $amount, $type, $description);
+ 
+ (Note: the following is mostly conjecture and guesswork.)
+ 
+ Updates the fine owed on an overdue book.
+ 
+ C<$itemnumber> is the book's item number.
+ 
+ C<$borrowernumber> is the borrower number of the patron who currently
+ has the book on loan.
+ 
+ C<$amount> is the current amount owed by the patron.
+ 
+ C<$type> will be used in the description of the fine.
+ 
+ C<$description> is a string that must be present in the description of
+ the fine. I think this is expected to be a date in DD/MM/YYYY format.
+ 
+ C<&UpdateFine> looks up the amount currently owed on the given item
+ and sets it to C<$amount>, creating, if necessary, a new entry in the
+ accountlines table of the Koha database.
+ 
+ =cut
+ #'
+ # FIXME - This API doesn't look right: why should the caller have to
+ # specify both the item number and the borrower number? A book can't
+ # be on loan to two different people, so the item number should be
+ # sufficient.
  sub UpdateFine {
    my ($itemnum,$bornum,$amount,$type,$due)address@hidden;
    my $dbh = C4::Context->dbh;
+   # FIXME - What exactly is this query supposed to do? It looks up an
+   # entry in accountlines that matches the given item and borrower
+   # numbers, where the description contains $due, and where the
+   # account type has one of several values, but what does this _mean_?
+   # Does it look up existing fines for this item?
+   # FIXME - What are these various account types? ("FU", "O", "F", "M")
    my $query="Select * from accountlines where itemnumber=$itemnum and
    borrowernumber=$bornum and (accounttype='FU' or accounttype='O' or
***************
*** 129,132 ****
--- 249,254 ----
  
    if (my $data=$sth->fetchrow_hashref){
+     # I think this if-clause deals with the case where we're updating
+     # an existing fine.
  #    print "in accounts ...";
      if ($data->{'amount'} != $amount){
***************
*** 135,138 ****
--- 257,261 ----
        my $diff=$amount - $data->{'amount'};
        my $out=$data->{'amountoutstanding'}+$diff;
+       # FIXME - Use $dbh->do()
        my $query2="update accountlines set date=now(), amount=$amount,
        amountoutstanding=$out,accounttype='FU' where
***************
*** 146,149 ****
--- 269,274 ----
      }
    } else {
+     # I think this else-clause deals with the case where we're adding
+     # a new fine.
      my $query2="select title from biblio,items where items.itemnumber=$itemnum
      and biblio.biblionumber=items.biblionumber";
***************
*** 157,164 ****
--- 282,292 ----
      my $sth3=$dbh->prepare($query2);
      $sth3->execute;
+     # FIXME - Make $accountno a scalar.
      my @accountno=$sth3->fetchrow_array;
      $sth3->finish;
      $accountno[0]++;
      $title->{'title'}=~ s/\'/\\\'/g;
+               # FIXME - There are probably other characters that need
+               # to be escaped. Use $dbh->quote.
      $query2="Insert into accountlines
      (borrowernumber,itemnumber,date,amount,
***************
*** 173,176 ****
--- 301,317 ----
  }
  
+ =item BorType
+ 
+   $borrower = &BorType($borrowernumber);
+ 
+ Looks up a patron by borrower number.
+ 
+ C<$borrower> is a reference-to-hash whose keys are all of the fields
+ from the borrowers and categories tables of the Koha database. Thus,
+ C<$borrower> contains all information about both the borrower and
+ category he or she belongs to.
+ 
+ =cut
+ #'
  sub BorType {
    my ($borrowernumber)address@hidden;
***************
*** 186,189 ****
--- 327,338 ----
  }
  
+ =item ReplacementCost
+ 
+   $cost = &ReplacementCost($itemnumber);
+ 
+ Returns the replacement cost of the item with the given item number.
+ 
+ =cut
+ #'
  sub ReplacementCost{
    my ($itemnum)address@hidden;
***************
*** 192,195 ****
--- 341,345 ----
    my $sth=$dbh->prepare($query);
    $sth->execute;
+   # FIXME - Use fetchrow_array or something.
    my $data=$sth->fetchrow_hashref;
    $sth->finish;
***************
*** 197,201 ****
  }
  
! END { }       # module clean-up code here (global destructor)
!   
!     
--- 347,358 ----
  }
  
! 1;
! __END__
! 
! =back
! 
! =head1 AUTHOR
! 
! Koha Developement team <address@hidden>
! 
! =cut