`getSelectedModels` sometimes includes undefined values

[twalker]

I don't have a unit test to isolate the issue, yet. But, when using this extension along with backbone pageabe, 'getSelectedModels' includes undefined values for previously selected models after pagination. I suspect it is because it's gathering selected models from this.collection: https://github.com/wyuenho/backgrid-select-all/blob/master/backgrid-select-all.js#L236

It should probably be this.collection.fullCollection when the collection is pageable.

I can work around the issue by removing the undefined values from the array.

return _.compact(result);

Another approach would be to use this.collection.fullCollection if it exists.

Would you welcome a PR for either of these approaches?

[wyuenho]

Using fullCollection when available sounds reasonable. A PR with tests would be great :)

[ErikEvenson]

I always get undefined values in the result array of getSelectedModels() when selecting rows across pages. I do get a model for models selected on the currently active page. My pageable collection from backgrid-paginator (which is a backbone-pageable 1.4.8 collection I believe) is in server mode.

I am using the following versions:

  "dependencies": {
    "underscore": ">=1.5.2",
    "backbone": "1.1.2",
    "bootstrap": "3.1.1",
    "marionette": "1.7.4",
    "json2": "*",
    "backbone.picky": "0.2.0",
    "backbone.syphon": "0.4.1",
    "backgrid": "0.3.5",
    "backgrid-select-all": "0.3.5",
    "backgrid-paginator": "0.3.5",
    "backgrid-filter": "0.3.5",
    "spin.js": "2.0.1",
    "backgrid-moment-cell": "0.3.5",
    "bootstrap3-datetimepicker": "3.0.0",
    "highcharts-release": "4.0.1",
    "numeral": "1.5.3",
    "moment": "2.5.1"
  }

I am using Chrome version 35.0.1916.153.

Thanks!

[ErikEvenson]

I changed my dependencies to the latest commit of backgrid-paginator so that the pageable collection would come from backbone.paginator 2.0.0. (It seems 0.3.5 via bower pulls in a pageable collection from backbone-pageable 1.4.8 which is deprecated.)

I have the same issue. If I click a select all box, I get a result array that includes only the models for the rows being displayed on the current page.

[ErikEvenson]

It seems like some of select-all's functionality (getSelectedModels and select-all events for example) depends on the use of the collection's fullCollection property. Unless I am mistaken, this property is really only useful when the collection is in client mode. In server mode, the entire collection is rarely present. Am I missing something?

[wyuenho]

@ErikEvenson That's correct.

[alexandraorth]

I'm having this same issue using backgrid-filter. After filtering a pageable collection, the getSelectedModels() method returns undefined values because the fullCollection property is not complete. In turn, this is causing select-all to malfunction. Is there any solution or workaround to this problem?

[wyuenho]

The original issue has already been fixed, so this must require a different steps to reproduce. Can you show me how to reproduce this issue?

[steverice]

It's fixed for pageable, but the fundamental issue is:

1. Backgrid has a collection model
2. Things are selected in the table, and references to the models in collection are held by backgrid-select-all
3. The collection is changed (via a reset)
4. backgrid-select-all still references the models in the old collection that are no longer part of it, so collection.get(reference) returns undefined

The easiest solution may be to just listen to remove and reset events on the collection and remove things from selectedModels if collection.get(selelectedModel) is undefined.

[wyuenho]

So select-all is causing problems for the vanilla Backbone.Collection is that what you are saying?

This may require the ability to distinguish a real remove and or reset from pagination events.

[christinedraper]

The original fix didn't address the issue with reset events. I've been running with the following patch. It works for me, but I don't know what unintended consequences it may have (I dont use pagination... yet).

/*
 * SelectAll extension to delete selected models on reset
 */
var originalInitialize = Backgrid.Extension.SelectAllHeaderCell.prototype.initialize;
Backgrid.Extension.SelectAllHeaderCell.prototype.initialize = function (options) {

    originalInitialize.apply(this, arguments);

    this.listenTo(this.collection, "reset", function (model) {
        var selectedModels = this.selectedModels;
        var checkAll = this.$el.find("input[type=checkbox]").prop("checked");

        Object.keys(selectedModels).forEach(function(modelId) {
            if (! this.collection.get(modelId)){
                delete selectedModels[modelId];
            }           
        }, this);
    });

    // if a model is added, select it if all checkbox is selected 
    this.listenTo(this.collection, "add", function (model) {
        var checkAll = this.$el.find("input[type=checkbox]").prop("checked");
        if (checkAll) {
            model.trigger('backgrid:select', model, true);
        }
    });
};