• 
      

    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