Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
from __future__ import annotations

import logging
from typing import Any

import sentry_sdk
Expand Down Expand Up @@ -30,13 +33,46 @@
Dashboard,
DashboardFavoriteUser,
DashboardLastVisited,
DashboardRevision,
DashboardTombstone,
)
from sentry.models.organization import Organization
from sentry.models.organizationmember import OrganizationMember

EDIT_FEATURE = "organizations:dashboards-edit"
READ_FEATURE = "organizations:dashboards-basic"
REVISIONS_FEATURE = "organizations:dashboards-revisions"

logger = logging.getLogger(__name__)


def _take_dashboard_snapshot(
organization: Organization,
dashboard: Dashboard | dict[Any, Any] | None,
user: Any,
) -> dict[str, Any] | None:
"""
Serialize the current dashboard state as a snapshot, or return None if the
revisions feature is disabled, the dashboard has no DB record to snapshot,
or serialization fails.

Must be called outside any transaction.atomic block because the serializer
makes hybrid-cloud RPC calls (user_service.serialize_many) that cannot run
inside a transaction.
"""
if not isinstance(dashboard, Dashboard):
return None
if not features.has(REVISIONS_FEATURE, organization, actor=user):
return None
try:
return serialize(dashboard, user)
except Exception:
# Snapshot failures must not block the dashboard save. Log and skip.
logger.exception(
"Failed to serialize dashboard snapshot; proceeding without creating revision",
extra={"dashboard_id": dashboard.id},
)
return None


class OrganizationDashboardBase(OrganizationEndpoint):
Expand Down Expand Up @@ -208,8 +244,12 @@ def put(
{"detail": "Cannot change the title of prebuilt Dashboards."}, status=409
)

snapshot = _take_dashboard_snapshot(organization, dashboard, request.user)

try:
with transaction.atomic(router.db_for_write(DashboardTombstone)):
if snapshot is not None and isinstance(dashboard, Dashboard):
DashboardRevision.create_for_dashboard(dashboard, request.user, snapshot)
serializer.save()
if tombstone:
DashboardTombstone.objects.get_or_create(
Expand Down
28 changes: 28 additions & 0 deletions src/sentry/models/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,9 @@ class Meta:
class DashboardRevision(DefaultFieldsModel):
__relocation_scope__ = RelocationScope.Organization

SNAPSHOT_SCHEMA_VERSION = 1
RETENTION_LIMIT = 10

created_by_id = HybridCloudForeignKey(
"sentry.User", db_index=True, null=True, on_delete="SET_NULL"
)
Expand All @@ -450,6 +453,31 @@ class Meta:
)
]

@classmethod
def create_for_dashboard(
cls, dashboard: Dashboard, user: Any, snapshot: dict[str, Any]
) -> DashboardRevision:
"""
Create a revision snapshot for the given dashboard and prune any revisions
beyond the retention limit. Must be called inside a transaction.atomic block.
"""
revision = cls.objects.create(
dashboard=dashboard,
created_by_id=user.id if user.is_authenticated else None,
title=dashboard.title,
snapshot=snapshot,
snapshot_schema_version=cls.SNAPSHOT_SCHEMA_VERSION,
)
old_revision_ids = list(
cls.objects.filter(dashboard=dashboard)
.exclude(id=revision.id)
.order_by("-date_added")
.values_list("id", flat=True)[cls.RETENTION_LIMIT - 1 :]
)
if old_revision_ids:
cls.objects.filter(id__in=old_revision_ids).delete()
return revision


@cell_silo_model
class DashboardLastVisited(DefaultFieldsModel):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
Dashboard,
DashboardFavoriteUser,
DashboardLastVisited,
DashboardRevision,
DashboardTombstone,
)
from sentry.models.dashboard_permissions import DashboardPermissions
Expand Down Expand Up @@ -4012,6 +4013,87 @@ def test_text_widget_errors_provided_queries(self) -> None:
assert "queries" in response.data["widgets"][1], response.data
assert response.data["widgets"][1]["queries"][0] == "Text widgets don't have queries"

def test_put_creates_dashboard_revision_when_feature_enabled(self) -> None:
with self.feature("organizations:dashboards-revisions"):
response = self.do_request(
"put", self.url(self.dashboard.id), data={"title": "Updated Title"}
)
assert response.status_code == 200, response.data
assert DashboardRevision.objects.filter(dashboard=self.dashboard).count() == 1

def test_put_does_not_create_revision_when_feature_disabled(self) -> None:
response = self.do_request(
"put", self.url(self.dashboard.id), data={"title": "Updated Title"}
)
assert response.status_code == 200, response.data
assert DashboardRevision.objects.filter(dashboard=self.dashboard).count() == 0

def test_put_snapshot_contains_pre_save_state(self) -> None:
with self.feature("organizations:dashboards-revisions"):
self.do_request("put", self.url(self.dashboard.id), data={"title": "New Title"})

revision = DashboardRevision.objects.get(dashboard=self.dashboard)
# Snapshot reflects the state before the update
assert revision.snapshot["title"] == self.dashboard.title
assert revision.title == self.dashboard.title
# The dashboard itself was updated
self.dashboard.refresh_from_db()
assert self.dashboard.title == "New Title"

def test_put_snapshot_includes_widgets_and_queries(self) -> None:
with self.feature("organizations:dashboards-revisions"):
self.do_request("put", self.url(self.dashboard.id), data={"title": "New Title"})

revision = DashboardRevision.objects.get(dashboard=self.dashboard)
snapshot_widgets = revision.snapshot["widgets"]

assert len(snapshot_widgets) == 4
assert snapshot_widgets[0]["id"] == str(self.widget_1.id)
assert snapshot_widgets[0]["title"] == self.widget_1.title
assert len(snapshot_widgets[0]["queries"]) == 2
assert snapshot_widgets[1]["id"] == str(self.widget_2.id)
assert snapshot_widgets[1]["title"] == self.widget_2.title
assert len(snapshot_widgets[1]["queries"]) == 1
assert snapshot_widgets[2]["id"] == str(self.widget_3.id)
assert snapshot_widgets[3]["id"] == str(self.widget_4.id)

def test_put_snapshot_captures_widget_state_before_widget_edit(self) -> None:
original_widget_title = self.widget_1.title
with self.feature("organizations:dashboards-revisions"):
self.do_request(
"put",
self.url(self.dashboard.id),
data={
"title": self.dashboard.title,
"widgets": [
{"id": str(self.widget_1.id), "title": "Updated Widget Title"},
{"id": str(self.widget_2.id)},
{"id": str(self.widget_3.id)},
{"id": str(self.widget_4.id)},
],
},
)

revision = DashboardRevision.objects.get(dashboard=self.dashboard)
snapshot_widget = revision.snapshot["widgets"][0]
# Snapshot reflects the widget state before the edit
assert snapshot_widget["id"] == str(self.widget_1.id)
assert snapshot_widget["title"] == original_widget_title
# The widget itself was updated
self.widget_1.refresh_from_db()
assert self.widget_1.title == "Updated Widget Title"

def test_put_does_not_create_revision_for_prebuilt_tombstone(self) -> None:
with self.feature("organizations:dashboards-revisions"):
response = self.do_request(
"put",
self.url("default-overview"),
data={"title": "default-overview"},
)
assert response.status_code == 200, response.data
# No revision should be created for a pre-built dashboard that hasn't been saved yet
assert DashboardRevision.objects.count() == 0


class OrganizationDashboardDetailsOnDemandTest(OrganizationDashboardDetailsTestCase):
widget_type = DashboardWidgetTypes.TRANSACTION_LIKE
Expand Down
56 changes: 55 additions & 1 deletion tests/sentry/models/test_dashboard.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from sentry.models.dashboard import Dashboard, DashboardFavoriteUser
from sentry.models.dashboard import Dashboard, DashboardFavoriteUser, DashboardRevision
from sentry.models.organization import Organization
from sentry.testutils.cases import TestCase
from sentry.users.models.user import User
Expand Down Expand Up @@ -187,3 +187,57 @@ def test_deletes_and_increments_existing_positions(self) -> None:
assert unfavorited.position is None
assert dashboard is not None
assert dashboard.position == 0


class DashboardRevisionTest(TestCase):
def setUp(self) -> None:
super().setUp()
self.dashboard = self.create_dashboard(title="My Dashboard", organization=self.organization)

def test_create_for_dashboard_stores_correct_fields(self) -> None:
snapshot = {"title": "My Dashboard", "widgets": []}
revision = DashboardRevision.create_for_dashboard(self.dashboard, self.user, snapshot)

assert revision.dashboard == self.dashboard
assert revision.title == self.dashboard.title
assert revision.snapshot == snapshot
assert revision.snapshot_schema_version == DashboardRevision.SNAPSHOT_SCHEMA_VERSION
assert revision.created_by_id == self.user.id

def test_create_for_dashboard_prunes_beyond_retention_limit(self) -> None:
for i in range(DashboardRevision.RETENTION_LIMIT):
DashboardRevision.objects.create(
dashboard=self.dashboard,
created_by_id=self.user.id,
title=f"Revision {i}",
snapshot={},
snapshot_schema_version=DashboardRevision.SNAPSHOT_SCHEMA_VERSION,
)

DashboardRevision.create_for_dashboard(self.dashboard, self.user, {})

assert (
DashboardRevision.objects.filter(dashboard=self.dashboard).count()
== DashboardRevision.RETENTION_LIMIT
)

def test_create_for_dashboard_retains_newest_revisions(self) -> None:
titles = [f"Revision {i}" for i in range(DashboardRevision.RETENTION_LIMIT)]
for title in titles:
DashboardRevision.objects.create(
dashboard=self.dashboard,
created_by_id=self.user.id,
title=title,
snapshot={},
snapshot_schema_version=DashboardRevision.SNAPSHOT_SCHEMA_VERSION,
)

new_revision = DashboardRevision.create_for_dashboard(self.dashboard, self.user, {})

surviving_ids = set(
DashboardRevision.objects.filter(dashboard=self.dashboard).values_list("id", flat=True)
)
# The oldest revision was pruned; the new one and the 9 most recent survive
oldest = DashboardRevision.objects.filter(title="Revision 0").first()
assert oldest is None
assert new_revision.id in surviving_ids
Loading