[Bug 9770] New: C4::ClassSortRoutine::Dewey can pad the wrong part of a call number internally
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 Bug ID: 9770 Summary: C4::ClassSortRoutine::Dewey can pad the wrong part of a call number internally Classification: Unclassified Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Cataloging Assignee: gmcharlt@gmail.com Reporter: jason@esilibrary.com CC: m.de.rooy@rijksmuseum.nl The subroutine get_class_sort_key tokenizes a call number string (splitting on periods and whitespace) and counts the number of tokens that solely contain digits. If there is only one such digit group, a comment in the code states that it will pad said digit group. However, the bug is that the code assumes said digit group is the first token, when this may not be the case. Patch to follow... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 --- Comment #1 from Jason Etheridge <jason@esilibrary.com> --- Created attachment 15977 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=15977&action=edit patch to Dewey.pm I may need to try git send-email again; don't think I had mail configured on my dev server. I'm also not versed in Koha's unit testing, so haven't tried to add a test yet. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #2 from Chris Cormack <chris@bigballofwax.co.nz> --- Hi Jason t/ClassSortRoutine_Dewey.t is what you want to take a look at. The patch looks good, so if you just added a test to that file that will be excellent -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 --- Comment #3 from Jason Etheridge <jason@esilibrary.com> --- Created attachment 19195 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19195&action=edit patch for ClassSortRoutine_Dewey.t Here's the test to go with the first patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 --- Comment #4 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 19383 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19383&action=edit Bug 9770: fix sorting of Dewey call numbers that contain prefixes C4::ClassSortRoutine::Dewey can pad the wrong part of a call number internally. The subroutine get_class_sort_key tokenizes a call number string (splitting on periods and whitespace) and counts the number of tokens that solely contain digits. If there is only one such digit group, a comment in the code states that it will pad said digit group. However, the bug is that the code assumes said digit group is the first token, when this may not be the case. In practice, this can cause poor sorting when used a call number is in the form of PREFIX _space_ 3DIGITS. To test: [1] Create two item records whose class scheme is set to 'ddc' (Dewey) and whose call numbers contain prefixes, e.g., J DVD 700.1 ABC and J DVD 850 DEF. [2] Use the inventory tool to produce a list of item items that include the two created in step 1. Obsere that that items are sorted in the incorrect order, with "J DVD 850 DEF" coming before "J DVD 700.1 ABC". Alternatively, run the following SQL to see the incorrect sort order: SELECT cn_sort, itemcallnumber FROM items WHERE itemcallnumber LIKE 'J DVD%' ORDER BY cn_sort; [4] Apply this patch. [5] Run misc/maintenance/touch_all_items.pl to force cn_sort to be recalculated. [6] Repeat step 2 and verify that the call numbers are now sorted corrected. Signed-off-by: Jason Etheridge <jason@esilibrary.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #15977|0 |1 is obsolete| | Attachment #19195|0 |1 is obsolete| | Attachment #19383|0 |1 is obsolete| | --- Comment #5 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 19384 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19384&action=edit Bug 9770: test case for sorting of Dewey call numbers that contain prefixes This adds a test for C4::ClassSortRoutine::Dewey to check that the call number "JR DVD 800.1" sorts before "JR DVD 900" To test: [1] Apply just this patch. [1] Run prove -v t/ClassSortRoutine_Dewey.t [2] Test #7 should fail. Signed-off-by: Jason Etheridge <jason@esilibrary.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 --- Comment #6 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 19385 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19385&action=edit Bug 9770: fix sorting of Dewey call numbers that contain prefixes C4::ClassSortRoutine::Dewey can pad the wrong part of a call number internally. The subroutine get_class_sort_key tokenizes a call number string (splitting on periods and whitespace) and counts the number of tokens that solely contain digits. If there is only one such digit group, a comment in the code states that it will pad said digit group. However, the bug is that the code assumes said digit group is the first token, when this may not be the case. In practice, this can cause poor sorting when used a call number is in the form of PREFIX _space_ 3DIGITS. To test: [1] Create two item records whose class scheme is set to 'ddc' (Dewey) and whose call numbers contain prefixes, e.g., J DVD 700.1 ABC and J DVD 850 DEF. [2] Use the inventory tool to produce a list of item items that include the two created in step 1. Obsere that that items are sorted in the incorrect order, with "J DVD 850 DEF" coming before "J DVD 700.1 ABC". Alternatively, run the following SQL to see the incorrect sort order: SELECT cn_sort, itemcallnumber FROM items WHERE itemcallnumber LIKE 'J DVD%' ORDER BY cn_sort; [4] Apply this patch. [5] Run misc/maintenance/touch_all_items.pl to force cn_sort to be recalculated. [6] Repeat step 2 and verify that the call numbers are now sorted corrected. Signed-off-by: Jason Etheridge <jason@esilibrary.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 --- Comment #7 from Galen Charlton <gmcharlt@gmail.com> --- I've tested the patches and attached my signoff. I've modified the commit messages to include test plans. Setting this bug to Needs Signoff (Jason, the next step is for a non-ESI person to review the patch). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 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=9770 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19384|0 |1 is obsolete| | --- Comment #8 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 19386 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19386&action=edit Bug 9770: test case for sorting of Dewey call numbers that contain prefixes This adds a test for C4::ClassSortRoutine::Dewey to check that the call number "JR DVD 800.1" sorts before "JR DVD 900" To test: [1] Apply just this patch. [1] Run prove -v t/ClassSortRoutine_Dewey.t [2] Test #7 should fail. Signed-off-by: Jason Etheridge <jason@esilibrary.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19385|0 |1 is obsolete| | --- Comment #9 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 19387 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19387&action=edit Bug 9770: fix sorting of Dewey call numbers that contain prefixes C4::ClassSortRoutine::Dewey can pad the wrong part of a call number internally. The subroutine get_class_sort_key tokenizes a call number string (splitting on periods and whitespace) and counts the number of tokens that solely contain digits. If there is only one such digit group, a comment in the code states that it will pad said digit group. However, the bug is that the code assumes said digit group is the first token, when this may not be the case. In practice, this can cause poor sorting when used a call number is in the form of PREFIX _space_ 3DIGITS. To test: [1] Create two item records whose class scheme is set to 'ddc' (Dewey) and whose call numbers contain prefixes, e.g., J DVD 700.1 ABC and J DVD 850 DEF. [2] Use the inventory tool to produce a list of item items that include the two created in step 1. Obsere that that items are sorted in the incorrect order, with "J DVD 850 DEF" coming before "J DVD 700.1 ABC". Alternatively, run the following SQL to see the incorrect sort order: SELECT cn_sort, itemcallnumber FROM items WHERE itemcallnumber LIKE 'J DVD%' ORDER BY cn_sort; [4] Apply this patch. [5] Run misc/maintenance/touch_all_items.pl to force cn_sort to be recalculated. [6] Repeat step 2 and verify that the call numbers are now sorted corrected. Signed-off-by: Jason Etheridge <jason@esilibrary.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 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=9770 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19386|0 |1 is obsolete| | Attachment #19387|0 |1 is obsolete| | --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 19629 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19629&action=edit [PASSED QA] Bug 9770: test case for sorting of Dewey call numbers that contain prefixes This adds a test for C4::ClassSortRoutine::Dewey to check that the call number "JR DVD 800.1" sorts before "JR DVD 900" To test: [1] Apply just this patch. [1] Run prove -v t/ClassSortRoutine_Dewey.t [2] Test #7 should fail. Signed-off-by: Jason Etheridge <jason@esilibrary.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes test plan and QA script. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 19630 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19630&action=edit [PASSED QA] Bug 9770: fix sorting of Dewey call numbers that contain prefixes C4::ClassSortRoutine::Dewey can pad the wrong part of a call number internally. The subroutine get_class_sort_key tokenizes a call number string (splitting on periods and whitespace) and counts the number of tokens that solely contain digits. If there is only one such digit group, a comment in the code states that it will pad said digit group. However, the bug is that the code assumes said digit group is the first token, when this may not be the case. In practice, this can cause poor sorting when used a call number is in the form of PREFIX _space_ 3DIGITS. To test: [1] Create two item records whose class scheme is set to 'ddc' (Dewey) and whose call numbers contain prefixes, e.g., J DVD 700.1 ABC and J DVD 850 DEF. [2] Use the inventory tool to produce a list of item items that include the two created in step 1. Obsere that that items are sorted in the incorrect order, with "J DVD 850 DEF" coming before "J DVD 700.1 ABC". Alternatively, run the following SQL to see the incorrect sort order: SELECT cn_sort, itemcallnumber FROM items WHERE itemcallnumber LIKE 'J DVD%' ORDER BY cn_sort; [4] Apply this patch. [5] Run misc/maintenance/touch_all_items.pl to force cn_sort to be recalculated. [6] Repeat step 2 and verify that the call numbers are now sorted corrected. Signed-off-by: Jason Etheridge <jason@esilibrary.com> Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #12 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Jason! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |tomascohen@gmail.com Version|unspecified |master --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This patch has been pushed to 3.12.x, will be in 3.12.3. Thanks Jason! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bgkriegel@gmail.com --- Comment #14 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Pushed to 3.10.x, will be in 3.10.10 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9770 Chris Hall <chrish@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chrish@catalyst.net.nz --- Comment #15 from Chris Hall <chrish@catalyst.net.nz> --- Pushed to 3.8.x, will be in 3.8.17 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org