Bug 7121 - remove comma on items info
Summary: remove comma on items info
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) minor (vote)
Assignee: Owen Leonard
QA Contact: Bugs List
URL: /cgi-bin/koha/catalogue/moredetail.pl...
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-31 19:26 UTC by Nicole C. Engard
Modified: 2013-12-05 19:57 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed fix (7.79 KB, patch)
2011-11-11 17:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Revised fix (7.89 KB, patch)
2011-11-11 17:05 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 7121 - remove comma on items info (7.85 KB, patch)
2011-11-11 17:17 UTC, Nicole C. Engard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2011-10-31 19:26:56 UTC
When looking at the items tab in the staff client, the comma between the 245$a and the 245$b causes things to look a bit messy sometimes.  I think we can safely remove that comma and it will greatly improve the display.
Comment 1 Owen Leonard 2011-11-11 17:01:35 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2011-11-11 17:05:49 UTC Comment hidden (obsolete)
Comment 3 Nicole C. Engard 2011-11-11 17:17:30 UTC
Created attachment 6291 [details] [review]
[SIGNED-OFF] Fix for Bug 7121 - remove comma on items info

Because I can't just take out one comma, I reworked
the way moredetail.pl fetches and outputs biblio
data based on the way it does in detail.pl. This
allows for simpler output in the template.  In the
process I've added subtitle information to the
template.

Revision adds line break for readability
Comment 4 Liz Rea 2011-11-11 18:00:20 UTC
this patch requires keyword to marc mappings for subtitles:

so:

subtitle  245 $b
subtitle  245 $p
subtitle  245 $n
Comment 5 Liz Rea 2011-11-11 18:02:04 UTC
sorry that should be as follows:


subtitle	245	b
parts  	245	p
number	245	n
medium	245	h
Comment 6 Nicole C. Engard 2011-11-11 18:16:00 UTC
But it works without the mapping as well - it just doesn't show the subtitle, which it didn't show before so this patch is an improvement over the way it was.

Nicole
Comment 7 Liz Rea 2011-11-11 19:16:24 UTC
except he explicitly stated that he made a change to the subtitle display, which you can't test without the mappings.
Comment 8 Nicole C. Engard 2011-11-11 19:22:27 UTC
I'm confused.  Are you asking for more tests? Or saying it doesn't work, cause it looks to me like my mappings are showing up right.

Nicole
Comment 9 Liz Rea 2011-11-11 19:45:34 UTC
No, what I'm saying is, that the subtitle view won't work unless you have the mapping. When I went to test it, I missed that you had already signed off on it, and in my install the subtitles weren't showing up, which led me to dig deeper. 

Since the bug states that subtitles are supposed to show on moredetail.pl, I asked about the circumstances of their display, and found out that for the subtitles to display, you have to have the mappings. I added them, and the subtitles were displayed.

I would have considered it reasonable to overturn your signoff had I not been able to make the subtitles work in my test install, but since my testing shows they are ok with proper configuration, I don't see a problem with letting it stand and concur with your findings that the patch is ok for inclusion.

The note also serves as a heads up for QA, who may or may not have the mappings in their system.
Comment 10 Paul Poulain 2011-12-13 16:33:31 UTC
QA comment:
prove xt/author/translatable-templates.t
xt/author/translatable-templates.t .. ok   
prove xt/author/valid-templates.t 
xt/author/valid-templates.t .. ok     
perlcritic catalogue/moredetail.pl 
catalogue/moredetail.pl source OK


passed QA

patch pushed, please test
Comment 11 Jared Camins-Esakov 2012-05-23 12:06:45 UTC
Included in 3.6 prior to 3.6.5.