From 73ab2890cb219f1a49d46e1650fe416fe74d9d95 Mon Sep 17 00:00:00 2001 From: ali Date: Wed, 15 Jul 2026 14:16:33 +0530 Subject: [PATCH] =?UTF-8?q?UN-3735=20[FIX]=20destination=20DB=20status=20?= =?UTF-8?q?=E2=80=94=20write=20enum=20.value,=20not=20the=20enum=20repr?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The database destination write assigned the FileProcessingStatus enum member instead of its .value, so a status column received "FileProcessingStatus.SUCCESS" / "FileProcessingStatus.ERROR" instead of "SUCCESS" / "ERROR". Any downstream query filtering on status = 'SUCCESS' silently matched nothing. Use .value at both write sites (workers + backend paths). Add regression tests that assert on str(value) — the workers enum is a (str, Enum) whose member compares == "SUCCESS" even with the bug, so only stringification (what the DB driver does) reveals it. Co-Authored-By: Claude Opus 4.8 --- .../endpoint_v2/database_utils.py | 7 +++- .../test_database_utils/test_status_value.py | 37 +++++++++++++++++++ .../shared/infrastructure/database/utils.py | 8 +++- .../tests/test_destination_status_value.py | 35 ++++++++++++++++++ 4 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 backend/workflow_manager/endpoint_v2/tests/test_database_utils/test_status_value.py create mode 100644 workers/tests/test_destination_status_value.py diff --git a/backend/workflow_manager/endpoint_v2/database_utils.py b/backend/workflow_manager/endpoint_v2/database_utils.py index a89594e186..bad823097b 100644 --- a/backend/workflow_manager/endpoint_v2/database_utils.py +++ b/backend/workflow_manager/endpoint_v2/database_utils.py @@ -160,8 +160,13 @@ def get_columns_and_values( if error and has_error_col: values[TableColumns.ERROR_MESSAGE] = error if has_status_col: + # Use `.value` so the destination column stores the plain string + # ("SUCCESS"/"ERROR"), not the enum repr ("FileProcessingStatus.SUCCESS") + # that stringifying the member produces. values[TableColumns.STATUS] = ( - FileProcessingStatus.ERROR if error else FileProcessingStatus.SUCCESS + FileProcessingStatus.ERROR.value + if error + else FileProcessingStatus.SUCCESS.value ) if column_mode == ColumnModes.WRITE_JSON_TO_A_SINGLE_COLUMN: v2_col_name = f"{single_column_name}_v2" diff --git a/backend/workflow_manager/endpoint_v2/tests/test_database_utils/test_status_value.py b/backend/workflow_manager/endpoint_v2/tests/test_database_utils/test_status_value.py new file mode 100644 index 0000000000..3cb1503267 --- /dev/null +++ b/backend/workflow_manager/endpoint_v2/tests/test_database_utils/test_status_value.py @@ -0,0 +1,37 @@ +"""Regression test for UN-3735. + +The destination `status` column must be written as the plain string +"SUCCESS"/"ERROR", never the enum repr "FileProcessingStatus.SUCCESS". + +The backend ``FileProcessingStatus`` is a plain ``Enum``, so a stringified member +renders as "FileProcessingStatus.SUCCESS"; the assertions check ``str(value)`` — +what the DB driver persists. No DB access needed, so ``SimpleTestCase``. +""" + +from django.test import SimpleTestCase + +from workflow_manager.endpoint_v2.constants import TableColumns +from workflow_manager.endpoint_v2.database_utils import DatabaseUtils + + +class DestinationStatusValueTests(SimpleTestCase): + def _status_for(self, error: str | None) -> object: + values = DatabaseUtils.get_columns_and_values( + column_mode_str="WRITE_JSON_TO_A_SINGLE_COLUMN", + data={"k": "v"}, + file_path="f", + execution_id="e", + table_info={ + "status": "STRING", + "error_message": "STRING", + "data": "STRING", + }, + error=error, + ) + return values[TableColumns.STATUS] + + def test_status_success_is_plain_string(self) -> None: + self.assertEqual(str(self._status_for(None)), "SUCCESS") + + def test_status_error_is_plain_string(self) -> None: + self.assertEqual(str(self._status_for("boom")), "ERROR") diff --git a/workers/shared/infrastructure/database/utils.py b/workers/shared/infrastructure/database/utils.py index d79ad2b4ab..79fc9a33ab 100644 --- a/workers/shared/infrastructure/database/utils.py +++ b/workers/shared/infrastructure/database/utils.py @@ -308,10 +308,14 @@ def _add_processing_columns( if error and has_error_col: values[TableColumns.ERROR_MESSAGE] = error - # Add status based on error presence + # Add status based on error presence. Use `.value` so the destination + # column stores the plain string ("SUCCESS"/"ERROR"), not the enum repr + # ("FileProcessingStatus.SUCCESS") that stringifying the member produces. if has_status_col: values[TableColumns.STATUS] = ( - FileProcessingStatus.ERROR if error else FileProcessingStatus.SUCCESS + FileProcessingStatus.ERROR.value + if error + else FileProcessingStatus.SUCCESS.value ) @staticmethod diff --git a/workers/tests/test_destination_status_value.py b/workers/tests/test_destination_status_value.py new file mode 100644 index 0000000000..41a1712d3d --- /dev/null +++ b/workers/tests/test_destination_status_value.py @@ -0,0 +1,35 @@ +"""Regression test for UN-3735. + +The destination `status` column must be written as the plain string +"SUCCESS"/"ERROR", never the enum repr "FileProcessingStatus.SUCCESS". + +Subtlety: ``FileProcessingStatus`` is a ``(str, Enum)``, so the *member* already +compares equal to its value ("SUCCESS"). The bug only shows when the value is +**stringified** on the way into the DB insert (``str(member)`` → +"FileProcessingStatus.SUCCESS"). So these tests assert on ``str(value)`` — what +the DB driver actually persists — not on ``==``. +""" + +from shared.enums.status_enums import FileProcessingStatus +from shared.infrastructure.database.utils import TableColumns, WorkerDatabaseUtils + + +def test_processing_status_success_is_plain_string() -> None: + values: dict = {} + WorkerDatabaseUtils._add_processing_columns( + values, table_info=None, metadata=None, error=None + ) + assert str(values[TableColumns.STATUS]) == "SUCCESS" + assert values[TableColumns.STATUS] == FileProcessingStatus.SUCCESS.value + # It must be a plain str, not the enum member (which stringifies to the repr). + assert not isinstance(values[TableColumns.STATUS], FileProcessingStatus) + + +def test_processing_status_error_is_plain_string() -> None: + values: dict = {} + WorkerDatabaseUtils._add_processing_columns( + values, table_info={"status": "STRING"}, metadata=None, error="boom" + ) + assert str(values[TableColumns.STATUS]) == "ERROR" + assert values[TableColumns.STATUS] == FileProcessingStatus.ERROR.value + assert not isinstance(values[TableColumns.STATUS], FileProcessingStatus)