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

django-tastypie REST API for django-polls #7

Open
wants to merge 40 commits into
base: master
Choose a base branch
from

Conversation

miraculixx
Copy link

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

  1. Polls can be added through the admin. Each poll has 1-n options that can be voted for.
  2. Votes can be given through REST API. Each poll can define: require logged in user or anonymous, allow multiple votes by same user or restrict to one user/one vote.
  3. Poll results (statistics) can be retrieved through REST API

Tasks

  1. Implement the poll app for Django 1.6 (see details below)
  2. Add the tastypie API
  3. Implement the statistics calculation as per below description
  4. Add unit tests for the API client

Implemenation notes

Basics

Model extension

  • The Poll model must be extended as follows:
    • a text field reference (unique, char20), which is to hold a client-specified reference. The default is a
      uuid4().
      • a start_votes timestamp // earliest time votes get accepted
    • a end_votes timestamp // latest time votes get accpeted
    • a closed boolean // default = False. once closed is set, no more votes are accepted
    • important the Poll model should be used to cast votes => Poll.vote(choice, user). This will check that start_votes, end_votes and closed flags are satisfied. Raise an PollNotOpen (current time < start_votes) or PollClosed (end_votes < current time, closed==True) exception accordingly.
  • The Vote model must be extended as follows:
    • a text field comment (unique, char144)
    • a datetime timestamp
    • a data field for JSON storage of arbitrary data (JSONField).

Statistics calculation

  • The statistics calculated will, for now, be limitted to data required for histogram graphs (i.e. for each choice there is one statistics: the percentage of votes for this choice)
  • Make sure the poll model has appropriate indexes to speed up statistics calculation
  • For each poll, calculate
    • the total number of votes for the poll nV (i.e. poll.votes.count())
    • for each choice the number of votes nC (i.e. for each choice in poll, choice.votes.count())
    • => percentage for each choice pC = nC / nV (floating point)
  • allow for an option to set "unique users only" => only one vote per user.

API

* Api("v1/poll")
* POST /poll/   -- create a new poll, shall allow to post choices in the same API call 
* POST /choice/ -- add a choice to an existing poll
* POST /vote/<pk>/ -- vote on poll with pk 
* PUT /choice/<pk>/ -- update choice data
* PUT /poll/<pk>/ -- update poll data
* GET /poll/<pk>/ -- retrieve the poll information, including choice details
* GET /result/<pk>/ -- retrieve the statistics on the poll. This shall return a JSON formatted like so. Note the actual statistics calculation shall be implemented in `poll.service.stats` (later on, this will be externalized into a batch job).

results

{
   "stats": {
       "values": [ n, ... ], // for each choice the percentage value of responses as decimal (0.1 = 10%, 0.2=20% etc.),
       "labels": [ "text", ... ], // for each choice the choice label text  
       "users": #, // count of unique users,
       "votes": #, // len of values
   },
   "poll" : <the poll uri>,
   "resource" : <the result uri>
}

armicron and others added 23 commits February 17, 2015 16:07
…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.
@noxan noxan self-assigned this Mar 15, 2015
@noxan
Copy link
Member

noxan commented Mar 15, 2015

an impressive pull request, thanks for your efforts in advance! - I will have closer look tomorrow :)

miraculixx and others added 5 commits March 18, 2015 17:32
* 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
@noxan
Copy link
Member

noxan commented Apr 4, 2015

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:

  • if there is a way then turn the api or the tastypie dependency to optional (some might not need it and there is no way to decide on this without patching or not using the urls.py)
  • try to split the features of the model in multiple classes or abstract models to allow subsets of the current feature set or to be modular at least (compare to https://github.com/byteweaver/django-posts/blob/master/posts/models.)

cheers, noxan

@miraculixx
Copy link
Author

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.

no need to apologize. things take time...

If it's possible I would like to hear your opinion first

sure, thanks for asking.

if there is a way then turn the api or the tastypie dependency to optional

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.

try to split the features of the model in multiple classes or abstract models to allow subsets of the current feature set or to be modular at least

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).

@noxan
Copy link
Member

noxan commented Jun 6, 2015

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 :)

miraculixx and others added 9 commits March 28, 2016 22:32
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
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.

3 participants