Bug 11357 - biblio.size value not correctly displayed in list emails sent from OPAC and intranet
Summary: biblio.size value not correctly displayed in list emails sent from OPAC and i...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin SOMERS
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-09 18:35 UTC by Katrin Fischer
Modified: 2015-12-03 22:02 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11357 - biblioitems.size value not correctly displayed in list emails sent from OPAC and intranet (13.46 KB, patch)
2014-06-19 13:15 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11357 - biblioitems.size value not correctly displayed in list emails sent from OPAC and intranet (13.54 KB, patch)
2014-06-27 12:04 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 11357 - biblioitems.size value not correctly displayed in list emails sent from OPAC and intranet (13.69 KB, patch)
2014-06-28 19:47 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2013-12-09 18:35:06 UTC
The size column in biblio is a bit problematic when used in TT, because instead of the size value from the biblio column it will give you the size of the variable. 

It's currently used in the templates for sending shelves from OPAC and intranet and maybe also in other places:

[% END %]
   [% IF BIBLIO_RESULT.size %]
   , [% BIBLIO_RESULT.size %]
[% END %]
Comment 1 David Cook 2014-03-23 23:14:00 UTC
Consider looking at bug 10466.

As far as I can tell, TT will give you the size of the hash rather than the value only if the size value is undefined. If it is defined, it will give you the size value. However, since the size column is often null in the database, you'll often get the size of the hash rather than a blank value.

Here's a relevant section from 10466:

"This bug has been identified in the opac-search.pl, search.pl and
addbooks.pl pages before...". The patch with the bug also touches C4/VirtualShelves/Page.pm.

I'm not sure what the best course of action is. If the database had a blank value rather than a null value, it would probably prevent this problem.
Comment 2 Fridolin SOMERS 2014-06-19 12:41:49 UTC
Has said in Bug 10466 and Bug 6374 the "size" is a virtual method of TT.
When biblio.size is not defined, [% BIBLIO_RESULT.size %] will return the number of keys : http://template-toolkit.org/docs/manual/VMethods.html#section_Hash_Virtual_Methods

There I think a better way of correcting than setting size in perl : using item virtual method in TT : http://template-toolkit.org/docs/manual/VMethods.html#section_item

I found this solution here : http://stackoverflow.com/questions/2311303/how-can-i-handle-hash-keys-containing-illegal-identifier-characters-in-template

This may append for other cases, like keys with space, hyphen, other colmns named like virtual method.

(I've been on this several months).
Comment 3 Fridolin SOMERS 2014-06-19 13:15:34 UTC Comment hidden (obsolete)
Comment 4 David Cook 2014-06-19 23:43:16 UTC
Fantastic, Fridolin! I don't have time today, but I'll look at this one next week if it is still untested.

It would be worth reverting bug 10466 as it's a bit of a hack to get around the problem which your patch seems to solve very nicely!
Comment 5 Fridolin SOMERS 2014-06-20 07:09:04 UTC
(In reply to David Cook from comment #4)
> Fantastic, Fridolin!
I'm happy to hear it.
Comment 6 Bernardo Gonzalez Kriegel 2014-06-27 12:04:24 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2014-06-28 19:47:32 UTC
Created attachment 29346 [details] [review]
[PASSED QA] Bug 11357 - biblioitems.size value not correctly displayed in list emails sent from OPAC and intranet

The size column in biblioitems is a bit problematic when used in TT, because instead of the size value from the biblio column it will give you the size of the variable.

It's currently used in the templates for sending shelves from OPAC and intranet and maybe also in other places:

[% END %]
   [% IF BIBLIO_RESULT.size %]
   , [% BIBLIO_RESULT.size %]
[% END %]

This patch corrects by using item() TT method.
See http://stackoverflow.com/questions/2311303/how-can-i-handle-hash-keys-containing-illegal-identifier-characters-in-template.

Test plan :
In each display :
=> Without this patch you see biblioitems.pages and then a number
=> With this patch you only see biblioitems.pages

- Create a record with biblioitems.pages defined (like "12p") but without biblioitems.size defined
Same for OPAC and intranet :
- Add it to the cart
- Open the cart
- Check the "Title" column
- Click on "More Details"
- Check the "Details" row
- Send the basket via email and check the result
- Add to a list
- Send the list via email and check the result

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works well!!
Tested on staff & opac, cart & list.
No koha-qa errors

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and tests.
Comment 8 Katrin Fischer 2014-06-28 19:48:47 UTC
Patch includes changes to the prog theme, but I think a small bug fix should be ok
Comment 9 Chris Cormack 2014-07-01 03:30:58 UTC
Small fix, pushed to master.
Comment 10 Mason James 2015-02-26 03:10:22 UTC
Possible selection for 3.16.x
Comment 11 Mason James 2015-04-22 10:04:35 UTC
Pushed to 3.16.x, will be in 3.16.10