383643 - "Find specific bug" response does not allow to change list sorting type

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Dmitry Repkin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4
Build Identifier: Bugzilla 3.0

If I try to make a search using "Find specific bug" I will not be able to resort results. For example, I could not sort results by date or by status


Reproducible: Always

Steps to Reproduce:
1. Make a Quick Search (Currently renamed to "Find specific bug")
2. Look to list headers

Actual Results:  
Headers are black and doest not allow to resort the list

Expected Results:  
Headers should be clickable and allow to resort the list by coulmn

Broken in 3.0, works in 2.22

Comment 1

[Frédéric Buclin]

(In reply to comment #0)
> Broken in 3.0, works in 2.22

Is it really broken in 3.0? This is working fine using Bugzilla 3.0.2. But the bug really exists in Bugzilla 3.1.2+. Alex, is that something you saw while working on the "sort columns" bug?

Comment 2

[Alex Schuilenburg]

No.  

However, I can reproduce what is being seen and this is IMHO "NOTABUG". The behaviour is as documented and coded.  From the "Find specific bug" tab, the search is a full text search where results are always sorted by relevance. It clearly states this in the page and on the result page:

"Find a specific bug by entering words that describe it. Bugzilla will search
bug descriptions and comments for those words and return a list of matching 
bugs sorted by relevance."
     ^^^^^^^^^^^^^^^^^^^^

and in the results:

"Bugs on this list are sorted by relevance, with the most relevant bugs at 
                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
the top."

There is no "relevance" column so no way to switch the "sort_by_relevance" order. What the user really wants in this case is the "Advanced Search" where changing the sort order makes sense.

Comment 3

[Frédéric Buclin]

I agree that the text says it will sort by relevance. But as soon as you have the list displayed, you should then be free to sort the list based on any column.

Comment 4

[Alex Schuilenburg]

Fair enough.  In theory easily done in the template.  To do that you need to remove from template/en/default/list/table.html.tmpl:

        [% IF sorted_by_relevance %]
          ID
        [% ELSE %]
        ...
        [% END %]

and 

    [% IF sorted_by_relevance %]
      [%- abbrev.$id.title || field_descs.$id || column.title -%]
    [% ELSE %]
    ...
    [% END %]

retaining the code within the [% ELSE %] [% END %]

Since the template specifically outputs the column headings without the sort order modifying links I would have to ask why the template was written with the conditionals in the first place?

OOI, I produced a patch to remove the IF ELSE and it gives the behaviour desired. I am not sure what other side-effects it has though without the answer to the above question first.

Note that the changes I made to fix bug 23473 are all within the ELSE clause, so have no effect.

Comment 5

[Alex Schuilenburg]

Attachment: Patch to remove "sorted_by_relevance" conditional

See comment #4

Also modified indentation whitespace in the patch to cater for removal of IF ELSE.

Comment 6

[Frédéric Buclin]

(In reply to comment #4)
> Since the template specifically outputs the column headings without the sort
> order modifying links I would have to ask why the template was written with the
> conditionals in the first place?

Very good question. I have no idea and IMO is an old thing from 1999 or so. Having a more consistent behavior makes more sense.

Comment 7

[Frédéric Buclin]

Comment on attachment 288327 [details] [diff] [review]
Patch to remove "sorted_by_relevance" conditional

The patch doesn't apply cleanly:

patch -p0 --dry-run < /root/patches/bug383643.pat
patching file template/en/default/list/table.html.tmpl
Hunk #1 succeeded at 85 (offset -1 lines).
Hunk #2 FAILED at 129.
1 out of 2 hunks FAILED -- saving rejects to file template/en/default/list/table.html.tmpl.rej

Moreover, we should replace $vars->{'sorted_by_relevance'} = 1; by $vars->{'message'} = 'buglist_sorted_by_relevance'; in buglist.cgi at line 968 and remove the now useless [% message = "buglist_sorted_by_relevance" IF sorted_by_relevance %] directive in list/list.html.tmpl.

Note that this message should not be displayed as soon as you change the sort order. This may need some work in buglist.cgi.

Comment 8

[Alex Schuilenburg]

Attachment: Patch to remove "sorted_by_relevance" conditional

I agree.  I just tested the attached patch and it appears to work.  Feel free to apply or discard according to the answer :-)

I just noticed I missed your space removal from my patch to bug 23473, hence the revised patch.

Comment 9

[Frédéric Buclin]

Hum, are you sure this patch is different from the previous one? I still cannot apply it:

Hunk #1 succeeded at 85 (offset -1 lines).
Hunk #2 FAILED at 129.

Comment 10

[Alex Schuilenburg]

Attachment: Patch to remove "sorted_by_relevance" conditional

Sorry, I missed that you also removed the minus.

This one will definitely apply to the latest CVS.  Patch just created from it.

Comment 11

[Frédéric Buclin]

Comment on attachment 288330 [details] [diff] [review]
Patch to remove "sorted_by_relevance" conditional

>retrieving revision 1.6
>diff -U3 -r1.6 table.html.tmpl
>--- template/en/default/list/table.html.tmpl	31 Oct 2007 15:06:32 -0000	1.6

How can you have revision 1.6 as the last revision number in CVS is 1.38 for this file? I still get the same error.

Comment 12

[Alex Schuilenburg]

Attachment: Patch to remove "sorted_by_relevance" conditional

Argh!  Humble apologies.  I don't know how that happened - I appear to have picked up the old patch. This time I have checked it was the correct file. :-(

Comment 13

[Frédéric Buclin]

Comment on attachment 288334 [details] [diff] [review]
Patch to remove "sorted_by_relevance" conditional

Your patch is working fine, but you still have to drop the message at the top of the buglist when sorting the list. One way to do it is to look at $cgi->param('order') and check if it starts with 'relevance'. If yes, set $vars->{'message'} as described in comment 7, else leave it undefined.

Comment 14

[Frédéric Buclin]

ping?

Comment 15

[Alex Schuilenburg]

pong.  Been on extended holiday - years overdue.  I'll finish this off once I have cleared my "paid-for" work backlog.

-- Alex

Comment 16

[Alex Schuilenburg]

Attachment: Patch to remove "sorted_by_relevance" conditional

Patch updated to drop the message at the top of the buglist when sorting the list as described in comment 13 with additional cleanup per comment 7.

Comment 17

[Frédéric Buclin]

Comment on attachment 296531 [details] [diff] [review]
Patch to remove "sorted_by_relevance" conditional

Looks good. r=LpSolit

Comment 18

[Frédéric Buclin]

Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.368; previous revision: 1.367
done
Checking in template/en/default/list/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.59; previous revision: 1.58
done
Checking in template/en/default/list/table.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/table.html.tmpl,v  <--  table.html.tmpl
new revision: 1.39; previous revision: 1.38
done