At OPAC side the preference AuthorisedValueImages already exists. In staff client, the search results always include these images. If they are not found, they are replaced by itemtype desc. This results in results like: 4. Iznik pottery and tiles in the Calouste Gulbenkian collection / Queiroz Ribeiro, Maria Description: [London] : Scala, 2009 : 136 p. : ; 31 cm. ISBN: 9728848587 Collection catalogs Collection catalogs Collection catalogs NOTE: The three Collection catalogs (itemtype desc) refer to three authorised values: a relator code, a relator term and a itemtype. This make no sense. (We made an authorised list for relator codes and terms.) The new preference AuthorisedValueImages adds the option to hide display of the images or the duplicate alternative descriptions.
Created attachment 3056 [details] [review] Patch (for functionality) This patch contains the relevant code. The followup patch only adds installation: adding the pref at upgrade or install time.
Created attachment 3057 [details] [review] Patch (for install) This followup patch only contains installation: adding the pref at upgrade or install time.
Created attachment 3058 [details] [review] Patch (for install) This followup patch only contains installation: adding the pref at upgrade or install time.
I'm not sure what effect this code has. The TMPL_VAR authorised_value_images is never set by catalogue/search.pl and so should never show up in results.tmpl in spite of being checked for there. If the result display is malformed, perhaps there is another syntax problem within the results.tmpl template. Or have I missed something in my testing?
Thank you for testing. Like to respond to your comments however: The TMPL_VAR is set in a module: C4/Search.pm, searchResults (called by catalogue/search.pl) $oldbiblio->{'authorised_value_images'} = C4::Items::get_authorised_value_images( C4::Biblio::get_biblio_authorised_values( $oldbiblio->{'biblionumber'}, $marcrecord ) ); It certainly shows up. See the example below. I eliminated it by first testing with the template. I do not suspect any syntax problem here.
(In reply to comment #5) > Thank you for testing. Like to respond to your comments however: > > The TMPL_VAR is set in a module: > > C4/Search.pm, searchResults (called by catalogue/search.pl) > > $oldbiblio->{'authorised_value_images'} = > C4::Items::get_authorised_value_images( > C4::Biblio::get_biblio_authorised_values( $oldbiblio->{'biblionumber'}, > $marcrecord ) ); > > It certainly shows up. See the example below. I eliminated it by first testing > with the template. > I do not suspect any syntax problem here. I see it now. It is part of the newresults array in search.pl. So a couple of questions: 1. How do I get authorised_value_images to populate? 2. Can you attach screen shots showing what the problem is and what the fix looks like? Thanks
Created attachment 3067 [details] Screen shot 1 Shows duplicate item type descriptions (alternative text for authorised value images)
Created attachment 3068 [details] Screen shot 2
> How do I get authorised_value_images to populate? Please see Comment1 also. Attach an authorised value list to a field in your framework, and use that in a record. We did so for item type, relator code and relator term. This generates either images or alternative text (if no image was found) for each one. See e.g. the second or fourth record in the screenshot.
Created attachment 3073 [details] [review] Revised patch (functionality) Commit 0088c5da53a56f44ec435f88a56b67ae99616a4d renamed staff_client.pref file
This bug is mentioned in: Bug 5687: Add pref StaffAuthorisedValueImages for staff search results http://lists.koha-community.org/pipermail/koha-patches/2011-February/013752.html Bug 5687: Add pref StaffAuthorisedValueImages for staff search results http://lists.koha-community.org/pipermail/koha-patches/2011-February/013753.html Bug 5687: Add pref StaffAuthorisedValueImages for staff search results http://lists.koha-community.org/pipermail/koha-patches/2011-February/013771.html
I'd recommend that a conditional be added to C4::Search::searchResults to only populate $oldbiblio->{'authorised_value_images'} if they're going to be used by the requesting search results page. That is: IF (($search_context == 'opac' and AuthorisedValueImages) OR ($search_context == 'intranet' and StaffAuthorisedValueImages)) The call to get authorised values can be very expensive, and if it's not going to be used in the output, we could save ourselves some processing time (possibly over 1 second's worth) when performing a search by never invoking the subroutine. This would also simplify the template: we wouldn't need a nested conditional for both the system preference AuthorisedValueImages and the existence of the images themselves; they would only exist if they were supposed to.
+1
Created attachment 3278 [details] [review] Revised patch (functionality) Revised patch populates authvalimages only if needed. No changes to template or search.pl needed anymore.
Created attachment 3279 [details] [review] Revised patch (install) Revised patch for installation of new syspref
Created attachment 3341 [details] [review] Patch for install (updated)
The revised patch for functionality works! Running a search through NYTProf before, I was getting over 1.29 s inclusive time spent on 20 instances of get_biblios_authorised_values. That has now disappeared, and has not recurred in other places. Also tested manually, by counting seconds for searches to return. Dropped from 4 seconds to 3 seconds, which I would call quite significant. If you pass along the full install patch, I'll happily sign off
Created attachment 3459 [details] [review] Patch for install (updated) Rebased on March 28
(In reply to comment #17) > The revised patch for functionality works! > Running a search through NYTProf before, I was getting over 1.29 s inclusive > time spent on 20 instances of get_biblios_authorised_values. That has now > disappeared, and has not recurred in other places. > Also tested manually, by counting seconds for searches to return. Dropped from > 4 seconds to 3 seconds, which I would call quite significant. > If you pass along the full install patch, I'll happily sign off Thanks for testing, Ian. I rebased the install patch. So if you could sign both patches, please go ahead.
Signed off patches sent to list
Patch pushed, please test
Works for me. Thanks for signoff and push. Anyone else to close it?
Can someone give me the steps to follow in order to view these images? I don't manage to get them...
(In reply to comment #23) > Can someone give me the steps to follow in order to view these images? I don't > manage to get them... Just clicking on the links causes them to display in my browser (FF).
(In reply to comment #24) > Just clicking on the links causes them to display in my browser (FF). Maybe I wasn't clear, setting StaffAuthorisedValueImages to "Show" or "Don't Show" doesn't make any changes. The only images I see in search results are the itemtype images. The description of this system preference is talking about lost statuses and locations, so I try to "attach" an image to theses authorised values, and change the status of some items to "Lost", for example. I don't see any changes. Can you be more precise please?
(In reply to comment #25) > (In reply to comment #24) > > Just clicking on the links causes them to display in my browser (FF). > Maybe I wasn't clear, setting StaffAuthorisedValueImages to "Show" or "Don't > Show" doesn't make any changes. The only images I see in search results are the > itemtype images. The description of this system preference is talking about > lost statuses and locations, so I try to "attach" an image to theses authorised > values, and change the status of some items to "Lost", for example. I don't see > any changes. > Can you be more precise please? Please see problem description. In screen shots you see repeated item type descriptions like "Collection catalogs Collection catalogs Collection catalogs". (The text is already an alternative for an image.) This comes from authorized values used on relator codes and relator terms. If you would not use authorized values on such fields, you will not see it.
This bug is mentioned in: Bug 5687: Add pref StaffAuthorisedValueImages for staff search results http://lists.koha-community.org/pipermail/koha-patches/2011-March/014018.html Bug 5687: Add pref StaffAuthorisedValueImages for staff search results (install) http://lists.koha-community.org/pipermail/koha-patches/2011-March/014019.html Bug 5687: Add pref StaffAuthorisedValueImages for staff search results (install) http://lists.koha-community.org/pipermail/koha-patches/2011-March/014113.html Bug 5687: Add pref StaffAuthorisedValueImages for staff search results (install) http://lists.koha-community.org/pipermail/koha-patches/2011-March/014255.html Bug 5687: Add pref StaffAuthorisedValueImages for staff search results http://lists.koha-community.org/pipermail/koha-patches/2011-March/014332.html Bug 5687: Add pref StaffAuthorisedValueImages for staff search results (install) http://lists.koha-community.org/pipermail/koha-patches/2011-March/014333.html
Closing this bug myself.