Summary

Fix "Update Diff" to do the right thing on review requests with history.

Review Request #13846 — Created May 8, 2024 and submitted May 21, 2024, 4:01 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-7.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

When a review request has commit history, it needs to always be updated
via RBTools. Attempting to use "Update Diff" to upload a diff file will
break in a confusing and useless way.

This change makes it so we show an informative error instead of letting
the user choose a diff file and then showing them an unknown error.

This also gets rid of an obsolete implementation for this that hadn't
been removed from ReviewRequestEditorView.

Testing Done

Clicked "Update Diff" on a review request with commit history and saw
the new error dialog. Did it on a review request without commit history
and saw the upload dialog as before.

Commits

Summary	ID
Fix "Update Diff" to do the right thing on review requests with history. When a review request has commit history, it needs to always be updated via RBTools. Attempting to use "Update Diff" to upload a diff file will break in a confusing and useless way. This change makes it so we show an informative error instead of letting the user choose a diff file and then showing them an unknown error. This also gets rid of an obsolete implementation for this that hadn't been removed from ReviewRequestEditorView. Testing Done: Clicked "Update Diff" on a review request with commit history and saw the new error dialog. Did it on a review request without commit history and saw the upload dialog as before.	be8e8f5d88985ee7074f62a0f42643a374719473

Summary

Fix "Update Diff" to do the right thing on review requests with history.

When a review request has commit history, it needs to always be updated via RBTools. Attempting to use "Update Diff" to upload a diff file will break in a confusing and useless way. This change makes it so we show an informative error instead of letting the user choose a diff file and then showing them an unknown error. This also gets rid of an obsolete implementation for this that hadn't been removed from ReviewRequestEditorView. Testing Done: Clicked "Update Diff" on a review request with commit history and saw the new error dialog. Did it on a review request without commit history and saw the upload dialog as before.

be8e8f5d88985ee7074f62a0f42643a374719473

Issues

Description	From	Last Updated
Can we explicitly compare length?	chipx86	May 9, 2024, 8:11 a.m.
Can we link to the RBTools page?	chipx86	May 9, 2024, 8:18 a.m.

flake8 passed.

JSHint passed.

reviewboard/static/rb/js/reviews/views/reviewRequestActions.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we explicitly compare length?
```
reviewboard/static/rb/js/reviews/views/reviewRequestActions.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we link to the RBTools page?
```

Commits:

	Summary	ID
	Fix "Update Diff" to do the right thing on review requests with history. When a review request has commit history, it needs to always be updated via RBTools. Attempting to use "Update Diff" to upload a diff file will break in a confusing and useless way. This change makes it so we show an informative error instead of letting the user choose a diff file and then showing them an unknown error. This also gets rid of an obsolete implementation for this that hadn't been removed from ReviewRequestEditorView. Testing Done: Clicked "Update Diff" on a review request with commit history and saw the new error dialog. Did it on a review request without commit history and saw the upload dialog as before.	c5b732062a883b76f389de5447a49a310cfd37af
	Fix "Update Diff" to do the right thing on review requests with history. When a review request has commit history, it needs to always be updated via RBTools. Attempting to use "Update Diff" to upload a diff file will break in a confusing and useless way. This change makes it so we show an informative error instead of letting the user choose a diff file and then showing them an unknown error. This also gets rid of an obsolete implementation for this that hadn't been removed from ReviewRequestEditorView. Testing Done: Clicked "Update Diff" on a review request with commit history and saw the new error dialog. Did it on a review request without commit history and saw the upload dialog as before.	eb90727582cbeff82839a8261fee45928b9b186a

Diff:

Revision 2 (+76 -60)

Show changes

	reviewboard/static/rb/js/reviews/views/reviewRequestActions.ts
	reviewboard/static/rb/js/reviews/views/reviewRequestEditorView.ts

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Rebase onto latest changes.

Commits:

	Summary	ID
	Fix "Update Diff" to do the right thing on review requests with history. When a review request has commit history, it needs to always be updated via RBTools. Attempting to use "Update Diff" to upload a diff file will break in a confusing and useless way. This change makes it so we show an informative error instead of letting the user choose a diff file and then showing them an unknown error. This also gets rid of an obsolete implementation for this that hadn't been removed from ReviewRequestEditorView. Testing Done: Clicked "Update Diff" on a review request with commit history and saw the new error dialog. Did it on a review request without commit history and saw the upload dialog as before.	eb90727582cbeff82839a8261fee45928b9b186a
	Fix "Update Diff" to do the right thing on review requests with history. When a review request has commit history, it needs to always be updated via RBTools. Attempting to use "Update Diff" to upload a diff file will break in a confusing and useless way. This change makes it so we show an informative error instead of letting the user choose a diff file and then showing them an unknown error. This also gets rid of an obsolete implementation for this that hadn't been removed from ReviewRequestEditorView. Testing Done: Clicked "Update Diff" on a review request with commit history and saw the new error dialog. Did it on a review request without commit history and saw the upload dialog as before.	acb98eb84723d2648d569501499bfa9a43cec4e3

Diff:

Revision 3 (+74 -60)

Show changes

	reviewboard/static/rb/js/reviews/views/reviewRequestActions.ts
	reviewboard/static/rb/js/reviews/views/reviewRequestEditorView.ts

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

We should also update the docs at /users/review-requests/uploading-diffs/ to be more explicit about how you can't use the the "Upload Diff" button for review requests created with RBTools.

Change Summary:

Update docs.

Commits:

	Summary	ID
	Fix "Update Diff" to do the right thing on review requests with history. When a review request has commit history, it needs to always be updated via RBTools. Attempting to use "Update Diff" to upload a diff file will break in a confusing and useless way. This change makes it so we show an informative error instead of letting the user choose a diff file and then showing them an unknown error. This also gets rid of an obsolete implementation for this that hadn't been removed from ReviewRequestEditorView. Testing Done: Clicked "Update Diff" on a review request with commit history and saw the new error dialog. Did it on a review request without commit history and saw the upload dialog as before.	acb98eb84723d2648d569501499bfa9a43cec4e3
	Fix "Update Diff" to do the right thing on review requests with history. When a review request has commit history, it needs to always be updated via RBTools. Attempting to use "Update Diff" to upload a diff file will break in a confusing and useless way. This change makes it so we show an informative error instead of letting the user choose a diff file and then showing them an unknown error. This also gets rid of an obsolete implementation for this that hadn't been removed from ReviewRequestEditorView. Testing Done: Clicked "Update Diff" on a review request with commit history and saw the new error dialog. Did it on a review request without commit history and saw the upload dialog as before.	be8e8f5d88985ee7074f62a0f42643a374719473

Diff:

Revision 4 (+84 -60)

Show changes

	docs/manual/users/review-requests/uploading-diffs.rst
	reviewboard/static/rb/js/reviews/views/reviewRequestActions.ts
	reviewboard/static/rb/js/reviews/views/reviewRequestEditorView.ts

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-7.x (d90c15f)