Add api endpoint to duplicate document #570
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lunika
reviewed
Jan 22, 2025
src/backend/core/tests/documents/test_api_documents_media_auth.py
Outdated
Show resolved
Hide resolved
sampaccoud
force-pushed
the
add-api-endpoint-to-duplicate-document
branch
from
a138321 to
95241cd
Compare
|
@lunika done implementing your feedbacks 🙏 |
AntoLC
reviewed
Jan 28, 2025
src/backend/core/migrations/0013_remove_is_public_add_field_attachments_and_duplicated_from.py
Outdated
Show resolved
Hide resolved
AntoLC
reviewed
Jan 28, 2025
| from core import enums | ||
|
|
||
|
|
||
| def base64_yjs_to_xml(base64_string): |
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.
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
force-pushed
the
add-api-endpoint-to-duplicate-document
branch
from
95241cd to
bddfd20
Compare
lunika
reviewed
Feb 6, 2025
Comment on lines
+41
to
+53
| ATTACHMENTS_FOLDER = "attachments" | ||
| UUID_REGEX = ( | ||
| r"[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}" | ||
| ) | ||
| FILE_EXT_REGEX = r"\.[a-zA-Z]{3,4}" | ||
| MEDIA_STORAGE_URL_PATTERN = re.compile( | ||
| f"{settings.MEDIA_URL:s}(?P<pk>{UUID_REGEX:s})/" | ||
| f"(?P<attachment>{ATTACHMENTS_FOLDER:s}/{UUID_REGEX:s}{FILE_EXT_REGEX:s})$" | ||
| ) | ||
| MEDIA_STORAGE_URL_EXTRACT = re.compile( | ||
| f"{settings.MEDIA_URL:s}({UUID_REGEX}/{ATTACHMENTS_FOLDER}/{UUID_REGEX}{FILE_EXT_REGEX})" | ||
| ) | ||
| COLLABORATION_WS_URL_PATTERN = re.compile(rf"(?:^|&)room=(?P<pk>{UUID_REGEX})(?:&|$)") |
There was a problem hiding this comment.
All except MEDIA_STORAGE_URL_EXTRACT are duplicated in the same file (from line 11 to 20)
| @@ -75,7 +91,7 @@ class Meta: | |||
|
|
|||
| title = factory.Sequence(lambda n: f"document{n}") | |||
| excerpt = factory.Sequence(lambda n: f"excerpt{n}") | |||
| content = factory.Sequence(lambda n: f"content{n}") | |||
| content = YDOC_HELLO_WORLD_BASE64 | |||
There was a problem hiding this comment.
To be consistent maybe excerpt can be computed from this content ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
We want users to be able to duplicate a document.
resolves #335 and #567
Proposal
While doing this work I was brought to refactor the way the
media-authandcollaboration-authactions were factorizing code and add missing tests for these actions.