Skip to content

Updated to support MongoDB which uses "_id" property instead of "id" #7

Closed
wants to merge 1 commit into from

2 participants

@vizo
vizo commented Mar 12, 2012

No description provided.

@dgeb
Cerebris Corporation member
dgeb commented Mar 12, 2012

Thanks for the PR. I've heard from other MongoDB users who were tripped up by _id and I'd be glad to make ember-rest more compatible.

For consistency, I'd prefer to go with _resourceId() instead of _recordId().

Also, return this.get('id') || this.get('_id'); will be a slight performance hit for MongoDB users. You'd be better off to override _resourceId() for every Ember.Resource like this:

Ember.Resource.reopen({
  _resourceId: function() {
    return this.get('_id');
  }
});

I'd prefer to just support the one default id scheme and let people override it as needed.

If you want to modify this PR and rebase, I'd be glad to merge it. Otherwise, I could make these tweaks for you quickly. Just let me know.

@vizo
vizo commented Mar 12, 2012

Maybe it would be good that "primary key" is just property somewhere like 'resourcePrimaryKey' or something .. ?

If it's ok to you, please make these tweaks. Thanks :)

@dgeb
Cerebris Corporation member
dgeb commented Mar 12, 2012

@vizo Good suggestion! I introduced a new resourceIdField, which is set to id by default. This should be even easier to override in your models. I also renamed _id() to _resourceId() as discussed.

See this commit for all the details.

@dgeb dgeb closed this Mar 12, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.