Bug 10428 - undefined check in add additem.pl
Summary: undefined check in add additem.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Fridolin Somers
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-07 08:17 UTC by Fridolin Somers
Modified: 2014-12-07 20:02 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (1.55 KB, patch)
2013-06-07 08:22 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 10428 - undefined check in add additem.pl (1.60 KB, patch)
2013-06-07 12:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10428 - undefined check in add additem.pl (1.69 KB, patch)
2013-06-10 09:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2013-06-07 08:17:20 UTC
In additem.pl, in the code that generates item form you find : 
@values = $itemrecord->field($tag)->subfield($subtag) if ($itemrecord && defined($itemrecord->field($tag)->subfield($subtag)));

This code is missing the check of $itemrecord->field($tag) undef.
I did not found a way to test it because item tag is always defined when editing an item, but it would be safer/prettier to add this check.
Comment 1 Fridolin Somers 2013-06-07 08:22:43 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2013-06-07 12:51:00 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2013-06-07 12:51:46 UTC
Agreed. Not easy to test, but this is a completely sensible additional test.
Comment 4 Marcel de Rooy 2013-06-10 09:18:36 UTC
Created attachment 18814 [details] [review]
Bug 10428 - undefined check in add additem.pl

In additem.pl, in the code that generates item form you find :
@values = $itemrecord->field($tag)->subfield($subtag) if ($itemrecord && defined($itemrecord->field($tag)->subfield($subtag)));

This patch adds the check of $itemrecord->field($tag) undef.
I did not found a way to test it because item tag is always defined when editing an item, but it would be safer/prettier to add this check.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passed-QA-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Galen Charlton 2013-06-10 14:25:39 UTC
(In reply to comment #0)
> In additem.pl, in the code that generates item form you find : 
> @values = $itemrecord->field($tag)->subfield($subtag) if ($itemrecord &&
> defined($itemrecord->field($tag)->subfield($subtag)));
> 
> This code is missing the check of $itemrecord->field($tag) undef.
> I did not found a way to test it because item tag is always defined when
> editing an item, but it would be safer/prettier to add this check.

Just double-checking: did you run into any situation where the absence of the test resulted in a user-visible problem?

It occurs to me that intentionally damaging the contents of the LastCreatedItem cookie would be a way of exercising this patch.
Comment 6 Galen Charlton 2013-07-03 11:32:01 UTC
Pushed to master.  Thanks, Fridolyn!
Comment 7 Tomás Cohen Arazi 2013-07-15 23:43:10 UTC
This patch has been pushed to 3.12.x, will be in 3.12.2.

Thanks Fridolyn!
Comment 8 Bernardo Gonzalez Kriegel 2013-08-03 18:13:51 UTC
Pushed to 3.10.x, will be in 3.10.10
Comment 9 Chris Hall 2013-08-18 04:23:22 UTC
Pushed to 3.8.x, will be in 3.8.17