Opened 17 years ago
Closed 16 years ago
#7512 closed defect (fixed)
report 8 (active tickets, mine first) sorts owner wrong
| Reported by: | Owned by: | Remy Blank | |
|---|---|---|---|
| Priority: | low | Milestone: | 0.11.7 |
| Component: | report system | Version: | |
| Severity: | normal | Keywords: | |
| Cc: | mailmaix@…, rrosauro@… | Branch: | |
| Release Notes: | |||
| API Changes: | |||
| Internal Changes: | |||
Description
Report 8 (active tickets, mine first) is sorting wrong. It first shows my tickets, then tickets owned by others, and then those with no owner. This might be useful, but it is not the intended behavior.
(And, additionally, some of the tickets have '' as owner, some have None, though they both have no owner. So some of the no-owner tickets are within the tickets with owner.)
This is caused by this line:
ORDER BY (owner = $USER) DESC, …
If owner is $USER, this returns 1, if it is something else, it returns 0. But if owner is NULL, it returns NULL.
It must be replaced by the following:
ORDER BY (CASE (owner = $USER) WHEN 1 THEN 1 ELSE 0 END) DESC,
Attachments (3)
Change History (18)
comment:1 by , 17 years ago
| Cc: | added |
|---|
comment:2 by , 17 years ago
| Cc: | added |
|---|
comment:3 by , 17 years ago
| Milestone: | → 0.11.3 |
|---|---|
| Owner: | set to |
by , 17 years ago
| Attachment: | 7512-report8-r7613.patch added |
|---|
Patch against 0.11-stable fixing sorting by owner in report 8
comment:4 by , 17 years ago
The patch above fixes report 8 so that NULL values don't mess up the sorting. It uses the function COALESCE() instead of the proposed CASE, though.
That some unassigned owners display as "None", whereas others display as the empty string is due to the data in the database: a NULL value translates to "None", and an empty string is returned as-is. I agree this is not intuitive, and the query module does it differently (NULL → "—" except for grouping, where NULL → "None"), but I believe this shouldn't be changed, as it risks breaking lots of reports that are out there.
If you would rather have an empty string for NULL values, replace owner with COALESCE(owner, '') as owner in the query.
comment:5 by , 17 years ago
Maybe it would be wise, if trac would leave an unset milestone field completely empty in the reports, no matter whether this is internally stored as a NULL value or as an empty string. (Or component or version.) "None" might look like "something" to the non-Pythonist.
This works for me:
--- report.py.orig 2008-11-14 13:25:14.000000000 +0100 +++ report.py 2008-11-14 13:32:55.000000000 +0100 @@ -406,7 +406,10 @@ for header_group in header_groups: cell_group = [] for header in header_group: - value = unicode(result[col_idx]) + if result[col_idx]: + value = unicode(result[col_idx]) + else: + value = "" cell = {'value': value, 'header': header, 'index': col_idx} col = header['col'] col_idx += 1
follow-up: 11 comment:6 by , 17 years ago
This is shorter than the previous patch and handles CSV/TSV as well:
--- report.py.orig 2008-11-14 13:25:14.000000000 +0100 +++ report.py 2008-11-14 17:12:01.000000000 +0100 @@ -406,7 +406,7 @@ for header_group in header_groups: cell_group = [] for header in header_group: - value = unicode(result[col_idx]) + value = unicode(result[col_idx] or "") cell = {'value': value, 'header': header, 'index': col_idx} col = header['col'] col_idx += 1 @@ -666,7 +666,7 @@ for row in rows: row = list(row) for i in xrange(len(row)): - row[i] = converters[i](row[i]).encode('utf-8') + row[i] = converters[i](row[i] or "").encode('utf-8') writer.writerow(row) raise RequestDone
comment:7 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:9 by , 16 years ago
Not to bring up something so old, but it looks like the fix (in r7766 - specifically value = unicode(result[col_idx] or "")) actually causes another issue where the number 0 is not displayed in reports. For example:
select 0 as 'Number' from ticket will not display 0
but
select 1 as 'Number' from ticket will display 1
This seems to only be an issue with the value 0, no other value.
I came across it trying to sum values from tables and discovered that anything that summed to 0 would not display.
comment:10 by , 16 years ago
| Milestone: | 0.11.3 → 0.11.7 |
|---|---|
| Resolution: | fixed |
| Status: | closed → reopened |
Good catch. A patch would be welcomed!
(hint: s/value or ''/value is 0 and '0' or value or ''/)
comment:11 by , 16 years ago
Here's a patch against trunk, and will attach the file in a moment.
--- report.py 2010-02-17 13:16:36.000000000 -0500 +++ 7512-r9194.py 2010-02-17 13:19:25.000000000 -0500 @@ -453,7 +453,8 @@ for header_group in header_groups: cell_group = [] for header in header_group: - value = unicode(result[col_idx] or '') + value = unicode(result[col_idx] is None and + result[col_idx] or '') cell = {'value': value, 'header': header, 'index': col_idx} col = header['col'] col_idx += 1
by , 16 years ago
| Attachment: | 7512-r9194.patch added |
|---|
Patch against trunk to allow the numerical 0 to be displayed in SQL Reports
comment:12 by , 16 years ago
I doubt this will work…
>>> 0 is None and 0 or '' '' >>> 1 is None and 1 or '' '' >>> None is None and None or '' ''
comment:13 by , 16 years ago
Ah. Sorry, I had working code under non-working code when testing, and submitted the wrong one. This I think is more in line with what you were suggesting.
--- report.py 2010-02-17 13:16:36.000000000 -0500 +++ 7512-r9194.py 2010-02-17 14:41:09.000000000 -0500 @@ -453,7 +453,8 @@ for header_group in header_groups: cell_group = [] for header in header_group: - value = unicode(result[col_idx] or '') + value = unicode(result[col_idx] is 0 and '0' or + result[col_idx] or '') cell = {'value': value, 'header': header, 'index': col_idx} col = header['col'] col_idx += 1
by , 16 years ago
| Attachment: | 7512-r9194.2.patch added |
|---|
Patch against trunk to allow the numerical 0 to be displayed in SQL Reports



Replying to mailmaix@…:
I assume that you mean the sorting of the second group "Active tickets" is wrong.
Yes, that's no good either. I'll add this to my to-do list.