Bug 2809

Summary:	Some search problems
Product:	Koha	Reporter:	Henri-Damien LAURENT <henridamien>
Component:	Searching	Assignee:	Henri-Damien LAURENT <henridamien>
Status:	CLOSED INVALID	QA Contact:	Bugs List <koha-bugs>
Severity:	normal
Priority:	PATCH-Sent (DO NOT USE)	CC:	chris, jonathan.druart, m.de.rooy, paul.poulain, stephane.delaune
Version:	Main
Hardware:	All
OS:	All
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. Bug 2809 : Rebased and Reformatted for master, please test thoroughly

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