Opened 3 years ago

Closed 3 years ago

#17675 closed Bug (fixed)

using date filter with regroup tag not working with 1.4 TZ

Reported by: ptone Owned by: aaugustin
Component: Core (Other) Version: 1.4-alpha-1
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If I try to use the date filter with regroup like so

{% regroup page_list by created|date:"B" as months %}

where page_list is a QS of PageRevision objects, I get an attribute error

'PageRevision' object has no attribute 'use_tz'

The issue seems to be that when localtime is called (inside template/base.py - resolve) with context.use_tz the context at this point is a PageRevision instance

I'm not sure, but perhaps this is some interaction of the way regroup uses a lambda and resolve?

Here is the tail of the traceback:

File "/Users/preston/Projects/Python/virtualenvs/lcah-apps/lib/python2.7/site-packages/django/template/debug.py" in render_node
  74.             return node.render(context)
File "/Users/preston/Projects/Python/virtualenvs/lcah-apps/lib/python2.7/site-packages/django/template/defaulttags.py" in render
  301.             groupby(obj_list, lambda v, f=self.expression.resolve: f(v, True))
File "/Users/preston/Projects/Python/virtualenvs/lcah-apps/lib/python2.7/site-packages/django/template/defaulttags.py" in <lambda>
  301.             groupby(obj_list, lambda v, f=self.expression.resolve: f(v, True))
File "/Users/preston/Projects/Python/virtualenvs/lcah-apps/lib/python2.7/site-packages/django/template/base.py" in resolve
  598.                 obj = localtime(obj, context.use_tz)

Exception Type: AttributeError at /wiki/special/recent/
Exception Value: 'PageRevision' object has no attribute 'use_tz'

If this is verified - it would be a 1.4 regression

Attachments (2)

17675.patch (3.6 KB) - added by aaugustin 3 years ago.
17675.2.patch (4.7 KB) - added by aaugustin 3 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Severity changed from Normal to Release blocker

comment:2 Changed 3 years ago by aaugustin

  • Owner changed from nobody to aaugustin

comment:3 Changed 3 years ago by ptone

Note that after resolving this, I realize that "B" is unsupported by the date filter, but that is not material to the bug. Using "b" as the date filter results in the same problem.

I can't seem to puzzle this out past this line in template/defaulttags.py:

303: groupby(obj_list, lambda v, f=self.expression.resolve: f(v, True))

comment:4 Changed 3 years ago by aaugustin

I can see how this bug happens: FilterExpression.resolve is called with items of the iterator you're grouping as the first argument, but it expects a context as the first argument.

I'm also struggling to figure out this very cryptic line :/

Last edited 3 years ago by aaugustin (previous) (diff)

comment:5 Changed 3 years ago by akaariai

Isn't that using a kwarg in a very non-necessary fashion? That line is doing:

def somef(v):
    return self.expression.resolve(v, True)
groupby(obj_list, somef)

Now, wouldn't be a surprise if the above is wrong. As said, the line is a bit cryptic...

comment:6 Changed 3 years ago by aaugustin

Indeed, it looks very wrong to me to call resolve with something that isn't a Context as the first positional argument (context). I'm currently refactoring the implementation of the regroup tag to avoid this.

comment:7 Changed 3 years ago by aaugustin

  • Has patch set
  • Triage Stage changed from Unreviewed to Accepted

Changed 3 years ago by aaugustin

comment:8 Changed 3 years ago by aaugustin

Disclaimer: the solution I used in this patch is quite hackish, but the previous one was even worse, and the semantics of the regroup tag themselves are hackish: they require the equivalent of Python's getattr, but with the behavior of the DTL's context lookups.

comment:9 Changed 3 years ago by akaariai

Looking quickly through the patch it doesn't seem that hackish to me. However, I would like if there was a comment of _why_ the context resolving dance is done.

comment:10 Changed 3 years ago by aaugustin

  • Triage Stage changed from Accepted to Ready for checkin

The same problem has "always" existed for template filters that have needs_autoescape = True. However, none of these are likely to be used in a {% regroup %} tag, so the bug went undetected. Since I added support for time zones, the problem also occurs for template filters that have expects_localtime = True: |date and |time, making it effectively a regression and a release blocker.

I'm uploading a new patch that adds a contrived example ('regroup04') in the tests for the shake of completeness, and more comments in the implementation of {% regroup %}. I believe it's good to go, I'm putting it in RFC in case someone wants to take a look or commit it.

Changed 3 years ago by aaugustin

comment:11 Changed 3 years ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed

In [17522]:

Fixed #17675 -- Changed the implementation of the {% regroup %} template tag to use the context properly when resolving expressions.

Note: See TracTickets for help on using tickets.
Back to Top