Summary

Split the rendering of JavaScript action models and views.

Review Request #14665 — Created Oct. 31, 2025 and updated Nov. 10, 2025, 2:49 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

We now have enough support for multiple views for the same action. It's
time to separate out the registration of the models and the views so
there's no longer a one-to-one association.

The new BaseAction.render_model_js() is responsible for writing the
JavaScript to render an action model. Every registered action in Python
will be registered in JavaScript.

The existing BaseAction.render_js() (old name currently kept for
compatibility reasons) will render the view. This is now only called
when the attachment points used on the page, rather than all attachment
points. That's handled in the {% actions_html %} template tag.

The template tag renders the HTML for the attachment point where the
template tag is placed, and then sets up a page injection containing the
JavaScript views. Then, when rendering the JavaScript models, the
injected JavaScript views content will be included.

Some of the design of this change is here to facilitate the upcoming
improvements to attachment points.

Testing Done

Unit tests pass.

Tested that all the actions on the page displayed and worked correctly.

Commits

Summary	ID
Split the rendering of JavaScript action models and views. We now have enough support for multiple views for the same action. It's time to separate out the registration of the models and the views so there's no longer a one-to-one association. The new `BaseAction.render_model_js()` is responsible for writing the JavaScript to render an action model. Every registered action in Python will be registered in JavaScript. The existing `BaseAction.render_js()` (old name currently kept for compatibility reasons) will render the view. This is now only called when the attachment points used on the page, rather than all attachment points. That's handled in the `{% actions_html %}` template tag. The template tag renders the HTML for the attachment point where the template tag is placed, and then sets up a page injection containing the JavaScript views. Then, when rendering the JavaScript models, the injected JavaScript views content will be included. Some of the design of this change is here to facilitate the upcoming improvements to attachment points.	64080b45a209b91ad223a0767d2bc35bd728c884

Summary

Split the rendering of JavaScript action models and views.

We now have enough support for multiple views for the same action. It's time to separate out the registration of the models and the views so there's no longer a one-to-one association. The new `BaseAction.render_model_js()` is responsible for writing the JavaScript to render an action model. Every registered action in Python will be registered in JavaScript. The existing `BaseAction.render_js()` (old name currently kept for compatibility reasons) will render the view. This is now only called when the attachment points used on the page, rather than all attachment points. That's handled in the `{% actions_html %}` template tag. The template tag renders the HTML for the attachment point where the template tag is placed, and then sets up a page injection containing the JavaScript views. Then, when rendering the JavaScript models, the injected JavaScript views content will be included. Some of the design of this change is here to facilitate the upcoming improvements to attachment points.

64080b45a209b91ad223a0767d2bc35bd728c884

Issues

Description	From	Last Updated
Might as well specify "action's Javascript model" here.	maubin	Nov. 10, 2025, 2:05 a.m.
We should be more specific here and say "... for action model".	maubin	Nov. 10, 2025, 2:05 a.m.
The part about setting up the model should be removed from the sentence now.	maubin	Nov. 10, 2025, 2:06 a.m.
It's a little bit confusing that we assign actions twice and iterate over for action in actions twice. Can we …	david	Nov. 10, 2025, 2:21 a.m.
Expected a string and instead saw %. Column: 2 Error code: W095	reviewbot	Oct. 31, 2025, 2:26 a.m.
Expected ':' and instead saw 'load'. Column: 4 Error code: E021	reviewbot	Oct. 31, 2025, 2:26 a.m.
Expected a JSON value. Column: 9 Error code: E003	reviewbot	Oct. 31, 2025, 2:26 a.m.
Expected '}' and instead saw 'actions'. Column: 9 Error code: E021	reviewbot	Oct. 31, 2025, 2:26 a.m.
Unrecoverable syntax error. (11% scanned). Column: 9 Error code: E041	reviewbot	Oct. 31, 2025, 2:26 a.m.
Same comment as the one on /r/14663, should we \|escapejs this?	maubin	Nov. 10, 2025, 2:07 a.m.
Expected a string and instead saw %. Column: 2 Error code: W095	reviewbot	Nov. 10, 2025, 2:21 a.m.
Expected ':' and instead saw 'load'. Column: 4 Error code: E021	reviewbot	Nov. 10, 2025, 2:21 a.m.
Expected '}' and instead saw 'actions'. Column: 9 Error code: E021	reviewbot	Nov. 10, 2025, 2:21 a.m.
Unrecoverable syntax error. (12% scanned). Column: 9 Error code: E041	reviewbot	Nov. 10, 2025, 2:21 a.m.
Expected a JSON value. Column: 9 Error code: E003	reviewbot	Nov. 10, 2025, 2:21 a.m.
Expected a string and instead saw %. Column: 2 Error code: W095	reviewbot	Nov. 10, 2025, 2:26 a.m.
Expected ':' and instead saw 'load'. Column: 4 Error code: E021	reviewbot	Nov. 10, 2025, 2:26 a.m.
Expected a JSON value. Column: 9 Error code: E003	reviewbot	Nov. 10, 2025, 2:26 a.m.
Expected '}' and instead saw 'actions'. Column: 9 Error code: E021	reviewbot	Nov. 10, 2025, 2:26 a.m.
Unrecoverable syntax error. (12% scanned). Column: 9 Error code: E041	reviewbot	Nov. 10, 2025, 2:26 a.m.

flake8 passed.

JSHint failed.

JSHint

reviewboard/templates/actions/action_view.js (Diff revision 1)
The issue has been dropped. Show all issues
```
Expected a string and instead saw %.

Column: 2
Error code: W095
```
reviewboard/templates/actions/action_view.js (Diff revision 1)
The issue has been dropped. Show all issues
```
Expected ':' and instead saw 'load'.

Column: 4
Error code: E021
```
reviewboard/templates/actions/action_view.js (Diff revision 1)
The issue has been dropped. Show all issues
```
Expected a JSON value.

Column: 9
Error code: E003
```
reviewboard/templates/actions/action_view.js (Diff revision 1)
The issue has been dropped. Show all issues
```
Expected '}' and instead saw 'actions'.

Column: 9
Error code: E021
```
reviewboard/templates/actions/action_view.js (Diff revision 1)
The issue has been dropped. Show all issues
```
Unrecoverable syntax error. (11% scanned).

Column: 9
Error code: E041
```

reviewboard/actions/base.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Might as well specify "action's Javascript model" here.
```
reviewboard/actions/base.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We should be more specific here and say "... for action model".
```
reviewboard/actions/base.py (Diff revision 1)
The issue has been resolved. Show all issues
```
The part about setting up the model should be removed from the sentence now.
```

reviewboard/templates/actions/action_view.js (Diff revision 1)

The issue has been resolved. Show all issues

Same comment as the one on /r/14663, should we |escapejs this?

chipx86 Nov. 10, 2025, 2:10 a.m.

Yeah. In fact, I'm going to move this into state injected by the template tag, so it's not micro-managed specially.

reviewboard/actions/templatetags/actions.py (Diff revision 1)

The issue has been resolved. Show all issues

It's a little bit confusing that we assign actions twice and iterate over for action in actions twice.

Can we either just move the get_for_attachment calls into the for lines, or have two different variable names (like toplevel_actions and actions_with_children?)

chipx86 Nov. 10, 2025, 2:10 a.m.

The first renders the models globally (only for top-level). The second renders each view of each action for the attachment point at every level, but injects as page state so it's included later.

I'll rename.

chipx86 Nov. 10, 2025, 2:21 a.m.

Oh, and this can be cleaned up after the placement change, because actions won't be in a hierarchy anymore natively, just their rendered placements. So we can have one loop. I'll update that accordingly.

chipx86 Nov. 10, 2025, 2:23 a.m.

Ignore that. I forgot this all changed with the attachment point change. So this all got rewritten anyway.

Change Summary:


Moved attachmentPointID for the JS view into js_view_data so it's serialized like all other options.
Clarified some docs.
Updated class attributes to use ClassVar.
Updated the actions_html template tag to better document the two different rendering passes, and to avoid the actions variable.

Commits:

	Summary	ID
	Split the rendering of JavaScript action models and views. We now have enough support for multiple views for the same action. It's time to separate out the registration of the models and the views so there's no longer a one-to-one association. The new `BaseAction.render_model_js()` is responsible for writing the JavaScript to render an action model. Every registered action in Python will be registered in JavaScript. The existing `BaseAction.render_js()` (old name currently kept for compatibility reasons) will render the view. This is now only called when the attachment points used on the page, rather than all attachment points. That's handled in the `{% actions_html %}` template tag. The template tag renders the HTML for the attachment point where the template tag is placed, and then sets up a page injection containing the JavaScript views. Then, when rendering the JavaScript models, the injected JavaScript views content will be included. Some of the design of this change is here to facilitate the upcoming improvements to attachment points.	ac05257d3e82428a725f410c1bcd8cc5f9766b95
	Split the rendering of JavaScript action models and views. We now have enough support for multiple views for the same action. It's time to separate out the registration of the models and the views so there's no longer a one-to-one association. The new `BaseAction.render_model_js()` is responsible for writing the JavaScript to render an action model. Every registered action in Python will be registered in JavaScript. The existing `BaseAction.render_js()` (old name currently kept for compatibility reasons) will render the view. This is now only called when the attachment points used on the page, rather than all attachment points. That's handled in the `{% actions_html %}` template tag. The template tag renders the HTML for the attachment point where the template tag is placed, and then sets up a page injection containing the JavaScript views. Then, when rendering the JavaScript models, the injected JavaScript views content will be included. Some of the design of this change is here to facilitate the upcoming improvements to attachment points.	7e8c747c4b614b28203a4125e5d54bea5aaa4236

Diff:

Revision 2 (+384 -300)

Show changes

	reviewboard/actions/base.py
	reviewboard/actions/renderers.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/actions/tests/test_base_action.py
	reviewboard/actions/tests/test_base_action_renderer.py
	reviewboard/actions/tests/test_button_action_renderer.py
	reviewboard/actions/tests/test_default_action_group_renderer.py
	reviewboard/actions/tests/test_default_action_renderer.py
	7 more

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/templates/actions/action_view.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Expected a string and instead saw %.

Column: 2
Error code: W095
```
reviewboard/templates/actions/action_view.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Expected ':' and instead saw 'load'.

Column: 4
Error code: E021
```
reviewboard/templates/actions/action_view.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Expected '}' and instead saw 'actions'.

Column: 9
Error code: E021
```
reviewboard/templates/actions/action_view.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Unrecoverable syntax error. (12% scanned).

Column: 9
Error code: E041
```
reviewboard/templates/actions/action_view.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Expected a JSON value.

Column: 9
Error code: E003
```

Change Summary:

Fixed up incorrect naming in my templatetag update. We're rendering HTML, not action models.

Commits:

	Summary	ID
	Split the rendering of JavaScript action models and views. We now have enough support for multiple views for the same action. It's time to separate out the registration of the models and the views so there's no longer a one-to-one association. The new `BaseAction.render_model_js()` is responsible for writing the JavaScript to render an action model. Every registered action in Python will be registered in JavaScript. The existing `BaseAction.render_js()` (old name currently kept for compatibility reasons) will render the view. This is now only called when the attachment points used on the page, rather than all attachment points. That's handled in the `{% actions_html %}` template tag. The template tag renders the HTML for the attachment point where the template tag is placed, and then sets up a page injection containing the JavaScript views. Then, when rendering the JavaScript models, the injected JavaScript views content will be included. Some of the design of this change is here to facilitate the upcoming improvements to attachment points.	7e8c747c4b614b28203a4125e5d54bea5aaa4236
	Split the rendering of JavaScript action models and views. We now have enough support for multiple views for the same action. It's time to separate out the registration of the models and the views so there's no longer a one-to-one association. The new `BaseAction.render_model_js()` is responsible for writing the JavaScript to render an action model. Every registered action in Python will be registered in JavaScript. The existing `BaseAction.render_js()` (old name currently kept for compatibility reasons) will render the view. This is now only called when the attachment points used on the page, rather than all attachment points. That's handled in the `{% actions_html %}` template tag. The template tag renders the HTML for the attachment point where the template tag is placed, and then sets up a page injection containing the JavaScript views. Then, when rendering the JavaScript models, the injected JavaScript views content will be included. Some of the design of this change is here to facilitate the upcoming improvements to attachment points.	64080b45a209b91ad223a0767d2bc35bd728c884

Diff:

Revision 3 (+386 -302)

Show changes

	reviewboard/actions/base.py
	reviewboard/actions/renderers.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/actions/tests/test_base_action.py
	reviewboard/actions/tests/test_base_action_renderer.py
	reviewboard/actions/tests/test_button_action_renderer.py
	reviewboard/actions/tests/test_default_action_group_renderer.py
	reviewboard/actions/tests/test_default_action_renderer.py
	7 more

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/templates/actions/action_view.js (Diff revision 3)
The issue has been dropped. Show all issues
```
Expected a string and instead saw %.

Column: 2
Error code: W095
```
reviewboard/templates/actions/action_view.js (Diff revision 3)
The issue has been dropped. Show all issues
```
Expected ':' and instead saw 'load'.

Column: 4
Error code: E021
```
reviewboard/templates/actions/action_view.js (Diff revision 3)
The issue has been dropped. Show all issues
```
Expected a JSON value.

Column: 9
Error code: E003
```
reviewboard/templates/actions/action_view.js (Diff revision 3)
The issue has been dropped. Show all issues
```
Expected '}' and instead saw 'actions'.

Column: 9
Error code: E021
```
reviewboard/templates/actions/action_view.js (Diff revision 3)
The issue has been dropped. Show all issues
```
Unrecoverable syntax error. (12% scanned).

Column: 9
Error code: E041
```

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```