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

Reduce get next question queries #12

Merged
merged 13 commits into from
Jul 11, 2017
13 changes: 13 additions & 0 deletions questions/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@ class Meta:
abstract = True


class QuestionManager(models.Manager):

def get_user_questions(self, user):

questions = self.filter(
tag__usertag__user=user,
tag__usertag__enabled=True
)

return questions


class Question(CreatedBy):
question = models.TextField()
answer = models.ForeignKey('Answer', null=True, blank=True)
Expand All @@ -36,6 +48,7 @@ class Question(CreatedBy):
# tag_set
# user
# user_set
objects = QuestionManager()

def __unicode__(self):
return '<Question id=[%s] question=[%s] datetime_added=[%s]>' % (self.id, self.question, self.datetime_added)
Expand Down
Empty file added questions/tests/__init__.py
Empty file.
77 changes: 77 additions & 0 deletions questions/tests/test_get_next_question.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
from django.test import TestCase

from django.db import connection
from django.test.utils import override_settings
from emailusername.models import User

from questions import models
from questions.views import _get_next_question
from questions.views import NextQuestion


class TestGetNextQuestion(TestCase):

def setUp(self):
# Create a user
self.PASSWORD = 'p'
self.USERNAME = 'foo@bar.com'
self.user = User(email=self.USERNAME)
self.user.set_password(self.PASSWORD)
self.user.save()

def _assign_question_to_user(self, user, question):
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name _assign_question_to_user is misleading to me, because it sounds like the question is getting assigned to the user. What is really happening is that the question is getting associated with a tag, and the user is selecting that tag. I don't know what a better name would be, maybe something generic like _associate_tags().

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm right, although what the method really accomplishes is linking a question to a user via Tags, which are basically relationships between questions and users with some metadata, correct?

What about a name like _associate_question_with_user?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or maybe my idea of what Tags are is a bit off 😕

tag = models.Tag.objects.create(name='faketag')

models.UserTag.objects.create(
tag=tag,
user=user,
enabled=True
)
models.QuestionTag.objects.create(
tag=tag,
question=question,
enabled=True
)

return tag

def test_user_with_no_questions(self):
next_question = _get_next_question(self.user)

self.assertIsInstance(next_question, NextQuestion)
self.assertIsNone(next_question.question)

@override_settings(DEBUG=True)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What effect does DEBUG=True have on the test?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allows you to count queries via - django.db.connections.queries

def test_user_with_one_question(self):
question = models.Question.objects.create(
question='fakebar',
)
self._assign_question_to_user(self.user, question)

next_question = _get_next_question(self.user)

self.assertIsInstance(next_question, NextQuestion)
self.assertIsNotNone(next_question.question)
self.assertEqual(len(connection.queries), 8)

questions = models.Question.objects.get_user_questions(self.user)

self.assertEqual(questions.count(), 1)

@override_settings(DEBUG=True)
def test_user_with_ten_questions(self):
for i in range(10):
question = models.Question.objects.create(
question='fakebar',
)
self._assign_question_to_user(self.user, question)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be interesting here to have:
a. one question with no tags
b. one question with a tag by the user, but with no associated UserTag
and then assert that get_user_questions() only returns a count of 8 questions.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c. one question with a tag by the user, with an associated UserTag with enabled=False

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, these were just the first two tests I came up with to explore the _get_next_question method. Appreciate you laying out some additional tests for me.


next_question = _get_next_question(self.user)

self.assertIsInstance(next_question, NextQuestion)
self.assertIsNotNone(next_question.question)
self.assertEqual(len(connection.queries), 53)

questions = models.Question.objects.get_user_questions(self.user)

self.assertEqual(questions.count(), 10)
File renamed without changes.
2 changes: 1 addition & 1 deletion questions/tests.py → questions/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from emailusername.models import User
from questions import forms, models
from questions.test_helpers import FuzzyInt
from questions.tests.test_helpers import FuzzyInt
from questions.views import _get_next_question

# By default, LiveServerTestCase uses port 8081.
Expand Down