♻️(backend) refactor get_select_options to take definitions dict

This will allow us to simplify the get_abilities method. It is also
more efficient because we have computed this definitions dict and
the the get_select_options method was doing the conversion again.

Author: sampaccoud

src/backend/core/choices.py

@@ -61,63 +61,52 @@ class LinkReachChoices(PriorityTextChoices):
     )  # Any authenticated user can access the document
     PUBLIC = "public", _("Public")  # Even anonymous users can access the document
 
+
     @classmethod
-    def get_select_options(cls, ancestors_links):
+    def get_select_options(cls, link_reach, link_role):
         """
         Determines the valid select options for link reach and link role depending on the
-        list of ancestors' link reach/role.
-        Args:
-            ancestors_links: List of dictionaries, each with 'link_reach' and 'link_role' keys
-                             representing the reach and role of ancestors links.
+        list of ancestors' link reach/role definitions.
         Returns:
             Dictionary mapping possible reach levels to their corresponding possible roles.
         """
         # If no ancestors, return all options
-        if not ancestors_links:
+        if not link_reach:
             return {
                 reach: LinkRoleChoices.values if reach != cls.RESTRICTED else None
                 for reach in cls.values
             }
 
-        # Initialize result with all possible reaches and role options as sets
+        # Initialize the result for all reaches with possible roles
         result = {
             reach: set(LinkRoleChoices.values) if reach != cls.RESTRICTED else None
             for reach in cls.values
         }
 
-        # Group roles by reach level
-        reach_roles = defaultdict(set)
-        for link in ancestors_links:
-            reach_roles[link["link_reach"]].add(link["link_role"])
-
-        # Rule 1: public/editor → override everything
-        if LinkRoleChoices.EDITOR in reach_roles.get(cls.PUBLIC, set()):
-            return {cls.PUBLIC: [LinkRoleChoices.EDITOR]}
-
-        # Rule 2: authenticated/editor
-        if LinkRoleChoices.EDITOR in reach_roles.get(cls.AUTHENTICATED, set()):
-            result[cls.AUTHENTICATED].discard(LinkRoleChoices.READER)
-            result.pop(cls.RESTRICTED, None)
-
-        # Rule 3: public/reader
-        if LinkRoleChoices.READER in reach_roles.get(cls.PUBLIC, set()):
-            result.pop(cls.AUTHENTICATED, None)
-            result.pop(cls.RESTRICTED, None)
-
-        # Rule 4: authenticated/reader
-        if LinkRoleChoices.READER in reach_roles.get(cls.AUTHENTICATED, set()):
-            result.pop(cls.RESTRICTED, None)
-
-        # Clean up: remove empty entries and convert sets to ordered lists
-        cleaned = {}
-        for reach in cls.values:
-            if reach in result:
-                if result[reach]:
-                    cleaned[reach] = [
-                        r for r in LinkRoleChoices.values if r in result[reach]
-                    ]
-                else:
-                    # Could be [] or None (for RESTRICTED reach)
-                    cleaned[reach] = result[reach]
-
-        return cleaned
+        # Handle special rules directly with early returns for efficiency
+
+        if link_role == LinkRoleChoices.EDITOR:
+            # Rule 1: public/editor → override everything
+            if link_reach == cls.PUBLIC:
+                return {cls.PUBLIC: [LinkRoleChoices.EDITOR]}
+
+            # Rule 2: authenticated/editor
+            if link_reach == cls.AUTHENTICATED:
+                result[cls.AUTHENTICATED].discard(LinkRoleChoices.READER)
+                result.pop(cls.RESTRICTED, None)
+
+        if link_role == LinkRoleChoices.READER:
+            # Rule 3: public/reader
+            if link_reach == cls.PUBLIC:
+                result.pop(cls.AUTHENTICATED, None)
+                result.pop(cls.RESTRICTED, None)
+
+            # Rule 4: authenticated/reader
+            if link_reach == cls.AUTHENTICATED:
+                result.pop(cls.RESTRICTED, None)
+
+        # Convert sets to ordered lists where applicable
+        return {
+            reach: sorted(roles, key=LinkRoleChoices.get_priority) if roles else roles
+            for reach, roles in result.items()
+        }

src/backend/core/models.py

@@ -818,7 +818,9 @@ def get_abilities(self, user, ancestors_links=None):
             "ancestors_links_definitions": {
                 k: list(v) for k, v in ancestors_links_definitions.items()
             },
-            "link_select_options": LinkReachChoices.get_select_options(ancestors_links),
+            "link_select_options": LinkReachChoices.get_select_options(
+                ancestors_links_definitions
+            ),
             "tree": can_get,
             "update": can_update,
             "versions_destroy": is_owner_or_admin,

src/backend/core/tests/documents/test_api_documents_retrieve.py

@@ -1,6 +1,7 @@
 """
 Tests for Documents API endpoint in impress's core app: retrieve
 """
+# pylint: disable=too-many-lines
 
 import random
 from datetime import timedelta
@@ -92,6 +93,7 @@ def test_api_documents_retrieve_anonymous_public_parent():
 
     assert response.status_code == 200
     links = document.get_ancestors().values("link_reach", "link_role")
+    links_definitions = document.get_ancestors_links_definitions(links)
     assert response.json() == {
         "id": str(document.id),
         "abilities": {
@@ -115,7 +117,9 @@ def test_api_documents_retrieve_anonymous_public_parent():
             "favorite": False,
             "invite_owner": False,
             "link_configuration": False,
-            "link_select_options": models.LinkReachChoices.get_select_options(links),
+            "link_select_options": models.LinkReachChoices.get_select_options(
+                links_definitions
+            ),
             "media_auth": True,
             "media_check": True,
             "move": False,
@@ -269,6 +273,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea
 
     assert response.status_code == 200
     links = document.get_ancestors().values("link_reach", "link_role")
+    links_definitions = document.get_ancestors_links_definitions(links)
     assert response.json() == {
         "id": str(document.id),
         "abilities": {
@@ -291,10 +296,12 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea
             "favorite": True,
             "invite_owner": False,
             "link_configuration": False,
-            "link_select_options": models.LinkReachChoices.get_select_options(links),
+            "link_select_options": models.LinkReachChoices.get_select_options(
+                links_definitions
+            ),
+            "move": False,
             "media_auth": True,
             "media_check": True,
-            "move": False,
             "partial_update": grand_parent.link_role == "editor",
             "restore": False,
             "retrieve": True,
@@ -454,6 +461,7 @@ def test_api_documents_retrieve_authenticated_related_parent():
     )
     assert response.status_code == 200
     links = document.get_ancestors().values("link_reach", "link_role")
+    links_definitions = document.get_ancestors_links_definitions(links)
     ancestors_roles = list({grand_parent.link_role, parent.link_role})
     assert response.json() == {
         "id": str(document.id),
@@ -474,7 +482,9 @@ def test_api_documents_retrieve_authenticated_related_parent():
             "favorite": True,
             "invite_owner": access.role == "owner",
             "link_configuration": access.role in ["administrator", "owner"],
-            "link_select_options": models.LinkReachChoices.get_select_options(links),
+            "link_select_options": models.LinkReachChoices.get_select_options(
+                links_definitions
+            ),
             "media_auth": True,
             "media_check": True,
             "move": access.role in ["administrator", "owner"],

src/backend/core/tests/test_models_documents.py

@@ -1184,124 +1184,70 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries):
 
 
 @pytest.mark.parametrize(
-    "ancestors_links, select_options",
+    "reach, role, select_options",
     [
         # One ancestor
         (
-            [{"link_reach": "public", "link_role": "reader"}],
+            "public",
+            "reader",
             {
                 "public": ["reader", "editor"],
             },
         ),
-        ([{"link_reach": "public", "link_role": "editor"}], {"public": ["editor"]}),
+        ("public", "editor", {"public": ["editor"]}),
         (
-            [{"link_reach": "authenticated", "link_role": "reader"}],
+            "authenticated",
+            "reader",
             {
                 "authenticated": ["reader", "editor"],
                 "public": ["reader", "editor"],
             },
         ),
         (
-            [{"link_reach": "authenticated", "link_role": "editor"}],
+            "authenticated",
+            "editor",
             {"authenticated": ["editor"], "public": ["reader", "editor"]},
         ),
         (
-            [{"link_reach": "restricted", "link_role": "reader"}],
+            "restricted",
+            "reader",
             {
                 "restricted": None,
                 "authenticated": ["reader", "editor"],
                 "public": ["reader", "editor"],
             },
         ),
         (
-            [{"link_reach": "restricted", "link_role": "editor"}],
+            "restricted",
+            "editor",
             {
                 "restricted": None,
                 "authenticated": ["reader", "editor"],
                 "public": ["reader", "editor"],
             },
         ),
-        # Multiple ancestors with different roles
-        (
-            [
-                {"link_reach": "public", "link_role": "reader"},
-                {"link_reach": "public", "link_role": "editor"},
-            ],
-            {"public": ["editor"]},
-        ),
-        (
-            [
-                {"link_reach": "authenticated", "link_role": "reader"},
-                {"link_reach": "authenticated", "link_role": "editor"},
-            ],
-            {"authenticated": ["editor"], "public": ["reader", "editor"]},
-        ),
-        (
-            [
-                {"link_reach": "restricted", "link_role": "reader"},
-                {"link_reach": "restricted", "link_role": "editor"},
-            ],
-            {
-                "restricted": None,
-                "authenticated": ["reader", "editor"],
-                "public": ["reader", "editor"],
-            },
-        ),
-        # Multiple ancestors with different reaches
+        # No ancestors (edge case)
         (
-            [
-                {"link_reach": "authenticated", "link_role": "reader"},
-                {"link_reach": "public", "link_role": "reader"},
-            ],
+            "public",
+            None,
             {
                 "public": ["reader", "editor"],
+                "authenticated": ["reader", "editor"],
+                "restricted": None,
             },
         ),
         (
-            [
-                {"link_reach": "restricted", "link_role": "reader"},
-                {"link_reach": "authenticated", "link_role": "reader"},
-                {"link_reach": "public", "link_role": "reader"},
-            ],
+            None,
+            "reader",
             {
                 "public": ["reader", "editor"],
-            },
-        ),
-        # Multiple ancestors with mixed reaches and roles
-        (
-            [
-                {"link_reach": "authenticated", "link_role": "editor"},
-                {"link_reach": "public", "link_role": "reader"},
-            ],
-            {"public": ["reader", "editor"]},
-        ),
-        (
-            [
-                {"link_reach": "authenticated", "link_role": "reader"},
-                {"link_reach": "public", "link_role": "editor"},
-            ],
-            {"public": ["editor"]},
-        ),
-        (
-            [
-                {"link_reach": "restricted", "link_role": "editor"},
-                {"link_reach": "authenticated", "link_role": "reader"},
-            ],
-            {
                 "authenticated": ["reader", "editor"],
-                "public": ["reader", "editor"],
+                "restricted": None,
             },
         ),
         (
-            [
-                {"link_reach": "restricted", "link_role": "reader"},
-                {"link_reach": "authenticated", "link_role": "editor"},
-            ],
-            {"authenticated": ["editor"], "public": ["reader", "editor"]},
-        ),
-        # No ancestors (edge case)
-        (
-            [],
+            None,
+            None,
             {
                 "public": ["reader", "editor"],
                 "authenticated": ["reader", "editor"],
@@ -1310,9 +1256,9 @@ def test_models_documents_restore_complex_bis(django_assert_num_queries):
         ),
     ],
 )
-def test_models_documents_get_select_options(ancestors_links, select_options):
+def test_models_documents_get_select_options(reach, role, select_options):
     """Validate that the "get_select_options" method operates as expected."""
-    assert models.LinkReachChoices.get_select_options(ancestors_links) == select_options
+    assert models.LinkReachChoices.get_select_options(reach, role) == select_options
 
 
 def test_models_documents_compute_ancestors_links_no_highest_readable():