Bug 14627 - GetAuthorisedValueByCode does not handle undef intelligently
Summary: GetAuthorisedValueByCode does not handle undef intelligently
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Colin Campbell
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-31 13:48 UTC by Colin Campbell
Modified: 2017-06-14 22:10 UTC (History)
6 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:
Circulation function:

Attachments
Proposed patch (982 bytes, patch)
2015-07-31 14:14 UTC, Colin Campbell 		Details | Diff | Splinter Review
Bug 14627 Check for undef passed to GetAuthorisedValueByCode (980 bytes, patch)
2015-10-04 07:29 UTC, Mark Tompsett 		Details | Diff | Splinter Review
Bug 14627: Improve SQL query (1.17 KB, patch)
2015-10-04 07:29 UTC, Mark Tompsett 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14627: Check for undef passed to GetAuthorisedValueByCode (1.03 KB, patch)
2015-10-31 00:22 UTC, Bernardo Gonzalez Kriegel 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14627: Improve SQL query (1.28 KB, patch)
2015-10-31 00:22 UTC, Bernardo Gonzalez Kriegel 		Details | Diff | Splinter Review
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2015-07-31 13:48:01 UTC 
GetAuthorisedValueByCode is used in many places throughout Koha, many of the fields being looked up are nullable but if passed undef the routine proceeds to prepare and execute a meaningless query it should check the passed value and proceed with random behaviour
Comment 1 Colin Campbell 2015-07-31 14:14:02 UTC 
Trivial patch - improves code flow saving unnecessary processing
Comment 2 Colin Campbell 2015-07-31 14:14:54 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi (tcohen) 2015-07-31 14:31:17 UTC 
Colin, your patch makes sense. But what do u think of focusing on bug 10363? It has lagged a bit. Maybe another couple of eyes can make it move faster.
Comment 4 Mark Tompsett 2015-10-04 07:29:44 UTC Comment hidden (obsolete)
Comment 5 Mark Tompsett 2015-10-04 07:29:51 UTC Comment hidden (obsolete)
Comment 6 Mark Tompsett 2015-10-04 07:33:07 UTC 
Just wanted to put my work here, before I blew it away. :)
I figure bug 10363 as mentioned by Tomas will probably make this obsoleted and not required.
Comment 7 Bernardo Gonzalez Kriegel 2015-10-31 00:22:21 UTC 
Created attachment 44263 [details] [review]
[SIGNED-OFF] Bug 14627: Check for undef passed to GetAuthorisedValueByCode

Dont prepare or execute unnecessary sql if passed an undef value
routine is used in various places that dont check if the value
returned from a nullable field is defined or not
(e.g. streettype in the borrower record which many sites do not use)

Lets save those cpu cycles for something useful

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 8 Bernardo Gonzalez Kriegel 2015-10-31 00:22:28 UTC 
Created attachment 44264 [details] [review]
[SIGNED-OFF] Bug 14627: Improve SQL query

A field=NULL condition does not run properly. If category is
undefined, then don't include it in the query.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No regressions detected, no errors
Comment 9 Katrin Fischer 2015-11-04 00:25:45 UTC 
Can we please get unit tests and a test plan for this?
Comment 10 Jonathan Druart 2016-10-03 09:55:19 UTC 
This subroutine is going to be removed by bug 17252