Repository: gregnb/mui-datatables
Pull request #585 (2019-04-30 20:38:52)

Bugfix/issue 305 data update inconsistencies
Fixes the remaining pagination issues in https://github.com/gregnb/mui-datatables/issues/305.

- Enhance table example to demonstrate issues with state resets
- Add testing for state reset issues related to pagination (and add ids for this purpose)
- Remove deprecated lifecycle hook
- Update only the necessary state rather than also resetting table to initial values

I know that testing via the DOM can be brittle, but I tried to make it as robust as possible by using ids. I worked for a long time trying to test it in another manner, but the bug only occurs on state change via the lifecycle hooks, so it looked like I needed to actually trigger those to test properly.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/23116497/badge)](https://coveralls.io/builds/23116497)

Coverage increased (+1.7%) to 74.674% when pulling **4cfa21f160f32634ec80163f529132839b3dcacc on bugfix/issue-305-data-update-inconsistencies** into **e3822dfd39e70de6d99616ae9bbe3291e40cda57 on master**.


Patch:
diff --git a/examples/custom-action-columns/index.js b/examples/custom-action-columns/index.js
index 6435bbb..7253004 100644
--- a/examples/custom-action-columns/index.js
+++ b/examples/custom-action-columns/index.js
@@ -3,10 +3,66 @@ import ReactDOM from "react-dom";
 import MUIDataTable from "../../src/";
 
 class Example extends React.Component {
+  constructor(props) {
+    super(props);
 
-  render() {
+    this.state = {
+      data: [
+        ["Gabby George", "Business Analyst", "Minneapolis", 30, "$100,000"],
+        ["Aiden Lloyd", "Business Consultant", "Dallas",  55, "$200,000"],
+        ["Jaden Collins", "Attorney", "Santa Ana", 27, "$500,000"],
+        ["Franky Rees", "Business Analyst", "St. Petersburg", 22, "$50,000"],
+        ["Aaren Rose", "Business Consultant", "Toledo", 28, "$75,000"],
+        ["Blake Duncan", "Business Management Analyst", "San Diego", 65, "$94,000"],
+        ["Frankie Parry", "Agency Legal Counsel", "Jacksonville", 71, "$210,000"],
+        ["Lane Wilson", "Commercial Specialist", "Omaha", 19, "$65,000"],
+        ["Robin Duncan", "Business Analyst", "Los Angeles", 20, "$77,000"],
+        ["Mel Brooks", "Business Consultant", "Oklahoma City", 37, "$135,000"],
+        ["Harper White", "Attorney", "Pittsburgh", 52, "$420,000"],
+        ["Kris Humphrey", "Agency Legal Counsel", "Laredo", 30, "$150,000"],
+        ["Frankie Long", "Industrial Analyst", "Austin", 31, "$170,000"],
+        ["Brynn Robbins", "Business Analyst", "Norfolk", 22, "$90,000"],
+        ["Justice Mann", "Business Consultant", "Chicago", 24, "$133,000"],
+        ["Addison Navarro", "Business Management Analyst", "New York", 50, "$295,000"],
+        ["Jesse Welch", "Agency Legal Counsel", "Seattle", 28, "$200,000"],
+        ["Eli Mejia", "Commercial Specialist", "Long Beach", 65, "$400,000"],
+        ["Gene Leblanc", "Industrial Analyst", "Hartford", 34, "$110,000"],
+        ["Danny Leon", "Computer Scientist", "Newark", 60, "$220,000"],
+        ["Lane Lee", "Corporate Counselor", "Cincinnati", 52, "$180,000"],
+        ["Jesse Hall", "Business Analyst", "Baltimore", 44, "$99,000"],
+        ["Danni Hudson", "Agency Legal Counsel", "Tampa", 37, "$90,000"],
+        ["Terry Macdonald", "Commercial Specialist", "Miami", 39, "$140,000"],
+        ["Justice Mccarthy", "Attorney", "Tucson", 26, "$330,000"],
+        ["Silver Carey", "Computer Scientist", "Memphis", 47, "$250,000" ],
+        ["Franky Miles", "Industrial Analyst", "Buffalo", 49, "$190,000"],
+        ["Glen Nixon", "Corporate Counselor", "Arlington", 44, "$80,000"],
+        ["Gabby Strickland", "Business Process Consultant", "Scottsdale", 26, "$45,000"],
+        ["Mason Ray", "Computer Scientist", "San Francisco", 39, "$142,000"]
+      ]
+    };
+  }
 
+  render() {
     const columns = [
+      {
+        name: "Delete",
+        options: {
+          filter: true,
+          sort: false,
+          empty: true,
+          customBodyRender: (value, tableMeta, updateValue) => {
+            return (
+              <button onClick={() => {
+                const { data } = this.state;
+                data.shift();
+                this.setState({ data });
+              }}>
+                Delete
+              </button>
+            );
+          }
+        }
+      },
       {
         name: "Edit",
         options: {
@@ -62,7 +118,11 @@ class Example extends React.Component {
           empty: true,
           customBodyRender: (value, tableMeta, updateValue) => {
             return (
-              <button onClick={() => window.alert(`Clicked "Add" for row ${tableMeta.rowIndex}`)}>
+              <button onClick={() => {
+                const { data } = this.state;
+                data.unshift(["Mason Ray", "Computer Scientist", "San Francisco", 39, "$142,000"]);
+                this.setState({ data });
+              }}>
                 Add
               </button>
             );
@@ -71,40 +131,6 @@ class Example extends React.Component {
       },
     ];
 
-
-    const data = [
-      ["Gabby George", "Business Analyst", "Minneapolis", 30, "$100,000"],
-      ["Aiden Lloyd", "Business Consultant", "Dallas",  55, "$200,000"],
-      ["Jaden Collins", "Attorney", "Santa Ana", 27, "$500,000"],
-      ["Franky Rees", "Business Analyst", "St. Petersburg", 22, "$50,000"],
-      ["Aaren Rose", "Business Consultant", "Toledo", 28, "$75,000"],
-      ["Blake Duncan", "Business Management Analyst", "San Diego", 65, "$94,000"],
-      ["Frankie Parry", "Agency Legal Counsel", "Jacksonville", 71, "$210,000"],
-      ["Lane Wilson", "Commercial Specialist", "Omaha", 19, "$65,000"],
-      ["Robin Duncan", "Business Analyst", "Los Angeles", 20, "$77,000"],
-      ["Mel Brooks", "Business Consultant", "Oklahoma City", 37, "$135,000"],
-      ["Harper White", "Attorney", "Pittsburgh", 52, "$420,000"],
-      ["Kris Humphrey", "Agency Legal Counsel", "Laredo", 30, "$150,000"],
-      ["Frankie Long", "Industrial Analyst", "Austin", 31, "$170,000"],
-      ["Brynn Robbins", "Business Analyst", "Norfolk", 22, "$90,000"],
-      ["Justice Mann", "Business Consultant", "Chicago", 24, "$133,000"],
-      ["Addison Navarro", "Business Management Analyst", "New York", 50, "$295,000"],
-      ["Jesse Welch", "Agency Legal Counsel", "Seattle", 28, "$200,000"],
-      ["Eli Mejia", "Commercial Specialist", "Long Beach", 65, "$400,000"],
-      ["Gene Leblanc", "Industrial Analyst", "Hartford", 34, "$110,000"],
-      ["Danny Leon", "Computer Scientist", "Newark", 60, "$220,000"],
-      ["Lane Lee", "Corporate Counselor", "Cincinnati", 52, "$180,000"],
-      ["Jesse Hall", "Business Analyst", "Baltimore", 44, "$99,000"],
-      ["Danni Hudson", "Agency Legal Counsel", "Tampa", 37, "$90,000"],
-      ["Terry Macdonald", "Commercial Specialist", "Miami", 39, "$140,000"],
-      ["Justice Mccarthy", "Attorney", "Tucson", 26, "$330,000"],
-      ["Silver Carey", "Computer Scientist", "Memphis", 47, "$250,000" ],
-      ["Franky Miles", "Industrial Analyst", "Buffalo", 49, "$190,000"],
-      ["Glen Nixon", "Corporate Counselor", "Arlington", 44, "$80,000"],
-      ["Gabby Strickland", "Business Process Consultant", "Scottsdale", 26, "$45,000"],
-      ["Mason Ray", "Computer Scientist", "San Francisco", 39, "$142,000"]
-    ];
-
     const data1 = [
       {Name: "Gabby George", Title: "Business Analyst", Location: "Minneapolis", Age: 30, Salary: "$100,000"},
       {Name: "Aiden Lloyd", Title: "Business Consultant", Location: "Dallas", Age: 55, Salary: "$200,000"},
@@ -142,10 +168,14 @@ class Example extends React.Component {
       filter: true,
       filterType: 'dropdown',
       responsive: 'stacked',
+      page: 2,
+      onColumnSortChange: (changedColumn, direction) => console.log('changedColumn: ', changedColumn, 'direction: ', direction),
+      onChangeRowsPerPage: numberOfRows => console.log('numberOfRows: ', numberOfRows),
+      onChangePage: currentPage => console.log('currentPage: ', currentPage)
     };
 
     return (
-      <MUIDataTable title={"ACME Employee list"} data={data} columns={columns} options={options} />
+      <MUIDataTable title={"ACME Employee list"} data={this.state.data} columns={columns} options={options} />
     );
 
   }
diff --git a/src/MUIDataTable.js b/src/MUIDataTable.js
index f4367ad..ba3dee8 100644
--- a/src/MUIDataTable.js
+++ b/src/MUIDataTable.js
@@ -188,13 +188,11 @@ class MUIDataTable extends React.Component {
     this.setHeadResizeable(this.headCellRefs, this.tableRef);
   }
 
-  componentWillReceiveProps(nextProps) {
-    if (this.props.data !== nextProps.data || this.props.columns !== nextProps.columns) {
-      this.initializeTable(nextProps);
+  componentDidUpdate(prevProps) {
+    if (this.props.data !== prevProps.data || this.props.columns !== prevProps.columns) {
+      this.setTableData(prevProps, TABLE_LOAD.INITIAL);
     }
-  }
 
-  componentDidUpdate() {
     if (this.options.resizableColumns) {
       this.setHeadResizeable(this.headCellRefs, this.tableRef);
       this.updateDividers();
diff --git a/src/components/TablePagination.js b/src/components/TablePagination.js
index 870d6fa..ba42f83 100644
--- a/src/components/TablePagination.js
+++ b/src/components/TablePagination.js
@@ -68,11 +68,21 @@ class TablePagination extends React.Component {
             labelRowsPerPage={textLabels.rowsPerPage}
             labelDisplayedRows={({ from, to, count }) => `${from}-${to} ${textLabels.displayRows} ${count}`}
             backIconButtonProps={{
+              id: 'pagination-back',
               'aria-label': textLabels.previous,
             }}
             nextIconButtonProps={{
+              id: 'pagination-next',
               'aria-label': textLabels.next,
             }}
+            SelectProps= {{
+              id: 'pagination-input',
+              SelectDisplayProps: { id: 'pagination-rows' },
+              MenuProps: {
+                id: 'pagination-menu',
+                MenuListProps: { id: 'pagination-menu-list' }
+              }
+            }}
             rowsPerPageOptions={options.rowsPerPageOptions}
             onChangePage={this.handlePageChange}
             onChangeRowsPerPage={this.handleRowChange}
diff --git a/src/components/TableToolbarSelect.js b/src/components/TableToolbarSelect.js
index e577a2f..ed02d65 100644
--- a/src/components/TableToolbarSelect.js
+++ b/src/components/TableToolbarSelect.js
@@ -27,6 +27,7 @@ const defaultToolbarSelectStyles = theme => ({
   },
   iconButton: {
     marginRight: '24px',
+    height: '48px',
     top: '50%',
     display: 'block',
     position: 'relative',
diff --git a/webpack.config.js b/webpack.config.js
index 13a1de6..43ad278 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -3,7 +3,7 @@ const webpack = require('webpack');
 
 module.exports = {
   entry: {
-    app: "./examples/hide-columns-print/index.js"
+    app: "./examples/custom-action-columns/index.js"
   },
   stats: "verbose",
   context: __dirname,


Test patch:
diff --git a/test/MUIDataTable.test.js b/test/MUIDataTable.test.js
index 174d217..0bea639 100644
--- a/test/MUIDataTable.test.js
+++ b/test/MUIDataTable.test.js
@@ -193,6 +193,51 @@ describe('<MUIDataTable />', function() {
     assert.deepEqual(state.data, expectedResult);
   });
 
+  it('should correctly re-build internal table data while maintaining pagination after state change', () => {
+    let currentPage;
+    const options = {
+      rowsPerPage: 1,
+      rowsPerPageOptions: [1, 2, 4],
+      page: 1,
+      onChangePage: current => currentPage = current,
+    };
+    const fullWrapper = mount(<MUIDataTable columns={columns} data={data} options={options} />);
+
+    // simulate paging backward to set `currentPage`
+    fullWrapper.find('#pagination-back').at(0).simulate('click');
+    assert.strictEqual(currentPage, 0);
+
+    // simulate changing pagination to set `rowsPerPage`
+    fullWrapper.find('#pagination-rows').simulate('click');
+    fullWrapper.find('#pagination-menu-list li').at(1).simulate('click');
+    let inputValue = fullWrapper.find('#pagination-input').at(0).text();
+    assert.strictEqual(inputValue, '2');
+
+    // add data to simulate state change
+    let newData = data.map(item => [...item]);
+    newData.push(['Harry Smith', 'Test Corp', 'Philadelphia', 'PA', undefined]);
+    fullWrapper.setProps({ data: newData });
+
+    // simulate paging forward to test whether or not the `currentPage` was reset
+    fullWrapper.find('#pagination-next').at(0).simulate('click');
+    assert.strictEqual(currentPage, 1);
+
+    // grab pagination value to test whether or not `rowsPerPage` was reset
+    inputValue = fullWrapper.find('#pagination-input').at(0).text();
+    assert.strictEqual(inputValue, '2');
+
+    // test that data updated properly
+    let props = fullWrapper.props();
+    const expectedResult = [
+      ['Joe James', 'Test Corp', 'Yonkers', 'NY'],
+      ['John Walsh', 'Test Corp', 'Hartford', null],
+      ['Bob Herm', 'Test Corp', 'Tampa', 'FL'],
+      ['James Houston', 'Test Corp', 'Dallas', 'TX'],
+      ['Harry Smith', 'Test Corp', 'Philadelphia', 'PA', undefined],
+    ];
+    assert.deepEqual(props.data, expectedResult);
+  });
+
   it('should not re-build internal table data and displayData structure with no prop change to data or columns', () => {
     const initializeTableSpy = spy(MUIDataTable.Naked.prototype, 'initializeTable');
     const mountWrapper = mount(shallow(<MUIDataTable columns={columns} data={data} />).get(0));