You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/10/26 12:59:54 UTC
(superset) branch master updated: fix(versioning): offer activity version actions only for existing snapshots (#45641)
This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new a813fb906c7 fix(versioning): offer activity version actions only for existing snapshots (#45641)
a813fb906c7 is described below
commit a813fb906c78c6a6334e8ed78440726895c58f87
Author: Mike Bridge <mi...@preset.io>
AuthorDate: Mon Oct 26 06:59:47 2026 -0600
fix(versioning): offer activity version actions only for existing snapshots (#45641)
---
docs/docs/using-superset/version-history.mdx | 14 +++-
.../src/features/versionHistory/ActionRow.tsx | 24 ++++--
.../DashboardVersionHistory.test.tsx | 21 ++++++
.../features/versionHistory/SaveGroupItem.test.tsx | 66 ++++++++++++++++
superset/translations/messages.pot | 3 +
superset/versioning/activity/orchestrator.py | 16 +++-
superset/versioning/activity/queries.py | 22 +++---
superset/versioning/activity/render.py | 15 ++--
superset/versioning/api_helpers.py | 6 ++
tests/unit_tests/versioning/test_activity.py | 59 ++++++++++++++-
tests/unit_tests/versioning/test_endpoint_gate.py | 52 +++++++++++++
.../unit_tests/versioning/test_runtime_capture.py | 87 +++++++++++++++++++++-
12 files changed, 353 insertions(+), 32 deletions(-)
diff --git a/docs/docs/using-superset/version-history.mdx b/docs/docs/using-superset/version-history.mdx
index 9e9073aa90c..6fbed8d3949 100644
--- a/docs/docs/using-superset/version-history.mdx
+++ b/docs/docs/using-superset/version-history.mdx
@@ -237,6 +237,14 @@ close and reopen it to see the new version.
Selecting a version loads it into the page so you can see it before deciding
anything.
+Some activity entries, such as adding a chart to a dashboard without changing
+the dashboard's saved properties, have no corresponding version snapshot.
+They remain visible in history, but cannot be previewed, restored, or opened
+as a new chart or dashboard. These rows show “No saved version to preview”
+and are not interactive. You can still expand their save group to read the
+changes. Preview and version actions are available only when a saved snapshot
+exists; the current version does not offer Restore because it is already live.
+
<img
src={useBaseUrl('/img/version-history/version-history-preview.jpg')}
alt="Illustrative chart preview from an earlier build, with Restore, Open as new chart, and Close preview actions; only the First tracked save group is highlighted"
@@ -299,8 +307,10 @@ and would overwrite the restore on the next sync.
## Opening a version as something new
Use the **… (actions)** menu on a save-group header to open that version as
-a new chart or dashboard. Individual field-change rows preview the version;
-they do not have separate action menus.
+a new chart or dashboard. Individual field-change rows with a saved snapshot
+select that version; selecting the current version exits a historical preview.
+Rows without a saved snapshot cannot be selected. Field-change rows do not
+have separate action menus.
**Open as new** creates a brand-new chart or dashboard from the selected
version and leaves the original untouched. This is the safer option when you
diff --git a/superset-frontend/src/features/versionHistory/ActionRow.tsx b/superset-frontend/src/features/versionHistory/ActionRow.tsx
index 7aeb78141c1..178c4d2c4a8 100644
--- a/superset-frontend/src/features/versionHistory/ActionRow.tsx
+++ b/superset-frontend/src/features/versionHistory/ActionRow.tsx
@@ -17,6 +17,7 @@
* under the License.
*/
import { KeyboardEvent } from 'react';
+import { t } from '@apache-superset/core/translation';
import { styled } from '@apache-superset/core/theme';
import type { ActivityRecord } from './types';
import {
@@ -25,15 +26,19 @@ import {
formatVersionDateTimeShort,
} from './display';
-const Row = styled.div`
- ${({ theme }) => `
+const Row = styled.div<{ interactive: boolean }>`
+ ${({ theme, interactive }) => `
display: flex;
align-items: stretch;
gap: ${theme.sizeUnit * 4}px;
- cursor: pointer;
+ cursor: ${interactive ? 'pointer' : 'default'};
border-radius: ${theme.borderRadius}px;
- &:hover {
+ ${
+ interactive
+ ? `&:hover {
background-color: ${theme.colorBgTextHover};
+ }`
+ : ''
}
`}
`;
@@ -113,6 +118,7 @@ export default function ActionRow({
onPreview,
}: ActionRowProps) {
const label = describeRecord(record);
+ const interactive = record.version_uuid != null;
const meta = `${formatAuthor(record.changed_by)} · ${formatVersionDateTimeShort(
record.issued_at,
)}`;
@@ -130,10 +136,11 @@ export default function ActionRow({
return (
<Row
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- timeline layout uses a focusable row with explicit keyboard activation
- role="button"
- tabIndex={0}
- onClick={onPreview}
- onKeyDown={handleKeyDown}
+ role={interactive ? 'button' : undefined}
+ tabIndex={interactive ? 0 : undefined}
+ onClick={interactive ? onPreview : undefined}
+ onKeyDown={interactive ? handleKeyDown : undefined}
+ interactive={interactive}
data-test="version-history-action-row"
>
<Rail>
@@ -143,6 +150,7 @@ export default function ActionRow({
<Content>
<Title title={label}>{label}</Title>
<Meta>{meta}</Meta>
+ {!interactive && <Meta>{t('No saved version to preview')}</Meta>}
</Content>
</Row>
);
diff --git a/superset-frontend/src/features/versionHistory/DashboardVersionHistory.test.tsx b/superset-frontend/src/features/versionHistory/DashboardVersionHistory.test.tsx
index a8717394226..31457793f66 100644
--- a/superset-frontend/src/features/versionHistory/DashboardVersionHistory.test.tsx
+++ b/superset-frontend/src/features/versionHistory/DashboardVersionHistory.test.tsx
@@ -214,6 +214,27 @@ test('a restore that also moves the save signal refreshes exactly once', () => {
expect(refresh).toHaveBeenCalledTimes(1);
});
+test('activity without a snapshot cannot start a dashboard preview', () => {
+ const store = makeStore();
+ renderAdapter(store);
+ const { onPreview } = mockPanelProps.mock.lastCall[0];
+ act(() => {
+ onPreview({
+ type: 'group',
+ transactionId: 10,
+ versionUuid: null,
+ issuedAt: '2025-12-05T17:18:00',
+ changedBy: null,
+ actionKind: null,
+ records: [],
+ });
+ });
+ expect(
+ store.actions.some(action => action.type === 'SET_VERSION_PREVIEW'),
+ ).toBe(false);
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+});
+
test('preview is blocked while the dashboard has unsaved edit-mode changes', () => {
const store = makeTestStore({
versionHistory: versionHistoryState(),
diff --git a/superset-frontend/src/features/versionHistory/SaveGroupItem.test.tsx b/superset-frontend/src/features/versionHistory/SaveGroupItem.test.tsx
index c70ed8d85c1..1116e2922a5 100644
--- a/superset-frontend/src/features/versionHistory/SaveGroupItem.test.tsx
+++ b/superset-frontend/src/features/versionHistory/SaveGroupItem.test.tsx
@@ -25,6 +25,54 @@ import {
} from 'spec/helpers/testing-library';
import type { ActivityRecord, SaveGroup, VersionedEntityType } from './types';
import SaveGroupItem, { SaveGroupItemProps } from './SaveGroupItem';
+import ActionRow from './ActionRow';
+
+test.each(['chart', 'field'])(
+ '%s activity without a snapshot is inert and explains why',
+ async kind => {
+ const record: ActivityRecord = {
+ version_uuid: null,
+ entity_kind: 'dashboard',
+ entity_uuid: 'entity-uuid',
+ entity_name: 'Dashboard',
+ entity_deleted: false,
+ entity_deletion_state: null,
+ source: 'self',
+ transaction_id: 7,
+ action_kind: null,
+ issued_at: '2026-08-31T17:46:00',
+ changed_by: null,
+ kind,
+ operation: 'add',
+ path: [],
+ from_value: null,
+ to_value: null,
+ summary: '',
+ impact: null,
+ };
+ const onPreview = jest.fn();
+ render(
+ <ActionRow
+ record={record}
+ isHighlighted={false}
+ isLast
+ onPreview={onPreview}
+ />,
+ );
+ const row = screen.getByTestId('version-history-action-row');
+ expect(row).not.toHaveAttribute('role');
+ expect(row).not.toHaveAttribute('tabindex');
+ expect(row).toHaveStyle({ cursor: 'default' });
+ expect(row).not.toHaveStyleRule('background-color', expect.any(String), {
+ target: ':hover',
+ });
+ expect(screen.getByText('No saved version to preview')).toBeVisible();
+ await userEvent.click(row);
+ fireEvent.keyDown(row, { key: 'Enter' });
+ fireEvent.keyDown(row, { key: ' ' });
+ expect(onPreview).not.toHaveBeenCalled();
+ },
+);
const makeGroup = (overrides: Partial<SaveGroup> = {}): SaveGroup => ({
type: 'group',
@@ -58,6 +106,24 @@ const groupBackground = (root: HTMLElement) =>
getComputedStyle(within(root).getByTestId('version-history-save-group'))
.backgroundColor;
+test.each<VersionedEntityType>(['chart', 'dashboard'])(
+ '%s saves without snapshots omit the creation preview and version menu',
+ entityType => {
+ renderItem({
+ entityType,
+ group: makeGroup({ versionUuid: null, creationKind: 'created' }),
+ });
+ expect(screen.getByTestId('version-history-save-group')).toBeVisible();
+ expect(
+ screen.queryByRole('button', { name: /preview/i }),
+ ).not.toBeInTheDocument();
+ expect(
+ screen.queryByRole('button', { name: 'More actions' }),
+ ).not.toBeInTheDocument();
+ expect(screen.queryByText('Restore this version')).not.toBeInTheDocument();
+ },
+);
+
test('a highlighted group carries the active treatment; a resting one does not', () => {
// Which single group is highlighted is the panel's decision (see the
// exclusivity test in VersionHistoryPanel.test.tsx); this component's
diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot
index 4639bb8568b..ba23d91efe7 100644
--- a/superset/translations/messages.pot
+++ b/superset/translations/messages.pot
@@ -9504,6 +9504,9 @@ msgstr ""
msgid "No saved metrics found"
msgstr ""
+msgid "No saved version to preview"
+msgstr ""
+
msgid "No selection modes are available for this control."
msgstr ""
diff --git a/superset/versioning/activity/orchestrator.py b/superset/versioning/activity/orchestrator.py
index fb6a00b03c7..d2acf94f499 100644
--- a/superset/versioning/activity/orchestrator.py
+++ b/superset/versioning/activity/orchestrator.py
@@ -43,6 +43,7 @@ request-shape metrics, on the same prefix the cross-coupling test pins.
from __future__ import annotations
import contextlib
+import logging
from collections.abc import Iterator
from datetime import datetime, timezone
from typing import Any
@@ -71,6 +72,7 @@ from superset.versioning.api_helpers import (
_DEFAULT_PAGE_SIZE: int = 25
_MAX_PAGE_SIZE: int = 200
_VALID_INCLUDE_VALUES: frozenset[str] = frozenset({"self", "related", "all"})
+logger: logging.Logger = logging.getLogger(__name__)
# Upper bound on the ``q`` search string. The search is a substring scan over
@@ -362,7 +364,19 @@ def get_activity(
path_id=path_id,
)
- return records[offset : offset + bounded_size], total, truncated
+ page_records: list[dict[str, Any]] = records[offset : offset + bounded_size]
+ missing_snapshots: int = sum(
+ record.get("version_uuid") is None for record in page_records
+ )
+ if missing_snapshots:
+ logger.debug(
+ "Activity rows without version snapshots: "
+ "entity_type=%s entity_id=%s count=%d",
+ path_kind,
+ path_id,
+ missing_snapshots,
+ )
+ return page_records, total, truncated
def activity_endpoint(
diff --git a/superset/versioning/activity/queries.py b/superset/versioning/activity/queries.py
index 0c0c59487e9..0b8f998307e 100644
--- a/superset/versioning/activity/queries.py
+++ b/superset/versioning/activity/queries.py
@@ -642,10 +642,10 @@ def _resolve_names_for_kind(
return resolved
-def _resolve_uuids_for_kind(
+def _resolve_snapshots_for_kind(
api_kind: str, pairs: set[tuple[int, int]]
-) -> dict[tuple[int, int], UUID]:
- """Resolve the entity UUID valid at each target transaction."""
+) -> dict[tuple[int, int], tuple[UUID, int]]:
+ """Resolve the entity UUID and snapshot transaction valid at each target."""
# pylint: disable=import-outside-toplevel
from sqlalchemy_continuum import version_class
@@ -681,7 +681,7 @@ def _resolve_uuids_for_kind(
for row in rows:
per_entity.setdefault(row[0], []).append((row[1], row[2], row[3]))
- resolved: dict[tuple[int, int], UUID] = {}
+ resolved: dict[tuple[int, int], tuple[UUID, int]] = {}
for entity_id, target_tx in pairs:
for start_tx, end_tx, entity_uuid in per_entity.get(entity_id, []):
if (
@@ -689,15 +689,15 @@ def _resolve_uuids_for_kind(
and start_tx <= target_tx
and (end_tx is None or end_tx > target_tx)
):
- resolved[(entity_id, target_tx)] = entity_uuid
+ resolved[(entity_id, target_tx)] = (entity_uuid, start_tx)
break
return resolved
-def resolve_historical_entity_uuids(
+def resolve_historical_entity_snapshots(
records: list[dict[str, Any]],
-) -> dict[tuple[str, int, int], UUID]:
- """Return UUIDs keyed by API kind, entity id, and record transaction."""
+) -> dict[tuple[str, int, int], tuple[UUID, int]]:
+ """Return (UUID, snapshot tx) keyed by API kind, id, and record tx."""
needed_by_kind: dict[str, set[tuple[int, int]]] = {}
for record in records:
api_kind = TABLE_KIND_TO_API.get(record["entity_kind"])
@@ -707,12 +707,12 @@ def resolve_historical_entity_uuids(
(record["entity_id"], record["transaction_id"])
)
- resolved: dict[tuple[str, int, int], UUID] = {}
+ resolved: dict[tuple[str, int, int], tuple[UUID, int]] = {}
for api_kind, pairs in needed_by_kind.items():
- for (entity_id, target_tx), entity_uuid in _resolve_uuids_for_kind(
+ for (entity_id, target_tx), snapshot in _resolve_snapshots_for_kind(
api_kind, pairs
).items():
- resolved[(api_kind, entity_id, target_tx)] = entity_uuid
+ resolved[(api_kind, entity_id, target_tx)] = snapshot
return resolved
diff --git a/superset/versioning/activity/render.py b/superset/versioning/activity/render.py
index c19fbecffbc..9d357eeb817 100644
--- a/superset/versioning/activity/render.py
+++ b/superset/versioning/activity/render.py
@@ -60,7 +60,7 @@ from superset.versioning.activity.kinds import (
)
from superset.versioning.activity.queries import (
check_entity_tombstones,
- resolve_historical_entity_uuids,
+ resolve_historical_entity_snapshots,
)
from superset.versioning.activity.visibility import resolve_editorship
from superset.versioning.queries import derive_version_uuid
@@ -111,7 +111,9 @@ def apply_record_decoration(
}
tombstones = check_entity_tombstones(distinct)
live_uuids = _lookup_entity_uuids(distinct, tombstones)
- historical_uuids = resolve_historical_entity_uuids(records)
+ historical_snapshots: dict[tuple[str, int, int], tuple[UUID, int]] = (
+ resolve_historical_entity_snapshots(records)
+ )
can_edit: dict[tuple[str, int], bool] = resolve_editorship(distinct)
# Pre-compute impact payloads (affected-chart ids + names) for the
# whole page in one batch query instead of one query per related
@@ -125,8 +127,10 @@ def apply_record_decoration(
(api_kind, entity_id), {"deleted": True, "deletion_state": None}
)
live_uuid = live_uuids.get((api_kind, entity_id))
- historical_uuid = historical_uuids.get(
- (api_kind, entity_id, record["transaction_id"])
+ historical_uuid: UUID | None
+ snapshot_tx: int | None
+ historical_uuid, snapshot_tx = historical_snapshots.get(
+ (api_kind, entity_id, record["transaction_id"]), (None, None)
)
reused_identity = (
live_uuid is not None
@@ -150,7 +154,8 @@ def apply_record_decoration(
record["source"] = "self" if is_self else "related"
record["version_uuid"] = (
str(derive_version_uuid(entity_uuid, record["transaction_id"]))
- if entity_uuid
+ # Membership-only activity can exist without a parent snapshot.
+ if entity_uuid and snapshot_tx == record["transaction_id"]
else None
)
record["changed_by"] = _changed_by_dict(record)
diff --git a/superset/versioning/api_helpers.py b/superset/versioning/api_helpers.py
index f50529a3489..c8d9dfa0611 100644
--- a/superset/versioning/api_helpers.py
+++ b/superset/versioning/api_helpers.py
@@ -462,6 +462,12 @@ def get_version_endpoint(
model_cls, entity_uuid, version_uuid, entity=entity
)
if snapshot is None:
+ logger.warning(
+ "Version snapshot not found: entity_type=%s entity_id=%s version_uuid=%s",
+ model_cls.__name__,
+ entity.id,
+ version_uuid,
+ )
return api.response_404()
# Normalize the version-level block through the schema; the entity
# scalar fields stay as the DAO shaped them (their keys are
diff --git a/tests/unit_tests/versioning/test_activity.py b/tests/unit_tests/versioning/test_activity.py
index 744fe27b595..75e6d156b85 100644
--- a/tests/unit_tests/versioning/test_activity.py
+++ b/tests/unit_tests/versioning/test_activity.py
@@ -30,6 +30,7 @@ by the integration suite in
from __future__ import annotations
+import logging
from datetime import datetime, timedelta
from types import SimpleNamespace
from typing import Any
@@ -287,6 +288,56 @@ def test_creation_row_respects_stream_filters(
# ---- intersect_windows ---------------------------------------------------
+@pytest.mark.parametrize("page, expected_count", [(0, 2), (1, 0)])
+def test_activity_logs_missing_snapshots_once_per_response(
+ caplog: pytest.LogCaptureFixture, page: int, expected_count: int
+) -> None:
+ """Only returned visible rows contribute; no names or values enter logs."""
+ entity: SimpleNamespace = SimpleNamespace(id=17, uuid=uuid4())
+ visible: list[dict[str, Any]] = [
+ {"version_uuid": None, "entity_name": "private-chart"},
+ {"version_uuid": None, "to_value": "private-value"},
+ {"version_uuid": "saved-version"},
+ ]
+ with (
+ patch.object(orchestrator, "first_tracked_tx", return_value=1),
+ patch.object(orchestrator, "resolve_scope", return_value=[("Model", 17, [])]),
+ patch.object(
+ orchestrator, "fetch_change_records", return_value=(visible, False)
+ ),
+ patch.object(
+ orchestrator, "filter_records_by_visibility", return_value=visible
+ ),
+ patch.object(orchestrator, "apply_entity_name_denormalization"),
+ patch.object(orchestrator, "mark_first_tracked_saves"),
+ patch.object(orchestrator, "apply_record_decoration"),
+ patch.object(orchestrator, "_emit_request_shape_attributes"),
+ caplog.at_level(logging.DEBUG, logger=orchestrator.__name__),
+ ):
+ orchestrator.get_activity(
+ Model,
+ entity.uuid,
+ resolved_entity=entity,
+ include="related",
+ page=page,
+ page_size=2,
+ )
+ messages: list[str] = [
+ record.getMessage()
+ for record in caplog.records
+ if record.name == orchestrator.__name__
+ ]
+ assert messages == (
+ [
+ "Activity rows without version snapshots: "
+ "entity_type=Model entity_id=17 count=2"
+ ]
+ if expected_count
+ else []
+ )
+ assert "private" not in caplog.text
+
+
@pytest.fixture
def decoration_lookups(mocker: MockerFixture) -> None:
"""Isolate access-redaction tests from unrelated decoration lookups."""
@@ -298,7 +349,7 @@ def decoration_lookups(mocker: MockerFixture) -> None:
"superset.versioning.activity.render._lookup_entity_uuids", return_value={}
)
mocker.patch(
- "superset.versioning.activity.render.resolve_historical_entity_uuids",
+ "superset.versioning.activity.render.resolve_historical_entity_snapshots",
return_value={},
)
mocker.patch(
@@ -633,8 +684,8 @@ def test_decoration_redacts_record_from_reused_entity_id() -> None:
return_value={("Slice", 7): live_uuid},
),
patch(
- "superset.versioning.activity.render.resolve_historical_entity_uuids",
- return_value={("Slice", 7, 20): historical_uuid},
+ "superset.versioning.activity.render.resolve_historical_entity_snapshots",
+ return_value={("Slice", 7, 20): (historical_uuid, 20)},
),
patch(
"superset.versioning.activity.render.batch_chart_impacts", return_value={}
@@ -1012,7 +1063,7 @@ def test_related_record_impact_payload_carries_affected_charts() -> None:
return_value={},
),
patch(
- "superset.versioning.activity.render.resolve_historical_entity_uuids",
+ "superset.versioning.activity.render.resolve_historical_entity_snapshots",
return_value={},
),
patch(
diff --git a/tests/unit_tests/versioning/test_endpoint_gate.py b/tests/unit_tests/versioning/test_endpoint_gate.py
index 1a34ed8ea37..0aa497fb976 100644
--- a/tests/unit_tests/versioning/test_endpoint_gate.py
+++ b/tests/unit_tests/versioning/test_endpoint_gate.py
@@ -26,14 +26,20 @@ environment-independent half of the control; the integration role matrix
(versions_api_tests / activity_view_tests) exercises it over HTTP.
"""
+import logging
from types import SimpleNamespace
from typing import Any
from unittest.mock import MagicMock
+from uuid import UUID
import pytest
+from flask import Response
+from flask_appbuilder import Model
from pytest_mock import MockerFixture
+from superset.connectors.sqla.models import SqlaTable
from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
from superset.versioning import api_helpers
from superset.versioning.api_helpers import (
PathEntityResponseError,
@@ -43,6 +49,52 @@ from superset.versioning.api_helpers import (
_UUID = "8b8c9f00-0000-4000-8000-000000000001"
+@pytest.mark.parametrize("model_cls", [Dashboard, Slice, SqlaTable])
+@pytest.mark.parametrize("missing", [True, False])
+def test_version_get_warns_only_for_missing_snapshot(
+ mocker: MockerFixture,
+ caplog: pytest.LogCaptureFixture,
+ model_cls: type[Model],
+ missing: bool,
+) -> None:
+ """Missing previews log identifiers once without logging entity contents."""
+ api: MagicMock = _api()
+ entity: SimpleNamespace = SimpleNamespace(id=17, name="private-entity-name")
+ version_uuid: str = "8b8c9f00-0000-4000-8000-000000000002"
+ mocker.patch.object(
+ api_helpers,
+ "resolve_endpoint_path_entity",
+ return_value=(entity, UUID(_UUID)),
+ )
+ mocker.patch.object(
+ api_helpers.VersionDAO,
+ "get_version",
+ return_value=None if missing else {"name": entity.name},
+ )
+ mocker.patch.object(api_helpers, "set_version_etag_by_uuid", return_value="ok")
+
+ with caplog.at_level(logging.WARNING, logger=api_helpers.__name__):
+ response: Response = api_helpers.get_version_endpoint(
+ api, model_cls, _UUID, version_uuid
+ )
+
+ assert response == ("resp-404" if missing else "ok")
+ assert caplog.record_tuples == (
+ [
+ (
+ api_helpers.__name__,
+ logging.WARNING,
+ "Version snapshot not found: "
+ f"entity_type={model_cls.__name__} entity_id=17 "
+ f"version_uuid={version_uuid}",
+ )
+ ]
+ if missing
+ else []
+ )
+ assert entity.name not in caplog.text
+
+
def _api() -> MagicMock:
api = MagicMock()
api.response_400.return_value = "resp-400"
diff --git a/tests/unit_tests/versioning/test_runtime_capture.py b/tests/unit_tests/versioning/test_runtime_capture.py
index eaa39c86ef2..8abc2240ea4 100644
--- a/tests/unit_tests/versioning/test_runtime_capture.py
+++ b/tests/unit_tests/versioning/test_runtime_capture.py
@@ -35,7 +35,9 @@ from superset.extensions import db
from superset.models.core import Database
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
-from superset.versioning.queries import derive_version_uuid
+from superset.versioning.activity.render import apply_record_decoration
+from superset.versioning.changes.table import version_changes_table
+from superset.versioning.queries import derive_version_uuid, get_version
pytestmark: pytest.MarkDecorator = pytest.mark.parametrize(
"app", [{"SQLALCHEMY_DATABASE_URI": "sqlite://"}], indirect=True
@@ -64,6 +66,89 @@ def history_counts(session: Session) -> dict[str, int]:
}
+@pytest.mark.parametrize("chart_side", [False, True])
+def test_membership_activity_only_offers_existing_snapshots(
+ capture_session: Session, chart_side: bool
+) -> None:
+ """Membership-only saves stay visible without advertising missing snapshots."""
+ first: Slice = Slice(slice_name="first", viz_type="table", params="{}")
+ dashboard: Dashboard = Dashboard(dashboard_title="original", slices=[first])
+ capture_session.add(dashboard)
+ capture_session.commit()
+ dashboard.dashboard_title = "renamed"
+ capture_session.commit()
+ second: Slice = Slice(slice_name="second", viz_type="table", params="{}")
+ capture_session.add(second)
+ capture_session.commit()
+ if chart_side:
+ second.dashboards = [dashboard]
+ else:
+ dashboard.slices.append(second)
+ capture_session.commit()
+ records: list[dict[str, Any]] = [
+ dict(row)
+ for row in capture_session.execute(
+ sa.select(version_changes_table).where(
+ version_changes_table.c.entity_kind == "dashboard",
+ version_changes_table.c.entity_id == dashboard.id,
+ )
+ ).mappings()
+ ]
+ assert any(record["kind"] == "chart" for record in records)
+ for record in records:
+ record["entity_name"] = dashboard.dashboard_title
+ with patch(
+ "superset.versioning.activity.render.resolve_editorship",
+ return_value={("Dashboard", dashboard.id): True},
+ ):
+ apply_record_decoration(records, "Dashboard", dashboard.id)
+ for record in records:
+ if record["kind"] == "chart":
+ assert record["version_uuid"] is None
+ else:
+ assert record["version_uuid"] is not None
+ assert (
+ get_version(Dashboard, dashboard.uuid, UUID(record["version_uuid"]))
+ is not None
+ )
+
+
+def test_chart_activity_requires_its_own_snapshot(capture_session: Session) -> None:
+ """A different chart's snapshot cannot make this chart previewable at that tx."""
+ chart: Slice = Slice(slice_name="first", viz_type="table", params="{}")
+ capture_session.add(chart)
+ capture_session.commit()
+ other: Slice = Slice(slice_name="other", viz_type="table", params="{}")
+ capture_session.add(other)
+ capture_session.commit()
+ shadow: Any = version_class(Slice)
+ own_tx: int = (
+ capture_session.query(shadow).filter_by(id=chart.id).one().transaction_id
+ )
+ other_tx: int = (
+ capture_session.query(shadow).filter_by(id=other.id).one().transaction_id
+ )
+ records: list[dict[str, Any]] = [
+ {
+ "entity_kind": "chart",
+ "entity_id": chart.id,
+ "transaction_id": tx,
+ "entity_name": chart.slice_name,
+ "kind": "field",
+ }
+ for tx in (own_tx, other_tx)
+ ]
+ with patch(
+ "superset.versioning.activity.render.resolve_editorship",
+ return_value={("Slice", chart.id): True},
+ ):
+ apply_record_decoration(records, "Slice", chart.id)
+ assert records[0]["version_uuid"] is not None
+ assert get_version(Slice, chart.uuid, UUID(records[0]["version_uuid"])) is not None
+ assert records[1]["version_uuid"] is None
+ assert records[1]["entity_uuid"] == str(chart.uuid)
+
+
@pytest.mark.parametrize("enabled", [False, True])
@pytest.mark.parametrize(
"model,field",