From a57a538b92d69b4a944ec34df5bd3e957c58320d Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Mon, 6 Apr 2026 09:29:00 -0500 Subject: [PATCH] Fix model_actions registry to use set operations The registry was changed to defaultdict(set) but the registration code still used list methods. Update .append() to .add() and fix tests to use set-compatible access patterns. --- netbox/utilities/permissions.py | 3 +-- netbox/utilities/tests/test_permissions.py | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/netbox/utilities/permissions.py b/netbox/utilities/permissions.py index a745a3347..3e0a18e6f 100644 --- a/netbox/utilities/permissions.py +++ b/netbox/utilities/permissions.py @@ -58,8 +58,7 @@ def register_model_actions(model: type[Model], actions: list[ModelAction | str]) raise ValueError("Action name must not be empty.") if action.name in RESERVED_ACTIONS: raise ValueError(f"'{action.name}' is a reserved action and cannot be registered.") - if action not in registry['model_actions'][label]: - registry['model_actions'][label].append(action) + registry['model_actions'][label].add(action) def get_action_model_map(model_actions): diff --git a/netbox/utilities/tests/test_permissions.py b/netbox/utilities/tests/test_permissions.py index c1518cf3e..bc00ea6b6 100644 --- a/netbox/utilities/tests/test_permissions.py +++ b/netbox/utilities/tests/test_permissions.py @@ -52,16 +52,17 @@ class RegisterModelActionsTest(TestCase): ]) actions = registry['model_actions']['dcim.site'] self.assertEqual(len(actions), 1) - self.assertEqual(actions[0].name, 'test_action') - self.assertEqual(actions[0].help_text, 'Test help') + action = next(iter(actions)) + self.assertEqual(action.name, 'test_action') + self.assertEqual(action.help_text, 'Test help') def test_register_string_actions(self): register_model_actions(Site, ['action1', 'action2']) actions = registry['model_actions']['dcim.site'] self.assertEqual(len(actions), 2) - self.assertIsInstance(actions[0], ModelAction) - self.assertEqual(actions[0].name, 'action1') - self.assertEqual(actions[1].name, 'action2') + action_names = {a.name for a in actions} + self.assertEqual(action_names, {'action1', 'action2'}) + self.assertTrue(all(isinstance(a, ModelAction) for a in actions)) def test_register_mixed_actions(self): register_model_actions(Site, [ @@ -70,16 +71,17 @@ class RegisterModelActionsTest(TestCase): ]) actions = registry['model_actions']['dcim.site'] self.assertEqual(len(actions), 2) - self.assertEqual(actions[0].help_text, 'Has help') - self.assertEqual(actions[1].help_text, '') + actions_by_name = {a.name: a for a in actions} + self.assertEqual(actions_by_name['with_help'].help_text, 'Has help') + self.assertEqual(actions_by_name['without_help'].help_text, '') def test_multiple_registrations_append(self): register_model_actions(Site, [ModelAction('first')]) register_model_actions(Site, [ModelAction('second')]) actions = registry['model_actions']['dcim.site'] self.assertEqual(len(actions), 2) - self.assertEqual(actions[0].name, 'first') - self.assertEqual(actions[1].name, 'second') + action_names = {a.name for a in actions} + self.assertEqual(action_names, {'first', 'second'}) def test_duplicate_registration_ignored(self): register_model_actions(Site, [ModelAction('sync')])