532511 - Knowledge Base results missing after first page

Status: VERIFIED FIXED

(support.mozilla.org :: Search, defect)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

STR:

1. Load http://support-stage.mozilla.org/search.php?q=Tastaturk%C3%BCrzel&lang=de&tags=&where=d&locale=de&advanced=1 (might need to be logged in)
2. Click "Next" or "2"

Expected Results:

Should go to the next page, if there are results; if not, it shouldn't show the pagination element at all

Actual Results:

"2" shows up but doesn't go anywhere

Comment 1

[James Socol [:jsocol, :james]]

The result count says 15, which should continue to page 2.

Comment 2

[Stephen Donner [:stephend] Not actively reading bugmail]

Easier URL (for English speakers): https://support-stage.mozilla.org/search.php?q=facebook&lang=en&tags=&where=d&locale=en-US&advanced=1

Comment 3

[James Socol [:jsocol, :james]]

Attachment: remove the duplicate slicing

The offset and results/page are already accounted for in the SphinxLib constructor, making this line unnecessary.

Comment 4

[Michael Morgan [:morgamic]]

Comment on attachment 415740 [details] [diff] [review]
remove the duplicate slicing

This makes sense to me.  Redundant code = no!

Comment 5

[James Socol [:jsocol, :james]]

r57193

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on https://support-stage.mozilla.org/search.php?locale=en-US&q=facebook&tags=&lang=en&where=d&advanced=1&offset=0 and http://support-stage.mozilla.org/search.php?locale=de&q=Tastaturk%C3%BCrzel&tags=&lang=de&where=d&advanced=1&offset=10; I'll do regression testing as well.

Comment 7

[James Socol [:jsocol, :james]]

Looks like simple search pagination is broken.

Comment 8

[James Socol [:jsocol, :james]]

Attachment: array_slicing on simple searches

The code that did the slicing was necessary for simple searches, but not for advanced document searches. I tested this with simple searches with where=f,d,all, and advanced with where=f,d. Advanced does not support where=all so it's not a meaningful case.

Comment 9

[Paul Craciunoiu [:paulc]]

Comment on attachment 415785 [details] [diff] [review]
array_slicing on simple searches

I think could just replace that new <if> condition with:
---
if ($search_parameters['where'] == SEARCH_WHERE_ALL)
---
Seems logically equivalent, right?

Comment 10

[James Socol [:jsocol, :james]]

(In reply to comment #9)
> (From update of attachment 415785 [details] [diff] [review])
> I think could just replace that new <if> condition with:
> ---
> if ($search_parameters['where'] == SEARCH_WHERE_ALL)
> ---
> Seems logically equivalent, right?

Just tried that and it didn't work for me. I'm wary of treating where=all and advanced=1 as completely exclusive possibilities. That's a dangerous assumption.

Comment 11

[Paul Craciunoiu [:paulc]]

Odd. They should be equivalent, because in validation where=all is treated as where=d for advanced search. See these lines:
---
$where_array = array(SEARCH_WHERE_D, SEARCH_WHERE_F);
if (!$is_advanced) $where_array[] = SEARCH_WHERE_ALL;
if (!in_array($search_parameters['where'], $where_array))
    if ($is_advanced) $search_parameters['where'] = SEARCH_WHERE_D;
    else              $search_parameters['where'] = SEARCH_WHERE_ALL;
---
Unfortunately I won't have time to look into this until later tomorrow. Anyway, if the patch works, that's fine.

Comment 12

[Michael Morgan [:morgamic]]

Comment on attachment 415785 [details] [diff] [review]
array_slicing on simple searches

This is to get the correct # of results?  If so, seems to work for me.

Comment 13

[James Socol [:jsocol, :james]]

r57215.

This should be returning correct results per page in both simple and advanced search.

Comment 14

[Vishal Kamdar [:vish_moz]]

Verified, FIXED.