From 6afa9f00273e9f687dc12f95f0afa8a6d4ab30ad Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 15:46:09 +0200 Subject: [PATCH 01/20] =?UTF-8?q?=E2=9C=A8(backend)=20add=20legacy=20conve?= =?UTF-8?q?rsion=20policy=20helpers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Legacy Office formats need an explicit conversion policy before they can be opened for editing. Keep the mapping small and configuration-driven so the backend only converts formats intentionally forced by the WOPI client. --- src/backend/drive/settings.py | 5 ++ src/backend/wopi/conversion/__init__.py | 1 + src/backend/wopi/conversion/policy.py | 42 ++++++++++ .../wopi/tests/test_conversion_policy.py | 79 +++++++++++++++++++ 4 files changed, 127 insertions(+) create mode 100644 src/backend/wopi/conversion/__init__.py create mode 100644 src/backend/wopi/conversion/policy.py create mode 100644 src/backend/wopi/tests/test_conversion_policy.py diff --git a/src/backend/drive/settings.py b/src/backend/drive/settings.py index 2672ce1c2..45ea33ef1 100755 --- a/src/backend/drive/settings.py +++ b/src/backend/drive/settings.py @@ -1398,6 +1398,11 @@ class Base(Configuration): WOPI_LOCK_TIMEOUT = values.IntegerValue( 30 * 60, environ_name="WOPI_LOCK_TIMEOUT", environ_prefix=None ) + WOPI_LEGACY_CONVERSION_TARGETS = { + "doc": "docx", + "xls": "xlsx", + "ppt": "pptx", + } WOPI_DISABLE_CHAT = values.IntegerValue( 0, environ_name="WOPI_DISABLE_CHAT", environ_prefix=None ) diff --git a/src/backend/wopi/conversion/__init__.py b/src/backend/wopi/conversion/__init__.py new file mode 100644 index 000000000..701c0f9e7 --- /dev/null +++ b/src/backend/wopi/conversion/__init__.py @@ -0,0 +1 @@ +"""Server-to-server conversion of legacy Office files via WOPI providers.""" diff --git a/src/backend/wopi/conversion/policy.py b/src/backend/wopi/conversion/policy.py new file mode 100644 index 000000000..703572c97 --- /dev/null +++ b/src/backend/wopi/conversion/policy.py @@ -0,0 +1,42 @@ +"""Forced-conversion policy for WOPI legacy formats.""" + +from django.conf import settings + + +def _normalize(value): + return value.lower() if isinstance(value, str) else value + + +def target_extension_for(source_extension): + """Return the converted extension for a legacy source extension. + + Item.extension preserves filename case (REPORT.DOC -> "DOC"), so the + lookup must be case-insensitive. + """ + if not source_extension: + return None + + return settings.WOPI_LEGACY_CONVERSION_TARGETS.get(source_extension.lower()) + + +def is_forced_conversion(item, client_options): + """Return True when the active WOPI client forces a server-side conversion. + + The decision is product policy, not discovery-derived: a client is allowed + to advertise an "edit" action for a legacy format and still require the + Drive backend to convert it first. + """ + if not client_options: + return False + + extension = _normalize(item.extension) + forced_extensions = {_normalize(e) for e in client_options.get("ForceConvertExtensions") or []} + if extension and extension in forced_extensions: + return True + + mimetype = _normalize(item.mimetype) + forced_mimetypes = {_normalize(m) for m in client_options.get("ForceConvertMimetypes") or []} + if mimetype and mimetype in forced_mimetypes: + return True + + return False diff --git a/src/backend/wopi/tests/test_conversion_policy.py b/src/backend/wopi/tests/test_conversion_policy.py new file mode 100644 index 000000000..e7e8f177e --- /dev/null +++ b/src/backend/wopi/tests/test_conversion_policy.py @@ -0,0 +1,79 @@ +"""Tests for the conversion policy helpers.""" + +from types import SimpleNamespace + +from django.conf import settings + +from wopi.conversion.policy import is_forced_conversion, target_extension_for + + +def _item(extension="doc", mimetype="application/msword"): + """Build a minimal item-like namespace for policy tests.""" + return SimpleNamespace(extension=extension, mimetype=mimetype) + + +def test_target_extension_for_known_legacy_formats(): + """Map known legacy Office extensions to their modern equivalents.""" + assert target_extension_for("doc") == "docx" + assert target_extension_for("xls") == "xlsx" + assert target_extension_for("ppt") == "pptx" + + +def test_target_extension_for_unknown_format_returns_none(): + """Return None for unsupported or empty source extensions.""" + assert target_extension_for("docx") is None + assert target_extension_for("") is None + assert target_extension_for(None) is None + + +def test_target_extension_for_is_case_insensitive(): + """Match the target extension regardless of source extension case.""" + assert target_extension_for("DOC") == "docx" + assert target_extension_for("Xls") == "xlsx" + + +def test_legacy_conversion_targets_only_lists_legacy_formats(): + """Restrict the conversion policy to legacy Office formats.""" + assert set(settings.WOPI_LEGACY_CONVERSION_TARGETS) == {"doc", "xls", "ppt"} + + +def test_is_forced_conversion_true_when_extension_listed(): + """Force conversion when the item extension is listed.""" + options = {"ForceConvertExtensions": ["doc", "xls", "ppt"]} + assert is_forced_conversion(_item(extension="doc"), options) is True + + +def test_is_forced_conversion_true_when_mimetype_listed(): + """Force conversion when the item mimetype is listed.""" + options = {"ForceConvertMimetypes": ["application/msword"]} + assert ( + is_forced_conversion(_item(extension="unknown", mimetype="application/msword"), options) + is True + ) + + +def test_is_forced_conversion_false_when_neither_matches(): + """Skip conversion when neither extension nor mimetype matches.""" + options = { + "ForceConvertExtensions": ["doc"], + "ForceConvertMimetypes": ["application/msword"], + } + item = _item(extension="docx", mimetype="application/vnd.openxmlformats") + assert is_forced_conversion(item, options) is False + + +def test_is_forced_conversion_false_when_options_missing(): + """Skip conversion when client options are missing.""" + assert is_forced_conversion(_item(), {}) is False + assert is_forced_conversion(_item(), None) is False + + +def test_is_forced_conversion_extension_check_is_case_insensitive(): + """Match forced extensions regardless of case.""" + # Item.extension preserves filename case (REPORT.DOC -> "DOC"); the + # policy must match regardless. + options = {"ForceConvertExtensions": ["doc"]} + assert is_forced_conversion(_item(extension="DOC"), options) is True + + options = {"ForceConvertExtensions": ["DOC"]} + assert is_forced_conversion(_item(extension="doc"), options) is True From 9fbd6890fb9171f415593452772a303139314454 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 15:46:25 +0200 Subject: [PATCH 02/20] =?UTF-8?q?=E2=9C=A8(backend)=20add=20OnlyOffice=20c?= =?UTF-8?q?onversion=20backend?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drive needs a server-side path to convert legacy Office files without opening an interactive editor session. Use OnlyOffice's conversion API as the first supported provider for the POC. --- compose.yaml | 1 + env.d/development/common | 3 + src/backend/drive/settings.py | 12 ++ .../wopi/conversion/backends/__init__.py | 1 + .../wopi/conversion/backends/onlyoffice.py | 108 ++++++++++ src/backend/wopi/conversion/exceptions.py | 21 ++ .../test_conversion_backends_onlyoffice.py | 189 ++++++++++++++++++ 7 files changed, 335 insertions(+) create mode 100644 src/backend/wopi/conversion/backends/__init__.py create mode 100644 src/backend/wopi/conversion/backends/onlyoffice.py create mode 100644 src/backend/wopi/conversion/exceptions.py create mode 100644 src/backend/wopi/tests/test_conversion_backends_onlyoffice.py diff --git a/compose.yaml b/compose.yaml index c0d490d87..f84ba2db6 100644 --- a/compose.yaml +++ b/compose.yaml @@ -270,6 +270,7 @@ services: environment: TZ: "Europe/Berlin" USE_UNAUTHORIZED_STORAGE: "true" + JWT_ENABLED: "false" ports: - "9981:80" volumes: diff --git a/env.d/development/common b/env.d/development/common index d6f8e5cfa..06d146b7d 100644 --- a/env.d/development/common +++ b/env.d/development/common @@ -84,6 +84,9 @@ WOPI_SRC_BASE_URL=http://app-dev:8000 # Disable rename for Collabora by default because it does not post messages when renaming a file. # So the frontend cannot reflect renaming operations. WOPI_COLLABORA_OPTIONS={"SupportsRename": False} +# Background conversion of legacy Office files (.doc, .xls, .ppt) is forced +# server-side via the OnlyOffice /converter endpoint. +WOPI_ONLYOFFICE_OPTIONS={"ForceConvertExtensions": ["doc", "xls", "ppt"], "ConvertServiceUrl": "http://onlyoffice/converter"} # Indexer # SEARCH_INDEXER_CLASS="core.services.search_indexers.SearchIndexer" diff --git a/src/backend/drive/settings.py b/src/backend/drive/settings.py index 45ea33ef1..4a11df593 100755 --- a/src/backend/drive/settings.py +++ b/src/backend/drive/settings.py @@ -1403,6 +1403,18 @@ class Base(Configuration): "xls": "xlsx", "ppt": "pptx", } + WOPI_ONLYOFFICE_CONVERT_HTTP_CONNECT_TIMEOUT = values.IntegerValue( + 5, environ_name="WOPI_ONLYOFFICE_CONVERT_HTTP_CONNECT_TIMEOUT", environ_prefix=None + ) + WOPI_ONLYOFFICE_CONVERT_HTTP_READ_TIMEOUT = values.IntegerValue( + 60, environ_name="WOPI_ONLYOFFICE_CONVERT_HTTP_READ_TIMEOUT", environ_prefix=None + ) + WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_CONNECT_TIMEOUT = values.IntegerValue( + 5, environ_name="WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_CONNECT_TIMEOUT", environ_prefix=None + ) + WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_READ_TIMEOUT = values.IntegerValue( + 30, environ_name="WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_READ_TIMEOUT", environ_prefix=None + ) WOPI_DISABLE_CHAT = values.IntegerValue( 0, environ_name="WOPI_DISABLE_CHAT", environ_prefix=None ) diff --git a/src/backend/wopi/conversion/backends/__init__.py b/src/backend/wopi/conversion/backends/__init__.py new file mode 100644 index 000000000..d3b6e6731 --- /dev/null +++ b/src/backend/wopi/conversion/backends/__init__.py @@ -0,0 +1 @@ +"""Conversion backends called by the WOPI conversion service.""" diff --git a/src/backend/wopi/conversion/backends/onlyoffice.py b/src/backend/wopi/conversion/backends/onlyoffice.py new file mode 100644 index 000000000..756e1f6a9 --- /dev/null +++ b/src/backend/wopi/conversion/backends/onlyoffice.py @@ -0,0 +1,108 @@ +"""OnlyOffice server-to-server conversion backend.""" + +from uuid import uuid4 + +from django.conf import settings +from django.core.files.base import ContentFile + +import jwt +import requests + +from wopi.conversion.exceptions import ( + ConversionMisconfigured, + ConversionProviderError, +) + + +class OnlyOfficeConversionBackend: + """Run a synchronous OnlyOffice conversion through the /converter endpoint.""" + + # pylint: disable-next=too-many-arguments,too-many-positional-arguments + def __init__( + self, + convert_service_url, + jwt_secret=None, + jwt_required=False, + http_timeout=None, + download_timeout=None, + ): + if jwt_required and not jwt_secret: + raise ConversionMisconfigured( + "OnlyOffice JWT is required but no ConvertJwtSecret is configured." + ) + self.convert_service_url = convert_service_url + self.jwt_secret = jwt_secret + self.http_timeout = http_timeout or ( + settings.WOPI_ONLYOFFICE_CONVERT_HTTP_CONNECT_TIMEOUT, + settings.WOPI_ONLYOFFICE_CONVERT_HTTP_READ_TIMEOUT, + ) + self.download_timeout = download_timeout or ( + settings.WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_CONNECT_TIMEOUT, + settings.WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_READ_TIMEOUT, + ) + + def _request(self, method, url, label, **kwargs): + """Issue an HTTP request and translate transport/HTTP errors.""" + try: + response = getattr(requests, method)(url, **kwargs) + except requests.exceptions.RequestException as exc: + raise ConversionProviderError(str(exc)) from exc + + if not response.ok: + raise ConversionProviderError( + f"OnlyOffice {label} returned status {response.status_code}" + ) + return response + + def _post_convert(self, payload, headers): + """Call /converter and return the parsed completion payload.""" + response = self._request( + "post", + self.convert_service_url, + label="/converter", + params={"shardkey": payload["key"]}, + json=payload, + headers=headers, + timeout=self.http_timeout, + ) + + try: + data = response.json() + except ValueError as exc: + raise ConversionProviderError("OnlyOffice returned a non-JSON body") from exc + + if data.get("error"): + raise ConversionProviderError(f"OnlyOffice error code {data['error']}") + if not data.get("endConvert") or not data.get("fileUrl"): + raise ConversionProviderError("OnlyOffice did not report a completed conversion") + return data + + def _download(self, file_url, target_extension): + """Fetch the converted file and wrap it as a ContentFile.""" + response = self._request( + "get", + file_url, + label="file download", + timeout=self.download_timeout, + ) + return ContentFile(response.content, name=f"converted.{target_extension}") + + def convert(self, item, source_url, target_extension): + """Convert the item via OnlyOffice and return the converted bytes.""" + key = f"{item.id}-{uuid4()}" + payload = { + "async": False, + "filetype": (item.extension or "").lower(), + "outputtype": target_extension, + "key": key, + "title": item.filename, + "url": source_url, + } + headers = {"Accept": "application/json"} + if self.jwt_secret: + token = jwt.encode({"payload": payload}, self.jwt_secret, algorithm="HS256") + payload = {**payload, "token": token} + headers["Authorization"] = f"Bearer {token}" + + data = self._post_convert(payload, headers) + return self._download(data["fileUrl"], target_extension) diff --git a/src/backend/wopi/conversion/exceptions.py b/src/backend/wopi/conversion/exceptions.py new file mode 100644 index 000000000..3ed2d2a4d --- /dev/null +++ b/src/backend/wopi/conversion/exceptions.py @@ -0,0 +1,21 @@ +"""Exceptions for server-to-server file conversion.""" + + +class ConversionError(Exception): + """Base exception for conversion service failures.""" + + +class ConversionPermissionDenied(ConversionError): + """Raised when the user cannot convert the source item.""" + + +class ConversionRejected(ConversionError): + """Raised when the conversion request cannot be satisfied.""" + + +class ConversionMisconfigured(ConversionError): + """Raised when the active conversion backend is missing required settings.""" + + +class ConversionProviderError(ConversionError): + """Raised when the document server fails to deliver the converted file.""" diff --git a/src/backend/wopi/tests/test_conversion_backends_onlyoffice.py b/src/backend/wopi/tests/test_conversion_backends_onlyoffice.py new file mode 100644 index 000000000..54dac1cc7 --- /dev/null +++ b/src/backend/wopi/tests/test_conversion_backends_onlyoffice.py @@ -0,0 +1,189 @@ +"""Tests for the OnlyOffice server-to-server conversion backend.""" + +import json +from types import SimpleNamespace +from unittest import mock +from uuid import uuid4 + +from django.conf import settings + +import jwt +import pytest +import requests +import responses + +from wopi.conversion import exceptions +from wopi.conversion.backends.onlyoffice import OnlyOfficeConversionBackend + +CONVERT_URL = "https://office.example/converter" +FILE_URL = "https://office.example/result/converted.docx" +SOURCE_URL = "https://drive.example/source" +JWT_SECRET = "test-secret-with-enough-length-for-hs256" + + +def _item(filename="document.doc", extension="doc"): + """Build a minimal Item-like namespace for backend tests.""" + return SimpleNamespace(id=uuid4(), filename=filename, extension=extension) + + +def _ok_body(): + """Return a successful OnlyOffice /converter response body.""" + return json.dumps({"endConvert": True, "fileUrl": FILE_URL, "percent": 100}) + + +@responses.activate +def test_convert_sends_synchronous_payload_with_shardkey(): + """Send a synchronous /converter call with required fields and shardkey.""" + responses.add(responses.POST, CONVERT_URL, body=_ok_body(), status=200) + responses.add(responses.GET, FILE_URL, body=b"converted", status=200) + + backend = OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL) + item = _item() + + backend.convert(item, SOURCE_URL, "docx") + + request = responses.calls[0].request + payload = json.loads(request.body) + key = payload.pop("key") + assert payload == { + "async": False, + "filetype": "doc", + "outputtype": "docx", + "title": "document.doc", + "url": SOURCE_URL, + } + assert str(item.id) in key + # OnlyOffice routes synchronous conversions by the ``shardkey`` query param, + # not by the body — load balancers can pin a key to a worker. + assert f"shardkey={key}" in request.url + + +@responses.activate +def test_convert_signs_payload_when_jwt_secret_is_set(): + """Wrap the payload in a JWT and send it in body and Authorization header.""" + responses.add(responses.POST, CONVERT_URL, body=_ok_body(), status=200) + responses.add(responses.GET, FILE_URL, body=b"converted", status=200) + + backend = OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL, jwt_secret=JWT_SECRET) + backend.convert(_item(), SOURCE_URL, "docx") + + request = responses.calls[0].request + payload = json.loads(request.body) + token = payload.pop("token") + + decoded = jwt.decode(token, JWT_SECRET, algorithms=["HS256"]) + assert decoded["payload"] == payload + assert request.headers.get("Authorization") == f"Bearer {token}" + + +@responses.activate +def test_convert_does_not_sign_when_no_secret(): + """Skip the token field when the backend has no JWT secret.""" + responses.add(responses.POST, CONVERT_URL, body=_ok_body(), status=200) + responses.add(responses.GET, FILE_URL, body=b"converted", status=200) + + backend = OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL) + backend.convert(_item(), SOURCE_URL, "docx") + + request = responses.calls[0].request + payload = json.loads(request.body) + assert "token" not in payload + + +def test_constructor_raises_when_jwt_required_but_secret_missing(): + """Fail fast at construction time when JWT is required but the secret is missing.""" + with pytest.raises(exceptions.ConversionMisconfigured, match="OnlyOffice JWT is required"): + OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL, jwt_required=True) + + +@responses.activate +def test_convert_returns_content_file_with_downloaded_bytes(): + """Wrap the downloaded bytes in a ContentFile named after the target.""" + responses.add(responses.POST, CONVERT_URL, body=_ok_body(), status=200) + responses.add(responses.GET, FILE_URL, body=b"converted-bytes", status=200) + + backend = OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL) + result = backend.convert(_item(), SOURCE_URL, "docx") + + assert result.read() == b"converted-bytes" + assert result.name.endswith(".docx") + + +def test_convert_uses_separate_connect_and_read_timeouts(): + """Apply the configured connect/read timeouts to /converter and the download.""" + with mock.patch("wopi.conversion.backends.onlyoffice.requests") as requests_mock: + post_response = mock.Mock(ok=True) + post_response.json.return_value = {"endConvert": True, "fileUrl": FILE_URL} + get_response = mock.Mock(ok=True, content=b"x") + requests_mock.post.return_value = post_response + requests_mock.get.return_value = get_response + requests_mock.exceptions = requests.exceptions + + OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL).convert( + _item(), SOURCE_URL, "docx" + ) + + assert requests_mock.post.call_args.kwargs["timeout"] == ( + settings.WOPI_ONLYOFFICE_CONVERT_HTTP_CONNECT_TIMEOUT, + settings.WOPI_ONLYOFFICE_CONVERT_HTTP_READ_TIMEOUT, + ) + assert requests_mock.get.call_args.kwargs["timeout"] == ( + settings.WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_CONNECT_TIMEOUT, + settings.WOPI_ONLYOFFICE_CONVERT_DOWNLOAD_READ_TIMEOUT, + ) + + +@pytest.mark.parametrize( + ("post_kwargs", "get_kwargs", "expected_message"), + [ + ({"body": requests.exceptions.Timeout()}, None, None), + ({"body": "not json", "status": 200}, None, "non-JSON body"), + ( + {"body": json.dumps({"endConvert": False, "percent": 50}), "status": 200}, + None, + "did not report a completed", + ), + ( + {"body": "boom", "status": 500}, + None, + "/converter returned status 500", + ), + ( + {"body": json.dumps({"error": -3}), "status": 200}, + None, + "error code -3", + ), + ( + {"body": _ok_body(), "status": 200}, + {"body": requests.exceptions.Timeout()}, + None, + ), + ( + {"body": _ok_body(), "status": 200}, + {"body": "boom", "status": 500}, + "file download returned status 500", + ), + ], + ids=[ + "convert_timeout", + "convert_non_json_body", + "convert_incomplete_sync_response", + "convert_5xx", + "convert_error_code", + "download_timeout", + "download_5xx", + ], +) +@responses.activate +def test_convert_maps_provider_failures_to_provider_error( + post_kwargs, get_kwargs, expected_message +): + """Map any transport or HTTP failure from OnlyOffice to ConversionProviderError.""" + responses.add(responses.POST, CONVERT_URL, **post_kwargs) + if get_kwargs is not None: + responses.add(responses.GET, FILE_URL, **get_kwargs) + + backend = OnlyOfficeConversionBackend(convert_service_url=CONVERT_URL) + + with pytest.raises(exceptions.ConversionProviderError, match=expected_message): + backend.convert(_item(), SOURCE_URL, "docx") From 771d79bf5952769971183cc91d2c8c83e6febd79 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 15:46:46 +0200 Subject: [PATCH 03/20] =?UTF-8?q?=E2=9C=A8(backend)=20build=20short-lived?= =?UTF-8?q?=20WOPI=20source=20URLs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OnlyOffice must fetch the source bytes from Drive during server-side conversion. Use short-lived WOPI access URLs so conversion does not depend on direct object storage reachability. --- docker/onlyoffice/local-development.json | 7 ++ src/backend/drive/settings.py | 3 + src/backend/wopi/conversion/source_url.py | 22 +++++++ src/backend/wopi/services/access.py | 19 +++--- .../wopi/tests/test_conversion_source_url.py | 65 +++++++++++++++++++ 5 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 src/backend/wopi/conversion/source_url.py create mode 100644 src/backend/wopi/tests/test_conversion_source_url.py diff --git a/docker/onlyoffice/local-development.json b/docker/onlyoffice/local-development.json index cedbb6eae..b54475fe9 100644 --- a/docker/onlyoffice/local-development.json +++ b/docker/onlyoffice/local-development.json @@ -18,5 +18,12 @@ "converter": { "maxDownloadBytes": 209715200 } + }, + "services": { + "CoAuthoring": { + "request-filtering-agent": { + "allowPrivateIPAddress": true + } + } } } diff --git a/src/backend/drive/settings.py b/src/backend/drive/settings.py index 4a11df593..ba4c1d68f 100755 --- a/src/backend/drive/settings.py +++ b/src/backend/drive/settings.py @@ -1395,6 +1395,9 @@ class Base(Configuration): WOPI_ACCESS_TOKEN_TIMEOUT = values.IntegerValue( 60 * 60 * 10, environ_name="WOPI_ACCESS_TOKEN_TIMEOUT", environ_prefix=None ) + WOPI_CONVERSION_SOURCE_TOKEN_TIMEOUT = values.IntegerValue( + 120, environ_name="WOPI_CONVERSION_SOURCE_TOKEN_TIMEOUT", environ_prefix=None + ) WOPI_LOCK_TIMEOUT = values.IntegerValue( 30 * 60, environ_name="WOPI_LOCK_TIMEOUT", environ_prefix=None ) diff --git a/src/backend/wopi/conversion/source_url.py b/src/backend/wopi/conversion/source_url.py new file mode 100644 index 000000000..47c3813ac --- /dev/null +++ b/src/backend/wopi/conversion/source_url.py @@ -0,0 +1,22 @@ +"""Build the short-lived WOPI URL OnlyOffice uses to fetch the source bytes.""" + +from django.conf import settings + +from wopi.conversion.exceptions import ConversionMisconfigured +from wopi.services.access import AccessUserItemService + + +def build_source_url(item, user): + """Return a short-lived WOPI GetFile URL pointing at the item for the user.""" + base_url = settings.WOPI_SRC_BASE_URL + + if not base_url: + raise ConversionMisconfigured("Missing WOPI_SRC_BASE_URL for conversion source URL") + + access_token, _ttl_ms = AccessUserItemService().insert_new_access( + item, user, ttl=settings.WOPI_CONVERSION_SOURCE_TOKEN_TIMEOUT + ) + return ( + f"{base_url.rstrip('/')}/api/{settings.API_VERSION}" + f"/wopi/files/{item.id}/contents/?access_token={access_token}" + ) diff --git a/src/backend/wopi/services/access.py b/src/backend/wopi/services/access.py index 857e85c01..537ba0089 100644 --- a/src/backend/wopi/services/access.py +++ b/src/backend/wopi/services/access.py @@ -68,22 +68,25 @@ def generate_token(): """Generate a random access token""" return token_urlsafe() - def insert_new_access(self, item: Item, user: AbstractUser) -> tuple[str, int]: + def insert_new_access( + self, item: Item, user: AbstractUser, ttl: int | None = None + ) -> tuple[str, int]: """ Insert a new access token for the user and item. Return an access_token and access_token_ttl - access_token_ttl must be a timestamp in milliseconds + access_token_ttl must be a timestamp in milliseconds. + + ttl overrides the default WOPI_ACCESS_TOKEN_TIMEOUT lifetime. Pass a short + ttl for one-shot uses (server-to-server conversion source download, etc.). """ abilities = item.get_abilities(user) if not abilities["retrieve"]: raise AccessUserItemNotAllowed() + + effective_ttl = ttl if ttl is not None else settings.WOPI_ACCESS_TOKEN_TIMEOUT token = self.generate_token() access_user_item = AccessUserItem(item=item, user=user) - token_eol = timezone.now() + timedelta(seconds=settings.WOPI_ACCESS_TOKEN_TIMEOUT) - cache.set( - token, - access_user_item.to_dict(), - timeout=settings.WOPI_ACCESS_TOKEN_TIMEOUT, - ) + token_eol = timezone.now() + timedelta(seconds=effective_ttl) + cache.set(token, access_user_item.to_dict(), timeout=effective_ttl) return token, int(round(token_eol.timestamp())) * 1000 def get_access_user_item(self, token: str) -> AccessUserItem: diff --git a/src/backend/wopi/tests/test_conversion_source_url.py b/src/backend/wopi/tests/test_conversion_source_url.py new file mode 100644 index 000000000..52742ed3f --- /dev/null +++ b/src/backend/wopi/tests/test_conversion_source_url.py @@ -0,0 +1,65 @@ +"""Tests for the WOPI source-URL builder used by the conversion service.""" + +from types import SimpleNamespace +from unittest import mock +from uuid import uuid4 + +import pytest + +from wopi.conversion import exceptions +from wopi.conversion.source_url import build_source_url + + +@pytest.fixture +def _access_service(monkeypatch): + """Mock the AccessUserItemService used by build_source_url.""" + service = mock.Mock() + service.return_value.insert_new_access.return_value = ("tok-abc", 1_700_000_000_000) + monkeypatch.setattr("wopi.conversion.source_url.AccessUserItemService", service) + return service + + +def test_build_source_url_uses_configured_wopi_base_url(settings, _access_service): + """Start the built URL with WOPI_SRC_BASE_URL and embed the access token.""" + settings.WOPI_SRC_BASE_URL = "https://drive.example" + item = SimpleNamespace(id=uuid4()) + user = SimpleNamespace() + + url = build_source_url(item, user) + + assert url.startswith("https://drive.example/api/v1.0/wopi/files/") + assert f"files/{item.id}/contents/" in url + assert "access_token=tok-abc" in url + + +def test_build_source_url_strips_trailing_slash_on_base_url(settings, _access_service): + """Strip a trailing slash on the base URL to avoid a double slash.""" + settings.WOPI_SRC_BASE_URL = "https://drive.example/" + item = SimpleNamespace(id=uuid4()) + user = SimpleNamespace() + + url = build_source_url(item, user) + + assert "//api" not in url + + +def test_build_source_url_raises_when_base_url_is_missing(settings, _access_service): + """Raise when WOPI_SRC_BASE_URL is missing.""" + settings.WOPI_SRC_BASE_URL = None + item = SimpleNamespace(id=uuid4()) + user = SimpleNamespace() + + with pytest.raises(exceptions.ConversionMisconfigured, match="Missing WOPI_SRC_BASE_URL"): + build_source_url(item, user) + + +def test_build_source_url_delegates_to_access_user_item_service(settings, _access_service): + """Issue a short-lived WOPI access token via the existing service.""" + settings.WOPI_SRC_BASE_URL = "https://drive.example" + settings.WOPI_CONVERSION_SOURCE_TOKEN_TIMEOUT = 90 + item = SimpleNamespace(id=uuid4()) + user = SimpleNamespace() + + build_source_url(item, user) + + _access_service.return_value.insert_new_access.assert_called_once_with(item, user, ttl=90) From c8cf73dfe42b3b23158831fbb8b116e93abc43f0 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 15:47:49 +0200 Subject: [PATCH 04/20] =?UTF-8?q?=E2=9C=A8(backend)=20add=20converting=20u?= =?UTF-8?q?pload=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Converted files should appear in the folder immediately while Celery is still working. Add a dedicated transient state instead of overloading upload or duplication states. --- .../migrations/0024_alter_item_upload_state.py | 18 ++++++++++++++++++ src/backend/core/models.py | 7 ++++++- 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 src/backend/core/migrations/0024_alter_item_upload_state.py diff --git a/src/backend/core/migrations/0024_alter_item_upload_state.py b/src/backend/core/migrations/0024_alter_item_upload_state.py new file mode 100644 index 000000000..01ceaa9b4 --- /dev/null +++ b/src/backend/core/migrations/0024_alter_item_upload_state.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.14 on 2026-05-28 07:10 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0023_userreconciliationcsvimport_userreconciliation'), + ] + + operations = [ + migrations.AlterField( + model_name='item', + name='upload_state', + field=models.CharField(blank=True, choices=[('pending', 'Pending'), ('duplicating', 'Duplicating'), ('converting', 'Converting'), ('analyzing', 'Analyzing'), ('suspicious', 'Suspicious'), ('file_too_large_to_analyze', 'File too large to analyze'), ('ready', 'Ready')], max_length=25, null=True), + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index d3f1575f4..fdc2abc0d 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -73,6 +73,7 @@ class ItemUploadStateChoices(models.TextChoices): PENDING = "pending", _("Pending") DUPLICATING = "duplicating", ("Duplicating") + CONVERTING = "converting", _("Converting") ANALYZING = "analyzing", _("Analyzing") SUSPICIOUS = "suspicious", _("Suspicious") FILE_TOO_LARGE_TO_ANALYZE = ( @@ -1033,7 +1034,11 @@ def save(self, *args, **kwargs): if ( self.created_at is None and self.type == ItemTypeChoices.FILE - and self.upload_state != ItemUploadStateChoices.DUPLICATING + and self.upload_state + not in ( + ItemUploadStateChoices.DUPLICATING, + ItemUploadStateChoices.CONVERTING, + ) ): self.upload_state = ItemUploadStateChoices.PENDING From b32ae760f5e42f78e28be3b8c30e90672bdb983c Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 15:48:32 +0200 Subject: [PATCH 05/20] =?UTF-8?q?=E2=9C=A8(backend)=20add=20placeholder-ba?= =?UTF-8?q?cked=20conversion=20service?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users need immediate feedback in the current folder while conversion continues in the background. Create the target item up front and complete it once converted bytes are available. --- src/backend/wopi/conversion/services.py | 160 ++++++++ .../wopi/tests/test_conversion_services.py | 342 ++++++++++++++++++ 2 files changed, 502 insertions(+) create mode 100644 src/backend/wopi/conversion/services.py create mode 100644 src/backend/wopi/tests/test_conversion_services.py diff --git a/src/backend/wopi/conversion/services.py b/src/backend/wopi/conversion/services.py new file mode 100644 index 000000000..09fc28f8a --- /dev/null +++ b/src/backend/wopi/conversion/services.py @@ -0,0 +1,160 @@ +"""Service layer for server-to-server legacy file conversion.""" + +from os.path import splitext + +from django.conf import settings +from django.core.files.storage import default_storage +from django.db import transaction + +from core import models +from core.api.utils import detect_mimetype +from core.models import Item +from core.utils.item_title import manage_unique_title +from wopi.conversion.backends.onlyoffice import OnlyOfficeConversionBackend +from wopi.conversion.exceptions import ( + ConversionMisconfigured, + ConversionPermissionDenied, + ConversionRejected, +) +from wopi.conversion.policy import is_forced_conversion, target_extension_for +from wopi.conversion.source_url import build_source_url + +MIME_SNIFF_BYTES = 2048 + + +def _validate_conversion(item, user): + """Run pre-flight checks and return the target extension and client options.""" + if item.type != models.ItemTypeChoices.FILE: + raise ConversionRejected("Source item is not a file.") + + if item.upload_state != models.ItemUploadStateChoices.READY: + raise ConversionRejected("Source item is not ready.") + + if not item.get_abilities(user).get("update"): + raise ConversionPermissionDenied("User cannot update the source item.") + + target_extension = target_extension_for(item.extension) + if not target_extension: + raise ConversionRejected("No target conversion for this extension.") + + onlyoffice_config = settings.WOPI_CLIENTS_CONFIGURATION.get("onlyoffice") + if onlyoffice_config is None: + raise ConversionRejected("No OnlyOffice client configured.") + + client_options = onlyoffice_config.get("options", {}) + if not is_forced_conversion(item, client_options): + raise ConversionRejected("Conversion not forced by the active WOPI client.") + + return target_extension, client_options + + +def resolve_backend(client_options): + """Build the OnlyOffice conversion backend from WOPI client options.""" + convert_service_url = client_options.get("ConvertServiceUrl") + if not convert_service_url: + raise ConversionMisconfigured("Missing OnlyOffice ConvertServiceUrl") + + return OnlyOfficeConversionBackend( + convert_service_url=convert_service_url, + jwt_secret=client_options.get("ConvertJwtSecret"), + jwt_required=bool(client_options.get("ConvertJwtRequired", False)), + ) + + +def _resolve_destination_parent(source_item, user): + """Return the source parent when the user can still update it, else None.""" + if source_item.depth <= 1: + return None + + parent = source_item.parent() + if not parent.get_abilities(user).get("update"): + return None + + return parent + + +def _target_filename(item, target_extension, parent, user): + """Return a sibling-free filename for the converted item.""" + if parent: + siblings = Item.objects.children(parent.path) + else: + siblings = Item.objects.filter(path__depth=1, accesses__user=user).distinct() + + base, _ = splitext(item.filename) + target = f"{base}.{target_extension}" + + return manage_unique_title(siblings, target) + + +def prepare_conversion(source_item, user): + """Validate inputs and create the placeholder Item receiving the conversion. + + The placeholder is created synchronously and returned to the caller so the + UI can display the converting state right away. The actual OnlyOffice + conversion happens later in a celery task. + """ + target_extension, _ = _validate_conversion(source_item, user) + parent = _resolve_destination_parent(source_item, user) + target_filename = _target_filename(source_item, target_extension, parent, user) + + with transaction.atomic(): + placeholder = Item.objects.create_child( + creator=user, + link_reach=None if parent else models.LinkReachChoices.RESTRICTED, + parent=parent, + title=target_filename, + type=models.ItemTypeChoices.FILE, + filename=target_filename, + upload_state=models.ItemUploadStateChoices.CONVERTING, + ) + if placeholder.is_root: + models.ItemAccess.objects.create( + item=placeholder, + user=user, + role=models.RoleChoices.OWNER, + ) + + return placeholder + + +def perform_conversion(source_item, placeholder, user): + """Run the OnlyOffice conversion and attach the result to the placeholder. + + Called from the celery task. The network call to OnlyOffice happens outside + of any DB transaction; only the final READY update is wrapped in one. + """ + target_extension, client_options = _validate_conversion(source_item, user) + source_url = build_source_url(source_item, user) + converted_file = resolve_backend(client_options).convert( + source_item, source_url, target_extension + ) + + converted_file.seek(0) + mimetype = detect_mimetype( + converted_file.read(min(MIME_SNIFF_BYTES, converted_file.size)), + filename=placeholder.filename, + ) + converted_file.seek(0) + + default_storage.save(placeholder.file_key, converted_file) + try: + with transaction.atomic(): + placeholder.mimetype = mimetype + placeholder.size = converted_file.size + placeholder.upload_state = models.ItemUploadStateChoices.READY + placeholder.save(update_fields=["mimetype", "size", "upload_state", "updated_at"]) + except Exception: + default_storage.delete(placeholder.file_key) + raise + + return placeholder + + +def convert_item(source_item, user): + """Prepare and run a conversion synchronously. + + Used by tests and any call site that wants the new converted item right away + instead of going through the celery task. + """ + placeholder = prepare_conversion(source_item, user) + return perform_conversion(source_item, placeholder, user) diff --git a/src/backend/wopi/tests/test_conversion_services.py b/src/backend/wopi/tests/test_conversion_services.py new file mode 100644 index 000000000..c152700ae --- /dev/null +++ b/src/backend/wopi/tests/test_conversion_services.py @@ -0,0 +1,342 @@ +"""Tests for the conversion service.""" + +from django.core.files.base import ContentFile + +import pytest + +from core import factories, models +from wopi.conversion import exceptions, services +from wopi.conversion.backends.onlyoffice import OnlyOfficeConversionBackend + +pytestmark = pytest.mark.django_db + + +class FakeBackend: + """Conversion backend returning fixed converted content.""" + + def convert(self, _item, _source_url, target_extension): + """Return fake converted content.""" + return ContentFile(b"converted", name=f"converted.{target_extension}") + + +@pytest.fixture(autouse=True) +def _fake_backend(request, monkeypatch): + """Use a fake backend except in tests that exercise backend resolution.""" + if request.node.name.startswith("test_resolve_backend"): + return + monkeypatch.setattr(services, "resolve_backend", lambda _options: FakeBackend()) + + +def _configure_wopi(settings, client="onlyoffice", options=None): + """Wire a WOPI client config covering .doc files.""" + settings.WOPI_SRC_BASE_URL = "https://drive.example" + settings.WOPI_CLIENTS_CONFIGURATION = { + client: { + "options": options + or { + "ForceConvertExtensions": ["doc"], + "ConvertServiceUrl": "http://onlyoffice/converter", + }, + } + } + + +def _file(user, **kwargs): + """Build a ready .doc file the user can update; kwargs override defaults.""" + defaults = { + "users": [(user, models.RoleChoices.EDITOR)], + "type": models.ItemTypeChoices.FILE, + "filename": "document.doc", + "mimetype": "application/msword", + "update_upload_state": models.ItemUploadStateChoices.READY, + } + defaults.update(kwargs) + return factories.ItemFactory(**defaults) + + +def test_convert_item_creates_converted_file_copy(settings): + """Create a modern-format copy for a forced legacy file conversion.""" + _configure_wopi(settings) + user = factories.UserFactory() + parent = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FOLDER, + ) + item = _file( + user, + parent=parent, + ) + + converted = services.convert_item(item, user) + + assert converted.filename == "document.docx" + assert converted.parent().id == parent.id + assert converted.upload_state == models.ItemUploadStateChoices.READY + + +def test_convert_item_keeps_original_file_unchanged(settings): + """Leave the source file unchanged after conversion.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user, size=123) + original_parent = item.parent() + + services.convert_item(item, user) + + item.refresh_from_db() + assert item.filename == "document.doc" + assert item.parent() == original_parent + assert item.upload_state == models.ItemUploadStateChoices.READY + assert item.size == 123 + + +def test_convert_item_does_not_copy_explicit_source_accesses(settings): + """Skip copying explicit source accesses to the converted file.""" + _configure_wopi(settings) + user = factories.UserFactory() + other_user = factories.UserFactory() + item = _file(user) + factories.UserItemAccessFactory( + item=item, + user=other_user, + role=models.RoleChoices.READER, + ) + + converted = services.convert_item(item, user) + + assert not converted.accesses.filter(user=other_user).exists() + + +def test_convert_item_keeps_title_and_filename_aligned_on_collision(settings): + """Keep the converted title and filename aligned when the target name exists.""" + _configure_wopi(settings) + user = factories.UserFactory() + parent = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FOLDER, + ) + factories.ItemFactory( + parent=parent, + type=models.ItemTypeChoices.FILE, + title="document.docx", + filename="document.docx", + update_upload_state=models.ItemUploadStateChoices.READY, + ) + item = _file(user, parent=parent) + + converted = services.convert_item(item, user) + + assert converted.title == "document_01.docx" + assert converted.filename == "document_01.docx" + + +def test_convert_item_uses_source_parent_when_user_can_update_it_via_link(settings): + """Keep the converted file in the parent when a writable link is present.""" + _configure_wopi(settings) + user = factories.UserFactory() + parent = factories.ItemFactory( + type=models.ItemTypeChoices.FOLDER, + link_reach=models.LinkReachChoices.AUTHENTICATED, + link_role=models.LinkRoleChoices.EDITOR, + ) + item = _file(user, parent=parent) + + converted = services.convert_item(item, user) + + assert converted.parent().id == parent.id + + +def test_convert_item_falls_back_to_root_when_parent_is_not_writable(settings): + """Fall back to the user's root when the parent is read-only.""" + _configure_wopi(settings) + user = factories.UserFactory() + parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + item = _file(user, parent=parent) + + converted = services.convert_item(item, user) + + assert converted.parent() is None + + +def test_convert_item_rejects_non_file_items(settings): + """Reject non-file items.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FOLDER, + ) + + with pytest.raises(exceptions.ConversionRejected, match="not a file"): + services.convert_item(item, user) + + +def test_convert_item_rejects_items_that_are_not_ready(settings): + """Reject files that are not yet ready.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user, update_upload_state=models.ItemUploadStateChoices.PENDING) + + with pytest.raises(exceptions.ConversionRejected, match="not ready"): + services.convert_item(item, user) + + +def test_convert_item_rejects_users_without_update_permission(settings): + """Reject users without update permission on the source file.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = factories.ItemFactory( + type=models.ItemTypeChoices.FILE, + filename="document.doc", + mimetype="application/msword", + update_upload_state=models.ItemUploadStateChoices.READY, + ) + + with pytest.raises(exceptions.ConversionPermissionDenied, match="cannot update"): + services.convert_item(item, user) + + +def test_convert_item_rejects_unsupported_extensions(settings): + """Reject extensions outside the configured legacy set.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user, filename="document.docx") + + with pytest.raises(exceptions.ConversionRejected, match="No target conversion"): + services.convert_item(item, user) + + +def test_convert_item_rejects_non_onlyoffice_clients(settings): + """Reject WOPI clients other than OnlyOffice.""" + _configure_wopi(settings, client="collabora") + user = factories.UserFactory() + item = _file(user) + + with pytest.raises(exceptions.ConversionRejected, match="No OnlyOffice client"): + services.convert_item(item, user) + + +def test_convert_item_rejects_files_without_forced_conversion(settings): + """Reject files outside the forced-conversion policy.""" + _configure_wopi(settings, options={"ForceConvertExtensions": ["xls"]}) + user = factories.UserFactory() + item = _file(user) + + with pytest.raises(exceptions.ConversionRejected, match="not forced"): + services.convert_item(item, user) + + +def test_convert_item_rejects_missing_onlyoffice_client_config(settings): + """Reject conversion when WOPI_CLIENTS_CONFIGURATION has no OnlyOffice entry.""" + settings.WOPI_CLIENTS_CONFIGURATION = {} + user = factories.UserFactory() + item = _file(user) + + with pytest.raises(exceptions.ConversionRejected, match="No OnlyOffice client"): + services.convert_item(item, user) + + +def test_resolve_backend_rejects_missing_convert_service_url(): + """Reject missing ConvertServiceUrl in OnlyOffice options.""" + with pytest.raises( + exceptions.ConversionMisconfigured, match="Missing OnlyOffice ConvertServiceUrl" + ): + services.resolve_backend({}) + + +def test_perform_conversion_removes_saved_file_when_database_save_fails(settings, monkeypatch): + """Delete saved bytes when flipping the placeholder to READY fails.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user) + placeholder = services.prepare_conversion(item, user) + saved_keys = [] + deleted_keys = [] + + class FakeStorage: + """Storage fake tracking saved and deleted keys.""" + + def save(self, key, _content): + """Track saved keys.""" + saved_keys.append(key) + return key + + def delete(self, key): + """Track deleted keys.""" + deleted_keys.append(key) + + original_save = models.Item.save + + def fail_ready_save(instance, *args, **kwargs): + """Raise when the placeholder is flipped to READY, pass otherwise.""" + if instance.id == placeholder.id and kwargs.get("update_fields"): + raise RuntimeError("database write failed") + return original_save(instance, *args, **kwargs) + + monkeypatch.setattr(services, "default_storage", FakeStorage()) + monkeypatch.setattr(models.Item, "save", fail_ready_save) + + with pytest.raises(RuntimeError, match="database write failed"): + services.perform_conversion(item, placeholder, user) + + assert deleted_keys == saved_keys + + +def test_resolve_backend_builds_onlyoffice_backend_from_options(): + """Build the OnlyOffice backend from the WOPI client options.""" + backend = services.resolve_backend( + { + "ConvertServiceUrl": "https://office.example/converter", + "ConvertJwtSecret": "test-secret", + }, + ) + + assert isinstance(backend, OnlyOfficeConversionBackend) + assert backend.convert_service_url == "https://office.example/converter" + assert backend.jwt_secret == "test-secret" + + +def test_resolve_backend_raises_when_onlyoffice_url_is_missing(): + """Raise when ConvertServiceUrl is missing from OnlyOffice options.""" + with pytest.raises( + exceptions.ConversionMisconfigured, match="Missing OnlyOffice ConvertServiceUrl" + ): + services.resolve_backend({}) + + +def test_prepare_conversion_returns_placeholder_in_converting_state(settings): + """Create the placeholder in the destination folder with upload_state CONVERTING.""" + _configure_wopi(settings) + user = factories.UserFactory() + parent = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FOLDER, + ) + item = _file(user, parent=parent) + + placeholder = services.prepare_conversion(item, user) + + assert placeholder.filename == "document.docx" + assert placeholder.upload_state == models.ItemUploadStateChoices.CONVERTING + assert placeholder.parent().id == parent.id + + +def test_prepare_conversion_rejects_unsupported_extensions(settings): + """Skip placeholder creation for files that cannot be converted.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user, filename="document.docx") + + with pytest.raises(exceptions.ConversionRejected, match="No target conversion"): + services.prepare_conversion(item, user) + + +def test_convert_item_accepts_uppercase_extension(settings): + """Convert REPORT.DOC the same way as report.doc.""" + _configure_wopi(settings) + user = factories.UserFactory() + item = _file(user, filename="REPORT.DOC") + + converted = services.convert_item(item, user) + + assert converted.filename.endswith(".docx") From 3c3b3b9b8ba960575d0ac6f25b56c4fccc635fcf Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 15:49:33 +0200 Subject: [PATCH 06/20] =?UTF-8?q?=E2=9C=A8(backend)=20expose=20legacy=20co?= =?UTF-8?q?nversion=20ability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The explorer needs to know when opening a legacy Office file should start conversion instead of WOPI editing. Expose the decision on item abilities so the frontend can keep the flow item- based. --- src/backend/core/models.py | 8 ++++ src/backend/core/tests/test_models_items.py | 43 ++++++++++++++++++- .../core/tests/test_models_items_root.py | 13 +++++- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/backend/core/models.py b/src/backend/core/models.py index fdc2abc0d..480838b47 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -43,6 +43,7 @@ from timezone_field import TimeZoneField from core.utils.item_title import manage_unique_title as manage_unique_title_utils +from wopi.conversion.policy import target_extension_for logger = getLogger(__name__) @@ -1293,6 +1294,12 @@ def get_abilities(self, user): and self.upload_state == ItemUploadStateChoices.READY ) can_export = can_get and self.type == ItemTypeChoices.FOLDER + must_convert = ( + can_update + and self.type == ItemTypeChoices.FILE + and self.upload_state == ItemUploadStateChoices.READY + and bool(target_extension_for(self.extension)) + ) return { "accesses_manage": can_manage, @@ -1318,6 +1325,7 @@ def get_abilities(self, user): "update": can_update, "upload_ended": can_update and user.is_authenticated, "wopi": can_get, + "must_convert": must_convert, } def send_email(self, subject, emails, context=None, language=None): diff --git a/src/backend/core/tests/test_models_items.py b/src/backend/core/tests/test_models_items.py index c3757e3c0..a0f4b87ee 100644 --- a/src/backend/core/tests/test_models_items.py +++ b/src/backend/core/tests/test_models_items.py @@ -212,6 +212,31 @@ def test_models_items_hard_delete(): # get_abilities +@pytest.mark.parametrize( + "extension,expected", + [ + ("doc", True), + ("DOC", True), + ("xls", True), + ("ppt", True), + ("docx", False), + ("pdf", False), + (None, False), + ], +) +def test_models_items_get_abilities_must_convert(extension, expected): + """Flag must_convert only for legacy extensions on READY files with update access.""" + user = factories.UserFactory() + filename = f"file.{extension}" if extension else None + item = factories.ItemFactory( + users=[(user, "editor")], + type=models.ItemTypeChoices.FILE, + filename=filename or "file", + update_upload_state=models.ItemUploadStateChoices.READY, + ) + assert item.get_abilities(user)["must_convert"] is expected + + @pytest.mark.parametrize( "is_authenticated,reach,role", [ @@ -256,6 +281,7 @@ def test_models_items_get_abilities_forbidden( "update": False, "upload_ended": False, "wopi": False, + "must_convert": False, } nb_queries = 1 if is_authenticated else 0 with django_assert_num_queries(nb_queries): @@ -305,6 +331,7 @@ def test_models_items_get_abilities_reader(is_authenticated, reach, django_asser "update": False, "upload_ended": False, "wopi": True, + "must_convert": False, } nb_queries = 1 if is_authenticated else 0 with django_assert_num_queries(nb_queries): @@ -380,6 +407,7 @@ def test_models_items_get_abilities_editor( # noqa: PLR0913 link_role="editor", type=item_type, update_upload_state=upload_state, + filename="document.pdf" if item_type == models.ItemTypeChoices.FILE else None, ) user = factories.UserFactory() if is_authenticated else AnonymousUser() @@ -408,6 +436,7 @@ def test_models_items_get_abilities_editor( # noqa: PLR0913 "update": True, "upload_ended": is_authenticated, "wopi": True, + "must_convert": False, } nb_queries = 1 if is_authenticated else 0 with django_assert_num_queries(nb_queries): @@ -443,7 +472,10 @@ def test_models_items_not_root_get_abilities_owner( """Check abilities returned for the owner of an item.""" user = factories.UserFactory() item = factories.ItemFactory( - users=[(user, "owner")], type=item_type, update_upload_state=upload_state + users=[(user, "owner")], + type=item_type, + update_upload_state=upload_state, + filename="document.pdf" if item_type == models.ItemTypeChoices.FILE else None, ) can_export = item_type == models.ItemTypeChoices.FOLDER expected_abilities = { @@ -474,6 +506,7 @@ def test_models_items_not_root_get_abilities_owner( "update": True, "upload_ended": True, "wopi": True, + "must_convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -503,6 +536,7 @@ def test_models_items_not_root_get_abilities_owner( "update": False, "upload_ended": False, "wopi": False, + "must_convert": False, } @@ -531,6 +565,7 @@ def test_models_items_not_root_get_abilities_administrator( users=[(user, "administrator")], type=item_type, update_upload_state=upload_state, + filename="document.pdf" if item_type == models.ItemTypeChoices.FILE else None, ) can_export = item_type == models.ItemTypeChoices.FOLDER expected_abilities = { @@ -561,6 +596,7 @@ def test_models_items_not_root_get_abilities_administrator( "update": True, "upload_ended": True, "wopi": True, + "must_convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -601,6 +637,7 @@ def test_models_items_not_root_get_abilities_editor_user( item = factories.ItemFactory( parent=parent, type=item_type, + filename="document.pdf" if item_type == models.ItemTypeChoices.FILE else None, update_upload_state=upload_state, ) link_select_options = LinkReachChoices.get_select_options(**item.ancestors_link_definition) @@ -629,6 +666,7 @@ def test_models_items_not_root_get_abilities_editor_user( "update": True, "upload_ended": True, "wopi": True, + "must_convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -681,6 +719,7 @@ def test_models_items_not_root_get_abilities_reader_user(django_assert_num_queri "update": access_from_link, "upload_ended": access_from_link, "wopi": True, + "must_convert": False, } with django_assert_num_queries(2): assert item.get_abilities(user) == expected_abilities @@ -738,6 +777,7 @@ def test_models_items_get_abilities_hard_delete_non_root_by_non_creator( "update": True, "upload_ended": True, "wopi": True, + "must_convert": False, } with django_assert_num_queries(1): assert child.get_abilities(owner) == expected_abilities @@ -768,6 +808,7 @@ def test_models_items_get_abilities_hard_delete_non_root_by_non_creator( "update": False, "upload_ended": False, "wopi": False, + "must_convert": False, } diff --git a/src/backend/core/tests/test_models_items_root.py b/src/backend/core/tests/test_models_items_root.py index d79bb596c..008f9e007 100644 --- a/src/backend/core/tests/test_models_items_root.py +++ b/src/backend/core/tests/test_models_items_root.py @@ -71,6 +71,7 @@ def test_models_sub_item_abilities_downgraded(): "update": True, "upload_ended": True, "wopi": True, + "must_convert": False, } # Downgrade the role on the root item @@ -106,6 +107,7 @@ def test_models_sub_item_abilities_downgraded(): "update": False, "upload_ended": False, "wopi": True, + "must_convert": False, } @@ -159,6 +161,7 @@ def test_models_items_root_get_abilities_owner( "update": True, "upload_ended": True, "wopi": True, + "must_convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -188,6 +191,7 @@ def test_models_items_root_get_abilities_owner( "update": False, "upload_ended": False, "wopi": False, + "must_convert": False, } @@ -215,6 +219,7 @@ def test_models_items_root_get_abilities_administrator( item = factories.ItemFactory( users=[(user, "administrator")], type=item_type, + filename=("document.pdf" if item_type == models.ItemTypeChoices.FILE else None), update_upload_state=upload_state, ) link_select_options = LinkReachChoices.get_select_options(**item.ancestors_link_definition) @@ -243,6 +248,7 @@ def test_models_items_root_get_abilities_administrator( "update": True, "upload_ended": True, "wopi": True, + "must_convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -277,7 +283,10 @@ def test_models_items_root_get_abilities_editor_user( """Check abilities returned for the editor of a root item.""" user = factories.UserFactory() item = factories.ItemFactory( - users=[(user, "editor")], type=item_type, update_upload_state=upload_state + users=[(user, "editor")], + type=item_type, + filename="document.pdf" if item_type == models.ItemTypeChoices.FILE else None, + update_upload_state=upload_state, ) link_select_options = LinkReachChoices.get_select_options(**item.ancestors_link_definition) can_export = item_type == models.ItemTypeChoices.FOLDER @@ -305,6 +314,7 @@ def test_models_items_root_get_abilities_editor_user( "update": True, "upload_ended": True, "wopi": True, + "must_convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -357,6 +367,7 @@ def test_models_items_root_get_abilities_reader_user( "update": access_from_link, "upload_ended": access_from_link, "wopi": True, + "must_convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities From 39d014bf0843d49b16a20000bcc0f479d7258836 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 15:49:46 +0200 Subject: [PATCH 07/20] =?UTF-8?q?=E2=9C=A8(backend)=20queue=20legacy=20fil?= =?UTF-8?q?e=20conversion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting a conversion should be quick for the API caller and continue in the background. Queue the placeholder-backed conversion task from the item endpoint. --- src/backend/core/api/permissions.py | 1 + src/backend/core/api/viewsets.py | 40 ++++++ .../tests/items/test_api_items_convert.py | 116 +++++++++++++++++ src/backend/wopi/apps.py | 5 + src/backend/wopi/tasks/__init__.py | 1 + src/backend/wopi/tasks/conversion.py | 61 +++++++++ .../wopi/tests/tasks/test_conversion.py | 121 ++++++++++++++++++ 7 files changed, 345 insertions(+) create mode 100644 src/backend/core/tests/items/test_api_items_convert.py create mode 100644 src/backend/wopi/tasks/conversion.py create mode 100644 src/backend/wopi/tests/tasks/test_conversion.py diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index 1e8aad414..2fec31254 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -11,6 +11,7 @@ ACTION_FOR_METHOD_TO_PERMISSION = { "versions_detail": {"DELETE": "versions_destroy", "GET": "versions_retrieve"}, "children": {"GET": "children_list", "POST": "children_create"}, + "convert": {"POST": "update"}, } diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 2a1d98974..b77c60f39 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -27,11 +27,13 @@ import rest_framework as drf from botocore.exceptions import ClientError +from celery.exceptions import CeleryError from corsheaders.middleware import ( ACCESS_CONTROL_ALLOW_METHODS, ACCESS_CONTROL_ALLOW_ORIGIN, ) from drf_spectacular.utils import extend_schema, extend_schema_view +from kombu.exceptions import KombuError from lasuite.drf.models.choices import ( PRIVILEGED_ROLES, LinkReachChoices, @@ -56,7 +58,10 @@ from core.storage import get_storage_compute_backend from core.tasks.item import duplicate_file, process_item_purge, rename_file from core.utils.analytics import posthog_capture +from wopi.conversion import exceptions as conversion_exceptions +from wopi.conversion.services import prepare_conversion from wopi.services import access as access_service +from wopi.tasks.conversion import convert_file as convert_file_task from wopi.utils import compute_wopi_launch_url, get_wopi_client_config from . import permissions, serializers, utils @@ -644,6 +649,41 @@ def hard_delete(self, request, *args, **kwargs): process_item_purge.delay(instance.id) return drf.response.Response(status=status.HTTP_204_NO_CONTENT) + @drf.decorators.action(detail=True, methods=["post"], url_path="convert") + def convert(self, request, *args, **kwargs): + """Queue a legacy Office file conversion. + + Creates a placeholder Item in CONVERTING state in the destination folder + and dispatches the celery task that will attach the converted bytes. + """ + source = self.get_object() + try: + placeholder = prepare_conversion(source, request.user) + except conversion_exceptions.ConversionPermissionDenied as exc: + raise drf.exceptions.PermissionDenied() from exc + except ( + conversion_exceptions.ConversionRejected, + conversion_exceptions.ConversionMisconfigured, + ) as exc: + raise drf.exceptions.ValidationError({"detail": str(exc)}) from exc + + try: + convert_file_task.delay( + source_item_id=str(source.id), + converted_item_id=str(placeholder.id), + user_id=str(request.user.id), + ) + except (CeleryError, KombuError): + placeholder.soft_delete() + placeholder.delete() + return drf.response.Response( + {"detail": "Conversion could not be queued."}, + status=status.HTTP_503_SERVICE_UNAVAILABLE, + ) + + serializer = self.get_serializer(placeholder) + return drf.response.Response(serializer.data, status=status.HTTP_201_CREATED) + def list(self, request, *args, **kwargs): """List top level items with pagination and filtering.""" # Not calling filter_queryset. We do our own cooking. diff --git a/src/backend/core/tests/items/test_api_items_convert.py b/src/backend/core/tests/items/test_api_items_convert.py new file mode 100644 index 000000000..29d8ebdb4 --- /dev/null +++ b/src/backend/core/tests/items/test_api_items_convert.py @@ -0,0 +1,116 @@ +"""Tests for the /items//convert/ endpoint.""" + +from unittest import mock + +import pytest +from kombu.exceptions import KombuError +from rest_framework.test import APIClient + +from core import factories, models + +pytestmark = pytest.mark.django_db + + +def _build_user_and_item(): + """Build an EDITOR user with a legacy .doc item ready for conversion.""" + user = factories.UserFactory() + parent = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FOLDER, + ) + item = factories.ItemFactory( + parent=parent, + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FILE, + filename="document.doc", + mimetype="application/msword", + update_upload_state=models.ItemUploadStateChoices.READY, + ) + return user, item + + +@pytest.fixture(autouse=True) +def _wopi_settings(settings): + """Configure the OnlyOffice WOPI client for the conversion tests.""" + settings.WOPI_SRC_BASE_URL = "https://drive.example" + settings.WOPI_CLIENTS_CONFIGURATION = { + "onlyoffice": { + "options": { + "ForceConvertExtensions": ["doc"], + "ConvertServiceUrl": "http://onlyoffice/converter", + }, + } + } + + +def test_convert_endpoint_creates_placeholder_and_queues_task(): + """Return the placeholder and enqueue the conversion task on POST /convert.""" + user, item = _build_user_and_item() + client = APIClient() + client.force_login(user) + + with mock.patch("core.api.viewsets.convert_file_task.delay") as delay_mock: + response = client.post(f"/api/v1.0/items/{item.id}/convert/") + + assert response.status_code == 201 + body = response.json() + placeholder = models.Item.objects.get(id=body["id"]) + assert placeholder.upload_state == models.ItemUploadStateChoices.CONVERTING + assert placeholder.filename == "document.docx" + assert placeholder.parent().id == item.parent().id + delay_mock.assert_called_once_with( + source_item_id=str(item.id), + converted_item_id=str(placeholder.id), + user_id=str(user.id), + ) + + +def test_convert_endpoint_returns_503_and_cleans_up_when_queueing_fails(): + """Remove the placeholder and return 503 when the broker fails.""" + user, item = _build_user_and_item() + client = APIClient() + client.force_login(user) + + with mock.patch("core.api.viewsets.convert_file_task.delay", side_effect=KombuError): + response = client.post(f"/api/v1.0/items/{item.id}/convert/") + + assert response.status_code == 503 + assert not models.Item.objects.filter( + upload_state=models.ItemUploadStateChoices.CONVERTING + ).exists() + + +def test_convert_endpoint_returns_403_when_user_cannot_update_item(): + """Return 403 and skip placeholder creation when the user cannot update.""" + _, item = _build_user_and_item() + other_user = factories.UserFactory() + client = APIClient() + client.force_login(other_user) + + with mock.patch("core.api.viewsets.convert_file_task.delay") as delay_mock: + response = client.post(f"/api/v1.0/items/{item.id}/convert/") + + assert response.status_code == 403 + assert not models.Item.objects.filter( + upload_state=models.ItemUploadStateChoices.CONVERTING + ).exists() + delay_mock.assert_not_called() + + +def test_convert_endpoint_returns_400_for_unsupported_extension(): + """Reject conversion with 400 for non-legacy file extensions.""" + user = factories.UserFactory() + item = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FILE, + filename="image.png", + update_upload_state=models.ItemUploadStateChoices.READY, + ) + client = APIClient() + client.force_login(user) + + with mock.patch("core.api.viewsets.convert_file_task.delay") as delay_mock: + response = client.post(f"/api/v1.0/items/{item.id}/convert/") + + assert response.status_code == 400 + delay_mock.assert_not_called() diff --git a/src/backend/wopi/apps.py b/src/backend/wopi/apps.py index 43a31009f..f6dc99de5 100644 --- a/src/backend/wopi/apps.py +++ b/src/backend/wopi/apps.py @@ -7,3 +7,8 @@ class WopiConfig(AppConfig): """Configuration class for the wopi app.""" name = "wopi" + + def ready(self): + """Import celery tasks.""" + # pylint: disable=import-outside-toplevel, unused-import + from .tasks import conversion # noqa: PLC0415,F401 diff --git a/src/backend/wopi/tasks/__init__.py b/src/backend/wopi/tasks/__init__.py index e69de29bb..91e916ad7 100644 --- a/src/backend/wopi/tasks/__init__.py +++ b/src/backend/wopi/tasks/__init__.py @@ -0,0 +1 @@ +"""Celery task package for the WOPI app.""" diff --git a/src/backend/wopi/tasks/conversion.py b/src/backend/wopi/tasks/conversion.py new file mode 100644 index 000000000..655b5e120 --- /dev/null +++ b/src/backend/wopi/tasks/conversion.py @@ -0,0 +1,61 @@ +"""Celery tasks for WOPI conversion.""" + +import logging + +from django.contrib.auth import get_user_model + +from core import models +from wopi.conversion.services import perform_conversion + +from drive.celery_app import app + +logger = logging.getLogger(__name__) + + +@app.task +def convert_file(source_item_id, converted_item_id, user_id): + """Convert the source item and attach the result to the placeholder. + + Mirrors the duplicate_file task pattern: the placeholder Item already + exists in CONVERTING state; on failure it is hard-deleted so the folder + stops showing a stuck spinner. + """ + User = get_user_model() # pylint: disable=invalid-name + + try: + source = models.Item.objects.get(id=source_item_id) + except models.Item.DoesNotExist: + logger.error("convert_file: source item %s does not exist, aborting", source_item_id) + return + + try: + placeholder = models.Item.objects.get(id=converted_item_id) + except models.Item.DoesNotExist: + logger.error( + "convert_file: placeholder item %s does not exist, aborting", converted_item_id + ) + return + + if placeholder.upload_state != models.ItemUploadStateChoices.CONVERTING: + logger.error( + "convert_file: placeholder %s upload_state is %s, not converting; aborting", + placeholder.id, + placeholder.upload_state, + ) + return + + try: + user = User.objects.get(id=user_id) + except User.DoesNotExist: + logger.error("convert_file: user %s does not exist, aborting", user_id) + placeholder.soft_delete() + placeholder.delete() + return + + try: + perform_conversion(source, placeholder, user) + except Exception: + logger.exception("convert_file: conversion failed for placeholder %s", placeholder.id) + placeholder.soft_delete() + placeholder.delete() + raise diff --git a/src/backend/wopi/tests/tasks/test_conversion.py b/src/backend/wopi/tests/tasks/test_conversion.py new file mode 100644 index 000000000..7bfe2c287 --- /dev/null +++ b/src/backend/wopi/tests/tasks/test_conversion.py @@ -0,0 +1,121 @@ +"""Tests for the conversion celery tasks.""" + +import pytest + +from core import factories, models +from wopi.conversion import exceptions +from wopi.tasks import conversion + +pytestmark = pytest.mark.django_db + + +def _source_and_placeholder(): + """Build a user with a ready source file and a converting placeholder.""" + user = factories.UserFactory() + source = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FILE, + update_upload_state=models.ItemUploadStateChoices.READY, + ) + placeholder = factories.ItemFactory( + users=[(user, models.RoleChoices.EDITOR)], + type=models.ItemTypeChoices.FILE, + filename="document.docx", + update_upload_state=models.ItemUploadStateChoices.CONVERTING, + ) + return user, source, placeholder + + +def test_convert_file_calls_perform_conversion(monkeypatch): + """Delegate to perform_conversion with the resolved entities on success.""" + user, source, placeholder = _source_and_placeholder() + calls = {} + + def fake_perform(src, ph, usr): + calls["args"] = (src.id, ph.id, usr.id) + + monkeypatch.setattr(conversion, "perform_conversion", fake_perform) + + conversion.convert_file( + source_item_id=str(source.id), + converted_item_id=str(placeholder.id), + user_id=str(user.id), + ) + + assert calls["args"] == (source.id, placeholder.id, user.id) + + +def test_convert_file_deletes_placeholder_on_conversion_error(monkeypatch): + """Hard-delete the placeholder when perform_conversion raises ConversionError.""" + user, source, placeholder = _source_and_placeholder() + + def fail(*_args, **_kwargs): + raise exceptions.ConversionError("boom") + + monkeypatch.setattr(conversion, "perform_conversion", fail) + + with pytest.raises(exceptions.ConversionError): + conversion.convert_file( + source_item_id=str(source.id), + converted_item_id=str(placeholder.id), + user_id=str(user.id), + ) + + assert not models.Item.objects.filter(id=placeholder.id).exists() + + +def test_convert_file_deletes_placeholder_on_unexpected_exception(monkeypatch): + """Clean up the placeholder and re-raise on any uncaught exception.""" + user, source, placeholder = _source_and_placeholder() + + def fail(*_args, **_kwargs): + raise RuntimeError("unexpected") + + monkeypatch.setattr(conversion, "perform_conversion", fail) + + with pytest.raises(RuntimeError): + conversion.convert_file( + source_item_id=str(source.id), + converted_item_id=str(placeholder.id), + user_id=str(user.id), + ) + + assert not models.Item.objects.filter(id=placeholder.id).exists() + + +def test_convert_file_aborts_when_placeholder_state_changed(monkeypatch): + """Skip work when the placeholder is no longer CONVERTING.""" + user, source, placeholder = _source_and_placeholder() + placeholder.upload_state = models.ItemUploadStateChoices.READY + placeholder.save(update_fields=["upload_state", "updated_at"]) + + def boom(*_args, **_kwargs): + raise AssertionError("perform_conversion should not be called") + + monkeypatch.setattr(conversion, "perform_conversion", boom) + + conversion.convert_file( + source_item_id=str(source.id), + converted_item_id=str(placeholder.id), + user_id=str(user.id), + ) + + +def test_convert_file_ignores_missing_source(): + """Ignore a missing source item without crashing.""" + conversion.convert_file( + source_item_id="00000000-0000-0000-0000-000000000000", + converted_item_id="00000000-0000-0000-0000-000000000000", + user_id="00000000-0000-0000-0000-000000000000", + ) + + +def test_convert_file_ignores_missing_placeholder(): + """Ignore a missing placeholder without crashing.""" + user, source, _ = _source_and_placeholder() + + conversion.convert_file( + source_item_id=str(source.id), + converted_item_id="00000000-0000-0000-0000-000000000000", + user_id=str(user.id), + ) From fc29a8812293da88cdd4f7f780c1598c1a1d39ed Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 15:49:59 +0200 Subject: [PATCH 08/20] =?UTF-8?q?=F0=9F=90=9B(backend)=20normalize=20WOPI?= =?UTF-8?q?=20extension=20lookup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uploaded filenames preserve extension case, while WOPI discovery stores extension keys in lowercase. Normalize the lookup so legacy files like REPORT.DOC follow the same conversion path. --- .../tests/items/test_api_items_retrieve.py | 5 +++- src/backend/wopi/tests/test_utils.py | 23 +++++++++++++++++++ src/backend/wopi/utils/__init__.py | 8 ++++--- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/backend/core/tests/items/test_api_items_retrieve.py b/src/backend/core/tests/items/test_api_items_retrieve.py index c8415f8bc..30b373185 100644 --- a/src/backend/core/tests/items/test_api_items_retrieve.py +++ b/src/backend/core/tests/items/test_api_items_retrieve.py @@ -1516,7 +1516,10 @@ def test_api_items_retrieve_wopi_supported(): WOPI_CONFIGURATION_CACHE_KEY, { "mimetypes": { - "application/vnd.oasis.opendocument.text": "https://vendorA.com/launch_url", + "application/vnd.oasis.opendocument.text": { + "url": "https://vendorA.com/launch_url", + "client": "vendorA", + }, }, "extensions": {}, }, diff --git a/src/backend/wopi/tests/test_utils.py b/src/backend/wopi/tests/test_utils.py index 8a098e791..e1e297311 100644 --- a/src/backend/wopi/tests/test_utils.py +++ b/src/backend/wopi/tests/test_utils.py @@ -352,6 +352,29 @@ def test_get_wopi_client_config(): assert get_wopi_client_config(item, user) is None +def test_get_wopi_client_config_normalizes_uppercase_extension(): + """Match the WOPI client config when the item extension is uppercase.""" + cache.set( + WOPI_CONFIGURATION_CACHE_KEY, + { + "mimetypes": {}, + "extensions": { + "doc": {"url": "https://vendorA.com/launch_url", "client": "vendorA"}, + }, + }, + ) + user = UserFactory() + item = ItemFactory( + type=models.ItemTypeChoices.FILE, + update_upload_state=models.ItemUploadStateChoices.READY, + filename="REPORT.DOC", + ) + assert get_wopi_client_config(item, user) == { + "url": "https://vendorA.com/launch_url", + "client": "vendorA", + } + + @pytest.mark.parametrize( "upload_state", [ diff --git a/src/backend/wopi/utils/__init__.py b/src/backend/wopi/utils/__init__.py index 4893cf9e5..bf2834b09 100644 --- a/src/backend/wopi/utils/__init__.py +++ b/src/backend/wopi/utils/__init__.py @@ -39,9 +39,11 @@ def get_wopi_client_config(item, user): return None result = None - # Extension must always be checked first. - if item.extension in wopi_configuration["extensions"]: - result = wopi_configuration["extensions"][item.extension] + # Extension must always be checked first. Filenames preserve case (REPORT.DOC), + # while the discovery stores extensions in lowercase, so normalize the lookup. + extension = item.extension.lower() if item.extension else None + if extension and extension in wopi_configuration["extensions"]: + result = wopi_configuration["extensions"][extension] elif item.mimetype in wopi_configuration["mimetypes"]: result = wopi_configuration["mimetypes"][item.mimetype] From e29fe9ebdb21e67ac0156c6de2198c067a881d24 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 15:50:34 +0200 Subject: [PATCH 09/20] =?UTF-8?q?=E2=9C=A8(frontend)=20wire=20conversion?= =?UTF-8?q?=20API=20client?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontend needs to treat conversion as item creation rather than job polling. Return the placeholder item from the driver so UI state stays aligned with the explorer list. --- src/frontend/apps/drive/src/features/drivers/Driver.ts | 1 + .../src/features/drivers/implementations/StandardDriver.ts | 7 +++++++ src/frontend/apps/drive/src/features/drivers/types.ts | 2 ++ 3 files changed, 10 insertions(+) diff --git a/src/frontend/apps/drive/src/features/drivers/Driver.ts b/src/frontend/apps/drive/src/features/drivers/Driver.ts index fba2a84a3..2cbc8c74a 100644 --- a/src/frontend/apps/drive/src/features/drivers/Driver.ts +++ b/src/frontend/apps/drive/src/features/drivers/Driver.ts @@ -159,6 +159,7 @@ export abstract class Driver { abstract deleteItems(ids: string[]): Promise; abstract hardDeleteItems(ids: string[]): Promise; abstract getWopiInfo(itemId: string): Promise; + abstract convertItem(itemId: string): Promise; abstract getEntitlements(): Promise; diff --git a/src/frontend/apps/drive/src/features/drivers/implementations/StandardDriver.ts b/src/frontend/apps/drive/src/features/drivers/implementations/StandardDriver.ts index 25cfd5a5b..0f6d75d0f 100644 --- a/src/frontend/apps/drive/src/features/drivers/implementations/StandardDriver.ts +++ b/src/frontend/apps/drive/src/features/drivers/implementations/StandardDriver.ts @@ -454,6 +454,13 @@ export class StandardDriver extends Driver { return jsonToItem(await response.json()); } + async convertItem(itemId: string): Promise { + const response = await fetchAPI(`items/${itemId}/convert/`, { + method: "POST", + }); + return jsonToItem(await response.json()); + } + async deleteItems(ids: string[]): Promise { for (const id of ids) { await fetchAPI(`items/${id}/`, { diff --git a/src/frontend/apps/drive/src/features/drivers/types.ts b/src/frontend/apps/drive/src/features/drivers/types.ts index 7a6b20ac2..b2c5283e2 100644 --- a/src/frontend/apps/drive/src/features/drivers/types.ts +++ b/src/frontend/apps/drive/src/features/drivers/types.ts @@ -20,6 +20,7 @@ export enum LinkRole { export enum ItemUploadState { PENDING = "pending", DUPLICATING = "duplicating", + CONVERTING = "converting", ANALYZING = "analyzing", SUSPICIOUS = "suspicious", FILE_TOO_LARGE_TO_ANALYZE = "file_too_large_to_analyze", @@ -93,6 +94,7 @@ export type Item = { tree: boolean; update: boolean; upload_ended: boolean; + must_convert?: boolean; }; policy?: string; }; From d6076ac01824dbfe114c70879a489d8d642d9162 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 15:51:52 +0200 Subject: [PATCH 10/20] =?UTF-8?q?=E2=9C=A8(frontend)=20add=20conversion=20?= =?UTF-8?q?modal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users should understand that opening a legacy file creates an editable copy. Keep the modal focused on confirmation and let the folder show the ongoing conversion state. --- .../components/app-view/AppExplorerGrid.tsx | 22 +++++- .../modals/ConvertLegacyFileModal.tsx | 69 +++++++++++++++++++ .../drive/src/features/i18n/translations.json | 36 +++++++++- 3 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 src/frontend/apps/drive/src/features/explorer/components/modals/ConvertLegacyFileModal.tsx diff --git a/src/frontend/apps/drive/src/features/explorer/components/app-view/AppExplorerGrid.tsx b/src/frontend/apps/drive/src/features/explorer/components/app-view/AppExplorerGrid.tsx index 206d61b9e..bac36f55a 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/app-view/AppExplorerGrid.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/app-view/AppExplorerGrid.tsx @@ -13,11 +13,14 @@ import { import { InfiniteScroll } from "@/features/ui/components/infinite-scroll/InfiniteScroll"; import { useRouter } from "next/router"; import { DefaultRoute, getDefaultRouteId } from "@/utils/defaultRoutes"; -import { useMemo } from "react"; +import { useMemo, useState } from "react"; import { canCreateChildren } from "@/features/items/utils"; import { Spinner } from "@gouvfr-lasuite/ui-kit"; import { openWopiInNewTab } from "@/features/wopi/openWopi"; import { itemToPreviewFile } from "@/features/explorer/utils/utils"; +import { useModal } from "@gouvfr-lasuite/cunningham-react"; +import { ConvertLegacyFileModal } from "@/features/explorer/components/modals/ConvertLegacyFileModal"; +import { useRefreshQueryCacheAfterMutation } from "@/features/explorer/hooks/useRefreshItems"; /** * Wrapper around EmbeddedExplorerGrid to display a list of items in a table. @@ -45,9 +48,18 @@ export const AppExplorerGrid = () => { const effectiveOnNavigate = appExplorer.onNavigate ?? onNavigate; + const convertModal = useModal(); + const [itemToConvert, setItemToConvert] = useState(null); + const refresh = useRefreshQueryCacheAfterMutation(); + const handleFileClick = appExplorer.onFileClick ?? ((item: Item) => { + if (item.abilities.must_convert) { + setItemToConvert(item); + convertModal.open(); + return; + } if (item.is_wopi_supported) { openWopiInNewTab(itemToPreviewFile(item)); return; @@ -171,6 +183,14 @@ export const AppExplorerGrid = () => { )} + {convertModal.isOpen && itemToConvert && ( + refresh(item?.originalId ?? item?.id)} + /> + )} ); }; diff --git a/src/frontend/apps/drive/src/features/explorer/components/modals/ConvertLegacyFileModal.tsx b/src/frontend/apps/drive/src/features/explorer/components/modals/ConvertLegacyFileModal.tsx new file mode 100644 index 000000000..e7710a31b --- /dev/null +++ b/src/frontend/apps/drive/src/features/explorer/components/modals/ConvertLegacyFileModal.tsx @@ -0,0 +1,69 @@ +import { useState } from "react"; +import { Button, Modal, ModalSize } from "@gouvfr-lasuite/cunningham-react"; +import { useTranslation } from "react-i18next"; +import { getDriver } from "@/features/config/Config"; +import { Item } from "@/features/drivers/types"; + +type Props = { + item: Item; + isOpen: boolean; + onClose: () => void; + onSuccess: () => void; +}; + +export const ConvertLegacyFileModal = ({ item, isOpen, onClose, onSuccess }: Props) => { + const { t } = useTranslation(); + const [isSubmitting, setIsSubmitting] = useState(false); + const [error, setError] = useState(null); + + const handleClose = () => { + if (isSubmitting) return; + setError(null); + onClose(); + }; + + const handleConvert = async () => { + setIsSubmitting(true); + setError(null); + try { + await getDriver().convertItem(item.id); + onSuccess(); + onClose(); + } catch { + setError(t("explorer.actions.convert.modal.error")); + setIsSubmitting(false); + } + }; + + return ( + + + + + } + > +
+ {error ? ( +

{error}

+ ) : ( + t("explorer.actions.convert.modal.content") + )} +
+ + ); +}; diff --git a/src/frontend/apps/drive/src/features/i18n/translations.json b/src/frontend/apps/drive/src/features/i18n/translations.json index 4b6614ca2..ec3597470 100644 --- a/src/frontend/apps/drive/src/features/i18n/translations.json +++ b/src/frontend/apps/drive/src/features/i18n/translations.json @@ -483,6 +483,15 @@ "cancel": "Cancel", "submit": "Rename" } + }, + "convert": { + "modal": { + "title": "Convert to open this file", + "content": "Drive will create an editable copy. Your original file stays untouched.", + "cancel": "Cancel", + "confirm": "Convert", + "error": "Conversion failed. Please try again." + } } }, "item": { @@ -501,7 +510,8 @@ "duplicate_error": "An error occurred while duplicating the item.", "delete": "Delete" }, - "duplicating": "duplication in progress" + "duplicating": "duplication in progress", + "converting": "conversion in progress" } }, "time": { @@ -1073,6 +1083,15 @@ "cancel": "Annuler", "submit": "Renommer" } + }, + "convert": { + "modal": { + "title": "Convertir pour ouvrir ce fichier", + "content": "Drive va créer une copie modifiable. Votre fichier d'origine reste intact.", + "cancel": "Annuler", + "confirm": "Convertir", + "error": "La conversion a échoué. Veuillez réessayer." + } } }, "item": { @@ -1091,7 +1110,8 @@ "duplicate_error": "Une erreur est survenue lors de la duplication.", "delete": "Supprimer" }, - "duplicating": "duplication en cours" + "duplicating": "duplication en cours", + "converting": "conversion en cours" } }, "time": { @@ -1657,6 +1677,15 @@ "cancel": "Annuleren", "submit": "Hernoem" } + }, + "convert": { + "modal": { + "title": "Converteren om dit bestand te openen", + "content": "Drive maakt een bewerkbare kopie. Uw originele bestand blijft ongewijzigd.", + "cancel": "Annuleren", + "confirm": "Converteren", + "error": "Conversie mislukt. Probeer het opnieuw." + } } }, "item": { @@ -1675,7 +1704,8 @@ "duplicate_error": "Er is een fout opgetreden bij het dupliceren.", "delete": "Verwijder" }, - "duplicating": "duplicatie bezig" + "duplicating": "duplicatie bezig", + "converting": "conversie bezig" } }, "time": { From f3d884abb7445f9fd254fe7feaeddd3d94bb64cb Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 16:02:18 +0200 Subject: [PATCH 11/20] =?UTF-8?q?=E2=9C=A8(frontend)=20render=20converting?= =?UTF-8?q?=20items=20as=20transient=20rows?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A converting file is visible but not ready for normal file actions. Show the transient state consistently and prevent opening or acting on the item until conversion completes. --- .../apps/drive/src/features/drivers/types.ts | 5 ++++ .../EmbeddedExplorerGrid.tsx | 10 ++++--- .../EmbeddedExplorerGridActionsCell.tsx | 4 +-- .../EmbeddedExplorerGridMobileCell.tsx | 20 +++++++++----- .../EmbeddedExplorerGridNameCell.tsx | 27 ++++++++++++------- .../EmbeddedExplorerGridRow.tsx | 7 ++--- 6 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/frontend/apps/drive/src/features/drivers/types.ts b/src/frontend/apps/drive/src/features/drivers/types.ts index b2c5283e2..1744e0c1a 100644 --- a/src/frontend/apps/drive/src/features/drivers/types.ts +++ b/src/frontend/apps/drive/src/features/drivers/types.ts @@ -27,6 +27,11 @@ export enum ItemUploadState { READY = "ready", } +export const TRANSIENT_UPLOAD_STATES: string[] = [ + ItemUploadState.DUPLICATING, + ItemUploadState.CONVERTING, +]; + export type ItemBreadcrumb = { id: string; originalId?: string; // Used to identify all occurrences of the same item in the tree diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx index 6955e998d..07745413c 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx @@ -1,4 +1,8 @@ -import { Item, ItemType, ItemUploadState } from "@/features/drivers/types"; +import { + Item, + ItemType, + TRANSIENT_UPLOAD_STATES, +} from "@/features/drivers/types"; import { createContext, useCallback, @@ -296,7 +300,7 @@ export const EmbeddedExplorerGrid = (props: EmbeddedExplorerGridProps) => { const handleRowClick = useCallback( (e: React.MouseEvent, row: Row) => { - if (row.original.upload_state === ItemUploadState.DUPLICATING) { + if (TRANSIENT_UPLOAD_STATES.includes(row.original.upload_state)) { return; } @@ -355,7 +359,7 @@ export const EmbeddedExplorerGrid = (props: EmbeddedExplorerGridProps) => { e.preventDefault(); e.stopPropagation(); - if (row.original.upload_state === ItemUploadState.DUPLICATING) { + if (TRANSIENT_UPLOAD_STATES.includes(row.original.upload_state)) { return; } diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridActionsCell.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridActionsCell.tsx index 642cab23b..034349454 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridActionsCell.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridActionsCell.tsx @@ -1,5 +1,5 @@ import { CellContext } from "@tanstack/react-table"; -import { Item, ItemUploadState } from "@/features/drivers/types"; +import { Item, TRANSIENT_UPLOAD_STATES } from "@/features/drivers/types"; import { memo, useState } from "react"; import { Button } from "@gouvfr-lasuite/cunningham-react"; import { Draggable } from "@/features/explorer/components/Draggable"; @@ -21,7 +21,7 @@ const EmbeddedExplorerGridActionsCellComponent = ( const { setIsActionModalOpen, isActionModalOpen } = useEmbeddedExplorerGirdContext(); - if (item.upload_state === ItemUploadState.DUPLICATING) { + if (TRANSIENT_UPLOAD_STATES.includes(item.upload_state)) { return null; } diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridMobileCell.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridMobileCell.tsx index 28bf4a087..9c78b2de5 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridMobileCell.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridMobileCell.tsx @@ -1,5 +1,9 @@ import { CellContext } from "@tanstack/react-table"; -import { Item, ItemUploadState } from "@/features/drivers/types"; +import { + Item, + ItemUploadState, + TRANSIENT_UPLOAD_STATES, +} from "@/features/drivers/types"; import { memo } from "react"; import { ItemIcon } from "@/features/explorer/components/icons/ItemIcon"; import { timeAgo } from "@/features/explorer/utils/utils"; @@ -15,11 +19,15 @@ const EmbeddedExplorerGridMobileCellComponent = ( ) => { const item = params.row.original; const { t } = useTranslation(); - const isDuplicating = item.upload_state === ItemUploadState.DUPLICATING; + const isConverting = item.upload_state === ItemUploadState.CONVERTING; + const isTransient = TRANSIENT_UPLOAD_STATES.includes(item.upload_state); + const transientLabel = isConverting + ? t("explorer.item.converting") + : t("explorer.item.duplicating"); return (
- {isDuplicating ? ( + {isTransient ? (
@@ -30,14 +38,14 @@ const EmbeddedExplorerGridMobileCellComponent = (
{removeFileExtension(item.title)} - {isDuplicating && ( + {isTransient && ( {" "} - ({t("explorer.item.duplicating")}) + ({transientLabel}) )} diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridNameCell.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridNameCell.tsx index d0192af30..99f7ac9b8 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridNameCell.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridNameCell.tsx @@ -1,5 +1,10 @@ import { CellContext } from "@tanstack/react-table"; -import { Item, ItemUploadState, LinkReach } from "@/features/drivers/types"; +import { + Item, + ItemUploadState, + LinkReach, + TRANSIENT_UPLOAD_STATES, +} from "@/features/drivers/types"; import { memo, useEffect, useMemo, useRef, useState } from "react"; import { Draggable } from "@/features/explorer/components/Draggable"; import { Tooltip } from "@gouvfr-lasuite/cunningham-react"; @@ -24,7 +29,11 @@ const EmbeddedExplorerGridNameCellComponent = ( const [isOverflown, setIsOverflown] = useState(false); const { disableItemDragAndDrop } = useEmbeddedExplorerGirdContext(); const isSelected = useIsItemSelected(item.id); - const isDuplicating = item.upload_state === ItemUploadState.DUPLICATING; + const isConverting = item.upload_state === ItemUploadState.CONVERTING; + const isTransient = TRANSIENT_UPLOAD_STATES.includes(item.upload_state); + const transientLabel = isConverting + ? t("explorer.item.converting") + : t("explorer.item.duplicating"); const disableDrag = useDisableDragGridItem(item); @@ -36,20 +45,20 @@ const EmbeddedExplorerGridNameCellComponent = ( id={params.cell.id + "-title"} item={item} className="explorer__grid__item__name__title-wrapper" - disabled={isDuplicating || disableItemDragAndDrop || isSelected} // If it's selected then we can drag on the entire cell + disabled={isTransient || disableItemDragAndDrop || isSelected} // If it's selected then we can drag on the entire cell >
{removeFileExtension(item.title)} - {isDuplicating && ( + {isTransient && ( {" "} - ({t("explorer.item.duplicating")}) + ({transientLabel}) )} {params.children} @@ -90,14 +99,14 @@ const EmbeddedExplorerGridNameCellComponent = (
- {isDuplicating ? ( + {isTransient ? (
diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridRow.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridRow.tsx index 5a8cb1a22..af417d135 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridRow.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGridRow.tsx @@ -1,7 +1,7 @@ import { memo } from "react"; import { Row, flexRender } from "@tanstack/react-table"; import clsx from "clsx"; -import { Item, ItemType, ItemUploadState } from "@/features/drivers/types"; +import { Item, ItemType, TRANSIENT_UPLOAD_STATES } from "@/features/drivers/types"; import { Droppable } from "@/features/explorer/components/Droppable"; import { useIsItemSelected } from "@/features/explorer/stores/selectionStore"; @@ -25,14 +25,15 @@ const EmbeddedExplorerGridRowComponent = ({ }: EmbeddedExplorerGridRowProps) => { const item = row.original; const isSelected = useIsItemSelected(item.id); + const isTransient = TRANSIENT_UPLOAD_STATES.includes(item.upload_state); return ( Date: Thu, 28 May 2026 18:00:58 +0200 Subject: [PATCH 12/20] =?UTF-8?q?=E2=99=BB=EF=B8=8F(frontend)=20generalize?= =?UTF-8?q?=20duplicating=20poller=20to=20transient=20items?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same polling loop fits any transient upload state. Rename the duplicating poller to a transient one driven by TRANSIENT_UPLOAD_STATES so converting items are picked up without copying the hook, and the explorer refreshes automatically when background conversion finishes. --- CHANGELOG.md | 1 + .../EmbeddedExplorerGrid.tsx | 4 +-- ...msPoller.ts => useTransientItemsPoller.ts} | 25 ++++++++++--------- 3 files changed, 16 insertions(+), 14 deletions(-) rename src/frontend/apps/drive/src/features/explorer/hooks/{useDuplicatingItemsPoller.ts => useTransientItemsPoller.ts} (64%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a09ccb22..843e1f9d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to - ✨(backend) manage reconciliation requests for user accounts - ✨(backend) add recursive folder export as ZIP archive - ✨(frontend) add folder export action +- ✨(backend) background conversion of legacy Office files ### Changed diff --git a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx index 07745413c..f2d4aa02b 100644 --- a/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx +++ b/src/frontend/apps/drive/src/features/explorer/components/embedded-explorer/EmbeddedExplorerGrid.tsx @@ -45,7 +45,7 @@ import { } from "../../types/columns"; import { ColumnHeader } from "./headers/ColumnHeader"; import { CustomizableColumnHeader } from "./headers/CustomizableColumnHeader"; -import { useDuplicatingItemsPoller } from "../../hooks/useDuplicatingItemsPoller"; +import { useTransientItemsPoller } from "../../hooks/useTransientItemsPoller"; import { EmbeddedExplorerGridRow } from "./EmbeddedExplorerGridRow"; import posthog from "posthog-js"; @@ -128,7 +128,7 @@ export const EmbeddedExplorerGrid = (props: EmbeddedExplorerGridProps) => { }); const contextMenu = useContextMenuContext(); - useDuplicatingItemsPoller(props.items ?? EMPTY_ARRAY); + useTransientItemsPoller(props.items ?? EMPTY_ARRAY); const selectionStore = useSelectionStore(); // TODO: This hook makes use of the ExplorerContext to manage the overred items. So, this component is not really standalone as it should be. diff --git a/src/frontend/apps/drive/src/features/explorer/hooks/useDuplicatingItemsPoller.ts b/src/frontend/apps/drive/src/features/explorer/hooks/useTransientItemsPoller.ts similarity index 64% rename from src/frontend/apps/drive/src/features/explorer/hooks/useDuplicatingItemsPoller.ts rename to src/frontend/apps/drive/src/features/explorer/hooks/useTransientItemsPoller.ts index 466dcee0c..d412a909c 100644 --- a/src/frontend/apps/drive/src/features/explorer/hooks/useDuplicatingItemsPoller.ts +++ b/src/frontend/apps/drive/src/features/explorer/hooks/useTransientItemsPoller.ts @@ -1,48 +1,49 @@ import { useMemo, useEffect, useRef } from "react"; import { useQueries } from "@tanstack/react-query"; import { getDriver } from "@/features/config/Config"; -import { Item, ItemUploadState } from "@/features/drivers/types"; +import { Item, TRANSIENT_UPLOAD_STATES } from "@/features/drivers/types"; import { useRefreshItemCache } from "./useRefreshItems"; const POLL_INTERVAL = 3000; const POLL_TIMEOUT = 10 * 60 * 1000; -export const useDuplicatingItemsPoller = (items: Item[]) => { +export const useTransientItemsPoller = (items: Item[]) => { const refreshItemCache = useRefreshItemCache(); const startTimesRef = useRef>(new Map()); - const duplicatingItems = useMemo( - () => items.filter((i) => i.upload_state === ItemUploadState.DUPLICATING), + const transientItems = useMemo( + () => + items.filter((i) => TRANSIENT_UPLOAD_STATES.includes(i.upload_state)), [items], ); useEffect(() => { - for (const item of duplicatingItems) { + for (const item of transientItems) { if (!startTimesRef.current.has(item.id)) { startTimesRef.current.set(item.id, Date.now()); } } - const duplicatingIds = new Set(duplicatingItems.map((i) => i.id)); + const transientIds = new Set(transientItems.map((i) => i.id)); for (const id of startTimesRef.current.keys()) { - if (!duplicatingIds.has(id)) { + if (!transientIds.has(id)) { startTimesRef.current.delete(id); } } - }, [duplicatingItems]); + }, [transientItems]); useQueries({ - queries: duplicatingItems.map((item) => ({ - queryKey: ["items", item.id, "duplicate-poll"], + queries: transientItems.map((item) => ({ + queryKey: ["items", item.id, "transient-poll"], queryFn: async () => { const updatedItem = await getDriver().getItem(item.id); - if (updatedItem.upload_state !== ItemUploadState.DUPLICATING) { + if (!TRANSIENT_UPLOAD_STATES.includes(updatedItem.upload_state)) { await refreshItemCache(item.id, updatedItem); } return updatedItem; }, refetchInterval: (query: { state: { data: Item | undefined } }) => { const data = query.state.data; - if (data && data.upload_state !== ItemUploadState.DUPLICATING) { + if (data && !TRANSIENT_UPLOAD_STATES.includes(data.upload_state)) { return false; } const startTime = startTimesRef.current.get(item.id) ?? Date.now(); From c067374a14026dbcf5c017f3406c55e6fe580455 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Wed, 27 May 2026 16:05:34 +0200 Subject: [PATCH 13/20] =?UTF-8?q?=E2=9C=85(e2e)=20cover=20conversion=20exp?= =?UTF-8?q?lorer=20flow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conversion flow crosses modal state, API calls, and explorer refreshes. Cover the expected user path with a mocked conversion ability and placeholder response. --- .../apps/e2e/__tests__/app-drive/wopi.spec.ts | 161 +++++++++++++++++- 1 file changed, 160 insertions(+), 1 deletion(-) diff --git a/src/frontend/apps/e2e/__tests__/app-drive/wopi.spec.ts b/src/frontend/apps/e2e/__tests__/app-drive/wopi.spec.ts index fc20b6a84..fd041c9a4 100644 --- a/src/frontend/apps/e2e/__tests__/app-drive/wopi.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-drive/wopi.spec.ts @@ -1,5 +1,5 @@ import path from "path"; -import test, { expect } from "@playwright/test"; +import test, { expect, Page } from "@playwright/test"; import { clearDb, login } from "./utils-common"; import { clickToMyFiles } from "./utils-navigate"; import { getRowItem } from "./utils-embedded-grid"; @@ -9,6 +9,165 @@ import { grantClipboardPermissions } from "./utils/various-utils"; const IMAGE_FILE_PATH = path.join(__dirname, "/assets/test-image.png"); const DOCX_FILE_PATH = path.join(__dirname, "/assets/empty_doc.docx"); +/** + * Intercepts items API responses to inject must_convert: true on all items. + */ +const mockRequiresConversion = async (page: Page) => { + const inject = (item: Record) => { + item.abilities = { + ...(item.abilities as Record), + must_convert: true, + }; + }; + await page.route( + (url) => /\/api\/v1\.0\/items(\/|$)/.test(url.pathname), + async (route) => { + if (route.request().method() !== "GET") { + await route.continue(); + return; + } + const response = await route.fetch(); + const json = await response.json(); + if (Array.isArray(json?.results)) { + json.results.forEach(inject); + } else if (json?.id) { + inject(json); + } + await route.fulfill({ response, json }); + }, + ); +}; + +test("Double-clicking a file with must_convert ability opens the conversion modal", async ({ + page, + browserName, +}) => { + test.skip(browserName !== "chromium", "Only runs on chromium"); + await clearDb(); + await login(page, "drive@example.com"); + await mockRequiresConversion(page); + await page.goto("/"); + await clickToMyFiles(page); + await expect(page.getByText("This tab is empty")).toBeVisible(); + + await uploadFile(page, DOCX_FILE_PATH); + await expect(page.getByText("Drop your files here")).not.toBeVisible(); + + const row = await getRowItem(page, "empty_doc"); + await row.dblclick(); + + await expect( + page.getByRole("heading", { name: "Convert to open this file" }), + ).toBeVisible(); +}); + +test("Cancelling the conversion modal (must_convert mocked) does not call the convert API", async ({ + page, + browserName, +}) => { + test.skip(browserName !== "chromium", "Only runs on chromium"); + await clearDb(); + await login(page, "drive@example.com"); + await mockRequiresConversion(page); + await page.goto("/"); + await clickToMyFiles(page); + await expect(page.getByText("This tab is empty")).toBeVisible(); + + await uploadFile(page, DOCX_FILE_PATH); + await expect(page.getByText("Drop your files here")).not.toBeVisible(); + + let convertCalled = false; + await page.route("**/api/v1.0/items/*/convert/", async (route) => { + convertCalled = true; + await route.continue(); + }); + + const row = await getRowItem(page, "empty_doc"); + await row.dblclick(); + await expect( + page.getByRole("heading", { name: "Convert to open this file" }), + ).toBeVisible(); + + await page.getByRole("button", { name: "Cancel" }).click(); + await expect( + page.getByRole("heading", { name: "Convert to open this file" }), + ).not.toBeVisible(); + expect(convertCalled).toBe(false); +}); + +test("Confirming the conversion modal (must_convert mocked) shows the converting placeholder in the folder", async ({ + page, + browserName, +}) => { + test.skip(browserName !== "chromium", "Only runs on chromium"); + await clearDb(); + await login(page, "drive@example.com"); + await mockRequiresConversion(page); + await page.goto("/"); + await clickToMyFiles(page); + await expect(page.getByText("This tab is empty")).toBeVisible(); + + await uploadFile(page, DOCX_FILE_PATH); + await expect(page.getByText("Drop your files here")).not.toBeVisible(); + + // The POST /convert/ now returns the placeholder Item directly. We mock it + // and let the front-end refresh the folder so the placeholder appears. + const placeholderId = "00000000-0000-0000-0000-000000000001"; + await page.route("**/api/v1.0/items/*/convert/", async (route) => { + await route.fulfill({ + status: 201, + contentType: "application/json", + body: JSON.stringify({ + id: placeholderId, + title: "empty_doc.docx", + filename: "empty_doc.docx", + type: "file", + upload_state: "converting", + abilities: {}, + }), + }); + }); + + // Inject the placeholder in the folder list returned by /items/ so it shows + // up after the post-conversion refresh. + await page.route( + (url) => /\/api\/v1\.0\/items(\/|$)/.test(url.pathname), + async (route) => { + if (route.request().method() !== "GET") { + await route.continue(); + return; + } + const response = await route.fetch(); + const json = await response.json(); + const placeholder = { + id: placeholderId, + title: "empty_doc.docx", + filename: "empty_doc.docx", + type: "file", + upload_state: "converting", + abilities: {}, + }; + if (Array.isArray(json?.results)) { + json.results.push(placeholder); + } + await route.fulfill({ response, json }); + }, + ); + + const row = await getRowItem(page, "empty_doc"); + await row.dblclick(); + await expect( + page.getByRole("heading", { name: "Convert to open this file" }), + ).toBeVisible(); + + await page.getByRole("button", { name: "Convert" }).click(); + + await expect( + page.getByRole("heading", { name: "Convert to open this file" }), + ).not.toBeVisible({ timeout: 5000 }); + await expect(page.getByText("conversion in progress")).toBeVisible(); +}); + test("Double-clicking a WOPI file opens the editor in a new tab", async ({ page, context, From be1eb1f5d7cc489678034dd500e76ab54d6d6286 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 1 Jun 2026 14:53:59 +0200 Subject: [PATCH 14/20] =?UTF-8?q?fixup!=20=E2=9C=A8(backend)=20add=20OnlyO?= =?UTF-8?q?ffice=20conversion=20backend?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../wopi/tests/test_conversion_backends_onlyoffice.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/backend/wopi/tests/test_conversion_backends_onlyoffice.py b/src/backend/wopi/tests/test_conversion_backends_onlyoffice.py index 54dac1cc7..6ec3ef659 100644 --- a/src/backend/wopi/tests/test_conversion_backends_onlyoffice.py +++ b/src/backend/wopi/tests/test_conversion_backends_onlyoffice.py @@ -1,9 +1,7 @@ """Tests for the OnlyOffice server-to-server conversion backend.""" import json -from types import SimpleNamespace from unittest import mock -from uuid import uuid4 from django.conf import settings @@ -12,6 +10,7 @@ import requests import responses +from core import factories from wopi.conversion import exceptions from wopi.conversion.backends.onlyoffice import OnlyOfficeConversionBackend @@ -21,9 +20,9 @@ JWT_SECRET = "test-secret-with-enough-length-for-hs256" -def _item(filename="document.doc", extension="doc"): - """Build a minimal Item-like namespace for backend tests.""" - return SimpleNamespace(id=uuid4(), filename=filename, extension=extension) +def _item(filename="document.doc"): + """Build a minimal in-memory Item for backend tests.""" + return factories.ItemFactory.build(filename=filename) def _ok_body(): From eacabb1c5da574ca6286b94ae26598b8b4f03af3 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 1 Jun 2026 14:53:59 +0200 Subject: [PATCH 15/20] =?UTF-8?q?fixup!=20=E2=9C=A8(backend)=20add=20legac?= =?UTF-8?q?y=20conversion=20policy=20helpers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../wopi/tests/test_conversion_policy.py | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/backend/wopi/tests/test_conversion_policy.py b/src/backend/wopi/tests/test_conversion_policy.py index e7e8f177e..e26f06bd2 100644 --- a/src/backend/wopi/tests/test_conversion_policy.py +++ b/src/backend/wopi/tests/test_conversion_policy.py @@ -1,15 +1,14 @@ """Tests for the conversion policy helpers.""" -from types import SimpleNamespace - from django.conf import settings +from core import factories from wopi.conversion.policy import is_forced_conversion, target_extension_for -def _item(extension="doc", mimetype="application/msword"): - """Build a minimal item-like namespace for policy tests.""" - return SimpleNamespace(extension=extension, mimetype=mimetype) +def _item(filename="document.doc", mimetype="application/msword"): + """Build a minimal in-memory Item for policy tests.""" + return factories.ItemFactory.build(filename=filename, mimetype=mimetype) def test_target_extension_for_known_legacy_formats(): @@ -40,16 +39,14 @@ def test_legacy_conversion_targets_only_lists_legacy_formats(): def test_is_forced_conversion_true_when_extension_listed(): """Force conversion when the item extension is listed.""" options = {"ForceConvertExtensions": ["doc", "xls", "ppt"]} - assert is_forced_conversion(_item(extension="doc"), options) is True + assert is_forced_conversion(_item(filename="document.doc"), options) is True def test_is_forced_conversion_true_when_mimetype_listed(): """Force conversion when the item mimetype is listed.""" options = {"ForceConvertMimetypes": ["application/msword"]} - assert ( - is_forced_conversion(_item(extension="unknown", mimetype="application/msword"), options) - is True - ) + item = _item(filename="document.unknown", mimetype="application/msword") + assert is_forced_conversion(item, options) is True def test_is_forced_conversion_false_when_neither_matches(): @@ -58,7 +55,7 @@ def test_is_forced_conversion_false_when_neither_matches(): "ForceConvertExtensions": ["doc"], "ForceConvertMimetypes": ["application/msword"], } - item = _item(extension="docx", mimetype="application/vnd.openxmlformats") + item = _item(filename="document.docx", mimetype="application/vnd.openxmlformats") assert is_forced_conversion(item, options) is False @@ -70,10 +67,8 @@ def test_is_forced_conversion_false_when_options_missing(): def test_is_forced_conversion_extension_check_is_case_insensitive(): """Match forced extensions regardless of case.""" - # Item.extension preserves filename case (REPORT.DOC -> "DOC"); the - # policy must match regardless. options = {"ForceConvertExtensions": ["doc"]} - assert is_forced_conversion(_item(extension="DOC"), options) is True + assert is_forced_conversion(_item(filename="REPORT.DOC"), options) is True options = {"ForceConvertExtensions": ["DOC"]} - assert is_forced_conversion(_item(extension="doc"), options) is True + assert is_forced_conversion(_item(filename="document.doc"), options) is True From 50eb6f8a97f9f4b363ac3d9c0dbb42bae15af533 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 1 Jun 2026 14:53:59 +0200 Subject: [PATCH 16/20] =?UTF-8?q?fixup!=20=E2=9C=A8(backend)=20build=20sho?= =?UTF-8?q?rt-lived=20WOPI=20source=20URLs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../wopi/tests/test_conversion_source_url.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/backend/wopi/tests/test_conversion_source_url.py b/src/backend/wopi/tests/test_conversion_source_url.py index 52742ed3f..ab2e2b7bb 100644 --- a/src/backend/wopi/tests/test_conversion_source_url.py +++ b/src/backend/wopi/tests/test_conversion_source_url.py @@ -1,11 +1,10 @@ """Tests for the WOPI source-URL builder used by the conversion service.""" -from types import SimpleNamespace from unittest import mock -from uuid import uuid4 import pytest +from core import factories from wopi.conversion import exceptions from wopi.conversion.source_url import build_source_url @@ -22,8 +21,8 @@ def _access_service(monkeypatch): def test_build_source_url_uses_configured_wopi_base_url(settings, _access_service): """Start the built URL with WOPI_SRC_BASE_URL and embed the access token.""" settings.WOPI_SRC_BASE_URL = "https://drive.example" - item = SimpleNamespace(id=uuid4()) - user = SimpleNamespace() + item = factories.ItemFactory.build() + user = factories.UserFactory.build() url = build_source_url(item, user) @@ -35,8 +34,8 @@ def test_build_source_url_uses_configured_wopi_base_url(settings, _access_servic def test_build_source_url_strips_trailing_slash_on_base_url(settings, _access_service): """Strip a trailing slash on the base URL to avoid a double slash.""" settings.WOPI_SRC_BASE_URL = "https://drive.example/" - item = SimpleNamespace(id=uuid4()) - user = SimpleNamespace() + item = factories.ItemFactory.build() + user = factories.UserFactory.build() url = build_source_url(item, user) @@ -46,8 +45,8 @@ def test_build_source_url_strips_trailing_slash_on_base_url(settings, _access_se def test_build_source_url_raises_when_base_url_is_missing(settings, _access_service): """Raise when WOPI_SRC_BASE_URL is missing.""" settings.WOPI_SRC_BASE_URL = None - item = SimpleNamespace(id=uuid4()) - user = SimpleNamespace() + item = factories.ItemFactory.build() + user = factories.UserFactory.build() with pytest.raises(exceptions.ConversionMisconfigured, match="Missing WOPI_SRC_BASE_URL"): build_source_url(item, user) @@ -57,8 +56,8 @@ def test_build_source_url_delegates_to_access_user_item_service(settings, _acces """Issue a short-lived WOPI access token via the existing service.""" settings.WOPI_SRC_BASE_URL = "https://drive.example" settings.WOPI_CONVERSION_SOURCE_TOKEN_TIMEOUT = 90 - item = SimpleNamespace(id=uuid4()) - user = SimpleNamespace() + item = factories.ItemFactory.build() + user = factories.UserFactory.build() build_source_url(item, user) From 265c65304f077fff7391558676b447445e405464 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 1 Jun 2026 15:21:20 +0200 Subject: [PATCH 17/20] =?UTF-8?q?fixup!=20=E2=9C=A8(backend)=20queue=20leg?= =?UTF-8?q?acy=20file=20conversion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/backend/core/api/permissions.py | 1 - src/backend/core/api/viewsets.py | 9 ++------- .../core/tests/items/test_api_items_convert.py | 14 +++++++------- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index 2fec31254..1e8aad414 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -11,7 +11,6 @@ ACTION_FOR_METHOD_TO_PERMISSION = { "versions_detail": {"DELETE": "versions_destroy", "GET": "versions_retrieve"}, "children": {"GET": "children_list", "POST": "children_create"}, - "convert": {"POST": "update"}, } diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index b77c60f39..bf42b18e3 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -27,13 +27,11 @@ import rest_framework as drf from botocore.exceptions import ClientError -from celery.exceptions import CeleryError from corsheaders.middleware import ( ACCESS_CONTROL_ALLOW_METHODS, ACCESS_CONTROL_ALLOW_ORIGIN, ) from drf_spectacular.utils import extend_schema, extend_schema_view -from kombu.exceptions import KombuError from lasuite.drf.models.choices import ( PRIVILEGED_ROLES, LinkReachChoices, @@ -673,13 +671,10 @@ def convert(self, request, *args, **kwargs): converted_item_id=str(placeholder.id), user_id=str(request.user.id), ) - except (CeleryError, KombuError): + except Exception: placeholder.soft_delete() placeholder.delete() - return drf.response.Response( - {"detail": "Conversion could not be queued."}, - status=status.HTTP_503_SERVICE_UNAVAILABLE, - ) + raise serializer = self.get_serializer(placeholder) return drf.response.Response(serializer.data, status=status.HTTP_201_CREATED) diff --git a/src/backend/core/tests/items/test_api_items_convert.py b/src/backend/core/tests/items/test_api_items_convert.py index 29d8ebdb4..9bb13c78c 100644 --- a/src/backend/core/tests/items/test_api_items_convert.py +++ b/src/backend/core/tests/items/test_api_items_convert.py @@ -65,16 +65,16 @@ def test_convert_endpoint_creates_placeholder_and_queues_task(): ) -def test_convert_endpoint_returns_503_and_cleans_up_when_queueing_fails(): - """Remove the placeholder and return 503 when the broker fails.""" +def test_convert_endpoint_cleans_up_placeholder_when_queueing_fails(): + """Remove the placeholder and let the broker error bubble up.""" user, item = _build_user_and_item() - client = APIClient() + client = APIClient(raise_request_exception=False) client.force_login(user) with mock.patch("core.api.viewsets.convert_file_task.delay", side_effect=KombuError): response = client.post(f"/api/v1.0/items/{item.id}/convert/") - assert response.status_code == 503 + assert response.status_code == 500 assert not models.Item.objects.filter( upload_state=models.ItemUploadStateChoices.CONVERTING ).exists() @@ -97,8 +97,8 @@ def test_convert_endpoint_returns_403_when_user_cannot_update_item(): delay_mock.assert_not_called() -def test_convert_endpoint_returns_400_for_unsupported_extension(): - """Reject conversion with 400 for non-legacy file extensions.""" +def test_convert_endpoint_returns_403_for_unsupported_extension(): + """Reject conversion with 403 for non-legacy file extensions.""" user = factories.UserFactory() item = factories.ItemFactory( users=[(user, models.RoleChoices.EDITOR)], @@ -112,5 +112,5 @@ def test_convert_endpoint_returns_400_for_unsupported_extension(): with mock.patch("core.api.viewsets.convert_file_task.delay") as delay_mock: response = client.post(f"/api/v1.0/items/{item.id}/convert/") - assert response.status_code == 400 + assert response.status_code == 403 delay_mock.assert_not_called() From 06c7ebf6c451750ec6da99e238f181da4804b50f Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 1 Jun 2026 15:21:20 +0200 Subject: [PATCH 18/20] =?UTF-8?q?fixup!=20=E2=9C=A8(backend)=20expose=20le?= =?UTF-8?q?gacy=20conversion=20ability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/backend/core/models.py | 4 +-- src/backend/core/tests/test_models_items.py | 26 +++++++++---------- .../core/tests/test_models_items_root.py | 14 +++++----- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 480838b47..501c40e86 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1294,7 +1294,7 @@ def get_abilities(self, user): and self.upload_state == ItemUploadStateChoices.READY ) can_export = can_get and self.type == ItemTypeChoices.FOLDER - must_convert = ( + can_convert = ( can_update and self.type == ItemTypeChoices.FILE and self.upload_state == ItemUploadStateChoices.READY @@ -1325,7 +1325,7 @@ def get_abilities(self, user): "update": can_update, "upload_ended": can_update and user.is_authenticated, "wopi": can_get, - "must_convert": must_convert, + "convert": can_convert, } def send_email(self, subject, emails, context=None, language=None): diff --git a/src/backend/core/tests/test_models_items.py b/src/backend/core/tests/test_models_items.py index a0f4b87ee..cd9ba4362 100644 --- a/src/backend/core/tests/test_models_items.py +++ b/src/backend/core/tests/test_models_items.py @@ -224,8 +224,8 @@ def test_models_items_hard_delete(): (None, False), ], ) -def test_models_items_get_abilities_must_convert(extension, expected): - """Flag must_convert only for legacy extensions on READY files with update access.""" +def test_models_items_get_abilities_convert(extension, expected): + """Flag convert only for legacy extensions on READY files with update access.""" user = factories.UserFactory() filename = f"file.{extension}" if extension else None item = factories.ItemFactory( @@ -234,7 +234,7 @@ def test_models_items_get_abilities_must_convert(extension, expected): filename=filename or "file", update_upload_state=models.ItemUploadStateChoices.READY, ) - assert item.get_abilities(user)["must_convert"] is expected + assert item.get_abilities(user)["convert"] is expected @pytest.mark.parametrize( @@ -281,7 +281,7 @@ def test_models_items_get_abilities_forbidden( "update": False, "upload_ended": False, "wopi": False, - "must_convert": False, + "convert": False, } nb_queries = 1 if is_authenticated else 0 with django_assert_num_queries(nb_queries): @@ -331,7 +331,7 @@ def test_models_items_get_abilities_reader(is_authenticated, reach, django_asser "update": False, "upload_ended": False, "wopi": True, - "must_convert": False, + "convert": False, } nb_queries = 1 if is_authenticated else 0 with django_assert_num_queries(nb_queries): @@ -436,7 +436,7 @@ def test_models_items_get_abilities_editor( # noqa: PLR0913 "update": True, "upload_ended": is_authenticated, "wopi": True, - "must_convert": False, + "convert": False, } nb_queries = 1 if is_authenticated else 0 with django_assert_num_queries(nb_queries): @@ -506,7 +506,7 @@ def test_models_items_not_root_get_abilities_owner( "update": True, "upload_ended": True, "wopi": True, - "must_convert": False, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -536,7 +536,7 @@ def test_models_items_not_root_get_abilities_owner( "update": False, "upload_ended": False, "wopi": False, - "must_convert": False, + "convert": False, } @@ -596,7 +596,7 @@ def test_models_items_not_root_get_abilities_administrator( "update": True, "upload_ended": True, "wopi": True, - "must_convert": False, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -666,7 +666,7 @@ def test_models_items_not_root_get_abilities_editor_user( "update": True, "upload_ended": True, "wopi": True, - "must_convert": False, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -719,7 +719,7 @@ def test_models_items_not_root_get_abilities_reader_user(django_assert_num_queri "update": access_from_link, "upload_ended": access_from_link, "wopi": True, - "must_convert": False, + "convert": False, } with django_assert_num_queries(2): assert item.get_abilities(user) == expected_abilities @@ -777,7 +777,7 @@ def test_models_items_get_abilities_hard_delete_non_root_by_non_creator( "update": True, "upload_ended": True, "wopi": True, - "must_convert": False, + "convert": False, } with django_assert_num_queries(1): assert child.get_abilities(owner) == expected_abilities @@ -808,7 +808,7 @@ def test_models_items_get_abilities_hard_delete_non_root_by_non_creator( "update": False, "upload_ended": False, "wopi": False, - "must_convert": False, + "convert": False, } diff --git a/src/backend/core/tests/test_models_items_root.py b/src/backend/core/tests/test_models_items_root.py index 008f9e007..90a103e3d 100644 --- a/src/backend/core/tests/test_models_items_root.py +++ b/src/backend/core/tests/test_models_items_root.py @@ -71,7 +71,7 @@ def test_models_sub_item_abilities_downgraded(): "update": True, "upload_ended": True, "wopi": True, - "must_convert": False, + "convert": False, } # Downgrade the role on the root item @@ -107,7 +107,7 @@ def test_models_sub_item_abilities_downgraded(): "update": False, "upload_ended": False, "wopi": True, - "must_convert": False, + "convert": False, } @@ -161,7 +161,7 @@ def test_models_items_root_get_abilities_owner( "update": True, "upload_ended": True, "wopi": True, - "must_convert": False, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -191,7 +191,7 @@ def test_models_items_root_get_abilities_owner( "update": False, "upload_ended": False, "wopi": False, - "must_convert": False, + "convert": False, } @@ -248,7 +248,7 @@ def test_models_items_root_get_abilities_administrator( "update": True, "upload_ended": True, "wopi": True, - "must_convert": False, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -314,7 +314,7 @@ def test_models_items_root_get_abilities_editor_user( "update": True, "upload_ended": True, "wopi": True, - "must_convert": False, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities @@ -367,7 +367,7 @@ def test_models_items_root_get_abilities_reader_user( "update": access_from_link, "upload_ended": access_from_link, "wopi": True, - "must_convert": False, + "convert": False, } with django_assert_num_queries(1): assert item.get_abilities(user) == expected_abilities From 22daed7f130053c2ae0d8182dabf4f9c764f46a4 Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 1 Jun 2026 15:21:20 +0200 Subject: [PATCH 19/20] =?UTF-8?q?fixup!=20=E2=9C=A8(backend)=20build=20sho?= =?UTF-8?q?rt-lived=20WOPI=20source=20URLs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/backend/wopi/tests/test_conversion_source_url.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/backend/wopi/tests/test_conversion_source_url.py b/src/backend/wopi/tests/test_conversion_source_url.py index ab2e2b7bb..035e38817 100644 --- a/src/backend/wopi/tests/test_conversion_source_url.py +++ b/src/backend/wopi/tests/test_conversion_source_url.py @@ -26,9 +26,9 @@ def test_build_source_url_uses_configured_wopi_base_url(settings, _access_servic url = build_source_url(item, user) - assert url.startswith("https://drive.example/api/v1.0/wopi/files/") - assert f"files/{item.id}/contents/" in url - assert "access_token=tok-abc" in url + assert url == ( + f"https://drive.example/api/v1.0/wopi/files/{item.id}/contents/?access_token=tok-abc" + ) def test_build_source_url_strips_trailing_slash_on_base_url(settings, _access_service): From 338bef701fcc20c9d4b8e896a54af02c2f8bdacd Mon Sep 17 00:00:00 2001 From: Nicolas Clerc Date: Mon, 1 Jun 2026 16:28:56 +0200 Subject: [PATCH 20/20] =?UTF-8?q?fixup!=20=E2=9C=A8(backend)=20add=20place?= =?UTF-8?q?holder-backed=20conversion=20service?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/backend/wopi/conversion/services.py | 15 +++++++-------- .../wopi/tests/test_conversion_services.py | 5 +++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/backend/wopi/conversion/services.py b/src/backend/wopi/conversion/services.py index 09fc28f8a..978df5ba0 100644 --- a/src/backend/wopi/conversion/services.py +++ b/src/backend/wopi/conversion/services.py @@ -4,7 +4,7 @@ from django.conf import settings from django.core.files.storage import default_storage -from django.db import transaction +from django.db import DatabaseError, transaction from core import models from core.api.utils import detect_mimetype @@ -67,7 +67,7 @@ def _resolve_destination_parent(source_item, user): return None parent = source_item.parent() - if not parent.get_abilities(user).get("update"): + if not parent.get_abilities(user).get("children_create"): return None return parent @@ -138,12 +138,11 @@ def perform_conversion(source_item, placeholder, user): default_storage.save(placeholder.file_key, converted_file) try: - with transaction.atomic(): - placeholder.mimetype = mimetype - placeholder.size = converted_file.size - placeholder.upload_state = models.ItemUploadStateChoices.READY - placeholder.save(update_fields=["mimetype", "size", "upload_state", "updated_at"]) - except Exception: + placeholder.mimetype = mimetype + placeholder.size = converted_file.size + placeholder.upload_state = models.ItemUploadStateChoices.READY + placeholder.save(update_fields=["mimetype", "size", "upload_state", "updated_at"]) + except DatabaseError: default_storage.delete(placeholder.file_key) raise diff --git a/src/backend/wopi/tests/test_conversion_services.py b/src/backend/wopi/tests/test_conversion_services.py index c152700ae..3239ada6d 100644 --- a/src/backend/wopi/tests/test_conversion_services.py +++ b/src/backend/wopi/tests/test_conversion_services.py @@ -1,6 +1,7 @@ """Tests for the conversion service.""" from django.core.files.base import ContentFile +from django.db import DatabaseError import pytest @@ -270,13 +271,13 @@ def delete(self, key): def fail_ready_save(instance, *args, **kwargs): """Raise when the placeholder is flipped to READY, pass otherwise.""" if instance.id == placeholder.id and kwargs.get("update_fields"): - raise RuntimeError("database write failed") + raise DatabaseError("database write failed") return original_save(instance, *args, **kwargs) monkeypatch.setattr(services, "default_storage", FakeStorage()) monkeypatch.setattr(models.Item, "save", fail_ready_save) - with pytest.raises(RuntimeError, match="database write failed"): + with pytest.raises(DatabaseError, match="database write failed"): services.perform_conversion(item, placeholder, user) assert deleted_keys == saved_keys