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 (vote)
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)
7 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:


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

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 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