From 1208492933a0818a007e30655ddbd0f8a80c8aa7 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 22 Apr 2026 18:13:10 +0530 Subject: [PATCH 1/5] [FIX] Auto-bump modified_at on QuerySet.update() and bulk_update() Django's auto_now=True only fires on Model.save(); QuerySet.update() and bulk_update() bypass save(), so BaseModel.modified_at silently stayed at the creation time for every bulk-path write. Audit trail drifted. Introduce BaseModelQuerySet that injects modified_at=timezone.now() into both paths, and expose it via BaseModelManager. Migrate all custom managers on BaseModel subclasses to compose BaseModelManager so their querysets inherit the overrides. Drop the ad-hoc modified_at=now() kwarg in FileHistoryHelper now that the queryset handles it. --- backend/adapter_processor_v2/models.py | 4 +- backend/api_v2/models.py | 4 +- backend/connector_v2/models.py | 4 +- backend/dashboard_metrics/models.py | 8 +- backend/pipeline_v2/models.py | 4 +- .../prompt_studio_core_v2/models.py | 4 +- .../prompt_studio_registry_v2/models.py | 4 +- backend/tags/models.py | 4 +- backend/tool_instance_v2/models.py | 4 +- backend/usage_v2/models.py | 4 +- backend/utils/models/base_model.py | 33 ++++++ backend/utils/models/org_aware_manager.py | 4 +- backend/utils/tests/test_base_model.py | 101 ++++++++++++++++++ .../workflow_manager/endpoint_v2/models.py | 4 +- .../workflow_manager/file_execution/models.py | 4 +- .../workflow_v2/file_history_helper.py | 1 - .../workflow_v2/models/execution.py | 4 +- .../workflow_v2/models/workflow.py | 4 +- 18 files changed, 166 insertions(+), 33 deletions(-) create mode 100644 backend/utils/tests/test_base_model.py diff --git a/backend/adapter_processor_v2/models.py b/backend/adapter_processor_v2/models.py index 13be07cdc0..a6fab0c1f9 100644 --- a/backend/adapter_processor_v2/models.py +++ b/backend/adapter_processor_v2/models.py @@ -10,7 +10,7 @@ from django.db.models import QuerySet from tenant_account_v2.models import OrganizationMember from utils.exceptions import InvalidEncryptionKey -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.models.organization_mixin import ( DefaultOrganizationManagerMixin, DefaultOrganizationMixin, @@ -29,7 +29,7 @@ logger = logging.getLogger(__name__) -class AdapterInstanceModelManager(DefaultOrganizationManagerMixin, models.Manager): +class AdapterInstanceModelManager(DefaultOrganizationManagerMixin, BaseModelManager): def get_queryset(self) -> QuerySet[Any]: return super().get_queryset() diff --git a/backend/api_v2/models.py b/backend/api_v2/models.py index 532b67d2dc..cc19902bde 100644 --- a/backend/api_v2/models.py +++ b/backend/api_v2/models.py @@ -7,7 +7,7 @@ from django.db.models.signals import post_delete from django.dispatch import receiver from pipeline_v2.models import Pipeline -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.models.organization_mixin import ( DefaultOrganizationManagerMixin, DefaultOrganizationMixin, @@ -24,7 +24,7 @@ API_ENDPOINT_MAX_LENGTH = 255 -class APIDeploymentModelManager(DefaultOrganizationManagerMixin, models.Manager): +class APIDeploymentModelManager(DefaultOrganizationManagerMixin, BaseModelManager): def for_user(self, user): """Filter API deployments that the user can access: - API deployments created by the user diff --git a/backend/connector_v2/models.py b/backend/connector_v2/models.py index c3d0e6108b..73ea38b57c 100644 --- a/backend/connector_v2/models.py +++ b/backend/connector_v2/models.py @@ -8,7 +8,7 @@ from connector_processor.constants import ConnectorKeys from django.db import models from utils.fields import EncryptedBinaryField -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.models.organization_mixin import ( DefaultOrganizationManagerMixin, DefaultOrganizationMixin, @@ -22,7 +22,7 @@ logger = logging.getLogger(__name__) -class ConnectorInstanceModelManager(DefaultOrganizationManagerMixin, models.Manager): +class ConnectorInstanceModelManager(DefaultOrganizationManagerMixin, BaseModelManager): def get_queryset(self) -> models.QuerySet: return super().get_queryset() diff --git a/backend/dashboard_metrics/models.py b/backend/dashboard_metrics/models.py index 44fec5439a..2bc4baf6ac 100644 --- a/backend/dashboard_metrics/models.py +++ b/backend/dashboard_metrics/models.py @@ -3,7 +3,7 @@ import uuid from django.db import models -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.models.organization_mixin import ( DefaultOrganizationManagerMixin, DefaultOrganizationMixin, @@ -25,19 +25,19 @@ class MetricType(models.TextChoices): HISTOGRAM = "histogram", "Histogram" -class EventMetricsHourlyManager(DefaultOrganizationManagerMixin): +class EventMetricsHourlyManager(DefaultOrganizationManagerMixin, BaseModelManager): """Manager for EventMetricsHourly with organization filtering.""" pass -class EventMetricsDailyManager(DefaultOrganizationManagerMixin): +class EventMetricsDailyManager(DefaultOrganizationManagerMixin, BaseModelManager): """Manager for EventMetricsDaily with organization filtering.""" pass -class EventMetricsMonthlyManager(DefaultOrganizationManagerMixin): +class EventMetricsMonthlyManager(DefaultOrganizationManagerMixin, BaseModelManager): """Manager for EventMetricsMonthly with organization filtering.""" pass diff --git a/backend/pipeline_v2/models.py b/backend/pipeline_v2/models.py index 00b12c8483..65fb5257a8 100644 --- a/backend/pipeline_v2/models.py +++ b/backend/pipeline_v2/models.py @@ -4,7 +4,7 @@ from django.conf import settings from django.db import models from django.db.models import Q -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.models.organization_mixin import ( DefaultOrganizationManagerMixin, DefaultOrganizationMixin, @@ -18,7 +18,7 @@ PIPELINE_NAME_LENGTH = 32 -class PipelineModelManager(DefaultOrganizationManagerMixin, models.Manager): +class PipelineModelManager(DefaultOrganizationManagerMixin, BaseModelManager): def for_user(self, user): """Filter pipelines that the user can access: - Pipelines created by the user diff --git a/backend/prompt_studio/prompt_studio_core_v2/models.py b/backend/prompt_studio/prompt_studio_core_v2/models.py index 406c157efc..1e1802b776 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/models.py +++ b/backend/prompt_studio/prompt_studio_core_v2/models.py @@ -8,7 +8,7 @@ from django.db.models import QuerySet from utils.file_storage.constants import FileStorageKeys from utils.file_storage.helpers.prompt_studio_file_helper import PromptStudioFileHelper -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.models.organization_mixin import ( DefaultOrganizationManagerMixin, DefaultOrganizationMixin, @@ -21,7 +21,7 @@ logger = logging.getLogger(__name__) -class CustomToolModelManager(DefaultOrganizationManagerMixin, models.Manager): +class CustomToolModelManager(DefaultOrganizationManagerMixin, BaseModelManager): def for_user(self, user: User) -> QuerySet[Any]: if getattr(user, "is_service_account", False): return self.all() diff --git a/backend/prompt_studio/prompt_studio_registry_v2/models.py b/backend/prompt_studio/prompt_studio_registry_v2/models.py index c4cd54cc0f..34478c8e70 100644 --- a/backend/prompt_studio/prompt_studio_registry_v2/models.py +++ b/backend/prompt_studio/prompt_studio_registry_v2/models.py @@ -5,7 +5,7 @@ from account_v2.models import User from django.db import models from django.db.models import QuerySet -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.models.organization_mixin import ( DefaultOrganizationManagerMixin, DefaultOrganizationMixin, @@ -21,7 +21,7 @@ logger = logging.getLogger(__name__) -class PromptStudioRegistryModelManager(DefaultOrganizationManagerMixin, models.Manager): +class PromptStudioRegistryModelManager(DefaultOrganizationManagerMixin, BaseModelManager): def get_queryset(self) -> QuerySet[Any]: return super().get_queryset() diff --git a/backend/tags/models.py b/backend/tags/models.py index 94c5c8cf54..7f366152b0 100644 --- a/backend/tags/models.py +++ b/backend/tags/models.py @@ -1,7 +1,7 @@ import uuid from django.db import models -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.models.organization_mixin import ( DefaultOrganizationManagerMixin, DefaultOrganizationMixin, @@ -9,7 +9,7 @@ from utils.user_context import UserContext -class TagModelManager(DefaultOrganizationManagerMixin, models.Manager): +class TagModelManager(DefaultOrganizationManagerMixin, BaseModelManager): def get_or_create_tags(self, tag_names: list[str]) -> list["Tag"]: """Retrieves or creates tags based on a list of tag names. diff --git a/backend/tool_instance_v2/models.py b/backend/tool_instance_v2/models.py index 971f1249bf..d858da2323 100644 --- a/backend/tool_instance_v2/models.py +++ b/backend/tool_instance_v2/models.py @@ -3,7 +3,7 @@ from account_v2.models import User from django.db import models from django.db.models import QuerySet -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from workflow_manager.workflow_v2.models.workflow import Workflow TOOL_ID_LENGTH = 64 @@ -11,7 +11,7 @@ TOOL_STATUS_LENGTH = 32 -class ToolInstanceManager(models.Manager): +class ToolInstanceManager(BaseModelManager): def get_instances_for_workflow(self, workflow: uuid.UUID) -> QuerySet["ToolInstance"]: return self.filter(workflow=workflow) diff --git a/backend/usage_v2/models.py b/backend/usage_v2/models.py index 8da3d751ba..57ae3d143d 100644 --- a/backend/usage_v2/models.py +++ b/backend/usage_v2/models.py @@ -1,7 +1,7 @@ import uuid from django.db import models -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.models.organization_mixin import ( DefaultOrganizationManagerMixin, DefaultOrganizationMixin, @@ -19,7 +19,7 @@ class LLMUsageReason(models.TextChoices): SUMMARIZE = "summarize", "Summarize" -class UsageModelManager(DefaultOrganizationManagerMixin, models.Manager): +class UsageModelManager(DefaultOrganizationManagerMixin, BaseModelManager): pass diff --git a/backend/utils/models/base_model.py b/backend/utils/models/base_model.py index b26f0f67d2..ae0320d086 100644 --- a/backend/utils/models/base_model.py +++ b/backend/utils/models/base_model.py @@ -1,9 +1,42 @@ from django.db import models +from django.utils import timezone + + +class BaseModelQuerySet(models.QuerySet): + """QuerySet that mirrors ``auto_now`` semantics for bulk update paths. + + ``modified_at = models.DateTimeField(auto_now=True)`` only fires on + ``Model.save()``. ``QuerySet.update()`` and ``QuerySet.bulk_update()`` + issue raw SQL and bypass ``save()``, so ``modified_at`` stays at the + original creation time — silently drifting the audit trail. This + QuerySet patches both paths so callers don't have to remember. + + Callers can still override by passing ``modified_at`` explicitly (or by + including ``modified_at`` in the ``fields`` list for ``bulk_update``). + """ + + def update(self, **kwargs): + kwargs.setdefault("modified_at", timezone.now()) + return super().update(**kwargs) + + def bulk_update(self, objs, fields, *args, **kwargs): + fields = list(fields) + if "modified_at" not in fields: + now = timezone.now() + for obj in objs: + obj.modified_at = now + fields.append("modified_at") + return super().bulk_update(objs, fields, *args, **kwargs) + + +BaseModelManager = models.Manager.from_queryset(BaseModelQuerySet) class BaseModel(models.Model): created_at = models.DateTimeField(auto_now_add=True) modified_at = models.DateTimeField(auto_now=True) + objects = BaseModelManager() + class Meta: abstract = True diff --git a/backend/utils/models/org_aware_manager.py b/backend/utils/models/org_aware_manager.py index 3ec6e70bd7..ab40303dbb 100644 --- a/backend/utils/models/org_aware_manager.py +++ b/backend/utils/models/org_aware_manager.py @@ -3,15 +3,15 @@ import logging from django.core.exceptions import ImproperlyConfigured -from django.db import models from django.db.utils import OperationalError, ProgrammingError +from utils.models.base_model import BaseModelManager from utils.models.org_path_discovery import get_org_path from utils.user_context import UserContext logger = logging.getLogger(__name__) -class OrgAwareManager(models.Manager): +class OrgAwareManager(BaseModelManager): """Manager that auto-discovers FK path to Organization and applies org filtering to all queries in request context. diff --git a/backend/utils/tests/test_base_model.py b/backend/utils/tests/test_base_model.py new file mode 100644 index 0000000000..cea113f960 --- /dev/null +++ b/backend/utils/tests/test_base_model.py @@ -0,0 +1,101 @@ +"""Tests for BaseModelQuerySet auto-injection of modified_at. + +Django's ``auto_now=True`` only fires on ``Model.save()``, so +``QuerySet.update()`` and ``QuerySet.bulk_update()`` silently skip the +``modified_at`` bump. ``BaseModelQuerySet`` patches both paths to mirror +``auto_now`` semantics. These tests exercise the override logic directly +without needing a DB round-trip. +""" + +import os +from datetime import datetime +from types import SimpleNamespace +from unittest.mock import patch + +import django + +os.environ.setdefault("DJANGO_SETTINGS_MODULE", "backend.settings.dev") +django.setup() + +from django.db import models # noqa: E402 + +from utils.models.base_model import BaseModelQuerySet # noqa: E402 + + +class _CapturingQuerySet(BaseModelQuerySet): + """Bypass ``models.QuerySet.__init__`` so we can call the overrides directly.""" + + def __new__(cls): + return object.__new__(cls) + + def __init__(self): + pass + + +class TestBaseModelQuerySetUpdate: + def test_update_injects_modified_at_when_missing(self): + qs = _CapturingQuerySet() + captured = {} + + def fake_super_update(self, **kwargs): + captured.update(kwargs) + return 1 + + with patch.object(models.QuerySet, "update", fake_super_update): + qs.update(status="done") + + assert "modified_at" in captured + assert isinstance(captured["modified_at"], datetime) + assert captured["status"] == "done" + + def test_update_preserves_explicit_modified_at(self): + qs = _CapturingQuerySet() + explicit = datetime(2020, 1, 1) + captured = {} + + def fake_super_update(self, **kwargs): + captured.update(kwargs) + return 1 + + with patch.object(models.QuerySet, "update", fake_super_update): + qs.update(status="done", modified_at=explicit) + + assert captured["modified_at"] is explicit + + +class TestBaseModelQuerySetBulkUpdate: + def test_bulk_update_appends_modified_at_and_sets_on_objs(self): + qs = _CapturingQuerySet() + objs = [SimpleNamespace(id=1), SimpleNamespace(id=2)] + captured = {} + + def fake_super_bulk(self, objs, fields, *args, **kwargs): + captured["objs"] = list(objs) + captured["fields"] = list(fields) + return len(objs) + + with patch.object(models.QuerySet, "bulk_update", fake_super_bulk): + qs.bulk_update(objs, ["status"]) + + assert "modified_at" in captured["fields"] + assert "status" in captured["fields"] + assert all(isinstance(obj.modified_at, datetime) for obj in objs) + # All objs share the same timestamp from a single timezone.now() call. + assert objs[0].modified_at == objs[1].modified_at + + def test_bulk_update_respects_explicit_modified_at_in_fields(self): + qs = _CapturingQuerySet() + preset = datetime(2020, 1, 1) + objs = [SimpleNamespace(id=1, modified_at=preset)] + captured = {} + + def fake_super_bulk(self, objs, fields, *args, **kwargs): + captured["fields"] = list(fields) + return len(objs) + + with patch.object(models.QuerySet, "bulk_update", fake_super_bulk): + qs.bulk_update(objs, ["status", "modified_at"]) + + # Caller opted in explicitly — do not overwrite their timestamp. + assert objs[0].modified_at is preset + assert captured["fields"].count("modified_at") == 1 diff --git a/backend/workflow_manager/endpoint_v2/models.py b/backend/workflow_manager/endpoint_v2/models.py index e07737ba7a..44742dfc36 100644 --- a/backend/workflow_manager/endpoint_v2/models.py +++ b/backend/workflow_manager/endpoint_v2/models.py @@ -2,12 +2,12 @@ from connector_v2.models import ConnectorInstance from django.db import models -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.user_context import UserContext from workflow_manager.workflow_v2.models.workflow import Workflow -class WorkflowEndpointModelManager(models.Manager): +class WorkflowEndpointModelManager(BaseModelManager): def get_queryset(self): # Validating organization organization = UserContext.get_organization() diff --git a/backend/workflow_manager/file_execution/models.py b/backend/workflow_manager/file_execution/models.py index d5106855e0..105b3875a9 100644 --- a/backend/workflow_manager/file_execution/models.py +++ b/backend/workflow_manager/file_execution/models.py @@ -4,7 +4,7 @@ from django.db import models from utils.common_utils import CommonUtils -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from workflow_manager.endpoint_v2.dto import FileHash from workflow_manager.workflow_v2.enums import ExecutionStatus @@ -15,7 +15,7 @@ MIME_TYPE_LENGTH = 128 -class WorkflowFileExecutionManager(models.Manager): +class WorkflowFileExecutionManager(BaseModelManager): def get_or_create_file_execution( self, workflow_execution: Any, diff --git a/backend/workflow_manager/workflow_v2/file_history_helper.py b/backend/workflow_manager/workflow_v2/file_history_helper.py index 12b0a9c2c0..687f88dfe9 100644 --- a/backend/workflow_manager/workflow_v2/file_history_helper.py +++ b/backend/workflow_manager/workflow_v2/file_history_helper.py @@ -288,7 +288,6 @@ def _increment_file_history( result=str(result), metadata=FileHistoryHelper._safe_str(metadata), error=FileHistoryHelper._safe_str(error), - modified_at=timezone.now(), ) # Refresh from DB to get updated values file_history.refresh_from_db() diff --git a/backend/workflow_manager/workflow_v2/models/execution.py b/backend/workflow_manager/workflow_v2/models/execution.py index 93623118d7..45886bd64e 100644 --- a/backend/workflow_manager/workflow_v2/models/execution.py +++ b/backend/workflow_manager/workflow_v2/models/execution.py @@ -12,7 +12,7 @@ from usage_v2.helper import UsageHelper from usage_v2.models import Usage from utils.common_utils import CommonUtils -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from workflow_manager.execution.dto import ExecutionCache from workflow_manager.execution.execution_cache_utils import ExecutionCacheUtils @@ -26,7 +26,7 @@ EXECUTION_ERROR_LENGTH = 256 -class WorkflowExecutionManager(models.Manager): +class WorkflowExecutionManager(BaseModelManager): """Custom manager for WorkflowExecution model to handle user-specific filtering.""" def for_user(self, user) -> QuerySet: diff --git a/backend/workflow_manager/workflow_v2/models/workflow.py b/backend/workflow_manager/workflow_v2/models/workflow.py index dd945fc7f9..0029f95997 100644 --- a/backend/workflow_manager/workflow_v2/models/workflow.py +++ b/backend/workflow_manager/workflow_v2/models/workflow.py @@ -4,7 +4,7 @@ from django.conf import settings from django.core.validators import MinValueValidator from django.db import models -from utils.models.base_model import BaseModel +from utils.models.base_model import BaseModel, BaseModelManager from utils.models.organization_mixin import ( DefaultOrganizationManagerMixin, DefaultOrganizationMixin, @@ -15,7 +15,7 @@ WORKFLOW_NAME_SIZE = 128 -class WorkflowModelManager(DefaultOrganizationManagerMixin, models.Manager): +class WorkflowModelManager(DefaultOrganizationManagerMixin, BaseModelManager): def for_user(self, user): """Filter workflows that the user can access: - Workflows created by the user From e2152347525242d27a6ab3eda4309b71026e12df Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Thu, 23 Apr 2026 11:26:06 +0530 Subject: [PATCH 2/5] [FIX] Materialize objs in BaseModelQuerySet.bulk_update to support generators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses PR review: if callers pass a non-rewindable iterable (generator, queryset iterator), the modified_at stamping loop would exhaust it before super().bulk_update() saw it, silently updating zero rows. list(objs) up front keeps generator callers working. Also drop the mock-based unit test — it needed django.setup() at module import which isn't viable without pytest-django, and proper DB-backed coverage is tracked separately. --- backend/utils/models/base_model.py | 3 + backend/utils/tests/test_base_model.py | 101 ------------------------- 2 files changed, 3 insertions(+), 101 deletions(-) delete mode 100644 backend/utils/tests/test_base_model.py diff --git a/backend/utils/models/base_model.py b/backend/utils/models/base_model.py index ae0320d086..2d02609b9a 100644 --- a/backend/utils/models/base_model.py +++ b/backend/utils/models/base_model.py @@ -20,6 +20,9 @@ def update(self, **kwargs): return super().update(**kwargs) def bulk_update(self, objs, fields, *args, **kwargs): + # Materialize objs before iterating so we don't exhaust a generator + # before Django's own tuple(objs) sees it. + objs = list(objs) fields = list(fields) if "modified_at" not in fields: now = timezone.now() diff --git a/backend/utils/tests/test_base_model.py b/backend/utils/tests/test_base_model.py deleted file mode 100644 index cea113f960..0000000000 --- a/backend/utils/tests/test_base_model.py +++ /dev/null @@ -1,101 +0,0 @@ -"""Tests for BaseModelQuerySet auto-injection of modified_at. - -Django's ``auto_now=True`` only fires on ``Model.save()``, so -``QuerySet.update()`` and ``QuerySet.bulk_update()`` silently skip the -``modified_at`` bump. ``BaseModelQuerySet`` patches both paths to mirror -``auto_now`` semantics. These tests exercise the override logic directly -without needing a DB round-trip. -""" - -import os -from datetime import datetime -from types import SimpleNamespace -from unittest.mock import patch - -import django - -os.environ.setdefault("DJANGO_SETTINGS_MODULE", "backend.settings.dev") -django.setup() - -from django.db import models # noqa: E402 - -from utils.models.base_model import BaseModelQuerySet # noqa: E402 - - -class _CapturingQuerySet(BaseModelQuerySet): - """Bypass ``models.QuerySet.__init__`` so we can call the overrides directly.""" - - def __new__(cls): - return object.__new__(cls) - - def __init__(self): - pass - - -class TestBaseModelQuerySetUpdate: - def test_update_injects_modified_at_when_missing(self): - qs = _CapturingQuerySet() - captured = {} - - def fake_super_update(self, **kwargs): - captured.update(kwargs) - return 1 - - with patch.object(models.QuerySet, "update", fake_super_update): - qs.update(status="done") - - assert "modified_at" in captured - assert isinstance(captured["modified_at"], datetime) - assert captured["status"] == "done" - - def test_update_preserves_explicit_modified_at(self): - qs = _CapturingQuerySet() - explicit = datetime(2020, 1, 1) - captured = {} - - def fake_super_update(self, **kwargs): - captured.update(kwargs) - return 1 - - with patch.object(models.QuerySet, "update", fake_super_update): - qs.update(status="done", modified_at=explicit) - - assert captured["modified_at"] is explicit - - -class TestBaseModelQuerySetBulkUpdate: - def test_bulk_update_appends_modified_at_and_sets_on_objs(self): - qs = _CapturingQuerySet() - objs = [SimpleNamespace(id=1), SimpleNamespace(id=2)] - captured = {} - - def fake_super_bulk(self, objs, fields, *args, **kwargs): - captured["objs"] = list(objs) - captured["fields"] = list(fields) - return len(objs) - - with patch.object(models.QuerySet, "bulk_update", fake_super_bulk): - qs.bulk_update(objs, ["status"]) - - assert "modified_at" in captured["fields"] - assert "status" in captured["fields"] - assert all(isinstance(obj.modified_at, datetime) for obj in objs) - # All objs share the same timestamp from a single timezone.now() call. - assert objs[0].modified_at == objs[1].modified_at - - def test_bulk_update_respects_explicit_modified_at_in_fields(self): - qs = _CapturingQuerySet() - preset = datetime(2020, 1, 1) - objs = [SimpleNamespace(id=1, modified_at=preset)] - captured = {} - - def fake_super_bulk(self, objs, fields, *args, **kwargs): - captured["fields"] = list(fields) - return len(objs) - - with patch.object(models.QuerySet, "bulk_update", fake_super_bulk): - qs.bulk_update(objs, ["status", "modified_at"]) - - # Caller opted in explicitly — do not overwrite their timestamp. - assert objs[0].modified_at is preset - assert captured["fields"].count("modified_at") == 1 From 256e3ba9d84cfa3bcd889e7e0220551013723bfe Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Thu, 23 Apr 2026 12:44:19 +0530 Subject: [PATCH 3/5] [FIX] Auto-inject modified_at into BaseModel.save(update_fields=...) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Django only runs auto_now for fields listed in update_fields, so every save(update_fields=["foo"]) on a BaseModel subclass silently drops the modified_at bump — same family of bug as QuerySet.update/bulk_update. Override BaseModel.save() to add modified_at to update_fields whenever the caller supplies a restricted list without it. Also drop two dead manual-assignment lines (execution.modified_at = timezone.now() before save()) that were redundant with auto_now on a full save(). --- backend/utils/models/base_model.py | 8 ++++++++ backend/workflow_manager/internal_views.py | 2 -- backend/workflow_manager/workflow_v2/views.py | 2 -- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/backend/utils/models/base_model.py b/backend/utils/models/base_model.py index 2d02609b9a..28ae7ec588 100644 --- a/backend/utils/models/base_model.py +++ b/backend/utils/models/base_model.py @@ -43,3 +43,11 @@ class BaseModel(models.Model): class Meta: abstract = True + + def save(self, *args, update_fields=None, **kwargs): + # Django only fires auto_now for fields listed in update_fields, so a + # partial save() silently drops the modified_at bump. Auto-include it + # whenever the caller restricts update_fields. + if update_fields is not None and "modified_at" not in update_fields: + update_fields = list(update_fields) + ["modified_at"] + return super().save(*args, update_fields=update_fields, **kwargs) diff --git a/backend/workflow_manager/internal_views.py b/backend/workflow_manager/internal_views.py index d11ae56179..c822e5e7b5 100644 --- a/backend/workflow_manager/internal_views.py +++ b/backend/workflow_manager/internal_views.py @@ -7,7 +7,6 @@ from django.db import transaction from django.shortcuts import get_object_or_404 -from django.utils import timezone from rest_framework import status, viewsets from rest_framework.decorators import action from rest_framework.response import Response @@ -2009,7 +2008,6 @@ def post(self, request): if update.get("execution_time") is not None: execution.execution_time = update["execution_time"] - execution.modified_at = timezone.now() execution.save() successful_updates.append( diff --git a/backend/workflow_manager/workflow_v2/views.py b/backend/workflow_manager/workflow_v2/views.py index dc428d948a..629b52bb22 100644 --- a/backend/workflow_manager/workflow_v2/views.py +++ b/backend/workflow_manager/workflow_v2/views.py @@ -6,7 +6,6 @@ from django.db import transaction from django.db.models.query import QuerySet from django.shortcuts import get_object_or_404 -from django.utils import timezone from django.views.decorators.csrf import csrf_exempt from permissions.permission import IsOwner, IsOwnerOrSharedUserOrSharedToOrg from pipeline_v2.models import Pipeline @@ -528,7 +527,6 @@ def status(self, request, id=None): if validated_data.get("execution_time") is not None: execution.execution_time = validated_data["execution_time"] - execution.modified_at = timezone.now() execution.save() logger.info( From ed5693e1d1627ab1017e1429e04ac77f1a923652 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Thu, 23 Apr 2026 13:02:07 +0530 Subject: [PATCH 4/5] [FIX] Auto-bump modified_at on upsert bulk_create and drop workarounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QuerySet.bulk_create(update_conflicts=True, update_fields=[...]) runs an UPDATE on conflict with only the listed fields — same auto_now-bypass as save(update_fields=...) and QuerySet.update(). Patch BaseModelQuerySet's bulk_create to inject modified_at into update_fields on upsert. With that in place, the explicit "modified_at" entries in dashboard_metrics upsert callers are redundant. Drop them. --- .../management/commands/backfill_metrics.py | 6 +++--- backend/dashboard_metrics/tasks.py | 6 +++--- backend/utils/models/base_model.py | 20 +++++++++++++++++++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/backend/dashboard_metrics/management/commands/backfill_metrics.py b/backend/dashboard_metrics/management/commands/backfill_metrics.py index daf0b3130e..9c4d82baca 100644 --- a/backend/dashboard_metrics/management/commands/backfill_metrics.py +++ b/backend/dashboard_metrics/management/commands/backfill_metrics.py @@ -424,7 +424,7 @@ def _bulk_upsert_hourly(self, aggregations: dict) -> int: "project", "tag", ], - update_fields=["metric_type", "metric_value", "metric_count", "modified_at"], + update_fields=["metric_type", "metric_value", "metric_count"], ) return len(objects) @@ -457,7 +457,7 @@ def _bulk_upsert_daily(self, aggregations: dict) -> int: "project", "tag", ], - update_fields=["metric_type", "metric_value", "metric_count", "modified_at"], + update_fields=["metric_type", "metric_value", "metric_count"], ) return len(objects) @@ -490,6 +490,6 @@ def _bulk_upsert_monthly(self, aggregations: dict) -> int: "project", "tag", ], - update_fields=["metric_type", "metric_value", "metric_count", "modified_at"], + update_fields=["metric_type", "metric_value", "metric_count"], ) return len(objects) diff --git a/backend/dashboard_metrics/tasks.py b/backend/dashboard_metrics/tasks.py index 3246e7e46e..181c985137 100644 --- a/backend/dashboard_metrics/tasks.py +++ b/backend/dashboard_metrics/tasks.py @@ -121,7 +121,7 @@ def _bulk_upsert_hourly(aggregations: dict) -> int: objects, update_conflicts=True, unique_fields=["organization", "timestamp", "metric_name", "project", "tag"], - update_fields=["metric_type", "metric_value", "metric_count", "modified_at"], + update_fields=["metric_type", "metric_value", "metric_count"], ) return len(objects) @@ -160,7 +160,7 @@ def _bulk_upsert_daily(aggregations: dict) -> int: objects, update_conflicts=True, unique_fields=["organization", "date", "metric_name", "project", "tag"], - update_fields=["metric_type", "metric_value", "metric_count", "modified_at"], + update_fields=["metric_type", "metric_value", "metric_count"], ) return len(objects) @@ -199,7 +199,7 @@ def _bulk_upsert_monthly(aggregations: dict) -> int: objects, update_conflicts=True, unique_fields=["organization", "month", "metric_name", "project", "tag"], - update_fields=["metric_type", "metric_value", "metric_count", "modified_at"], + update_fields=["metric_type", "metric_value", "metric_count"], ) return len(objects) diff --git a/backend/utils/models/base_model.py b/backend/utils/models/base_model.py index 28ae7ec588..82c760a454 100644 --- a/backend/utils/models/base_model.py +++ b/backend/utils/models/base_model.py @@ -31,6 +31,26 @@ def bulk_update(self, objs, fields, *args, **kwargs): fields.append("modified_at") return super().bulk_update(objs, fields, *args, **kwargs) + def bulk_create( + self, objs, *args, update_conflicts=False, update_fields=None, **kwargs + ): + # On upsert-on-conflict Django runs an UPDATE with only the listed + # fields, which skips auto_now the same way save(update_fields=...) + # does. Insert-only bulk_create already handles auto_now itself. + if ( + update_conflicts + and update_fields is not None + and "modified_at" not in update_fields + ): + update_fields = list(update_fields) + ["modified_at"] + return super().bulk_create( + objs, + *args, + update_conflicts=update_conflicts, + update_fields=update_fields, + **kwargs, + ) + BaseModelManager = models.Manager.from_queryset(BaseModelQuerySet) From d238afe79ee65028819f3bad4cec1766dcc194f0 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Fri, 24 Apr 2026 12:08:39 +0530 Subject: [PATCH 5/5] [REFACTOR] Tighten BaseModel auto-bump helpers and edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract `_with_modified_at` helper; single source of truth for the "inject modified_at into a partial field list" rule across `bulk_update`, `bulk_create` and `BaseModel.save`. - Preserve Django's documented `save(update_fields=[])` no-op (signals-only save, no column writes) instead of rewriting it to `["modified_at"]`. Apply the same guard to `bulk_create(update_conflicts=True, update_fields=[])`. - Match Django's positional `save()` signature (`force_insert`, `force_update`, `using`, `update_fields`) so callers passing flags positionally still hit the auto-bump override. - Skip the per-obj `modified_at` stamp + `objs` materialization in `bulk_update` when the caller already listed `modified_at` — lets the opt-in path stay O(1) before the `super()` delegation. - Docstring corrections: "previous save() timestamp" (not just creation time); manager-level convention note; precise `auto_now` semantics (attribute still updates in-memory, just isn't persisted without `update_fields` inclusion). --- backend/utils/models/base_model.py | 95 ++++++++++++++++++++++-------- 1 file changed, 72 insertions(+), 23 deletions(-) diff --git a/backend/utils/models/base_model.py b/backend/utils/models/base_model.py index 82c760a454..c2b288b9ef 100644 --- a/backend/utils/models/base_model.py +++ b/backend/utils/models/base_model.py @@ -1,34 +1,56 @@ from django.db import models from django.utils import timezone +_AUTO_NOW_FIELD = "modified_at" + + +def _with_modified_at(fields): + """Return a new list containing ``fields`` plus ``modified_at`` if absent. + + Centralises the "inject modified_at into a partial field list" rule so + ``bulk_update``, ``bulk_create`` and ``BaseModel.save`` apply it the same + way. + """ + fields = list(fields) + if _AUTO_NOW_FIELD not in fields: + fields.append(_AUTO_NOW_FIELD) + return fields + class BaseModelQuerySet(models.QuerySet): """QuerySet that mirrors ``auto_now`` semantics for bulk update paths. ``modified_at = models.DateTimeField(auto_now=True)`` only fires on ``Model.save()``. ``QuerySet.update()`` and ``QuerySet.bulk_update()`` - issue raw SQL and bypass ``save()``, so ``modified_at`` stays at the - original creation time — silently drifting the audit trail. This - QuerySet patches both paths so callers don't have to remember. + issue raw SQL and bypass ``save()``, leaving ``modified_at`` at whatever + value it had before the bulk path ran (creation time for never-saved + rows, the previous save() timestamp for others) — silently drifting the + audit trail. This QuerySet patches both paths so callers don't have to + remember. Callers can still override by passing ``modified_at`` explicitly (or by including ``modified_at`` in the ``fields`` list for ``bulk_update``). + + Note: this is a manager-level convention, not a model-level guarantee. + Subclasses that reassign ``objects`` to a plain ``models.Manager``, raw + SQL, and migration-time models returned by ``apps.get_model()`` all + bypass these overrides. """ def update(self, **kwargs): - kwargs.setdefault("modified_at", timezone.now()) + kwargs.setdefault(_AUTO_NOW_FIELD, timezone.now()) return super().update(**kwargs) def bulk_update(self, objs, fields, *args, **kwargs): - # Materialize objs before iterating so we don't exhaust a generator - # before Django's own tuple(objs) sees it. - objs = list(objs) - fields = list(fields) - if "modified_at" not in fields: + # Stamp modified_at on each obj only when the caller didn't list it; + # materialize objs first because we iterate the sequence twice (once + # to stamp, once via super()) and a generator would be exhausted. + if _AUTO_NOW_FIELD not in fields: + objs = list(objs) now = timezone.now() for obj in objs: obj.modified_at = now - fields.append("modified_at") + fields = _with_modified_at(fields) return super().bulk_update(objs, fields, *args, **kwargs) def bulk_create( @@ -37,12 +59,8 @@ def bulk_create( # On upsert-on-conflict Django runs an UPDATE with only the listed # fields, which skips auto_now the same way save(update_fields=...) # does. Insert-only bulk_create already handles auto_now itself. - if ( - update_conflicts - and update_fields is not None - and "modified_at" not in update_fields - ): - update_fields = list(update_fields) + ["modified_at"] + if update_conflicts and update_fields: + update_fields = _with_modified_at(update_fields) return super().bulk_create( objs, *args, @@ -56,6 +74,18 @@ def bulk_create( class BaseModel(models.Model): + """Abstract base with managed ``created_at`` / ``modified_at`` timestamps. + + Subclasses inherit ``BaseModelManager`` as the default manager, which + auto-bumps ``modified_at`` on ``QuerySet.update()``, ``bulk_update()`` + and upsert-mode ``bulk_create()``. The ``save()`` override below does + the same for partial ``save(update_fields=[...])`` calls. + + Subclasses that need a custom manager should compose ``BaseModelManager`` + (e.g. ``class FooManager(MyMixin, BaseModelManager)``) — otherwise the + auto-bump on bulk paths is silently lost. + """ + created_at = models.DateTimeField(auto_now_add=True) modified_at = models.DateTimeField(auto_now=True) @@ -64,10 +94,29 @@ class BaseModel(models.Model): class Meta: abstract = True - def save(self, *args, update_fields=None, **kwargs): - # Django only fires auto_now for fields listed in update_fields, so a - # partial save() silently drops the modified_at bump. Auto-include it - # whenever the caller restricts update_fields. - if update_fields is not None and "modified_at" not in update_fields: - update_fields = list(update_fields) + ["modified_at"] - return super().save(*args, update_fields=update_fields, **kwargs) + def save( + self, + force_insert=False, + force_update=False, + using=None, + update_fields=None, + **kwargs, + ): + # Django's save(update_fields=...) only writes the listed columns. + # auto_now still updates modified_at on the in-memory instance, but + # the new value is never persisted unless modified_at is in + # update_fields. Auto-include it so partial saves don't silently drop + # the bump. Preserve Django's documented no-op semantics for + # update_fields=[] (signals-only save, no column writes). + # + # Signature mirrors Django's positional order so callers passing + # force_insert/force_update positionally still hit this override. + if update_fields: + update_fields = _with_modified_at(update_fields) + return super().save( + force_insert=force_insert, + force_update=force_update, + using=using, + update_fields=update_fields, + **kwargs, + )