[Koha-patches] [PATCH] kohabug 2365 Adding non-digit chars to cost fields in items causes data corruption
Chris Nighswonger
cnighswonger at foundations.edu
Tue Sep 23 17:49:24 CEST 2008
Adding a non-digit char to either cost field on the add/edit item form causes data corruption due to storing the item cost info in biblioitems.marcxml (longtext) as well as items.cost and items.replacementcost (both integer). Thus if we enter '$10.00' the items fields end up NULL while '$10.00' is stored in the marcxml field. This causes problems when item data is retrieved from the items table rather than the biblioitems.marcxml record.
This patch adds code to check the data submitted for that field for chars other than digits and return an error to the user if such chars exist.
---
cataloguing/additem.pl | 66 +++++++++++++++----
.../prog/en/modules/cataloguing/additem.tmpl | 6 +-
2 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl
index f8780c2..bbf1c0e 100755
--- a/cataloguing/additem.pl
+++ b/cataloguing/additem.pl
@@ -113,16 +113,35 @@ if ($op eq "additem") {
$record->insert_fields_ordered($fieldItem);
}
}
-# check for item barcode # being unique
+ #FIXME Error trapping needs to be improved
+ # Check price field formating
+ # Due to items.price and items.replacementprice being decimal types they do not allow chars other than numerals.
+ # biblioitems.marcxml is a longtext type and so will allow other than numerals. So we need to enforce the limitations of the items table fields as the LCD here.
+ my $error = 0;
+ my ($tagfield,$tagsubfield) = &GetMarcFromKohaField("items.price",$frameworkcode);
+ if (($record->field($tagfield)->subfield($tagsubfield)) && ($record->field($tagfield)->subfield($tagsubfield) !~ m/^[0-9]+(?:\.|\,)?[0-9]*/)) {
+ push (@errors, "price_format_error");
+ $error = 1;
+ }
+ ($tagfield,$tagsubfield) = &GetMarcFromKohaField("items.replacementprice",$frameworkcode);
+ if (($record->field($tagfield)->subfield($tagsubfield)) && ($record->field($tagfield)->subfield($tagsubfield) !~ m/^[0-9]+(?:\.|\,)?[0-9]*/)) {
+ push (@errors, "repl_price_format_error");
+ $error = 1;
+ }
+ # check for item barcode # being unique
my $addedolditem = TransformMarcToKoha($dbh,$record);
my $exist_itemnumber = get_item_from_barcode($addedolditem->{'barcode'});
- push @errors,"barcode_not_unique" if($exist_itemnumber);
- # if barcode exists, don't create, but report The problem.
- my ($oldbiblionumber,$oldbibnum,$oldbibitemnum) = AddItemFromMarc($record,$biblionumber) unless ($exist_itemnumber);
if ($exist_itemnumber) {
- $nextop = "additem";
+ push (@errors,"barcode_not_unique");
+ $error = 1;
+ }
+ if ($error) {
+ # if there is an error we want to show the user exactly what they entered
$itemrecord = $record;
- } else {
+ $nextop = "additem";
+ }
+ else {
+ my ($oldbiblionumber,$oldbibnum,$oldbibitemnum) = AddItemFromMarc($record,$biblionumber);
$nextop = "additem";
}
#-------------------------------------------------------------------------------
@@ -166,21 +185,40 @@ if ($op eq "additem") {
# build indicator hash.
my @ind_tag = $input->param('ind_tag');
my @indicator = $input->param('indicator');
- # my $itemnumber = $input->param('itemnumber');
my $xml = TransformHtmlToXml(\@tags,\@subfields,\@values,\@indicator,\@ind_tag,'ITEM');
my $itemtosave=MARC::Record::new_from_xml($xml, 'UTF-8');
- # MARC::Record builded => now, record in DB
- # warn "R: ".$record->as_formatted;
+ #FIXME Error trapping needs to be improved
+ # Check price field formating
+ # Due to items.price and items.replacementprice being decimal types they do not allow chars other than numerals.
+ # biblioitems.marcxml is a longtext type and so will allow other than numerals. So we need to enforce the limitations of the items table fields as the LCD here.
+ my $error = 0;
+ my ($tagfield,$tagsubfield) = &GetMarcFromKohaField("items.price",$frameworkcode);
+ if (($itemtosave->field($tagfield)->subfield($tagsubfield)) && ($itemtosave->field($tagfield)->subfield($tagsubfield) !~ m/^[0-9]+(?:\.|\,)?[0-9]*/)) {
+ push (@errors, "price_format_error");
+ $error = 1;
+ }
+ ($tagfield,$tagsubfield) = &GetMarcFromKohaField("items.replacementprice",$frameworkcode);
+ if (($itemtosave->field($tagfield)->subfield($tagsubfield)) && ($itemtosave->field($tagfield)->subfield($tagsubfield) !~ m/^[0-9]+(?:\.|\,)?[0-9]*/)) {
+ push (@errors, "repl_price_format_error");
+ $error = 1;
+ }
# check that the barcode don't exist already
my $addedolditem = TransformMarcToKoha($dbh,$itemtosave);
my $exist_itemnumber = get_item_from_barcode($addedolditem->{'barcode'});
if ($exist_itemnumber && $exist_itemnumber != $itemnumber) {
push @errors,"barcode_not_unique";
- } else {
+ $error = 1;
+ }
+ if ($error) {
+ # if there is an error we want to show the user exactly what they entered
+ $itemrecord = $itemtosave;
+ $nextop="saveitem";
+ }
+ else {
my ($oldbiblionumber,$oldbibnum,$oldbibitemnum) = ModItemFromMarc($itemtosave,$biblionumber,$itemnumber);
- $itemnumber="";
+ $itemnumber="";
+ $nextop="additem";
}
- $nextop="additem";
}
#
@@ -215,7 +253,7 @@ foreach my $field (@fields) {
|| $subf[$i][1];
}
- if (($field->tag eq $branchtagfield) && ($subf[$i][$0] eq $branchtagsubfield) && C4::Context->preference("IndependantBranches")) {
+ if (($field->tag eq $branchtagfield) && ($subf[$i][0] eq $branchtagsubfield) && C4::Context->preference("IndependantBranches")) {
#verifying rights
my $userenv = C4::Context->userenv();
unless (($userenv->{'flags'} == 1) or (($userenv->{'branch'} eq $subf[$i][1]))){
@@ -448,7 +486,7 @@ foreach my $tag (sort keys %{$tagslib}) {
<a href=\"#\" class=\"buttonDot\" onclick=\"Clic$function_name('$subfield_data{id}'); return false;\" title=\"Tag Editor\">...</a>
$javascript";
} else {
- warn "Plugin Failed: $plugin";
+ #warn "Plugin Failed: $plugin";
# supply default input form
$subfield_data{marc_value} =
"<input type=\"text\"
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tmpl
index 3f48ac2..cccbd5b 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tmpl
@@ -143,11 +143,11 @@ function CreateKey(){
//]]>
</script>
-<link type="text/css" rel="stylesheet" href="<!-- TMPL_VAR NAME="themelang" -->/css/addbiblio.css" />
+<link rel="stylesheet" href="<!-- TMPL_VAR NAME="themelang" -->/css/addbiblio.css" />
</head>
<body>
<!-- TMPL_INCLUDE NAME="header.inc" -->
-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/cataloguing/addbooks.pl">Cataloging</a> › <a href="/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=<!-- TMPL_VAR NAME="biblionumber" -->">Edit <!-- TMPL_VAR name="title" --> <!-- TMPL_IF NAME="author" --> by <!-- TMPL_VAR name="author" --><!-- /TMPL_IF --> (Record #<!-- TMPL_VAR NAME="biblionumber" -->)</a> › Items</div>
+<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/cataloguing/addbooks.pl">Cataloguing</a> › <a href="/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=<!-- TMPL_VAR NAME="biblionumber" -->">Edit <!-- TMPL_VAR name="title" --> <!-- TMPL_IF NAME="author" --> by <!-- TMPL_VAR name="author" --><!-- /TMPL_IF --> (Record #<!-- TMPL_VAR NAME="biblionumber" -->)</a> › Items</div>
<div id="doc3" class="yui-t2">
@@ -161,6 +161,8 @@ function CreateKey(){
<!-- TMPL_IF NAME="barcode_not_unique" --><div class="dialog alert"><strong>Error saving item</strong>: Barcode must be unique.</div><!-- /TMPL_IF -->
<!-- TMPL_IF NAME="book_on_loan" --><div class="dialog alert"><strong>Cannot Delete</strong>: item is checked out.</div><!-- /TMPL_IF -->
<!-- TMPL_IF NAME="book_reserved" --><div class="dialogalert"><strong>Cannot Delete</strong>: item has a waiting hold.</div><!-- /TMPL_IF -->
+<!-- TMPL_IF NAME="price_format_error" --><div class="dialog alert"><strong>Error saving item</strong>: Purchase price field must contain only a decimal number.</div><!-- /TMPL_IF -->
+<!-- TMPL_IF NAME="repl_price_format_error" --><div class="dialog alert"><strong>Error saving item</strong>: Replacement price field must contain only a decimal number.</div><!-- /TMPL_IF -->
<div id="cataloguing_additem_itemlist">
<!-- TMPL_IF name="item_loop" -->
--
1.5.3.7
More information about the Koha-patches
mailing list