Fixing Database Definitions for Statuses *PARTIAL*
authorJoshua Ferraro <jmf@liblime.com>
Thu, 3 Jan 2008 18:36:12 +0000 (12:36 -0600)
committerJoshua Ferraro <jmf@liblime.com>
Thu, 3 Jan 2008 22:23:04 +0000 (16:23 -0600)
Prior to this fix, the status fields had three 'off' values, NULL, "",
and 0. I've reduced it to two in the db, removing the option for NULL, and
setting the default value to 0, however, we need to verify that we don't ever
write out as "" as this needlessly complicates the indexing process,
critical for searching or limiting by status (e.g., availability). Also,
queries that attempt to write a NULL value to one of these fields will fail
(based on my tests).

This patch includes the following changes:

* Updated the database definition for notforloan, damaged, itemlost, and
wthdrawn in kohastructure.sql to forbid NULL and default to 0; MySQL
can't forbid other values (such as empty ""), so this has to be handled
at the application layer and REQUIRES further patching.

* Fixed the 'limit by availability' query node in Search.pm to use a
much less confusing definition of 'available'

* Added code to set values to 0 where they are NULL or empty ( "" ) for
notforloan, damaged, itemlost or wthdrawn in both the MARC and the items
table:

  * Biblio.pm -> AddBiblioAndItems
  * catalogue/updateitem.pl
  * SEE NOTE BELOW, REQUIRES UPDATE TO THE REST OF KOHA'S ITEM MGT!

* Removed code in bulkmarcimport.pl that sets notforloan status depending
  on item-level or bib-level itemtype -- that flag is designed to be set
  only to override the notforloan setting for the item's (or bib's,
  depending on the syspref) assigned itemtype (it doesn't need to override
  to 'for loan', only to 'not for loan').

  added $dbh->do("truncate zebraqueue"); when operation is 'delete'

* I updated some notes in catalogue/updateitem.pl as to why ModItem can't be
used -- we don't have _a_ place where we can change the item and marc :/

  I've tested the following:

  bulkmarcimport.pl..........................MARC/items OK
  Staged Records Import......................NOT OK
  updateitem.pl (via moredetail.pl)..........MARC/items OK
  circulation.pl.............................NOT OK
  returns.pl.................................NOT OK
  addbiblio.pl...............................NOT OK
  additem.pl.................................NOT OK

Basically, there isn't a single place to apply this patch that will
update both item data and MARC data in one place ... a future patch
needs to address this issue.

Signed-off-by: Galen Charlton <galen.charlton@liblime.com>
Signed-off-by: Chris Cormack <crc@liblime.com>
Signed-off-by: Joshua Ferraro <jmf@liblime.com>
C4/Biblio.pm
C4/Search.pm
catalogue/updateitem.pl
installer/data/mysql/kohastructure.sql
misc/migration_tools/bulkmarcimport.pl

index 6f54a84..35cb1b3 100755 (executable)
@@ -31,7 +31,6 @@ use C4::Branch;
 use C4::Dates qw/format_date/;
 use C4::Log; # logaction
 use C4::ClassSource;
-
 use vars qw($VERSION @ISA @EXPORT);
 
 # TODO: fix version
@@ -323,24 +322,14 @@ sub AddBiblioAndItems {
             warn "ERROR: cannot add item $item->{'barcode'} for biblio $biblionumber: duplicate barcode\n";
         }
 
-        # figure out what item type to use -- biblioitem-level or item-level
-        my $itemtype;
-        if (C4::Context->preference('item-level_itypes')) {
-            $itemtype = $item->{'itype'};
-        } else {
-            $itemtype = $olddata->{'itemtype'};
-        }
-
-        # FIXME - notforloan stuff copied from AddItem
-        my $sth = $dbh->prepare("SELECT notforloan FROM itemtypes WHERE itemtype=?");
-        $sth->execute($itemtype);
-        my $notforloan = $sth->fetchrow;
-        ##Change the notforloan field if $notforloan found
-        if ( $notforloan > 0 ) {
-            $item->{'notforloan'} = $notforloan;
-            &MARCitemchange( $temp_item_marc, "items.notforloan", $notforloan );
+        # Make sure item statuses are set to 0 if empty or NULL in both the item and the MARC
+        for ('notforloan', 'damaged','itemlost','wthdrawn') {
+            if (!$item->{$_} or $item->{$_} eq "") {
+                $item->{$_} = 0;
+                &MARCitemchange( $temp_item_marc, "items.$_", 0 );
+            }
         }
-
         # FIXME - dateaccessioned stuff copied from AddItem
         if ( !$item->{'dateaccessioned'} || $item->{'dateaccessioned'} eq '' ) {
 
@@ -422,7 +411,6 @@ sub _repack_item_errors {
 sub AddItem {
     my ( $record, $biblionumber ) = @_;
     my $dbh = C4::Context->dbh;
-    
     # add item in old-DB
     my $frameworkcode = GetFrameworkCode( $biblionumber );
     my $item = &TransformMarcToKoha( $dbh, $record, $frameworkcode );
@@ -4141,7 +4129,6 @@ my ($itemnumber,$error) = _koha_new_items( $dbh, $item, $barcode );
 sub _koha_new_items {
     my ( $dbh, $item, $barcode ) = @_;
     my $error;
-
     my ($items_cn_sort) = GetClassSort($item->{'items.cn_source'}, $item->{'itemcallnumber'}, "");
 
     # if dateaccessioned is provided, use it. Otherwise, set to NOW()
index 6e8cfd2..232fa42 100644 (file)
@@ -1019,10 +1019,11 @@ sub buildQuery {
     foreach my $this_limit (@limits) {
         if ( $this_limit =~ /available/ ) {
 
-# available is defined as (items.notloan is NULL) and (items.itemlost > 0 or NULL) (last clause handles NULL values for lost in zebra)
-# all records not indexed in the onloan register and allrecords not indexed in the lost register, or where the value of lost is equal to or less than 0
+# 'available' is defined as (items.onloan is NULL) and (items.itemlost = 0)
+# In English:
+# all records not indexed in the onloan register (zebra) and all records with a value of lost equal to 0
             $availability_limit .=
-"( ( allrecords,AlwaysMatches='' not onloan,AlwaysMatches='') and ((lost,st-numeric <= 0) or ( allrecords,AlwaysMatches='' not lost,AlwaysMatches='')) )";
+"( ( allrecords,AlwaysMatches='' not onloan,AlwaysMatches='') and (lost,st-numeric=0) )"; #or ( allrecords,AlwaysMatches='' not lost,AlwaysMatches='')) )";
             $limit_cgi  .= "&limit=available";
             $limit_desc .= "";
         }
index 8beae00..83a11ee 100755 (executable)
@@ -39,68 +39,80 @@ my $damaged=$cgi->param('damaged');
 
 my $confirm=$cgi->param('confirm');
 my $dbh = C4::Context->dbh;
+
 # get the rest of this item's information
 my $item_data_hashref = GetItem($itemnumber, undef);
-my $newitemdata;
+
+# make sure item statuses are set to 0 if empty or NULL
+for ($damaged,$itemlost,$wthdrawn) {
+    if (!$_ or $_ eq "") {
+        $_ = 0;
+    }
+}
+
 # modify MARC item if input differs from items table.
 if ( $itemnotes ne $item_data_hashref->{'itemnotes'}) {
     ModItemInMarconefield($biblionumber, $itemnumber, 'items.itemnotes', $itemnotes);
-       $newitemdata->{'itemnotes'} = $itemnotes;
+    $item_data_hashref->{'itemnotes'} = $itemnotes;
 } elsif ($itemlost ne $item_data_hashref->{'itemlost'}) {
     ModItemInMarconefield($biblionumber, $itemnumber, 'items.itemlost', $itemlost);
-       $newitemdata->{'itemlost'} = $itemlost;
+    $item_data_hashref->{'itemlost'} = $itemlost;
 } elsif ($wthdrawn ne $item_data_hashref->{'wthdrawn'}) {
     ModItemInMarconefield($biblionumber, $itemnumber, 'items.wthdrawn', $wthdrawn);
-       $newitemdata->{'wthdrawn'} = $wthdrawn;
+    $item_data_hashref->{'wthdrawn'} = $wthdrawn;
 } elsif ($damaged ne $item_data_hashref->{'damaged'}) {
     ModItemInMarconefield($biblionumber, $itemnumber, 'items.damaged', $damaged);
-       $newitemdata->{'damaged'} = $damaged;
+    $item_data_hashref->{'damaged'} = $damaged;
 } else {
-       #nothings changed, so do nothing.
-       print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber");
+    #nothings changed, so do nothing.
+    print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber");
 }
 
 # FIXME: eventually we'll use Biblio.pm, but it's currently too buggy  (is this current ??)
+# yes as of dec 30 2007, ModItem doesn't update zebra for status changes, it requires
+# a MARC record be passed in
 #ModItem( $dbh,'',$biblionumber,$itemnumber,'',$item_hashref );
-       $newitemdata->{'itemnumber'} = $itemnumber;
-#      &C4::Biblio::_koha_modify_item($dbh,$newitemdata);
-
-       my $sth = $dbh->prepare("UPDATE items SET wthdrawn=?,itemlost=?,damaged=?,itemnotes=? WHERE itemnumber=?");
-       $sth->execute($wthdrawn,$itemlost,$damaged,$itemnotes,$itemnumber);
-       &ModZebra($biblionumber,"specialUpdate","biblioserver");
-       
+#   &C4::Biblio::_koha_modify_item($dbh,$item_data_hashref);
+    my $sth = $dbh->prepare("UPDATE items SET wthdrawn=?,itemlost=?,damaged=?,itemnotes=? WHERE itemnumber=?");
+    $sth->execute($wthdrawn,$itemlost,$damaged,$itemnotes,$itemnumber);
+    &ModZebra($biblionumber,"specialUpdate","biblioserver");
+    
 # check issues iff itemlost.
- # FIXME : is there documentation or enforcement that itemlost value must be '1'?  if no replacement price, then borrower just doesn't get charged?
+# http://wiki.koha.org/doku.php?id=en:development:kohastatuses
+# lost ==1 Lost, lost==2 longoverdue, lost==3 lost and paid for
+# FIXME: itemlost should be set to 3 after payment is made, should be a warning to the interface that
+# a charge has been added
+# FIXME : if no replacement price, borrower just doesn't get charged?
 if ($itemlost==1) {
-       my $sth=$dbh->prepare("SELECT * FROM issues WHERE (itemnumber=? AND returndate IS NULL)");
-       $sth->execute($itemnumber);
-       my $issues=$sth->fetchrow_hashref();
+    my $sth=$dbh->prepare("SELECT * FROM issues WHERE (itemnumber=? AND returndate IS NULL)");
+    $sth->execute($itemnumber);
+    my $issues=$sth->fetchrow_hashref();
 
-       # if a borrower lost the item, add a replacement cost to the their record
-       if ( ($issues->{borrowernumber}) && ($itemlost==1) ){
+    # if a borrower lost the item, add a replacement cost to the their record
+    if ( ($issues->{borrowernumber}) && ($itemlost==1) ){
 
-               # first make sure the borrower hasn't already been charged for this item
-               my $sth1=$dbh->prepare("SELECT * from accountlines
-               WHERE borrowernumber=? AND itemnumber=?");
-               $sth1->execute($issues->{'borrowernumber'},$itemnumber);
-               my $existing_charge_hashref=$sth1->fetchrow_hashref();
+        # first make sure the borrower hasn't already been charged for this item
+        my $sth1=$dbh->prepare("SELECT * from accountlines
+        WHERE borrowernumber=? AND itemnumber=?");
+        $sth1->execute($issues->{'borrowernumber'},$itemnumber);
+        my $existing_charge_hashref=$sth1->fetchrow_hashref();
 
-               # OK, they haven't
-               unless ($existing_charge_hashref) {
-                       # This item is on issue ... add replacement cost to the borrower's record and mark it returned
-                       my $accountno = getnextacctno('',$issues->{'borrowernumber'},$dbh);
-                       my $sth2=$dbh->prepare("INSERT INTO accountlines
-                       (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding,itemnumber)
-                       VALUES
-                       (?,?,now(),?,?,'L',?,?)");
-                       $sth2->execute($issues->{'borrowernumber'},$accountno,$item_data_hashref->{'replacementprice'},
-                       "Lost Item $item_data_hashref->{'title'} $item_data_hashref->{'barcode'}",
-                       $item_data_hashref->{'replacementprice'},$itemnumber);
-                       $sth2->finish;
-               # FIXME: Log this ?
-               }
-       }
-       $sth->finish;
+        # OK, they haven't
+        unless ($existing_charge_hashref) {
+            # This item is on issue ... add replacement cost to the borrower's record and mark it returned
+            my $accountno = getnextacctno('',$issues->{'borrowernumber'},$dbh);
+            my $sth2=$dbh->prepare("INSERT INTO accountlines
+            (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding,itemnumber)
+            VALUES
+            (?,?,now(),?,?,'L',?,?)");
+            $sth2->execute($issues->{'borrowernumber'},$accountno,$item_data_hashref->{'replacementprice'},
+            "Lost Item $item_data_hashref->{'title'} $item_data_hashref->{'barcode'}",
+            $item_data_hashref->{'replacementprice'},$itemnumber);
+            $sth2->finish;
+        # FIXME: Log this ?
+        }
+    }
+    $sth->finish;
 }
 
 print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber");
index 456b790..d19aa05 100644 (file)
@@ -806,10 +806,10 @@ CREATE TABLE `deleteditems` (
   `datelastborrowed` date default NULL,
   `datelastseen` date default NULL,
   `stack` tinyint(1) default NULL,
-  `notforloan` tinyint(1) default NULL,
-  `damaged` tinyint(1) default NULL,
-  `itemlost` tinyint(1) default NULL,
-  `wthdrawn` tinyint(1) default NULL,
+  `notforloan` tinyint(1) NOT NULL default 0,
+  `damaged` tinyint(1) NOT NULL default 0,
+  `itemlost` tinyint(1) NOT NULL default 0,
+  `wthdrawn` tinyint(1) NOT NULL default 0,
   `itemcallnumber` varchar(30) default NULL,
   `issues` smallint(6) default NULL,
   `renewals` smallint(6) default NULL,
@@ -1019,10 +1019,10 @@ CREATE TABLE `items` (
   `datelastborrowed` date default NULL,
   `datelastseen` date default NULL,
   `stack` tinyint(1) default NULL,
-  `notforloan` tinyint(1) default NULL,
-  `damaged` tinyint(1) default NULL,
-  `itemlost` tinyint(1) default NULL,
-  `wthdrawn` tinyint(1) default NULL,
+  `notforloan` tinyint(1) NOT NULL default 0,
+  `damaged` tinyint(1) NOT NULL default 0,
+  `itemlost` tinyint(1) NOT NULL default 0,
+  `wthdrawn` tinyint(1) NOT NULL default 0,
   `itemcallnumber` varchar(30) default NULL,
   `issues` smallint(6) default NULL,
   `renewals` smallint(6) default NULL,
index de1ae96..c75eac4 100755 (executable)
@@ -1,5 +1,5 @@
 #!/usr/bin/perl
-# small script that import an iso2709 file into koha 2.0
+# Import an iso2709 file into Koha 3
 
 use strict;
 # use warnings;
@@ -189,6 +189,7 @@ if ($delete) {
     $dbh->do("truncate biblio");
     $dbh->do("truncate biblioitems");
     $dbh->do("truncate items");
+    $dbh->do("truncate zebraqueue");
 }
 if ($fk_off) {
        $dbh->do("SET FOREIGN_KEY_CHECKS = 0");