diff --git a/CHANGELOG.md b/CHANGELOG.md index 930959780..47f1eaca3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,6 +134,11 @@ and this project adheres to in the process serving the request. A document whose content cannot be read is left out of the batch rather than indexed empty, which would have erased it from the search backend +- ♻️(backend) duplicate the onboarding sandbox document through the + collaboration server: its content is read from there and copied under the + identity of the user the sandbox is created for. A collaboration server that + cannot be reached skips the sandbox, as a missing template already did, and + never fails the signup - 🔥(backend) remove the unused `CollaborationService` - 💥(backend) remove the `documents/{id}/content/` endpoint - 💥(backend) remove the `documents/{id}/can-edit/` endpoint diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 920f9931c..80d97983b 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -40,6 +40,7 @@ from core.choices import ( RoleChoices, get_equivalent_link_definition, ) +from core.services.yhub_services import YHubError, YHubService from core.utils.treebeard import create_tree_node_with_retry from core.validators import sub_validator @@ -314,6 +315,10 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin): """ If the user is new and there is a sandbox document configured, duplicate the sandbox document for the user + + The content of the template is read from the collaboration server, which + owns it, and seeded into the copy under the identity of the user: the + sandbox is theirs from its very first revision. """ if settings.USER_ONBOARDING_SANDBOX_DOCUMENT: sandbox_id = settings.USER_ONBOARDING_SANDBOX_DOCUMENT @@ -325,19 +330,36 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin): sandbox_id, ) return - with transaction.atomic(): - sandbox_document = create_tree_node_with_retry( - lambda: Document.add_root( - title=template_document.title, - content=template_document.content, - attachments=template_document.attachments, - duplicated_from=template_document, - creator=self, - ) - ) - DocumentAccess.objects.create( - user=self, document=sandbox_document, role=RoleChoices.OWNER + service = YHubService(user=self) + try: + # a template the collaboration server holds nothing for is an + # empty template: the sandbox is created, empty as well + ydoc_update = service.get_ydoc(template_document) + + with transaction.atomic(): + sandbox_document = create_tree_node_with_retry( + lambda: Document.add_root( + title=template_document.title, + attachments=template_document.attachments, + duplicated_from=template_document, + creator=self, + ) + ) + + DocumentAccess.objects.create( + user=self, document=sandbox_document, role=RoleChoices.OWNER + ) + + if ydoc_update: + service.create_ydoc(sandbox_document, ydoc_update) + except YHubError: + # Onboarding is not worth failing a signup for, and a sandbox + # the content of which could not be copied is not one we want + # to leave behind: the transaction takes it back. + logger.exception( + "Onboarding sandbox document with id %s could not be copied. Skipping.", + sandbox_id, ) def _convert_valid_invitations(self): diff --git a/src/backend/core/tests/test_models_users.py b/src/backend/core/tests/test_models_users.py index 2cf0d50d3..196aa8681 100644 --- a/src/backend/core/tests/test_models_users.py +++ b/src/backend/core/tests/test_models_users.py @@ -13,9 +13,33 @@ from django.test.utils import override_settings import pytest from core import factories, models +from core.services.yhub_services import ServiceUnavailableError, YHubService pytestmark = pytest.mark.django_db +# what the collaboration server serves for the onboarding template +TEMPLATE_UPDATE = b"\x01\x02the content of the template" + + +@pytest.fixture(name="collaboration_server", autouse=True) +def collaboration_server_fixture(): + """ + Serve the content of the onboarding template, and take the copies. + + The sandbox is duplicated through the collaboration server, which owns the + content of the documents; every test creating a user goes through it as + soon as USER_ONBOARDING_SANDBOX_DOCUMENT is set. + """ + with ( + patch.object( + YHubService, "get_ydoc", autospec=True, return_value=TEMPLATE_UPDATE + ) as mock_get_ydoc, + patch.object(YHubService, "create_ydoc", autospec=True) as mock_create_ydoc, + ): + # autospec, so the calls carry the service itself: who it acts for is + # what the collaboration server attributes the content to + yield mock_get_ydoc, mock_create_ydoc + def test_models_users_str(): """The str representation should be the email.""" @@ -283,6 +307,93 @@ def test_models_users_duplicate_onboarding_sandbox_document_with_invalid_templat assert sandbox_docs.count() == 0 +def test_models_users_duplicate_onboarding_sandbox_document_copies_the_content( + collaboration_server, +): + """ + The content of the sandbox is the one the collaboration server holds for + the template, copied under the identity of the user it is created for. + """ + mock_get_ydoc, mock_create_ydoc = collaboration_server + template_document = factories.DocumentFactory(title="Getting started with Docs") + + with override_settings(USER_ONBOARDING_SANDBOX_DOCUMENT=str(template_document.id)): + user = factories.UserFactory() + + sandbox_document = models.Document.objects.get( + creator=user, title="Getting started with Docs" + ) + + # read from the template, written to the sandbox + _service, read_document = mock_get_ydoc.call_args[0] + service, written_document, update = mock_create_ydoc.call_args[0] + + assert read_document.id == template_document.id + assert written_document.id == sandbox_document.id + assert update == TEMPLATE_UPDATE + # the service acts for the new user: the content is attributed to them, and + # not to the backend itself + assert service.user == user + + +def test_models_users_duplicate_onboarding_sandbox_document_empty_template( + collaboration_server, +): + """A template the collaboration server holds no content for yields an empty sandbox.""" + mock_get_ydoc, mock_create_ydoc = collaboration_server + mock_get_ydoc.return_value = None + template_document = factories.DocumentFactory(title="Getting started with Docs") + + with override_settings(USER_ONBOARDING_SANDBOX_DOCUMENT=str(template_document.id)): + user = factories.UserFactory() + + assert models.Document.objects.filter( + creator=user, title="Getting started with Docs" + ).exists() + mock_create_ydoc.assert_not_called() + + +def test_models_users_duplicate_onboarding_sandbox_document_unreadable_template( + collaboration_server, +): + """ + A signup is not failed over a collaboration server that cannot be reached. + + The sandbox is skipped instead, as it is when its template does not exist. + """ + mock_get_ydoc, _mock_create_ydoc = collaboration_server + mock_get_ydoc.side_effect = ServiceUnavailableError("yhub is unreachable") + template_document = factories.DocumentFactory(title="Getting started with Docs") + + with override_settings(USER_ONBOARDING_SANDBOX_DOCUMENT=str(template_document.id)): + user = factories.UserFactory() + + assert user.pk is not None + assert not models.Document.objects.filter(creator=user).exists() + assert not models.DocumentAccess.objects.filter(user=user).exists() + + +def test_models_users_duplicate_onboarding_sandbox_document_content_not_copied( + collaboration_server, +): + """ + A sandbox whose content could not be copied is not left behind. + + An empty document titled after the template would be more confusing than + no document at all. + """ + _mock_get_ydoc, mock_create_ydoc = collaboration_server + mock_create_ydoc.side_effect = ServiceUnavailableError("yhub is unreachable") + template_document = factories.DocumentFactory(title="Getting started with Docs") + + with override_settings(USER_ONBOARDING_SANDBOX_DOCUMENT=str(template_document.id)): + user = factories.UserFactory() + + assert user.pk is not None + assert not models.Document.objects.filter(creator=user).exists() + assert not models.DocumentAccess.objects.filter(user=user).exists() + + def test_models_users_duplicate_onboarding_sandbox_document_creates_unique_sandbox_per_user(): """ Each new user should get their own independent sandbox document.