Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How Could the Response be None? #45

Closed
href opened this issue Feb 6, 2015 · 6 comments
Closed

How Could the Response be None? #45

href opened this issue Feb 6, 2015 · 6 comments
Assignees
Labels

Comments

@href
Copy link

href commented Feb 6, 2015

We sometimes have the problem that adding a new comment won't redirect to the conversation. The reason seems to be this code:

I figure this is a long shot, but does anyone have any clue why the response could ever be None? Everything else seems to work fine. Even if the response is None, the comment is actually added.

Maybe there's a more robust way of accessing the response?

Anyway, if anyone has any clues I'll return the favor with a brew or pizza ;)

@href href added the question label Feb 6, 2015
@sureshvv
Copy link
Contributor

sureshvv commented Feb 6, 2015

Looks bogus to me too.

Whole stuff can be replaced with:

context.REQUEST.RESPONSE.redirect(...)

IIRC, that is how it used to be when I last noticed it.

@sureshvv
Copy link
Contributor

sureshvv commented Feb 6, 2015

I made said change and all tests pass. This code may be hiding some other insidious error. I checked the old SVN. Looks like this code goes way back.

Can @cekk @keul @jensens @hvelarde comment?

@sureshvv sureshvv self-assigned this Feb 6, 2015
@keul
Copy link
Member

keul commented Feb 6, 2015

@sureshvv @href no bloody idea, but I think that all that stuff can be replaced as you suggested.
Mantainers of ploneboard will probably appreciate any pull request 👍

@href
Copy link
Author

href commented Feb 6, 2015

Oh good it's not just me. Unfortunately I can't reproduce this locally. This only seems to be None at times on production. For all I know this might not even be the fault of Ploneboard. Also, it does only seem to affect users with less than admin permissions, though I can't be sure that's a factor.

sureshvv added a commit that referenced this issue Feb 7, 2015
@sureshvv sureshvv closed this as completed Feb 7, 2015
@jean
Copy link
Member

jean commented Feb 7, 2015

For what it's worth, the response is not None arrived in this commit by @optilude: ed8e672

@jensens
Copy link
Member

jensens commented Feb 19, 2015

Beside the topic: First I would convert the script into a view-class. I stopped fighting with Python-Script side effects. It's impossible to debug in a good manner (yeah, I know the tools).

Checking Request for None is indeed strange. Even in async scenarios and tests the request should be mocked. So I think its save to remove the check. IMO its not the problem of the script, but of the caller, to provide an acquired request.

But all this ambiguity will go if its proper code in a view class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants