Skip to content

Commit

Permalink
feat: disable edits on external assets (#19344)
Browse files Browse the repository at this point in the history
* feat: disable edits on external assets

* Update tests
  • Loading branch information
betodealmeida committed Mar 28, 2022
1 parent b689ac2 commit d304849
Show file tree
Hide file tree
Showing 21 changed files with 86 additions and 11 deletions.
15 changes: 12 additions & 3 deletions superset-frontend/src/components/Button/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 ? (
<span css={{ cursor: 'not-allowed' }}>{button}</span>
<span
css={{
cursor: 'not-allowed',
'& > .superset-button': {
marginLeft: theme.gridUnit * 2,
},
}}
>
{button}
</span>
) : (
button
)}
Expand Down
13 changes: 12 additions & 1 deletion superset-frontend/src/components/Datasource/DatasourceModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,18 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
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')}
</Button>
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/dashboard/components/Header/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type DashboardInfo = {
slug: string;
certifiedBy: string;
certificationDetails: string;
isManagedExternally: boolean;
};

const PropertiesModal = ({
Expand Down Expand Up @@ -151,13 +152,15 @@ const PropertiesModal = ({
owners,
roles,
metadata,
is_managed_externally,
} = dashboardData;
const dashboardInfo = {
id,
title: dashboard_title,
slug: slug || '',
certifiedBy: certified_by || '',
certificationDetails: certification_details || '',
isManagedExternally: is_managed_externally || false,
};

form.setFieldsValue(dashboardInfo);
Expand Down Expand Up @@ -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}
</Button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')}
Expand Down
5 changes: 4 additions & 1 deletion superset-frontend/src/explore/components/SaveModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
}

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() {
Expand Down
2 changes: 2 additions & 0 deletions superset-frontend/src/types/Chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export interface Chart {
form_data: {
viz_type: string;
};
is_managed_externally: boolean;
}

export type Slice = {
Expand All @@ -55,6 +56,7 @@ export type Slice = {
certification_details?: string;
form_data?: QueryFormData;
query_context?: object;
is_managed_externally: boolean;
};

export default Chart;
1 change: 1 addition & 0 deletions superset-frontend/src/views/CRUD/chart/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ export type ChartObject = {
cache_timeout?: number;
datasource_id?: number;
datasource_type?: number;
is_managed_externally: boolean;
};
Original file line number Diff line number Diff line change
Expand Up @@ -817,12 +817,24 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
return [];
};

const renderEditModalFooter = () => (
const renderEditModalFooter = (db: Partial<DatabaseObject> | null) => (
<>
<StyledFooterButton key="close" onClick={onClose}>
{t('Close')}
</StyledFooterButton>
<StyledFooterButton key="submit" buttonStyle="primary" onClick={onSave}>
<StyledFooterButton
key="submit"
buttonStyle="primary"
onClick={onSave}
disabled={db?.is_managed_externally}
tooltip={
db?.is_managed_externally
? t(
"This database is managed externally, and can't be edited in Superset",
)
: ''
}
>
{t('Finish')}
</StyledFooterButton>
</>
Expand Down Expand Up @@ -1033,7 +1045,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
title={
<h4>{isEditMode ? t('Edit database') : t('Connect a database')}</h4>
}
footer={isEditMode ? renderEditModalFooter() : renderModalFooter()}
footer={isEditMode ? renderEditModalFooter(db) : renderModalFooter()}
>
<StyledStickyHeader>
<TabHeader>
Expand Down
3 changes: 3 additions & 0 deletions superset-frontend/src/views/CRUD/data/database/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ export type DatabaseObject = {
disable_data_preview?: boolean; // in SQL Lab
};

// External management
is_managed_externally: boolean;

// Temporary storage
catalog?: Array<CatalogObject>;
query_input?: string;
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/views/CRUD/data/dataset/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,5 @@ export type DatasetObject = {
columns: ColumnObject[];
metrics: MetricObject[];
extra?: string;
is_managed_externally: boolean;
};
1 change: 1 addition & 0 deletions superset-frontend/src/views/CRUD/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}

Expand Down
2 changes: 2 additions & 0 deletions superset/charts/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,11 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]:
"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",
Expand Down
1 change: 1 addition & 0 deletions superset/dashboards/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]:
"owners.email",
"roles.id",
"roles.name",
"is_managed_externally",
]
list_select_columns = list_columns + ["changed_on", "changed_by_fk"]
order_columns = [
Expand Down
1 change: 1 addition & 0 deletions superset/dashboards/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions superset/databases/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
"parameters_schema",
"server_cert",
"sqlalchemy_uri",
"is_managed_externally",
]
list_columns = [
"allow_file_upload",
Expand Down
6 changes: 5 additions & 1 deletion superset/datasets/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
1 change: 1 addition & 0 deletions superset/models/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ def data(self) -> Dict[str, Any]:
"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(
Expand Down
1 change: 1 addition & 0 deletions superset/models/slice.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ def data(self) -> Dict[str, Any]:
"slice_url": self.slice_url,
"certified_by": self.certified_by,
"certification_details": self.certification_details,
"is_managed_externally": self.is_managed_externally,
}

@property
Expand Down
1 change: 1 addition & 0 deletions tests/integration_tests/charts/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@ def test_get_chart(self):
"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)
Expand Down
1 change: 1 addition & 0 deletions tests/integration_tests/dashboards/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ def test_get_dashboard(self):
"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"])
Expand Down

0 comments on commit d304849

Please sign in to comment.