Bug 2809 - Some search problems
Summary: Some search problems
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: PATCH-Sent (DO NOT USE) normal
Assignee: Henri-Damien LAURENT
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-18 08:46 UTC by Henri-Damien LAURENT
Modified: 2012-10-26 00:41 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
This patch fixes that bug. (2.24 KB, patch)
2008-11-18 20:47 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 2809 : Rebased and Reformatted for master, please test thoroughly (2.67 KB, patch)
2011-07-07 21:05 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 00:56:40 UTC


---- Reported by henridamien@koha-fr.org 2008-11-18 20:46:46 ----

When not using rewrite rules, then empty limits are still used in the query and makes it fail.
If limit was null string, it still added "and".

indexes_set was set on yr and date searches.
This lead to search date string in all the fields (index was forgotten)



---- Additional Comments From henridamien@koha-fr.org 2008-11-18 20:47:38 ----

Created an attachment
This patch fixes that bug.





--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:56 UTC  ---

This bug was previously known as _bug_ 2809 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=2809
Imported an attachment (id=861)

Actual time not defined. Setting to 0.0
The original submitter of attachment 861 [details] [review] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.

Comment 1 Chris Cormack 2010-12-15 01:29:32 UTC
This patch doesnt look like it was ever pushed
Comment 2 Marcel de Rooy 2011-01-31 08:45:38 UTC
../rma/signoff/bug2809.patch:55: trailing whitespace.
            }
error: patch failed: C4/Search.pm:950
error: C4/Search.pm: patch does not apply

Changing patch status.
Comment 3 Chris Cormack 2011-07-07 21:05:36 UTC
Created attachment 4580 [details] [review]
Bug 2809 : Rebased and Reformatted for master, please test thoroughly
Comment 4 Chris Cormack 2011-07-07 21:06:36 UTC
New patch created for master, no guarantee it works
Comment 5 Paul Poulain 2011-08-02 16:11:53 UTC
"no guarantee it works" => back to needs signoff
Comment 6 Jonathan Druart 2011-08-05 08:34:29 UTC
"indexes_set was set on yr and date searches."

lines 'indexes_set++' are commented, then this unless condition

unless ( $indexes_set || !$index || $index =~ /(st-|phr|ext|wrdl|nb|ns)/ ) {
    $struct_attr = ",wrdl";
}

become useless, right ?
Comment 7 Katrin Fischer 2011-08-14 16:16:01 UTC
Please provide a test plan for this - I have no idea what it is supposed to fix or do.
Comment 8 Marcel de Rooy 2011-09-02 07:09:02 UTC
GBSD: I am checking this one now..
Comment 9 Marcel de Rooy 2011-09-02 08:34:59 UTC
GBSD#4 comments including some QA:

First, this patch does not have a test plan; it is very hard to know if it actually makes a difference in current Koha.

Second, the patch comments the indexes_set++ for indexes yr and acqdate; this should not be done! It actually make some searches resultless.
Test something like: yourserver/cgi-bin/koha/opac-search.pl?idx=yr&q=2005
Without the patch, it could have results.
Note however that this URL is not constructed as such by advanced search: advanced search adds a yr-limit string.

Third, the test "if ($this_limit){" should be extended to not only test for empty string but also spaces. If an empty &limit= should be appended after a search URL, it does not even hit that test.
If I append &limit=%20, it makes the search crash. This test could eliminate that (rather fictitious) situation.

My preliminary conclusion: this patch is actually not needed anymore in current Koha and in its current condition it fails QA. I would suggest to close this report. If I overlook something, please provide examples for current master to demonstrate it.
Comment 10 Paul Poulain 2011-09-02 09:29:40 UTC
* very old entry
* no test plan
* no one understand what it was supposed to fix

=> closing