refactor: Removes the deprecated KV Store feature

RESOURCES/FEATURE_FLAGS.md

@@ -28,9 +28,7 @@ These features are considered **unfinished** and should only be used on developm
 [//]: # "PLEASE KEEP THE LIST SORTED ALPHABETICALLY"
 
 - ENABLE_ADVANCED_DATA_TYPES
-- KV_STORE
 - PRESTO_EXPAND_DATA
-- SHARE_QUERIES_VIA_KV_STORE
 - TAGGING_SYSTEM
 
 ## In Testing

UPDATING.md

@@ -29,6 +29,8 @@ assists people when migrating to a new version.
 
 ### Breaking Changes
 
+- [26376](https://github.com/apache/superset/pull/26376): Removes the deprecated KV Store feature and its related assets such as the API endpoint, the `keyvalue` table, and the `KV_STORE` and `SHARE_QUERIES_VIA_KV_STORE` feature flags. The previous value of both feature flags was `False` and now the feature is permanently removed.
+
 ### Potential Downtime
 
 ### Other

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

@@ -53,7 +53,6 @@ export enum FeatureFlag {
   HORIZONTAL_FILTER_BAR = 'HORIZONTAL_FILTER_BAR',
   LISTVIEWS_DEFAULT_CARD_VIEW = 'LISTVIEWS_DEFAULT_CARD_VIEW',
   SCHEDULED_QUERIES = 'SCHEDULED_QUERIES',
-  SHARE_QUERIES_VIA_KV_STORE = 'SHARE_QUERIES_VIA_KV_STORE',
   SQLLAB_BACKEND_PERSISTENCE = 'SQLLAB_BACKEND_PERSISTENCE',
   SQL_VALIDATORS_BY_ENGINE = 'SQL_VALIDATORS_BY_ENGINE',
   THUMBNAILS = 'THUMBNAILS',

superset-frontend/src/SqlLab/actions/sqlLab.js

@@ -1160,24 +1160,6 @@ export function persistEditorHeight(queryEditor, northPercent, southPercent) {
   };
 }
 
-export function popStoredQuery(urlId) {
-  return function (dispatch) {
-    return SupersetClient.get({ endpoint: `/kv/${urlId}` })
-      .then(({ json }) =>
-        dispatch(
-          addQueryEditor({
-            name: json.name ? json.name : t('Shared query'),
-            dbId: json.dbId ? parseInt(json.dbId, 10) : null,
-            schema: json.schema ? json.schema : null,
-            autorun: json.autorun ? json.autorun : false,
-            sql: json.sql ? json.sql : 'SELECT ...',
-            templateParams: json.templateParams,
-          }),
-        ),
-      )
-      .catch(() => dispatch(addDangerToast(ERR_MSG_CANT_LOAD_QUERY)));
-  };
-}
 export function popSavedQuery(saveQueryId) {
   return function (dispatch) {
     return SupersetClient.get({

superset-frontend/src/SqlLab/components/ShareSqlLabQuery/ShareSqlLabQuery.test.tsx

@@ -19,14 +19,10 @@
 import React from 'react';
 import configureStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
-import fetchMock from 'fetch-mock';
-import * as uiCore from '@superset-ui/core';
 import { Provider } from 'react-redux';
 import { supersetTheme, ThemeProvider } from '@superset-ui/core';
-import { render, screen, act } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 import '@testing-library/jest-dom/extend-expect';
-import userEvent from '@testing-library/user-event';
-import * as utils from 'src/utils/common';
 import ShareSqlLabQuery from 'src/SqlLab/components/ShareSqlLabQuery';
 import { initialState } from 'src/SqlLab/fixtures';
 
@@ -57,130 +53,21 @@ const mockState = {
   },
 };
 const store = mockStore(mockState);
-let isFeatureEnabledMock: jest.SpyInstance;
 
 const standardProvider: React.FC = ({ children }) => (
   <ThemeProvider theme={supersetTheme}>
     <Provider store={store}>{children}</Provider>
   </ThemeProvider>
 );
 
-const unsavedQueryEditor = {
-  id: defaultProps.queryEditorId,
-  dbId: 9888,
-  name: 'query title changed',
-  schema: 'query_schema_updated',
-  sql: 'SELECT * FROM Updated Limit 100',
-  autorun: true,
-  templateParams: '{ "my_value": "foo" }',
-};
-
-const standardProviderWithUnsaved: React.FC = ({ children }) => (
-  <ThemeProvider theme={supersetTheme}>
-    <Provider
-      store={mockStore({
-        ...initialState,
-        sqlLab: {
-          ...initialState.sqlLab,
-          unsavedQueryEditor,
-        },
-      })}
-    >
-      {children}
-    </Provider>
-  </ThemeProvider>
-);
-
-describe('ShareSqlLabQuery', () => {
-  const storeQueryUrl = 'glob:*/kv/store/';
-  const storeQueryMockId = '123';
-
-  beforeEach(async () => {
-    fetchMock.post(storeQueryUrl, () => ({ id: storeQueryMockId }), {
-      overwriteRoutes: true,
-    });
-    fetchMock.resetHistory();
-    jest.clearAllMocks();
-  });
-
-  afterAll(fetchMock.reset);
-
-  describe('via /kv/store', () => {
-    beforeAll(() => {
-      isFeatureEnabledMock = jest
-        .spyOn(uiCore, 'isFeatureEnabled')
-        .mockImplementation(() => true);
-    });
-
-    afterAll(() => {
-      isFeatureEnabledMock.mockReset();
-    });
-
-    it('calls storeQuery() with the query when getCopyUrl() is called', async () => {
-      await act(async () => {
-        render(<ShareSqlLabQuery {...defaultProps} />, {
-          wrapper: standardProvider,
-        });
-      });
-      const button = screen.getByRole('button');
-      const { id, remoteId, ...expected } = mockQueryEditor;
-      const storeQuerySpy = jest.spyOn(utils, 'storeQuery');
-      userEvent.click(button);
-      expect(storeQuerySpy.mock.calls).toHaveLength(1);
-      expect(storeQuerySpy).toBeCalledWith(expected);
-      storeQuerySpy.mockRestore();
-    });
-
-    it('calls storeQuery() with unsaved changes', async () => {
-      await act(async () => {
-        render(<ShareSqlLabQuery {...defaultProps} />, {
-          wrapper: standardProviderWithUnsaved,
-        });
-      });
-      const button = screen.getByRole('button');
-      const { id, ...expected } = unsavedQueryEditor;
-      const storeQuerySpy = jest.spyOn(utils, 'storeQuery');
-      userEvent.click(button);
-      expect(storeQuerySpy.mock.calls).toHaveLength(1);
-      expect(storeQuerySpy).toBeCalledWith(expected);
-      storeQuerySpy.mockRestore();
-    });
-  });
-
-  describe('via saved query', () => {
-    beforeAll(() => {
-      isFeatureEnabledMock = jest
-        .spyOn(uiCore, 'isFeatureEnabled')
-        .mockImplementation(() => false);
-    });
-
-    afterAll(() => {
-      isFeatureEnabledMock.mockReset();
-    });
-
-    it('does not call storeQuery() with the query when getCopyUrl() is called and feature is not enabled', async () => {
-      await act(async () => {
-        render(<ShareSqlLabQuery {...defaultProps} />, {
-          wrapper: standardProvider,
-        });
-      });
-      const storeQuerySpy = jest.spyOn(utils, 'storeQuery');
-      const button = screen.getByRole('button');
-      userEvent.click(button);
-      expect(storeQuerySpy.mock.calls).toHaveLength(0);
-      storeQuerySpy.mockRestore();
-    });
-
-    it('button is disabled and there is a request to save the query', async () => {
-      const updatedProps = {
-        queryEditorId: disabled.id,
-      };
+test('button is disabled and there is a request to save the query', async () => {
+  const updatedProps = {
+    queryEditorId: disabled.id,
+  };
 
-      render(<ShareSqlLabQuery {...updatedProps} />, {
-        wrapper: standardProvider,
-      });
-      const button = await screen.findByRole('button', { name: /copy link/i });
-      expect(button).toBeDisabled();
-    });
+  render(<ShareSqlLabQuery {...updatedProps} />, {
+    wrapper: standardProvider,
   });
+  const button = await screen.findByRole('button', { name: /copy link/i });
+  expect(button).toBeDisabled();
 });

superset-frontend/src/SqlLab/components/ShareSqlLabQuery/index.tsx

@@ -17,24 +17,15 @@
  * under the License.
  */
 import React from 'react';
-import {
-  FeatureFlag,
-  styled,
-  t,
-  useTheme,
-  isFeatureEnabled,
-} from '@superset-ui/core';
+import { styled, t, useTheme } from '@superset-ui/core';
 import Button from 'src/components/Button';
 import Icons from 'src/components/Icons';
 import withToasts from 'src/components/MessageToasts/withToasts';
 import CopyToClipboard from 'src/components/CopyToClipboard';
-import { storeQuery } from 'src/utils/common';
-import { getClientErrorObject } from 'src/utils/getClientErrorObject';
 import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor';
 
 interface ShareSqlLabQueryProps {
   queryEditorId: string;
-  addDangerToast: (msg: string) => void;
 }
 
 const StyledIcon = styled(Icons.Link)`
@@ -47,36 +38,10 @@ const StyledIcon = styled(Icons.Link)`
   }
 `;
 
-const ShareSqlLabQuery = ({
-  queryEditorId,
-  addDangerToast,
-}: ShareSqlLabQueryProps) => {
+const ShareSqlLabQuery = ({ queryEditorId }: ShareSqlLabQueryProps) => {
   const theme = useTheme();
 
-  const { dbId, name, schema, autorun, sql, remoteId, templateParams } =
-    useQueryEditor(queryEditorId, [
-      'dbId',
-      'name',
-      'schema',
-      'autorun',
-      'sql',
-      'remoteId',
-      'templateParams',
-    ]);
-
-  const getCopyUrlForKvStore = (callback: Function) => {
-    const sharedQuery = { dbId, name, schema, autorun, sql, templateParams };
-
-    return storeQuery(sharedQuery)
-      .then(shortUrl => {
-        callback(shortUrl);
-      })
-      .catch(response => {
-        getClientErrorObject(response).then(() => {
-          addDangerToast(t('There was an error with your request'));
-        });
-      });
-  };
+  const { remoteId } = useQueryEditor(queryEditorId, ['remoteId']);
 
   const getCopyUrlForSavedQuery = (callback: Function) => {
     let savedQueryToastContent;
@@ -91,12 +56,7 @@ const ShareSqlLabQuery = ({
       callback(savedQueryToastContent);
     }
   };
-  const getCopyUrl = (callback: Function) => {
-    if (isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE)) {
-      return getCopyUrlForKvStore(callback);
-    }
-    return getCopyUrlForSavedQuery(callback);
-  };
+  const getCopyUrl = (callback: Function) => getCopyUrlForSavedQuery(callback);
 
   const buildButton = (canShare: boolean) => {
     const tooltip = canShare
@@ -115,8 +75,7 @@ const ShareSqlLabQuery = ({
     );
   };
 
-  const canShare =
-    !!remoteId || isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE);
+  const canShare = !!remoteId;
 
   return (
     <>

superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx

@@ -35,7 +35,6 @@ import { newQueryTabName } from 'src/SqlLab/utils/newQueryTabName';
 
 fetchMock.get('glob:*/api/v1/database/*', {});
 fetchMock.get('glob:*/api/v1/saved_query/*', {});
-fetchMock.get('glob:*/kv/*', {});
 
 describe('TabbedSqlEditors', () => {
   const middlewares = [thunk];
@@ -107,11 +106,6 @@ describe('TabbedSqlEditors', () => {
       window.history.replaceState.restore();
       uriStub.restore();
     });
-    it('should handle id', async () => {
-      uriStub.returns({ id: 1 });
-      await mountWithAct();
-      expect(window.history.replaceState.getCall(0).args[2]).toBe('/sqllab');
-    });
     it('should handle savedQueryId', async () => {
       uriStub.returns({ savedQueryId: 1 });
       await mountWithAct();

superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx

@@ -120,7 +120,6 @@ class TabbedSqlEditors extends React.PureComponent {
     const bootstrapData = getBootstrapData();
     const queryParameters = URI(window.location).search(true);
     const {
-      id,
       name,
       sql,
       savedQueryId,
@@ -139,10 +138,8 @@ class TabbedSqlEditors extends React.PureComponent {
     };
 
     // Popping a new tab based on the querystring
-    if (id || sql || savedQueryId || datasourceKey || queryId) {
-      if (id) {
-        this.props.actions.popStoredQuery(id);
-      } else if (savedQueryId) {
+    if (sql || savedQueryId || datasourceKey || queryId) {
+      if (savedQueryId) {
         this.props.actions.popSavedQuery(savedQueryId);
       } else if (queryId) {
         this.props.actions.popQuery(queryId);

superset-frontend/src/features/datasets/AddDataset/EditDataset/UsageTab/UsageTab.test.tsx

@@ -352,7 +352,7 @@ test('show chart dashboards', async () => {
 
 test('paginates', async () => {
   const charts = [];
-  for (let i = 0; i < 65; i += 1) {
+  for (let i = 0; i < 40; i += 1) {
     charts.push(
       getMockChart({
         id: i + 1,
@@ -384,22 +384,7 @@ test('paginates', async () => {
     name: /sample chart/i,
   });
 
-  expect(chartNameValues).toHaveLength(25);
-  expect(chartNameValues[0]).toHaveTextContent('Sample chart 26');
-  expect(chartNameValues[24]).toHaveTextContent('Sample chart 50');
-
-  // Third page
-  userEvent.click(
-    screen.getByRole('button', {
-      name: /right/i,
-    }),
-  );
-
-  chartNameValues = await screen.findAllByRole('cell', {
-    name: /sample chart/i,
-  });
-
   expect(chartNameValues).toHaveLength(15);
-  expect(chartNameValues[0]).toHaveTextContent('Sample chart 51');
-  expect(chartNameValues[14]).toHaveTextContent('Sample chart 65');
+  expect(chartNameValues[0]).toHaveTextContent('Sample chart 26');
+  expect(chartNameValues[14]).toHaveTextContent('Sample chart 40');
 });

superset-frontend/src/utils/common.js

@@ -17,7 +17,6 @@
  * under the License.
  */
 import {
-  SupersetClient,
   getTimeFormatter,
   TimeFormats,
   ensureIsArray,
@@ -36,17 +35,6 @@ export const SHORT_TIME = 'h:m a';
 
 const DATETIME_FORMATTER = getTimeFormatter(TimeFormats.DATABASE_DATETIME);
 
-export function storeQuery(query) {
-  return SupersetClient.post({
-    endpoint: '/kv/store/',
-    postPayload: { data: query },
-  }).then(response => {
-    const baseUrl = window.location.origin + window.location.pathname;
-    const url = `${baseUrl}?id=${response.json.id}`;
-    return url;
-  });
-}
-
 export function optionLabel(opt) {
   if (opt === null) {
     return NULL_STRING;