When doing an inventory a range of GT90 to GT100 for LC callnumbers the results won't include GT95 We should convert the numbers and use the cn_sort column for fetching items
Created attachment 70274 [details] [review] Bug 19915 - Unit test prove -v t/db_dependent/Items/GetItemsForInventory.t
Created attachment 70275 [details] [review] Bug 19915 - Use cn_sort values for getting inventory items To test: 1 - Catalog an item under LCC with callnumber GT95 2 - cn_sort should calculate as GT0095 3 - Go to inventory tool 4 - Enter a range that should have your item e.g LC GT90 to GT100 5 - Item is not returned 6 - Apply patch 7 - Item should be returned 8 - Verify things otherwise work as expected
*** Bug 14983 has been marked as a duplicate of this bug. ***
+my $schema = Koha::Database->new()->schema(); +my @class_sources = $schema->resultset('ClassSource')->search(); Koha::Object ?
Created attachment 70338 [details] [review] Bug 19915 - Unit test prove -v t/db_dependent/Items/GetItemsForInventory.t
Created attachment 70339 [details] [review] Bug 19915 - Use cn_sort values for getting inventory items To test: 1 - Catalog an item under LCC with callnumber GT95 2 - cn_sort should calculate as GT0095 3 - Go to inventory tool 4 - Enter a range that should have your item e.g LC GT90 to GT100 5 - Item is not returned 6 - Apply patch 7 - Item should be returned 8 - Verify things otherwise work as expected
After applying the patch, at http://pro.user10-koha.sandbox.biblibre.eu/cgi-bin/koha/tools/inventory.pl I get the following error message : Software error: Can't locate Koha/ClassificationSources.pm in @INC (you may need to install the Koha::ClassificationSources module) (@INC contains: /home/koha/src /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.22.1 /usr/local/share/perl/5.22.1 /usr/lib/x86_64-linux-gnu/perl5/5.22 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.22 /usr/share/perl/5.22 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base .) at /home/koha/src/tools/inventory.pl line 42. BEGIN failed--compilation aborted at /home/koha/src/tools/inventory.pl line 42. For help, please send mail to the webmaster ([no address given]), giving this error message and the time and date of the error.
Created attachment 72950 [details] [review] Bug 19915 - Unit test prove -v t/db_dependent/Items/GetItemsForInventory.t
Created attachment 72951 [details] [review] Bug 19915 - Use cn_sort values for getting inventory items To test: 1 - Catalog an item under LCC with callnumber GT95 2 - cn_sort should calculate as GT0095 3 - Go to inventory tool 4 - Enter a range that should have your item e.g LC GT90 to GT100 5 - Item is not returned 6 - Apply patch 7 - Item should be returned 8 - Verify things otherwise work as expected
Did you make sure to apply the dependencies
Tested with the dependencies : doesn't work better... Anne-Claire
The test plan here uses numbers valid for LC callnumbers. I think you will need to find a valid callnumber range if using a different classification source
Works fine when having the right classification scheme defined in the syspref DefaultClassificationSource :)
Patch tested with a sandbox, by Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr>
Created attachment 72988 [details] [review] Bug 19915 - Unit test prove -v t/db_dependent/Items/GetItemsForInventory.t Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr>
Created attachment 72989 [details] [review] Bug 19915 - Use cn_sort values for getting inventory items To test: 1 - Catalog an item under LCC with callnumber GT95 2 - cn_sort should calculate as GT0095 3 - Go to inventory tool 4 - Enter a range that should have your item e.g LC GT90 to GT100 5 - Item is not returned 6 - Apply patch 7 - Item should be returned 8 - Verify things otherwise work as expected Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr>
Hi Nick, I am a little confused about the need for the new classification source pull down on the inventory form. Some questions: 1) Why is it needed? 2) Should we not only show the classification sources marked as 'in use'? 3) Why can't I unset it? 4) Is it supposed to limit the search? Because when I select LOC classification, items are found, although they don't have a classification source set.
(In reply to Katrin Fischer from comment #17) > Hi Nick, > > I am a little confused about the need for the new classification source pull > down on the inventory form. Some questions: > > 1) Why is it needed? We need to know which source to use for converting the callnumbers entered, currently we test against unconverted callnumbers which doesn't work for manyschemes, the reanges won't be returned correctly > 2) Should we not only show the classification sources marked as 'in use'? Good idea, I will followup > 3) Why can't I unset it? Even when cn_source is not set for an item it uses the default, unsetting it shouldn't be necessary > 4) Is it supposed to limit the search? Because when I select LOC > classification, items are found, although they don't have a classification > source set. It doesn't limit the search per se, it just converts the range to the correct values for cn_sort
Created attachment 76120 [details] [review] Bug 19915: Only show class sources in use
I would suggest to add release notes. If people did not really care about such things as classification schemes, they might be up for a surprise when their data is not consistent in that area.
Created attachment 76721 [details] [review] Bug 19915: Add unit test to GetItemsForInventory.t prove -v t/db_dependent/Items/GetItemsForInventory.t Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Moved the betwen typo edit back here.
Created attachment 76722 [details] [review] Bug 19915: (QA follow-up) Tidy up GetItemsForInventory.t Standard edits like modules, schema, Koha objects instead of DBIC. Removing unneeded autoflush. Moving initial test to a first subtest. No need to clear issues if you clear items. Adding a FIXME for OldWay; would be nice to remove that code.. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 76723 [details] [review] Bug 19915: Use cn_sort values for getting inventory items To test: 1 - Catalog an item under LCC with callnumber GT95 2 - cn_sort should calculate as GT0095 3 - Go to inventory tool 4 - Enter a range that should have your item e.g LC GT90 to GT100 5 - Item is not returned 6 - Apply patch 7 - Item should be returned 8 - Verify things otherwise work as expected Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 76724 [details] [review] Bug 19915: Only show class sources in use Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
No blocker for me, but adding an option to control the switch from callnumber to cn_sort might be handy for some people?
(In reply to Marcel de Rooy from comment #25) > No blocker for me, but adding an option to control the switch from > callnumber to cn_sort might be handy for some people? What is the use case for sorting by callnumber? I cannot think of a reason one would want to sort by a non-normalized call number. Especially for inventory purposes, having a sort that supposedly matches your shelves is the only useful one, unless I'm missing something.
Koha libraries might have migrated without filling the classification source subfield which would result in an empty cn_sort. Also, as we only have a limited selection of filing rules available, maybe there are use cases where a simple sort works better? Not sure about that, but it feels like almost every library here has their own homegrown classification scheme.
I talked with enough people to agree this must be optional - will provide a followup
I am so confused by this bug. Isn't the inventory already sorting by call number?
Yes, it's sorted by call number but that does not sort correctly. It should be sorted by items.cn_sort which will sort correctly.
(In reply to Carl from comment #30) > Yes, it's sorted by call number but that does not sort correctly. It should > be sorted by items.cn_sort which will sort correctly. I am certain that I just looked at this a couple months ago and it was using the normalized call numbers. I had an issue where the normalized call numbers were wrong for old records, but after re-generating the values, all was well. Let me see if I can find any details on that issue I had...
Here we go... https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21447
(In reply to David Cook from comment #32) > Here we go... > > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21447 I wish that I had written that bug report better. In hindsight, I think maybe #19915 fixes the underlying problem that I observed, which means that both the list view and export views should be fixed after these changes...
Created attachment 81816 [details] [review] Bug 19915: Add unit test to GetItemsForInventory.t prove -v t/db_dependent/Items/GetItemsForInventory.t Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Moved the betwen typo edit back here. Current patch-complexity: Small patch
Created attachment 81817 [details] [review] Bug 19915: (QA follow-up) Tidy up GetItemsForInventory.t Standard edits like modules, schema, Koha objects instead of DBIC. Removing unneeded autoflush. Moving initial test to a first subtest. No need to clear issues if you clear items. Adding a FIXME for OldWay; would be nice to remove that code.. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 81818 [details] [review] Bug 19915: Use cn_sort values for getting inventory items To test: 1 - Catalog an item under LCC with callnumber GT95 2 - cn_sort should calculate as GT0095 3 - Go to inventory tool 4 - Enter a range that should have your item e.g LC GT90 to GT100 5 - Item is not returned 6 - Apply patch 7 - Item should be returned 8 - Verify things otherwise work as expected Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 81819 [details] [review] Bug 19915: Only show class sources in use Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 81820 [details] [review] Bug 19915: Don't delete in tests
Created attachment 81821 [details] [review] Bug 19915: Make behaviour syspref controlled To test: 0 - Apply patch, run updatedatabase 1 - Catalog an item under LCC classification source with callnumber GT95 2 - Check db, cn_sort should calculate as GT0095 SELECT cn_sort FROM items where callnumber="GT95"; 3 - Catalog an item under LCC classification source with callnumber GT101 4 - Check db, cn_sort should calculate as GT0101 SELECT cn_sort FROM items where callnumber="GT101"; 5 - Go to inventory tool 6 - Enter a range from GT90 to GT100 7 - Neither item is not returned 8 - Enter range from GT100 to GT90 9 - Second item is returned 10 - Set UseCNSortForInventory to 'Use' 11 - Go to invenotry tool 12 - Enter a range from GT90 to GT100 13 - The first item is returned, the second is not 14 - Enter a range from GT100 to GT90 15 - Neither item is returned 16 - prove -v t/db_dependent/Items/GetItemsForInventory.t
In bug 21629 we corrected a similar case where we changed from callnumber to cn_sort sorting, but we did not require a system preference to enable/disable that change. I personally don't feel we should require the system preference here either.. or at least we should be consistent elsewhere if we are going to introduce such a preference... Can we push this back to the mailing list for comment?
(In reply to Martin Renvoize from comment #40) > In bug 21629 we corrected a similar case where we changed from callnumber to > cn_sort sorting, but we did not require a system preference to > enable/disable that change. > > I personally don't feel we should require the system preference here > either.. or at least we should be consistent elsewhere if we are going to > introduce such a preference... > > Can we push this back to the mailing list for comment? I think the concern was some migrations where the cn_sort may not be populated or not populated correctly. I am fine either way (would prefer without the switch, it would highlight data that should be fixed) I will email the lists.
After giving this some more thought I think we should make using cn_sort the default behaviour without a configuration option. Just a thought: would it be hard to use itemcallnumber if cn_sort is empty/not set? That might give some flexibility. It might be nice to add a script to help people populating this field if it was somehow missed during migration or set to a wrong value, but that is out of scope here for sure!
Created attachment 83644 [details] [review] Bug 19915: Add unit test to GetItemsForInventory.t prove -v t/db_dependent/Items/GetItemsForInventory.t Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Moved the betwen typo edit back here. https://bugs.koha-community.org/show_bug.cgi?id=19905
Created attachment 83645 [details] [review] Bug 19915: (QA follow-up) Tidy up GetItemsForInventory.t Standard edits like modules, schema, Koha objects instead of DBIC. Removing unneeded autoflush. Moving initial test to a first subtest. No need to clear issues if you clear items. Adding a FIXME for OldWay; would be nice to remove that code.. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> https://bugs.koha-community.org/show_bug.cgi?id=19905
Created attachment 83646 [details] [review] Bug 19915: Use cn_sort values for getting inventory items To test: 1 - Catalog an item under LCC with callnumber GT95 2 - cn_sort should calculate as GT0095 3 - Go to inventory tool 4 - Enter a range that should have your item e.g LC GT90 to GT100 5 - Item is not returned 6 - Apply patch 7 - Item should be returned 8 - Verify things otherwise work as expected Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> https://bugs.koha-community.org/show_bug.cgi?id=19905
Created attachment 83647 [details] [review] Bug 19915: Only show class sources in use Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> https://bugs.koha-community.org/show_bug.cgi?id=19905
Created attachment 83648 [details] [review] Bug 19915: Don't delete in tests https://bugs.koha-community.org/show_bug.cgi?id=19905
Created attachment 83649 [details] [review] Bug 19915: (follow-up) Fix test count from rebase https://bugs.koha-community.org/show_bug.cgi?id=19905
(In reply to Katrin Fischer from comment #42) > Just a thought: would it be hard to use itemcallnumber if cn_sort is > empty/not set? That might give some flexibility. We could, but I am afraid ti wouldn't help since the range passed into the template is transformed using the classification rules - so an untransformed callnumebr would likely remain outside the range > It might be nice to add a script to help people populating this field if it > was somehow missed during migration or set to a wrong value, but that is out > of scope here for sure! Thank you, file a bug and we can look at a future time :-)
> > It might be nice to add a script to help people populating this field if it > > was somehow missed during migration or set to a wrong value, but that is out > > of scope here for sure! > > Thank you, file a bug and we can look at a future time :-) Done: bug 22263
Created attachment 84665 [details] [review] Bug 19915: Add unit test to GetItemsForInventory.t prove -v t/db_dependent/Items/GetItemsForInventory.t Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Moved the betwen typo edit back here. https://bugs.koha-community.org/show_bug.cgi?id=19905 Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 84666 [details] [review] Bug 19915: (QA follow-up) Tidy up GetItemsForInventory.t Standard edits like modules, schema, Koha objects instead of DBIC. Removing unneeded autoflush. Moving initial test to a first subtest. No need to clear issues if you clear items. Adding a FIXME for OldWay; would be nice to remove that code.. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> https://bugs.koha-community.org/show_bug.cgi?id=19905 Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 84667 [details] [review] Bug 19915: Use cn_sort values for getting inventory items To test: 1 - Catalog an item under LCC with callnumber GT95 2 - cn_sort should calculate as GT0095 3 - Go to inventory tool 4 - Enter a range that should have your item e.g LC GT90 to GT100 5 - Item is not returned 6 - Apply patch 7 - Item should be returned 8 - Verify things otherwise work as expected Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> https://bugs.koha-community.org/show_bug.cgi?id=19905 Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 84668 [details] [review] Bug 19915: Only show class sources in use Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> https://bugs.koha-community.org/show_bug.cgi?id=19905 Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 84669 [details] [review] Bug 19915: Don't delete in tests https://bugs.koha-community.org/show_bug.cgi?id=19905 Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 84670 [details] [review] Bug 19915: (follow-up) Fix test count from rebase https://bugs.koha-community.org/show_bug.cgi?id=19905 Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.03
cant cleanly apply this patchset to 18.05.x. won't backport
Perhaps this is a problem with cn_sort itself or something else but I've noticed that our inventory _still_ doesn't sort correctly. Here is an example for one of our locations with call numbers B0 - B30: - B 29 V55 1998 - B29 .C55 2008 - B29 .L24 1983 - B29 .L264 1998 - B29 .P666 2004 I've had a JS hack in place, originally for addressing this bug, but I've never been able to remove it because of problems like the above. This is the correct sort after the hack is applied: - B29 .C55 2008 - B29 .L24 1983 - B29 .L264 1998 - B29 .P666 2004 - B 29 V55 1998 It's also what I see if I run a query like SELECT itemnumber, cn_sort, itemcallnumber FROM items WHERE homebranch = 'OAK' AND items.itemcallnumber LIKE 'B29%' OR items.itemcallnumber LIKE 'B%29%' OR items.itemcallnumber LIKE ' B29%' OR items.itemcallnumber LIKE ' B%29%' ORDER BY cn_sort itemnumber cn_sort itemcallnumber 72613 B0029 C55 02008 B29 .C55 2008 59337 B0029 L24 01983 B29 .L24 1983 69756 B0029 L264 01998 B29 .L264 1998 65867 B0029 P666 02004 B29 .P666 2004 40918 B0029 V55 01998 B 29 V55 1998 Why is my report sorted correctly, but not the inventory page? It leads me to believe that the inventory page isn't really sorting by cn_sort.