TicketQuery with format=progress causes red box

[anonymous]

I am testing ​th:ChildTicketsPlugin, which suggests the following use of TicketQuery:

[[TicketQuery(parent=#1,format=progress)]]

However, when the query is empty, this causes an error:

Error: Macro TicketQuery(parent=#1,format=progress) failed
'>' not supported between instances of 'NoneType' and 'int'

When I remove format=progress then the result simply displays No results (not as an error box).

See below for the log information generated.

2022-04-05 17:03:15,726 Trac[formatter] ERROR: Macro TicketQuery(parent=#1,format=progress) failed for <Resource 'ticket:2503'>:
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/trac/wiki/formatter.py", line 813, in _macro_formatter
    return macro.ensure_inline(macro.process(args), in_paragraph)
  File "/usr/local/lib/python3.10/dist-packages/trac/wiki/formatter.py", line 380, in process
    text = self.processor(text)
  File "/usr/local/lib/python3.10/dist-packages/trac/wiki/formatter.py", line 351, in _macro_processor
    return self.macro_provider.expand_macro(self.formatter, self.name,
  File "/usr/local/lib/python3.10/dist-packages/trac/ticket/query.py", line 1444, in expand_macro
    chrome.render_fragment(req, 'progress_bar.html', data),
  File "/usr/local/lib/python3.10/dist-packages/trac/web/chrome.py", line 1459, in render_fragment
    return self.render_template_string(template, data, text)
  File "/usr/local/lib/python3.10/dist-packages/trac/web/chrome.py", line 1550, in render_template_string
    string = template.render(data)
  File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 1291, in render
    self.environment.handle_exception()
  File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 925, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "/usr/local/lib/python3.10/dist-packages/trac/templates/progress_bar.html", line 32, in top-level template code
    }|htmlattr}>
  File "/usr/local/lib/python3.10/dist-packages/trac/util/presentation.py", line 172, in is_greaterthan
    return a > b
TypeError: '>' not supported between instances of 'NoneType' and 'int'

[Clemens Mosig]

I will have a look at this one.

[anonymous]

One way to fix this is replacing the default percentage of the intervals that are used to display the progress bars with 0 (was None). See this ​commit.

The initial error looked as follows

The issue is that templates/progress_bar.html is unable to handle None values for the percentage of intervals.

Now, with the implemented fix, a progress bar for a query that yields no tickets looks as follows:

It is definitely better than a python error message but it does not look especially pretty. One could try to display an empty progress bar here instead?

I would not opt for a "No Results" message because when using format=table we get an empty table and not "No Results". An empty progress bar would be more consistent.

Open to suggestions! What is the expected behavior? Or would handling None values in templates/progress_bar.html be more preferable?

[Clemens Mosig]

Replying to anonymous:

Forgot to but my name in there… —> Clemens

[Jun Omae]

Root cause is that behavior of > operator with None and an integer differs between Python 2 and 3.

Python 2.7.18 (default, Mar  8 2021, 13:02:45)
[GCC 9.3.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> None > 42
False
>>>

Python 3.9.12 (main, Apr 18 2022, 22:40:46)
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> None > 42
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: '>' not supported between instances of 'NoneType' and 'int'

We could fix templates (greaterthan(x) → x is none and greaterthan(x)), however functions which have the issue in trac.util.presentation are used in many templates. Also, we cannot fix plugins which used the functions in templates.

I consider that we should allow to pass None in the functions.

$ git grep '\<\(greaterthan\|greaterthanorequal\|lessthan\|lessthanorequal\)\>' -- '*.html'
trac/templates/attachment.html:          <label>File{% if max_size is greaterthanorequal 0 %}
trac/templates/diff_div.html:                clines[loop.index0] if loop.index0 is lessthan(len(clines))
trac/templates/diff_div.html:            #           set within_change = loop.index0 is lessthan(len(block.changed.lines))
trac/templates/diff_div.html:            #           set within_change = loop.index0 is lessthan(len(block.base.lines))
trac/templates/diff_div.html:        #     if loop.index is lessthan(len(item.diffs)):
trac/templates/diff_options.html:         value="${'all' if diff.options.contextlines is lessthan(0) else
trac/templates/diff_view.html:      # set multi = num_changes is greaterthan(1)
trac/templates/history_view.html:        # if len(history) is greaterthan(10):
trac/templates/progress_bar.html:                        interval.percent is greaterthan 0
trac/ticket/templates/ticket.html:                      'trac-new' if change_date is greaterthan(start_time) and
trac/ticket/templates/ticket_change.html:#   if not show_editor and len(change.comment_history) is greaterthan(1):
trac/ticket/templates/ticket_change.html:  #   if comment_version is greaterthan(0):
trac/ticket/templates/ticket_change.html:  #   if comment_version is lessthan(max_version):
trac/ticket/templates/ticket_change.html:  #   if comment_version is greaterthan(0):
trac/ticket/templates/ticket_preview.html:           'trac-new' if change_date is greaterthan(start_time) and
trac/timeline/templates/timeline.html:        #   set highlight = precision and precisedate and timedelta(0) is lessthanorequal(event_delta) and event_delta is lessthan(precision)
trac/versioncontrol/templates/changeset_content.html:  #   elif ndiffs + nprops is greaterthan(0):
trac/versioncontrol/templates/changeset_content.html:      #   if delta is lessthan(timedelta(0, 3600)):
trac/versioncontrol/templates/changeset_content.html:    #   if max_diff_bytes and diff_bytes is greaterthan(max_diff_bytes):
trac/versioncontrol/templates/changeset_content.html:    #   if max_diff_files and diff_files is greaterthan(max_diff_files):
trac/versioncontrol/templates/revisionlog.html:        # if len(items) is greaterthan(10) and len(item_ranges) == 1:
trac/wiki/templates/wiki_diff.html:        # if (new_version - old_version) is greaterthan(1):

[Jun Omae]

Proposed fix (unit tests should be added):

trac/util/presentation.py

@@ -174 +174 @@
     return a != b
 
 def is_greaterthan(a, b):
+    if b is None:
+        raise ValueError('Must not be `None`')
+    if a is None:
+        return False
     return a > b
 
 def is_greaterthanorequal(a, b):
+    if b is None:
+        raise ValueError('Must not be `None`')
+    if a is None:
+        return False
     return a >= b
 
 def is_lessthan(a, b):
+    if b is None:
+        raise ValueError('Must not be `None`')
+    if a is None:
+        return True
     return a < b
 
 def is_lessthanorequal(a, b):
+    if b is None:
+        raise ValueError('Must not be `None`')
+    if a is None:
+        return True
     return a <= b
 
 def is_in(a, b):

[anonymous]

Root cause is that behavior of > operator with None and an integer differs between Python 2 and 3. We could fix templates (greaterthan(x) → x is none and greaterthan(x)), however functions which have the issue in trac.util.presentation are used in many templates. Also, we cannot fix plugins which used the functions in templates.

I consider that we should allow to pass None in the functions.

Hmm, I see.

Why would we allow a to be None, but not b? In python 2 both x < None (returns False) and None < x (returns True) works. Wouldn't we want to replicate python 2 behavior completely if we allow None for one of the arguments?

[trac@…]

I'm going to try using Jun's patch. I have a use case for this feature now.

I understand there might be a more compatible change, but if this works for the progress bar use case, then it's fine with me. Everything can always be made better, but we have to start somewhere.

Thanks!

[Jun Omae]

Patch from the reporter solves the crash, however progress of the macro disappears. I'm going to fix it and add unit tests.

[Jun Omae]

Ah, the same has been pointed out from the reporter at comment:4….

[Jun Omae]

Replying to anonymous:

It is definitely better than a python error message but it does not look especially pretty. One could try to display an empty progress bar here instead?

I've filed #13564 as a new ticket because Trac 1.4.x has also the issue.

[Jun Omae]

Committed the original patch with unit tests in [17675]. Reconsidering it, I decided not to modify methods in presentation.py.