Created attachment 54997 [details] screenshot of bug Example screenshot attached, here's the bib record in our catalog: https://library.cca.edu/cgi-bin/koha/opac-detail.pl?biblionumber=21729&shelfbrowse_itemnumber=34420#holdings Basically, since starting_homebranch is null in the template file, nothing is printed out, but the next IF condition wrongly assumes that the first must have been true and prints out a comma before it's text string. Steps to recreate: - in settings, set ShelfBrowserUsesHomeBranch to "Don't Use" - set ShelfBrowserUsesLocation to "Use" - visit an item, click the "Browse shelf" link in the Holdings' tab's Call Number column - note the extraneous comma in the <h5> tag that appears Patch coming shortly, it appears that simply adding an additional condition around the comma itself should resolve this problem.
Created attachment 54998 [details] [review] patch patch anticipates same problem with collection code in list, too
If you have ShelfBrowserUsesCcode, ShelfBrowserUsesHomeBranch, and ShelfBrowserUsesLocation all set to 'Use," but you are browsing an item which doesn't have a shelving location there are extra commas: " Browsing Athens Shelves , , Collection code: Juvenile fiction " I wonder if that scenario can be handled as well? By the way, please include a test plan in your commit messages. And if you would like to be credited by name in Koha's release notes, about page, etc., configure git with your real name so it appears in the patch. Thanks for the patch!
Thanks for the feedback Owen, I'll take another look & develop a better testing plan. It's difficult because there are so many permutations of settings and then fields that could be empty on the item records but that's all the more reason to have a well-defined plan. I want to be credited to my GitHub username (phette23) so the lack of my real name in the commit notes is intentional.
I think you can still get credited if it's not a direct match between your commit message and your user name.
Created attachment 97267 [details] [review] Bug 17221: Remove orphan commas in shelf browser This patch adds more conditions to the shelf browser template such that the displayed text reads as a proper list no matter what combination of system preferences are utilized Sponsored-by: California College of the Arts Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 97268 [details] [review] Bug 17221: (follow-up) Orphan comma in shelf browser This further improves the display if no location or no collection has been set. Example output: Browsing Centerville shelves, Collection: Non-fiction Also changes 'Collection code' to 'collection' and takes care of additional spaces before the comma. To test: - Make sure your OpacShelfBrowser pref is set to "Show" - Search for a record with items and callnumbers in your OPAC - "Browse shelf" - Navigate back and forth, verify the information showing on top of the list is correctly formatted - Try different combinations for the following prefs: - ShelfBrowserUsesCcode - ShelfBrowserUsesHomeBranch - ShelfBrowserUsesLocation - Verify the display is always nicely formatted Note: fixing capitalization here is tricky, as it also depends on what was displayed before. If you have no homebranch, you would want "Shelving...", if you have one we'd like "shelving...". Left as I found it for now. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 98830 [details] [review] Bug 17221: Remove orphan commas in shelf browser This patch adds more conditions to the shelf browser template such that the displayed text reads as a proper list no matter what combination of system preferences are utilized Sponsored-by: California College of the Arts Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 98831 [details] [review] Bug 17221: (follow-up) Orphan comma in shelf browser This further improves the display if no location or no collection has been set. Example output: Browsing Centerville shelves, Collection: Non-fiction Also changes 'Collection code' to 'collection' and takes care of additional spaces before the comma. To test: - Make sure your OpacShelfBrowser pref is set to "Show" - Search for a record with items and callnumbers in your OPAC - "Browse shelf" - Navigate back and forth, verify the information showing on top of the list is correctly formatted - Try different combinations for the following prefs: - ShelfBrowserUsesCcode - ShelfBrowserUsesHomeBranch - ShelfBrowserUsesLocation - Verify the display is always nicely formatted Note: fixing capitalization here is tricky, as it also depends on what was displayed before. If you have no homebranch, you would want "Shelving...", if you have one we'd like "shelving...". Left as I found it for now. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 99188 [details] [review] Bug 17221: Remove orphan commas in shelf browser This patch adds more conditions to the shelf browser template such that the displayed text reads as a proper list no matter what combination of system preferences are utilized Sponsored-by: California College of the Arts Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 99189 [details] [review] Bug 17221: (follow-up) Orphan comma in shelf browser This further improves the display if no location or no collection has been set. Example output: Browsing Centerville shelves, Collection: Non-fiction Also changes 'Collection code' to 'collection' and takes care of additional spaces before the comma. To test: - Make sure your OpacShelfBrowser pref is set to "Show" - Search for a record with items and callnumbers in your OPAC - "Browse shelf" - Navigate back and forth, verify the information showing on top of the list is correctly formatted - Try different combinations for the following prefs: - ShelfBrowserUsesCcode - ShelfBrowserUsesHomeBranch - ShelfBrowserUsesLocation - Verify the display is always nicely formatted Note: fixing capitalization here is tricky, as it also depends on what was displayed before. If you have no homebranch, you would want "Shelving...", if you have one we'd like "shelving...". Left as I found it for now. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to master for 20.05
Pushed to 19.11.x branch for 19.11.04
backported to 19.05.x for 19.05.09