fix(sql lab): MultiSelector component render twice (apache#20706)

* fix(sql lab): MultiSelector component render twice * filter null/undefined tables
cemremengu · Jul 19, 2022 · 115ab70 · 115ab70
1 parent e60083b
commit 115ab70
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 17 deletions.
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -40,7 +40,7 @@ export const QUERY_EDITOR_SAVED = 'QUERY_EDITOR_SAVED';
 export const CLONE_QUERY_TO_NEW_TAB = 'CLONE_QUERY_TO_NEW_TAB';
 export const REMOVE_QUERY_EDITOR = 'REMOVE_QUERY_EDITOR';
 export const MERGE_TABLE = 'MERGE_TABLE';
-export const REMOVE_TABLE = 'REMOVE_TABLE';
+export const REMOVE_TABLES = 'REMOVE_TABLES';
 export const END_QUERY = 'END_QUERY';
 export const REMOVE_QUERY = 'REMOVE_QUERY';
 export const EXPAND_TABLE = 'EXPAND_TABLE';
@@ -1213,16 +1213,21 @@ export function collapseTable(table) {
   };
 }
 
-export function removeTable(table) {
+export function removeTables(tables) {
   return function (dispatch) {
+    const tablesToRemove = tables?.filter(Boolean) ?? [];
     const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)
-      ? SupersetClient.delete({
-          endpoint: encodeURI(`/tableschemaview/${table.id}`),
-        })
+      ? Promise.all(
+          tablesToRemove.map(table =>
+            SupersetClient.delete({
+              endpoint: encodeURI(`/tableschemaview/${table.id}`),
+            }),
+          ),
+        )
       : Promise.resolve();
 
     return sync
-      .then(() => dispatch({ type: REMOVE_TABLE, table }))
+      .then(() => dispatch({ type: REMOVE_TABLES, tables: tablesToRemove }))
       .catch(() =>
         dispatch(
           addDangerToast(

diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
@@ -835,23 +835,40 @@ describe('async actions', () => {
       });
     });
 
-    describe('removeTable', () => {
+    describe('removeTables', () => {
       it('updates the table schema state in the backend', () => {
         expect.assertions(2);
 
         const table = { id: 1 };
         const store = mockStore({});
         const expectedActions = [
           {
-            type: actions.REMOVE_TABLE,
-            table,
+            type: actions.REMOVE_TABLES,
+            tables: [table],
           },
         ];
-        return store.dispatch(actions.removeTable(table)).then(() => {
+        return store.dispatch(actions.removeTables([table])).then(() => {
           expect(store.getActions()).toEqual(expectedActions);
           expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1);
         });
       });
+
+      it('deletes multiple tables and updates the table schema state in the backend', () => {
+        expect.assertions(2);
+
+        const tables = [{ id: 1 }, { id: 2 }];
+        const store = mockStore({});
+        const expectedActions = [
+          {
+            type: actions.REMOVE_TABLES,
+            tables,
+          },
+        ];
+        return store.dispatch(actions.removeTables(tables)).then(() => {
+          expect(store.getActions()).toEqual(expectedActions);
+          expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(2);
+        });
+      });
     });
 
     describe('migrateQueryEditorFromLocalStorage', () => {

diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
@@ -167,7 +167,7 @@ export default function SqlEditorLeftBar({
       actions.addTable(queryEditor, database, tableName, schemaName),
     );
 
-    currentTables.forEach(table => actions.removeTable(table));
+    actions.removeTables(currentTables);
   };
 
   const onToggleTable = (updatedTables: string[]) => {

diff --git a/superset-frontend/src/SqlLab/components/TableElement/index.tsx b/superset-frontend/src/SqlLab/components/TableElement/index.tsx
@@ -57,7 +57,7 @@ export interface TableElementProps {
   table: Table;
   actions: {
     removeDataPreview: (table: Table) => void;
-    removeTable: (table: Table) => void;
+    removeTables: (tables: Table[]) => void;
   };
 }
 
@@ -85,7 +85,7 @@ const TableElement = ({ table, actions, ...props }: TableElementProps) => {
 
   const removeTable = () => {
     actions.removeDataPreview(table);
-    actions.removeTable(table);
+    actions.removeTables([table]);
   };
 
   const toggleSortColumns = () => {

diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js
@@ -175,8 +175,12 @@ export default function sqlLabReducer(state = {}, action) {
     [actions.COLLAPSE_TABLE]() {
       return alterInArr(state, 'tables', action.table, { expanded: false });
     },
-    [actions.REMOVE_TABLE]() {
-      return removeFromArr(state, 'tables', action.table);
+    [actions.REMOVE_TABLES]() {
+      const tableIds = action.tables.map(table => table.id);
+      return {
+        ...state,
+        tables: state.tables.filter(table => !tableIds.includes(table.id)),
+      };
     },
     [actions.START_QUERY_VALIDATION]() {
       let newState = { ...state };

diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js
@@ -181,8 +181,8 @@ describe('sqlLabReducer', () => {
     });
     it('should remove a table', () => {
       const action = {
-        type: actions.REMOVE_TABLE,
-        table: newTable,
+        type: actions.REMOVE_TABLES,
+        tables: [newTable],
       };
       newState = sqlLabReducer(newState, action);
       expect(newState.tables).toHaveLength(0);