worldcompany / djangoembed

rich media consuming and providing with django
http://djangoembed.readthedocs.org
MIT License
138 stars 38 forks source link

Problem with mock_request() function and Context Processors #13

Closed BenAtWide closed 12 years ago

BenAtWide commented 13 years ago

I am getting a template error when using OEmbed tags, which I believe is due to the behaviour of the mock_request() function in utils.py.

It seems that mock_request() does not add a user object to the request object when it runs the context processors, normally provided by django.contrib.auth.context_processors.auth. This means that context processors of other apps that depend on the existence of a user object throw an exception.

In my case I have django_messages installed, whose context processor requires a user object to be present on Request. When the template is rendered, I get:

Caught AttributeError while rendering: 'HttpRequest' object has no attribute 'user'

dryan commented 13 years ago

I just hit the same issue with a context processor looking for request.META['SERVER_PORT']

silentworks commented 12 years ago

Is this addressed as yet? I am getting the same error also, what is the workaround fix for this?

BenAtWide commented 12 years ago

I worked around this by editing the context processor so that it checked for the existence of the User object. But that is very much a workaround and not a fix - if I started using any similar context processors in the project I would have to edit those as well.

I wonder if it would be better to use some other method of passing context to the oEmbed rendering process than the mock_request() approach?

jserver commented 12 years ago

I worked around this issue by adding the context to the client.parse call of the render method in the OEmbedNode class.

in my case I had some extra context_processors being added to the render method for this particular view that needed things out of the request object. you can look at my fork to see the change.

silentworks commented 12 years ago

This is already fixed, have you taken a pull from the repository before adding your comments?

silentworks commented 12 years ago

look at issue #21 and you will see the pull request was merged that fixes this issue.

coleifer commented 12 years ago

Fixed

jserver commented 12 years ago

forked it today and was getting the error

jserver commented 12 years ago

I think I found the problem, if you use the tag in it's most basic form {% oembed %}{{ blah.oembed_url }}{% endoembed %} the height and width are set to None and the context is never added to the kwargs, which is why my patch worked adding the context explicitly to the parse call underneath. Setting the context in the kwargs outside of the if that checks height and width solves my problem as well and there probably should be a separate if check for template_dir as well. I created a patch in my fork that does the above.

Although I'm still confused on why you want to create a new RequestContext in the render_oembed method? this code breaks the use of the oembed filter since you can't have a context passed through. Our project adds things to the request in the middleware, those things are then used throughout are views and context_processors. But from what I can tell middlewares don't get run when you create a new RequestContext using a mock_request thus during creation of the request object it tries to run the context_processors but they will fail.

I created another patch that removes those context lines in render_oembed, the parse method underneath already creates a context if one isn't passed in and I just can't find a reason for the new RequestContext but maybe there is a good reason.