From 5c23369af2536896396a8584b302b807423243ae Mon Sep 17 00:00:00 2001 From: Joshua Ferraro Date: Thu, 3 Jan 2008 12:36:12 -0600 Subject: [PATCH] Fixing Database Definitions for Statuses *PARTIAL* 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 Signed-off-by: Chris Cormack Signed-off-by: Joshua Ferraro --- C4/Biblio.pm | 27 ++------ C4/Search.pm | 7 +- catalogue/updateitem.pl | 94 +++++++++++++++----------- installer/data/mysql/kohastructure.sql | 16 ++--- misc/migration_tools/bulkmarcimport.pl | 3 +- 5 files changed, 74 insertions(+), 73 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 6f54a84535..35cb1b33c9 100755 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -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() diff --git a/C4/Search.pm b/C4/Search.pm index 6e8cfd2fbf..232fa42ea5 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -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 .= ""; } diff --git a/catalogue/updateitem.pl b/catalogue/updateitem.pl index 8beae002c1..83a11ee4fd 100755 --- a/catalogue/updateitem.pl +++ b/catalogue/updateitem.pl @@ -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"); diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 456b790ba8..d19aa05cc5 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -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, diff --git a/misc/migration_tools/bulkmarcimport.pl b/misc/migration_tools/bulkmarcimport.pl index de1ae960e8..c75eac4ebd 100755 --- a/misc/migration_tools/bulkmarcimport.pl +++ b/misc/migration_tools/bulkmarcimport.pl @@ -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"); -- 2.39.2