Summary

Convert BaseResource.save to return a promise.

Review Request #11642 — Created June 2, 2021 and submitted Dec. 3, 2021, 3:48 p.m.

Information

Owner

david

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This change updates the save machinery for various resources to use
promises and async/await instead of callbacks.

Testing Done


Ran js-tests.
Manually tested API token creation, comment editing, review request

  field editing, post-commit review request creation, ship-it revocation,

  and file attachment uploading.

Commits

Summary	ID
Convert BaseResource.save to return a promise. This change updates the save machinery for various resources to use promises and async/await instead of callbacks. Testing Done: - Ran js-tests. - Manually tested API token creation, comment editing, review request field editing, post-commit review request creation, ship-it revocation, and file attachment uploading.	fe13a59893bbf8cdbb32a68242c9a9926c5d7ec9

Issues

Description	From	Last Updated
Col: 15 'commit' was used before it was defined.	reviewbot	June 2, 2021, 2:49 p.m.
Col: 39 Missing semicolon.	reviewbot	June 2, 2021, 2:48 p.m.
Does .and.resolveTo() work instead of having to explicitly return Promise.resolve()?	chipx86	Oct. 18, 2021, 9:53 p.m.
Think this can be .and.rejectWith(new BackboneError(...))	chipx86	Oct. 18, 2021, 9:53 p.m.
, optional?	chipx86	Oct. 18, 2021, 9:53 p.m.

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/newReviewRequest/views/tests/postCommitViewTests.es6.js (Diff revision 1)

The issue has been resolved. Show all issues

Col: 15
 'commit' was used before it was defined.

reviewboard/static/rb/js/resources/models/tests/baseResourceModelTests.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 39
 Missing semicolon.
```

Commits:

	Summary	ID
	Convert BaseResource.save to return a promise. This change updates the save machinery for various resources to use promises and async/await instead of callbacks. Testing Done: - Ran js-tests. - Manually tested API token creation, comment editing, review request field editing, post-commit review request creation, ship-it revocation, and file attachment uploading.	5aee07394f3cf72fb0a2595a8b585c0c10682b85
	Convert BaseResource.save to return a promise. This change updates the save machinery for various resources to use promises and async/await instead of callbacks. Testing Done: - Ran js-tests. - Manually tested API token creation, comment editing, review request field editing, post-commit review request creation, ship-it revocation, and file attachment uploading.	fe570fdcdea6875baaf88600256a4b6b2511b21f

Diff:

Revision 2 (+1978 -2068)

Show changes

	reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js
	reviewboard/static/rb/js/models/commentEditorModel.es6.js
	reviewboard/static/rb/js/models/commentIssueManagerModel.es6.js
	reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js
	reviewboard/static/rb/js/models/uploadDiffModel.es6.js
	reviewboard/static/rb/js/models/userSessionModel.es6.js
	reviewboard/static/rb/js/models/tests/commentEditorModelTests.es6.js
	reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.es6.js
	25 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commits:

	Summary	ID
	Convert BaseResource.save to return a promise. This change updates the save machinery for various resources to use promises and async/await instead of callbacks. Testing Done: - Ran js-tests. - Manually tested API token creation, comment editing, review request field editing, post-commit review request creation, ship-it revocation, and file attachment uploading.	fe570fdcdea6875baaf88600256a4b6b2511b21f
	Convert BaseResource.save to return a promise. This change updates the save machinery for various resources to use promises and async/await instead of callbacks. Testing Done: - Ran js-tests. - Manually tested API token creation, comment editing, review request field editing, post-commit review request creation, ship-it revocation, and file attachment uploading.	2a68656268cbc391c8a76116055c98b34de93d28

Diff:

Revision 3 (+2418 -2198)

Show changes

	reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js
	reviewboard/static/rb/js/models/commentEditorModel.es6.js
	reviewboard/static/rb/js/models/commentIssueManagerModel.es6.js
	reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js
	reviewboard/static/rb/js/models/uploadDiffModel.es6.js
	reviewboard/static/rb/js/models/userSessionModel.es6.js
	reviewboard/static/rb/js/models/tests/commentEditorModelTests.es6.js
	reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.es6.js
	25 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Use expectAsync

Commits:

	Summary	ID
	Convert BaseResource.save to return a promise. This change updates the save machinery for various resources to use promises and async/await instead of callbacks. Testing Done: - Ran js-tests. - Manually tested API token creation, comment editing, review request field editing, post-commit review request creation, ship-it revocation, and file attachment uploading.	2a68656268cbc391c8a76116055c98b34de93d28
	Convert BaseResource.save to return a promise. This change updates the save machinery for various resources to use promises and async/await instead of callbacks. Testing Done: - Ran js-tests. - Manually tested API token creation, comment editing, review request field editing, post-commit review request creation, ship-it revocation, and file attachment uploading.	3a7c5d1c2b30161af04cc48196b0a352156dac62

Diff:

Revision 4 (+2402 -2198)

Show changes

	reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js
	reviewboard/static/rb/js/models/commentEditorModel.es6.js
	reviewboard/static/rb/js/models/commentIssueManagerModel.es6.js
	reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js
	reviewboard/static/rb/js/models/uploadDiffModel.es6.js
	reviewboard/static/rb/js/models/userSessionModel.es6.js
	reviewboard/static/rb/js/models/tests/commentEditorModelTests.es6.js
	reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.es6.js
	25 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Looks good. There's some Jasmine stuff we can probably use for making spies that resolve/reject. Only highlighted one instance of each.

Doesn't have to be a blocker for this change.

reviewboard/static/rb/js/models/tests/commentEditorModelTests.es6.js (Diff revision 4)
The issue has been resolved. Show all issues
```
Does .and.resolveTo() work instead of having to explicitly return Promise.resolve()?
```

reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.es6.js (Diff revision 4)

The issue has been resolved. Show all issues

Think this can be .and.rejectWith(new BackboneError(...))

reviewboard/static/rb/js/resources/models/draftReviewRequestModel.es6.js (Diff revision 4)

The issue has been resolved. Show all issues

, optional?

Commits:

	Summary	ID
	Convert BaseResource.save to return a promise. This change updates the save machinery for various resources to use promises and async/await instead of callbacks. Testing Done: - Ran js-tests. - Manually tested API token creation, comment editing, review request field editing, post-commit review request creation, ship-it revocation, and file attachment uploading.	3a7c5d1c2b30161af04cc48196b0a352156dac62
	Convert BaseResource.save to return a promise. This change updates the save machinery for various resources to use promises and async/await instead of callbacks. Testing Done: - Ran js-tests. - Manually tested API token creation, comment editing, review request field editing, post-commit review request creation, ship-it revocation, and file attachment uploading.	fe13a59893bbf8cdbb32a68242c9a9926c5d7ec9

Diff:

Revision 5 (+2324 -2186)

Show changes

	reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js
	reviewboard/static/rb/js/models/commentEditorModel.es6.js
	reviewboard/static/rb/js/models/commentIssueManagerModel.es6.js
	reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js
	reviewboard/static/rb/js/models/uploadDiffModel.es6.js
	reviewboard/static/rb/js/models/userSessionModel.es6.js
	reviewboard/static/rb/js/models/tests/commentEditorModelTests.es6.js
	reviewboard/static/rb/js/models/tests/reviewRequestEditorModelTests.es6.js
	25 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (476e5dc)