Skip to content

Fix link reach/role and add ancestors link access info #846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sampaccoud
Copy link
Member

@sampaccoud sampaccoud commented Apr 6, 2025

Purpose

We are returning too many select options for the link access configuration.

Proposal

  • when the "restricted" reach is an option (key present in the returned dictionary), the possible values for link roles are now always None to make it clearer that they don't matter and no select box should be shown for roles.
  • Never propose "restricted" as option for link reach when the ancestors already offer a public access. Indeed, restricted/editor was shown when the ancestors had public/read access. The logic was to propose editor role on more restricted reaches... but this does not make sense for restricted since the role does is not taken into account for this reach. Roles are set by each access line assign to users/teams.
  • Never propose "authenticated" as option for link reach when the ancestors already offer a public access whatever the role. The logic was to propose editor role for authenticated user when the public access from ancestors was only readonly... but this was juged too much complexity for not enough value created. It can still be brought back later.
  • add ancestors links definitions to document abilities : the frontend needs to display inherited link accesses when it displays possible selection options. We need to return this information to the client.

We also realized that we needed to display specific access rights inherited from ancestors. This PR adds inherited accesses to the list of accesses returned for a document on /api/v1.0/documents/{uuid}/accesses/

@sampaccoud sampaccoud requested review from AntoLC and lunika April 6, 2025 19:29
@sampaccoud sampaccoud self-assigned this Apr 6, 2025
@sampaccoud sampaccoud added bug Something isn't working python Pull requests that update Python code backend refacto labels Apr 6, 2025
@sampaccoud sampaccoud force-pushed the fix-link_reach_role_and_add_ancestors_info branch from 68fd301 to 28dac80 Compare April 6, 2025 19:32

Unverified

This user has not yet uploaded their public signing key.
We were returning too many select options for the restricted link reach:
- when the "restricted" reach is an option (key present in the returned
  dictionary), the possible values for link roles are now always None to
  make it clearer that they don't matter and no select box should be
  shown for roles.
- Never propose "restricted" as option for link reach when the ancestors
  already offer a public access. Indeed, restricted/editor was shown when
  the ancestors had public/read access. The logic was to propose editor
  role on more restricted reaches... but this does not make sense for
  restricted since the role does is not taken into account for this reach.
  Roles are set by each access line assign to users/teams.
The frontend needs to display inherited link accesses when it displays
possible selection options. We need to return this information to the
client.
If anonymous users have reader access on a parent, we were considering
that an edge use case was interesting: allowing an authenticated user
to still be editor on the child.

Although this use case could be interesting, we consider, as a first
approach, that the value it carries is not big enough to justify the
complexity for the user to understand this complex access right heritage.
The document viewset was overriding the get_queryset method from its
own mixin. This was a sign that the mixin was not optimal anymore.
In the next commit I will need to complexify it further so it's time
to refactor the mixin.
The methods to annotate a document queryset were factorized on the
viewset but the correct place is the custom queryset itself now that
we have one.
The document accesses a user have on a document's ancestors also apply
to this document. The frontend needs to list them as "inherited" so we
need to add them to the list.
Adding a "document_id" field on the output will allow the frontend to
differentiate between inherited and direct accesses on a document.
@sampaccoud sampaccoud force-pushed the fix-link_reach_role_and_add_ancestors_info branch from 28dac80 to d85d20f Compare April 12, 2025 16:44
@AntoLC AntoLC requested review from PanchoutNathan and removed request for AntoLC April 22, 2025 13:02
@AntoLC
Copy link
Collaborator

AntoLC commented Apr 22, 2025

@PanchoutNathan The request types documents/[DOC_ID]/accesses/ changed, it is not a pagination anymore.
See: https://github.com/suitenumerique/docs/blob/main/src/frontend/apps/impress/src/features/docs/doc-share/api/useDocAccesses.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend bug Something isn't working python Pull requests that update Python code refacto
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants