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.

[blynch1@…]

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.

[cboos]

Good catch. A patch would be welcomed!

(hint: s/value or ''/value is 0 and '0' or value or ''/)

[blynch1@…]

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

[blynch1@…]

Patch against trunk to allow the numerical 0 to be displayed in SQL Reports

[cboos]

I doubt this will work...

>>> 0 is None and 0 or ''
''
>>> 1 is None and 1 or ''
''
>>> None is None and None or ''
''

[blynch1@…]

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

[blynch1@…]

Patch against trunk to allow the numerical 0 to be displayed in SQL Reports

[cboos]

Thanks for the patch, which was the starting point for the final fix r9202 (r9204 on trunk).

[cboos]

(forgot to close)