bug/pager - DISMISS_EDITOR, REMOVE_ROW, and ADD_NEW_ROW now set total correctly

[Vanderslice]

Fix for bug #132.

Feedback Requested: I moved the logic for updating total from addNewRows and dismissEditor to saveRow so the grid totals update on save. I can see arguments for either.

Note: tests are currently failing due to moving the logic, will update after getting feedback

[bencripps]

I think these changes look good 👍

Let's get the tests fixed; I think this is a good/sensible change.

[codecov-io]

Codecov Report

Merging #133 into master will increase coverage by 0.01%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
+ Coverage   81.34%   81.36%   +0.01%     
==========================================
  Files         111      111              
  Lines        4492     4496       +4     
==========================================
+ Hits         3654     3658       +4     
  Misses        838      838

Impacted Files	Coverage Δ
src/reducers/actionHelpers/datasource.js	66.07% <80%> (+0.82%)	✅

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ecf836...e1dc52a. Read the comment docs.

[Vanderslice]

I went ahead and moved the logic for updating the totals back to addNewRow and dismissEditor for the time being due to a conflict with the latest changes to allow for inserting a new row editor at any point in the grid

[bencripps]

Changes look good. Sort of unrelated, how come we don't update the total in removeRow like we do with addRow? Here's the snippet from the actionHandler for removeRow:

const updated = record.merge({
        data: remainingRows,
        proxy: remainingRows,
        currentRecords: remainingRows,
        lastUpdate: generateLastUpdate()
    });

Should we add total to this update?

[bencripps]

Probably need to be a little bit more defensive here sadly. I would add a check to make sure existingState has been initialized, otherwise this will throw

[Vanderslice]

Thanks for the feedback! I updated the removeRow actionHandler to decrement the total and fixed the tests to handle this change.

I had previously missed this because we do a setData after removing rows but I could see where this may not be wanted/needed plus it makes the actionHandlers behave consistently.

Deleting rows on the demo grid now gives you instant feedback that the number of currently displayed rows and total have changed without requiring setData/refresh

[bencripps]

Everything looks great, two tiny things!

[bencripps]

Run npm run lint and see if you get any errors

[bencripps]

This is a nit, but if total resolved to 0, then we're setting the total to -1 which probably isn't best.