Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Allow another name convention for User model and AccessToken model #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

clark0x
Copy link
Contributor

@clark0x clark0x commented Aug 18, 2014

If project doesn't extend built-in models User and
AccessToken as user and accessToken, givenModel()
can NOT find these models by keys user and accessToken via
app.models[key].

By classify the modelName, givenModel() can always find the
proper built-in model.

Note: When user model extends User model, it will register
both user and User to app.models.

FYI: This doesn't fix the case of extending User model with
other name (e.g. Account), and AccessToken also.

Signed-off-by: Clark Wang [email protected]

@slnode
Copy link

slnode commented Aug 18, 2014

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@bajtos
Copy link
Contributor

bajtos commented Sep 17, 2014

@ritch could you please review and land? I did a superficial review and did not find any issues, but you know the code better than I do.

@ritch
Copy link
Contributor

ritch commented Sep 18, 2014

ok to test

helpers.beforeEach.withApp(testApp, { userModel: Account });
helpers.beforeEach.givenUser({ email: '[email protected]', password: '000000' });
it('should create an user of Account type', function () {
assert(this['account'] instanceof Account);
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: this.account

@guanbo
Copy link

guanbo commented Sep 23, 2014

@clarkorz, can you recommit and make @ritch merge into master. I have same problem. thanks!

@clark0x
Copy link
Contributor Author

clark0x commented Dec 9, 2014

@ritch I have switched to a more simpler approach. Could you review again?

@@ -207,11 +213,11 @@ _beforeEach.givenLoggedInUserWithRole = function(credentials, role, optionalHand
}

_beforeEach.givenAnUnauthenticatedToken = function(attrs, optionalHandler) {
_beforeEach.givenModel('accessToken', attrs, optionalHandler);
_beforeEach.givenModel('AccessToken', attrs, optionalHandler);

Choose a reason for hiding this comment

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

Shouldn't it be options.AccessToken?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@justin-lau this doesn't point to the test object. 'AccessToken' is the key of options, it will be use to retrieve the configured model name in _beforeEach#givenModel().

@zero5100
Copy link
Contributor

Could someone resolve this please? I use a different naming convention for the model overrides in my application.

If project doesn't extend built-in models **User** and
**AccessToken** as **user** and **accessToken**, `givenModel()`
can NOT find these models by keys `user` and `accessToken` via
`app.models[key]`.

By capitalizing the keys, `givenModel()` can always find the
proper built-in model.

Note: When `user` model extends `User` model, it will register
 both `user` and `User` to `app.models`.

Signed-off-by: Clark Wang <[email protected]>

Allow to specify user and accessToken models

Add an options param to `lt.beforeEach.withApp()`, allow to specify
the user model and/or accessToken model that will be used in the
tests.

Signed-off-by: Clark Wang <[email protected]>

refactor according to @ritch's notes

Signed-off-by: Clark Wang <[email protected]>

use a simple approach

Signed-off-by: Clark Wang <[email protected]>

remove unused dep

Signed-off-by: Clark Wang <[email protected]>

pass tests

Signed-off-by: Clark Wang <[email protected]>

fix missing dep

upgrade to use lodash string util methods

Signed-off-by: Clark Wang <[email protected]>
@clark0x clark0x force-pushed the fix/built-in-models-not-found branch from 8a472d8 to 63a1727 Compare March 18, 2015 02:28
@clark0x clark0x changed the title Fix built-in models not found Allow another name convention for User model and AccessToken model Mar 18, 2015
@clark0x
Copy link
Contributor Author

clark0x commented Mar 18, 2015

@ritch could you please review and land?

@zero5100
Copy link
Contributor

roleMapping is also referenced by name on line 141 and 143. It would be nice if the model names loopback-testing uses could be overridden at runtime when importing the module. I might fork this later and implement something like that to get this working for me without needing the extra model extensions.

@zero5100
Copy link
Contributor

See #57 for a potential fix for this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

7 participants