diff --git a/src/documents/models.py b/src/documents/models.py index 325bca552..ff5b23927 100644 --- a/src/documents/models.py +++ b/src/documents/models.py @@ -336,6 +336,16 @@ class DocumentVersion(DocumentBase): def source_file(self): return self.source_path.open("rb") + @property + def modified(self): + """Compatibility shim for conditionals.py which expects .modified. + + DocumentVersion tracks creation time via added; callers that need a + last-modified value for HTTP caching use this property. + Task 9 will refactor conditionals.py to stop using this path. + """ + return self.added + class Document(DocumentBase, SoftDeleteModel, ModelWithOwner): # type: ignore[django-manager-missing] MAX_STORED_FILENAME_LENGTH: Final[int] = 1024 diff --git a/src/documents/tests/test_api_document_versions.py b/src/documents/tests/test_api_document_versions.py index d95e78fe9..116b4c625 100644 --- a/src/documents/tests/test_api_document_versions.py +++ b/src/documents/tests/test_api_document_versions.py @@ -1,7 +1,6 @@ from __future__ import annotations from typing import TYPE_CHECKING -from unittest import TestCase from unittest import mock from auditlog.models import LogEntry # type: ignore[import-untyped] @@ -17,6 +16,7 @@ from documents.data_models import DocumentSource from documents.filters import EffectiveContentFilter from documents.filters import TitleContentFilter from documents.models import Document +from documents.models import DocumentVersion from documents.tests.utils import DirectoriesMixin if TYPE_CHECKING: @@ -41,79 +41,50 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): path.parent.mkdir(parents=True, exist_ok=True) path.write_bytes(content) - def _create_pdf( - self, - *, - title: str, - checksum: str, - root_document: Document | None = None, - ) -> Document: + def _create_doc(self, *, title: str, checksum: str) -> Document: doc = Document.objects.create( title=title, checksum=checksum, mime_type="application/pdf", - root_document=root_document, ) self._write_file(doc.source_path, b"pdf") self._write_file(doc.thumbnail_path, b"thumb") return doc - def test_root_endpoint_returns_root_for_version_and_root(self) -> None: - root = Document.objects.create( - title="root", - checksum="root", - mime_type="application/pdf", - ) - version = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - ) - - resp_root = self.client.get(f"/api/documents/{root.id}/root/") - self.assertEqual(resp_root.status_code, status.HTTP_200_OK) - self.assertEqual(resp_root.data["root_id"], root.id) - - resp_version = self.client.get(f"/api/documents/{version.id}/root/") - self.assertEqual(resp_version.status_code, status.HTTP_200_OK) - self.assertEqual(resp_version.data["root_id"], root.id) - - def test_root_endpoint_returns_404_for_missing_document(self) -> None: - resp = self.client.get("/api/documents/9999/root/") - - self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) - - def test_root_endpoint_returns_403_when_user_lacks_permission(self) -> None: - owner = User.objects.create_user(username="owner") - viewer = User.objects.create_user(username="viewer") - viewer.user_permissions.add( - Permission.objects.get(codename="view_document"), - ) - root = Document.objects.create( - title="root", - checksum="root", - mime_type="application/pdf", - owner=owner, - ) - self.client.force_authenticate(user=viewer) - - resp = self.client.get(f"/api/documents/{root.id}/root/") - - self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) - - def test_delete_version_disallows_deleting_root(self) -> None: + def _create_version( + self, + doc: Document, + *, + version_number: int, + checksum: str, + ) -> DocumentVersion: + v = DocumentVersion.objects.create( + document=doc, + version_number=version_number, + checksum=checksum, + mime_type="application/pdf", + filename=f"version_{doc.pk}_v{version_number}.pdf", + ) + self._write_file(v.source_path, b"pdf") + self._write_file(v.thumbnail_path, b"thumb") + return v + + def test_delete_version_disallows_deleting_last_version(self) -> None: root = Document.objects.create( title="root", checksum="root", mime_type="application/pdf", ) + version = self._create_version(root, version_number=1, checksum="v1") with mock.patch("documents.search.get_backend"): - resp = self.client.delete(f"/api/documents/{root.id}/versions/{root.id}/") + resp = self.client.delete( + f"/api/documents/{root.id}/versions/{version.pk}/", + ) self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) - self.assertTrue(Document.objects.filter(id=root.id).exists()) + self.assertIn("only remaining version", resp.content.decode()) + self.assertTrue(DocumentVersion.objects.filter(pk=version.pk).exists()) def test_delete_version_deletes_version_and_returns_current_version(self) -> None: root = Document.objects.create( @@ -122,38 +93,31 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): mime_type="application/pdf", content="root-content", ) - v1 = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - content="v1-content", - ) - v2 = Document.objects.create( - title="v2", - checksum="v2", - mime_type="application/pdf", - root_document=root, - content="v2-content", - ) + v1 = self._create_version(root, version_number=1, checksum="v1") + v1.content = "v1-content" + v1.save(update_fields=["content"]) + v2 = self._create_version(root, version_number=2, checksum="v2") + v2.content = "v2-content" + v2.save(update_fields=["content"]) with mock.patch("documents.search.get_backend"): - resp = self.client.delete(f"/api/documents/{root.id}/versions/{v2.id}/") + resp = self.client.delete( + f"/api/documents/{root.id}/versions/{v2.pk}/", + ) self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertFalse(Document.objects.filter(id=v2.id).exists()) - self.assertEqual(resp.data["current_version_id"], v1.id) + self.assertFalse(DocumentVersion.objects.filter(pk=v2.pk).exists()) + self.assertEqual(resp.data["current_version_id"], v1.pk) root.refresh_from_db() - self.assertEqual(root.content, "root-content") + self.assertEqual(root.content, "v1-content") with mock.patch("documents.search.get_backend"): - resp = self.client.delete(f"/api/documents/{root.id}/versions/{v1.id}/") + resp = self.client.delete( + f"/api/documents/{root.id}/versions/{v1.pk}/", + ) - self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertFalse(Document.objects.filter(id=v1.id).exists()) - self.assertEqual(resp.data["current_version_id"], root.id) - root.refresh_from_db() - self.assertEqual(root.content, "root-content") + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("only remaining version", resp.content.decode()) def test_delete_version_writes_audit_log_entry(self) -> None: root = Document.objects.create( @@ -161,17 +125,13 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): checksum="root", mime_type="application/pdf", ) - version = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - ) - version_id = version.id + self._create_version(root, version_number=1, checksum="v1") + v2 = self._create_version(root, version_number=2, checksum="v2") + version_pk = v2.pk with mock.patch("documents.search.get_backend"): resp = self.client.delete( - f"/api/documents/{root.id}/versions/{version_id}/", + f"/api/documents/{root.id}/versions/{version_pk}/", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) @@ -193,10 +153,10 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): self.assertEqual(entry.action, LogEntry.Action.UPDATE) self.assertEqual( entry.changes, - {"Version Deleted": ["None", version_id]}, + {"Version Deleted": ["None", version_pk]}, ) additional_data = entry.additional_data or {} - self.assertEqual(additional_data.get("version_id"), version_id) + self.assertEqual(additional_data.get("version_id"), version_pk) def test_delete_version_returns_404_when_version_not_related(self) -> None: root = Document.objects.create( @@ -209,42 +169,20 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): checksum="other", mime_type="application/pdf", ) - other_version = Document.objects.create( - title="other-v1", + other_v1 = self._create_version( + other_root, + version_number=1, checksum="other-v1", - mime_type="application/pdf", - root_document=other_root, ) + self._create_version(other_root, version_number=2, checksum="other-v2") with mock.patch("documents.search.get_backend"): resp = self.client.delete( - f"/api/documents/{root.id}/versions/{other_version.id}/", + f"/api/documents/{root.id}/versions/{other_v1.pk}/", ) self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) - def test_delete_version_accepts_version_id_as_root_parameter(self) -> None: - root = Document.objects.create( - title="root", - checksum="root", - mime_type="application/pdf", - ) - version = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - ) - - with mock.patch("documents.search.get_backend"): - resp = self.client.delete( - f"/api/documents/{version.id}/versions/{version.id}/", - ) - - self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertFalse(Document.objects.filter(id=version.id).exists()) - self.assertEqual(resp.data["current_version_id"], root.id) - def test_delete_version_returns_404_when_root_missing(self) -> None: resp = self.client.delete("/api/documents/9999/versions/123/") @@ -256,22 +194,17 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): checksum="root", mime_type="application/pdf", ) - version = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - ) + self._create_version(root, version_number=1, checksum="v1") + v2 = self._create_version(root, version_number=2, checksum="v2") with mock.patch("documents.search.get_backend") as mock_get_backend: mock_backend = mock.MagicMock() mock_get_backend.return_value = mock_backend resp = self.client.delete( - f"/api/documents/{root.id}/versions/{version.id}/", + f"/api/documents/{root.id}/versions/{v2.pk}/", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) - mock_backend.remove.assert_called_once_with(version.pk) mock_backend.add_or_update.assert_called_once() self.assertEqual(mock_backend.add_or_update.call_args[0][0].id, root.id) @@ -287,16 +220,11 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): mime_type="application/pdf", owner=owner, ) - version = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - ) + version = self._create_version(root, version_number=1, checksum="v1") self.client.force_authenticate(user=other) resp = self.client.delete( - f"/api/documents/{root.id}/versions/{version.id}/", + f"/api/documents/{root.id}/versions/{version.pk}/", ) self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) @@ -318,16 +246,14 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): checksum="root", mime_type="application/pdf", ) - version = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - version_label="old", - ) + # version_number=1 is considered the root version; use 2 for a non-root. + self._create_version(root, version_number=1, checksum="v1") + version = self._create_version(root, version_number=2, checksum="v2") + version.version_label = "old" + version.save(update_fields=["version_label"]) resp = self.client.patch( - f"/api/documents/{root.id}/versions/{version.id}/", + f"/api/documents/{root.id}/versions/{version.pk}/", {"version_label": " Label 1 "}, format="json", ) @@ -336,7 +262,7 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): version.refresh_from_db() self.assertEqual(version.version_label, "Label 1") self.assertEqual(resp.data["version_label"], "Label 1") - self.assertEqual(resp.data["id"], version.id) + self.assertEqual(resp.data["id"], version.pk) self.assertFalse(resp.data["is_root"]) def test_update_version_label_clears_on_blank(self) -> None: @@ -344,18 +270,20 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): title="root", checksum="root", mime_type="application/pdf", - version_label="Root Label", ) + version = self._create_version(root, version_number=1, checksum="v1") + version.version_label = "Root Label" + version.save(update_fields=["version_label"]) resp = self.client.patch( - f"/api/documents/{root.id}/versions/{root.id}/", + f"/api/documents/{root.id}/versions/{version.pk}/", {"version_label": " "}, format="json", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) - root.refresh_from_db() - self.assertIsNone(root.version_label) + version.refresh_from_db() + self.assertIsNone(version.version_label) self.assertIsNone(resp.data["version_label"]) self.assertTrue(resp.data["is_root"]) @@ -371,16 +299,11 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): mime_type="application/pdf", owner=owner, ) - version = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - ) + version = self._create_version(root, version_number=1, checksum="v1") self.client.force_authenticate(user=other) resp = self.client.patch( - f"/api/documents/{root.id}/versions/{version.id}/", + f"/api/documents/{root.id}/versions/{version.pk}/", {"version_label": "Blocked"}, format="json", ) @@ -398,15 +321,14 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): checksum="other", mime_type="application/pdf", ) - other_version = Document.objects.create( - title="other-v1", + other_version = self._create_version( + other_root, + version_number=1, checksum="other-v1", - mime_type="application/pdf", - root_document=other_root, ) resp = self.client.patch( - f"/api/documents/{root.id}/versions/{other_version.id}/", + f"/api/documents/{root.id}/versions/{other_version.pk}/", {"version_label": "Nope"}, format="json", ) @@ -414,7 +336,7 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) def test_download_version_param_errors(self) -> None: - root = self._create_pdf(title="root", checksum="root") + root = self._create_doc(title="root", checksum="root") resp = self.client.get( f"/api/documents/{root.id}/download/?version=not-a-number", @@ -424,41 +346,37 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): resp = self.client.get(f"/api/documents/{root.id}/download/?version=9999") self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) - other_root = self._create_pdf(title="other", checksum="other") - other_version = self._create_pdf( - title="other-v1", + other_root = self._create_doc(title="other", checksum="other") + other_version = self._create_version( + other_root, + version_number=1, checksum="other-v1", - root_document=other_root, ) resp = self.client.get( - f"/api/documents/{root.id}/download/?version={other_version.id}", + f"/api/documents/{root.id}/download/?version={other_version.pk}", ) self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) def test_download_preview_thumb_with_version_param(self) -> None: - root = self._create_pdf(title="root", checksum="root") - version = self._create_pdf( - title="v1", - checksum="v1", - root_document=root, - ) + root = self._create_doc(title="root", checksum="root") + version = self._create_version(root, version_number=1, checksum="v1") self._write_file(version.source_path, b"version") self._write_file(version.thumbnail_path, b"thumb") resp = self.client.get( - f"/api/documents/{root.id}/download/?version={version.id}", + f"/api/documents/{root.id}/download/?version={version.pk}", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(resp.content, b"version") resp = self.client.get( - f"/api/documents/{root.id}/preview/?version={version.id}", + f"/api/documents/{root.id}/preview/?version={version.pk}", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(resp.content, b"version") resp = self.client.get( - f"/api/documents/{root.id}/thumb/?version={version.id}", + f"/api/documents/{root.id}/thumb/?version={version.pk}", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(resp.content, b"thumb") @@ -469,24 +387,19 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): checksum="root", mime_type="application/pdf", ) - version = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - ) + version = self._create_version(root, version_number=1, checksum="v1") with mock.patch("documents.views.DocumentViewSet.get_metadata") as metadata: metadata.return_value = [] resp = self.client.get( - f"/api/documents/{root.id}/metadata/?version={version.id}", + f"/api/documents/{root.id}/metadata/?version={version.pk}", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertTrue(metadata.called) def test_metadata_version_param_errors(self) -> None: - root = self._create_pdf(title="root", checksum="root") + root = self._create_doc(title="root", checksum="root") resp = self.client.get( f"/api/documents/{root.id}/metadata/?version=not-a-number", @@ -496,14 +409,14 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): resp = self.client.get(f"/api/documents/{root.id}/metadata/?version=9999") self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) - other_root = self._create_pdf(title="other", checksum="other") - other_version = self._create_pdf( - title="other-v1", + other_root = self._create_doc(title="other", checksum="other") + other_version = self._create_version( + other_root, + version_number=1, checksum="other-v1", - root_document=other_root, ) resp = self.client.get( - f"/api/documents/{root.id}/metadata/?version={other_version.id}", + f"/api/documents/{root.id}/metadata/?version={other_version.pk}", ) self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) @@ -553,39 +466,6 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): self.assertEqual(overrides.version_label, "New Version") self.assertEqual(overrides.actor_id, self.user.id) - def test_update_version_with_version_pk_normalizes_to_root(self) -> None: - root = Document.objects.create( - title="root", - checksum="root", - mime_type="application/pdf", - ) - version = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - ) - upload = self._make_pdf_upload() - - async_task = mock.Mock() - async_task.id = "task-123" - - with mock.patch("documents.views.consume_file") as consume_mock: - consume_mock.delay.return_value = async_task - resp = self.client.post( - f"/api/documents/{version.id}/update_version/", - {"document": upload, "version_label": " New Version "}, - format="multipart", - ) - - self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertEqual(resp.data, "task-123") - consume_mock.delay.assert_called_once() - input_doc, overrides = consume_mock.delay.call_args[0] - self.assertEqual(input_doc.root_document_id, root.id) - self.assertEqual(overrides.version_label, "New Version") - self.assertEqual(overrides.actor_id, self.user.id) - def test_update_version_returns_500_on_consume_failure(self) -> None: root = Document.objects.create( title="root", @@ -654,34 +534,28 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): mime_type="application/pdf", content="root-content", ) - v1 = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - content="v1-content", - ) - v2 = Document.objects.create( - title="v2", - checksum="v2", - mime_type="application/pdf", - root_document=root, - content="v2-content", - ) + v1 = self._create_version(root, version_number=1, checksum="v1") + v1.content = "v1-content" + v1.save(update_fields=["content"]) + v2 = self._create_version(root, version_number=2, checksum="v2") + v2.content = "v2-content" + v2.save(update_fields=["content"]) - resp = self.client.patch( - f"/api/documents/{root.id}/", - {"content": "edited-content"}, - format="json", - ) + with mock.patch("documents.search.get_backend"): + resp = self.client.patch( + f"/api/documents/{root.id}/", + {"content": "edited-content"}, + format="json", + ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(resp.data["content"], "edited-content") - root.refresh_from_db() v1.refresh_from_db() v2.refresh_from_db() + root.refresh_from_db() + # The latest version (v2) and the Document cache are both updated. self.assertEqual(v2.content, "edited-content") - self.assertEqual(root.content, "root-content") + self.assertEqual(root.content, "edited-content") self.assertEqual(v1.content, "v1-content") def test_patch_content_updates_selected_version_content(self) -> None: @@ -691,55 +565,41 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): mime_type="application/pdf", content="root-content", ) - v1 = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - content="v1-content", - ) - v2 = Document.objects.create( - title="v2", - checksum="v2", - mime_type="application/pdf", - root_document=root, - content="v2-content", - ) + v1 = self._create_version(root, version_number=1, checksum="v1") + v1.content = "v1-content" + v1.save(update_fields=["content"]) + v2 = self._create_version(root, version_number=2, checksum="v2") + v2.content = "v2-content" + v2.save(update_fields=["content"]) - resp = self.client.patch( - f"/api/documents/{root.id}/?version={v1.id}", - {"content": "edited-v1"}, - format="json", - ) + with mock.patch("documents.search.get_backend"): + resp = self.client.patch( + f"/api/documents/{root.id}/?version={v1.pk}", + {"content": "edited-v1"}, + format="json", + ) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(resp.data["content"], "edited-v1") - root.refresh_from_db() v1.refresh_from_db() v2.refresh_from_db() + root.refresh_from_db() self.assertEqual(v1.content, "edited-v1") self.assertEqual(v2.content, "v2-content") self.assertEqual(root.content, "root-content") - def test_retrieve_returns_latest_version_content(self) -> None: + def test_retrieve_returns_document_content(self) -> None: root = Document.objects.create( title="root", checksum="root", mime_type="application/pdf", - content="root-content", - ) - Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - content="v1-content", + content="latest-content", ) resp = self.client.get(f"/api/documents/{root.id}/") self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertEqual(resp.data["content"], "v1-content") + self.assertEqual(resp.data["content"], "latest-content") def test_retrieve_with_version_param_returns_selected_version_content(self) -> None: root = Document.objects.create( @@ -748,21 +608,17 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase): mime_type="application/pdf", content="root-content", ) - v1 = Document.objects.create( - title="v1", - checksum="v1", - mime_type="application/pdf", - root_document=root, - content="v1-content", - ) + v1 = self._create_version(root, version_number=1, checksum="v1") + v1.content = "v1-content" + v1.save(update_fields=["content"]) - resp = self.client.get(f"/api/documents/{root.id}/?version={v1.id}") + resp = self.client.get(f"/api/documents/{root.id}/?version={v1.pk}") self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(resp.data["content"], "v1-content") -class TestVersionAwareFilters(TestCase): +class TestVersionAwareFilters: def test_title_content_filter_falls_back_to_content(self) -> None: queryset = mock.Mock() fallback_queryset = mock.Mock() @@ -770,8 +626,8 @@ class TestVersionAwareFilters(TestCase): result = TitleContentFilter().filter(queryset, " latest ") - self.assertIs(result, fallback_queryset) - self.assertEqual(queryset.filter.call_count, 2) + assert result is fallback_queryset + assert queryset.filter.call_count == 2 def test_effective_content_filter_falls_back_to_content_lookup(self) -> None: queryset = mock.Mock() @@ -783,16 +639,16 @@ class TestVersionAwareFilters(TestCase): " latest ", ) - self.assertIs(result, fallback_queryset) + assert result is fallback_queryset first_kwargs = queryset.filter.call_args_list[0].kwargs second_kwargs = queryset.filter.call_args_list[1].kwargs - self.assertEqual(first_kwargs, {"effective_content__icontains": "latest"}) - self.assertEqual(second_kwargs, {"content__icontains": "latest"}) + assert first_kwargs == {"effective_content__icontains": "latest"} + assert second_kwargs == {"content__icontains": "latest"} def test_effective_content_filter_returns_input_for_empty_values(self) -> None: queryset = mock.Mock() result = EffectiveContentFilter(lookup_expr="icontains").filter(queryset, " ") - self.assertIs(result, queryset) + assert result is queryset queryset.filter.assert_not_called() diff --git a/src/documents/versioning.py b/src/documents/versioning.py index efd73cb2a..ae0e22070 100644 --- a/src/documents/versioning.py +++ b/src/documents/versioning.py @@ -41,6 +41,52 @@ def get_version_by_pk(doc: Document, version_pk: int) -> DocumentVersion | None: return DocumentVersion.objects.filter(pk=version_pk, document=doc).first() +@dataclass(frozen=True, slots=True) +class EffectiveDocumentResolution: + """Compatibility shim for conditionals.py callers that access .document. + + Task 9 will refactor these callers to use VersionResolution.version directly. + """ + + document: DocumentVersion | None + + +def resolve_effective_document_by_pk( + pk: int, + request: Any, +) -> EffectiveDocumentResolution: + """Resolve the effective DocumentVersion by document pk and request params. + + This is a compatibility stub used by conditionals.py; Task 9 will refactor + the callers to use resolve_requested_version directly. + """ + try: + doc = Document.objects.get(pk=pk) + except Document.DoesNotExist: + return EffectiveDocumentResolution(document=None) + resolution = resolve_requested_version(doc, request) + return EffectiveDocumentResolution(document=resolution.version) + + +def get_root_document(doc: Document) -> Document: + """Return the root document. + + In the new model, every Document IS the root. This is a compatibility stub + used by bulk_edit.py; Task 10 will refactor the callers. + """ + return doc + + +def get_latest_version_for_root(doc: Document) -> Document: + """Return the document to use as the version source. + + In the new model, DocumentVersion holds per-version files. This stub + returns the document itself so that bulk_edit callers that have not yet + been updated to the new model do not crash. Task 10 will replace this. + """ + return doc + + def resolve_requested_version( doc: Document, request: Any, diff --git a/src/documents/views.py b/src/documents/views.py index 81bc36a79..ca2b0fc5d 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -964,7 +964,9 @@ class DocumentViewSet( data = request.data.copy() serializer_partial = partial - if content_updated and content_doc.id != root_doc.id: + # content_doc is a DocumentVersion (separate table); write goes there. + content_is_versioned = isinstance(content_doc, DocumentVersion) + if content_updated and content_is_versioned: if updated_content is None: raise ValidationError({"content": ["This field may not be null."]}) data.pop("content", None) @@ -978,11 +980,20 @@ class DocumentViewSet( serializer.is_valid(raise_exception=True) self.perform_update(serializer) - if content_updated and content_doc.id != root_doc.id: - content_doc.content = ( - str(updated_content) if updated_content is not None else "" - ) - content_doc.save(update_fields=["content", "modified"]) + if content_updated and content_is_versioned: + new_content = str(updated_content) if updated_content is not None else "" + content_doc.content = new_content + # DocumentVersion has no database ``modified`` field. + content_doc.save(update_fields=["content"]) + + # Keep Document.content in sync when the latest version is edited. + is_latest = not DocumentVersion.objects.filter( + document=root_doc, + version_number__gt=content_doc.version_number, + ).exists() + if is_latest: + root_doc.content = new_content + root_doc.save(update_fields=["content"]) refreshed_doc = self.get_queryset().get(pk=root_doc.pk) response_data = self.get_serializer(refreshed_doc).data