console: update selected rows state after single row delete (close #4654)

CHANGELOG.md

@@ -59,6 +59,7 @@ Read more about the session argument for computed fields in the [docs](https://h
 - console: fix checkbox for forwarding client headers in actions (#4595)
 - console: re-enable foreign tables to be listed as views (fix #4714) (#4742)
 - console: display rows limit in permissions editor if set to zero (fix #4559)
+- console: fix inconsistency between selected rows state and displayed rows (fix #4654) (#4673)
 - cli: list all available commands in root command help (fix #4623)
 - docs: add section on actions vs. remote schemas to actions documentation (#4284)
 - docs: fix wrong info about excluding scheme in CORS config (#4685)

console/cypress/integration/data/insert-browse/spec.js

@@ -481,11 +481,12 @@ export const checkViewRelationship = () => {
   cy.wait(1000);
   cy.get(getElementFromAlias('table-browse-rows')).click();
   cy.wait(1000);
-  cy.get('a')
-    .contains('View')
-    .first()
-    .click();
-  cy.wait(1000);
+  cy.get('.rt-table').within(() => {
+    cy.get('a')
+      .contains('View')
+      .click();
+    cy.wait(1000);
+  });
   cy.get('a')
     .contains('Close')
     .first()
@@ -495,13 +496,48 @@ export const checkViewRelationship = () => {
 export const passDeleteRow = () => {
   cy.get(getElementFromAlias('table-browse-rows')).click();
   cy.wait(5000);
+  cy.get(getElementFromAlias('table-browse-rows')).contains('21');
   cy.get(getElementFromAlias('row-delete-button-0')).click();
   cy.on('window:confirm', str => {
     expect(
       str.indexOf('This will permanently delete this row from this table') !==
         -1
     ).to.be.true;
   });
+  cy.get(getElementFromAlias('table-browse-rows')).contains('20');
   // cy.get('.notification-error');
   cy.wait(14000);
 };
+
+export const passBulkDeleteRows = () => {
+  cy.get(getElementFromAlias('table-browse-rows')).click();
+  cy.wait(5000);
+  cy.get(getElementFromAlias('table-browse-rows')).contains('20');
+  cy.get(getElementFromAlias('row-checkbox-0')).click();
+  cy.get(getElementFromAlias('row-checkbox-1')).click();
+  cy.get(getElementFromAlias('bulk-delete')).click();
+  cy.wait(1000);
+  cy.on('window:confirm', str => {
+    expect(
+      str.indexOf('This will permanently delete rows from this table') !== -1
+    ).to.be.true;
+  });
+  cy.get(getElementFromAlias('table-browse-rows')).contains('18');
+  cy.wait(14000);
+};
+
+export const passBulkDeleteAllRows = () => {
+  cy.get(getElementFromAlias('table-browse-rows')).click();
+  cy.wait(5000);
+  cy.get(getElementFromAlias('table-browse-rows')).contains('18');
+  cy.get(getElementFromAlias('select-all-rows')).click();
+  cy.get(getElementFromAlias('bulk-delete')).click();
+  cy.wait(1000);
+  cy.on('window:confirm', str => {
+    expect(
+      str.indexOf('This will permanently delete rows from this table') !== -1
+    ).to.be.true;
+  });
+  cy.get(getElementFromAlias('table-browse-rows')).contains('(8)');
+  cy.wait(14000);
+};

console/cypress/integration/data/insert-browse/test.js

@@ -20,6 +20,8 @@ import {
   passCloneButton,
   checkViewRelationship,
   passDeleteRow,
+  passBulkDeleteRows,
+  passBulkDeleteAllRows,
 } from './spec';
 
 import { setMetaData } from '../../validators/validators';
@@ -51,6 +53,8 @@ export const runInsertBrowseTests = () => {
     it('Check edit button', passEditButton);
     it('Check for clone clear', passCloneButton);
     it('Delete the row', passDeleteRow);
+    it('Bulk delete rows', passBulkDeleteRows);
+    it('Bulk delete all rows', passBulkDeleteAllRows);
     it('Check view relationship', checkViewRelationship);
     it('Delete test table', deleteBITestTable);
   });

console/src/components/Services/Data/TableBrowseRows/ViewRows.js

@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React, { useState, useEffect } from 'react';
 import 'react-table/react-table.css';
 import '../../../Common/TableCommon/ReactTableOverrides.css';
 import DragFoldTable, {
@@ -55,6 +55,8 @@ import {
   getPersistedColumnsOrder,
   persistPageSizeChange,
 } from './localStorageUtils';
+import { compareRows, isTableWithPK } from './utils';
+import styles from '../../../Common/TableCommon/Table.scss';
 
 const ViewRows = ({
   curTableName,
@@ -84,8 +86,9 @@ const ViewRows = ({
   readOnlyMode,
 }) => {
   const [selectedRows, setSelectedRows] = useState([]);
-
-  const styles = require('../../../Common/TableCommon/Table.scss');
+  useEffect(() => {
+    setSelectedRows([]);
+  }, [curTableName, currentSchema]);
 
   const NO_PRIMARY_KEY_MSG = 'No primary key to identify row';
 
@@ -132,12 +135,6 @@ const ViewRows = ({
     return _isSingleRow;
   };
 
-  const checkIfHasPrimaryKey = _tableSchema => {
-    return (
-      _tableSchema.primary_key && _tableSchema.primary_key.columns.length > 0
-    );
-  };
-
   const getGridHeadings = (_columns, _relationships, _disableBulkSelect) => {
     const _gridHeadings = [];
 
@@ -160,6 +157,7 @@ const ViewRows = ({
             title={_disableBulkSelect ? 'No primary key to identify row' : ''}
             type="checkbox"
             onChange={handleAllCheckboxChange}
+            data-test="select-all-rows"
           />
         </div>
       ),
@@ -214,30 +212,12 @@ const ViewRows = ({
     return _gridHeadings;
   };
 
-  const compareRows = (row1, row2, _tableSchema, _hasPrimaryKey) => {
-    let same = true;
-    if (!isView && _hasPrimaryKey) {
-      _tableSchema.primary_key.columns.map(pk => {
-        if (row1[pk] !== row2[pk]) {
-          same = false;
-        }
-      });
-      return same;
-    }
-    _tableSchema.columns.map(k => {
-      if (row1[k.column_name] !== row2[k.column_name]) {
-        return false;
-      }
-    });
-    return same;
-  };
-
-  const handleCheckboxChange = (row, e, ...rest) => {
+  const handleCheckboxChange = (row, e, tableSchema) => {
     if (e.target.checked) {
       setSelectedRows(prev => [...prev, row]);
     } else {
       setSelectedRows(prev =>
-        prev.filter(prevRow => !compareRows(prevRow, row, ...rest))
+        prev.filter(prevRow => !compareRows(prevRow, row, tableSchema, isView))
       );
     }
   };
@@ -360,6 +340,9 @@ const ViewRows = ({
           const deleteIcon = <i className="fa fa-trash" />;
 
           const handleDeleteClick = () => {
+            setSelectedRows(prev =>
+              prev.filter(r => !compareRows(r, pkClause, _tableSchema, isView))
+            );
             dispatch(deleteItem(pkClause, curTableName, currentSchema));
           };
 
@@ -495,11 +478,10 @@ const ViewRows = ({
             disabled={_disableBulkSelect}
             title={_disableBulkSelect ? NO_PRIMARY_KEY_MSG : ''}
             checked={selectedRows.some(selectedRow =>
-              compareRows(selectedRow, row, _tableSchema, _hasPrimaryKey)
+              compareRows(selectedRow, row, _tableSchema, isView)
             )}
-            onChange={e =>
-              handleCheckboxChange(row, e, _tableSchema, _hasPrimaryKey)
-            }
+            onChange={e => handleCheckboxChange(row, e, _tableSchema, isView)}
+            data-test={`row-checkbox-${rowIndex}`}
           />
         </div>
       );
@@ -641,7 +623,7 @@ const ViewRows = ({
   const tableColumnsSorted = tableSchema.columns.sort(ordinalColSort);
   const tableRelationships = tableSchema.relationships;
 
-  const hasPrimaryKey = checkIfHasPrimaryKey(tableSchema);
+  const hasPrimaryKey = isTableWithPK(tableSchema);
 
   const isSingleRow = checkIfSingleRow(curRelName);
 
@@ -718,6 +700,7 @@ const ViewRows = ({
             className={`${styles.add_mar_right_small} btn btn-xs btn-default ${styles.bulkDeleteButton}`}
             title="Delete selected rows"
             onClick={handleDeleteItems}
+            data-test="bulk-delete"
           >
             <i className="fa fa-trash" />
           </button>

console/src/components/Services/Data/TableBrowseRows/utils.ts

@@ -0,0 +1,39 @@
+type TableSchema = {
+  primary_key?: { columns: string[] };
+  columns: Array<{ column_name: string }>;
+};
+
+type TableSchemaWithPK = {
+  [P in keyof TableSchema]-?: TableSchema[P];
+};
+
+export const isTableWithPK = (
+  tableSchema: TableSchema
+): tableSchema is TableSchemaWithPK => {
+  return (
+    !!tableSchema.primary_key && tableSchema.primary_key.columns.length > 0
+  );
+};
+
+export const compareRows = (
+  row1: Record<string, any>,
+  row2: Record<string, any>,
+  tableSchema: TableSchema,
+  isView: boolean
+) => {
+  let same = true;
+  if (!isView && isTableWithPK(tableSchema)) {
+    tableSchema.primary_key.columns.forEach(pk => {
+      if (row1[pk] !== row2[pk]) {
+        same = false;
+      }
+    });
+    return same;
+  }
+  tableSchema.columns.forEach(k => {
+    if (row1[k.column_name] !== row2[k.column_name]) {
+      same = false;
+    }
+  });
+  return same;
+};