Fix a variety of bad bugs with diff comments and commit ranges.

Review Request #13547 — Created Feb. 16, 2024 and submitted — Latest diff uploaded

Information

Review Board
master

Reviewers

When making comments on diffs for review requests that have multiple
commits, there were a handful of bugs of varying severity. These all
occurred when adding comments to subsets of the commit range, rather
than the cumulative diff.

  • Medium bad: The link URL for comments added to commit ranges went to
    the regular diff page without including any commit info in the query
    string. We therefore wouldn't load the correct view.
  • Bad: When looking at commit ranges, we'd add comment flags to the diff
    view for all comments where the tip filediff matched. This would end
    up showing comment flags even if the base commit didn't match.
  • Very bad: The diff fragment shown for comments on commit ranges wasn't
    taking the base commit into account, instead always showing the diff
    between the base of the entire change and the selected tip commit. In
    most cases this wouldn't matter but if there are repeated changes to
    the same file across many commits, the shown diff fragment would be
    completely wrong.

These bugs all more or less come down to the same underlying problem:
not plumbing through the right commit information. We thankfully are
correctly storing the "base filediff" in the comment extra_data, which
is the ID of the filediff for the given file in the selected base commit
of the range. We therefore need to plumb that through so that we're
always getting the correct things from get_diff_files and other
diffutils methods.

In order to make this testable, I've pulled some commit-specific test
infrastructure out of test_diffutils.py and made it available from
our main testcase file.

  • Ran unit tests.
  • Posted a review request with a number of commits that all changed the
    same file. Added comments across various ranges of those commits. Saw
    the following:

  • In the review box, the link for comments always went to the correct
    commit range for the comment.
  • In the review box, the diff fragment for the comment was the same as
    what was selected when making the comment.
  • When viewing commit ranges in the diff viewer, I only saw comments
    that matched the currently visible commit range.

Commits

Files

    Loading...