Bug 17221

Summary: Orphan comma in shelf browser
Product: Koha Reporter: Eric Phetteplace <ephetteplace>
Component: OPACAssignee: Katrin Fischer <katrin.fischer>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: 1joynelson, black23, ephetteplace, katrin.fischer, lucas
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.04, 19.05.09
Attachments: screenshot of bug
patch
Bug 17221: Remove orphan commas in shelf browser
Bug 17221: (follow-up) Orphan comma in shelf browser
Bug 17221: Remove orphan commas in shelf browser
Bug 17221: (follow-up) Orphan comma in shelf browser
Bug 17221: Remove orphan commas in shelf browser
Bug 17221: (follow-up) Orphan comma in shelf browser

Description Eric Phetteplace 2016-08-29 20:49:56 UTC
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.
Comment 1 Eric Phetteplace 2016-08-29 21:11:09 UTC
Created attachment 54998 [details] [review]
patch

patch anticipates same problem with collection code in list, too
Comment 2 Owen Leonard 2016-08-30 15:17:51 UTC
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!
Comment 3 Eric Phetteplace 2016-08-30 22:47:39 UTC
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.
Comment 4 Katrin Fischer 2016-09-04 19:22:47 UTC
I think you can still get credited if it's not a direct match between your commit message and your user name.
Comment 5 Katrin Fischer 2020-01-12 11:07:57 UTC
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>
Comment 6 Katrin Fischer 2020-01-12 11:08:00 UTC
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>
Comment 7 Owen Leonard 2020-02-13 14:05:38 UTC
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>
Comment 8 Owen Leonard 2020-02-13 14:05:40 UTC
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>
Comment 9 Jonathan Druart 2020-02-18 14:24:48 UTC
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>
Comment 10 Jonathan Druart 2020-02-18 14:24:51 UTC
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>
Comment 11 Martin Renvoize 2020-02-19 11:33:59 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 12 Joy Nelson 2020-03-13 00:16:39 UTC
Pushed to 19.11.x branch for 19.11.04
Comment 13 Lucas Gass 2020-03-18 17:16:56 UTC
backported to 19.05.x for 19.05.09