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

Breaking change in Ember Data 3.2: Can no longer create a relation and persist it #5728

Closed
psbanka opened this issue Oct 31, 2018 · 5 comments

Comments

@psbanka
Copy link

psbanka commented Oct 31, 2018

Description

Prior to Ember Data 3.2, it used to be possible, given the following models:

// TlsSubscription
export default DS.Model.extend({
  tlsDomains: hasMany('tlsDomain'),
})

// TlsDomain
export default DS.Model.extend({
})

to do the following:

const newTlsDomain = this.store.createRecord('tls_domain', {
  id: domainName,
})
const customer = this.get('store').peekRecord('customer', customerId)
this.get('tlsSubscriptions').createRecord({
  certificateAuthority: 'lets-encrypt',
  tlsDomains: [newTlsDomain]
}).save()

That would produce the following (valid) JSON-API payload to the server:

{
  "data": {
    "attributes": {
      "certificate_authority": "lets-encrypt",
    },
    "relationships": {
      "tls_domains": {
        "data": [
          {
            "type": "tls_domain",
            "id": "test.com"
          }
        ]
      }
    },
    "type": "tls_subscription"
  }
}

However, due to this pull-request: #5317 from @ryanto

Ember Data now refuses to serialize the object that has not yet been persisted to the server, and now we get the following payload:

{
  "data": {
    "attributes": {
      "certificate_authority": "lets-encrypt",
    },
    "relationships": {},
    "type": "tls_subscription"
  }
}

It seems to me that this is a breaking API change and should have been delayed until the 4.x release (although this is a PR from about a year ago, so that ship has probably already sailed). It also seems to be a useful feature in Ember Data to be able to construct an API request in this fashion without necessarily saving other records off to the database.

@jamesarosen
Copy link

jamesarosen commented Oct 31, 2018

Essentially, the problem is that #5317 didn't consider the case of client-generated IDs.

Some hacks that might work, or might cause more problems:

// model:tls-domain
isDeleted: computed(() => false),
isLoaded: computed(() => true),
isLoading: computed(() => false),
isNew: computed(() => false),
isReloading: computed(() => false),
isSaving: computed(() => false),

or

const newTlsDomain = this.store.createRecord('tls_domain', {
  id: domainName,
  isNew: false,
})

@jamesarosen
Copy link

Last option: serializeBelongsTo is part of the public API. The answer could just be "override it." Unfortunately, it's a lot of logic to duplicate in a custom serializer.

@psbanka
Copy link
Author

psbanka commented Oct 31, 2018

well, isNew: false on the model did the trick (did not need to be a CP), so as for hacks that seems to be the least horrible for now. Thanks, @jamesarosen

@ryanto
Copy link
Contributor

ryanto commented Oct 31, 2018

Yikes, sorry about that @psbanka !

If you're building a record that's not new, in other words it has an ID that the backend would know what to do with, then I would recommend using store.push instead of createRecord. That will let you add models to the store that the backend knows about.

You can also override serializeBelongsTo if you need this functionality, it's the best place for this sort of escape hatch.

@psbanka
Copy link
Author

psbanka commented Nov 2, 2018

@ryantostore.push() works like a charm. Thank you! 👍

@psbanka psbanka closed this as completed Nov 2, 2018
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

3 participants