Bug 22611 - Typo introduced into Koha::EDI by bug 15685
Summary: Typo introduced into Koha::EDI by bug 15685
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 15685
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-29 17:09 UTC by Martin Renvoize
Modified: 2019-04-30 06:10 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.05.00, 18.11.05


Attachments
Bug 22611: Correction for typo in Koha::EDI (995 bytes, patch)
2019-03-29 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
patch to make test consistent with initial copy (1.86 KB, patch)
2019-04-01 11:36 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 19380: Use Koha::Item::get_transfer instead of GetTransfers (4.77 KB, patch)
2019-04-02 15:47 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22611: Correction for call to get create_item in EDI.pm (1.91 KB, patch)
2019-04-02 15:47 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22611: Correction for call to get create_item in EDI.pm (1.96 KB, patch)
2019-04-02 20:08 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 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 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