Bug 22611

Summary: Typo introduced into Koha::EDI by bug 15685
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: AcquisitionsAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: critical    
Priority: P5 - low CC: colin.campbell, fridolin.somers, lucas, nick, wizzyrea
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:
19.05.00, 18.11.05
Bug Depends on: 15685    
Bug Blocks:    
Attachments: Bug 22611: Correction for typo in Koha::EDI
patch to make test consistent with initial copy
Bug 19380: Use Koha::Item::get_transfer instead of GetTransfers
Bug 22611: Correction for call to get create_item in EDI.pm
Bug 22611: Correction for call to get create_item in EDI.pm

Description Martin Renvoize 2019-03-29 17:09:20 UTC
Koha/EDI.pm - $basket->effective_create_item (Missing the 's'), which will cause Koha::Objects to throw an 'The method is untested' error whenever his codepath is followed.
Comment 1 Martin Renvoize 2019-03-29 17:13:32 UTC
Created attachment 87225 [details] [review]
Bug 22611: Correction for typo in Koha::EDI

A typo in Koha::EDI leads to 'The method is not tested' errors to be
thrown when the codepath is taken and can lead to subsequent data
corruption.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 2 Colin Campbell 2019-04-01 11:36:53 UTC
Created attachment 87271 [details] [review]
patch to make test consistent with initial copy

In addition to the typo, the patch used a different call to for subsequent copies to the initial copy. That could create confusion in the future
Comment 3 Liz Rea 2019-04-02 15:47:35 UTC
Created attachment 87326 [details] [review]
Bug 19380: Use Koha::Item::get_transfer instead of GetTransfers

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 4 Liz Rea 2019-04-02 15:47:38 UTC
Created attachment 87327 [details] [review]
Bug 22611: Correction for call to get create_item in EDI.pm

There was a typo in the addition of a call to
effective_create_items causing processing to abort after adding
a single copy.

This patch reverts to using the C4::Context call as used for the
first copy created, the intended call to effective_create_items
is a wrapper around that as the field in the basket is guaranteed
to be null (its not set when we call AddBasket). Being consistent
and explicit in what we call should guard against future changes
in dependencies exposing the logic flaw

Added some comments to make logical flow of code clearer

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 5 Liz Rea 2019-04-02 16:08:26 UTC
Comment on attachment 87225 [details] [review]
Bug 22611: Correction for typo in Koha::EDI

Sorry I'm making a lot of noise >.<
Comment 6 Katrin Fischer 2019-04-02 20:08:42 UTC
Created attachment 87333 [details] [review]
Bug 22611: Correction for call to get create_item in EDI.pm

There was a typo in the addition of a call to
effective_create_items causing processing to abort after adding
a single copy.

This patch reverts to using the C4::Context call as used for the
first copy created, the intended call to effective_create_items
is a wrapper around that as the field in the basket is guaranteed
to be null (its not set when we call AddBasket). Being consistent
and explicit in what we call should guard against future changes
in dependencies exposing the logic flaw

Added some comments to make logical flow of code clearer

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Nick Clemens (kidclamp) 2019-04-05 13:12:31 UTC
Awesome work all!

Pushed to master for 19.05
Comment 8 Martin Renvoize 2019-04-15 10:31:09 UTC
Pushed to 18.11.x for 18.11.05
Comment 9 Lucas Gass 2019-04-15 16:44:17 UTC
backported to 18.05.x for 18.05.12
Comment 10 Fridolin Somers 2019-04-30 06:10:03 UTC
Looks like its not needed for 17.11.x