fix remove handler to also delete references to existing models

[twalker]

The backgrid-select-all extension was removing references to unsaved models but not saved models. This fixes the issue by checking for model.id instead of just model.cid:

  this.listenTo(collection, "remove", function (model) {
    delete selectedModels[model.id || model.cid];
  });

[wyuenho]

Saved model also have cids no?

[twalker]

Interesting. Yes, saved models do have a cid.
But shouldn't the "true id" from the server take precedence over the automatically client assigned temporary id?

I would need to verify to be certain, but I suspect that I was seeing a mis-association between cid and id. Probably due to destroy calls followed by auto-assignement of different cid's. Giving model.id precedence kept the selectedModels from having an undefined value for a cid key.

Would you like further verification?

[wyuenho]

For backgrid's purposes, the select-all extension only needs 1 candidate key to keep track of which models within the collection were selected. cids are globally unique within a browser page so that would be sufficient. I'm not entirely sure what problem this PR is suppose to fix. Perhaps you could file a separate ticket illustrating your problem?

[twalker]

Sorry I'm not explaining the issue clearly. I think taking a look at the surrounding code will do a better job.
selectedModels is adding model references by the id for existing models:

https://github.com/wyuenho/backgrid/blob/master/src/extensions/select-all/backgrid-select-all.js#L150

Yet the remove handler is trying to delete by cid

https://github.com/wyuenho/backgrid/blob/master/src/extensions/select-all/backgrid-select-all.js#L158

There's a key mismatch. Which resulted in selectedModels having references to removed models--undefined.

The PR fixes the problem by using the same approach to removing a selectedModel as it does for adding a model.

[wyuenho]

Ah Ok thanks. Good catch.