Created attachment 3485 [details] [review] Proposed Patch
Created attachment 3657 [details] [review] Followup - change Location to Shelving Location
Comment on attachment 3657 [details] [review] Followup - change Location to Shelving Location Attached the patch to the wrong bug, sorry!
Patch applies but I have: Missing right curly or square bracket at /home/koha/versions/koha_community/opac/opac-search.pl line 759, at end of line syntax error at /home/koha/versions/koha_community/opac/opac-search.pl line 759, at EOF Execution of /home/koha/versions/koha_community/opac/opac-search.pl aborted due to compilation errors. ~/cgi-bin/koha/opac-search.pl
Created attachment 4798 [details] [review] proposed patch for T::TT + fix for missing bracket The patch contained some duplicated lines, probably the result of a split/rebase Problem reported by Claire fixed by removing the 4 duplicated lines + patch updated for T::T
Can someone provide an explanation of what this patch does and a test plan? What am I looking for when I apply this patch? Nicole
Nicole : before this patch, if you do an advanced search, with limits like branch/itemtype, it was not saved correctly in the search history, so if you re-do a search from there you won't get the same results. After this patch = it works correctly Updating severity, it's a bug not an ENH !
This patch should not have a number in the updatedatabse: +$DBversion = "3.05.00.005"; It should be +$DBversion = "3.05.00.XXX"; The RM is the one who chooses the next number.
Created attachment 4860 [details] [review] proposed patch (updated for Nicole comment about updatedatabase XXX)
The current iteration of the patch is not signed off...
Yes it was, I signed off on it as soon as Paul sent in the updated patch. http://lists.koha-community.org/pipermail/koha-patches/2011-August/015586.html Nicole
Changes in C4/Auth.pm include references to system preferences that do not exist (TemplateEncoding, IntranetXSLTResultsDisplay and IntranetXSLTDetailsDisplay) and are not used anywhere else in the patch. Also defines a subroutine, enabled_opac_search_views, that is not used in the patch. There is also something in here about Tags, which appears to be outside the scope of this patch. Because of all this unnecessary inclusion, I'm marking this as Failed QA. Please strip this patch down to only deal with the issue presented in this bug report, and file separate bug reports and patches for separate issues.
I've checked the patches on biblibre/master (2), they are much smaller than this one. That should be a merge/split mistake, will submit a fixed version asap
Created attachment 5277 [details] [review] proposed patch, rebased, with QA problems fixed This patch is smaller than the previous one because a lot of code had nothing to do in this patch. (some lines with a TAB have been changed to spaces)
GBSD#4 Looking at this one.
If I search (coming from Advanced Search) with a limit on year: cgi-bin/koha/opac-search.pl?idx=kw&q=vermeer&idx=kw&idx=kw&do=Zoek&limit-yr=-2005&sort_by=pubdate_dsc the limit is saved in the database (looking via sql), but is not applied to the search when I run it again from the search history form, giving different results: cgi-bin/koha/opac-search.pl?idx=kw&q=vermeer I would expect something with: kw,wrdl: vermeer yr,st-numeric=-2005 Changing status
Alex has worked on this and should send an updated patch soon
Created attachment 5422 [details] [review] Proposed patch
*** Bug 4029 has been marked as a duplicate of this bug. ***
Created attachment 5659 [details] [review] Signed-off patches, squashed Tested several combinations of advanced searches including keyword types, publication date ranges, itemtype limits, location, and sort. All search history results matched the results of the original search.
This patch is already SIGNED, but unfortunately does not apply. Please rebase. Applying: Bug 5981 : (MT 4231) Adds limits to search history /usr/share/koha/testclone/.git/rebase-apply/patch:101: trailing whitespace. ADD `limit_cgi` VARCHAR( 255 ) NULL DEFAULT NULL AFTER `limit_desc` warning: C4/Auth.pm has type 100755, expected 100644 error: patch failed: C4/Auth.pm:130 error: C4/Auth.pm: patch does not apply error: patch failed: installer/data/mysql/updatedatabase.pl:4446 error: installer/data/mysql/updatedatabase.pl: patch does not apply Using index info to reconstruct a base tree... <stdin>:101: trailing whitespace. ADD `limit_cgi` VARCHAR( 255 ) NULL DEFAULT NULL AFTER `limit_desc` warning: 1 line adds whitespace errors. Falling back to patching base and 3-way merge... Auto-merged C4/Auth.pm CONFLICT (content): Merge conflict in C4/Auth.pm Auto-merged C4/Search.pm Auto-merged installer/data/mysql/kohastructure.sql Auto-merged installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merged koha-tmpl/opac-tmpl/prog/en/modules/opac-search-history.tt Auto-merged opac/opac-search.pl Failed to merge in the changes.
Created attachment 10087 [details] [review] Bug 5981 - OPAC: Add limits to search history
This patch does the same thing than the previous one but it should apply.
Created attachment 10316 [details] [review] [SIGNED-OFF] Bug 5981 - OPAC: Add limits to search history Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Fixed a conflict in updatedatabase and removed kohaversion.pl from the patch. Patch works nicely, tested with different searches and limits. Note: Sort options don't get into the history still.
Created attachment 10632 [details] [review] [SIGNED-OFF] Bug 5981 - OPAC: Add limits to search history Rebased patch + reindentation (replaces \t with 4 spaces) Works as expected. Marking as Passed QA
Bugfix, pushed to 3.8.x will be in 3.8.3
Reverted out of 3.8.x this breaks the opac search navigation, see bug 8381.
There are some strange side effects of this patch on master too. It all resumes to working normally as soon as I turn opacsearchhistory off or revert this patch out. Symptoms: - Every search seems to freeze the browser, but after a minute or so the search result page appears - Paging works for me, but at a 1 page/minute speed I tried lots of different things before I figured out it was this patch making the difference: - rebuilt my indexes several times - ran Makefile to make sure I have all current zebra files - changed from grs to dom indexing - changed to icu None of that worked. I have no idea what's going on here, but I think reverting out of master would be good until we figure it out.
(In reply to comment #28) > There are some strange side effects of this patch on master too. It all > resumes to working normally as soon as I turn opacsearchhistory off or > revert this patch out. > > Symptoms: > - Every search seems to freeze the browser, but after a minute or so the > search result page appears > - Paging works for me, but at a 1 page/minute speed Katrin, I can't reproduce your problem, it works well for me, I just tested again. Is it still relevant for you and do you have something in your setup (plack or no ? zebra DOM or GRS1 ? how many records indexed ?adv search is working or also very slow ? anything in your logs ? (I'm also Investigating 8381)
Hi Paul, nothing in the logs - I know, there should probably be something. I started with grs, switched to DOM and ICU. The problem remained. I did a keyword search for "der" (German article). Staff search worked without any problems (no search history), the problem only occured in OPAC and vanished after deactivating the search history or reverting the patch. I did sign off on this patch and I am sure it was not there when I initially tested it - but it's a big problem now. I suspect something changed in between that is causing the problem.
(In reply to comment #30) > I did sign off on this patch and I am sure it was not there when I initially > tested it - but it's a big problem now. I suspect something changed in > between that is causing the problem. really weird... let's wait until Alex find a fix for bug 8381, maybe they are related ? (even if I don't see how they could be)
No Plack, but memcache - restarted it several times. Maybe that is it?
(In reply to comment #32) > No Plack, but memcache - restarted it several times. Maybe that is it? Should not be memcache, but I have no memcache by default. Could you try after shutting down memcache server ?
I can try that tonight.
Hi Paul and Jonathan, I checked the search worked for me on current master with search history deactivated. Next I stopped the memcache server and reactivated the search history using the system preference. I still see the same systems - the browser seems to freeze, it takes over a minute, then I get to see a result list. I noticed something else, that is weird: on the result list the first line with the title of the first result on the result list is missing. It's also missing on other the other pages of the same search. XSLT results list is activated and used.
Pushed to master, follow-up in bug 8381 also pushed, resolving this one, chris_c, you can push both on 3.8.x I think
Either this patch or the patch to bug 8381 has broken the OPAC search in Chrome. With EnableOPACSearchHistory enabled, Chrome crashes on a regular keyword search (q=smith), and hangs on searches using idx=kw&q=smith. Disabling EnableOPACSearchHistory fixes the problem.
(In reply to comment #37) > Either this patch or the patch to bug 8381 has broken the OPAC search in > Chrome. With EnableOPACSearchHistory enabled, Chrome crashes on a regular > keyword search (q=smith), and hangs on searches using idx=kw&q=smith. > Disabling EnableOPACSearchHistory fixes the problem. mmm... strange, I can't reproduce this problem. I use chromium shipped with Ubuntu: 18.0.1025.168 (Build de développement 134367 Linux) Ubuntu 12.04 I don't think i've any plugin installed (I don't use chrome daily) I played with some searches, history page, running a search from history,... Could you describe exactly what you were doing ?
Jared could you add some more information or, close this if the problem has gone away in master please.
The hanging is no longer an issue on Master.