Add api endpoint to duplicate document by sampaccoud · Pull Request #570 · suitenumerique/docs

sampaccoud · 2025-01-21T23:07:27Z

Purpose

We want users to be able to duplicate a document.

resolves #335 and #567

Proposal

Refactor the way access control is done on attachment files to play well with document duplication
Add a "duplicate" action to the document API endpoint.
Extract attachment keys from content when a document is saved, and add them to the list of readable attachments on the current document but only if they were already readable on another document by the editing user...

While doing this work I was brought to refactor the way the media-auth and collaboration-auth actions were factorizing code and add missing tests for these actions.

src/backend/core/tests/test_utils_base64_yjs_to_text.py

Dockerfile

src/backend/core/api/viewsets.py

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

src/backend/core/api/viewsets.py

src/backend/core/tests/test_utils_base64_yjs_to_text.py

sampaccoud · 2025-01-28T07:44:50Z

@lunika done implementing your feedbacks 🙏

AntoLC

If you have existing data the last migration does not succeed.

I got a bit confused about the with_accesses as a query string instead of from the payload.
Except that works well! I used it frontend side, I challenged it a bit, works like excepted.

src/backend/core/migrations/0013_remove_is_public_add_field_attachments_and_duplicated_from.py

src/backend/core/api/viewsets.py

src/backend/core/models.py

AntoLC · 2025-01-28T08:44:01Z

src/backend/core/utils.py

+from core import enums
+
+
+def base64_yjs_to_xml(base64_string):


Nice to see it used ^^

src/backend/core/enums.py

src/backend/core/factories.py

src/backend/core/migrations/0018_remove_is_public_add_field_attachments_and_duplicated_from.py

src/backend/core/models.py

AntoLC

I didn't retry all the process, but the migration works fine now.

lunika · 2025-03-11T09:22:36Z

src/backend/core/utils.py

+    """Extract text from base64 yjs document."""
+
+    blocknote_structure = base64_yjs_to_xml(base64_string)
+    soup = BeautifulSoup(blocknote_structure, "html.parser")


Look if the lxml parser can be used. It seems faster.

sampaccoud · 2025-03-23T21:57:03Z

@lunika ready for final call.

Documents content is stored in the Ydoc format. We need a util to extract it as xml/text.

These 2 actions had factorized code but a few iterations lead to spaghetti code where factorized code includes "if" clauses. Refactor abstractions so that code factorization really works.

Tests were forgotten. While writing the tests, I fixed a few edge cases like the possibility to connect to the collaboration server for an anonymous user.

These methods were involved in a bug that was fixed without first evidencing the error in a test: #556 Fixes #567

Migration tests should not import and use factories or models directly from the code because they would not be in sync with the database in the state that each state needs to test it. Instead the migrator object passed as argument allows us to retrieve a minimal version of the models in sync with the state of the database that we are testing. What we get is a minimal model and we need to simulate all the methods that we could have on the real model and that are needed for testing.

We took this opportunity to refactor the way access is controlled on media attachments. We now add the media key to a list on the document instance each time a media is uploaded to a document. This list is passed along when a document is duplicated, allowing us to grant access to readers on the new document, even if they don't have or lost access to the original document. We also propose an option to reproduce the same access rights on the duplicate document as what was in place on the original document. This can be requested by passing the "with_accesses=true" option in the query string. The tricky point is that we need to extract attachment keys from the existing documents and set them on the new "attachments" field that is now used to track access rights on media files.

We can't prevent document editors from copy/pasting content to from one document to another. The problem is that copying content, will copy the urls pointing to attachments but if we don't do anything, the reader of the document to which the content is being pasted, may not be allowed to access the attachment files from the original document. Using the work from the previous commit, we can grant access to the readers of the target document by extracting the attachment keys from the content and adding themto the target document's "attachments" field. Before doing this, we check that the current user can indeed access the attachment files extracted from the content and that they are allowed to edit the current document.

The idea behind wrapping choices in `lazy` function was to allow overriding the list of languages in tests with `override_settings`. This was causin makemigrations to keep on including the field in migrations when it is not needed. Since we finally don't override the LANGUAGES setting in tests, we can remove it to fix the problem.

sampaccoud self-assigned this Jan 21, 2025

sampaccoud requested review from AntoLC, lunika, qbey and virgile-dev January 21, 2025 23:07

lunika reviewed Jan 22, 2025

View reviewed changes

sampaccoud force-pushed the add-api-endpoint-to-duplicate-document branch from a138321 to 95241cd Compare January 28, 2025 07:44

sampaccoud requested a review from lunika January 28, 2025 07:44

AntoLC reviewed Jan 28, 2025

View reviewed changes

src/backend/core/migrations/0013_remove_is_public_add_field_attachments_and_duplicated_from.py Outdated Show resolved Hide resolved

src/backend/core/api/viewsets.py Show resolved Hide resolved

src/backend/core/models.py Show resolved Hide resolved

AntoLC reviewed Jan 28, 2025

View reviewed changes

virgile-dev mentioned this pull request Jan 29, 2025

Onboarding Docs for new users #393

Open

sampaccoud mentioned this pull request Jan 29, 2025

⚗️ Extract text from base64 yjs document #270

Closed

sampaccoud force-pushed the add-api-endpoint-to-duplicate-document branch from 95241cd to bddfd20 Compare January 30, 2025 18:42

sampaccoud requested a review from AntoLC February 2, 2025 13:59

lunika reviewed Feb 6, 2025

View reviewed changes

src/backend/core/enums.py Outdated Show resolved Hide resolved

src/backend/core/factories.py Show resolved Hide resolved

AntoLC reviewed Feb 10, 2025

View reviewed changes

src/backend/core/migrations/0018_remove_is_public_add_field_attachments_and_duplicated_from.py Outdated Show resolved Hide resolved

qbey reviewed Feb 20, 2025

View reviewed changes

src/backend/core/models.py Outdated Show resolved Hide resolved

sampaccoud force-pushed the add-api-endpoint-to-duplicate-document branch from bddfd20 to 88c1cbb Compare February 28, 2025 08:18

AntoLC self-requested a review February 28, 2025 09:14

AntoLC approved these changes Feb 28, 2025

View reviewed changes

lunika reviewed Mar 11, 2025

View reviewed changes

lunika self-assigned this Mar 18, 2025

sampaccoud force-pushed the add-api-endpoint-to-duplicate-document branch from 88c1cbb to 5fa6784 Compare March 23, 2025 21:50

sampaccoud requested a review from lunika March 23, 2025 21:56

lunika force-pushed the add-api-endpoint-to-duplicate-document branch from 5fa6784 to d4b5552 Compare March 24, 2025 09:27

lunika approved these changes Mar 24, 2025

View reviewed changes

lunika force-pushed the add-api-endpoint-to-duplicate-document branch from d4b5552 to 8fd1ec4 Compare March 24, 2025 09:31

✨(backend) add util to extract text from Ydoc content

e3c33a5

Documents content is stored in the Ydoc format. We need a util to extract it as xml/text.

sampaccoud added 7 commits March 24, 2025 10:32

♻️(backend) refactor media_auth and collaboration_auth for flexibility

8a87496

These 2 actions had factorized code but a few iterations lead to spaghetti code where factorized code includes "if" clauses. Refactor abstractions so that code factorization really works.

✅(backend) add missing tests for collaboration auth

c9ff316

Tests were forgotten. While writing the tests, I fixed a few edge cases like the possibility to connect to the collaboration server for an anonymous user.

✅(backend) add missing test on media-auth and collaboration-auth

dcd55d1

These methods were involved in a bug that was fixed without first evidencing the error in a test: #556 Fixes #567

lunika force-pushed the add-api-endpoint-to-duplicate-document branch from 8fd1ec4 to ff8b7e4 Compare March 24, 2025 09:32

lunika enabled auto-merge (rebase) March 24, 2025 09:32

lunika merged commit c882f13 into main Mar 24, 2025
25 of 26 checks passed

lunika deleted the add-api-endpoint-to-duplicate-document branch March 24, 2025 09:43

lunika mentioned this pull request Mar 26, 2025

Why Yjs server in ts when you can use pycrdt instead ? #728

Closed

AntoLC mentioned this pull request May 1, 2025

✨Get doc content from backend #264

Closed

Comments

Conversation

sampaccoud commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Proposal

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sampaccoud commented Jan 28, 2025

Uh oh!

AntoLC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AntoLC Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AntoLC left a comment

Choose a reason for hiding this comment

Uh oh!

lunika Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

sampaccoud commented Mar 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sampaccoud commented Jan 21, 2025 •

edited

Loading