Skip to content

Commit

Permalink
fix(explore): update overwrite button on perm change (apache#16437)
Browse files Browse the repository at this point in the history
* fix(explore): update overwrite on perm change

* remove redundant user_id prop

* fix types

* fix user type

* fix tests

* fix lint
  • Loading branch information
villebro authored Aug 26, 2021
1 parent eb790c9 commit ca37d03
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,16 @@ describe('SaveModal', () => {
dashboards: [],
},
explore: {
can_overwrite: true,
user_id: '1',
datasource: {},
slice: {
slice_id: 1,
slice_name: 'title',
owners: [1],
},
alert: null,
user: {
userId: 1,
},
},
};
const store = mockStore(initialState);
Expand Down Expand Up @@ -104,7 +106,7 @@ describe('SaveModal', () => {

it('disable overwrite option for non-owner', () => {
const wrapperForNonOwner = getWrapper();
wrapperForNonOwner.setProps({ can_overwrite: false });
wrapperForNonOwner.setProps({ userId: 2 });
const overwriteRadio = wrapperForNonOwner.find('#overwrite-radio');
expect(overwriteRadio).toHaveLength(1);
expect(overwriteRadio.prop('disabled')).toBe(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,6 @@ function mapStateToProps(state) {
timeout: explore.common.conf.SUPERSET_WEBSERVER_TIMEOUT,
ownState: dataMask[form_data.slice_id ?? 0]?.ownState, // 0 - unsaved chart
impressionId,
userId: explore.user_id,
user: explore.user,
reports,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export default function PropertiesModal({
buttonStyle="primary"
// @ts-ignore
onClick={onSubmit}
disabled={!owners || submitting || !name}
disabled={submitting || !name}
cta
>
{t('Save')}
Expand Down
16 changes: 9 additions & 7 deletions superset-frontend/src/explore/components/SaveModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,10 @@ const SK_DASHBOARD_ID = 'save_chart_recent_dashboard';
const SELECT_PLACEHOLDER = t('**Select** a dashboard OR **create** a new one');

type SaveModalProps = {
can_overwrite?: boolean;
onHide: () => void;
actions: Record<string, any>;
form_data?: Record<string, any>;
userId: string;
userId: number;
dashboards: Array<any>;
alert?: string;
sliceName?: string;
Expand Down Expand Up @@ -70,14 +69,18 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
saveToDashboardId: null,
newSliceName: props.sliceName,
alert: null,
action: props.can_overwrite ? 'overwrite' : 'saveas',
action: this.canOverwriteSlice() ? 'overwrite' : 'saveas',
};
this.onDashboardSelectChange = this.onDashboardSelectChange.bind(this);
this.onSliceNameChange = this.onSliceNameChange.bind(this);
this.changeAction = this.changeAction.bind(this);
this.saveOrOverwrite = this.saveOrOverwrite.bind(this);
}

canOverwriteSlice(): boolean {
return this.props.slice?.owners?.includes(this.props.userId);
}

componentDidMount() {
this.props.actions.fetchDashboards(this.props.userId).then(() => {
const dashboardIds = this.props.dashboards.map(
Expand Down Expand Up @@ -196,7 +199,7 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
disabled={!this.state.newSliceName}
data-test="btn-modal-save"
>
{!this.props.can_overwrite && this.props.slice
{!this.canOverwriteSlice() && this.props.slice
? t('Save as new chart')
: t('Save')}
</Button>
Expand Down Expand Up @@ -225,7 +228,7 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
<FormItem data-test="radio-group">
<Radio
id="overwrite-radio"
disabled={!(this.props.can_overwrite && this.props.slice)}
disabled={!this.canOverwriteSlice()}
checked={this.state.action === 'overwrite'}
onChange={() => this.changeAction('overwrite')}
data-test="save-overwrite-radio"
Expand Down Expand Up @@ -289,8 +292,7 @@ function mapStateToProps({
return {
datasource: explore.datasource,
slice: explore.slice,
can_overwrite: explore.can_overwrite,
userId: explore.user_id,
userId: explore.user?.userId,
dashboards: saveModal.dashboards,
alert: saveModal.saveModalAlert,
};
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/explore/reducers/exploreReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ export default function exploreReducer(state = {}, action) {
slice: {
...state.slice,
...action.slice,
owners: action.slice.owners ?? null,
},
sliceName: action.slice.slice_name ?? state.sliceName,
};
Expand Down
13 changes: 8 additions & 5 deletions superset-frontend/src/explore/reducers/getInitialState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import {
ControlStateMapping,
DatasourceMeta,
} from '@superset-ui/chart-controls';
import { CommonBootstrapData } from 'src/types/bootstrapTypes';
import {
CommonBootstrapData,
UserWithPermissionsAndRoles,
} from 'src/types/bootstrapTypes';
import getToastsFromPyFlashMessages from 'src/messageToasts/utils/getToastsFromPyFlashMessages';

import { ChartState, Slice } from 'src/explore/types';
Expand All @@ -37,15 +40,15 @@ export interface ExlorePageBootstrapData extends JsonObject {
can_add: boolean;
can_download: boolean;
can_overwrite: boolean;
common: CommonBootstrapData;
datasource: DatasourceMeta;
form_data: QueryFormData;
datasource_id: number;
datasource_type: DatasourceType;
forced_height: string | null;
form_data: QueryFormData;
slice: Slice | null;
standalone: boolean;
user_id: number;
forced_height: string | null;
common: CommonBootstrapData;
user: UserWithPermissionsAndRoles;
}

export default function getInitialState(
Expand Down
4 changes: 2 additions & 2 deletions superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,9 @@ def data(self) -> Dict[str, Any]:
)


class SqlaTable( # pylint: disable=too-many-instance-attributes,too-many-public-methods
class SqlaTable(
Model, BaseDatasource
):
): # pylint: disable=too-many-instance-attributes,too-many-public-methods
"""An ORM object for SqlAlchemy table references"""

type = "table"
Expand Down
4 changes: 1 addition & 3 deletions superset/models/slice.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,7 @@ def data(self) -> Dict[str, Any]:
"form_data": self.form_data,
"query_context": self.query_context,
"modified": self.modified(),
"owners": [
f"{owner.first_name} {owner.last_name}" for owner in self.owners
],
"owners": [owner.id for owner in self.owners],
"slice_id": self.id,
"slice_name": self.slice_name,
"slice_url": self.slice_url,
Expand Down
4 changes: 0 additions & 4 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,6 @@ def import_dashboards(self) -> FlaskResponse:
def explore( # pylint: disable=too-many-locals
self, datasource_type: Optional[str] = None, datasource_id: Optional[int] = None
) -> FlaskResponse:
user_id = g.user.get_id() if g.user else None
form_data, slc = get_form_data(use_slice_data=True)
query_context = request.form.get("query_context")
# Flash the SIP-15 message if the slice is owned by the current user and has not
Expand Down Expand Up @@ -843,14 +842,12 @@ def explore( # pylint: disable=too-many-locals
bootstrap_data = {
"can_add": slice_add_perm,
"can_download": slice_download_perm,
"can_overwrite": slice_overwrite_perm,
"datasource": datasource_data,
"form_data": form_data,
"datasource_id": datasource_id,
"datasource_type": datasource_type,
"slice": slc.data if slc else None,
"standalone": standalone_mode,
"user_id": user_id,
"user": bootstrap_user_data(g.user, include_perms=True),
"forced_height": request.args.get("height"),
"common": common_bootstrap_payload(),
Expand Down Expand Up @@ -1020,7 +1017,6 @@ def save_or_overwrite_slice(
response = {
"can_add": slice_add_perm,
"can_download": slice_download_perm,
"can_overwrite": is_owner(slc, g.user),
"form_data": slc.form_data,
"slice": slc.data,
"dashboard_url": dash.url if dash else None,
Expand Down

0 comments on commit ca37d03

Please sign in to comment.