Bug 11820 - dead code in basketgroup.pl : "add" operation is never invoked without a booksellerid
Summary: dead code in basketgroup.pl : "add" operation is never invoked without a book...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Galen Charlton
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-21 20:00 UTC by Galen Charlton
Modified: 2015-06-04 23:33 UTC (History)
2 users (show)

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


Attachments
Bug 11820: remove dead code in basket group management (5.95 KB, patch)
2014-02-21 20:11 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 11820: remove dead code in basket group management (6.01 KB, patch)
2014-02-24 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 11820: remove dead code in basket group management (6.13 KB, patch)
2014-03-12 11:19 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 Galen Charlton 2014-02-21 20:00:33 UTC
There is code in basketgroup.pl that handles the case where the "add" operation is called without a booksellerid being passed as well.  However, there is no sign that this actually happens.

Remove the extra conditional branch will simplify the script slightly.
Comment 1 Galen Charlton 2014-02-21 20:03:06 UTC
Indeed, upon review, it's /very/ unlikely that it's called that way: otherwise, folks would be seeing crashes like this:

Undefined subroutine &main::GetBookSeller called at /home/gmc/koha/acqui/basketgroup.pl line 258, <DATA> line 558.
Comment 2 Galen Charlton 2014-02-21 20:11:40 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2014-02-24 09:06:28 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2014-03-12 11:19:25 UTC
Created attachment 26159 [details] [review]
[PASSED QA] Bug 11820: remove dead code in basket group management

There was some code in acqui/basketgroup.pl that was apparently
intended to let one create a basket group for no (or an unknown)
vendor.  However, this code was never reached, as there is nothing
in the templates that invokes basketgroup.pl with 'add' as the
operation that doesn't also pass the vendor ID along.

This patch removes that dead code.

To test:

[1] Create a new basket group for a vendor and verify that it is
    created correctly.
[2] Edit an existing basket group, including moving baskets in and
    and out of, and verify that it works.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
No regressions found, passes all tests and QA script.
Comment 5 Galen Charlton 2014-03-12 14:57:39 UTC
Pushed to master.
Comment 6 Fridolin Somers 2014-06-04 09:35:33 UTC
Patch does not apply on 3.14.x