enewe101 / digidemo

digital democracy engine
1 stars 0 forks source link

Use F() to register votes #18

Closed enewe101 closed 9 years ago

enewe101 commented 10 years ago

Voting creates a race condition. When a vote is being handled by the server, the object that got voted on (say a letter) is pulled out of the database, its score is read, and then the score is incremented and put back in the database. If two votes are processed at the same time, one vote might not get registered.

Note: this doesn't have to do with the value of current score displayed on the page from which the vote is made -- that isn't used to update the score.

The F() function provides a way to avoid this: https://docs.djangoproject.com/en/1.4/ref/models/instances/#updating-attributes-based-on-existing-fields

enewe101 commented 10 years ago

This issue remains open. The original description was sort of a mental note, here is more information so that the issue can be open to others.

Intro

On the front end, voting is enabled by a vote form, which is rendered using a special widget template called ~/templates/_w_vote_form.html. When a user clicks the up- or down-vote icon, the form is posted via an ajax request. The endpoint (the function that processes said request) is one of the flavors of voting endpoints found in ~/ajax.py, for example vote_question(), vote_answer(), etc. Each of these endpoints ultimately delegates to the function vote().

The models contains different objects for votes of different kinds. For example, there is a LetterVote, ProposalVote, and so on. The reason there are many kinds is because each has a different kind of target. So, the LetterVote's target is, of course a Letter.

Three things need to happen when a user votes:

  1. a record of the vote is made (this prevents double voting).
  2. the target of the vote (e.g., the Letter) gets its score incremented by a certain amount
  3. the user that created the target (author of the Letter) gets her reputation incremented by a certain amount

Each of these three things is carried out in the ajax function vote(). However, notice that vote() delegates the reputation incrementing to the UserProfile. This means that the amount by which rep changes is decided on the UserProfile object, which is good, because it provides one place where we can program the policies of how user reputation is affected by voting actions. After all, not all votes are of equal significance: having one's comment upvoted is less significant than having one's petition signed.

Task

The task is to make it so that vote() also delegates incrementing the target score to the target object itself, just like it delegates incementing user rep to the UserProfile object itself.

To do this, create a function called increment_score() on the abstract model called ScoredPost. Notice that the various scored content in app that have a score all inherit from ScoredPost. So for example, Letter inherets from ScoredPost. (You should verify that all the scored content really does inherit from that). Also create a function called decrement_score(). Now, inside vote() you can call increment_score() rather than manually adding the score and saving inside vote.

increment_score()

Take care when you make this function. Remember that the target needs to be saved after increasing it's score. Look at the documentation for the F() function in django. This allegedly helps us cure race conditions that arise when updating existing models. Do you see why incrementing a score constitutes a possible race-condition?

apply_rep()

Once you made increment_score() and its decrement version, go and look at apply_rep() and undo_rep() (which are on the UserProfile model). These don't use the F() function, but they should. Adapt these functions so that they use the F() function, to overcome the race-condition that arises when altering user's reputation.

Testing

make a testing class in ~/test.py which tests that the target score and author's rep are correctly updated when a given VoteForm is posted to its appropriate ajax endpoint. This should use the django test client. Note, you don't need to use a LiveServerTestCase, nor do you need to use a SeleniumTestCase -- a TestCase or a TransactionTestCase should suffice. Look at the test case UserProfileTest inside ~/test.py as an example. You only need to use TransactionTestCase if F() implies using transactions, which it might. Test the behavior of every vote endpoint in ajax.py.

enewe101 commented 10 years ago

This will be important, but isn't very important yet.

enewe101 commented 9 years ago

Is it really important?

enewe101 commented 9 years ago

Not important, at least not right now. Closing!