🐛Link role update

CHANGELOG.md

@@ -22,6 +22,7 @@ and this project adheres to
 
 - 🐛(makefile) Windows compatibility fix for Docker volume mounting #1264
 - 🐛(minio) fix user permission error with Minio and Windows #1264
+- 🐛link role update #1287
 
 
 ## [3.5.0] - 2025-07-31

src/backend/core/api/serializers.py

@@ -486,13 +486,77 @@ class LinkDocumentSerializer(serializers.ModelSerializer):
     We expose it separately from document in order to simplify and secure access control.
     """
 
+    link_reach = serializers.ChoiceField(
+        choices=models.LinkReachChoices.choices, required=True
+    )
+
     class Meta:
         model = models.Document
         fields = [
             "link_role",
             "link_reach",
         ]
 
+    def validate(self, attrs):
+        """Validate that link_role and link_reach are compatible using get_select_options."""
+        link_reach = attrs.get("link_reach")
+        link_role = attrs.get("link_role")
+
+        if not link_reach:
+            raise serializers.ValidationError(
+                {"link_reach": _("This field is required.")}
+            )
+
+        # Get available options based on ancestors' link definition
+        available_options = models.LinkReachChoices.get_select_options(
+            **self.instance.ancestors_link_definition
+        )
+
+        # Validate link_reach is allowed
+        if link_reach not in available_options:
+            msg = _(
+                "Link reach '%(link_reach)s' is not allowed based on parent document configuration."
+            )
+            raise serializers.ValidationError(
+                {"link_reach": msg % {"link_reach": link_reach}}
+            )
+
+        # Validate link_role is compatible with link_reach
+        allowed_roles = available_options[link_reach]
+
+        if link_reach == models.LinkReachChoices.RESTRICTED:
+            # For restricted reach, link_role is ignored but
+            # we shouldn't allow meaningful roles to be set
+            if link_role is not None and link_role in [
+                models.LinkRoleChoices.READER,
+                models.LinkRoleChoices.EDITOR,
+            ]:
+                msg = _(
+                    "Cannot set link_role when link_reach is 'restricted'. "
+                    "Link role must be null for restricted reach."
+                )
+                raise serializers.ValidationError({"link_role": msg})
+        # For non-restricted reach, validate link_role is in allowed roles
+        elif link_role is not None and link_role not in allowed_roles:
+            msg = _(
+                "Link role '%(link_role)s' is not allowed for link reach '%(link_reach)s'. "
+                "Allowed roles: %(allowed_roles)s"
+            )
+            raise serializers.ValidationError(
+                {
+                    "link_role": msg
+                    % {
+                        "link_role": link_role,
+                        "link_reach": link_reach,
+                        "allowed_roles": ", ".join(allowed_roles)
+                        if allowed_roles
+                        else "none",
+                    }
+                }
+            )
+
+        return attrs
+
 
 class DocumentDuplicationSerializer(serializers.Serializer):
     """

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

@@ -133,7 +133,10 @@ def test_api_documents_link_configuration_update_authenticated_related_success(
     client = APIClient()
     client.force_login(user)
 
-    document = factories.DocumentFactory()
+    document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.AUTHENTICATED,
+        link_role=models.LinkRoleChoices.READER,
+    )
     if via == USER:
         factories.UserDocumentAccessFactory(document=document, user=user, role=role)
     elif via == TEAM:
@@ -143,7 +146,10 @@ def test_api_documents_link_configuration_update_authenticated_related_success(
         )
 
     new_document_values = serializers.LinkDocumentSerializer(
-        instance=factories.DocumentFactory()
+        instance=factories.DocumentFactory(
+            link_reach=models.LinkReachChoices.PUBLIC,
+            link_role=models.LinkRoleChoices.EDITOR,
+        )
     ).data
 
     with mock_reset_connections(document.id):
@@ -158,3 +164,240 @@ def test_api_documents_link_configuration_update_authenticated_related_success(
         document_values = serializers.LinkDocumentSerializer(instance=document).data
         for key, value in document_values.items():
             assert value == new_document_values[key]
+
+
+def test_api_documents_link_configuration_update_role_restricted_forbidden():
+    """
+    Test that trying to set link_role on a document with restricted link_reach
+    returns a validation error.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.RESTRICTED,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    # Try to set a meaningful role on a restricted document
+    new_data = {
+        "link_reach": models.LinkReachChoices.RESTRICTED,
+        "link_role": models.LinkRoleChoices.EDITOR,
+    }
+
+    response = client.put(
+        f"/api/v1.0/documents/{document.id!s}/link-configuration/",
+        new_data,
+        format="json",
+    )
+
+    assert response.status_code == 400
+    assert "link_role" in response.json()
+    assert (
+        "Cannot set link_role when link_reach is 'restricted'"
+        in response.json()["link_role"][0]
+    )
+
+
+def test_api_documents_link_configuration_update_link_reach_required():
+    """
+    Test that link_reach is required when updating link configuration.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.PUBLIC,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    # Try to update without providing link_reach
+    new_data = {"link_role": models.LinkRoleChoices.EDITOR}
+
+    response = client.put(
+        f"/api/v1.0/documents/{document.id!s}/link-configuration/",
+        new_data,
+        format="json",
+    )
+
+    assert response.status_code == 400
+    assert "link_reach" in response.json()
+    assert "This field is required" in response.json()["link_reach"][0]
+
+
+def test_api_documents_link_configuration_update_restricted_without_role_success(
+    mock_reset_connections,  # pylint: disable=redefined-outer-name
+):
+    """
+    Test that setting link_reach to restricted without specifying link_role succeeds.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.PUBLIC,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    # Only specify link_reach, not link_role
+    new_data = {
+        "link_reach": models.LinkReachChoices.RESTRICTED,
+    }
+
+    with mock_reset_connections(document.id):
+        response = client.put(
+            f"/api/v1.0/documents/{document.id!s}/link-configuration/",
+            new_data,
+            format="json",
+        )
+
+        assert response.status_code == 200
+        document.refresh_from_db()
+        assert document.link_reach == models.LinkReachChoices.RESTRICTED
+
+
+@pytest.mark.parametrize(
+    "reach", [models.LinkReachChoices.PUBLIC, models.LinkReachChoices.AUTHENTICATED]
+)
+@pytest.mark.parametrize("role", models.LinkRoleChoices.values)
+def test_api_documents_link_configuration_update_non_restricted_with_valid_role_success(
+    reach,
+    role,
+    mock_reset_connections,  # pylint: disable=redefined-outer-name
+):
+    """
+    Test that setting non-restricted link_reach with valid link_role succeeds.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.RESTRICTED,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    new_data = {
+        "link_reach": reach,
+        "link_role": role,
+    }
+
+    with mock_reset_connections(document.id):
+        response = client.put(
+            f"/api/v1.0/documents/{document.id!s}/link-configuration/",
+            new_data,
+            format="json",
+        )
+
+        assert response.status_code == 200
+        document.refresh_from_db()
+        assert document.link_reach == reach
+        assert document.link_role == role
+
+
+def test_api_documents_link_configuration_update_with_ancestor_constraints():
+    """
+    Test that link configuration respects ancestor constraints using get_select_options.
+    This test may need adjustment based on the actual get_select_options implementation.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    parent_document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.PUBLIC,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    child_document = factories.DocumentFactory(
+        parent=parent_document,
+        link_reach=models.LinkReachChoices.PUBLIC,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=child_document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    # Try to set child to PUBLIC when parent is RESTRICTED
+    new_data = {
+        "link_reach": models.LinkReachChoices.RESTRICTED,
+        "link_role": models.LinkRoleChoices.READER,
+    }
+
+    response = client.put(
+        f"/api/v1.0/documents/{child_document.id!s}/link-configuration/",
+        new_data,
+        format="json",
+    )
+
+    assert response.status_code == 400
+    assert "link_reach" in response.json()
+    assert (
+        "Link reach 'restricted' is not allowed based on parent"
+        in response.json()["link_reach"][0]
+    )
+
+
+def test_api_documents_link_configuration_update_invalid_role_for_reach_validation():
+    """
+    Test the specific validation logic that checks if link_role is allowed for link_reach.
+    This tests the code section that validates allowed_roles from get_select_options.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    parent_document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.AUTHENTICATED,
+        link_role=models.LinkRoleChoices.EDITOR,
+    )
+
+    child_document = factories.DocumentFactory(
+        parent=parent_document,
+        link_reach=models.LinkReachChoices.RESTRICTED,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=child_document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    new_data = {
+        "link_reach": models.LinkReachChoices.AUTHENTICATED,
+        "link_role": models.LinkRoleChoices.READER,  # This should be rejected
+    }
+
+    response = client.put(
+        f"/api/v1.0/documents/{child_document.id!s}/link-configuration/",
+        new_data,
+        format="json",
+    )
+
+    assert response.status_code == 400
+    assert "link_role" in response.json()
+    error_message = response.json()["link_role"][0]
+    assert (
+        "Link role 'reader' is not allowed for link reach 'authenticated'"
+        in error_message
+    )
+    assert "Allowed roles: editor" in error_message

src/frontend/apps/e2e/__tests__/app-impress/doc-inherited-share.spec.ts

@@ -52,6 +52,12 @@ test.describe('Inherited share accesses', () => {
     await expect(docVisibilityCard.getByText('Connected')).toBeVisible();
     await expect(docVisibilityCard.getByText('Reading')).toBeVisible();
 
+    await docVisibilityCard.getByText('Reading').click();
+    await page.getByRole('menuitem', { name: 'Editing' }).click();
+
+    await expect(docVisibilityCard.getByText('Reading')).toBeHidden();
+    await expect(docVisibilityCard.getByText('Editing')).toBeVisible();
+
     // Verify inherited link
     await docVisibilityCard.getByText('Connected').click();
     await expect(
@@ -61,17 +67,13 @@ test.describe('Inherited share accesses', () => {
     // Update child link
     await page.getByRole('menuitem', { name: 'Public' }).click();
 
-    await docVisibilityCard.getByText('Reading').click();
-    await page.getByRole('menuitem', { name: 'Editing' }).click();
-
     await expect(docVisibilityCard.getByText('Connected')).toBeHidden();
-    await expect(docVisibilityCard.getByText('Reading')).toBeHidden();
     await expect(
       docVisibilityCard.getByText('Public', {
         exact: true,
       }),
     ).toBeVisible();
-    await expect(docVisibilityCard.getByText('Editing')).toBeVisible();
+
     await expect(
       docVisibilityCard.getByText(
         'The link sharing rules differ from the parent document',

src/frontend/apps/impress/src/features/docs/doc-management/api/useUpdateDocLink.tsx

@@ -6,8 +6,8 @@ import { APIError, errorCauses, fetchAPI } from '@/api';
 import { Doc, KEY_DOC } from '@/docs/doc-management';
 import { useBroadcastStore } from '@/stores';
 
-export type UpdateDocLinkParams = Pick<Doc, 'id'> &
-  Partial<Pick<Doc, 'link_role' | 'link_reach'>>;
+export type UpdateDocLinkParams = Pick<Doc, 'id' | 'link_reach'> &
+  Partial<Pick<Doc, 'link_role'>>;
 
 export const updateDocLink = async ({
   id,