Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: disable edits on external assets #19344

Merged
Merged
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
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 @@ -816,12 +816,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 @@ -1032,7 +1044,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