A little Hacker News in Django (part 5)

Let’s write a method to upvote a post. Let’s look at our PostUpvote model.

class PostUpvote(models.Model):
    post = models.ForeignKey(Post, on_delete=models.CASCADE)
    user = models.ForeignKey(User, related_name='post_upvotes', on_delete=models.CASCADE)

Ok, let’s add an upvote method to the Post class.

def upvote(self, user):
    self.upvotes.add(user)

Seriously? 1 line? It’s not even helping. Let’s just delete this method until there is a real need.

I just realized something important. A user can only upvote a post once. Let’s try to see if we can upvote multiple times. But first, let’s install the stuff necessary to get a shell_plus.

$ pipenv install --dev django-extensions ipython

We need to change our settings.

INSTALLED_APPS = [
    'django.contrib.admin',
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.messages',
    'django.contrib.staticfiles',
    'django_extensions',
    'hnews.posts.apps.PostsConfig',
]

If it was a real project, here is a time to have different settings files. I would add this app only on the dev settings.

Now let’s use our shell_plus and try to upvote multiple times.

$ ./manage.py shell_plus
...
In [1]: p = Post.objects.first()
In [2]: u = User.objects.first()
In [3]: u
Out[3]: <User: jean>
In [4]: p.upvotes.add(u)

~/.local/share/virtualenvs/hnews-XZcvnMjX/lib/python3.6/site-packages/django/db/models/fields/related_descriptors.py in add(self, *objs)
    891                     "Cannot use add() on a ManyToManyField which specifies an "
    892                     "intermediary model. Use %s.%s's Manager instead." %
--> 893                     (opts.app_label, opts.object_name)
    894                 )
    895             self._remove_prefetched_objects()
AttributeError: Cannot use add() on a ManyToManyField which specifies an intermediary model. Use posts.PostUpvote's Manager instead.

OK turns out, I cannot do it even one time. Now we really need an upvote method.

def upvote(self, user):
    PostUpvote.objects.create(post=self, user=user)

Let’s try again:

In [1]: p = Post.objects.first()
In [2]: u = User.objects.first()
In [3]: p.upvote(u)
In [4]: p.upvote(u)
In [5]: p.upvotes.all()
Out[5]: <QuerySet [<User: jean>, <User: jean>]>
In [6]: p.upvotes.all().delete()
Out[6]:
(3,
 {'admin.LogEntry': 0,
  'auth.User_groups': 0,
  'auth.User_user_permissions': 0,
  'posts.PostUpvote': 2,
  'posts.CommentUpvote': 0,
  'auth.User': 1})

OK, that’s not good. Let’s fix our PostUpvote model. Django has something for this problem: https://docs.djangoproject.com/en/2.0/ref/models/options/#unique-together

class PostUpvote(models.Model):
    post = models.ForeignKey(Post, on_delete=models.CASCADE)
    user = models.ForeignKey(User, related_name='post_upvotes', on_delete=models.CASCADE)

    class Meta:
        unique_together = ('post', 'user')

Now, we would need to make a migration to modify the database. Since we’re not on production, I can just delete the current db and rewrite the migrations.

$ rm db.sqlite3
$ rm hnews/posts/migrations/0001_initial.py
$ ./manage.py makemigrations
Migrations for 'posts':
  hnews/posts/migrations/0001_initial.py
    - Create model Comment
    - Create model CommentUpvote
    - Create model Post
    - Create model PostUpvote
    - Add field upvotes to post
    - Add field post to comment
    - Add field upvotes to comment
    - Alter unique_together for postupvote (1 constraint(s))
$ ./manage.py migrate && ./manage.py create_dev_data
Operations to perform:
  Apply all migrations: admin, auth, contenttypes, posts, sessions
Running migrations:
  Applying contenttypes.0001_initial... OK
  Applying auth.0001_initial... OK
  Applying admin.0001_initial... OK
  Applying admin.0002_logentry_remove_auto_add... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  Applying auth.0002_alter_permission_name_max_length... OK
  Applying auth.0003_alter_user_email_max_length... OK
  Applying auth.0004_alter_user_username_opts... OK
  Applying auth.0005_alter_user_last_login_null... OK
  Applying auth.0006_require_contenttypes_0002... OK
  Applying auth.0007_alter_validators_add_error_messages... OK
  Applying auth.0008_alter_user_username_max_length... OK
  Applying auth.0009_alter_user_last_name_max_length... OK
  Applying posts.0001_initial... OK
  Applying sessions.0001_initial... OK

Now let’s try to upvote two times again:

In [3]: p.upvote(u)
In [4]: p.upvote(u)
---------------------------------------------------------------------------
IntegrityError                            Traceback (most recent call last)
...

Now we have to do the same thing for the CommentUpvote.

class Comment(models.Model):
    creation_date = models.DateTimeField(auto_now_add=True)
    creator = models.ForeignKey(
        User,
        related_name='comments',
        on_delete=models.SET_NULL,
        null=True,
    )
    post = models.ForeignKey(Post, related_name='comments', on_delete=models.CASCADE)
    parent = models.ForeignKey('Comment', related_name='replies', on_delete=models.CASCADE, null=True, default=None)
    content = models.TextField(null=True)
    upvotes = models.ManyToManyField(User, through='CommentUpvote')
    
    def upvote(self, user):
        CommentUpvote.objects.create(post=self, user=user)


class CommentUpvote(models.Model):
    comment = models.ForeignKey(Comment, on_delete=models.CASCADE)
    user = models.ForeignKey(User, related_name='comment_upvotes', on_delete=models.CASCADE)
    
    class Meta:
        unique_together = ('comment', 'user')

I’ll skip the remake of the migrations.

Now, I’m asking myself if upvote should be idempotent (for a function it means that f o f = f). In our case it means that if we do:

p.upvote(u)
p.upvote(u)

it should be the same as

p.upvote(u)

Meaning that the method shouldn’t raise an exception. Ok let’s just replace .create by .get_or_create.

In [1]: p = Post.objects.first()
In [2]: u = User.objects.first()
In [3]: p.upvote(u)
In [4]: p.upvote(u)
In [5]: p.upvotes.all()
Out[5]: <QuerySet [<User: jean>]>

Now let’s write something to unupvote. Should we write and unupvote method? Or should we have a method to set an upvoted flag? I think I’ll change upvote to a method to set a flag. This way we can have a route to set that flag.

def set_upvoted(self, user, *, upvoted):
    if upvoted:
        PostUpvote.objects.get_or_create(post=self, user=user)
    else:
        self.upvotes.filter(user=user).delete()

and

def set_upvoted(self, user, *, upvoted):
    if upvoted:
        CommentUpvote.objects.get_or_create(comment=self, user=user)
    else:
        self.upvotes.filter(user=user).delete()

Let’s write tests for those two methods.

def test_set_upvoted_true(self):
    user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
    post = Post.objects.create(url='https://google.com', title='Google', creator=user)
    post.set_upvoted(user, upvoted=True)
    self.assertEqual(1, post.upvotes.filter(id=user.id).count())

def test_set_upvoted_false(self):
    user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
    post = Post.objects.create(url='https://google.com', title='Google', creator=user)
    post.set_upvoted(user, upvoted=True)
    post.set_upvoted(user, upvoted=False)
    self.assertEqual(0, post.upvotes.filter(id=user.id).count())

and

class CommentTestCase(TestCase):
    def test_set_upvoted_true(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        comment = Comment.objects.create(creator=user, content='Cool', post=post)
        comment.set_upvoted(user, upvoted=True)
        self.assertEqual(1, comment.upvotes.filter(id=user.id).count())

    def test_set_upvoted_false(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        comment = Comment.objects.create(creator=user, content='Cool', post=post)
        comment.set_upvoted(user, upvoted=True)
        comment.set_upvoted(user, upvoted=False)
        self.assertEqual(0, comment.upvotes.filter(id=user.id).count())

Ok, I messed up on the many-to-many Manager. You cannot do:

post.upvotes.filter(user=user)

You must do:

post.upvotes.filter(id=user.id)

That’s why the test_set_upvoted_false failed. Let’s fix our code:

def set_upvoted(self, user, *, upvoted):
    if upvoted:
        PostUpvote.objects.get_or_create(post=self, user=user)
    else:
        self.upvotes.filter(id=user.id).delete()

and

def set_upvoted(self, user, *, upvoted):
    if upvoted:
        CommentUpvote.objects.get_or_create(post=self, user=user)
    else:
        self.upvotes.filter(id=user.id).delete()

Now the tests pass. Let’s write the view to upvote a Post.

from django.contrib.auth.decorators import login_required
from django.shortcuts import get_object_or_404, redirect
from django.views.decorators.http import require_POST

@require_POST
@login_required
def set_upvoted_post(request, post_id):
    post = get_object_or_404(Post, id=post_id)
    upvoted = request.POST.get('upvoted')
    post.set_upvoted(request.user, upvoted=upvoted)
    return redirect('posts:list')

Let’s add it in the routes.

from django.urls import path

from . import views

app_name = 'posts'
urlpatterns = [
    path('', views.PostListView.as_view(), name='list'),
    path('<int:post_id>/set_upvoted/', views.set_upvoted_post, name='set_upvoted_post'),
]

Now let’s write a test to check that our view works. We’ll use the test client: https://docs.djangoproject.com/en/2.0/topics/testing/tools/#the-test-client

class SetUpvotedTestCase(TestCase):
    def test_set_upvoted_true_creates_post_upvote(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        client = Client()

        client.login(username='jean', password='hey')
        uri = reverse('posts:set_upvoted_post', kwargs={'post_id': post.id})
        client.post(uri, {'upvoted': True})
        self.assertEqual(1, post.upvotes.filter(id=user.id).count())

    def test_set_upvoted_true_redirects_to_list_view(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        client = Client()

        client.login(username='jean', password='hey')
        uri = reverse('posts:set_upvoted_post', kwargs={'post_id': post.id})
        response = client.post(uri, {'upvoted': True})
        self.assertRedirects(response, reverse('posts:list'))

    def test_set_upvoted_needs_authentication(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        client = Client()

        uri = reverse('posts:set_upvoted_post', kwargs={'post_id': post.id})
        response = client.post(uri, {'upvoted': True})
        # It should redirect to the login page
        self.assertEqual(response.status_code, 302)
        self.assertEqual(response.url, f'/accounts/login/?next={uri}')

    def test_set_upvoted_false_removes_post_upvote(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        client = Client()

        client.login(username='jean', password='hey')
        uri = reverse('posts:set_upvoted_post', kwargs={'post_id': post.id})
        client.post(uri, {'upvoted': True})
        client.post(uri, {'upvoted': False})
        self.assertEqual(0, post.upvotes.filter(id=user.id).count())

    def test_set_upvoted_false_redirects_to_list_view(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        client = Client()

        client.login(username='jean', password='hey')
        uri = reverse('posts:set_upvoted_post', kwargs={'post_id': post.id})
        response = client.post(uri, {'upvoted': False})
        self.assertRedirects(response, reverse('posts:list'))

One test fails: test_set_upvoted_false_removes_post_upvote. So I had to use pdb.

@require_POST
@login_required
def set_upvoted_post(request, post_id):
    post = get_object_or_404(Post, id=post_id)
    upvoted = request.POST.get('upvoted')
    import pdb;pdb.set_trace()
    post.set_upvoted(request.user, upvoted=upvoted)
    return redirect('posts:list')

PDB:

$ ./manage.py test
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
....................> /Users/daniel/Code/hnews/hnews/posts/views.py(21)set_upvoted_post()
-> post.set_upvoted(request.user, upvoted=upvoted)
(Pdb) upvoted
'False'

Now I’m thinking that maybe I should write a form, so that I don’t have to think about that kind of things.

$ touch hnews/posts/forms.py
$ cat hnews/posts/forms.py
from django import forms
class SetUpvotedForm(forms.Form):
    upvoted = forms.BooleanField()

I changed my view to this:

@require_POST
@login_required
def set_upvoted_post(request, post_id):
    post = get_object_or_404(Post, id=post_id)
    form = SetUpvotedForm(data=request.POST)
    if form.is_valid():
        post.set_upvoted(request.user, upvoted=form.cleaned_data['upvoted'])
        return redirect('posts:list')
    else:
        return HttpResponseBadRequest()

But it didn’t work, because the BooleanField is supposed to be used with a checkbox.

I’ll just consider that this view will be used with an AJAX call. Let’s remove the form.

@require_POST
@login_required
def set_upvoted_post(request, post_id):
    post = get_object_or_404(Post, id=post_id)
    try:
        upvoted = json.loads(request.body.decode('utf-8'))['upvoted']
    except (json.JSONDecodeError, KeyError):
        return HttpResponseBadRequest()
    post.set_upvoted(request.user, upvoted=upvoted)
    # 204: No content
    return HttpResponse(status=204)

FYI Documentation about 204

Gotcha: request.body is not a str, it’s a bytes.

Now let’s rewrite the tests

class SetUpvotedTestCase(TestCase):
    def test_set_upvoted_true_creates_post_upvote(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        client = Client()

        client.login(username='jean', password='hey')
        uri = reverse('posts:set_upvoted_post', kwargs={'post_id': post.id})
        client.post(uri, json.dumps({'upvoted': True}), content_type='application/json')
        self.assertEqual(1, post.upvotes.filter(id=user.id).count())

    def test_set_upvoted_true_returns_204(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        client = Client()

        client.login(username='jean', password='hey')
        uri = reverse('posts:set_upvoted_post', kwargs={'post_id': post.id})
        response = client.post(uri, json.dumps({'upvoted': True}), content_type='application/json')
        self.assertEqual(response.status_code, 204)

    def test_set_upvoted_needs_authentication(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        client = Client()

        uri = reverse('posts:set_upvoted_post', kwargs={'post_id': post.id})
        response = client.post(uri, json.dumps({'upvoted': True}), content_type='application/json')
        # It should redirect to the login page
        self.assertEqual(response.status_code, 302)
        self.assertEqual(response.url, f'/accounts/login/?next={uri}')

    def test_set_upvoted_false_removes_post_upvote(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        client = Client()

        client.login(username='jean', password='hey')
        uri = reverse('posts:set_upvoted_post', kwargs={'post_id': post.id})
        client.post(uri, json.dumps({'upvoted': True}), content_type='application/json')
        client.post(uri, json.dumps({'upvoted': False}), content_type='application/json')
        self.assertEqual(0, post.upvotes.filter(id=user.id).count())

    def test_set_upvoted_false_returns_204(self):
        user = User.objects.create_user(username='jean', email='jean@example.com', password='hey')
        post = Post.objects.create(url='https://google.com', title='Google', creator=user)
        client = Client()

        client.login(username='jean', password='hey')
        uri = reverse('posts:set_upvoted_post', kwargs={'post_id': post.id})
        response = client.post(uri, json.dumps({'upvoted': False}), content_type='application/json')
        self.assertEqual(response.status_code, 204)

Everything is OK now. I spent so much time debugging the Django client. Let’s call it a day.

See ya!

• A little Hacker News in Django (Part 1)
• A little Hacker News in Django (Part 2)
• A little Hacker News in Django (Part 3)
• A little Hacker News in Django (Part 4)
• A little Hacker News in Django (Part 6)
• A little Hacker News in Django (Part 7)
• A little Hacker News in Django (Part 8)
• A little Hacker News in Django (Part 9)
• Github repository: hnews