Pageable breaks when cloning collection

[PaRoxUs]

First, thanks for a great plugin, what an amazing work you are doing!

I found an issue when using .clone() on a collection created with PageableCollection. What happens is that the backgrid paginator only displays the first set of models selected in state.pageSize. So instead of showing《〈 1 2 3 4 〉》it simply shows 《〈 1 〉》

So if I use this.collection = global.collection it works but it don't if I set this.collection = global.collection.clone();

When I analyzed the different before and after clone I found that this.collection.fullCollection.pageableCollection.fullCollection seem to be the place where the clone have difficulties.

The cloned collection simply gets the value from the paged collection (if pageSize = 100, then the length of the models array is set to 100 instead of the full collection).

I also found that some values in this.collection._initState gets the wrong values.

my best ~ Pontus

[wyuenho]

Yep this is the expected behavior because the current page is supposed to work exactly the same as Backbone.Collection by default. But you've raised a good point in that the overridden does not return an instance of PageableCollection, so some information is lost. I'd be happy to accept a PR for this

[PaRoxUs]

This is not well tested yet but adding this to backbone-pageable seem to do the trick. clone : function() { return _.clone(this); },

[wyuenho]

This won't work because the prototypes aren't setup correctly.

Sent from my iPhone

On 4 Feb, 2014, at 7:18 pm, Pontus Karlsson notifications@github.com wrote:

This is not well tested yet but at first look after adding this to backbone-pageable seem to do the trick. clone : function() { return _.clone(this); },

— Reply to this email directly or view it on GitHub.

[PaRoxUs]

Yeah you are right, although the complete list renders the sorting function breaks.

[wyuenho]

You can look at Backbone.Collection's source code to find out how it clones. You can do pretty much the same thing with Backbone.PageableCollection, just pass the models from the fullCollection for client and infinite mode and just this.models in server mode, the state and queryParams into the constructor. Should be a pretty straight forward.

[PaRoxUs]

Do you mean something like this? So far this works for us.

 // Create a new collection with an identical list of models as this one.
    clone : function() {
        if (this.mode != "server") {
            return new this.constructor(this.fullCollection.models,this.queryParams);
        } else {
            return new this.constructor(this.collection.models,this.queryParams);
        }
    },

[PaRoxUs]

Thanks for the improvement but I found out that when cloning a filtered collection the fullCollection is the fullCollection after the filtering is done.

I found that approach a bit limiting when you want to get the true full collection. Why not store the full non filtered collection in the collection so when cloning it the collection is still returning the true fullCollection? Does that make sense to you?

[jessepeterson]

I'm having an issue that might be related to this. This might just be a misunderstanding on my part, but if I clone() a PageableCollection, perform a filter() on it, then pass that back to the PageableCollection in a reset() (basically implementing a custom filter) it appears that the collection's individual models no longer point to the collection upon page changes.

However in theory since PageableCollection.getPage does perform a reset() on itself then Backbone's reset() should end up performing the _addReference() to re-associate the individual model's collection back to the PageableCollection. But it doesn't seem to be doing that. It does get a collection just not the original PageableCollection (which I have some properties set on that I need to access from individual model views (cells).

What am I missing?

[wyuenho]

If you use the latest version, the code for cloning was removed. There was a brief moment in time where cloning performed a deep copy so it's normal that you got a different reference after a clone.

[jessepeterson]

But the PageableCollection should be reset()ing the current page's collection in the getPage method, right? Thus the PageableCollection's per-page collection should be (eventually) calling Backone's Collection._addReference() to re-associate the correct collection. Or is something in PageableCollection preventing that? Is that how it's supposed to work now? If so should that be changed?

If that is the case then what's the proper way to do filtering that keeps the individual models associated to PageableCollection? Perhaps implement my own .clone() that copies the attributes I need to my own "shadow" fullCollection?

[wyuenho]

pageable's current and shadow collections do share model references. I'm not sure what kind of filtering you are looking for as pageable doesn't come with any out of the box.

Also, please be noted that pageable doesn't override backbone.collection#clone. So if you call it directly you'll get a backbone.collection back instead of a backbone.pageablecollection.

[jessepeterson]

Perhaps I can put together a fiddle or something to demonstrate. But it's not the model references it's the collection reference in the models. For example:

PageableCollection.models[0].collection should point to the original PageableCollection. And it does. Until one does a PageableCollection.fullCollection.clone(). Then the PageableCollection.fullCollection.models[0].collection points to the cloned copy (as it should). That's fine. But then when PageableCollection.getPage happens on a page change the the PageableCollection.models[0].collection still points to the cloned collection. It should point to the original PageableCollection (because a Backbone reset should re-assign the model's collection property to it in the _addReference method).

Any clearer? I'll try and find time to put together something more demonstrable. :)

[wyuenho]

Ah ok. Actually I'm not sure why you'd need the collection reference from the model. It's only there for backbone to know how to general a URL for requests. Is there's a specific use case where you must have the Model#collection pointing to the current page all the time? In any case this is a completely separate issue. If this bothers you then please open a new issue.

[jessepeterson]

I set some properties on the collection (the PageableCollection that is) that the model views reference in their event handlers. E.g. this.model.collection.collectionWideProperty in my view. In my case a small array of "allowed" values for a particular model attribute that gets checked for validation and whatnot. The weirdest part about all this is that the first page of of the collection always works and is associated with the PageableCollection. But no subsequent pages.

As an alternative I could put that small array in every model, but that seems wasteful. I could reference the PageableCol in that view, but that feels dirty and is worse for encapsulation. Global seems yucky. Happy to entertain ideas. :)

[wyuenho]

What's wrong with subclassing a model and put the enum in the definition? If it's in the prototype, all instances share that reference.