Bug 26438

Summary: Follow up to bug 23463 - return from Koha::Item overwrites existing variable
Product: Koha Reporter: Colin Campbell <colin.campbell>
Component: AcquisitionsAssignee: Colin Campbell <colin.campbell>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P3 CC: aleisha, jonathan.druart, kyle, lucas, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
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
Bug Depends on: 23463, 26082    
Bug Blocks:    
Attachments: patch to give the Koha::Item returns a unique name
Bug 26438: Rename temporary Koha::Item variables to avoid conflict
Bug 26438: Rename temporary Koha::Item variables to avoid conflict

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