fix (dataset editor): allow Source tab readOnly mode (#11781)

* fix (dataset editor) add read-only mode for Source tab

* add feature flag, add unit tests

* rebase and fix comment

* add message for padlock

* move padlock to the bottom of tab
This commit is contained in:
Grace Guo 2020-12-01 17:10:33 -08:00 committed by GitHub
parent ac9761c730
commit f292015ccd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 152 additions and 23 deletions

View File

@ -21,11 +21,15 @@ import { shallow } from 'enzyme';
import configureStore from 'redux-mock-store'; import configureStore from 'redux-mock-store';
import fetchMock from 'fetch-mock'; import fetchMock from 'fetch-mock';
import thunk from 'redux-thunk'; import thunk from 'redux-thunk';
import { Radio } from 'react-bootstrap';
import Icon from 'src/components/Icon';
import Tabs from 'src/common/components/Tabs'; import Tabs from 'src/common/components/Tabs';
import DatasourceEditor from 'src/datasource/DatasourceEditor'; import DatasourceEditor from 'src/datasource/DatasourceEditor';
import Field from 'src/CRUD/Field'; import Field from 'src/CRUD/Field';
import mockDatasource from 'spec/fixtures/mockDatasource'; import mockDatasource from 'spec/fixtures/mockDatasource';
import * as featureFlags from 'src/featureFlags';
import TableSelector from 'src/components/TableSelector';
const props = { const props = {
datasource: mockDatasource['7__table'], datasource: mockDatasource['7__table'],
@ -44,6 +48,7 @@ describe('DatasourceEditor', () => {
let wrapper; let wrapper;
let el; let el;
let inst; let inst;
let isFeatureEnabledMock;
beforeEach(() => { beforeEach(() => {
el = <DatasourceEditor {...props} />; el = <DatasourceEditor {...props} />;
@ -142,4 +147,65 @@ describe('DatasourceEditor', () => {
wrapper.find(Field).find({ fieldKey: 'fetch_values_predicate' }).exists(), wrapper.find(Field).find({ fieldKey: 'fetch_values_predicate' }).exists(),
).toBe(true); ).toBe(true);
}); });
describe('enable edit Source tab', () => {
beforeAll(() => {
isFeatureEnabledMock = jest
.spyOn(featureFlags, 'isFeatureEnabled')
.mockImplementation(
feature => feature === 'ENABLE_DATASET_SOURCE_EDIT',
);
wrapper = shallow(el, { context: { store } }).dive();
});
afterAll(() => {
isFeatureEnabledMock.mockRestore();
});
it('Source Tab: edit mode', () => {
wrapper.setState({ activeTabKey: 0, isEditMode: true });
const sourceTab = wrapper.find(Tabs.TabPane).first();
expect(sourceTab.find(Radio).first().prop('disabled')).toBe(false);
const icon = sourceTab.find(Icon);
expect(icon.prop('name')).toBe('lock-unlocked');
const tableSelector = sourceTab.find(Field).shallow().find(TableSelector);
expect(tableSelector.length).toBe(1);
expect(tableSelector.prop('readOnly')).toBe(false);
});
it('Source Tab: readOnly mode', () => {
const sourceTab = wrapper.find(Tabs.TabPane).first();
expect(sourceTab.find(Radio).length).toBe(2);
expect(sourceTab.find(Radio).first().prop('disabled')).toBe(true);
const icon = sourceTab.find(Icon);
expect(icon.prop('name')).toBe('lock-locked');
icon.parent().simulate('click');
expect(wrapper.state('isEditMode')).toBe(true);
const tableSelector = sourceTab.find(Field).shallow().find(TableSelector);
expect(tableSelector.length).toBe(1);
expect(tableSelector.prop('readOnly')).toBe(true);
});
});
it('disable edit Source tab', () => {
// when edit is disabled, show readOnly controls and no padlock
isFeatureEnabledMock = jest
.spyOn(featureFlags, 'isFeatureEnabled')
.mockImplementation(() => true);
wrapper = shallow(el, { context: { store } }).dive();
wrapper.setState({ activeTabKey: 0 });
const sourceTab = wrapper.find(Tabs.TabPane).first();
expect(sourceTab.find(Radio).length).toBe(2);
expect(sourceTab.find(Radio).first().prop('disabled')).toBe(true);
const icon = sourceTab.find(Icon);
expect(icon).toHaveLength(0);
isFeatureEnabledMock.mockRestore();
});
}); });

View File

@ -64,6 +64,7 @@ interface DatabaseSelectorProps {
onDbChange?: (db: any) => void; onDbChange?: (db: any) => void;
onSchemaChange?: (arg0?: any) => {}; onSchemaChange?: (arg0?: any) => {};
onSchemasLoad?: (schemas: Array<object>) => void; onSchemasLoad?: (schemas: Array<object>) => void;
readOnly?: boolean;
schema?: string; schema?: string;
sqlLabMode?: boolean; sqlLabMode?: boolean;
onChange?: ({ onChange?: ({
@ -87,6 +88,7 @@ export default function DatabaseSelector({
onDbChange, onDbChange,
onSchemaChange, onSchemaChange,
onSchemasLoad, onSchemasLoad,
readOnly = false,
schema, schema,
sqlLabMode = false, sqlLabMode = false,
}: DatabaseSelectorProps) { }: DatabaseSelectorProps) {
@ -237,7 +239,7 @@ export default function DatabaseSelector({
mutator={dbMutator} mutator={dbMutator}
placeholder={t('Select a database')} placeholder={t('Select a database')}
autoSelect autoSelect
isDisabled={!isDatabaseSelectEnabled} isDisabled={!isDatabaseSelectEnabled || readOnly}
/>, />,
null, null,
); );
@ -245,7 +247,7 @@ export default function DatabaseSelector({
function renderSchemaSelect() { function renderSchemaSelect() {
const value = schemaOptions.filter(({ value }) => currentSchema === value); const value = schemaOptions.filter(({ value }) => currentSchema === value);
const refresh = !formMode && ( const refresh = !formMode && !readOnly && (
<RefreshLabel <RefreshLabel
onClick={() => changeDataBase({ id: dbId }, true)} onClick={() => changeDataBase({ id: dbId }, true)}
tooltipContent={t('Force refresh schema list')} tooltipContent={t('Force refresh schema list')}
@ -266,6 +268,7 @@ export default function DatabaseSelector({
isLoading={schemaLoading} isLoading={schemaLoading}
autosize={false} autosize={false}
onChange={item => changeSchema(item)} onChange={item => changeSchema(item)}
isDisabled={readOnly}
/>, />,
refresh, refresh,
); );

View File

@ -97,6 +97,7 @@ interface TableSelectorProps {
onSchemasLoad?: () => void; onSchemasLoad?: () => void;
onTableChange?: (tableName: string, schema: string) => void; onTableChange?: (tableName: string, schema: string) => void;
onTablesLoad?: (options: Array<any>) => {}; onTablesLoad?: (options: Array<any>) => {};
readOnly?: boolean;
schema?: string; schema?: string;
sqlLabMode?: boolean; sqlLabMode?: boolean;
tableName?: string; tableName?: string;
@ -116,6 +117,7 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
onSchemasLoad, onSchemasLoad,
onTableChange, onTableChange,
onTablesLoad, onTablesLoad,
readOnly = false,
schema, schema,
sqlLabMode = true, sqlLabMode = true,
tableName, tableName,
@ -286,28 +288,22 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
getTableList={fetchTables} getTableList={fetchTables}
handleError={handleError} handleError={handleError}
onChange={onSelectionChange} onChange={onSelectionChange}
onDbChange={onDbChange} onDbChange={readOnly ? undefined : onDbChange}
onSchemaChange={onSchemaChange} onSchemaChange={readOnly ? undefined : onSchemaChange}
onSchemasLoad={onSchemasLoad} onSchemasLoad={onSchemasLoad}
schema={currentSchema} schema={currentSchema}
sqlLabMode={sqlLabMode} sqlLabMode={sqlLabMode}
isDatabaseSelectEnabled={isDatabaseSelectEnabled} isDatabaseSelectEnabled={isDatabaseSelectEnabled && !readOnly}
readOnly={readOnly}
/> />
); );
} }
function renderTableSelect() { function renderTableSelect() {
let tableSelectPlaceholder;
let tableSelectDisabled = false;
if (database && database.allow_multi_schema_metadata_fetch) {
tableSelectPlaceholder = t('Type to search ...');
} else {
tableSelectPlaceholder = t('Select table ');
tableSelectDisabled = true;
}
const options = tableOptions; const options = tableOptions;
let select = null; let select = null;
if (currentSchema && !formMode) { if (currentSchema && !formMode) {
// dataset editor
select = ( select = (
<Select <Select
name="select-table" name="select-table"
@ -321,6 +317,7 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
value={currentTableName} value={currentTableName}
optionRenderer={renderTableOption} optionRenderer={renderTableOption}
valueRenderer={renderTableOption} valueRenderer={renderTableOption}
isDisabled={readOnly}
/> />
); );
} else if (formMode) { } else if (formMode) {
@ -339,6 +336,15 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
/> />
); );
} else { } else {
// sql lab
let tableSelectPlaceholder;
let tableSelectDisabled = false;
if (database && database.allow_multi_schema_metadata_fetch) {
tableSelectPlaceholder = t('Type to search ...');
} else {
tableSelectPlaceholder = t('Select table ');
tableSelectDisabled = true;
}
select = ( select = (
<AsyncSelect <AsyncSelect
name="async-select-table" name="async-select-table"
@ -353,7 +359,7 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
/> />
); );
} }
const refresh = !formMode && ( const refresh = !formMode && !readOnly && (
<RefreshLabel <RefreshLabel
onClick={() => changeSchema({ value: schema }, true)} onClick={() => changeSchema({ value: schema }, true)}
tooltipContent={t('Force refresh table list')} tooltipContent={t('Force refresh table list')}

View File

@ -20,12 +20,13 @@ import React from 'react';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import { Alert, Badge, Col, Radio, Well } from 'react-bootstrap'; import { Alert, Badge, Col, Radio, Well } from 'react-bootstrap';
import shortid from 'shortid'; import shortid from 'shortid';
import { styled, SupersetClient, t } from '@superset-ui/core'; import { styled, SupersetClient, t, supersetTheme } from '@superset-ui/core';
import Tabs from 'src/common/components/Tabs'; import Tabs from 'src/common/components/Tabs';
import Button from 'src/components/Button'; import Button from 'src/components/Button';
import CertifiedIconWithTooltip from 'src/components/CertifiedIconWithTooltip'; import CertifiedIconWithTooltip from 'src/components/CertifiedIconWithTooltip';
import DatabaseSelector from 'src/components/DatabaseSelector'; import DatabaseSelector from 'src/components/DatabaseSelector';
import Icon from 'src/components/Icon';
import Label from 'src/components/Label'; import Label from 'src/components/Label';
import Loading from 'src/components/Loading'; import Loading from 'src/components/Loading';
import TableSelector from 'src/components/TableSelector'; import TableSelector from 'src/components/TableSelector';
@ -45,6 +46,7 @@ import Fieldset from 'src/CRUD/Fieldset';
import Field from 'src/CRUD/Field'; import Field from 'src/CRUD/Field';
import withToasts from 'src/messageToasts/enhancers/withToasts'; import withToasts from 'src/messageToasts/enhancers/withToasts';
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
const DatasourceContainer = styled.div` const DatasourceContainer = styled.div`
.change-warning { .change-warning {
@ -66,6 +68,15 @@ const FlexRowContainer = styled.div`
} }
`; `;
const EditLockContainer = styled.div`
font-size: ${supersetTheme.typography.sizes.s}px;
display: flex;
align-items: center;
a {
padding: 0 10px;
}
`;
const checkboxGenerator = (d, onChange) => ( const checkboxGenerator = (d, onChange) => (
<CheckboxControl value={d} onChange={onChange} /> <CheckboxControl value={d} onChange={onChange} />
); );
@ -279,6 +290,7 @@ class DatasourceEditor extends React.PureComponent {
isSqla: isSqla:
props.datasource.datasource_type === 'table' || props.datasource.datasource_type === 'table' ||
props.datasource.type === 'table', props.datasource.type === 'table',
isEditMode: false,
databaseColumns: props.datasource.columns.filter(col => !col.expression), databaseColumns: props.datasource.columns.filter(col => !col.expression),
calculatedColumns: props.datasource.columns.filter( calculatedColumns: props.datasource.columns.filter(
col => !!col.expression, col => !!col.expression,
@ -291,12 +303,16 @@ class DatasourceEditor extends React.PureComponent {
}; };
this.onChange = this.onChange.bind(this); this.onChange = this.onChange.bind(this);
this.onChangeEditMode = this.onChangeEditMode.bind(this);
this.onDatasourcePropChange = this.onDatasourcePropChange.bind(this); this.onDatasourcePropChange = this.onDatasourcePropChange.bind(this);
this.onDatasourceChange = this.onDatasourceChange.bind(this); this.onDatasourceChange = this.onDatasourceChange.bind(this);
this.syncMetadata = this.syncMetadata.bind(this); this.syncMetadata = this.syncMetadata.bind(this);
this.setColumns = this.setColumns.bind(this); this.setColumns = this.setColumns.bind(this);
this.validateAndChange = this.validateAndChange.bind(this); this.validateAndChange = this.validateAndChange.bind(this);
this.handleTabSelect = this.handleTabSelect.bind(this); this.handleTabSelect = this.handleTabSelect.bind(this);
this.allowEditSource = !isFeatureEnabled(
FeatureFlag.DISABLE_DATASET_SOURCE_EDIT,
);
} }
onChange() { onChange() {
@ -315,6 +331,10 @@ class DatasourceEditor extends React.PureComponent {
this.props.onChange(newDatasource, this.state.errors); this.props.onChange(newDatasource, this.state.errors);
} }
onChangeEditMode() {
this.setState(prevState => ({ isEditMode: !prevState.isEditMode }));
}
onDatasourceChange(datasource) { onDatasourceChange(datasource) {
this.setState({ datasource }, this.validateAndChange); this.setState({ datasource }, this.validateAndChange);
} }
@ -636,6 +656,7 @@ class DatasourceEditor extends React.PureComponent {
inline inline
onChange={this.onDatasourceTypeChange.bind(this, type.key)} onChange={this.onDatasourceTypeChange.bind(this, type.key)}
checked={this.state.datasourceType === type.key} checked={this.state.datasourceType === type.key}
disabled={!this.state.isEditMode}
> >
{type.label} {type.label}
</Radio> </Radio>
@ -655,13 +676,16 @@ class DatasourceEditor extends React.PureComponent {
dbId={datasource.database.id} dbId={datasource.database.id}
schema={datasource.schema} schema={datasource.schema}
onSchemaChange={schema => onSchemaChange={schema =>
this.state.isEditMode &&
this.onDatasourcePropChange('schema', schema) this.onDatasourcePropChange('schema', schema)
} }
onDbChange={database => onDbChange={database =>
this.state.isEditMode &&
this.onDatasourcePropChange('database', database) this.onDatasourcePropChange('database', database)
} }
formMode={false} formMode={false}
handleError={this.props.addDangerToast} handleError={this.props.addDangerToast}
readOnly={!this.state.isEditMode}
/> />
} }
/> />
@ -675,6 +699,7 @@ class DatasourceEditor extends React.PureComponent {
this.onDatasourcePropChange('table_name', table); this.onDatasourcePropChange('table_name', table);
}} }}
placeholder={t('dataset name')} placeholder={t('dataset name')}
disabled={!this.state.isEditMode}
/> />
} }
/> />
@ -692,6 +717,7 @@ class DatasourceEditor extends React.PureComponent {
offerEditInModal={false} offerEditInModal={false}
minLines={25} minLines={25}
maxLines={25} maxLines={25}
readOnly={!this.state.isEditMode}
/> />
} }
/> />
@ -727,16 +753,25 @@ class DatasourceEditor extends React.PureComponent {
schema={datasource.schema} schema={datasource.schema}
sqlLabMode={false} sqlLabMode={false}
tableName={datasource.table_name} tableName={datasource.table_name}
onSchemaChange={schema => onSchemaChange={
this.onDatasourcePropChange('schema', schema) this.state.isEditMode
? schema =>
this.onDatasourcePropChange('schema', schema)
: undefined
} }
onDbChange={database => onDbChange={
this.onDatasourcePropChange('database', database) this.state.isEditMode
? database =>
this.onDatasourcePropChange('database', database)
: undefined
} }
onTableChange={table => { onTableChange={
this.onDatasourcePropChange('table_name', table); this.state.isEditMode
}} ? table =>
isDatabaseSelectEnabled={false} this.onDatasourcePropChange('table_name', table)
: undefined
}
readOnly={!this.state.isEditMode}
/> />
} }
description={t( description={t(
@ -749,6 +784,22 @@ class DatasourceEditor extends React.PureComponent {
</Col> </Col>
)} )}
</Fieldset> </Fieldset>
{this.allowEditSource && (
<EditLockContainer>
<a href="#" onClick={this.onChangeEditMode}>
<Icon
color={supersetTheme.colors.grayscale.base}
name={this.state.isEditMode ? 'lock-unlocked' : 'lock-locked'}
/>
</a>
{!this.state.isEditMode && (
<div>{t('Click the lock to make changes.')}</div>
)}
{this.state.isEditMode && (
<div>{t('Click the lock to prevent further changes.')}</div>
)}
</EditLockContainer>
)}
</div> </div>
); );
} }

View File

@ -30,6 +30,7 @@ export enum FeatureFlag {
THUMBNAILS = 'THUMBNAILS', THUMBNAILS = 'THUMBNAILS',
LISTVIEWS_DEFAULT_CARD_VIEW = 'LISTVIEWS_DEFAULT_CARD_VIEW', LISTVIEWS_DEFAULT_CARD_VIEW = 'LISTVIEWS_DEFAULT_CARD_VIEW',
ENABLE_REACT_CRUD_VIEWS = 'ENABLE_REACT_CRUD_VIEWS', ENABLE_REACT_CRUD_VIEWS = 'ENABLE_REACT_CRUD_VIEWS',
DISABLE_DATASET_SOURCE_EDIT = 'DISABLE_DATASET_SOURCE_EDIT',
DISPLAY_MARKDOWN_HTML = 'DISPLAY_MARKDOWN_HTML', DISPLAY_MARKDOWN_HTML = 'DISPLAY_MARKDOWN_HTML',
ESCAPE_MARKDOWN_HTML = 'ESCAPE_MARKDOWN_HTML', ESCAPE_MARKDOWN_HTML = 'ESCAPE_MARKDOWN_HTML',
VERSIONED_EXPORT = 'VERSIONED_EXPORT', VERSIONED_EXPORT = 'VERSIONED_EXPORT',

View File

@ -304,6 +304,7 @@ DEFAULT_FEATURE_FLAGS: Dict[str, bool] = {
"ALLOW_DASHBOARD_DOMAIN_SHARDING": True, "ALLOW_DASHBOARD_DOMAIN_SHARDING": True,
# Experimental feature introducing a client (browser) cache # Experimental feature introducing a client (browser) cache
"CLIENT_CACHE": False, "CLIENT_CACHE": False,
"DISABLE_DATASET_SOURCE_EDIT": False,
"ENABLE_EXPLORE_JSON_CSRF_PROTECTION": False, "ENABLE_EXPLORE_JSON_CSRF_PROTECTION": False,
"ENABLE_TEMPLATE_PROCESSING": False, "ENABLE_TEMPLATE_PROCESSING": False,
"KV_STORE": False, "KV_STORE": False,

View File

@ -70,6 +70,7 @@ FRONTEND_CONF_KEYS = (
"SUPERSET_DASHBOARD_POSITION_DATA_LIMIT", "SUPERSET_DASHBOARD_POSITION_DATA_LIMIT",
"SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT", "SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT",
"SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE", "SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE",
"DISABLE_DATASET_SOURCE_EDIT",
"ENABLE_JAVASCRIPT_CONTROLS", "ENABLE_JAVASCRIPT_CONTROLS",
"DEFAULT_SQLLAB_LIMIT", "DEFAULT_SQLLAB_LIMIT",
"SQL_MAX_ROW", "SQL_MAX_ROW",