flake8
passed.
JSHint
passed.
Speed up looking up actions by attachment point.
Review Request #14639 — Created Oct. 16, 2025 and updated
When populating the UI, we fetch actions based on various attachment
points. When we do this, the registry has to iterate through every
action every time, returning each one matching the attachment point.
This is algorithmically slow, and only gets worse as we expand usage of
actions.Now, when registering an action, the action gets placed in a cache of
attachment points to actions. Unregistering will remove that action from
the cache. This makes lookup very fast.There's also a bit of future-proofing thrown in for upcoming support for
actions that attach to multiple attachment points (part of a larger set
of work for enhanced actions in Review Board 7.1).
Unit tests pass.
Tested various parts of the UI, making sure the right actions are in
the right places.
| Summary | ID |
|---|---|
| 145b2020d753a75f8bc15d98b170e4703441afb9 |
| Description | From | Last Updated |
|---|---|---|
|
We should use a walrus operator here to make a variable out of action.parent_id, like you did below. |
 maubin
|
|
|
Can use action_id here too. |
maubin
|
|
|
Might as well do the walrus operator here too. |
maubin
|
|
|
I'm assuming this is part of the future proofing, where in the future action.attachment can possibly be a list of … |
maubin
|
|
|
Same notes here about the attachment variable. |
maubin
|
-
-
We should use a walrus operator here to make a variable out of
action.parent_id, like you did below. -
-
-
I'm assuming this is part of the future proofing, where in the future
action.attachmentcan possibly be a list of strings?We should name the attachments list
attachmentsinstead of having all variables beattachment. Note that the for loop variable is also namedattachment. After the loop runs, the attachment list variable would now point to the last item in the list instead of the list itself. Doesn't really matter since we don't use the variable again, but its confusing and will cause problems in case we do use it in the future.How about something like this
attachments = action.attachment if attachments: if isinstance(attachments, str): attachments = [attachments] for attachment in attachments: ... -