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

706948 no replies #506

Closed
wants to merge 2 commits into from
Closed

706948 no replies #506

wants to merge 2 commits into from

Conversation

willkg
Copy link
Member

@willkg willkg commented Feb 22, 2012

This fixes the problem where num_answers gets out of sync with the actual number of answers for the question.

My steps to reproduce:

  1. Go through all the shenanigans to create a question. Once you've created a question, you're on the answers page for that question.
  2. On the answers page, reply with "first post" because that's the bestest reply ever.
  3. Go to the questions listing page. The question lists as having "no replies".
  4. Click on the question and go to the answers page. It'll show No Replies, but show replies.

With this fix, the "no replies" bits become correct.

Three things:

  1. This fixes the issue I was seeing locally with the steps I was using to reproduce the issue.
  2. I'm seeing some other weird caching behavior, so it's possible this doesn't fix it on -dev/-stage. It's possible there are other problems, too.
  3. There is a test for this, but the test passed before and after my fix.

r?

self.question.answers.count() was pulling from cache, so when we create
a new Answer, it wasn't reflected in the count and then question.num_answers
would be 0 which was a mismatch.

This forces it to come from the db and not cache and thus we get the
right answer.
@rlr
Copy link
Contributor

rlr commented Feb 22, 2012

Looks good to me. r+

@willkg
Copy link
Member Author

willkg commented Feb 22, 2012

41a5e73

@willkg willkg closed this Feb 22, 2012
@@ -425,7 +424,10 @@ def save(self, no_update=False, no_notify=False, *args, **kwargs):
super(Answer, self).save(*args, **kwargs)

if new:
self.question.num_answers = self.question.answers.count()
# Need to pull from uncached, otherwise we get the cached
# query which doesn't reflect the answer we just saved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, since COUNT queries aren't cached, afaik.

Copy link
Member Author

Choose a reason for hiding this comment

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

So, I think this was a screwup on my part. I had cache machine caching counts locally and didn't discover it until after this went out. I'm now using cache settings more like what's on stage.

Given that I had "bad settings", this should get backed out since the comment is wrong and the code change shouldn't have any effect.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that I had "bad settings", this should get backed out since the comment is wrong and the code change shouldn't have any effect.

I agree. If we were to opt into caching counts, I think we'd want them to be consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants