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

Add better error messaging for adapters that do not implement createR… #4283

Merged
merged 1 commit into from
Oct 23, 2016

Conversation

bmac
Copy link
Member

@bmac bmac commented Mar 28, 2016

…ecord, updateRecord or deleteRecord.

@bmac bmac force-pushed the commit-assert branch 2 times, most recently from 6244659 to c540b27 Compare April 15, 2016 00:54
@@ -562,7 +562,7 @@ Store = Service.extend({
var adapter = this.adapterFor(typeClass.modelName);

assert("You tried to find a record but you have no adapter (for " + typeClass + ")", adapter);
assert("You tried to find a record but your adapter (for " + typeClass + ") does not implement 'findRecord'", typeof adapter.findRecord === 'function' || typeof adapter.find === 'function');
assert("You tried to find a record but your adapter (for " + typeClass + ") does not implement 'findRecord'", typeof adapter.findRecord === 'function');
Copy link
Member

Choose a reason for hiding this comment

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

👍

@pangratz
Copy link
Member

pangratz commented May 4, 2016

This looks good. Do you think there should be tests for updateRecord and deleteRecord as well? Or is the one test sufficient since all three use _commit? Your call, feel free to merge!

@pangratz pangratz self-assigned this May 23, 2016
@bmac bmac force-pushed the commit-assert branch from c540b27 to 0911b46 Compare May 23, 2016 14:17
@pangratz pangratz merged commit 05e8436 into emberjs:master Oct 23, 2016
@pangratz
Copy link
Member

🎉

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

Successfully merging this pull request may close these issues.

2 participants