Fix incorrectly calculating the number of issue counts.

Review Request #5505 — Created Feb. 19, 2014 and submitted

Information

Review Board
master
f41b059...

Reviewers

The query for calculating the number of issues led to duplicate results,
which could cause the returned issue count to be abnormally high.

This only happened when reviewing more than one type of thing. For
example, a diff and a file attachment, instead of just a diff or just a
file attachment.

The reason is that the query would return one result for each comment,
but that result would contain issue counts for each type of comment. So,
if commenting on 2 diffs and 1 file attachment, there may be a result
for diff comment 1, and diff comment 2, but both may include the issue
status for file attachment comment 1 as well.

What we needed to do was filter out comment entries we've already seen.
To do this, we request the comment IDs along with the issue statuses.
During processing of issue statuses, we check and see if the comment ID
for the status is one we've already seen. If so, we'll skip that issue
status.

Reproduced the problem locally by making 1 comment on a file attachment
and 2 on a diff. Saw the comment count as 4 instead of 3.

Made the test case, which gave me 24 open issues instead of 7.

After the patch, my issue count for the above manual repro case was 3,
and the unit test gave me 7.

Description From Last Updated

This would be a bit nicer using iteritems: for key, comments in six.iteritems(comment_fields): comment_pk = issue_fields[key + '__pk'] if comment_pk …

daviddavid

"with a mix" ? Can you also add a comment here explaining what the bug was to motivate this test?

daviddavid
david
  1. 
      
  2. reviewboard/reviews/models/review_request.py (Diff revision 1)
     
     
     
     
     
     
     
    Show all issues

    This would be a bit nicer using iteritems:

    for key, comments in six.iteritems(comment_fields):
        comment_pk = issue_fields[key + '__pk']
    
        if comment_pk not in comments:
            comments.add(comment_pk)
            issue_status = issue_fields[key + '__issue_status']
    
  3. reviewboard/reviews/tests.py (Diff revision 1)
     
     
    Show all issues

    "with a mix" ?

    Can you also add a comment here explaining what the bug was to motivate this test?

    1. Sure. Honestly, I should have had this test from the beginning, though, so I don't know that it's special enough to warrant a comment, but I'll add one.

  4. 
      
chipx86
david
  1. Ship It!

  2. 
      
chipx86
Review request changed
Status:
Completed