Bug 26438 - Follow up to bug 23463 - return from Koha::Item overwrites existing variable
Summary: Follow up to bug 23463 - return from Koha::Item overwrites existing variable
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P3 major (vote)
Assignee: Colin Campbell
QA Contact: Testopia
URL:
Keywords:
Depends on: 23463 26082
Blocks:
  Show dependency treegraph
 
Reported: 2020-09-11 17:11 UTC by Colin Campbell
Modified: 2021-06-14 21:31 UTC (History)
5 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:
20.11.00, 20.05.05


Attachments
patch to give the Koha::Item returns a unique name (2.60 KB, patch)
2020-09-11 17:28 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 26438: Rename temporary Koha::Item variables to avoid conflict (2.67 KB, patch)
2020-09-23 11:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26438: Rename temporary Koha::Item variables to avoid conflict (2.71 KB, patch)
2020-09-23 21:03 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 Colin Campbell 2020-09-11 17:11:09 UTC
The added calls to Koha::Item use a temporary variable called $item to store their return this hides the existing variable $item which is the lineitem being processed. Calling lineitem methods on it causes an error
Comment 1 Colin Campbell 2020-09-11 17:28:31 UTC
Created attachment 109966 [details] [review]
patch to give the Koha::Item returns a unique name

This renames all the Koha::Item->new returns to $kitem so as not to hide the $item variable (one case had a more restricted scope but this will guard against it causing the problem if the logic changes)
Comment 2 Katrin Fischer 2020-09-20 22:55:46 UTC
Should this be a bug instead of an enh?
Comment 3 Colin Campbell 2020-09-23 08:12:33 UTC
(In reply to Katrin Fischer from comment #2)
> Should this be a bug instead of an enh?

Yes - think value reset in entry and I didnt check
Comment 4 Kyle M Hall 2020-09-23 11:29:32 UTC
Created attachment 110590 [details] [review]
Bug 26438: Rename temporary Koha::Item variables to avoid conflict

Returns from Koha::Item calls were stored in temporary variables
called $item. Unfortunately they occur in routines already with
an $item variable holding a lineitem and hide that variable although
only required for two lines of code. An error occurs if girfield
is called on the wrong variable. Renamed all three occurences
of $item to $kitem to avoid the error

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Katrin Fischer 2020-09-23 21:03:02 UTC
Created attachment 110631 [details] [review]
Bug 26438: Rename temporary Koha::Item variables to avoid conflict

Returns from Koha::Item calls were stored in temporary variables
called $item. Unfortunately they occur in routines already with
an $item variable holding a lineitem and hide that variable although
only required for two lines of code. An error occurs if girfield
is called on the wrong variable. Renamed all three occurences
of $item to $kitem to avoid the error

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2020-09-23 21:03:40 UTC
As this was a side effect from another bug and is a major issue, I think we should push as is, but it would be really great to have some unit tests for EDI!
Comment 7 Jonathan Druart 2020-09-28 09:10:52 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 8 Lucas Gass 2020-10-16 16:22:48 UTC
backported to 20.05.x for 20.05.05
Comment 9 Aleisha Amohia 2020-10-20 01:41:47 UTC
missing dependencies, not backported to 19.11.x