From d304849b46b39bb6a261b735b7ca658962bc31e0 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 28 Mar 2022 16:32:57 -0700 Subject: [PATCH] feat: disable edits on external assets (#19344) * feat: disable edits on external assets * Update tests --- .../src/components/Button/index.tsx | 15 ++++++++++++--- .../components/Datasource/DatasourceModal.tsx | 13 ++++++++++++- .../src/dashboard/components/Header/index.jsx | 3 ++- .../components/PropertiesModal/index.tsx | 11 +++++++++++ .../components/PropertiesModal/index.tsx | 9 ++++++++- .../src/explore/components/SaveModal.tsx | 5 ++++- superset-frontend/src/types/Chart.ts | 2 ++ .../src/views/CRUD/chart/types.ts | 1 + .../CRUD/data/database/DatabaseModal/index.tsx | 18 +++++++++++++++--- .../src/views/CRUD/data/database/types.ts | 3 +++ .../src/views/CRUD/data/dataset/types.ts | 1 + superset-frontend/src/views/CRUD/hooks.ts | 1 + superset/charts/api.py | 2 ++ superset/dashboards/api.py | 1 + superset/dashboards/schemas.py | 1 + superset/databases/api.py | 1 + superset/datasets/api.py | 6 +++++- superset/models/dashboard.py | 1 + superset/models/slice.py | 1 + tests/integration_tests/charts/api_tests.py | 1 + .../integration_tests/dashboards/api_tests.py | 1 + 21 files changed, 86 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/components/Button/index.tsx b/superset-frontend/src/components/Button/index.tsx index bfe8bdb659..ea8cd4cd35 100644 --- a/superset-frontend/src/components/Button/index.tsx +++ b/superset-frontend/src/components/Button/index.tsx @@ -229,10 +229,19 @@ export default function Button(props: ButtonProps) { id={`${kebabCase(tooltip)}-tooltip`} title={tooltip} > - {/* this ternary wraps the button in a span so that the tooltip shows up - when the button is disabled. */} + {/* wrap the button in a span so that the tooltip shows up + when the button is disabled. */} {disabled ? ( - {button} + .superset-button': { + marginLeft: theme.gridUnit * 2, + }, + }} + > + {button} + ) : ( button )} diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index 92f35d622e..e03c416505 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx @@ -226,7 +226,18 @@ const DatasourceModal: FunctionComponent = ({ buttonStyle="primary" data-test="datasource-modal-save" onClick={onClickSave} - disabled={isSaving || errors.length > 0} + disabled={ + isSaving || + errors.length > 0 || + currentDatasource.is_managed_externally + } + tooltip={ + currentDatasource.is_managed_externally + ? t( + "This dataset is managed externally, and can't be edited in Superset", + ) + : '' + } > {t('Save')} diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 89b1b9bee6..7fd1afc82e 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -492,7 +492,8 @@ class Header extends React.PureComponent { } = this.props; const userCanEdit = dashboardInfo.dash_edit_perm && - filterboxMigrationState !== FILTER_BOX_MIGRATION_STATES.REVIEWING; + filterboxMigrationState !== FILTER_BOX_MIGRATION_STATES.REVIEWING && + !dashboardInfo.is_managed_externally; const userCanShare = dashboardInfo.dash_share_perm; const userCanSaveAs = dashboardInfo.dash_save_perm && diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx index 67c86cb1fc..effd18b3d0 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx @@ -75,6 +75,7 @@ type DashboardInfo = { slug: string; certifiedBy: string; certificationDetails: string; + isManagedExternally: boolean; }; const PropertiesModal = ({ @@ -151,6 +152,7 @@ const PropertiesModal = ({ owners, roles, metadata, + is_managed_externally, } = dashboardData; const dashboardInfo = { id, @@ -158,6 +160,7 @@ const PropertiesModal = ({ slug: slug || '', certifiedBy: certified_by || '', certificationDetails: certification_details || '', + isManagedExternally: is_managed_externally || false, }; form.setFieldsValue(dashboardInfo); @@ -515,6 +518,14 @@ const PropertiesModal = ({ buttonStyle="primary" className="m-r-5" cta + disabled={dashboardInfo?.isManagedExternally} + tooltip={ + dashboardInfo?.isManagedExternally + ? t( + "This dashboard is managed externally, and can't be edited in Superset", + ) + : '' + } > {saveLabel} diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx b/superset-frontend/src/explore/components/PropertiesModal/index.tsx index 484ae7d9ab..7fa4ef4580 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/index.tsx @@ -204,7 +204,14 @@ function PropertiesModal({ buttonSize="small" buttonStyle="primary" onClick={form.submit} - disabled={submitting || !name} + disabled={submitting || !name || slice.is_managed_externally} + tooltip={ + slice.is_managed_externally + ? t( + "This chart is managed externally, and can't be edited in Superset", + ) + : '' + } cta > {t('Save')} diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx index b5c47145e0..9c3e01eba0 100644 --- a/superset-frontend/src/explore/components/SaveModal.tsx +++ b/superset-frontend/src/explore/components/SaveModal.tsx @@ -79,7 +79,10 @@ class SaveModal extends React.Component { } canOverwriteSlice(): boolean { - return this.props.slice?.owners?.includes(this.props.userId); + return ( + this.props.slice?.owners?.includes(this.props.userId) && + !this.props.slice?.is_managed_externally + ); } componentDidMount() { diff --git a/superset-frontend/src/types/Chart.ts b/superset-frontend/src/types/Chart.ts index 866cdedf24..cf4a64b6ba 100644 --- a/superset-frontend/src/types/Chart.ts +++ b/superset-frontend/src/types/Chart.ts @@ -43,6 +43,7 @@ export interface Chart { form_data: { viz_type: string; }; + is_managed_externally: boolean; } export type Slice = { @@ -55,6 +56,7 @@ export type Slice = { certification_details?: string; form_data?: QueryFormData; query_context?: object; + is_managed_externally: boolean; }; export default Chart; diff --git a/superset-frontend/src/views/CRUD/chart/types.ts b/superset-frontend/src/views/CRUD/chart/types.ts index 209d009fd3..e16b42a23f 100644 --- a/superset-frontend/src/views/CRUD/chart/types.ts +++ b/superset-frontend/src/views/CRUD/chart/types.ts @@ -24,4 +24,5 @@ export type ChartObject = { cache_timeout?: number; datasource_id?: number; datasource_type?: number; + is_managed_externally: boolean; }; diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx index 783516231b..a92d6f6440 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx @@ -817,12 +817,24 @@ const DatabaseModal: FunctionComponent = ({ return []; }; - const renderEditModalFooter = () => ( + const renderEditModalFooter = (db: Partial | null) => ( <> {t('Close')} - + {t('Finish')} @@ -1033,7 +1045,7 @@ const DatabaseModal: FunctionComponent = ({ title={

{isEditMode ? t('Edit database') : t('Connect a database')}

} - footer={isEditMode ? renderEditModalFooter() : renderModalFooter()} + footer={isEditMode ? renderEditModalFooter(db) : renderModalFooter()} > diff --git a/superset-frontend/src/views/CRUD/data/database/types.ts b/superset-frontend/src/views/CRUD/data/database/types.ts index c03891689e..f8e1a7806e 100644 --- a/superset-frontend/src/views/CRUD/data/database/types.ts +++ b/superset-frontend/src/views/CRUD/data/database/types.ts @@ -95,6 +95,9 @@ export type DatabaseObject = { disable_data_preview?: boolean; // in SQL Lab }; + // External management + is_managed_externally: boolean; + // Temporary storage catalog?: Array; query_input?: string; diff --git a/superset-frontend/src/views/CRUD/data/dataset/types.ts b/superset-frontend/src/views/CRUD/data/dataset/types.ts index f8fdc7bdcd..97d6f5a280 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/types.ts +++ b/superset-frontend/src/views/CRUD/data/dataset/types.ts @@ -59,4 +59,5 @@ export type DatasetObject = { columns: ColumnObject[]; metrics: MetricObject[]; extra?: string; + is_managed_externally: boolean; }; diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index ae58aef0d9..b0ca13d96a 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -566,6 +566,7 @@ export const useChartEditModal = ( cache_timeout: chart.cache_timeout, certified_by: chart.certified_by, certification_details: chart.certification_details, + is_managed_externally: chart.is_managed_externally, }); } diff --git a/superset/charts/api.py b/superset/charts/api.py index df36a203ea..260ae4442c 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -125,9 +125,11 @@ class ChartRestApi(BaseSupersetModelRestApi): "slice_name", "viz_type", "query_context", + "is_managed_externally", ] show_select_columns = show_columns + ["table.id"] list_columns = [ + "is_managed_externally", "certified_by", "certification_details", "cache_timeout", diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index df67d8f775..5e761cdd1f 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -147,6 +147,7 @@ class DashboardRestApi(BaseSupersetModelRestApi): "owners.email", "roles.id", "roles.name", + "is_managed_externally", ] list_select_columns = list_columns + ["changed_on", "changed_by_fk"] order_columns = [ diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 09119db9e1..6cb1a3caee 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -166,6 +166,7 @@ class DashboardGetResponseSchema(Schema): owners = fields.List(fields.Nested(UserSchema)) roles = fields.List(fields.Nested(RolesSchema)) changed_on_humanized = fields.String(data_key="changed_on_delta_humanized") + is_managed_externally = fields.Boolean(allow_none=True, default=False) class DatabaseSchema(Schema): diff --git a/superset/databases/api.py b/superset/databases/api.py index 3737addede..44581ff4c8 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -123,6 +123,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): "parameters_schema", "server_cert", "sqlalchemy_uri", + "is_managed_externally", ] list_columns = [ "allow_file_upload", diff --git a/superset/datasets/api.py b/superset/datasets/api.py index b94b9ca124..bf7147cb7f 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -163,7 +163,11 @@ class DatasetRestApi(BaseSupersetModelRestApi): "url", "extra", ] - show_columns = show_select_columns + ["columns.type_generic", "database.backend"] + show_columns = show_select_columns + [ + "columns.type_generic", + "database.backend", + "is_managed_externally", + ] add_model_schema = DatasetPostSchema() edit_model_schema = DatasetPutSchema() add_columns = ["database", "schema", "table_name", "owners"] diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index 135f36a3fa..812b054468 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -279,6 +279,7 @@ class Dashboard(Model, AuditMixinNullable, ImportExportMixin): "slices": [slc.data for slc in self.slices], "position_json": positions, "last_modified_time": self.changed_on.replace(microsecond=0).timestamp(), + "is_managed_externally": self.is_managed_externally, } @cache_manager.cache.memoize( diff --git a/superset/models/slice.py b/superset/models/slice.py index 80b11c294f..862edb9ec8 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -227,6 +227,7 @@ class Slice( # pylint: disable=too-many-public-methods "slice_url": self.slice_url, "certified_by": self.certified_by, "certification_details": self.certification_details, + "is_managed_externally": self.is_managed_externally, } @property diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index 65ed10fe13..daff4f14f6 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -749,6 +749,7 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin): "slice_name": "title", "viz_type": None, "query_context": None, + "is_managed_externally": False, } data = json.loads(rv.data.decode("utf-8")) self.assertEqual(data["result"], expected_result) diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 8ee6bb5925..938de31414 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -349,6 +349,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi "url": "/superset/dashboard/slug1/", "slug": "slug1", "thumbnail_url": dashboard.thumbnail_url, + "is_managed_externally": False, } data = json.loads(rv.data.decode("utf-8")) self.assertIn("changed_on", data["result"])