🐛Link role update

src/backend/core/api/serializers.py

@@ -506,13 +506,69 @@ 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]
+
+ # Restricted reach: link_role must be None
+ if link_reach == models.LinkReachChoices.RESTRICTED:
+ if link_role is not None:
+ raise serializers.ValidationError(
+ {
+ "link_role": (
+ "Cannot set link_role when link_reach is 'restricted'. "
+ "Link role must be null for restricted reach."
+ )
+ }
+ )
+ return attrs
+ # Non-restricted: link_role must be in allowed roles
+ if link_role not in allowed_roles:
+ allowed_roles_str = ", ".join(allowed_roles) if allowed_roles else "none"
+ raise serializers.ValidationError(
+ {
+ "link_role": (
+ f"Link role '{link_role}' is not allowed for link reach '{link_reach}'. "
+ f"Allowed roles: {allowed_roles_str}"
+ )
+ }
+ )
+ 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-header.spec.ts

@@ -434,7 +434,9 @@ test.describe('Doc Header', () => {
  test('it pins a document', async ({ page, browserName }) => {
  const [docTitle] = await createDoc(page, `Pin doc`, browserName);
 
- await page.getByLabel('Open the document options').click();
+ await page
+ .getByRole('button', { name: 'Open the document options' })
+ .click();
 
  // Pin
  await page.getByText('push_pin').click();
@@ -453,11 +455,15 @@ test.describe('Doc Header', () => {
  await expect(leftPanelFavorites.getByText(docTitle)).toBeVisible();
 
  await row.getByText(docTitle).click();
- await page.getByLabel('Open the document options').click();
+ await page
+ .getByRole('button', { name: 'Open the document options' })
+ .click();
 
  // Unpin
  await page.getByText('Unpin').click();
- await page.getByLabel('Open the document options').click();
+ await page
+ .getByRole('button', { name: 'Open the document options' })
+ .click();
  await expect(page.getByText('push_pin')).toBeVisible();
 
  await page.goto('/');

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

@@ -60,7 +60,7 @@ export interface Doc {
  path: string;
  is_favorite: boolean;
  link_reach: LinkReach;
- link_role: LinkRole;
+ link_role?: LinkRole;
  nb_accesses_direct: number;
  nb_accesses_ancestors: number;
  computed_link_reach: LinkReach;

src/frontend/apps/impress/src/features/docs/doc-management/utils.ts

@@ -1,7 +1,7 @@
 import emojiRegex from 'emoji-regex';
 import * as Y from 'yjs';
 
-import { Doc, LinkReach, LinkRole } from './types';
+import { Doc, LinkReach } from './types';
 
 export const base64ToYDoc = (base64: string) => {
  const uint8Array = Buffer.from(base64, 'base64');
@@ -18,7 +18,7 @@ export const getDocLinkReach = (doc: Doc): LinkReach => {
  return doc.computed_link_reach ?? doc.link_reach;
 };
 
-export const getDocLinkRole = (doc: Doc): LinkRole => {
+export const getDocLinkRole = (doc: Doc): Doc['link_role'] => {
  return doc.computed_link_role ?? doc.link_role;
 };