Possible issue with inherited _onModelEvent about filtered-collection HOT 8 CLOSED

How interesting. The idea here is to remove the element from the original collection via the original collection's event and then modify the filtered one to remove it as well (thus propagating the event and synchronizing). So yes, two events would be fired (in the grand scheme of things).

Sorry I don't use parse's backbone at all. I hope there does not need to be a fork for the parse version.

In any case let me know what u find, thanks for investigating.

from filtered-collection.

No problem. I think it's doing something like this:

"destroy" event on model
  originalCollection.remove
    "remove" event on originalCollection
      filteredCollection.removeModel
        filteredCollection._forceRemoveModel
  filteredCollection.remove
    throw error

Because the FilteredCollection, whenever it adds an item, is still calling Backbone.Collection.prototype.add, which is still doing (model = models[i]).on('all', this._onModelEvent, this);. And _onModelEvent is unchanged.

From what I see, there's no test case for model.destroy, only collection.remove. And model.destroy is the only thing in the original Backbone framework that calls Backbone.sync('delete', ...) AFAIK, so it's by far the easiest way to implement a delete button on an item view.

from filtered-collection.

Gotcha. Let me know if you make any modifications, or a failing spec.

from filtered-collection.

FYI: I am not using this in any current project, sorry, so it will be a bit of time before I look into this if you want me to fix it.

from filtered-collection.

No worries, I've already fixed it, but I did the quick thing which was to copy the file into my project, change indentation to match my settings, add a quick line for Parse functionality. So it'll be a bit of time before I can make a pull request or a diff, but I'll take care of doing so as soon as I have the chance.

from filtered-collection.

No problem. Fork away. If you can just give me your test case, I think it would be already useful.

Dmitriy Likhten

On Tuesday, November 20, 2012 at 12:34 PM, Brenton Partridge wrote:

No worries, I've already fixed it, but I did the quick thing which was to copy the file into my project, change indentation to match my settings, add a quick line for Parse functionality. So it'll be a bit of time before I can make a pull request or a diff, but I'll take care of doing so as soon as I have the chance.

—
Reply to this email directly or view it on GitHub (#3 (comment)).

from filtered-collection.

If you can fork and show me your fixes, it would be helpful. I'm seing the same problem. I'll probably wind up fixing it too.

from filtered-collection.

Ah, fix is simple. _onModelEvent should be a noop for a filtered collection. Makes life simpler. That code makes sense because if the model is "deleted" it should be removed for all collections that contain the model. But an add/remove can occur on any collection, really, and should not affect collections that are not the ones added/removed from.

Our collection is special, it does not really contain elements, in the classical sense, it uses some collection mechanisms but in the end it behaves like a collection, except all "containership" work is done by the underlying collection.

from filtered-collection.