Summary: | Build better $query_cgi | ||
---|---|---|---|
Product: | Koha | Reporter: | David Gustafsson <glasklas> |
Component: | Searching - Elasticsearch | Assignee: | Bugs List <koha-bugs> |
Status: | RESOLVED DUPLICATE | QA Contact: | |
Severity: | normal | ||
Priority: | P5 - low | CC: | joonas.kylmala, nick, nicolas.legrand, simon.voyer |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18319 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: |
Bug 20114: Build better $query_cgi
Bug 20114: (follow-up) Don't add sort in query cgi Alternative solution |
Description
David Gustafsson
2018-01-31 14:58:53 UTC
Created attachment 71089 [details] [review] Bug 20114: Build better $query_cgi Build a more complete $query_cgi fixing issues with pagination after performing either an advanced search or visiting a search link from biblio details. To test: 1) Go to the details page for a biblio. 2) Click a link that will produce enough hits to trigger pagination. 3) Try going to next page. 4) This will produce a search error. 5) Apply patch. 6) Repeat steps 1 to 3. 7) Pagination should now work as expected. Sponsored-by: Gothenburg University Library Discoverd a double-encoding issue, and problem with sort_by being appended multiple times. So probably best to avoid reviewing this issue until this is fixed. Created attachment 73025 [details] [review] Bug 20114: (follow-up) Don't add sort in query cgi To test: 1 - Do a search for"*:*" 2 - Try to limit using a facet 3 - Get a search error 4 - Note the URL says kw=*:* 5 - Apply patch 6 - Repeate search 7 - Facet should work 8 - Note URL is better formed 9 - Test with advanced searches as well Works great till 9, where it sent lots of idx=kw: localhost:9999/cgi-bin/koha/catalogue/search.pl?idx=kw&q=*%3A*&op=and&idx=kw&q=&op=and&idx=kw&q=&limit-yr=&limit=&limit=&limit=&limit=&limit=&limit=&limit=&limit=&limit=&limit=&limit=&limit=&limit=&limit=&sort_by=relevance and get: Error: Unable to perform your search. Please try again. Created attachment 80410 [details] [review] Alternative solution How would you like the solution in the attachment (it is made for older version of koha so it won't apply)? I.e. completely forego the recreation of search query. Actually the recreation code is used in just a couple other places (addbooks.pl, neworderbiblio.pl, opac-search.pl) in Koha so maybe it could be somehow replaced completely. Comment on attachment 71089 [details] [review] Bug 20114: Build better $query_cgi Review of attachment 71089 [details] [review]: ----------------------------------------------------------------- ::: Koha/SearchEngine/Elasticsearch/QueryBuilder.pm @@ +45,4 @@ > use List::MoreUtils qw/ each_array /; > use Modern::Perl; > use URI::Escape; > +use URI::QueryParam; Is this a new dependency? @@ +269,4 @@ > ); > } > > +sub _build_query_query_string { Can we get a more descriptive name or some comments to clarify what this means? I have more or less given up on this one for the current being since the query parsing for the search page CGI is so fubar and probably needs a larger overhaul and refactoring than the approach in this patch. If I remember correctly I tried fixing the current issues of the patch but each fix tended to break other things. Hi, this might be fixed by the bug #22413 but I have not yet tested but it looks very similar to this bug. I think David has some points, we could do more here in the future. As he has abandoned this code and 22413 seems to fix the issues caused by this bug I think we can close *** This bug has been marked as a duplicate of bug 22413 *** Sadly I'll have to bump this one as still open. On our side it seems like a regression from 19.05, but it could come from our weird branch setup and my misunderstanding of Koha. Here's an example from our dev server (excuse my bad knowledge of procedures, I don't think I've seen anyone sharing direct links to their servers) : 1 -- http://dev.koha.collecto.ca/cgi-bin/koha/opac-search.pl?advsearch=1&idx=kw&q=&op=and&idx=kw&q=&op=and&idx=kw&q=&limit-yr=1900-1901&limit=&limit=&sort_by=pubdate_dsc&limit=&limit=&limit=&limit= 2 -- Sort the results Title A-Z (or any option actually) 3 -- Anything query-related is gone from the URL, except for the new sort_by parameter and a weird &addto=Add+to... that seems to be taken from the results box. Any pointers would be appreciated. At this point I feel pretty defeated, as I'm a new developer fresh outta Internet school where they teach you JS and Python. Perl seems to me like a huge monster trying to eat me alive every day. Ok, news update! It does come from our branches setup. Carry on with your lives, I'll make sure to use Koha's stable version instead of the fixed version we have with our «home-made» Koha branch. |