Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update support for collections. #446

Closed
wants to merge 1 commit into from
Closed

Conversation

dalyons
Copy link

@dalyons dalyons commented Jun 29, 2011

This pull request resolves the issue described here:
#137

It adds in-place updating support for collections, which is the ability to refresh the contents of a collection without creating new model instances for models that already exist client-side.

The use case is something like this:

  • I have a collection representing a list of bookings.
  • I have a view bound to each booking in the collection.
  • I can edit bookings, other people can add/edit bookings.
  • I want to be able to fetch the current bookings from the server, and update my screen without having to recreate/rebind all my views for existing bookings.
  • I want to leverage all of the usual backbone mechanisims, for example when an attribute of a booking is changed in the most recent fetch(), it will update & trigger a change event for the booking model I already have clientside & bound to my views.

It works well for us. I think some flavour of this mechanism would be very useful to include in backbone, even if you think this initial implementation isn't quite right from an API pov.

Let me know what you think!

…ns. Added update option to Collection.fetch().
@reconbot
Copy link
Contributor

I love the idea! I've added an updateInto() function to my collections that checks for the existence of each model and either adds it or updates it. The fact it works with fetch is a plus =)

@sujithrs
Copy link

+1. Exactly what I was looking for. Thanks dude!

@ghost
Copy link

ghost commented Jul 14, 2011

+1. This can be useful to many people.

@pirelenito
Copy link

+1 Just what I needed!

@cavi21
Copy link

cavi21 commented Jul 22, 2011

+1 Just what I'm looking for, and came accross this issue

@vizjerai
Copy link

+1 would be very helpful in my current project.

@vinilios
Copy link

+1

@PaulUithol
Copy link
Contributor

It might be a better idea to have this functionality as an option to add, like add( { update: true } )? I like the option to update existing models, but removing models that aren't in a response doesn't seem to be as big a use case to me.

@vizjerai
Copy link

While removing models that wasn't in the response may not be a large use case, there are some people who could use it. Would it be too hard to have another flag/callback to remove models that weren't in the response?

add( { update: true, remove: true } )

Or would it be better for the whole collection to be cleared and then add models in the response?

@reconbot
Copy link
Contributor

I'd be much happier with a removal of the models too, I can think of many use cases.

A buddy list for example, when people sign off line maybe you want to remove the user from the list, grey out their chat windows, and set off any notifications that might be set for the user.

I do see however that you might want this optional, so you can poll a url for only changed models and not have to pull down the entire list each time.

@vizjerai
Copy link

Any chance of this getting accepted? I'm using 0.5.3 with this patch and it works great.

@bruth
Copy link

bruth commented Sep 15, 2011

+1

@bruth
Copy link

bruth commented Oct 3, 2011

On line 525 you should use the methods relative to this e.g. this.reduce(...), since Collection has been extended with a bunch of Underscore methods already: http://documentcloud.github.com/backbone/#Collection-Underscore-Methods

@bruth
Copy link

bruth commented Oct 13, 2011

I'm wrong, on 525 could use the built-in. Edited.

bruth added a commit to bruth/backbone-common that referenced this pull request Oct 13, 2011
bruth added a commit to chop-dbhi/cilantro that referenced this pull request Oct 13, 2011
Includes:
- jashkenas/backbone#569 ability to define local elements
- jashkenas/backbone#446 integrate Collection.update
- jashkenas/backbone#570 ability to pass a single attr/value pair in `set' and `save'
- jashkenas/backbone#567 this.el being the jQuery/Zepto object
bruth added a commit to chop-dbhi/cilantro that referenced this pull request Oct 13, 2011
@reconbot
Copy link
Contributor

Hi dalyons, I noticed you don't have options support in this method. I wont be able to pass {silent:true} or error functions.

@sergiocampama
Copy link

This is exactly what I was looking for. I was almost considering doing

vehicles.each (vehicle) ->
    vehicle.fetch()

@sergiocampama
Copy link

I merged this with in my version of backbone, but I had to remove the removeMissing functionality as I don't need it, and hangs up when I use it on firefox (the "the script has become unresponsive", but in a file called HUD or something)

@jashkenas
Copy link
Owner

You can now call fetch and pass the add: true option, which will add fetched methods to the collection, instead of reset-ing the entire collection. Using this in combination with requesting only new models from the server should allow you to get exactly this behavior, without having to make a custom request (not that doing a custom request for stuff like this is necessarily a bad idea)...

@jashkenas jashkenas closed this Jan 23, 2012
@bruth
Copy link

bruth commented Jan 23, 2012

@jashkenas Your solution does not cover all the functionality proposed here. It certainly works for new instances created on the server, but if existing instances are updated on the server or removed, these would not be reflected on the client. A typical scenario is for any kind of polling implementation where something needs to be kept up-to-date on the client.

Also the idea of having a client request what they don't have is a bit odd in practice. Of course this can be done using timestamps with respect to the last request, but it seems simpler for the client to request the whole collection and then figure out what needs updating vs. creating vs. removing to ensure client state is in sync with the server's.

@sergiocampama
Copy link

I have to agree with bruth here. This doesn't add a functionality for updating with the same request an entire collection.

@jashkenas
Copy link
Owner

Yes, you're quite right ... addOrUpdate logic is still something you should be doing in your app -- not something that Backbone should have built-in.

@sergiocampama
Copy link

I think that it should be included. Updating a collection is needed in nearly every kind of app using BackBone. Of course you should have the ability to update (or fetch) a particular record. But when you have a list of many objects, and need to update them all at once, iterating over the elements and applying fetch() is very inefficient, resulting in as many requests as elements you have, when updating a collection would only take one request.

@neolf
Copy link

neolf commented Jun 18, 2012

+1

2 similar comments
@shawndrost
Copy link

+1

@jtreitz
Copy link

jtreitz commented Oct 11, 2012

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet