update get_definitions() for scope=None (#1535)

bihealth · Jan 9, 2025 · b50794c · b50794c
1 parent 9be43c2
commit b50794c
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 9 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -26,6 +26,7 @@ Changed
 - **Projectroles**
     - Deprecate ``get_user_display_name()``, use ``SODARUser.get_display_name()`` (#1487)
     - Deprecate declaring app setting definitions as dict (#1456)
+    - Allow ``scope=None`` in ``AppSettingAPI.get_definitions()`` (#1535)
 
 Removed
 -------

diff --git a/docs/source/major_changes.rst b/docs/source/major_changes.rst
@@ -18,6 +18,7 @@ Release Highlights
 
 - Add app setting type constants
 - Add app setting definition as objects
+- Update app settings API
 - Remove support for features deprecated in v1.0
 - Remove squashed migrations
 

diff --git a/projectroles/app_settings.py b/projectroles/app_settings.py
@@ -711,15 +711,16 @@ def get_definition(cls, name, plugin=None, plugin_name=None):
     @classmethod
     def get_definitions(
         cls,
-        scope,
+        scope=None,
         plugin=None,
         plugin_name=None,
         user_modifiable=False,
     ):
         """
-        Return app setting definitions of a specific scope from a plugin.
+        Return app setting definitions from a plugin, optionally limited by
+        scope.
 
-        :param scope: PROJECT, USER or PROJECT_USER
+        :param scope: String or None
         :param plugin: Plugin object or None
         :param plugin_name: App plugin name (string, equals "name" in plugin)
         :param user_modifiable: Only return non-superuser modifiable settings if
@@ -728,12 +729,14 @@ def get_definitions(
         :raise: ValueError if scope is invalid or if neither plugin_name nor
                 plugin are set
         """
-        PluginAppSettingDef.validate_scope(scope)
+        if scope:
+            PluginAppSettingDef.validate_scope(scope)
         defs = cls._get_defs(plugin, plugin_name)
         return {
             k: v
             for k, v in defs.items()
-            if v.scope == scope and (not user_modifiable or v.user_modifiable)
+            if (not scope or v.scope == scope)
+            and (not user_modifiable or v.user_modifiable)
         }
 
     @classmethod

diff --git a/projectroles/tests/test_app_settings.py b/projectroles/tests/test_app_settings.py
@@ -289,6 +289,8 @@ def setUp(self):
         )
         # Init test data
         self.settings = self.init_app_settings()
+        example_plugin = get_app_plugin(EXAMPLE_APP_NAME)
+        self.example_defs = example_plugin.app_settings
 
     def test_get(self):
         """Test get()"""
@@ -730,7 +732,10 @@ def test_get_definitions_project(self):
             APP_SETTING_SCOPE_PROJECT, plugin_name=EXAMPLE_APP_NAME
         )
         self.assertIsInstance(defs, dict)
-        self.assertEqual(len(defs), 12)
+        ex_defs = [
+            d for d in self.example_defs if d.scope == APP_SETTING_SCOPE_PROJECT
+        ]
+        self.assertEqual(len(defs), len(ex_defs))
         self.assertTrue(
             all([d.scope == APP_SETTING_SCOPE_PROJECT for d in defs.values()])
         )
@@ -740,7 +745,10 @@ def test_get_definitions_user(self):
         defs = app_settings.get_definitions(
             APP_SETTING_SCOPE_USER, plugin_name=EXAMPLE_APP_NAME
         )
-        self.assertEqual(len(defs), 9)
+        ex_defs = [
+            d for d in self.example_defs if d.scope == APP_SETTING_SCOPE_USER
+        ]
+        self.assertEqual(len(defs), len(ex_defs))
         self.assertTrue(
             all([d.scope == APP_SETTING_SCOPE_USER for d in defs.values()])
         )
@@ -750,7 +758,12 @@ def test_get_definitions_project_user(self):
         defs = app_settings.get_definitions(
             APP_SETTING_SCOPE_PROJECT_USER, plugin_name=EXAMPLE_APP_NAME
         )
-        self.assertEqual(len(defs), 6)
+        ex_defs = [
+            d
+            for d in self.example_defs
+            if d.scope == APP_SETTING_SCOPE_PROJECT_USER
+        ]
+        self.assertEqual(len(defs), len(ex_defs))
         self.assertTrue(
             all(
                 [
@@ -765,9 +778,19 @@ def test_get_definitions_site(self):
         defs = app_settings.get_definitions(
             APP_SETTING_SCOPE_SITE, plugin_name=EXAMPLE_APP_NAME
         )
-        self.assertEqual(len(defs), 1)
+        ex_defs = [
+            d for d in self.example_defs if d.scope == APP_SETTING_SCOPE_SITE
+        ]
+        self.assertEqual(len(defs), len(ex_defs))
         self.assertTrue(list(defs.values())[0].scope, APP_SETTING_SCOPE_SITE)
 
+    def test_get_definitions_no_scope(self):
+        """Test get_definitions() with no scope"""
+        defs = app_settings.get_definitions(
+            scope=None, plugin_name=EXAMPLE_APP_NAME
+        )
+        self.assertEqual(len(defs), len(self.example_defs))
+
     def test_get_definitions_modifiable(self):
         """Test get_definitions() with user_modifiable arg"""
         defs = app_settings.get_definitions(