Summary

Fix storing DiffX data when commits aren't used

Review Request #11769 — Created Aug. 3, 2021 and submitted Aug. 9, 2021, 12:40 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

DiffXParser stores parsed information on a DiffX file in DiffSet,
DiffCommit, and FileDiff, and then uses that information to
regenerate the DiffX file.

The problem is, if a diff is uploaded without commit history (due to API
usage or SCM limitation), there is no DiffCommit, so we were losing
out on all the change-related data and all associated files. This isn't
just an issue for DiffX, but any parser storing data in
ParsedDiffChange.extra_data.

FileDiff creation now checks for this situation and stores any
ParsedDiffChange.extra_data content in a special key in DiffSet
(change_extra_data for a main diff, parent_change_extra_data for a
parent diff). DiffXParser will look for this and use it if commits
aren't available.

Testing Done

Unit tests pass on Python 2 and 3.

Verified that this fixed issues in production with non-commit-backed
diffs.

Commits

Summary	ID
Fix storing DiffX data when commits aren't used, and fix default revisions. `DiffXParser` stores parsed information on a DiffX file in `DiffSet`, `DiffCommit`, and `FileDiff`, and then uses that information to regenerate the DiffX file. The problem is, if a diff is uploaded without commit history (due to API usage or SCM limitation), there is no `DiffCommit`, so we were losing out on all the change-related data and all associated files. This isn't just an issue for DiffX, but any parser storing data in `ParsedDiffChange.extra_data`. `FileDiff` creation now checks for this situation and stores any `ParsedDiffChange.extra_data` content in a special key in `DiffSet` (`change_extra_data` for a main diff, `parent_change_extra_data` for a parent diff). `DiffXParser` will look for this and use it if commits aren't available.	a926a9ecaf7a3c3cec7a7ab386c5460d49d44603

Summary

Fix storing DiffX data when commits aren't used, and fix default revisions.

`DiffXParser` stores parsed information on a DiffX file in `DiffSet`, `DiffCommit`, and `FileDiff`, and then uses that information to regenerate the DiffX file. The problem is, if a diff is uploaded without commit history (due to API usage or SCM limitation), there is no `DiffCommit`, so we were losing out on all the change-related data and all associated files. This isn't just an issue for DiffX, but any parser storing data in `ParsedDiffChange.extra_data`. `FileDiff` creation now checks for this situation and stores any `ParsedDiffChange.extra_data` content in a special key in `DiffSet` (`change_extra_data` for a main diff, `parent_change_extra_data` for a parent diff). `DiffXParser` will look for this and use it if commits aren't available.

a926a9ecaf7a3c3cec7a7ab386c5460d49d44603

Issues

Description	From	Last Updated
F841 local variable 'diffset_extra_data' is assigned to but never used	reviewbot	Aug. 3, 2021, 9:05 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Aug. 3, 2021, 9:05 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/parser.py (Diff revision 1)

Show all issues

F841 local variable 'diffset_extra_data' is assigned to but never used

reviewboard/diffviewer/tests/test_diffx_parser.py (Diff revision 1)
Show all issues
```
E501 line too long (80 > 79 characters)
```

Summary:

-	Fix storing DiffX data when commits aren't used, and fix default revisions.
+	Fix storing DiffX data when commits aren't used

Change Summary:


Fixed a line length issue.
Removed an unused variable.

Commits:

	Summary	ID
	Fix storing DiffX data when commits aren't used, and fix default revisions. `DiffXParser` stores parsed information on a DiffX file in `DiffSet`, `DiffCommit`, and `FileDiff`, and then uses that information to regenerate the DiffX file. The problem is, if a diff is uploaded without commit history (due to API usage or SCM limitation), there is no `DiffCommit`, so we were losing out on all the change-related data and all associated files. This isn't just an issue for DiffX, but any parser storing data in `ParsedDiffChange.extra_data`. `FileDiff` creation now checks for this situation and stores any `ParsedDiffChange.extra_data` content in a special key in `DiffSet` (`change_extra_data` for a main diff, `parent_change_extra_data` for a parent diff). `DiffXParser` will look for this and use it if commits aren't available.	21c6ea8ec6140aaf4fb13d4975f417106ae558d0
	Fix storing DiffX data when commits aren't used, and fix default revisions. `DiffXParser` stores parsed information on a DiffX file in `DiffSet`, `DiffCommit`, and `FileDiff`, and then uses that information to regenerate the DiffX file. The problem is, if a diff is uploaded without commit history (due to API usage or SCM limitation), there is no `DiffCommit`, so we were losing out on all the change-related data and all associated files. This isn't just an issue for DiffX, but any parser storing data in `ParsedDiffChange.extra_data`. `FileDiff` creation now checks for this situation and stores any `ParsedDiffChange.extra_data` content in a special key in `DiffSet` (`change_extra_data` for a main diff, `parent_change_extra_data` for a parent diff). `DiffXParser` will look for this and use it if commits aren't available.	a926a9ecaf7a3c3cec7a7ab386c5460d49d44603

Diff:

Revision 2 (+586 -48)

Show changes

	reviewboard/diffviewer/filediff_creator.py
	reviewboard/diffviewer/parser.py
	reviewboard/diffviewer/tests/test_diffset_manager.py
	reviewboard/diffviewer/tests/test_diffx_parser.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (3c90032)