Bug 23912

Summary: Get correct authorised values for location and collections facets
Product: Koha Reporter: Lari Taskula <lari.taskula>
Component: Searching - ZebraAssignee: Bugs List <koha-bugs>
Status: NEW --- QA Contact:
Severity: normal    
Priority: P5 - low CC: fridolin.somers, jonathan.druart
Version: unspecifiedKeywords: regression
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26323
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 17248, 18322    
Bug Blocks:    
Attachments: Bug 23912: Get correct authorised value description for location and ccode facets in intranet
Bug 23912: Use C4::Context->interface instead of passing $opac

Description Lari Taskula 2019-10-28 13:18:38 UTC
Remove unused variable $opac from C4/Search.pm getRecords subroutine.
Comment 1 Lari Taskula 2019-10-28 14:40:55 UTC
It appears Bug 17248 made C4::Search::getRecords() parameter $opac unused and causes location and collection facets to prefer OPAC description over intranet one in all interfaces, including intranet itself.
Comment 2 Lari Taskula 2019-10-28 14:43:25 UTC
(In reply to Lari Taskula from comment #1)

Renamed bug title to match issue described in comment #1
Comment 3 Lari Taskula 2019-10-28 14:59:30 UTC
Created attachment 94812 [details] [review]
Bug 23912: Get correct authorised value description for location and ccode facets in intranet

Bug 17248 made C4::Search::getRecords() parameter $opac unused and causes
location and collection facets to prefer OPAC description over intranet
one in all interfaces, including intranet itself.

This patch restores the functionality provided by $opac parameter.

To reproduce:
1. Go to /cgi-bin/koha/admin/authorised_values.pl?searchfield=LOC and change
   both Description and Description (OPAC) of one row to be different
2. Do the same for /cgi-bin/koha/admin/authorised_values.pl?searchfield=CCODE
3. In intranet, search a record and make sure you are able to see facets after
   performing the search (your search must return multiple records)
4. Add/edit an item of one of these biblios on your results screen
5. Give this item the location and collection code that you changed in steps 1&2
6. In intranet, perform the same search from step 3 again
7. Observe your location and collections facets show Description (OPAC) value
   from step 1 and 2 instead of Description.

To test:
1. Apply patch
2. Try to reproduce the bug again in intranet
3. Observe you will now see Description value instead of Description (OPAC)
4. prove t/db_dependent/Search.t

Sponsored-by: Koha-Suomi Oy
Comment 4 Lari Taskula 2019-10-28 14:59:48 UTC
Created attachment 94813 [details] [review]
Bug 23912: Use C4::Context->interface instead of passing $opac

Since most of Koha's codebase using getRecords() is not passing $opac into the
subroutine (the only exception being opac-search.pl and search.pl), remove
parameter $opac from list of subroutine parameters and replace the functionality
with C4::Context->interface.

To test:
1. See test plan of the previous patch

Sponsored-by: Koha-Suomi Oy
Comment 5 Jonathan Druart 2019-10-30 11:43:46 UTC
Change for LOC is a regression from bug 17248 (>= 16.11). CCODE is coming from bug 18322 (>= 18.11).
Comment 6 Jonathan Druart 2019-10-30 11:50:59 UTC
(In reply to Lari Taskula from comment #4)
> Created attachment 94813 [details] [review] [review]
> Bug 23912: Use C4::Context->interface instead of passing $opac

Also, maybe we should move this one to its own bug report (more a code improvement than a bug fix).
It seems that we could (should?) replace the first parameter of 
searchResults at the same time ( { interface => 'opac' } )
Comment 7 Fridolin Somers 2020-09-29 12:38:59 UTC
I think it needs review to use code like in Bug 26323.

And also :

Why is there in C4::Search::getRecords

my $av = Koha::AuthorisedValues->search({ category => 'CCODE', authorised_value => $one_facet });

When the description is fetched in template :

[% IF facets_loo.type_label_CollectionCodes %][% SET facet.facet_label_value = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode', authorised_value => facet.facet_label_value, opac =>1 ) || facet.facet_label_value %][% END %]

Shouldn't we better add code for location facet that is like for ccode ?
And remove AV manipulation from C4::Search.
Comment 8 Fridolin Somers 2022-06-14 20:30:01 UTC
up
Comment 9 Lari Taskula 2022-06-14 21:17:23 UTC
(In reply to Fridolin Somers from comment #8)
> up

Thanks! I'm currently not able to contribute to this Bug and probably will not be in the near future, so feel free to take over this Bug.
Comment 10 Katrin Fischer 2022-06-17 20:56:15 UTC
I'll reclassify as bug, as we should show the intranet descriptons in the intranet and the OPAC descriptions in the OPAC. OPAC description in intranet is a bug and also sounds like a regression (comment#1