Closed Bug 510944 Opened 15 years ago Closed 15 years ago

Sorting bug lists on some fields can fail

Categories

(Bugzilla :: Query/Bug List, defect)

3.4.1
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 3.4

People

(Reporter: gregaryh, Assigned: LpSolit)

References

Details

Attachments

(1 file, 2 obsolete files)

In the case where a custom field name matches part of another field name, it can result in the following error: The custom sort order specified in your form submission contains an invalid column name.

This appears to be cause by the code in list/table.html.tmpl which removes items in the order list around line 137:

[% order = order.remove("$id( DESC)?,?") %]

If I have a custom field named cf_my_priority for example, and I have sorted on the regular priority field, the name of this custom field is munged in the order list to remove the 'priority' string leaving only cf_my_

It seems this ought to be fixable by doing a whole word match in the regex but I have not had success using modifiers like \b in the Template code.
Attached patch V1 (obsolete) — Splinter Review
OK, so it seems that placing \b inside the regex doesn't work directly, but concatenation with the regex does... Wierd.
Assignee: query-and-buglist → ghendricks
Attachment #394871 - Flags: review?(mkanat)
Attached patch Alternate (obsolete) — Splinter Review
This works too. I guess I should have tried that first. 
Pick one. :)
Attachment #394880 - Flags: review?(mkanat)
Yes, this error was mentioned in bug 164009 comment 26. Your "alternate" patch is the way to go.
Blocks: 164009
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86 → All
Target Milestone: --- → Bugzilla 3.4
Attachment #394871 - Attachment is obsolete: true
Attachment #394871 - Flags: review?(mkanat)
Comment on attachment 394880 [details] [diff] [review]
Alternate

If I click twice on the "assignee realname" header, I still get the error, with the "assignee" field also displayed.
Attachment #394880 - Flags: review?(mkanat) → review-
Attached patch patch, v2Splinter Review
I hope you don't mind if I take this bug, but I was already working on this issue as part of my initial patch in bug 491467 (which had a syntax error, btw). The missing point in your patch was the way the trailing comma was handled. The current code is wrong as it looks for ",?", making the comma optional, which is a bad assumption. If the comma is not present, then we must be at the end of the string.
Assignee: ghendricks → LpSolit
Attachment #394880 - Attachment is obsolete: true
Attachment #394950 - Flags: review?(ghendricks)
Comment on attachment 394950 [details] [diff] [review]
patch, v2

I have no problem with you taking this. I am not very l33t with the regex so I am glad you caught. I just assumed that everything else was working with it when I patched it, but I guess I shouldn't assume huh. ;)
Attachment #394950 - Flags: review?(ghendricks) → review+
Flags: approval3.4+
Flags: approval+
tip:

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.46; previous revision: 1.45
done

3.4.1:

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.42.2.3; previous revision: 1.42.2.2
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Summary: Sorting bug lists on custom fields can fail → Sorting bug lists on some fields can fail
Blocks: 470214
You need to log in before you can comment on or make changes to this bug.