Select all rows (include group rows) in the journal grid

[mbayopanda]

This PR permits to select all rows in the journal grid, this selection include the group header rows.

Closes #1512.

[jniles]

@mbayopanda, I've made some comments for code clarity.

[jniles]

The ALL_CAPITAL_LETTERS syntax is usually for constants. Does this variable need to be global to the entire service? It looks like it is only used in hadnleBatchSelection().

[jniles]

What does isSelected do? It looks like it is always set but never used. Is it an internal ui-grid thing?

[mbayopanda]

The isSelected is responsible to set the row selected (checked) in the ui grid.

[jniles]

Cool - I was wondering if that was the case 👍

[jniles]

Do you need to set this every loop through the gridRows? It looks like what you are really doing is:

gridRows.forEach(function (row) {
  var parentRow = row.treeNode.parentRow;
  if (parentRow.treeLevel === 0 && !parents[parentRow.uid]) {
    parentRow.isSelected = true;
    parents[parentRow.uid] = parentRow;
  }
});

SELECTED_GROUP_HEADERS = parents;

If that is true, then it is easier to read (and might be more performant) to remove the SELECTED_GROUP_HEADERS variable assignment from the loop.

[mbayopanda]

the SELECTED_GROUP_HEADERS is a global variable and must be updated only when there are new parent row (enter in the if clause).

If it is out the if we cannot deselect parent rows which were previously selected because

var parents = {};

/* this code is not executed
gridRows.forEach(function (row) {
  var parentRow = row.treeNode.parentRow;
  if (parentRow.treeLevel === 0 && !parents[parentRow.uid]) {
    parentRow.isSelected = true;
    parents[parentRow.uid] = parentRow;
  }
}); */

SELECTED_GROUP_HEADERS = parents; // SELECTED_GROUP_HEADERS = {}

[jniles]

Aha, that makes sense. I don't think there is a clearer way of expressing what you wrote. LGTM 👍

[jniles]

I know what this is because we've talked about it, but the code is hard to read as it is. Could you move this check into a function?

// this function identifies parent rows that we haven't seen yet
function isUnusedParentRow(row) {
  return row.treeLevel === 0 && !parents[row.uid];
}

gridRows.forEach(function (row) {
  var parentRow = row.treeNode.parentRow; 
  if (isUnusedParentRow(parentRow)) {
     // do something.
  }
});

[jniles]

Nice variable assignment 👍

[jniles]

All my comments have been addressed 👍