user10000033
user10000033

Reputation:

Django - User should only able to vote once

i want that a user is only able to vote once for a category-request but somehow i get the following error and i dont know how to "properly" call the instance at that point:

Cannot assign "1": "CategoryRequests_Voter.voted" must be a "CategoryRequests" instance.

models.py

# Category Requests Model
class CategoryRequests(models.Model):
    author = models.ForeignKey(User, on_delete=models.CASCADE)

    ....

# Vote(s) of Category Requests Model
class CategoryRequests_Voter(models.Model):
    voter = models.ForeignKey(User, on_delete=models.CASCADE)
    voted = models.ForeignKey(CategoryRequests, on_delete=models.CASCADE)
    published_date = models.DateField(auto_now_add=True, null=True)

    def publish(self):
        self.published_date = timezone.now()
        self.save()

views.py

def category_request_up_vote (request, pk):
    category_request = get_object_or_404(CategoryRequests, pk=pk)
    if request.method == 'GET':
        if CategoryRequests_Voter.objects.filter(voter=request.user, voted=category_request.pk).exists():
            messages.error(request, 'You already voted for this request.')
        else:
            category_request.up_vote = F('up_vote') + 1
            category_request.save()
            CategoryRequests_Voter.objects.create(voter=request.user, voted=category_request.pk)
            messages.success(request, 'You have successfully Provided an Up-Vote for this Request')
            return redirect('category_request_detail', pk=category_request.pk)
    else:
        messages.error(request, 'Uuups, something went wrong, please try again.')
        return redirect('category_request_detail', pk=category_request.pk)

Thanks in advance

Upvotes: 2

Views: 274

Answers (2)

willeM_ Van Onsem
willeM_ Van Onsem

Reputation: 476709

You need to fix the voted argument to just category_request, not its primary key, like:

CategoryRequests_Voter.objects.create(voter=request.user, voted=category_request)

You can however improve your model and view to improve consistency, and elegance. In order to prevent a User from voting twice or more, you can prevent creating creating a CategoryRequest_Voter object twice for the same voter and voted, by using a unique_together constraint:

class CategoryRequests_Voter(models.Model):
    voter = models.ForeignKey(User, on_delete=models.CASCADE)
    voted = models.ForeignKey(CategoryRequests, on_delete=models.CASCADE)
    published_date = models.DateField(auto_now_add=True, null=True)

    class Meta:
        unique_together = ('voter', 'voted')

    def publish(self):
        self.published_date = timezone.now()
        self.save()

Furthermore we can use a get_or_create and thus make only a single fetch from the database. Typically a view that changes data should do this with a POST request, not with a GET. GET requests are not supposed to have side effects.

def category_request_up_vote (request, pk):
    category_request = get_object_or_404(CategoryRequests, pk=pk)
    if request.method == 'POST':
        __, created = CategoryRequests_Voter.objects.get_or_create(
            voter=request.user,
            voted=category_request
        )
        if created:
            category_request.up_vote = F('up_vote') + 1
            category_request.save()
            messages.success(request, 'You have successfully Provided an Up-Vote for this Request')
        else:
            messages.error(request, 'You already voted for this request.')
    else:
        messages.error(request, 'Uuups, something went wrong, please try again.')
    return redirect('category_request_detail', pk=category_request.pk)

It might be worth to count the number of CategoryRequest_Voters, instead of incrementing the vote count, since it is possible that, for example due to deletion of Users, or votes, eventually the number of votes is no longer in harmony with the number of CategoryRequests_Voters for that CategoryRequests object.

Perhaps counting the number of objects is not that efficient per vote, but you could make a task that runs every now and then, and thus calculate:

CategoryRequests_Voter.objects.filter(voted=category_request).count()

to count the number of CategoryRequests_Voter for the given category_request.

Upvotes: 2

user10000033
user10000033

Reputation:

def category_request_up_vote (request, pk):
    category_request = get_object_or_404(CategoryRequests, pk=pk)
    try:
        if request.method == 'GET':
            if CategoryRequests_Voter.objects.filter(voter=request.user, voted=category_request).exists():
                messages.error(request, 'You already voted for this request.')
                return redirect('category_request_detail', pk=category_request.pk)
            else:
                category_request.up_vote = F('up_vote') + 1
                category_request.save()
                CategoryRequests_Voter.objects.create(voter=request.user, voted=category_request)
                messages.success(request, 'You have successfully Provided an Up-Vote for this Request')
                return redirect('category_request_detail', pk=category_request.pk)
        else:
            messages.error(request, 'Uuups, something went wrong, please try again.')
            return redirect('category_request_detail', pk=category_request.pk)
    except:
        messages.error(request, 'Uuups, something went wrong, please try again.')
        return redirect('category_request_detail', pk=category_request.pk)

Upvotes: 0

Related Questions