Fix computation of new-file state with commit ranges.

Review Request #13674 — Created March 28, 2024 and submitted

Information

Review Board
release-7.x

Reviewers

When selecting a range of commits where the first commit adds a file and
the later commit(s) change it, the is_new_file state was being
incorrectly computed as True. In the case where we have a base
filediff, we instead should be treating it as an existing file rather
than a new one.

Loaded a commit range that was previously triggering the bug (especially
noticeable with the way binary files were loaded). Saw that I was now
seeing it treated as an existing file rather than new.

Summary ID
Fix computation of new-file state with commit ranges.
When selecting a range of commits where the first commit adds a file and the later commit(s) change it, the `is_new_file` state was being incorrectly computed as `True`. In the case where we have a base filediff, we instead should be treating it as an existing file rather than a new one. Testing Done: Loaded a commit range that was previously triggering the bug (especially noticeable with the way binary files were loaded). Saw that I was now seeing it treated as an existing file rather than new.
2410a01f205ffbe9a31848d4b08968dcd24f9d05
Description From Last Updated

Given the nature of the fix, we should have comprehensive unit testing for the various states. I wish we had …

chipx86chipx86
chipx86
  1. 
      
  2. Given the nature of the fix, we should have comprehensive unit testing for the various states.

    I wish we had a better idea of what the more complex logic was working to do.

    1. Looking at the history of the code, it looks like that was added during an intermediate version of the multi-commit implementation, where we were computing the cumulative diff ourselves based on all the individual commits (whereas now we have the client upload the cumulative diff separately).

  3. 
      
david
chipx86
  1. Ship It!
  2. 
      
maubin
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-7.x (442e57f)
Loading...