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

Difficult to implement "find or create" #1523

Closed
simonexmachina opened this issue Nov 27, 2013 · 8 comments
Closed

Difficult to implement "find or create" #1523

simonexmachina opened this issue Nov 27, 2013 · 8 comments

Comments

@simonexmachina
Copy link
Contributor

The following code is I think a reasonable way to implement a "find or create if not found" method, but it fails an assertion because "The id foo has already been used with another record of type (subclass of DS.Model)"

findOrCreate: (type, properties)->
  deferred = Em.RSVP.defer()
  @store.find(type, properties.id)
    .then deferred.resolve, (err)=>
      if err.status == 404
        @store.createRecord(type, properties).save()
          .then deferred.resolve, deferred.reject
      else
        deferred.reject err
  deferred.promise

The workaround is as follows, but I think that calling createRecord().save() shouldn't result in a failed assertion.

findOrCreate: (type, properties)->
  deferred = Em.RSVP.defer()
  @store.find(type, properties.id)
    .then deferred.resolve, (err)=>
      if err.status == 404
        record = @store.recordForId(type, properties.id)
        record.loadedData()
        record.setProperties properties
        record.save()
          .then deferred.resolve, deferred.reject
      else
        deferred.reject err
  deferred.promise
@stefanpenner
Copy link
Member

your last example should reduce to:
The intermediate deferred is not needed as promise/a+ chaining and assimilation semantics got your back bro.

findOrCreate: (type, properties)->
  @store.find(type, properties.id).fail (reason)=>
    if reason.status == 404
      record = @store.recordForId(type, properties.id)
      record.loadedData()
      record.setProperties properties
      record.save()
    else
      throw reason

@simonexmachina
Copy link
Contributor Author

That's so awesome :)

Actually, now that I look at it having to call loadedData is no problem - just seemed like a bug at first. Closing.

@simonexmachina
Copy link
Contributor Author

Actually, only problem there is that there's no fail method in the PromiseProxyMixin. Easily fixed, I'll submit a PR now. Workaround for now is fine, but not as readable:

findOrCreate: (type, properties)->
  @store.find(type, properties.id).then null, (reason)=>
    if reason.status == 404
      record = @store.recordForId(type, properties.id)
      record.loadedData()
      record.setProperties(properties)
      record.save()
    else
      throw reason

@stefanpenner
Copy link
Member

ah good catch.

@simonexmachina
Copy link
Contributor Author

I've made the change and added a test, but can't send the PR through GitHub until I move another fork that's already on my account. Will send it over when that's done - it's hardly a big issue :)

Hey @stefanpenner, I'd really like to have a quick chat about my Ember Data pagination PR - can I get you on chat for a few minutes? Is there an IRC room we can meet in?

@stefanpenner
Copy link
Member

channle: #ember on freenode should work.

@simonexmachina
Copy link
Contributor Author

Okay, I'm on there as aexmachina (I think - I'm an IRC noob!)

@stefanpenner
Copy link
Member

oops it #ember.js

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

No branches or pull requests

2 participants