Summary

Port the repository config code to Backbone.

Review Request #12750 — Created Dec. 6, 2022 and updated Feb. 1, 2023, 8:51 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-6.x

Bugs

Depends On

Blocks

12754

Reviewers

Groups

reviewboard

People

Description

The JavaScript side of the repository configuration form is very old,
and has existed as a bunch of flat functions that just used jQuery event
handling and global state to do anything.

For a long time now, we've wanted to revisit repository configuration,
but both the backend and frontend implementations have made this tricky.

To begin that process, this change ports the old JavaScript to a new
RB.RepositoryFormView class. This is a pure port of the old code,
quirks and all, with the idea being that we can then begin to more
easily clean it up piece-by-piece.

This does add some asserts to make sure we've correctly loaded all parts
of the DOM, converts gettext code to use our localization template
literals, and properly uses .prop() on checkboxes instead of .val().
Otherwise, nothing's really changed.

Future changes will start to move state into a model, and then begin
cleaning up this form to work like the rest of our codebase.

For now, there are no unit tests. This code isn't intended to work this
way long enough to be worth having tests. Those will come as part of
the modernization work.

Testing Done

Tested all aspects of the repository configuration form:

Configuring various tools for custom repositories
Configuring various hosting service repositories, with different tools
and plans
Configuring bug trackers, with and without hosting services (and the
"Use hosting service's bug tracker" checkbox)
Configuring ACLs
Re-editing hosting service credentials
Toggling SSH key display

Commits

Summary	ID
Port the repository config code to Backbone. The JavaScript side of the repository configuration form is very old, and has existed as a bunch of flat functions that just used jQuery event handling and global state to do anything. For a long time now, we've wanted to revisit repository configuration, but both the backend and frontend implementations have made this tricky. To begin that process, this change ports the old JavaScript to a new `RB.RepositoryFormView` class. This is a pure port of the old code, quirks and all, with the idea being that we can then begin to more easily clean it up piece-by-piece. This does add some asserts to make sure we've correctly loaded all parts of the DOM, converts gettext code to use our localization template literals, and properly uses `.prop()` on checkboxes instead of `.val()`. Otherwise, nothing's really changed. Future changes will start to move state into a model, and then begin cleaning up this form to work like the rest of our codebase.	edcf1aaa07cb854231510d9010c005594da3fc53

Summary

Port the repository config code to Backbone.

The JavaScript side of the repository configuration form is very old, and has existed as a bunch of flat functions that just used jQuery event handling and global state to do anything. For a long time now, we've wanted to revisit repository configuration, but both the backend and frontend implementations have made this tricky. To begin that process, this change ports the old JavaScript to a new `RB.RepositoryFormView` class. This is a pure port of the old code, quirks and all, with the idea being that we can then begin to more easily clean it up piece-by-piece. This does add some asserts to make sure we've correctly loaded all parts of the DOM, converts gettext code to use our localization template literals, and properly uses `.prop()` on checkboxes instead of `.val()`. Otherwise, nothing's really changed. Future changes will start to move state into a model, and then begin cleaning up this form to work like the rest of our codebase.

edcf1aaa07cb854231510d9010c005594da3fc53

Issues

Description	From	Last Updated
Outer parens shouldn't be necessary here.	david	Jan. 25, 2023, 5:47 p.m.
Or here.	david	Jan. 25, 2023, 5:47 p.m.
Elsewhere we indent these as: let text = (account.hosting_url ? `${account.username} (${account.hosting_url})` : account.username)	david	Jan. 25, 2023, 5:47 p.m.
These days we should be good to use for (let planType of planTypes)	david	Jan. 25, 2023, 5:47 p.m.
At this point, I think we should be using const and trailing commas in our templates as well.	david	Feb. 1, 2023, 8:51 a.m.

flake8 passed.

JSHint passed.

reviewboard/static/rb/js/admin/views/repositoryFormView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Outer parens shouldn't be necessary here.
```
1. chipx86 Jan. 25, 2023, 6:07 p.m.
  I think I was initially hitting some ESLint complaints or something.
reviewboard/static/rb/js/admin/views/repositoryFormView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Or here.
```

reviewboard/static/rb/js/admin/views/repositoryFormView.es6.js (Diff revision 1)

The issue has been resolved. Show all issues

Elsewhere we indent these as:

let text = (account.hosting_url
            ? `${account.username} (${account.hosting_url})`
            : account.username)

reviewboard/static/rb/js/admin/views/repositoryFormView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
These days we should be good to use for (let planType of planTypes)
```

Change Summary:

Fixed some code inconsistency issues, and updated a for loop to use for...of.

Commits:

	Summary	ID
	Port the repository config code to Backbone. The JavaScript side of the repository configuration form is very old, and has existed as a bunch of flat functions that just used jQuery event handling and global state to do anything. For a long time now, we've wanted to revisit repository configuration, but both the backend and frontend implementations have made this tricky. To begin that process, this change ports the old JavaScript to a new `RB.RepositoryFormView` class. This is a pure port of the old code, quirks and all, with the idea being that we can then begin to more easily clean it up piece-by-piece. This does add some asserts to make sure we've correctly loaded all parts of the DOM, converts gettext code to use our localization template literals, and properly uses `.prop()` on checkboxes instead of `.val()`. Otherwise, nothing's really changed. Future changes will start to move state into a model, and then begin cleaning up this form to work like the rest of our codebase.	7c37399f3be8dffc8e2b9b7e26a5f5b3fb08c5cd
	Port the repository config code to Backbone. The JavaScript side of the repository configuration form is very old, and has existed as a bunch of flat functions that just used jQuery event handling and global state to do anything. For a long time now, we've wanted to revisit repository configuration, but both the backend and frontend implementations have made this tricky. To begin that process, this change ports the old JavaScript to a new `RB.RepositoryFormView` class. This is a pure port of the old code, quirks and all, with the idea being that we can then begin to more easily clean it up piece-by-piece. This does add some asserts to make sure we've correctly loaded all parts of the DOM, converts gettext code to use our localization template literals, and properly uses `.prop()` on checkboxes instead of `.val()`. Otherwise, nothing's really changed. Future changes will start to move state into a model, and then begin cleaning up this form to work like the rest of our codebase.	58198d801d4a1bc9eb29960e455b3e3f97e1d8eb

Diff:

Revision 2 (+1494 -912)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/admin/repositoryform.es6.js
	reviewboard/static/rb/js/admin/views/repositoryFormView.es6.js
	reviewboard/templates/admin/scmtools/repository/change_form.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:


Fixed some bad variable references.
Fixed a bad element ID reference in events.
Removed an unused variable.
Cleaned up some bits of code and fixed some lint warnings.

Commits:

	Summary	ID
	Port the repository config code to Backbone. The JavaScript side of the repository configuration form is very old, and has existed as a bunch of flat functions that just used jQuery event handling and global state to do anything. For a long time now, we've wanted to revisit repository configuration, but both the backend and frontend implementations have made this tricky. To begin that process, this change ports the old JavaScript to a new `RB.RepositoryFormView` class. This is a pure port of the old code, quirks and all, with the idea being that we can then begin to more easily clean it up piece-by-piece. This does add some asserts to make sure we've correctly loaded all parts of the DOM, converts gettext code to use our localization template literals, and properly uses `.prop()` on checkboxes instead of `.val()`. Otherwise, nothing's really changed. Future changes will start to move state into a model, and then begin cleaning up this form to work like the rest of our codebase.	58198d801d4a1bc9eb29960e455b3e3f97e1d8eb
	Port the repository config code to Backbone. The JavaScript side of the repository configuration form is very old, and has existed as a bunch of flat functions that just used jQuery event handling and global state to do anything. For a long time now, we've wanted to revisit repository configuration, but both the backend and frontend implementations have made this tricky. To begin that process, this change ports the old JavaScript to a new `RB.RepositoryFormView` class. This is a pure port of the old code, quirks and all, with the idea being that we can then begin to more easily clean it up piece-by-piece. This does add some asserts to make sure we've correctly loaded all parts of the DOM, converts gettext code to use our localization template literals, and properly uses `.prop()` on checkboxes instead of `.val()`. Otherwise, nothing's really changed. Future changes will start to move state into a model, and then begin cleaning up this form to work like the rest of our codebase.	edcf1aaa07cb854231510d9010c005594da3fc53

Diff:

Revision 3 (+1486 -912)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/admin/repositoryform.es6.js
	reviewboard/static/rb/js/admin/views/repositoryFormView.es6.js
	reviewboard/templates/admin/scmtools/repository/change_form.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Fix it!

reviewboard/templates/admin/scmtools/repository/change_form.html (Diff revision 3)

An issue was opened. Show all issues

At this point, I think we should be using const and trailing commas in our templates as well.