Summary

Store and organize action views by attachment point.

Review Request #14663 — Created Oct. 31, 2025 and updated Nov. 11, 2025, 8:09 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

ActionView now takes the attachment point that the view is placed in,
for easy lookup and for registration.

The views are no longer stored in one long list. Instead, they're
organized by the attachment point. This will ultimately allow for
multiple views for the same action in different attachment points, and
to enable looking up the correct action within the right attachment
point.

Testing Done

Unit tests passed.

Tested that all actions, including menus and Quick Access actions,
all worked and expected.

Tested that Quick Access actions could be customized.

Commits

Summary	ID
Store and organize action views by attachment point. `ActionView` now takes the attachment point that the view is placed in, for easy lookup and for registration. The views are no longer stored in one long list. Instead, they're organized by the attachment point. This will ultimately allow for multiple views for the same action in different attachment points, and to enable looking up the correct action within the right attachment point.	179aed05a8866af4a7d9e64eac194c8889c3c184

Summary

Store and organize action views by attachment point.

`ActionView` now takes the attachment point that the view is placed in, for easy lookup and for registration. The views are no longer stored in one long list. Instead, they're organized by the attachment point. This will ultimately allow for multiple views for the same action in different attachment points, and to enable looking up the correct action within the right attachment point.

179aed05a8866af4a7d9e64eac194c8889c3c184

Issues

Description	From	Last Updated
This needs a "Version Changed" block.	david	Nov. 10, 2025, 3:28 a.m.
I'm not sure I have all your changes in my head at the same time, so it's possible that this …	david	Nov. 10, 2025, 3:33 a.m.
This needs to be added to the Args section.	maubin	Nov. 10, 2025, 3:27 a.m.
It might be nice to fail more gracefully if attachmentPointID is not a valid key.	david	Nov. 10, 2025, 3:32 a.m.
It might be nice to fail more gracefully if attachmentPointID is not a valid key. Right now the call to …	david	Nov. 10, 2025, 3:32 a.m.
Should we \|escapejs this? I guess there could be a scenario where a malicious action author sets the attachment_point_id to …	maubin	Nov. 10, 2025, 3:27 a.m.

flake8 passed.

JSHint passed.

reviewboard/static/rb/js/common/views/pageView.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
This needs to be added to the Args section.
```

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

The issue has been dropped. Show all issues

Should we |escapejs this? I guess there could be a scenario where a malicious action author sets the attachment_point_id to something hacky.

chipx86 Nov. 10, 2025, 3:37 a.m.

Yep, very good point. Ended up fixing this in another change to include as part of the new JS state data. It's a bit easier to do it there, with further cleanup after this change.

reviewboard/static/rb/js/common/views/pageView.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
This needs a "Version Changed" block.
```

reviewboard/static/rb/js/common/views/pageView.ts (Diff revision 1)

The issue has been resolved. Show all issues

I'm not sure I have all your changes in my head at the same time, so it's possible that this is addressed in a different change, but I see that MenuActionView calls page.getActionView() with just the child ID, and there's nothing about that in this change. Is that changed elsewhere?

chipx86 Nov. 10, 2025, 3:37 a.m.

This was updated, but in the wrong change. It ended up in /r/14641/. I'll fix that up to be in here.

reviewboard/static/rb/js/common/views/pageView.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
It might be nice to fail more gracefully if attachmentPointID is not a valid key.
```

reviewboard/static/rb/js/common/views/pageView.ts (Diff revision 1)

The issue has been resolved. Show all issues

It might be nice to fail more gracefully if attachmentPointID is not a valid key. Right now the call to _.filter will probably error out.

Change Summary:


Added changes to MenuActionView that ended up in a previous change.
Added better handling of invalid action point IDs or action IDs in PageView.
Added missing version docs.

Commits:

	Summary	ID
	Store and organize action views by attachment point. `ActionView` now takes the attachment point that the view is placed in, for easy lookup and for registration. The views are no longer stored in one long list. Instead, they're organized by the attachment point. This will ultimately allow for multiple views for the same action in different attachment points, and to enable looking up the correct action within the right attachment point.	0acd57412d565c59c180330c9250261a7dc23cd2
	Store and organize action views by attachment point. `ActionView` now takes the attachment point that the view is placed in, for easy lookup and for registration. The views are no longer stored in one long list. Instead, they're organized by the attachment point. This will ultimately allow for multiple views for the same action in different attachment points, and to enable looking up the correct action within the right attachment point.	179aed05a8866af4a7d9e64eac194c8889c3c184

Diff:

Revision 2 (+190 -28)

Show changes

	reviewboard/actions/renderers.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
	reviewboard/actions/tests/test_detailed_menu_item_action_renderer.py
	reviewboard/actions/tests/test_group_action.py
	7 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```