-
Notifications
You must be signed in to change notification settings - Fork 20
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
django-tastypie REST API for django-polls #7
base: master
Are you sure you want to change the base?
Conversation
…efined on the table 'polls.polls_poll' on mysql backend
* uri was empty => namespaced tastypie uri - http://stackoverflow.com/a/27162751 * support poll details and result by reference and pk
a job well done. thank you.
small improvements
an impressive pull request, thanks for your efforts in advance! - I will have closer look tomorrow :) |
* votes on anonymous polls don't store user * choices add a mnemonic code field for easier choice reference
* new method get_stats to keep labels and values in sync on /result * tests for invalid votes * tests with data on votes * indexing on votes
various fixes and improvements
first of all please excuse the delay of my response, it took me a lot more time than I expected to go through all your changes. In general I will merge your pull request soon, but I wanted to let you know about some improvements or wishes of mine I had been thinking about. If it's possible I would like to hear your opinion first, before it feels like I do modify all of your code after merging. I also would assign the task to work on those to myself. My thoughts on possible improvements:
cheers, noxan |
no need to apologize. things take time...
sure, thanks for asking.
not using the urls.py should essentially disable/not load it, but I haven't tested this. The better way would to be put the api in its own app, say 'polls_api' (keep it in the same repository, just another Django app alongside polls) so it is only loaded if listed in INSTALLED_APPS. In either case tastypie shouldn't be required.
Which features would you want to split off? Personally I like the simplicity of the model right now - the few extensions there are to enable a sensible API in a public facing poll (e.g. anonymous or not). |
First I am about to cherry pick a few of your commits in order to release a django 1.6 compatible version. Your idea regarding the urls.py sounds perfect, I will try to get it working based on your source code. I am still just struggling with all the additional features, I would like to integrate all of them. Especially because you already put a lot of effort into. But on the other side I would like to keep the application as simple, generic and extendable as possible. Maybe I find a way to split it the feature / the model into different mixins like in django-posts (https://github.com/byteweaver/django-posts/blob/master/posts/models.py) I will keep you updated :) |
various fixes and improvements
fixes: - adopt authorization to new tastypie logic (DjangoAuthorization, default ReadOnlyAuthorization) - limit anonymous voting by ip or clientid in cookie
migrate to django 1.7, fixed bugs, increased stability
accept votes by poll reference
closes #8
allow multiple votes
Extension with django-tastypie. There is a sample client application at https://github.com/miraculixx/poll. The specs for this PR and the client API by @miraculixx, implementation mostly by @armicron. Specs included below for transparency.
Expected behavior
Tasks
Implemenation notes
Basics
Model extension
Poll
model must be extended as follows:reference
(unique, char20), which is to hold a client-specified reference. The default is auuid4().
Vote
model must be extended as follows:comment
(unique, char144)Statistics calculation
API
results