feat: update conditionals.py to use resolve_requested_version and DocumentVersion

- Replace resolve_effective_document_by_pk stub with direct get_object_or_404
  + resolve_requested_version calls in all five conditional functions
- Switch from .modified (shim) to .added (DocumentVersion native field)
- Switch thumbnail cache key to use version.id instead of document id
- Re-add get_root_document/get_latest_version_for_root stubs to versioning.py
  (bulk_edit.py still needs them; Task 10 will remove them)
- Update TestVersionAwareFilters tests to reflect simplified filter behavior
  (no more FieldError fallback; filters query content directly)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Trenton H
2026-04-14 08:05:34 -07:00
parent 2116ea3329
commit 19d930a81a
3 changed files with 58 additions and 44 deletions
+31 -25
View File
@@ -4,6 +4,7 @@ from typing import Any
from django.conf import settings
from django.core.cache import cache
from django.shortcuts import get_object_or_404
from documents.caching import CACHE_5_MINUTES
from documents.caching import CACHE_50_MINUTES
@@ -13,7 +14,7 @@ from documents.caching import CLASSIFIER_VERSION_KEY
from documents.caching import get_thumbnail_modified_key
from documents.classifier import DocumentClassifier
from documents.models import Document
from documents.versioning import resolve_effective_document_by_pk
from documents.versioning import resolve_requested_version
def suggestions_etag(request, pk: int) -> str | None:
@@ -73,48 +74,53 @@ def metadata_etag(request, pk: int) -> str | None:
Metadata is extracted from the original file, so use its checksum as the
ETag
"""
doc = resolve_effective_document_by_pk(pk, request).document
if doc is None:
doc = get_object_or_404(Document, pk=pk)
resolution = resolve_requested_version(doc, request)
version = resolution.version
if version is None:
return None
return doc.checksum
return version.checksum
def metadata_last_modified(request, pk: int) -> datetime | None:
"""
Metadata is extracted from the original file, so use its modified. Strictly speaking, this is
not the modification of the original file, but of the database object, but might as well
error on the side of more cautious
Metadata is extracted from the original file, so use its added time.
"""
doc = resolve_effective_document_by_pk(pk, request).document
if doc is None:
doc = get_object_or_404(Document, pk=pk)
resolution = resolve_requested_version(doc, request)
version = resolution.version
if version is None:
return None
return doc.modified
return version.added
def preview_etag(request, pk: int) -> str | None:
"""
ETag for the document preview, using the original or archive checksum, depending on the request
"""
doc = resolve_effective_document_by_pk(pk, request).document
if doc is None:
doc = get_object_or_404(Document, pk=pk)
resolution = resolve_requested_version(doc, request)
version = resolution.version
if version is None:
return None
use_original = (
hasattr(request, "query_params")
and "original" in request.query_params
and request.query_params["original"] == "true"
)
return doc.checksum if use_original else doc.archive_checksum
return version.checksum if use_original else version.archive_checksum
def preview_last_modified(request, pk: int) -> datetime | None:
"""
Uses the documents modified time to set the Last-Modified header. Not strictly
speaking correct, but close enough and quick
Uses the version added time to set the Last-Modified header.
"""
doc = resolve_effective_document_by_pk(pk, request).document
if doc is None:
doc = get_object_or_404(Document, pk=pk)
resolution = resolve_requested_version(doc, request)
version = resolution.version
if version is None:
return None
return doc.modified
return version.added
def thumbnail_last_modified(request: Any, pk: int) -> datetime | None:
@@ -123,22 +129,22 @@ def thumbnail_last_modified(request: Any, pk: int) -> datetime | None:
Cache should be (slightly?) faster than filesystem
"""
try:
doc = resolve_effective_document_by_pk(pk, request).document
if doc is None:
doc = get_object_or_404(Document, pk=pk)
resolution = resolve_requested_version(doc, request)
version = resolution.version
if version is None:
return None
if not doc.thumbnail_path.exists():
if not version.thumbnail_path.exists():
return None
# Use the effective document id for cache key
doc_key = get_thumbnail_modified_key(doc.id)
doc_key = get_thumbnail_modified_key(version.id)
cache_hit = cache.get(doc_key)
if cache_hit is not None:
cache.touch(doc_key, CACHE_50_MINUTES)
return cache_hit
# No cache, get the timestamp and cache the datetime
last_modified = datetime.fromtimestamp(
doc.thumbnail_path.stat().st_mtime,
version.thumbnail_path.stat().st_mtime,
tz=UTC,
)
cache.set(doc_key, last_modified, CACHE_50_MINUTES)
@@ -7,7 +7,6 @@ from auditlog.models import LogEntry # type: ignore[import-untyped]
from django.contrib.auth.models import Permission
from django.contrib.auth.models import User
from django.contrib.contenttypes.models import ContentType
from django.core.exceptions import FieldError
from django.core.files.uploadedfile import SimpleUploadedFile
from rest_framework import status
from rest_framework.test import APITestCase
@@ -619,31 +618,21 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
class TestVersionAwareFilters:
def test_title_content_filter_falls_back_to_content(self) -> None:
def test_title_content_filter_queries_content_directly(self) -> None:
queryset = mock.Mock()
fallback_queryset = mock.Mock()
queryset.filter.side_effect = [FieldError("missing field"), fallback_queryset]
result = TitleContentFilter().filter(queryset, " latest ")
TitleContentFilter().filter(queryset, " latest ")
assert result is fallback_queryset
assert queryset.filter.call_count == 2
assert queryset.filter.call_count == 1
def test_effective_content_filter_falls_back_to_content_lookup(self) -> None:
def test_effective_content_filter_queries_content_directly(self) -> None:
queryset = mock.Mock()
fallback_queryset = mock.Mock()
queryset.filter.side_effect = [FieldError("missing field"), fallback_queryset]
result = EffectiveContentFilter(lookup_expr="icontains").filter(
queryset,
" latest ",
)
EffectiveContentFilter(lookup_expr="icontains").filter(queryset, " latest ")
assert result is fallback_queryset
first_kwargs = queryset.filter.call_args_list[0].kwargs
second_kwargs = queryset.filter.call_args_list[1].kwargs
assert first_kwargs == {"effective_content__icontains": "latest"}
assert second_kwargs == {"content__icontains": "latest"}
assert queryset.filter.call_count == 1
kwargs = queryset.filter.call_args_list[0].kwargs
assert kwargs == {"content__icontains": "latest"}
def test_effective_content_filter_returns_input_for_empty_values(self) -> None:
queryset = mock.Mock()
+19
View File
@@ -41,6 +41,25 @@ def get_version_by_pk(doc: Document, version_pk: int) -> DocumentVersion | None:
return DocumentVersion.objects.filter(pk=version_pk, document=doc).first()
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,