[Bug 15877] New: C4::Barcodes does not correctly calculate db_max for 'annual' barcodes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Bug ID: 15877 Summary: C4::Barcodes does not correctly calculate db_max for 'annual' barcodes Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org The SQL query to find the database max value for annual barcodes is: 42 my $query = "SELECT max(substring_index(barcode,'-',-1)) AS chunk,barcode FROM items WHERE barcode LIKE ? GROUP BY barcode"; 43 # FIXME: unreasonably expensive query on large datasets The 'max' in the first column creates confusion and disrupts the normal pairing of the db. To recreate: 1 - Add two 'annual' type barcodes to the system (2016-0003, 2016-0006) 2 - Run the query: SELECT max(substring_index(barcode,'-',-1)) AS chunk,barcode FROM items WHERE barcode LIKE '2016-%' GROUP BY barcode 3 - Note that returned values may not pair up, in my case I got things like: chunk | barcode 0006 | 2016-0003 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 48363 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48363&action=edit Bug 15877 - C4::Barcodes does not correctly calculate db_max for 'annual' barcodes This patch alters the MySQL query to remove a max statement which incorrectly groups full annual barcodes with numeric values. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 48365 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48365&action=edit Bug 15877 - Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Test plan: 1 - prove t/db_dependent/Barcodes.t 2 - All should pass 3 - Apply second patch (unit tests update) 4 - Test 3 should fail (annual) db_max equals greatest value when more than 1 present 5 - Apply first patch 6 - All tests should now pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |nick@bywatersolutions.com CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |14752 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14752 [Bug 14752] Add multiple copies to a basket at once -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48365|0 |1 is obsolete| | --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 48367 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48367&action=edit Bug 15877 - Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48367|0 |1 is obsolete| | --- Comment #5 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 49197 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49197&action=edit [SIGNED-OFF] Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48363|0 |1 is obsolete| | --- Comment #6 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 49198 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49198&action=edit [SIGNED-OFF] Bug 15877 - C4::Barcodes does not correctly calculate db_max for 'annual' barcodes This patch alters the MySQL query to remove a max statement which incorrectly groups full annual barcodes with numeric values. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 49197 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49197 [SIGNED-OFF] Bug 15877 - Unit tests Review of attachment 49197: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=15877&attachment=49197) ----------------------------------------------------------------- A warning is raised by C4::Barcodes, use warning_is to catch it. ::: t/db_dependent/Barcodes.t @@ +53,5 @@
+ barcode => $barcodevalue + } +}); + +ok($barcodevalue eq $barcodeobj->db_max(), "(annual) First barcode saved to db is equal to db_max" );
use is instead of ok. @@ +59,5 @@
+$barcodevalue = $barcodeobj->next_value(); +$barcodevalue = $barcodeobj->next_value($barcodevalue); +$barcodevalue = $barcodeobj->next_value($barcodevalue); +$barcodevalue = $barcodeobj->next_value($barcodevalue); +$barcodevalue = $barcodeobj->next_value($barcodevalue);
Why are you calling 4x this method? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #49197|0 |1 is obsolete| | --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 49600 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49600&action=edit Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #7)
Review of attachment 49197 [details] [review]: use is instead of ok. ok (or should I say is ;-) )
@@ +59,5 @@
+$barcodevalue = $barcodeobj->next_value(); +$barcodevalue = $barcodeobj->next_value($barcodevalue); +$barcodevalue = $barcodeobj->next_value($barcodevalue); +$barcodevalue = $barcodeobj->next_value($barcodevalue); +$barcodevalue = $barcodeobj->next_value($barcodevalue); Why are you calling 4x this method?
To set the value higher without using any constants. It's arbitrary to use 4, I just knew that triggered the problem for me each time and it felt more illustrative than 1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #49600|0 |1 is obsolete| | --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 49926 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49926&action=edit Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #49926|0 |1 is obsolete| | --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 49927 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49927&action=edit Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #49927|0 |1 is obsolete| | --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 49928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49928&action=edit Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- Edited the: $dbh->do(q|DELETE FROM items|); And replaced with: $builder->clear( { source => 'Item' } ); Same for issues, a test crash wiped my database of all items so not an ideal construction -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #49198|0 |1 is obsolete| | Attachment #49928|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 49961 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49961&action=edit Bug 15877 - C4::Barcodes does not correctly calculate db_max for 'annual' barcodes This patch alters the MySQL query to remove a max statement which incorrectly groups full annual barcodes with numeric values. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 49962 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49962&action=edit Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |brendan@bywatersolutions.co | |m --- Comment #16 from Brendan Gallagher <brendan@bywatersolutions.com> --- Pushed to Master - Should be in the May 2016 release. Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |16561 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16561 [Bug 16561] Regression caused by 15877 - t/db_dependent/Barcodes.t deletes all items from a DB -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15877 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com Severity|enhancement |normal Status|Pushed to Master |Pushed to Stable --- Comment #17 from Julian Maurice <julian.maurice@biblibre.com> --- Patches pushed to 3.22.x, will be in 3.22.7 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org