-
Notifications
You must be signed in to change notification settings - Fork 4
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
Conversation
self.user.set_password(self.PASSWORD) | ||
self.user.save() | ||
|
||
def _assign_question_to_user(self, user, question): |
There was a problem hiding this comment.
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()
.
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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 😕
self.assertIsInstance(next_question, NextQuestion) | ||
self.assertIsNone(next_question.question) | ||
|
||
@override_settings(DEBUG=True) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
question = models.Question.objects.create( | ||
question='fakebar', | ||
) | ||
self._assign_question_to_user(self.user, question) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
No description provided.