Summary

Point binary file diff's SHA256 hashes to their corresponding file attachment.

Review Request #14792 — Created Jan. 29, 2026 and submitted Feb. 2, 2026, 6:15 p.m.

Information

Owner

maubin

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Filediffs have patched_sha256 and orig_sha256 attributes that are
hashes of their patched and original file contents. These get set for
non-binary file diffs the first time that they're loaded into the diff
viewer. Originally, these don't get set for binary file diffs.

We recently gave file attachments a sha256_checksum attribute which
is also a hash of its file contents. For binary file diffs, we can point
to it's corresponding file attachment's hash for both the patched file
and original file. Now, the SHAs will be set for binary file diffs, and
accessible through the file diff _sha256 attributes or the file
attachments' sha256_checksum.

Testing Done


Ran unit tests.
Accessed the patched_sha256 and orig_sha256 attributes for a

  binary file diff and saw it was the same as the file attachments'

  versions.

Commits

Summary	ID
Point binary file diff's SHA256 hashes to their corresponding file attachment. Filediffs have `patched_sha256` and `orig_sha256` attributes that are hashes of their patched and original file contents. These get set for non-binary file diffs the first time that they're loaded into the diff viewer. Originally, these don't get set for binary file diffs. We recently gave file attachments a `sha256_checksum` attribute which is also a hash of its file contents. For binary file diffs, we can point to it's corresponding file attachment's hash for both the patched file and original file. Now, the SHAs will be set for binary file diffs, and accessible through the file diff `_sha256` attributes or the file attachments' `sha256_checksum`.	e41e1bff09890e9836339049f2361cb6c0921133

Summary

Point binary file diff's SHA256 hashes to their corresponding file attachment.

Filediffs have `patched_sha256` and `orig_sha256` attributes that are hashes of their patched and original file contents. These get set for non-binary file diffs the first time that they're loaded into the diff viewer. Originally, these don't get set for binary file diffs. We recently gave file attachments a `sha256_checksum` attribute which is also a hash of its file contents. For binary file diffs, we can point to it's corresponding file attachment's hash for both the patched file and original file. Now, the SHAs will be set for binary file diffs, and accessible through the file diff `_sha256` attributes or the file attachments' `sha256_checksum`.

e41e1bff09890e9836339049f2361cb6c0921133

Issues

Description	From	Last Updated
I think this would be clearer if we had more of an if/else structure, since right now it feels like …	david	Jan. 31, 2026, 10:22 p.m.
Same here.	david	Jan. 31, 2026, 10:22 p.m.
This feels a bit wonky to read due to how it's wrapped. I think in this case, it might better …	chipx86	Feb. 1, 2026, 5:32 p.m.
Same here.	chipx86	Feb. 1, 2026, 5:32 p.m.

flake8 passed.

JSHint passed.

reviewboard/diffviewer/models/filediff.py (Diff revision 1)

The issue has been resolved. Show all issues

I think this would be clearer if we had more of an if/else structure, since right now it feels like prioritizing the attachment case. We also need to make sure we have an explicit return in all code paths. Something like this?

if not self.binary:
    return self.extra_data.get('orig_sha256')
elif (attachment := FileAttachment.objects.get_for_filediff(
    self, modified=False)):
    return attachment.sha256_checksum
else:
    return None

reviewboard/diffviewer/models/filediff.py (Diff revision 1)

The issue has been resolved. Show all issues

Same here.

Commits:

	Summary	ID
	Point binary file diff's SHA256 hashes to their corresponding file attachment. Filediffs have `patched_sha256` and `orig_sha256` attributes that are hashes of their patched and original file contents. These get set for non-binary file diffs the first time that they're loaded into the diff viewer. Originally, these don't get set for binary file diffs. We recently gave file attachments a `sha256_checksum` attribute which is also a hash of its file contents. For binary file diffs, we can point to it's corresponding file attachment's hash for both the patched file and original file. Now, the SHAs will be set for binary file diffs, and accessible through the file diff `_sha256` attributes or the file attachments' `sha256_checksum`.	9798bfd77ad01d9ecb8cf13ec5606520d3555f13
	Point binary file diff's SHA256 hashes to their corresponding file attachment. Filediffs have `patched_sha256` and `orig_sha256` attributes that are hashes of their patched and original file contents. These get set for non-binary file diffs the first time that they're loaded into the diff viewer. Originally, these don't get set for binary file diffs. We recently gave file attachments a `sha256_checksum` attribute which is also a hash of its file contents. For binary file diffs, we can point to it's corresponding file attachment's hash for both the patched file and original file. Now, the SHAs will be set for binary file diffs, and accessible through the file diff `_sha256` attributes or the file attachments' `sha256_checksum`.	4096511ef3fc512c7249aa62b7c625beba9322b6

Diff:

Revision 2 (+38 -4)

Show changes

reviewboard/diffviewer/models/filediff.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/models/filediff.py (Diff revision 2)

The issue has been resolved. Show all issues

This feels a bit wonky to read due to how it's wrapped. I think in this case, it might better to just fetch in an else and then check the result.

reviewboard/diffviewer/models/filediff.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Same here.
```

Commits:

	Summary	ID
	Point binary file diff's SHA256 hashes to their corresponding file attachment. Filediffs have `patched_sha256` and `orig_sha256` attributes that are hashes of their patched and original file contents. These get set for non-binary file diffs the first time that they're loaded into the diff viewer. Originally, these don't get set for binary file diffs. We recently gave file attachments a `sha256_checksum` attribute which is also a hash of its file contents. For binary file diffs, we can point to it's corresponding file attachment's hash for both the patched file and original file. Now, the SHAs will be set for binary file diffs, and accessible through the file diff `_sha256` attributes or the file attachments' `sha256_checksum`.	4096511ef3fc512c7249aa62b7c625beba9322b6
	Point binary file diff's SHA256 hashes to their corresponding file attachment. Filediffs have `patched_sha256` and `orig_sha256` attributes that are hashes of their patched and original file contents. These get set for non-binary file diffs the first time that they're loaded into the diff viewer. Originally, these don't get set for binary file diffs. We recently gave file attachments a `sha256_checksum` attribute which is also a hash of its file contents. For binary file diffs, we can point to it's corresponding file attachment's hash for both the patched file and original file. Now, the SHAs will be set for binary file diffs, and accessible through the file diff `_sha256` attributes or the file attachments' `sha256_checksum`.	e41e1bff09890e9836339049f2361cb6c0921133

Diff:

Revision 3 (+52 -4)

Show changes

reviewboard/diffviewer/models/filediff.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-7.1.x (dda5350)