From 2cbc37c21bbe15392eca7213f77d2eec58c82459 Mon Sep 17 00:00:00 2001 From: Javier Rivero Iglesias Date: Fri, 1 May 2026 00:30:55 +0200 Subject: [PATCH 1/2] fix(explore): explain disabled chart overwrite option --- .../src/explore/components/SaveModal.test.tsx | 7 ++++++- .../src/explore/components/SaveModal.tsx | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/explore/components/SaveModal.test.tsx b/superset-frontend/src/explore/components/SaveModal.test.tsx index 1c79be556f5f..6a6be705dd6d 100644 --- a/superset-frontend/src/explore/components/SaveModal.test.tsx +++ b/superset-frontend/src/explore/components/SaveModal.test.tsx @@ -255,7 +255,7 @@ test('disables overwrite option for new slice', () => { }); test('disables overwrite option for non-owner', () => { - const { getByRole } = setup( + const { getByRole, getByText } = setup( {}, mockStore({ ...initialState, @@ -263,6 +263,11 @@ test('disables overwrite option for non-owner', () => { }), ); expect(getByRole('radio', { name: 'Save (Overwrite)' })).toBeDisabled(); + expect( + getByText( + 'Must be a chart owner to overwrite the existing chart. Save as a new chart instead.', + ), + ).toBeInTheDocument(); }); test('updates slice name and selected dashboard', async () => { diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx index 6e72769f0780..62503fc77310 100755 --- a/superset-frontend/src/explore/components/SaveModal.tsx +++ b/superset-frontend/src/explore/components/SaveModal.tsx @@ -34,6 +34,7 @@ import { Loading, Divider, Flex, + Typography, TreeSelect, } from '@superset-ui/core/components'; import { logging } from '@apache-superset/core/utils'; @@ -597,18 +598,32 @@ class SaveModal extends Component { renderSaveChartModal = () => { const info = this.info(); + const canOverwriteSlice = this.canOverwriteSlice(); + const isChartOwner = this.props.slice?.owners?.includes( + this.props.user.userId, + ); + const isExistingChartNonOwner = Boolean(this.props.slice && !isChartOwner); return (
this.changeAction('overwrite')} data-test="save-overwrite-radio" > {t('Save (Overwrite)')} + {isExistingChartNonOwner && ( +
+ + {t( + 'Must be a chart owner to overwrite the existing chart. Save as a new chart instead.', + )} + +
+ )} Date: Mon, 4 May 2026 23:57:16 +0200 Subject: [PATCH 2/2] Clarify overwrite help text in Explore save modal --- .../src/explore/components/SaveModal.test.tsx | 24 ++++++++++++++++++- .../src/explore/components/SaveModal.tsx | 16 ++++++------- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/explore/components/SaveModal.test.tsx b/superset-frontend/src/explore/components/SaveModal.test.tsx index 6a6be705dd6d..2265f6d57bd6 100644 --- a/superset-frontend/src/explore/components/SaveModal.test.tsx +++ b/superset-frontend/src/explore/components/SaveModal.test.tsx @@ -265,7 +265,29 @@ test('disables overwrite option for non-owner', () => { expect(getByRole('radio', { name: 'Save (Overwrite)' })).toBeDisabled(); expect( getByText( - 'Must be a chart owner to overwrite the existing chart. Save as a new chart instead.', + 'Must be a chart owner to overwrite this chart. Save as a new chart instead.', + ), + ).toBeInTheDocument(); +}); + +test('disables overwrite option for externally managed slice', () => { + const { getByRole, getByText } = setup( + {}, + mockStore({ + ...initialState, + explore: { + ...initialState.explore, + slice: { + ...initialState.explore.slice, + is_managed_externally: true, + }, + }, + }), + ); + expect(getByRole('radio', { name: 'Save (Overwrite)' })).toBeDisabled(); + expect( + getByText( + "This chart is managed externally and can't be overwritten in Superset.", ), ).toBeInTheDocument(); }); diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx index 62503fc77310..ec1696712715 100755 --- a/superset-frontend/src/explore/components/SaveModal.tsx +++ b/superset-frontend/src/explore/components/SaveModal.tsx @@ -599,10 +599,6 @@ class SaveModal extends Component { renderSaveChartModal = () => { const info = this.info(); const canOverwriteSlice = this.canOverwriteSlice(); - const isChartOwner = this.props.slice?.owners?.includes( - this.props.user.userId, - ); - const isExistingChartNonOwner = Boolean(this.props.slice && !isChartOwner); return ( @@ -615,12 +611,16 @@ class SaveModal extends Component { > {t('Save (Overwrite)')} - {isExistingChartNonOwner && ( + {this.props.slice && !canOverwriteSlice && (
- {t( - 'Must be a chart owner to overwrite the existing chart. Save as a new chart instead.', - )} + {this.props.slice.is_managed_externally + ? t( + "This chart is managed externally and can't be overwritten in Superset.", + ) + : t( + 'Must be a chart owner to overwrite this chart. Save as a new chart instead.', + )}
)}