Bug 7239 - Acquisitions, add order to basket, from new, no item field filed, AcqCreateItem=ordering cause internal error
Summary: Acquisitions, add order to basket, from new, no item field filed, AcqCreateIt...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) minor (vote)
Target Milestone: ---
Assignee: Henri-Damien LAURENT
QA Contact: Ian Walls
URL:
Whiteboard:
Keywords:
: 4955 7692 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-11-16 23:19 UTC by Chris Hall
Modified: 2013-12-05 19:59 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Patch adds a stronger check on adding a closing </datafield> tag (453 bytes, patch)
2011-11-17 03:17 UTC, Chris Hall
Details | Diff | Splinter Review
Patch adds a stronger check on adding a closing </datafield> tag (786 bytes, patch)
2011-12-06 03:05 UTC, Chris Hall
Details | Diff | Splinter Review
[SIGNED-OFF] bug 7239 fix to avoid error being thrown even though a record is created (987 bytes, patch)
2011-12-06 06:01 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Hall 2011-11-16 23:19:18 UTC
When creating a new basket in Acquisitions the fields "Title" and "quantity" are red (indicating mandatory) so I fill them in and click save, however if you do not select an option from "Source of classification or shelving scheme" (leaving it blank) then after clicking save an internal error occurs[1].

Using the back button to go back and fill in this field then clicking save will then show a basket with an excess entry (as the entry from before, although it caused an error, appears to still be added to the basket).


Should "Source of classification or shelving scheme" be marked mandatory, or is this a bug in the display of the basket?


[1] on mykoha.co.nz it is mentioned as "an internal error occured", however using my local setup I get "mismatched tag at line 7, column 2, byte 273 at /usr/lib/perl5/XML/Parser.pm line 187".
Comment 1 Chris Hall 2011-11-17 03:17:04 UTC Comment hidden (obsolete)
Comment 2 Chris Hall 2011-11-17 03:18:35 UTC
Here is a further explanation of the bug, I was planning to post it with the patch but the attach patch button took me to another page.

The bug seems to be caused by the line:
$xml .= "</datafield>\n" if @$tags > 0;
in C4/Biblio.pm (line 1955)

It seems that if the Source of classification or shelving scheme is not filed in then the resulting xml is:
<collection$
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd"
  xmlns="http://www.loc.gov/MARC21/slim">
<record>
</datafield>
</record>
</collection>

and if you do fill in the field then the xml is:
<collection
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd"
  xmlns="http://www.loc.gov/MARC21/slim">
<record>
<datafield tag="952" ind1=" " ind2=" ">
<subfield code="2">ddc</subfield>
</datafield>
</record>
</collection>

Patch places a stronger check on that line (checking if $xml contains a <datafield tag, rather than if $tags size is > 0)
Comment 3 Katrin Fischer 2011-11-17 07:16:39 UTC
I think the problem is perhaps not classification source, but that you have to fill out at least one field from the items form. Normally I use itemtype, branches would probably work too. Bug 4955 might be a duplicate.
Comment 4 Chris Hall 2011-11-18 00:55:01 UTC
Hey Katrin

It appears you are right about any one of the fields having to be filled out, silly me.

The issue described in bug 4955 seems to be related, sorry for the duplicate.
Comment 5 Katrin Fischer 2011-11-18 05:49:42 UTC
I spent a lot of time in acq running into this problem a lot. Glad you came up with a patch for it - it's an annoying bug.
Comment 6 Katrin Fischer 2011-12-05 20:59:50 UTC
Hi Chris,
 
something seems to be wrong with your patch:

git bz apply 7239
Bug 7239 - Acquisitions basket, add order to basket, from new, Source of classification or shelving scheme being blank caused internal error

Patch adds a stronger check on adding a closing </datafield> tag
Apply? [yn] y

Patch format detection failed.

git am -is3 /tmp/Patch-adds-a-stronger-check-on-adding-a-closing-da-zOWPN2.patch
Patch format detection failed.

Can you check and attach a new file please? Thx!
Comment 7 Chris Hall 2011-12-06 03:05:30 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2011-12-06 06:01:52 UTC
Created attachment 6589 [details] [review]
[SIGNED-OFF] bug 7239 fix to avoid error being thrown even though a record is created

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
When creating an item in acquisitions while ordering and not filling out
any fields, there is no longer shown a perl error message.
Comment 9 Paul Poulain 2011-12-14 16:43:25 UTC
QA comment: description updated, everything else fine, 1 line patch works as expected
Comment 10 Katrin Fischer 2011-12-27 20:38:06 UTC
*** Bug 4955 has been marked as a duplicate of this bug. ***
Comment 11 Nicole C. Engard 2012-03-28 19:08:13 UTC
*** Bug 7692 has been marked as a duplicate of this bug. ***
Comment 12 Jared Camins-Esakov 2012-05-24 00:43:21 UTC
This patch was included in 3.6.x prior to 3.6.4.