In XSLTParseForDisplay we fetch the coins for the OPAC even on the staff interface On opac results we fetch the biblio object, this is already fetched in the searchResults subroutine Additionally - we can fetch the coins in the searchResults to move logic out of the scripts
Created attachment 121123 [details] [review] Bug 28375: Inefficiencies in fetching COinS This patchset adds an 'interface' parameter to XSLTParseForDisplay to avoid fetching coins when not needed Additionally we move some logic from the scripts to searchResults to avoid an extra fetch fo the biblio object To test: 1 - Enable COinSinOPACResults syspref 2 - Also enable OPACShowOpenURL and OPACOpenURLItemTypes - adding an itemtype that you can find 3 - Perform a search on the OPAC and confirm coins are included and openurl shown 4 - Perform a search on staff client and confirm openurls do not show 5 - Apply patch 6 - Results should be the same
Created attachment 121124 [details] [review] Bug 28375: Inefficiencies in fetching COinS This patchset adds an 'interface' parameter to XSLTParseForDisplay to avoid fetching coins when not needed Additionally we move some logic from the scripts to searchResults to avoid an extra fetch fo the biblio object To test: 1 - Enable COinSinOPACResults syspref 2 - Also enable OPACShowOpenURL and OPACOpenURLItemTypes - adding an itemtype that you can find 3 - Perform a search on the OPAC and confirm coins are included and openurl shown 4 - Perform a search on staff client and confirm openurls do not show 5 - Apply patch 6 - Results should be the same
The code looks great and simple. But it doesn't apply. Please resubmit.
Created attachment 127582 [details] [review] Bug 28375: Inefficiencies in fetching COinS This patchset adds an 'interface' parameter to XSLTParseForDisplay to avoid fetching coins when not needed Additionally we move some logic from the scripts to searchResults to avoid an extra fetch fo the biblio object To test: 1 - Enable COinSinOPACResults syspref 2 - Also enable OPACShowOpenURL and OPACOpenURLItemTypes - adding an itemtype that you can find 3 - Perform a search on the OPAC and confirm coins are included and openurl shown 4 - Perform a search on staff client and confirm openurls do not show 5 - Apply patch 6 - Results should be the same
I get this error: Could not compile /kohadevbox/koha/opac/opac-search.pl: Global symbol "$oldbiblio_object" requires explicit package name (did you forget to declare "my $oldbiblio_object"?) at /kohadevbox/koha/C4/Search.pm line 2006.
Created attachment 140955 [details] [review] Bug 28375: Inefficiencies in fetching COinS This patchset adds an 'interface' parameter to XSLTParseForDisplay to avoid fetching coins when not needed Additionally we move some logic from the scripts to searchResults to avoid an extra fetch fo the biblio object To test: 1 - Enable COinSinOPACResults syspref 2 - Also enable OPACShowOpenURL and OPACOpenURLItemTypes - adding an itemtype that you can find 3 - Perform a search on the OPAC and confirm coins are included and openurl shown 4 - Perform a search on staff client and confirm openurls do not show 5 - Apply patch 6 - Results should be the same
Created attachment 141051 [details] [review] Bug 28375: Inefficiencies in fetching COinS This patchset adds an 'interface' parameter to XSLTParseForDisplay to avoid fetching coins when not needed Additionally we move some logic from the scripts to searchResults to avoid an extra fetch fo the biblio object To test: 1 - Enable COinSinOPACResults syspref 2 - Also enable OPACShowOpenURL and OPACOpenURLItemTypes - adding an itemtype that you can find 3 - Perform a search on the OPAC and confirm coins are included and openurl shown 4 - Perform a search on staff client and confirm openurls do not show 5 - Apply patch 6 - Results should be the same Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. Had to add an address to the OpenURLResolverURL system preference for the OpenURL link to display under the title in the OPAC search results. 2. The actual format of the link for OpenURL was the same in the OPAC search results both before and after the patch was applied (I compared the link for a specific item in the search results).
Lots of failures on QA tools.
Created attachment 141126 [details] [review] Bug 28375: (follow-up) Remove guessed interface parameter - always pass Previous patch added interface parameter passed on each call. There was an existing one that 'guessed' - this patch removes that Fixes missing comma
Great. No failures anymore from qa tools. Here is another question :) @@ -179,6 +179,7 @@ sub XSLTParse4Display { my $hidden_items = $params->{hidden_items} || []; my $variables = $params->{xslt_variables}; my $items_rs = $params->{items_rs}; + my $interface = $params->{interface}; Why do you use a parameter? Did you consider using C4::Context->interface ?
Created attachment 141490 [details] [review] Bug 28375: Inefficiencies in fetching COinS This patchset adds an 'interface' parameter to XSLTParseForDisplay to avoid fetching coins when not needed Additionally we move some logic from the scripts to searchResults to avoid an extra fetch fo the biblio object To test: 1 - Enable COinSinOPACResults syspref 2 - Also enable OPACShowOpenURL and OPACOpenURLItemTypes - adding an itemtype that you can find 3 - Perform a search on the OPAC and confirm coins are included and openurl shown 4 - Perform a search on staff client and confirm openurls do not show 5 - Apply patch 6 - Results should be the same Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 141491 [details] [review] Bug 28375: (follow-up) Remove guessed interface parameter - always pass Previous patch added interface parameter passed on each call. There was an existing one that 'guessed' - this patch removes that Fixes missing comma Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Adding a second signoff here as I missed that Marcel had started QAing... Leaving QA to Marcel once he's had an answer to his question.. it does bare asking.. the code would appear much simple if we just use the context.
Created attachment 141497 [details] [review] Bug 28375: (follow-up) Use C4::Context->interface
Created attachment 141955 [details] [review] Bug 28375: Inefficiencies in fetching COinS This patchset adds an 'interface' parameter to XSLTParseForDisplay to avoid fetching coins when not needed Additionally we move some logic from the scripts to searchResults to avoid an extra fetch fo the biblio object To test: 1 - Enable COinSinOPACResults syspref 2 - Also enable OPACShowOpenURL and OPACOpenURLItemTypes - adding an itemtype that you can find 3 - Perform a search on the OPAC and confirm coins are included and openurl shown 4 - Perform a search on staff client and confirm openurls do not show 5 - Apply patch 6 - Results should be the same Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 141956 [details] [review] Bug 28375: (follow-up) Remove guessed interface parameter - always pass Previous patch added interface parameter passed on each call. There was an existing one that 'guessed' - this patch removes that Fixes missing comma Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 141957 [details] [review] Bug 28375: (follow-up) Use C4::Context->interface Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 141958 [details] [review] Bug 28375: (QA follow-up) Remove two remaining interface params No test plan: parameter is already obsolete. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 22.11. Nice work everyone, thanks!