diff --git a/.github/workflows/superset-e2e.yml b/.github/workflows/superset-e2e.yml index bc47d6a17752f..be0df99551a40 100644 --- a/.github/workflows/superset-e2e.yml +++ b/.github/workflows/superset-e2e.yml @@ -24,7 +24,6 @@ jobs: browser: ["chrome"] env: FLASK_ENV: development - ENABLE_REACT_CRUD_VIEWS: true SUPERSET_CONFIG: tests.integration_tests.superset_test_config SUPERSET__SQLALCHEMY_DATABASE_URI: postgresql+psycopg2://superset:superset@127.0.0.1:15432/superset PYTHONPATH: ${{ github.workspace }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 910572e3db7d1..84a948511ab15 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -804,7 +804,6 @@ We use [Cypress](https://www.cypress.io/) for integration tests. Tests can be ru ```bash export SUPERSET_CONFIG=tests.integration_tests.superset_test_config export SUPERSET_TESTENV=true -export ENABLE_REACT_CRUD_VIEWS=true export CYPRESS_BASE_URL="http://localhost:8081" superset db upgrade superset load_test_users diff --git a/RESOURCES/FEATURE_FLAGS.md b/RESOURCES/FEATURE_FLAGS.md index 69f8f8a5a691d..7ea9aefcbeb4f 100644 --- a/RESOURCES/FEATURE_FLAGS.md +++ b/RESOURCES/FEATURE_FLAGS.md @@ -64,4 +64,3 @@ These features flags currently default to True and **will be removed in a future - ALLOW_DASHBOARD_DOMAIN_SHARDING - DISPLAY_MARKDOWN_HTML -- ENABLE_REACT_CRUD_VIEWS diff --git a/UPDATING.md b/UPDATING.md index 2f00b14ef8bf9..4272ecc56bb32 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -29,6 +29,7 @@ assists people when migrating to a new version. ### Breaking Changes +- [19231](https://github.com/apache/superset/pull/19231): The `ENABLE_REACT_CRUD_VIEWS` feature flag has been removed (permanently enabled). Any deployments which had set this flag to false will need to verify that the React views support their use case. - [17556](https://github.com/apache/superset/pull/17556): Bumps mysqlclient from v1 to v2 - [19113](https://github.com/apache/superset/pull/19113): The `ENABLE_JAVASCRIPT_CONTROLS` setting has moved from app config to a feature flag. Any deployments who overrode this setting will now need to override the feature flag from here onward. - [18976](https://github.com/apache/superset/pull/18976): When running the app in debug mode, the app will default to use `SimpleCache` for `FILTER_STATE_CACHE_CONFIG` and `EXPLORE_FORM_DATA_CACHE_CONFIG`. When running in non-debug mode, a cache backend will need to be defined, otherwise the application will fail to start. For installations using Redis or other caching backends, it is recommended to use the same backend for both cache configs. diff --git a/docker/docker-bootstrap.sh b/docker/docker-bootstrap.sh index 67e5294be5fdc..150f351e4b0d7 100755 --- a/docker/docker-bootstrap.sh +++ b/docker/docker-bootstrap.sh @@ -23,7 +23,6 @@ REQUIREMENTS_LOCAL="/app/docker/requirements-local.txt" if [ "$CYPRESS_CONFIG" == "true" ]; then export SUPERSET_CONFIG=tests.integration_tests.superset_test_config export SUPERSET_TESTENV=true - export ENABLE_REACT_CRUD_VIEWS=true export SUPERSET__SQLALCHEMY_DATABASE_URI=postgresql+psycopg2://superset:superset@db:5432/superset fi # diff --git a/docker/docker-init.sh b/docker/docker-init.sh index d5ead5039857e..07830694048a7 100755 --- a/docker/docker-init.sh +++ b/docker/docker-init.sh @@ -43,7 +43,6 @@ if [ "$CYPRESS_CONFIG" == "true" ]; then ADMIN_PASSWORD="general" export SUPERSET_CONFIG=tests.superset_test_config export SUPERSET_TESTENV=true - export ENABLE_REACT_CRUD_VIEWS=true export SUPERSET__SQLALCHEMY_DATABASE_URI=postgresql+psycopg2://superset:superset@db:5432/superset fi # Initialize the database diff --git a/docs/docs/contributing/testing-locally.mdx b/docs/docs/contributing/testing-locally.mdx index 17a1c81086444..22a628b661502 100644 --- a/docs/docs/contributing/testing-locally.mdx +++ b/docs/docs/contributing/testing-locally.mdx @@ -76,7 +76,6 @@ We use [Cypress](https://www.cypress.io/) for integration tests. Tests can be ru ```bash export SUPERSET_CONFIG=tests.integration_tests.superset_test_config export SUPERSET_TESTENV=true -export ENABLE_REACT_CRUD_VIEWS=true export CYPRESS_BASE_URL="http://localhost:8081" superset db upgrade superset load_test_users diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index 8ed617cc3e631..6e9a824a47a91 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -32,7 +32,6 @@ export enum FeatureFlag { THUMBNAILS = 'THUMBNAILS', LISTVIEWS_DEFAULT_CARD_VIEW = 'LISTVIEWS_DEFAULT_CARD_VIEW', DISABLE_LEGACY_DATASOURCE_EDITOR = 'DISABLE_LEGACY_DATASOURCE_EDITOR', - ENABLE_REACT_CRUD_VIEWS = 'ENABLE_REACT_CRUD_VIEWS', DISABLE_DATASET_SOURCE_EDIT = 'DISABLE_DATASET_SOURCE_EDIT', DISPLAY_MARKDOWN_HTML = 'DISPLAY_MARKDOWN_HTML', ESCAPE_MARKDOWN_HTML = 'ESCAPE_MARKDOWN_HTML', diff --git a/superset-frontend/src/SqlLab/components/App/index.jsx b/superset-frontend/src/SqlLab/components/App/index.jsx index bce43f477800e..c98ff12e87017 100644 --- a/superset-frontend/src/SqlLab/components/App/index.jsx +++ b/superset-frontend/src/SqlLab/components/App/index.jsx @@ -21,7 +21,6 @@ import PropTypes from 'prop-types'; import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; import { t, supersetTheme, ThemeProvider } from '@superset-ui/core'; -import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import throttle from 'lodash/throttle'; import ToastContainer from 'src/components/MessageToasts/ToastContainer'; import { @@ -32,7 +31,6 @@ import { import * as Actions from 'src/SqlLab/actions/sqlLab'; import TabbedSqlEditors from '../TabbedSqlEditors'; import QueryAutoRefresh from '../QueryAutoRefresh'; -import QuerySearch from '../QuerySearch'; class App extends React.PureComponent { constructor(props) { @@ -96,29 +94,14 @@ class App extends React.PureComponent { } render() { - let content; if (this.state.hash && this.state.hash === '#search') { - if (isFeatureEnabled(FeatureFlag.ENABLE_REACT_CRUD_VIEWS)) { - return window.location.replace('/superset/sqllab/history/'); - } - content = ( - - ); - } else { - content = ( - <> - - - - ); + return window.location.replace('/superset/sqllab/history/'); } return (
- {content} + +
diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx index c9d608817794d..9743e3a325f36 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.test.jsx @@ -24,7 +24,7 @@ import { Provider } from 'react-redux'; import fetchMock from 'fetch-mock'; import thunk from 'redux-thunk'; import sinon from 'sinon'; -import { supersetTheme, ThemeProvider, FeatureFlag } from '@superset-ui/core'; +import { supersetTheme, ThemeProvider } from '@superset-ui/core'; import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; import Modal from 'src/components/Modal'; @@ -70,11 +70,7 @@ describe('DatasourceModal', () => { let wrapper; let isFeatureEnabledMock; beforeEach(async () => { - isFeatureEnabledMock = jest - .spyOn(featureFlags, 'isFeatureEnabled') - .mockImplementation( - featureFlag => featureFlag === FeatureFlag.ENABLE_REACT_CRUD_VIEWS, - ); + isFeatureEnabledMock = jest.spyOn(featureFlags, 'isFeatureEnabled'); fetchMock.reset(); wrapper = await mountAndWait(); }); @@ -125,28 +121,3 @@ describe('DatasourceModal', () => { ).toExist(); }); }); - -describe('DatasourceModal without legacy data btn', () => { - let wrapper; - let isFeatureEnabledMock; - beforeEach(async () => { - isFeatureEnabledMock = jest - .spyOn(featureFlags, 'isFeatureEnabled') - .mockReturnValue(false); - fetchMock.reset(); - wrapper = await mountAndWait(); - }); - - afterAll(() => { - isFeatureEnabledMock.restore(); - }); - - it('hides legacy data source btn', () => { - isFeatureEnabledMock = jest - .spyOn(featureFlags, 'isFeatureEnabled') - .mockReturnValue(false); - expect( - wrapper.find('button[data-test="datasource-modal-legacy-edit"]'), - ).not.toExist(); - }); -}); diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index 124c404082612..92f35d622edd8 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx @@ -183,9 +183,9 @@ const DatasourceModal: FunctionComponent = ({ }); }; - const showLegacyDatasourceEditor = - isFeatureEnabled(FeatureFlag.ENABLE_REACT_CRUD_VIEWS) && - !isFeatureEnabled(FeatureFlag.DISABLE_LEGACY_DATASOURCE_EDITOR); + const showLegacyDatasourceEditor = !isFeatureEnabled( + FeatureFlag.DISABLE_LEGACY_DATASOURCE_EDITOR, + ); return ( Optional[str]: "TAGGING_SYSTEM": False, "SQLLAB_BACKEND_PERSISTENCE": True, "LISTVIEWS_DEFAULT_CARD_VIEW": False, - # Enables the replacement React views for all the FAB views (list, edit, show) with - # designs introduced in https://github.com/apache/superset/issues/8976 - # (SIP-34). This is a work in progress so not all features available in FAB have - # been implemented. - "ENABLE_REACT_CRUD_VIEWS": True, # When True, this flag allows display of HTML tags in Markdown components "DISPLAY_MARKDOWN_HTML": True, # When True, this escapes HTML (rather than rendering it) in Markdown components diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index a16ffa49f62ba..4b43d2f046f6b 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -647,7 +647,4 @@ class RefreshResults: @expose("/list/") @has_access def list(self) -> FlaskResponse: - if not is_feature_enabled("ENABLE_REACT_CRUD_VIEWS"): - return super().list() - return super().render_app_template() diff --git a/superset/views/alerts.py b/superset/views/alerts.py index 416966fbe7c35..04640fa223fe8 100644 --- a/superset/views/alerts.py +++ b/superset/views/alerts.py @@ -92,10 +92,7 @@ class BaseAlertReportView(BaseSupersetView): @has_access @permission_name("read") def list(self) -> FlaskResponse: - if not ( - is_feature_enabled("ENABLE_REACT_CRUD_VIEWS") - and is_feature_enabled("ALERT_REPORTS") - ): + if not is_feature_enabled("ALERT_REPORTS"): return abort(404) return super().render_app_template() @@ -103,10 +100,7 @@ def list(self) -> FlaskResponse: @has_access @permission_name("read") def log(self, pk: int) -> FlaskResponse: # pylint: disable=unused-argument - if not ( - is_feature_enabled("ENABLE_REACT_CRUD_VIEWS") - and is_feature_enabled("ALERT_REPORTS") - ): + if not is_feature_enabled("ALERT_REPORTS"): return abort(404) return super().render_app_template() diff --git a/superset/views/annotations.py b/superset/views/annotations.py index dc1df5642af35..87150c5cdda31 100644 --- a/superset/views/annotations.py +++ b/superset/views/annotations.py @@ -23,7 +23,6 @@ from flask_babel import lazy_gettext as _ from wtforms.validators import StopValidation -from superset import is_feature_enabled from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.models.annotations import Annotation, AnnotationLayer from superset.superset_typing import FlaskResponse @@ -100,9 +99,6 @@ def pre_update(self, item: "AnnotationModelView") -> None: @expose("//annotation/", methods=["GET"]) @has_access def annotation(self, pk: int) -> FlaskResponse: # pylint: disable=unused-argument - if not is_feature_enabled("ENABLE_REACT_CRUD_VIEWS"): - return super().list() - return super().render_app_template() @@ -128,7 +124,4 @@ class AnnotationLayerModelView(SupersetModelView): @expose("/list/") @has_access def list(self) -> FlaskResponse: - if not is_feature_enabled("ENABLE_REACT_CRUD_VIEWS"): - return super().list() - return super().render_app_template() diff --git a/superset/views/chart/views.py b/superset/views/chart/views.py index 9ecc69f7b9e8e..72058c32e7f33 100644 --- a/superset/views/chart/views.py +++ b/superset/views/chart/views.py @@ -21,7 +21,6 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import lazy_gettext as _ -from superset import is_feature_enabled from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.models.slice import Slice from superset.superset_typing import FlaskResponse @@ -73,9 +72,6 @@ def add(self) -> FlaskResponse: @expose("/list/") @has_access def list(self) -> FlaskResponse: - if not is_feature_enabled("ENABLE_REACT_CRUD_VIEWS"): - return super().list() - return super().render_app_template() diff --git a/superset/views/core.py b/superset/views/core.py index 6957296ab634c..50a56569c547c 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2919,9 +2919,6 @@ def sqllab(self) -> FlaskResponse: @expose("/sqllab/history/", methods=["GET"]) @event_logger.log_this def sqllab_history(self) -> FlaskResponse: - if not is_feature_enabled("ENABLE_REACT_CRUD_VIEWS"): - return redirect("/superset/sqllab#search", code=307) - return super().render_app_template() @api diff --git a/superset/views/css_templates.py b/superset/views/css_templates.py index 2cfbd43ae962a..74505e7109dc6 100644 --- a/superset/views/css_templates.py +++ b/superset/views/css_templates.py @@ -19,7 +19,6 @@ from flask_appbuilder.security.decorators import has_access from flask_babel import lazy_gettext as _ -from superset import is_feature_enabled from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.models import core as models from superset.superset_typing import FlaskResponse @@ -46,9 +45,6 @@ class CssTemplateModelView(SupersetModelView, DeleteMixin): @expose("/list/") @has_access def list(self) -> FlaskResponse: - if not is_feature_enabled("ENABLE_REACT_CRUD_VIEWS"): - return super().list() - return super().render_app_template() diff --git a/superset/views/dashboard/views.py b/superset/views/dashboard/views.py index 49ba61d08e0d2..471d168e1ec64 100644 --- a/superset/views/dashboard/views.py +++ b/superset/views/dashboard/views.py @@ -61,9 +61,6 @@ class DashboardModelView( @has_access @expose("/list/") def list(self) -> FlaskResponse: - if not is_feature_enabled("ENABLE_REACT_CRUD_VIEWS"): - return super().list() - return super().render_app_template() @action("mulexport", __("Export"), __("Export dashboards?"), "fa-database") diff --git a/superset/views/database/views.py b/superset/views/database/views.py index aea4e04383570..659a1be78d8e3 100644 --- a/superset/views/database/views.py +++ b/superset/views/database/views.py @@ -31,7 +31,7 @@ from wtforms.validators import ValidationError import superset.models.core as models -from superset import app, db, is_feature_enabled +from superset import app, db from superset.connectors.sqla.models import SqlaTable from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.exceptions import CertificateException @@ -106,9 +106,6 @@ def _delete(self, pk: int) -> None: @expose("/list/") @has_access def list(self) -> FlaskResponse: - if not is_feature_enabled("ENABLE_REACT_CRUD_VIEWS"): - return super().list() - return super().render_app_template() diff --git a/superset/views/sql_lab.py b/superset/views/sql_lab.py index 49336a84a18d6..0e17f46f16f07 100644 --- a/superset/views/sql_lab.py +++ b/superset/views/sql_lab.py @@ -22,7 +22,7 @@ from flask_babel import lazy_gettext as _ from sqlalchemy import and_ -from superset import db, is_feature_enabled +from superset import db from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.models.sql_lab import Query, SavedQuery, TableSchema, TabState from superset.superset_typing import FlaskResponse @@ -79,9 +79,6 @@ class SavedQueryView(SupersetModelView, DeleteMixin): @expose("/list/") @has_access def list(self) -> FlaskResponse: - if not is_feature_enabled("ENABLE_REACT_CRUD_VIEWS"): - return super().list() - return super().render_app_template() def pre_add(self, item: "SavedQueryView") -> None: diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index 26674054ae394..f573e68075441 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -428,17 +428,6 @@ def test_slices(self): resp = self.client.get(url) self.assertEqual(resp.status_code, 200) - def test_tablemodelview_list(self): - self.login(username="admin") - - url = "/tablemodelview/list/" - resp = self.get_resp(url) - - # assert that a table is listed - table = db.session.query(SqlaTable).first() - assert table.name in resp - assert "/superset/explore/table/{}".format(table.id) in resp - def test_add_slice(self): self.login(username="admin") # assert that /chart/add responds with 200 diff --git a/tests/integration_tests/csv_upload_tests.py b/tests/integration_tests/csv_upload_tests.py index d5da25c38c115..c9bc11db98557 100644 --- a/tests/integration_tests/csv_upload_tests.py +++ b/tests/integration_tests/csv_upload_tests.py @@ -203,318 +203,3 @@ def mock_upload_to_s3(filename: str, upload_prefix: str, table: Table) -> str: container.exec_run(f"hdfs dfs -put {src} {dest}") # hive external table expectes a directory for the location return dest_dir - - -@pytest.mark.usefixtures("setup_csv_upload") -@pytest.mark.usefixtures("create_csv_files") -@mock.patch( - "superset.models.core.config", - {**app.config, "ALLOWED_USER_CSV_SCHEMA_FUNC": lambda d, u: ["admin_database"]}, -) -@mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3) -@mock.patch("superset.views.database.views.event_logger.log_with_context") -def test_import_csv_enforced_schema(mock_event_logger): - if utils.backend() == "sqlite": - pytest.skip("Sqlite doesn't support schema / database creation") - - full_table_name = f"admin_database.{CSV_UPLOAD_TABLE_W_SCHEMA}" - - # Invalid table name - resp = upload_csv(CSV_FILENAME1, full_table_name) - assert "Table name cannot contain a schema" in resp - - # no schema specified, fail upload - resp = upload_csv(CSV_FILENAME1, CSV_UPLOAD_TABLE_W_SCHEMA, extra={"schema": None}) - assert ( - f'Database "{CSV_UPLOAD_DATABASE}" schema "None" is not allowed for csv uploads' - in resp - ) - - success_msg = f'CSV file "{CSV_FILENAME1}" uploaded to table "{full_table_name}"' - resp = upload_csv( - CSV_FILENAME1, - CSV_UPLOAD_TABLE_W_SCHEMA, - extra={"schema": "admin_database", "if_exists": "replace"}, - ) - assert success_msg in resp - mock_event_logger.assert_called_with( - action="successful_csv_upload", - database=get_upload_db().name, - schema="admin_database", - table=CSV_UPLOAD_TABLE_W_SCHEMA, - ) - - engine = get_upload_db().get_sqla_engine() - data = engine.execute( - f"SELECT * from {ADMIN_SCHEMA_NAME}.{CSV_UPLOAD_TABLE_W_SCHEMA}" - ).fetchall() - assert data == [("john", 1), ("paul", 2)] - - # user specified schema doesn't match, fail - resp = upload_csv( - CSV_FILENAME1, CSV_UPLOAD_TABLE_W_SCHEMA, extra={"schema": "gold"} - ) - assert ( - f'Database "{CSV_UPLOAD_DATABASE}" schema "gold" is not allowed for csv uploads' - in resp - ) - - # user specified schema matches the expected schema, append - if utils.backend() == "hive": - pytest.skip("Hive database doesn't support append csv uploads.") - resp = upload_csv( - CSV_FILENAME1, - CSV_UPLOAD_TABLE_W_SCHEMA, - extra={"schema": "admin_database", "if_exists": "append"}, - ) - assert success_msg in resp - - -@mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3) -def test_import_csv_explore_database(setup_csv_upload, create_csv_files): - schema = utils.get_example_default_schema() - full_table_name = ( - f"{schema}.{CSV_UPLOAD_TABLE_W_EXPLORE}" - if schema - else CSV_UPLOAD_TABLE_W_EXPLORE - ) - - if utils.backend() == "sqlite": - pytest.skip("Sqlite doesn't support schema / database creation") - - resp = upload_csv(CSV_FILENAME1, CSV_UPLOAD_TABLE_W_EXPLORE) - assert f'CSV file "{CSV_FILENAME1}" uploaded to table "{full_table_name}"' in resp - table = SupersetTestCase.get_table(name=CSV_UPLOAD_TABLE_W_EXPLORE) - assert table.database_id == superset.utils.database.get_example_database().id - - -@pytest.mark.usefixtures("setup_csv_upload") -@pytest.mark.usefixtures("create_csv_files") -@mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3) -@mock.patch("superset.views.database.views.event_logger.log_with_context") -def test_import_csv(mock_event_logger): - schema = utils.get_example_default_schema() - full_table_name = f"{schema}.{CSV_UPLOAD_TABLE}" if schema else CSV_UPLOAD_TABLE - success_msg_f1 = f'CSV file "{CSV_FILENAME1}" uploaded to table "{full_table_name}"' - - test_db = get_upload_db() - - # initial upload with fail mode - resp = upload_csv(CSV_FILENAME1, CSV_UPLOAD_TABLE) - assert success_msg_f1 in resp - - # upload again with fail mode; should fail - fail_msg = ( - f'Unable to upload CSV file "{CSV_FILENAME1}" to table "{CSV_UPLOAD_TABLE}"' - ) - resp = upload_csv(CSV_FILENAME1, CSV_UPLOAD_TABLE) - assert fail_msg in resp - - if utils.backend() != "hive": - # upload again with append mode - resp = upload_csv( - CSV_FILENAME1, CSV_UPLOAD_TABLE, extra={"if_exists": "append"} - ) - assert success_msg_f1 in resp - mock_event_logger.assert_called_with( - action="successful_csv_upload", - database=test_db.name, - schema=schema, - table=CSV_UPLOAD_TABLE, - ) - - # upload again with replace mode and specific columns - resp = upload_csv( - CSV_FILENAME1, - CSV_UPLOAD_TABLE, - extra={"if_exists": "replace", "usecols": '["a"]'}, - ) - assert success_msg_f1 in resp - - # make sure only specified column name was read - table = SupersetTestCase.get_table(name=CSV_UPLOAD_TABLE) - assert "b" not in table.column_names - - # upload again with replace mode - resp = upload_csv(CSV_FILENAME1, CSV_UPLOAD_TABLE, extra={"if_exists": "replace"}) - assert success_msg_f1 in resp - - # try to append to table from file with different schema - resp = upload_csv(CSV_FILENAME2, CSV_UPLOAD_TABLE, extra={"if_exists": "append"}) - fail_msg_f2 = ( - f'Unable to upload CSV file "{CSV_FILENAME2}" to table "{CSV_UPLOAD_TABLE}"' - ) - assert fail_msg_f2 in resp - - # replace table from file with different schema - resp = upload_csv(CSV_FILENAME2, CSV_UPLOAD_TABLE, extra={"if_exists": "replace"}) - success_msg_f2 = f'CSV file "{CSV_FILENAME2}" uploaded to table "{full_table_name}"' - assert success_msg_f2 in resp - - table = SupersetTestCase.get_table(name=CSV_UPLOAD_TABLE) - # make sure the new column name is reflected in the table metadata - assert "d" in table.column_names - - # ensure user is assigned as an owner - assert security_manager.find_user("admin") in table.owners - - # null values are set - upload_csv( - CSV_FILENAME2, - CSV_UPLOAD_TABLE, - extra={"null_values": '["", "john"]', "if_exists": "replace"}, - ) - # make sure that john and empty string are replaced with None - engine = test_db.get_sqla_engine() - data = engine.execute(f"SELECT * from {CSV_UPLOAD_TABLE}").fetchall() - assert data == [(None, 1, "x"), ("paul", 2, None)] - - # default null values - upload_csv(CSV_FILENAME2, CSV_UPLOAD_TABLE, extra={"if_exists": "replace"}) - # make sure that john and empty string are replaced with None - data = engine.execute(f"SELECT * from {CSV_UPLOAD_TABLE}").fetchall() - assert data == [("john", 1, "x"), ("paul", 2, None)] - - -@pytest.mark.usefixtures("setup_csv_upload") -@pytest.mark.usefixtures("create_excel_files") -@mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3) -@mock.patch("superset.views.database.views.event_logger.log_with_context") -def test_import_excel(mock_event_logger): - if utils.backend() == "hive": - pytest.skip("Hive doesn't excel upload.") - - schema = utils.get_example_default_schema() - full_table_name = f"{schema}.{EXCEL_UPLOAD_TABLE}" if schema else EXCEL_UPLOAD_TABLE - test_db = get_upload_db() - - success_msg = f'Excel file "{EXCEL_FILENAME}" uploaded to table "{full_table_name}"' - - # initial upload with fail mode - resp = upload_excel(EXCEL_FILENAME, EXCEL_UPLOAD_TABLE) - assert success_msg in resp - mock_event_logger.assert_called_with( - action="successful_excel_upload", - database=test_db.name, - schema=schema, - table=EXCEL_UPLOAD_TABLE, - ) - - # ensure user is assigned as an owner - table = SupersetTestCase.get_table(name=EXCEL_UPLOAD_TABLE) - assert security_manager.find_user("admin") in table.owners - - # upload again with fail mode; should fail - fail_msg = f'Unable to upload Excel file "{EXCEL_FILENAME}" to table "{EXCEL_UPLOAD_TABLE}"' - resp = upload_excel(EXCEL_FILENAME, EXCEL_UPLOAD_TABLE) - assert fail_msg in resp - - if utils.backend() != "hive": - # upload again with append mode - resp = upload_excel( - EXCEL_FILENAME, EXCEL_UPLOAD_TABLE, extra={"if_exists": "append"} - ) - assert success_msg in resp - - # upload again with replace mode - resp = upload_excel( - EXCEL_FILENAME, EXCEL_UPLOAD_TABLE, extra={"if_exists": "replace"} - ) - assert success_msg in resp - mock_event_logger.assert_called_with( - action="successful_excel_upload", - database=test_db.name, - schema=schema, - table=EXCEL_UPLOAD_TABLE, - ) - - # make sure that john and empty string are replaced with None - data = ( - test_db.get_sqla_engine() - .execute(f"SELECT * from {EXCEL_UPLOAD_TABLE}") - .fetchall() - ) - assert data == [(0, "john", 1), (1, "paul", 2)] - - -@pytest.mark.usefixtures("setup_csv_upload") -@pytest.mark.usefixtures("create_columnar_files") -@mock.patch("superset.db_engine_specs.hive.upload_to_s3", mock_upload_to_s3) -@mock.patch("superset.views.database.views.event_logger.log_with_context") -def test_import_parquet(mock_event_logger): - if utils.backend() == "hive": - pytest.skip("Hive doesn't allow parquet upload.") - - schema = utils.get_example_default_schema() - full_table_name = ( - f"{schema}.{PARQUET_UPLOAD_TABLE}" if schema else PARQUET_UPLOAD_TABLE - ) - test_db = get_upload_db() - - success_msg_f1 = f'Columnar file "[\'{PARQUET_FILENAME1}\']" uploaded to table "{full_table_name}"' - - # initial upload with fail mode - resp = upload_columnar(PARQUET_FILENAME1, PARQUET_UPLOAD_TABLE) - assert success_msg_f1 in resp - - # upload again with fail mode; should fail - fail_msg = f'Unable to upload Columnar file "[\'{PARQUET_FILENAME1}\']" to table "{PARQUET_UPLOAD_TABLE}"' - resp = upload_columnar(PARQUET_FILENAME1, PARQUET_UPLOAD_TABLE) - assert fail_msg in resp - - if utils.backend() != "hive": - # upload again with append mode - resp = upload_columnar( - PARQUET_FILENAME1, PARQUET_UPLOAD_TABLE, extra={"if_exists": "append"} - ) - assert success_msg_f1 in resp - mock_event_logger.assert_called_with( - action="successful_columnar_upload", - database=test_db.name, - schema=schema, - table=PARQUET_UPLOAD_TABLE, - ) - - # upload again with replace mode and specific columns - resp = upload_columnar( - PARQUET_FILENAME1, - PARQUET_UPLOAD_TABLE, - extra={"if_exists": "replace", "usecols": '["a"]'}, - ) - assert success_msg_f1 in resp - - table = SupersetTestCase.get_table(name=PARQUET_UPLOAD_TABLE, schema=None) - # make sure only specified column name was read - assert "b" not in table.column_names - - # ensure user is assigned as an owner - assert security_manager.find_user("admin") in table.owners - - # upload again with replace mode - resp = upload_columnar( - PARQUET_FILENAME1, PARQUET_UPLOAD_TABLE, extra={"if_exists": "replace"} - ) - assert success_msg_f1 in resp - - data = ( - test_db.get_sqla_engine() - .execute(f"SELECT * from {PARQUET_UPLOAD_TABLE} ORDER BY b") - .fetchall() - ) - assert data == [("john", 1), ("paul", 2)] - - # replace table with zip file - resp = upload_columnar( - ZIP_FILENAME, PARQUET_UPLOAD_TABLE, extra={"if_exists": "replace"} - ) - success_msg_f2 = ( - f'Columnar file "[\'{ZIP_FILENAME}\']" uploaded to table "{full_table_name}"' - ) - assert success_msg_f2 in resp - - data = ( - test_db.get_sqla_engine() - .execute(f"SELECT * from {PARQUET_UPLOAD_TABLE} ORDER BY b") - .fetchall() - ) - assert data == [("john", 1), ("paul", 2), ("max", 3), ("bob", 4)] diff --git a/tests/integration_tests/dashboards/security/base_case.py b/tests/integration_tests/dashboards/security/base_case.py index 75e8772b59a4f..bbb5fad831166 100644 --- a/tests/integration_tests/dashboards/security/base_case.py +++ b/tests/integration_tests/dashboards/security/base_case.py @@ -16,6 +16,7 @@ # under the License. from typing import List, Optional +import pytest from flask import escape, Response from superset.models.dashboard import Dashboard @@ -32,31 +33,6 @@ def assert_dashboard_api_response( self.assert200(response) assert response.json["id"] == dashboard_to_access.id - def assert_dashboards_list_view_response( - self, - response: Response, - expected_counts: int, - expected_dashboards: Optional[List[Dashboard]] = None, - not_expected_dashboards: Optional[List[Dashboard]] = None, - ) -> None: - self.assert200(response) - response_html = response.data.decode("utf-8") - if expected_counts == 0: - assert "No records found" in response_html - else: - # # a way to parse number of dashboards returns - # in the list view as an html response - assert ( - "Record Count: {count}".format(count=str(expected_counts)) - in response_html - ) - expected_dashboards = expected_dashboards or [] - for dashboard in expected_dashboards: - assert dashboard.url in response_html - not_expected_dashboards = not_expected_dashboards or [] - for dashboard in not_expected_dashboards: - assert dashboard.url not in response_html - def assert_dashboards_api_response( self, response: Response, diff --git a/tests/integration_tests/dashboards/security/security_rbac_tests.py b/tests/integration_tests/dashboards/security/security_rbac_tests.py index 5a1f02f1e3243..62b0e1b4c2755 100644 --- a/tests/integration_tests/dashboards/security/security_rbac_tests.py +++ b/tests/integration_tests/dashboards/security/security_rbac_tests.py @@ -198,36 +198,6 @@ def test_get_dashboard_view__public_user_access_with_dashboard_permission(self): # post revoke_access_to_dashboard(dashboard_to_access, "Public") - def test_get_dashboards_list__admin_get_all_dashboards(self): - # arrange - create_dashboard_to_db( - owners=[], slices=[create_slice_to_db()], published=False - ) - dashboard_counts = count_dashboards() - - self.login("admin") - - # act - response = self.get_dashboards_list_response() - - # assert - self.assert_dashboards_list_view_response(response, dashboard_counts) - - def test_get_dashboards_list__owner_get_all_owned_dashboards(self): - # arrange - ( - not_owned_dashboards, - owned_dashboards, - ) = self._create_sample_dashboards_with_owner_access() - - # act - response = self.get_dashboards_list_response() - - # assert - self.assert_dashboards_list_view_response( - response, 2, owned_dashboards, not_owned_dashboards - ) - def _create_sample_dashboards_with_owner_access(self): username = random_str() new_role = f"role_{random_str()}" @@ -251,42 +221,6 @@ def _create_sample_dashboards_with_owner_access(self): self.login(username) return not_owned_dashboards, owned_dashboards - def test_get_dashboards_list__user_without_any_permissions_get_empty_list(self): - - # arrange - username = random_str() - new_role = f"role_{random_str()}" - self.create_user_with_roles(username, [new_role], should_create_roles=True) - - create_dashboard_to_db(published=True) - self.login(username) - - # act - response = self.get_dashboards_list_response() - - # assert - self.assert_dashboards_list_view_response(response, 0) - - def test_get_dashboards_list__user_get_only_published_permitted_dashboards(self): - # arrange - ( - new_role, - draft_dashboards, - published_dashboards, - ) = self._create_sample_only_published_dashboard_with_roles() - - # act - response = self.get_dashboards_list_response() - - # assert - self.assert_dashboards_list_view_response( - response, len(published_dashboards), published_dashboards, draft_dashboards, - ) - - # post - for dash in published_dashboards + draft_dashboards: - revoke_access_to_dashboard(dash, new_role) - def _create_sample_only_published_dashboard_with_roles(self): username = random_str() new_role = f"role_{random_str()}" @@ -304,49 +238,6 @@ def _create_sample_only_published_dashboard_with_roles(self): self.login(username) return new_role, draft_dashboards, published_dashboards - @pytest.mark.usefixtures("public_role_like_gamma") - def test_get_dashboards_list__public_user_without_any_permissions_get_empty_list( - self, - ): - create_dashboard_to_db(published=True) - - # act - response = self.get_dashboards_list_response() - - # assert - self.assert_dashboards_list_view_response(response, 0) - - @pytest.mark.usefixtures("public_role_like_gamma") - def test_get_dashboards_list__public_user_get_only_published_permitted_dashboards( - self, - ): - # arrange - published_dashboards = [ - create_dashboard_to_db(published=True), - create_dashboard_to_db(published=True), - ] - draft_dashboards = [ - create_dashboard_to_db(published=False), - create_dashboard_to_db(published=False), - ] - - for dash in published_dashboards + draft_dashboards: - grant_access_to_dashboard(dash, "Public") - - self.logout() - - # act - response = self.get_dashboards_list_response() - - # assert - self.assert_dashboards_list_view_response( - response, len(published_dashboards), published_dashboards, draft_dashboards, - ) - - # post - for dash in published_dashboards + draft_dashboards: - revoke_access_to_dashboard(dash, "Public") - def test_get_dashboards_api__admin_get_all_dashboards(self): # arrange create_dashboard_to_db( diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index 7626de677bf01..fe493a5504aed 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -290,12 +290,11 @@ def pg_test_query_parameter(query_parameter, expected_response): ) ) schema_values = [ - "admin_database", "information_schema", "public", ] expected_response = { - "count": 3, + "count": 2, "result": [{"text": val, "value": val} for val in schema_values], } self.login(username="admin") @@ -321,8 +320,10 @@ def pg_test_query_parameter(query_parameter, expected_response): pg_test_query_parameter( query_parameter, { - "count": 3, - "result": [{"text": "admin_database", "value": "admin_database"}], + "count": 2, + "result": [ + {"text": "information_schema", "value": "information_schema"} + ], }, ) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index d54e400817e62..d8f608ecfc2b3 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -553,24 +553,6 @@ def test_gamma_user_schema_access_to_dashboards(self): self.assertIn("/superset/dashboard/world_health/", data) self.assertNotIn("/superset/dashboard/births/", data) - def test_gamma_user_schema_access_to_tables(self): - self.login(username="gamma") - data = str(self.client.get("tablemodelview/list/").data) - self.assertIn("wb_health_population", data) - self.assertNotIn("birth_names", data) - - @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") - def test_gamma_user_schema_access_to_charts(self): - self.login(username="gamma") - data = str(self.client.get("api/v1/chart/").data) - self.assertIn( - "Life Expectancy VS Rural %", data - ) # wb_health_population slice, has access - self.assertIn( - "Parallel Coordinates", data - ) # wb_health_population slice, has access - self.assertNotIn("Girl Name Cloud", data) # birth_names slice, no access - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @pytest.mark.usefixtures("public_role_like_gamma") def test_public_sync_role_data_perms(self): diff --git a/tests/integration_tests/superset_test_config.py b/tests/integration_tests/superset_test_config.py index 7c862328294b5..983476490fe13 100644 --- a/tests/integration_tests/superset_test_config.py +++ b/tests/integration_tests/superset_test_config.py @@ -61,7 +61,6 @@ "KV_STORE": True, "SHARE_QUERIES_VIA_KV_STORE": True, "ENABLE_TEMPLATE_PROCESSING": True, - "ENABLE_REACT_CRUD_VIEWS": os.environ.get("ENABLE_REACT_CRUD_VIEWS", False), "ALERT_REPORTS": True, "DASHBOARD_NATIVE_FILTERS": True, } diff --git a/tox.ini b/tox.ini index 88a51278ec2ae..774aa681b6396 100644 --- a/tox.ini +++ b/tox.ini @@ -56,7 +56,6 @@ setenv = SUPERSET_TESTENV = true SUPERSET_CONFIG = tests.integration_tests.superset_test_config SUPERSET_HOME = {envtmpdir} - ENABLE_REACT_CRUD_VIEWS = true commands = npm install -g npm@'>=6.5.0' pip install -e {toxinidir}/ @@ -70,7 +69,6 @@ setenv = SUPERSET_TESTENV = true SUPERSET_CONFIG = tests.integration_tests.superset_test_config SUPERSET_HOME = {envtmpdir} - ENABLE_REACT_CRUD_VIEWS = true commands = npm install -g npm@'>=6.5.0' pip install -e {toxinidir}/ @@ -84,7 +82,6 @@ setenv = SUPERSET_TESTENV = true SUPERSET_CONFIG = tests.integration_tests.superset_test_config SUPERSET_HOME = {envtmpdir} - ENABLE_REACT_CRUD_VIEWS = true commands = npm install -g npm@'>=6.5.0' pip install -e {toxinidir}/ @@ -98,7 +95,6 @@ setenv = SUPERSET_TESTENV = true SUPERSET_CONFIG = tests.integration_tests.superset_test_config SUPERSET_HOME = {envtmpdir} - ENABLE_REACT_CRUD_VIEWS = true commands = npm install -g npm@'>=6.5.0' pip install -e {toxinidir}/ @@ -112,7 +108,6 @@ setenv = SUPERSET_TESTENV = true SUPERSET_CONFIG = tests.integration_tests.superset_test_config SUPERSET_HOME = {envtmpdir} - ENABLE_REACT_CRUD_VIEWS = true commands = npm install -g npm@'>=6.5.0' pip install -e {toxinidir}/