Skip to content
This repository was archived by the owner on Apr 11, 2023. It is now read-only.

Add support for setting collection with a promise #103

Open
reubano wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add support for setting collection with a promise #103

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
353d169
Add support for setting collection with a promise
reubano Mar 28, 2014
8255088
Fix async reference to model
reubano Apr 3, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions backbone.dualstorage.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,13 +274,21 @@ dualsync = (method, model, options) ->
collection = model
idAttribute = collection.model.prototype.idAttribute
localsync('clear', collection, options) unless options.add
for modelAttributes in resp
model = collection.get(modelAttributes[idAttribute])
if model
responseModel = modelUpdatedWithResponse(model, modelAttributes)
else
responseModel = new collection.model(modelAttributes)
localsync('update', responseModel, options)

@nilbus nilbus May 10, 2014

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a reminder to either one of us to remove the indentation on this blank line.

setResp = (resp, model) ->
for modelAttributes in resp
model = collection.get(modelAttributes[idAttribute])
if model
responseModel = modelUpdatedWithResponse(model, modelAttributes)
else
responseModel = new collection.model(modelAttributes)
localsync('update', responseModel, options)

if resp?.done
do (model) -> resp.done (data) -> setResp(data, model)

@nilbus nilbus May 10, 2014

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed you added a do (model) -> closure wrapper, and you also are now passing model to setResp. Is this just to help with understandability with the model/collection naming mess, or is there a functional benefit that I'm missing?

@reubano reubano May 12, 2014

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The closure is needed because .done is aysnc. Without the closure, the value of model will have changed before .done is resolved. http://coffeescript.org/#slices (section directly above this). I'm passing model to setResp to preserve the original functionality.

else
setResp(resp, model)

@nilbus nilbus May 10, 2014

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indentation also needs to be fixed.


else
responseModel = modelUpdatedWithResponse(model, resp)
localsync('update', responseModel, options)
Expand Down