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

handleFindAll() and buildList() can make diverse models #161

Merged
merged 3 commits into from
Dec 10, 2015

Conversation

afn
Copy link
Contributor

@afn afn commented Dec 9, 2015

The handleFindAll function is a bit lacking in that all of the models it generates are identical (except for sequences). This PR allows it to be called with a list of traits/options instead of a number.

// findAll('user') will return Alice, Bob, and Charlotte (who is an admin)
TestHelper.handleFindAll('user', {name: 'Alice'}, {name: 'Bob'}, ['admin', {name: 'Charlotte'}]);

@danielspaniel
Copy link
Collaborator

This is a really interesting idea .. me like!
So, if this is alternative to

 handleFindAll('user', 2, 'trait1', {name: 'blah'})

how to do you differentiate between that and

TestHelper.handleFindAll('user', {name: 'Alice'}, {name: 'Bob'}, ['admin', {name: 'Charlotte'}]);

do you just say .. hey .. not number .. look for all args as separate to each new user?
and is ['admin', {name: 'Charlotte'}] supposed to mean 'admin' is trait ( the way to mix traits and opts )

@afn
Copy link
Contributor Author

afn commented Dec 9, 2015

That's right: if the second argument is not a number, then it's assumed that the second and subsequent arguments are the specification for each model. Each of these arguments can be an array ['trait1', 'trait2', {key: 'value'}], an object {key: 'value'}, or a single trait 'trait1'.

@danielspaniel
Copy link
Collaborator

This clearly is a fun feature. So thanks for the great idea. Can you add a few more tests ( you have one I think ) .. need one or two for handleFindAll and then change the README to show the new feature ..

Tony Novak added 2 commits December 9, 2015 14:47
Also making the test more rigorous (previously, it could have passed even if
the created_at attribute was not being set correctly)
@afn
Copy link
Contributor Author

afn commented Dec 10, 2015

@danielspaniel How does that look?

danielspaniel added a commit that referenced this pull request Dec 10, 2015
handleFindAll() and buildList() can make diverse models
@danielspaniel danielspaniel merged commit 1fec17f into adopted-ember-addons:master Dec 10, 2015
@danielspaniel
Copy link
Collaborator

Pretty good mate .. thanks! :)

@afn
Copy link
Contributor Author

afn commented Dec 16, 2015

Thanks for merging this so quickly! Are you planning a new release soon?

@danielspaniel
Copy link
Collaborator

Yes, I will do this today ( I was wondering when you would ask .. so thanks for the nudge ). Was waiting to do other upgrades, but I am taking too long.

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