chore!: remove ENABLE_REACT_CRUD_VIEWS feature flag (permanently en…

…able) (#19231)

* remove ENABLE_REACT_CRUD_VIEWS feature flag

* docs

* deal with problematic tests

* empty test suite

* skip test

* test conditions changed

* removing the tests instead of skipping

.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 }}

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

RESOURCES/FEATURE_FLAGS.md

@@ -63,4 +63,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

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.

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
 #

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

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

superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts

@@ -31,7 +31,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',

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 = (
-        <QuerySearch
-          actions={this.props.actions}
-          displayLimit={this.props.common.conf.DISPLAY_MAX_ROW}
-        />
-      );
-    } else {
-      content = (
-        <>
-          <QueryAutoRefresh />
-          <TabbedSqlEditors />
-        </>
-      );
+      return window.location.replace('/superset/sqllab/history/');
     }
     return (
       <ThemeProvider theme={supersetTheme}>
         <div className="App SqlLab">
-          {content}
+          <QueryAutoRefresh />
+          <TabbedSqlEditors />
           <ToastContainer />
         </div>
       </ThemeProvider>

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();
-  });
-});

superset-frontend/src/components/Datasource/DatasourceModal.tsx

@@ -183,9 +183,9 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
     });
   };
 
-  const showLegacyDatasourceEditor =
-    isFeatureEnabled(FeatureFlag.ENABLE_REACT_CRUD_VIEWS) &&
-    !isFeatureEnabled(FeatureFlag.DISABLE_LEGACY_DATASOURCE_EDITOR);
+  const showLegacyDatasourceEditor = !isFeatureEnabled(
+    FeatureFlag.DISABLE_LEGACY_DATASOURCE_EDITOR,
+  );
 
   return (
     <StyledDatasourceModal

superset-frontend/src/views/routes.tsx

@@ -16,7 +16,6 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
 import React, { lazy } from 'react';
 
 // not lazy loaded since this is the home page.
@@ -182,7 +181,6 @@ const frontEndRoutes = routes
   );
 
 export function isFrontendRoute(path?: string) {
-  if (!isFeatureEnabled(FeatureFlag.ENABLE_REACT_CRUD_VIEWS)) return false;
   if (path) {
     const basePath = path.split(/[?#]/)[0]; // strip out query params and link bookmarks
     return !!frontEndRoutes[basePath];

superset/config.py

@@ -393,11 +393,6 @@ def _try_json_readsha(filepath: str, length: int) -> 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

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()

superset/views/alerts.py

@@ -92,21 +92,15 @@ 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()
 
     @expose("/<pk>/log/", methods=["GET"])
     @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()

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("/<pk>/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()

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()
 
 

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

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()
 
 

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")

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()
 
 

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:

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