last modified added (#1321)

* last modified added

* PR comments changes

* Add merge migration for 0158 conflict

Resolves conflicting leaf nodes between 0158_allow_null_period_end_on_snapshot
(from main) and 0158_merge_20260415_1814 (last_modified_by feature).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* added one more test case

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: siddhant3030

ddpui/api/report_api.py

@@ -155,7 +155,7 @@ def update_snapshot(request, snapshot_id: int, payload: SnapshotUpdate):
     """Update a snapshot"""
     orguser: OrgUser = request.orguser
     try:
-        snapshot = ReportService.update_snapshot(snapshot_id, orguser.org, payload)
+        snapshot = ReportService.update_snapshot(snapshot_id, payload, orguser)
         return api_response(
             success=True,
             data=SnapshotUpdateResponse.from_model(snapshot),

ddpui/core/reports/report_service.py

@@ -448,9 +448,9 @@ def get_snapshot(snapshot_id: int, org: Org) -> ReportSnapshot:
             SnapshotNotFoundError: If snapshot doesn't exist or doesn't belong to org
         """
         try:
-            return ReportSnapshot.objects.select_related("created_by__user").get(
-                id=snapshot_id, org=org
-            )
+            return ReportSnapshot.objects.select_related(
+                "created_by__user", "last_modified_by__user"
+            ).get(id=snapshot_id, org=org)
         except ReportSnapshot.DoesNotExist:
             raise SnapshotNotFoundError(snapshot_id)
 
@@ -515,6 +515,9 @@ def get_snapshot_view_data(snapshot_id: int, org: Org) -> Dict[str, Any]:
             "created_at": snapshot.created_at,
             "updated_at": snapshot.updated_at,
             "created_by": snapshot.created_by.user.email if snapshot.created_by else None,
+            "last_modified_by": (
+                snapshot.last_modified_by.user.email if snapshot.last_modified_by else None
+            ),
             "dashboard_title": snapshot.frozen_dashboard.get("title", ""),
             "dashboard_id": snapshot.frozen_dashboard.get("dashboard_id"),
         }
@@ -526,24 +529,25 @@ def get_snapshot_view_data(snapshot_id: int, org: Org) -> Dict[str, Any]:
         }
 
     @staticmethod
-    def update_snapshot(snapshot_id: int, org: Org, data: SnapshotUpdate) -> ReportSnapshot:
+    def update_snapshot(snapshot_id: int, data: SnapshotUpdate, orguser: OrgUser) -> ReportSnapshot:
         """Update mutable fields on a snapshot.
 
         Args:
             snapshot_id: The snapshot ID to update
-            org: The organization to filter by
             data: Validated update payload
+            orguser: The user making the update
 
         Returns:
             ReportSnapshot: The updated snapshot instance
 
         Raises:
             SnapshotNotFoundError: If snapshot doesn't exist or doesn't belong to org
         """
-        snapshot = ReportService.get_snapshot(snapshot_id, org)
+        snapshot = ReportService.get_snapshot(snapshot_id, orguser.org)
         if data.summary is not None:
             snapshot.summary = data.summary
-            snapshot.save(update_fields=["summary"])
+            snapshot.last_modified_by = orguser
+            snapshot.save(update_fields=["summary", "last_modified_by", "updated_at"])
         return snapshot
 
     @staticmethod

ddpui/migrations/0157_add_last_modified_by_to_report_snapshot.py

@@ -0,0 +1,25 @@
+# Generated by Django 4.2 on 2026-04-15 13:05
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("ddpui", "0156_add_comment_snapshot_index"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="reportsnapshot",
+            name="last_modified_by",
+            field=models.ForeignKey(
+                blank=True,
+                help_text="User who last modified the summary",
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name="modified_snapshots",
+                to="ddpui.orguser",
+            ),
+        ),
+    ]

ddpui/migrations/0158_merge_20260415_1814.py

@@ -0,0 +1,12 @@
+# Generated by Django 4.2 on 2026-04-15 18:14
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("ddpui", "0157_add_last_modified_by_to_report_snapshot"),
+        ("ddpui", "0157_orgdbt_is_repo_managed_by_system"),
+    ]
+
+    operations = []

ddpui/migrations/0159_merge_20260504_0910.py

@@ -0,0 +1,12 @@
+# Generated by Django 4.2 on 2026-05-04 09:10
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("ddpui", "0158_allow_null_period_end_on_snapshot"),
+        ("ddpui", "0158_merge_20260415_1814"),
+    ]
+
+    operations = []

ddpui/models/report.py

@@ -77,6 +77,14 @@ class ReportSnapshot(models.Model):
         null=True,
         help_text="User who created this snapshot",
     )
+    last_modified_by = models.ForeignKey(
+        OrgUser,
+        on_delete=models.SET_NULL,
+        null=True,
+        blank=True,
+        related_name="modified_snapshots",
+        help_text="User who last modified the summary",
+    )
     org = models.ForeignKey(Org, on_delete=models.CASCADE)
     created_at = models.DateTimeField(auto_now_add=True)
     updated_at = models.DateTimeField(auto_now=True)

ddpui/tests/core/reports/test_report_service.py

@@ -510,27 +510,71 @@ def test_component_with_missing_chart_id(self, orguser, org):
 class TestUpdateSnapshot:
     """Tests for ReportService.update_snapshot"""
 
-    def test_update_summary(self, sample_snapshot, org):
+    def test_update_summary(self, sample_snapshot, org, orguser):
         """Updating 'summary' succeeds"""
         data = SnapshotUpdate(summary="New summary")
-        updated = ReportService.update_snapshot(sample_snapshot.id, org, data)
+        updated = ReportService.update_snapshot(sample_snapshot.id, data, orguser)
         assert updated.summary == "New summary"
 
-    def test_update_not_found_raises(self, org):
+    def test_update_not_found_raises(self, org, orguser):
         """Updating nonexistent snapshot raises SnapshotNotFoundError"""
         data = SnapshotUpdate(summary="test")
         with pytest.raises(SnapshotNotFoundError):
-            ReportService.update_snapshot(99999, org, data)
+            ReportService.update_snapshot(99999, data, orguser)
 
-    def test_update_with_none_value_skips(self, sample_snapshot, org):
+    def test_update_with_none_value_skips(self, sample_snapshot, org, orguser):
         """Updating with None value does not change the field"""
         sample_snapshot.summary = "Original"
         sample_snapshot.save(update_fields=["summary"])
 
         data = SnapshotUpdate(summary=None)
-        updated = ReportService.update_snapshot(sample_snapshot.id, org, data)
+        updated = ReportService.update_snapshot(sample_snapshot.id, data, orguser)
         assert updated.summary == "Original"
 
+    def test_update_sets_last_modified_by(self, sample_snapshot, org, orguser):
+        """Updating summary sets last_modified_by to the editing user"""
+        assert sample_snapshot.last_modified_by is None
+
+        data = SnapshotUpdate(summary="Edited summary")
+        updated = ReportService.update_snapshot(sample_snapshot.id, data, orguser)
+
+        assert updated.last_modified_by == orguser
+        assert updated.last_modified_by.user.email == orguser.user.email
+
+    def test_update_tracks_different_modifier(self, sample_snapshot, org, other_orguser):
+        """last_modified_by reflects the user who made the latest edit"""
+        data = SnapshotUpdate(summary="Edited by other user")
+        updated = ReportService.update_snapshot(sample_snapshot.id, data, other_orguser)
+
+        assert updated.last_modified_by == other_orguser
+        assert updated.last_modified_by.user.email == other_orguser.user.email
+
+    def test_update_none_summary_does_not_set_last_modified_by(self, sample_snapshot, org, orguser):
+        """When summary is None (no change), last_modified_by is not updated"""
+        assert sample_snapshot.last_modified_by is None
+
+        data = SnapshotUpdate(summary=None)
+        updated = ReportService.update_snapshot(sample_snapshot.id, data, orguser)
+
+        assert updated.last_modified_by is None
+
+    def test_creator_writes_then_other_user_edits(
+        self, sample_snapshot, org, orguser, other_orguser
+    ):
+        """Creator writes the initial summary, then another user edits it"""
+        # Creator writes the first summary
+        data = SnapshotUpdate(summary="Initial summary by creator")
+        updated = ReportService.update_snapshot(sample_snapshot.id, data, orguser)
+        assert updated.summary == "Initial summary by creator"
+        assert updated.last_modified_by == orguser
+
+        # A different user edits the summary
+        data = SnapshotUpdate(summary="Revised by another user")
+        updated = ReportService.update_snapshot(sample_snapshot.id, data, other_orguser)
+        assert updated.summary == "Revised by another user"
+        assert updated.last_modified_by == other_orguser
+        assert updated.created_by == orguser  # creator unchanged
+
 
 # ================================================================================
 # Test delete_snapshot
@@ -653,6 +697,32 @@ def test_warehouse_discovered_column_injects_chart_filters(
         col_names = [f["column"] for f in filters]
         assert "updated_at" in col_names
 
+    def test_view_data_includes_last_modified_by_null(
+        self, mock_org_warehouse_model, mock_factory, sample_snapshot, org
+    ):
+        """report_metadata includes last_modified_by as None for unedited snapshots"""
+        mock_org_warehouse_model.objects.filter.return_value.first.return_value = MagicMock()
+        mock_factory.get_warehouse_client.return_value = MagicMock()
+
+        view_data = ReportService.get_snapshot_view_data(sample_snapshot.id, org)
+        rm = view_data["report_metadata"]
+        assert "last_modified_by" in rm
+        assert rm["last_modified_by"] is None
+
+    def test_view_data_includes_last_modified_by_email(
+        self, mock_org_warehouse_model, mock_factory, sample_snapshot, org, orguser
+    ):
+        """report_metadata includes last_modified_by email after an update"""
+        mock_org_warehouse_model.objects.filter.return_value.first.return_value = MagicMock()
+        mock_factory.get_warehouse_client.return_value = MagicMock()
+
+        data = SnapshotUpdate(summary="Updated summary")
+        ReportService.update_snapshot(sample_snapshot.id, data, orguser)
+
+        view_data = ReportService.get_snapshot_view_data(sample_snapshot.id, org)
+        rm = view_data["report_metadata"]
+        assert rm["last_modified_by"] == orguser.user.email
+
     def test_not_found(self, mock_org_warehouse_model, mock_factory, org):
         """Viewing nonexistent snapshot raises SnapshotNotFoundError"""
         with pytest.raises(SnapshotNotFoundError):