Summary

Refactor away ReviewBoxListView.

Review Request #8318 — Created Aug. 4, 2016 and submitted Aug. 8, 2016, 2:28 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

2de908c...

Reviewers

Groups

reviewboard

People

Description

This change makes it so that instead of ReviewBoxListView inspecting the DOM
and creating models and views to go along with it, those models and views are
created directly with the appropriate data. After this, ReviewBoxListView was
pretty useless, and because ReviewRequestPageView already had its fingers in
the pie, I've just moved all its functionality into there.

Testing Done


Ran js-tests.
Smoke tested the review request detail page with multiple reviews and change

  description boxes.

Issues

Description	From	Last Updated
Trailing comma.	chipx86	Aug. 5, 2016, 3:19 p.m.
Trailing comma.	chipx86	Aug. 5, 2016, 3:19 p.m.
Trailing comma.	chipx86	Aug. 5, 2016, 3:19 p.m.
Trailing comma.	chipx86	Aug. 5, 2016, 3:20 p.m.
Are these not under this.el? Can we not use this.$() ?	brennie	Aug. 8, 2016, 1:32 p.m.
Let's use listenTo.	chipx86	Aug. 8, 2016, 1:52 p.m.

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/views/reviewBoxListView.es6.js
    reviewboard/static/rb/js/pages/views/reviewRequestPageView.es6.js
    reviewboard/templates/reviews/boxes/review.html
    reviewboard/static/rb/js/pages/views/tests/reviewRequestPageViewTests.es6.js
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/templates/reviews/review_detail.html
    reviewboard/static/rb/js/views/tests/reviewBoxListViewTests.js
    reviewboard/templates/reviews/boxes/change.html
    reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js
    reviewboard/static/rb/js/views/reviewBoxView.es6.js



Tool: Pyflakes
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/views/reviewBoxListView.es6.js
    reviewboard/static/rb/js/pages/views/reviewRequestPageView.es6.js
    reviewboard/templates/reviews/boxes/review.html
    reviewboard/static/rb/js/pages/views/tests/reviewRequestPageViewTests.es6.js
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/templates/reviews/review_detail.html
    reviewboard/static/rb/js/views/tests/reviewBoxListViewTests.js
    reviewboard/templates/reviews/boxes/change.html
    reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js
    reviewboard/static/rb/js/views/reviewBoxView.es6.js

chipx86 Aug. 5, 2016, 12:32 a.m.

Also, this will break that one e-mail extension, and probably other third-party code (I seem to recall talking to someone else about ReviewBoxListView once). Might be considered enough of an internal detail to worry about, but would be good to add to that list on Notion I mentioned on other review requests for tracking purposes.

reviewboard/templates/reviews/review_detail.html (Diff revision 1)
The issue has been resolved. Show all issues
```
Trailing comma.
```
reviewboard/templates/reviews/review_detail.html (Diff revision 1)
The issue has been resolved. Show all issues
```
Trailing comma.
```
reviewboard/templates/reviews/review_detail.html (Diff revision 1)
The issue has been resolved. Show all issues
```
Trailing comma.
```

reviewboard/templates/reviews/reviewable_page_data.js (Diff revision 1)

The issue has been resolved. Show all issues

Trailing comma.

Commit:

f206e83df3d254c9ed987fdadcfe76df7272f038

2de908c4b4e3f7bc8b8d4aa06e7b3141e9551473

Diff:

Revision 2 (+264 -332)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/pages/views/reviewRequestPageView.es6.js
	reviewboard/static/rb/js/pages/views/tests/reviewRequestPageViewTests.es6.js
	reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js
	reviewboard/static/rb/js/views/reviewBoxListView.es6.js
	reviewboard/static/rb/js/views/reviewBoxView.es6.js
	reviewboard/static/rb/js/views/tests/reviewBoxListViewTests.js
	reviewboard/templates/reviews/review_detail.html
	3 more

Tool: Pyflakes
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/views/reviewBoxListView.es6.js
    reviewboard/static/rb/js/pages/views/reviewRequestPageView.es6.js
    reviewboard/templates/reviews/boxes/review.html
    reviewboard/static/rb/js/pages/views/tests/reviewRequestPageViewTests.es6.js
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/templates/reviews/review_detail.html
    reviewboard/static/rb/js/views/tests/reviewBoxListViewTests.js
    reviewboard/templates/reviews/boxes/change.html
    reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js
    reviewboard/static/rb/js/views/reviewBoxView.es6.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/views/reviewBoxListView.es6.js
    reviewboard/static/rb/js/pages/views/reviewRequestPageView.es6.js
    reviewboard/templates/reviews/boxes/review.html
    reviewboard/static/rb/js/pages/views/tests/reviewRequestPageViewTests.es6.js
    reviewboard/templates/reviews/reviewable_page_data.js
    reviewboard/templates/reviews/review_detail.html
    reviewboard/static/rb/js/views/tests/reviewBoxListViewTests.js
    reviewboard/templates/reviews/boxes/change.html
    reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js
    reviewboard/static/rb/js/views/reviewBoxView.es6.js

Ship it!

reviewboard/static/rb/js/pages/views/reviewRequestPageView.es6.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Are these not under this.el? Can we not use this.$() ?
```
1. david Aug. 8, 2016, 1:32 p.m.
  No, they're not. I may change that in a future cleanup.

Ship it!

```
Happy after this and Barret's comment.
```
reviewboard/static/rb/js/pages/views/reviewRequestPageView.es6.js (Diff revision 2)
The issue has been resolved. Show all issues
```
Let's use listenTo.
```

Status:: Completed
Change Summary:: Pushed to release-3.0.x (9861125)