Summary

Add field view getter to request editor

Review Request #9256 — Created Oct. 10, 2017 and submitted Oct. 28, 2017, 3:45 p.m.

Information

Owner

erijo

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

9e77ac6...

Reviewers

Groups

reviewboard

People

Description

With the new getFieldView method, JavaScript code can get hold of the
view that handles editing for a given field ID, such as target_groups.

Testing Done

Unit tests.

Issues

Description	From	Last Updated
There's a format we like to see for descriptions. They should always stand alone from the summary, wrap to <= …	chipx86	Oct. 11, 2017, 12:31 p.m.
Unit tests for getFieldView and matchContains should be added to js/views/tests/reviewRequestEditorViewTests.js and listed in the Testing Done. This will be …	chipx86	Oct. 11, 2017, 12:31 p.m.
While useful for JavaScript extensions, this change isn't extension-specific. It'd be better to word the summary and description to be …	chipx86	Oct. 19, 2017, 12:34 p.m.
Should be "Return the view ..." and "ID".	chipx86	Oct. 11, 2017, 12:31 p.m.
This is missing a "Returns" section.	chipx86	Oct. 11, 2017, 12:32 p.m.
"ID"	chipx86	Oct. 11, 2017, 12:32 p.m.
The tests in this file could use some cleanup, but would you mind making a top-level describe() for "Methods", and …	chipx86	Oct. 19, 2017, 12:34 p.m.

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

There's a format we like to see for descriptions. They should always stand alone from the summary, wrap to <= 72 characters, and explain what problem needed solving, why, and how it was solved.

We use these directly for commit messages, so we have a writeup covering both:

https://www.notion.so/reviewboard/Writing-Good-Change-Descriptions-10529e7c207743fa8ca90153d4b21fea

The summary should ideally also explicitly mention "JavaScript extensions", and end with a period.

The issue has been resolved. Show all issues

Unit tests for getFieldView and matchContains should be added to js/views/tests/reviewRequestEditorViewTests.js and listed in the Testing Done. This will be more useful to us than your extension's testing, because it'll ensure there won't be regressions, and is "proof" that this works as expected.

You can run the test suite by going to http://localhost:8080/js-tests/ (assuming usage of a dev server).

reviewboard/static/rb/js/views/reviewRequestEditorView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Should be "Return the view ..." and "ID".
```

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

The issue has been resolved. Show all issues

This is missing a "Returns" section.

reviewboard/static/rb/js/views/reviewRequestEditorView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
"ID"
```

Change Summary:

Fixed comments and dropped the matchContains change after discussions on mailing list.

Summary:

Make it possible for extensions to access field views

Make it possible for JavaScript extensions to get field views.

Description:

~		and set the matchContains parameter used when autocompleting.
~
~		This support makes it possible to autocomplete group names that contain key (https://reviews.reviewboard.org/r/8585/) using an extension.
	~	JavaScript extensions can use the getFieldView() method on
	~	reviewRequestEditorView to get hold of a field view
	~	(e.g. target_groups).

Testing Done:

~		Wrote an extension that had the following template hook (base-scripts-post) and a custom WebAPIResource to autocomplete group names that contained the key entered.
	~	Unit tests.
-
-		`<script>`
-		RB.PageManager.beforeRender(function(page) {
-		let view = page.reviewRequestEditorView.getFieldView('target_groups');
-		let ac = view.autocomplete;
-		ac['resourceName'] = 'extensions/{{ extension.id }}/' + ac.fieldName;
-		ac['matchContains'] = true;
-		view.autocomplete = ac;
-		});
-		</script>
-

Commit:

b7e438a06799ce3b4243ed815c689992e3144c8e

773ed1d7e99d92354a5ed0a5e3ddf95a616d8e32

Diff:

Revision 2 (+25)

Show changes

	reviewboard/static/rb/js/views/reviewRequestEditorView.es6.js
	reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

While useful for JavaScript extensions, this change isn't extension-specific. It'd be better to word the summary and description to be more about the purpose of the method itself, leaving off the extensions parts.

Text also wraps a bit too short. ~72 characters would be best.

reviewboard/static/rb/js/views/tests/reviewRequestEditorViewTests.js (Diff revision 2)

The issue has been resolved. Show all issues

The tests in this file could use some cleanup, but would you mind making a top-level describe() for "Methods", and place this in there? The description should be the method name.

Summary:

Make it possible for JavaScript extensions to get field views.

Add field view getter to request editor

Description:

~		JavaScript extensions can use the getFieldView() method on
~		reviewRequestEditorView to get hold of a field view
	~	With the new getFieldView method, JavaScript code can get hold of the
	~	view that handles editing for a given field ID, such as target_groups.
-		(e.g. target_groups).