report 8 (active tickets, mine first) sorts owner wrong

[mailmaix@…]

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,

[rblank]

Replying to mailmaix@…:

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.

I assume that you mean the sorting of the second group "Active tickets" is wrong.

(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.)

Yes, that's no good either. I'll add this to my to-do list.

[rblank]

Patch against 0.11-stable fixing sorting by owner in report 8

[rblank]

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.

[W. Martin Borgert <debacle@…>]

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

[W. Martin Borgert <debacle@…>]

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

[rblank]

The report 8 has been fixed in [7765]. This will only affect newly created environments. For existing environments, the following change has to be done by hand:

(owner = $USER) -> (COALESCE(owner, '') = $USER)

I'll also have a look at comment:6, stay tuned.

[rblank]

[7766] implements a slight variation of the patch in comment:6.