mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-11-02 04:55:36 +00:00
Fix: Various small bugs found during typing updates (#14593)
* Fix: match Apple Mail tag colors case-insensitively * Fix: reject OAuth callbacks with an unrecognized scope * Fix: refuse to refresh OAuth tokens for non-OAuth accounts or missing refresh tokens * Fix: parse datetime strings in the format_datetime filter and reject invalid ones * Fix: reject a PAPERLESS_URL without a hostname * Fix: report a database error in the system status instead of failing with a 500 * Fix: surface the database error when refreshing a document before moving its files * Fix: report no unapplied migrations when the applied migrations cannot be read * Fix: return the parsed value directly in format_datetime for typing * Chore: annotate the paperless_dirs fixture in the refresh failure test * Allow null for the latest_migration system status type --------- Co-authored-by: shamoon <4887959+shamoon@users.noreply.github.com>
This commit is contained in:
1 parent
d98918dd97
commit
3289be7383
14 files changed
+241
-14
No files matched your search
@@ -476,13 +476,13 @@ def update_filename_and_move_files(
|
||||
return
|
||||
|
||||
with FileLock(settings.MEDIA_LOCK):
|
||||
try:
|
||||
# If this was waiting for the lock, the filename or archive_filename
|
||||
# of this document may have been updated. This happens if multiple updates
|
||||
# get queued from the UI for the same document
|
||||
# So freshen up the data before doing anything
|
||||
instance.refresh_from_db()
|
||||
# If this was waiting for the lock, the filename or archive_filename
|
||||
# of this document may have been updated. This happens if multiple updates
|
||||
# get queued from the UI for the same document
|
||||
# So freshen up the data before doing anything
|
||||
instance.refresh_from_db()
|
||||
|
||||
try:
|
||||
old_filename = instance.filename
|
||||
old_source_path = instance.source_path
|
||||
move_original = False
|
||||
|
||||
@@ -44,7 +44,10 @@ def localize_date(value: date | datetime | str, format: str, locale: str) -> str
|
||||
|
||||
def format_datetime(value: str | datetime, format: str) -> str:
|
||||
if isinstance(value, str):
|
||||
value = parse_date(value)
|
||||
parsed = parse_datetime(value) or parse_date(value)
|
||||
if parsed is None:
|
||||
raise ValueError(f"Invalid date or datetime string: {value!r}")
|
||||
return parsed.strftime(format=format)
|
||||
return value.strftime(format=format)
|
||||
|
||||
|
||||
|
||||
@@ -539,3 +539,50 @@ class TestSystemStatus(APITestCase):
|
||||
self.assertEqual(response.data["tasks"]["summary"]["success_count"], 1)
|
||||
self.assertEqual(response.data["tasks"]["summary"]["failure_count"], 1)
|
||||
self.assertEqual(response.data["tasks"]["summary"]["pending_count"], 1)
|
||||
|
||||
def test_system_status_reports_database_error(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- Reading the migration state fails
|
||||
WHEN:
|
||||
- The system status is requested
|
||||
THEN:
|
||||
- The response reports the database error rather than a server error
|
||||
"""
|
||||
self.client.force_login(self.user)
|
||||
with mock.patch(
|
||||
"documents.views.MigrationLoader",
|
||||
side_effect=Exception("database is down"),
|
||||
):
|
||||
response = self.client.get(self.ENDPOINT)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
database = response.data["database"]
|
||||
self.assertEqual(database["status"], "ERROR")
|
||||
self.assertIsNotNone(database["error"])
|
||||
self.assertEqual(
|
||||
database["migration_status"],
|
||||
{"latest_migration": None, "unapplied_migrations": []},
|
||||
)
|
||||
|
||||
def test_system_status_migration_query_error(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- The migration graph loads but reading the applied migrations fails
|
||||
WHEN:
|
||||
- The system status is requested
|
||||
THEN:
|
||||
- No migrations are reported as unapplied, since the state is unknown
|
||||
"""
|
||||
self.client.force_login(self.user)
|
||||
with mock.patch("documents.views.MigrationRecorder") as recorder:
|
||||
recorder.Migration.objects.all.side_effect = Exception("query failed")
|
||||
response = self.client.get(self.ENDPOINT)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
database = response.data["database"]
|
||||
self.assertEqual(database["status"], "ERROR")
|
||||
self.assertEqual(
|
||||
database["migration_status"],
|
||||
{"latest_migration": None, "unapplied_migrations": []},
|
||||
)
|
||||
@@ -31,6 +31,7 @@ from documents.serialisers import DocumentSerializer
|
||||
from documents.tasks import empty_trash
|
||||
from paperless_testing.assertions import FileSystemAssertsMixin
|
||||
from paperless_testing.dirs import DirectoriesMixin
|
||||
from paperless_testing.dirs import PaperlessDirs
|
||||
from paperless_testing.factories import DocumentFactory
|
||||
from paperless_testing.factories import UserFactory
|
||||
|
||||
@@ -2134,3 +2135,28 @@ class TestPathDateLocalization:
|
||||
with override_settings(FILENAME_FORMAT=filename_format):
|
||||
filename = generate_filename(document)
|
||||
assert filename == Path(expected_filename)
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_refresh_failure_surfaces_database_error(
|
||||
paperless_dirs: PaperlessDirs,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- A document with a stored filename
|
||||
WHEN:
|
||||
- Saving it triggers a filename update and refreshing it from the database fails
|
||||
THEN:
|
||||
- The database error is raised, not an unbound variable error from the rollback
|
||||
"""
|
||||
document = DocumentFactory(filename="some.pdf")
|
||||
|
||||
with (
|
||||
mock.patch.object(
|
||||
Document,
|
||||
"refresh_from_db",
|
||||
side_effect=DatabaseError("database is locked"),
|
||||
),
|
||||
pytest.raises(DatabaseError, match="database is locked"),
|
||||
):
|
||||
document.save()
|
||||
@@ -4,6 +4,7 @@ from typing import Literal
|
||||
|
||||
import pytest
|
||||
|
||||
from documents.templating.filters import format_datetime
|
||||
from documents.templating.filters import localize_date
|
||||
|
||||
|
||||
@@ -294,3 +295,23 @@ class TestDateLocalization:
|
||||
Tests `localize_date` with `date` string across different locales and formats.
|
||||
"""
|
||||
assert localize_date(value, format_style, locale_str) == expected_output
|
||||
|
||||
|
||||
class TestFormatDatetime:
|
||||
@pytest.mark.parametrize(
|
||||
("value", "expected"),
|
||||
[
|
||||
pytest.param("2026-09-21", "2026-09-21 00:00", id="date"),
|
||||
pytest.param(
|
||||
"2026-09-21T13:45:00+00:00",
|
||||
"2026-09-21 13:45",
|
||||
id="datetime",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_parses_iso_strings(self, value: str, expected: str) -> None:
|
||||
assert format_datetime(value, "%Y-%m-%d %H:%M") == expected
|
||||
|
||||
def test_rejects_unparseable_string(self) -> None:
|
||||
with pytest.raises(ValueError, match="not a date"):
|
||||
format_datetime("not a date", "%Y")
|
||||
+10
-3
@@ -5268,7 +5268,9 @@ class CustomFieldViewSet(PermissionsAwareDocumentCountMixin, ModelViewSet[Custom
|
||||
"migration_status": inline_serializer(
|
||||
name="MigrationStatus",
|
||||
fields={
|
||||
"latest_migration": serializers.CharField(),
|
||||
"latest_migration": serializers.CharField(
|
||||
allow_null=True,
|
||||
),
|
||||
"unapplied_migrations": serializers.ListSerializer(
|
||||
child=serializers.CharField(),
|
||||
),
|
||||
@@ -5343,6 +5345,8 @@ class SystemStatusView(PassUserMixin):
|
||||
db_conn = connections["default"]
|
||||
db_url = str(db_conn.settings_dict["NAME"])
|
||||
db_error = None
|
||||
all_migrations: list[str] = []
|
||||
applied_migrations: list[str] = []
|
||||
|
||||
try:
|
||||
db_conn.ensure_connection()
|
||||
@@ -5353,7 +5357,8 @@ class SystemStatusView(PassUserMixin):
|
||||
f"{m.app}.{m.name}"
|
||||
for m in MigrationRecorder.Migration.objects.all().order_by("id")
|
||||
]
|
||||
except Exception as e: # pragma: no cover
|
||||
except Exception as e:
|
||||
all_migrations = []
|
||||
applied_migrations = []
|
||||
db_status = "ERROR"
|
||||
logger.exception(
|
||||
@@ -5549,7 +5554,9 @@ class SystemStatusView(PassUserMixin):
|
||||
"status": db_status,
|
||||
"error": db_error,
|
||||
"migration_status": {
|
||||
"latest_migration": applied_migrations[-1],
|
||||
"latest_migration": (
|
||||
applied_migrations[-1] if applied_migrations else None
|
||||
),
|
||||
"unapplied_migrations": [
|
||||
m for m in all_migrations if m not in applied_migrations
|
||||
],
|
||||
|
||||
Reference in new issue
Block a user