[Bug 11338] New: wrong SQL in DelItemCheck
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Bug ID: 11338 Summary: wrong SQL in DelItemCheck Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Cataloging Assignee: gmcharlt@gmail.com Reporter: fridolyn.somers@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: m.de.rooy@rijksmuseum.nl In C4::Items::DelItemCheck, there are two SQL queries : one to check if item is on loan, the other if item is reserved. Those two queries use "SELECT * FROM table", fetch the datas with "$var = $sth->fetchrow", and use "$var" as a boolean condition. This is not correct, SQL query should be "SELECT COUNT(*) FROM table". Code actually works, but is dangerous. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 --- Comment #1 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 23303 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23303&action=edit Bug 11338 - wrong SQL in DelItemCheck In C4::Items::DelItemCheck, there are two SQL queries : one to check if item is on loan, the other if item is reserved. Those two queries use "SELECT * FROM table", fetch the datas with "$var = $sth->fetchrow", and use "$var" as a boolean condition. This is not correct, SQL query should be "SELECT COUNT(*) FROM table". Code actually works, but is dangerous. This patch corrects the SQL queries and sets my ($var) to show that fetchrow returns an array. Test plan : - Set an item A onloan - Set an item B reserved and the reserve waiting - Go to items cataloguing : cgi-bin/koha/cataloguing/additem.pl?biblionumber=XXX - Try to delete item A => You get an alert and item is not deleted - Try to delete item B => You get an alert and item is not deleted -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #2 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- (first time I use git bz, whouuu) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |fridolyn.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23303|0 |1 is obsolete| | --- Comment #3 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 23761 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23761&action=edit Bug 11338 - wrong SQL in DelItemCheck In C4::Items::DelItemCheck, there are two SQL queries : one to check if item is on loan, the other if item is reserved. Those two queries use "SELECT * FROM table", fetch the datas with "$var = $sth->fetchrow", and use "$var" as a boolean condition. This is not correct, SQL query should be "SELECT COUNT(*) FROM table". Code actually works, but is dangerous. This patch corrects the SQL queries and sets my ($var) to show that fetchrow returns an array. Test plan : - Set an item A onloan - Set an item B reserved and the reserve waiting - Go to items cataloguing : cgi-bin/koha/cataloguing/additem.pl?biblionumber=XXX - Try to delete item A => You get an alert and item is not deleted - Try to delete item B => You get an alert and item is not deleted Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Works, and has the added bonus of being a tiny bit faster. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23761|0 |1 is obsolete| | --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 23792 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23792&action=edit [PASSED QA] Bug 11338 - wrong SQL in DelItemCheck In C4::Items::DelItemCheck, there are two SQL queries : one to check if item is on loan, the other if item is reserved. Those two queries use "SELECT * FROM table", fetch the datas with "$var = $sth->fetchrow", and use "$var" as a boolean condition. This is not correct, SQL query should be "SELECT COUNT(*) FROM table". Code actually works, but is dangerous. This patch corrects the SQL queries and sets my ($var) to show that fetchrow returns an array. Test plan : - Set an item A onloan - Set an item B reserved and the reserve waiting - Go to items cataloguing : cgi-bin/koha/cataloguing/additem.pl?biblionumber=XXX - Try to delete item A => You get an alert and item is not deleted - Try to delete item B => You get an alert and item is not deleted Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Works, and has the added bonus of being a tiny bit faster. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes t, xt and QA script tests. Also tried deleting via batch delete - correct warnings are displayed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gmcharlt@gmail.com Summary|wrong SQL in DelItemCheck |items that are captured for | |holds can be deleted | |without warning -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #5 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master, along with unit tests for DelItemCheck(). Thanks, Fridolin! Note that I've amended the bug title and the patch description significantly to reflect the fact that this patch corrects a nasty, user-visible bug. Suffice it to say that DelItemCheck() was *not* working properly prior to this patch, as it permitted deleting items that were captured for hold requests. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11338 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #6 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Patches pushed to 3.14.x, will be in 3.14.2. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org