Summary

Move more template context to the action renderers.

Review Request #14671 — Created Nov. 5, 2025 and submitted Nov. 28, 2025, 9:47 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

BaseAction.get_dom_element_id() and the JavaScript Action model's
domID attribute are now both deprecated.

There's no good way to shim domID, as it's an action model attribute
and is fundamentally incompatible with per-view dom IDs. We only use
this ourselves for menus, and it doesn't appear we ever actually do
anything with it anywhere, so we're just scrapping it.

Testing Done

Unit tests pass.

Tested that all actions displayed and worked correctly.

Commits

Summary	ID
Move more template context to the action renderers. In preparation for the final changes in separating actions from their views, this change pulls out some of the view-specific state from the actions and sets them in the renderers instead. This includes the `has_parent` flag and `dom_element_id` (which previously required a call to `get_dom_element_id()`). `BaseAction.get_dom_element_id()` and the JavaScript `Action` model's `domID` attribute are now both deprecated. There's no good way to shim `domID`, as it's an action model attribute and is fundamentally incompatible with per-view dom IDs. We only use this ourselves for menus, and it doesn't appear we ever actually do anything with it anywhere, so we're just scrapping it.	d96528a2c2f20b7aca85a1cb0594fbe4b8fe1657

Summary

Move more template context to the action renderers.

In preparation for the final changes in separating actions from their views, this change pulls out some of the view-specific state from the actions and sets them in the renderers instead. This includes the `has_parent` flag and `dom_element_id` (which previously required a call to `get_dom_element_id()`). `BaseAction.get_dom_element_id()` and the JavaScript `Action` model's `domID` attribute are now both deprecated. There's no good way to shim `domID`, as it's an action model attribute and is fundamentally incompatible with per-view dom IDs. We only use this ourselves for menus, and it doesn't appear we ever actually do anything with it anywhere, so we're just scrapping it.

d96528a2c2f20b7aca85a1cb0594fbe4b8fe1657

Issues

Description	From	Last Updated
Your description says this is deprecated, but it's not marked as such, and the renderer is still calling it. Either …	david	Nov. 7, 2025, 12:50 p.m.
Oh, also, should we have a version added block here explaining that this method was moved from BaseGroupAction?	david	Nov. 7, 2025, 2:19 p.m.

flake8 passed.

JSHint passed.

Change Summary:

Removed the has_parent context variable from BaseAction.get_extra_context() in favor of that from the renderer.

Commits:

	Summary	ID
	Move more template context to the action renderers. In preparation for the final changes in separating actions from their views, this change pulls out some of the view-specific state from the actions and sets them in the renderers instead. This includes the `has_parent` flag and `dom_element_id` (which previously required a call to `get_dom_element_id()`). `BaseAction.get_dom_element_id()` and the JavaScript `Action` model's `domID` attribute are now both deprecated. There's no good way to shim `domID`, as it's an action model attribute and is fundamentally incompatible with per-view dom IDs. We only use this ourselves for menus, and it doesn't appear we ever actually do anything with it anywhere, so we're just scrapping it.	a8d69a2dfdcee94676208d864983ed0c04b43938
	Move more template context to the action renderers. In preparation for the final changes in separating actions from their views, this change pulls out some of the view-specific state from the actions and sets them in the renderers instead. This includes the `has_parent` flag and `dom_element_id` (which previously required a call to `get_dom_element_id()`). `BaseAction.get_dom_element_id()` and the JavaScript `Action` model's `domID` attribute are now both deprecated. There's no good way to shim `domID`, as it's an action model attribute and is fundamentally incompatible with per-view dom IDs. We only use this ourselves for menus, and it doesn't appear we ever actually do anything with it anywhere, so we're just scrapping it.	d96528a2c2f20b7aca85a1cb0594fbe4b8fe1657

Diff:

Revision 2 (+200 -204)

Show changes

	reviewboard/actions/base.py
	reviewboard/actions/renderers.py
	reviewboard/actions/tests/test_base_action.py
	reviewboard/actions/tests/test_base_action_renderer.py
	reviewboard/actions/tests/test_group_action.py
	reviewboard/static/rb/js/common/actions/views/menuActionView.ts
	reviewboard/templates/accounts/account_menu_action.html
	reviewboard/templates/actions/action_base.html
	4 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/actions/base.py (Diff revision 2)

The issue has been dropped. Show all issues

Your description says this is deprecated, but it's not marked as such, and the renderer is still calling it.

Either we should probably move the actual functionality out of here and actually mark it as deprecated, or change the description.

chipx86

Nov. 7, 2025, 12:50 p.m.

It will be in one of the follow-up changes. It's necessary for this change to stay functional.

reviewboard/actions/renderers.py (Diff revision 2)

Might be nicer to write this as:

return {
    **action.get_extra_context(request=request,
                               context=context),
    'action': action,
    'action_renderer': self,
    'attachment_point_id': action.attachment,
    ...
}

chipx86

Nov. 7, 2025, 12:50 p.m.

This wastes a dictionary. We already have a new dictionary to populate. Better to update that than to create a new one and copy from the other new one into it.

chipx86

Nov. 7, 2025, 12:51 p.m.

Actually I guess either way we have a throw-away dictionary.

chipx86

Nov. 7, 2025, 1:14 p.m.

These are cheap enough not to worry too much about this in practice, but I got curious and decided to test things:

def test1():
    a = {
      'a': 1,
      'b': 2,
      'c': 3,
    }
    a.update({
        'd': 4,
        'e': 5,
        'f': 6,
    })

def test2():
  a = {
      'a': 1,
      'b': 2,
      'c': 3,
  }
  b = {
      **a,
      'd': 4,
      'e': 5,
      'f': 6,
  }

def test3():
  a = {
      'a': 1,
      'b': 2,
      'c': 3,
  }
  a['d'] = 4
  a['e'] = 5
  a['f'] = 6


Results:



Python
dict.update()
{**dict}:
dict[...]




3.8
0.002631750000000002
0.002332332999999999
0.0015086249999999995


3.9
0.002569000000000002
0.0029753749999999954
0.001817375000000003


3.10
0.002573375008068979
0.0027398329984862357
0.0016685000155121088


3.11
0.0025507090031169355
0.00264904199866578
0.0016354580002371222


3.12
0.002828041004249826
0.0030305830005090684
0.001743832981446758


3.13
0.003182916989317164
0.003386458003660664
0.00205779101816006


3.14
0.003014665999216959
0.0031265830039046705
0.002212874998804182



They all get slower over time, which is sad to see, but dict.update() is the fastest of the two, and of course setting keys individually avoids the iteration and extra dictionary so it's much faster.

Python	`dict.update()`	`{**dict}:`	`dict[...]`
3.8	0.002631750000000002	0.002332332999999999	0.0015086249999999995
3.9	0.002569000000000002	0.0029753749999999954	0.001817375000000003
3.10	0.002573375008068979	0.0027398329984862357	0.0016685000155121088
3.11	0.0025507090031169355	0.00264904199866578	0.0016354580002371222
3.12	0.002828041004249826	0.0030305830005090684	0.001743832981446758
3.13	0.003182916989317164	0.003386458003660664	0.00205779101816006
3.14	0.003014665999216959	0.0031265830039046705	0.002212874998804182

reviewboard/actions/renderers.py (Diff revision 2)

The issue has been dropped. Show all issues

Oh, also, should we have a version added block here explaining that this method was moved from BaseGroupAction?

chipx86

Nov. 7, 2025, 2:19 p.m.

BaseGroupAction and action renderers are all new in 7.1, so I don't think we need to.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-7.1.x (12dc085)