Use FileAttachment.added_in_filediff for modified files as well as new.

Review Request #13548 — Created Feb. 17, 2024 and submitted

Information

Review Board
master

Reviewers

In the initial design of the FileAttachment in diffs feature, we had
four possible fields:
- added_in_filediff
- repository
- repo_path
- repo_revision

For newly-added files, we'd set the added_in_filediff relation. For
all others, we'd set the other three. There are two major problems with
this.

First, the "revision" for a modified file is unique when talking about
git (a blob SHA), but isn't in many other systems. For example, the
FileDiff.dest_detail for an SVN change might just be "(working copy)".
This would result in attachments corresponding to multiple revisions of
the diff stomping on each other.

Second, once we had the file, any FileAttachmentComments would not be
able to link back to the diff in question, because that information was
not available.

This change modifies things so we use the added_in_filediff relation
for both newly-added and the modified version of any binary files. We
still use the repository/path/revision fields for the original version
of files.

  • Ran unit tests.
  • Uploaded a diff with binary files and checked the relations.
Summary ID
Use FileAttachment.added_in_filediff for modified files as well as new.
In the initial design of the FileAttachment in diffs feature, we had four possible fields: - `added_in_filediff` - `repository` - `repo_path` - `repo_revision` For newly-added files, we'd set the `added_in_filediff` relation. For all others, we'd set the other three. There are two major problems with this. First, the "revision" for a modified file is unique when talking about git (a blob SHA), but isn't in many other systems. For example, the `FileDiff.dest_detail` for an SVN change might just be "(working copy)". This would result in attachments corresponding to multiple revisions of the diff stomping on each other. Second, once we had the file, any FileAttachmentComments would not be able to link back to the diff in question, because that information was not available. This change modifies things so we use the `added_in_filediff` relation for both newly-added and the modified version of any binary files. We still use the repository/path/revision fields for the original version of files. Testing Done: - Ran unit tests. - Uploaded a diff with binary files and checked the relations.
f8729a6fb2439576d6bccdc52dae37c9fe5a787f
Description From Last Updated

These should be assertIsNone.

chipx86chipx86
chipx86
  1. 
      
  2. reviewboard/attachments/tests.py (Diff revision 1)
     
     
     
     
    Show all issues

    These should be assertIsNone.

  3. 
      
david
maubin
  1. Ship It!
  2. 
      
chipx86
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-7.x (9a15d76)