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

Debug behavior redux #6

Merged
merged 1 commit into from
Mar 18, 2012
Merged

Debug behavior redux #6

merged 1 commit into from
Mar 18, 2012

Conversation

leetrout
Copy link
Contributor

There are some bits in the testing that don't feel great. Either way this is a great jumping off point for what was in my head.

Let me know what you think...

Thanks!

@leetrout
Copy link
Contributor Author

leetrout commented Feb 4, 2012

Glad you got your electrons back :)

Thanks for taking the time to look at the PR. Yes, I totally agree the helper would be much cleaner. I tend to stuff things of that nature in utils.py (ala django). Purely a stylistic opinion.

The more I've been using this the past week the more I've been desiring a decorator with arguments that would allow this to be more flexible, especially on enforcing the post method.

@twilio_view
def my_view():
    # normal tag behavior

@twilio_view(method='GET')
def my_view():
    # doesn't enforce POST so Twilio will cache my response
    # could also just be POST=False


@twilio_view(blacklist=False)
def my_view():
    # doesn't check the blacklist

@justquick suggested a class based view which would ultimately prove more extensible (and I may develop it since I'm using 1.3 and see how it feels). This would allow methods like twilio_cbv.validate_request() which could call twilio_cbv.is_blacklisted() which could introspect on twilio_cbv.caller_model or similar so that by default it would work as expected now but I could override the model with one that matches the interface (has the phone number and blacklisted fields) or I could write my own custom is_blacklisted function. I've probably spent way too much time thinking about it...

rdegges pushed a commit that referenced this pull request Mar 18, 2012
@rdegges rdegges merged commit 771574f into rdegges:develop Mar 18, 2012
@rdegges
Copy link
Owner

rdegges commented Mar 18, 2012

Ok, so I just merged this in preparation for release :)

I'm going to open a separate issue about the GET / POST options in the decorator (which I think are definitely needed), so that we can move the conversation there.

I think that in the next stable release I'll shoot for the class based view approach, which would make things pretty sexy.

Thanks so much!

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

Successfully merging this pull request may close these issues.

2 participants