Opened 12 years ago

Closed 12 years ago

#17675 closed Bug (fixed)

using date filter with regroup tag not working with 1.4 TZ

Reported by: Preston Holmes Owned by: Aymeric Augustin
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 Aymeric Augustin 12 years ago.
17675.2.patch (4.7 KB ) - added by Aymeric Augustin 12 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 by Aymeric Augustin, 12 years ago

Severity: NormalRelease blocker

comment:2 by Aymeric Augustin, 12 years ago

Owner: changed from nobody to Aymeric Augustin

comment:3 by Preston Holmes, 12 years ago

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 by Aymeric Augustin, 12 years ago

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 12 years ago by Aymeric Augustin (previous) (diff)

comment:5 by Anssi Kääriäinen, 12 years ago

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 by Aymeric Augustin, 12 years ago

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 by Aymeric Augustin, 12 years ago

Has patch: set
Triage Stage: UnreviewedAccepted

by Aymeric Augustin, 12 years ago

Attachment: 17675.patch added

comment:8 by Aymeric Augustin, 12 years ago

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 by Anssi Kääriäinen, 12 years ago

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 by Aymeric Augustin, 12 years ago

Triage Stage: AcceptedReady 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.

by Aymeric Augustin, 12 years ago

Attachment: 17675.2.patch added

comment:11 by Aymeric Augustin, 12 years ago

Resolution: fixed
Status: newclosed

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