|
Description
Jonathan Druart
2013-09-10 11:53:29 UTC
Nice idea of improvement Jonathan. Mathieu Created attachment 20946 [details] [review] Bug 10857: Add facets cancellation When searching (at the OPAC or pro), facets can be enabled but never disabled. So a user is obliged to relaunch the search. This patch adds a new link "[x]" at the right of each selected facet. This link relaunch the search without this facet. Test plan: - Launch a search (OPAC and pro) - Enable some facets - Disable some facets I made 4 tests : click on a Series facet, then remove it => OK click on an Author facet, then remove it => OK click on a Serial facet, then an Author facet, then remove the Author facet, then remove the Serial facet => OK click on a Serial facet, then an Author facet, then remove the Serial facet, then remove the Author facet => KO For me the last test failed : if the facets are removed in a different order than the order used to enable them, the [x] disappears, but Koha does not relaunch the search. Do you think you can fix that? I hope this is not too complex. Mathieu (In reply to mathieu saby from comment #3) > then remove the Author facet => KO > > For me the last test failed : if the facets are removed in a different order > than the order used to enable them, the [x] disappears, but Koha does not > relaunch the search. I cannot reproduce the issue. There is no importance on the order. If a problem exists, it is not this one :) Could you retest with others facets please? Maybe a js problem in my browser! I will take an other look. Mathieu Sorry, still not working. I made a little screencast. Better than long speeches... https://www.youtube.com/watch?v=5ANbVtjpnNA Mathieu (In reply to mathieu saby from comment #6) > Sorry, still not working. > I made a little screencast. Better than long speeches... > https://www.youtube.com/watch?v=5ANbVtjpnNA > > Mathieu How many results did you get with only the author facet selected? 5 results with one facet, 2 results with 2 facets. Maybe my records are not very "clean" and that could explain the issue. Should I try on a sandbox? Mathieu (In reply to mathieu saby from comment #8) > 5 results with one facet, 2 results with 2 facets. On the screencast I see 5 results for the "Series" facets. So you got 5 results for the author facet too? If you got only 2, the behavior is normal. If you got 5, it is not. > Maybe my records are not very "clean" and that could explain the issue. > Should I try on a sandbox? Yes, it will be easiest to debug an issue on a sandbox. Request sent on sandbox 3 ;-) Test results this afternoon. Mathieu Hi Mathieu, did you have a chance to test? I would really like to see this happen :) I made some tests : I was not satisfied, and had probably not related problems (I crashed my browers 3 times because of too big http header or something like that). I doubt it is related to this bug. So, I prefer someone else to test it. Mathieu And sandbox 3 crashed to, but probably for another reason... Mathieu Patch applied cleanly, go forth and signoff (In reply to mathieu saby from comment #12) > I made some tests : I was not satisfied, and had probably not related > problems (I crashed my browers 3 times because of too big http header or > something like that). Here's what I got after testing a few times: Bad Request Your browser sent a request that this server could not understand. Size of a request header field exceeds server limit. Cookie /n OH, exactly the same bug here! But I thought it was not linked. M. Saby Hi all, I think this bug is only due to the cookies size limitation and the way Koha handle them (or maybe the problem comes from Apache or the web browser I don't know), but it is not specific to this patch. So I wrote a patch to store anonymous search history into CGI::Session instead of cookies. Please see Bug 10952 As Julian said, I am not sure the problem is from this patch. I think the issue already exists in master. Switch back to needs signoff. I haven't run across the "Bad request" error again, but I did find this: 1. Perform a search 2. Click a library facet 3. Cancel the library facet 4. Click another library facet At this point there is no "[x]" by the facet I clicked in step 4. Same results in the OPAC and staff client. Hi Owen & Jonathan I experienced the same bug during my tests. Mathieu (In reply to Owen Leonard from comment #19) > I haven't run across the "Bad request" error again, but I did find this: > > 1. Perform a search > 2. Click a library facet > 3. Cancel the library facet > 4. Click another library facet > > At this point there is no "[x]" by the facet I clicked in step 4. Same > results in the OPAC and staff client. I cannot reproduced, tested in the OPAC and staff. The link appears and works. What is the url generated in step 4? 1. Keyword search from the main search bar: /cgi-bin/koha/opac-search.pl?idx=&q=blue 2. Click library facet: /cgi-bin/koha/opac-search.pl?q=blue%2520%2523relevance_dsc&sort_by=relevance_dsc&limit=branch:APL 3. Cancel library facet: /cgi-bin/koha/opac-search.pl?idx=kw&q=blue%2520&limit=branch:APL&sort_by=relevance_dsc&nolimit=branch:APL 4. Click another library facet: /cgi-bin/koha/opac-search.pl?q=kw%253Ablue%2520%2523relevance_dsc&sort_by=relevance_dsc&limit=branch:NPL (In reply to Owen Leonard from comment #22) > 1. Keyword search from the main search bar: > /cgi-bin/koha/opac-search.pl?idx=&q=blue > > 2. Click library facet: > > /cgi-bin/koha/opac-search. > pl?q=blue%2520%2523relevance_dsc&sort_by=relevance_dsc&limit=branch:APL > > 3. Cancel library facet: > > /cgi-bin/koha/opac-search.pl?idx=kw&q=blue%2520&limit=branch: > APL&sort_by=relevance_dsc&nolimit=branch:APL > > 4. Click another library facet: > > /cgi-bin/koha/opac-search. > pl?q=kw%253Ablue%2520%2523relevance_dsc&sort_by=relevance_dsc&limit=branch: > NPL Here I get: 1/ opac-search.pl?q=d 2/ opac-search.pl?idx=kw&q=d&sort_by=author_asc&limit=branch:CPL 3/ opac-search.pl?idx=kw&q=d&limit=branch:CPL&sort_by=author_asc&nolimit=branch:CPL 4/ opac-search.pl?idx=kw&q=d&sort_by=author_asc&limit=branch:MPL I don't understand why between your 3 and 4 the idx=kw&q=blue become kw:blue. But in all cases, a facet is considered as active if a limit param is index:value. Since it works with the first query, it is really odd that it does not work with the second one. Mathieu, did you reproduce on a sandbox?
>
> Mathieu, did you reproduce on a sandbox?
Hi
I tried this morning, but
- Sandbox 1 did not set up
- Sandbox 8 (Marc21) did set up, but 0 results to any search on staff interface and opac (maybe zebra not running?)
Mathieu
(In reply to mathieu saby from comment #24) > - Sandbox 1 did not set up The sandbox is now available. I blocked it for tests. Great. I spent a lot of time on your patch for invoice prices (prices give me headache...) but I will try this one on sandbox 1 this afternoon. Mathieu Note : it is easier to test by setting maxRecordsForFacets to a big number (say 500 or 1000) and activate displayFacetCount. Mathieu Test made on sandbox 1 Not easy at all to test, because (in Unimarc at least) - Author facets with "," are broken Ex : http://catalogue.test1.biblibre.com/cgi-bin/koha/opac-search.pl?idx=kw&q=fr%252A&limit=au:Balzac,%20Honor%C3%A9%20de => 0 results - Topic facets with "-" are broken Ex : http://catalogue.test1.biblibre.com/cgi-bin/koha/opac-search.pl?idx=kw&q=fr%252A&limit=su-to:Nature%20-%20revue => 0 results I did not check, but I think they are broken in master too as your patch is not supposed to change their behavior Test 1 on OPAC: maxRecordsForFacets = 1000 initial search ="a*" 1st click on series facet "Folio(39)" => [x] before facet Folio; 147 results http://catalogue.test1.biblibre.com/cgi-bin/koha/opac-search.pl?idx=kw&q=a%252A&limit=se:Folio 2d click on topic facet "823 (9)" => [x] before facet "823" ; facet "Folio" still activated ; 2 results http://catalogue.test1.biblibre.com/cgi-bin/koha/opac-search.pl?idx=kw&q=a%252A&limit=se:Folio&limit=su-to:823 3rd click on series facet "Folio" => [x] disapear before "Folio" ; 52 results http://catalogue.test1.biblibre.com/cgi-bin/koha/opac-search.pl?idx=kw&q=a%252A&limit=se:Folio&limit=su-to:823&nolimit=se:Folio Just to check : manually editing the url to limit only to "823" topic : 52 results catalogue.test1.biblibre.com/cgi-bin/koha/opac-search.pl?idx=kw&q=a%252A&limit=su-to:823 Click on "Folio (2)" series facet => the 2 facets now have [x] ; 9 results are showing (why not 2?) Click on "823" facet topic => the [x] disapear before "823" ; 147 results => it seems OK Test 2 on staff interface : paying with 3 different facets, checking, unchecking in a different order => it seems OK I will try to reproduce Owen's issue Mathieu Test 3 : I confirm the problem if the 2 facets are in the same category. And I experienced again the crash server again, but I have no idea whether it is related to your patch. - search "e*" - click on library "Le Canet (1000)" => [x] before 3950 result(s) found for 'kw,wrdl: e*' with limit(s): 'branch:Le Cannet' in Médiathèque municipale Le Cannet des Maures Catalog. http://pro.test1.biblibre.com/cgi-bin/koha/catalogue/search.pl?idx=kw&q=e%252A&limit=branch:MAURES - click on library "Médiathèque départementale de prêt (4)" http://pro.test1.biblibre.com/cgi-bin/koha/catalogue/search.pl?idx=kw&q=e%252A&limit=branch:MAURES&limit=branch:BDP BOUM! FF Error "La connexion avec le serveur a été réinitialisée pendant le chargement de la page." I broke sandbox staff interface! I had made the same test 5 before, but I did not note the results at that time. And I did not experience the server error. Strange. I try to test the same search on OPAC - search "e*" - click on library "Le Canet (1000)" => [x] before http://catalogue.test1.biblibre.com/cgi-bin/koha/opac-search.pl?idx=kw&q=e%252A&limit=branch:MAURES Your search returned 3950 results - click on library "Médiathèque départementale de prêt" [x] before the 2 facets http://catalogue.test1.biblibre.com/cgi-bin/koha/opac-search.pl?idx=kw&q=e%252A&limit=branch:MAURES&limit=branch:BDP Your search returned 21 results. Subscribe to this search - uncheck "Le Canet" => still 21 results! http://catalogue.test1.biblibre.com/cgi-bin/koha/opac-search.pl?idx=kw&q=e%252A&limit=branch:MAURES&limit=branch:BDP&nolimit=branch:MAURES Your search returned 21 results. Médiathèque Départementale de Prêt [x] Le Cannet (21) Mathieu Hem, wait a second, in fact I think it working as expecteded... In fact, there is only 21 documents in BDP http://catalogue.test1.biblibre.com/cgi-bin/koha/opac-search.pl?idx=kw&q=e%252A&limit=branch:BDP and all of these seems to be also in "Le Canet" (I cannot see "Le Canet" items on records, maybe they are hidden). So my 3d test does not prove anything :( And I cannot go back to staff interface so suppress some items, as I broke it... Mathieu So, still "need signoff" Patch applied cleanly, go forth and signoff Created attachment 22746 [details] [review] Bug 10857: Encode 'limit' parameter values to allow facets with '&' character. Created attachment 24081 [details] [review] Bug 10857: Add facets cancellation When searching (at the OPAC or pro), facets can be enabled but never disabled. So a user is obliged to relaunch the search. This patch adds a new link "[x]" at the right of each selected facet. This link relaunch the search without this facet. Test plan: - Launch a search (OPAC and pro) - Enable some facets - Disable some facets Created attachment 24082 [details] [review] Bug 10857: Encode 'limit' parameter values to allow facets with '&' character. rebased patch. Only available for the prog theme (for now). I made several searchs and tested several facets : authors - series - topics and I have used several facets in the same time (in the same categries or not). When I remove a facet with [x], I have the logical number of results. I have the same results in OPAC or in pro interface. It's ok for me. Sonia Patch tested with a sandbox, by sonia bouis <sonia.bouis@univ-lyon3.fr> Created attachment 24125 [details] [review] Bug 10857: Add facets cancellation When searching (at the OPAC or pro), facets can be enabled but never disabled. So a user is obliged to relaunch the search. This patch adds a new link "[x]" at the right of each selected facet. This link relaunch the search without this facet. Test plan: - Launch a search (OPAC and pro) - Enable some facets - Disable some facets Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr> Created attachment 24126 [details] [review] Bug 10857: Encode 'limit' parameter values to allow facets with '&' character. Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr> Needs updated to include bootstrap theme. Other than that, it looks great! Created attachment 24162 [details] [review] Bug 10857: Add facets cancellation - bootstrap theme. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 24163 [details] [review] Bug 10857: Add facets cancellation - bootstrap theme. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 24201 [details] [review] [PASSED QA] Bug 10857: Add facets cancellation When searching (at the OPAC or pro), facets can be enabled but never disabled. So a user is obliged to relaunch the search. This patch adds a new link "[x]" at the right of each selected facet. This link relaunch the search without this facet. Test plan: - Launch a search (OPAC and pro) - Enable some facets - Disable some facets Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Patches pass all tests and QA script. Nice feature! Tested in Boostrap and Prog, adding end removing multiple facets in different sequences, adding and removing the availability limit. Created attachment 24202 [details] [review] [PASSED QA] Bug 10857: Encode 'limit' parameter values to allow facets with '&' character. Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 24203 [details] [review] [PASSED QA] Bug 10857: Add facets cancellation - bootstrap theme. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> I think maybe the little x could be formatted a bit differently to be more obvious, but that is not enough for failing QA on this :) I discovered bug 11524 while testing this, which could be related to bug 10952 this depends on. *** Bug 11774 has been marked as a duplicate of this bug. *** Pushed to master. Thanks, Jonathan! |