Skip to content
This repository was 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
clark0x wants to merge 1 commit into
strongloop-archive:masterfrom
clark0x:fix/built-in-models-not-found
Open

Allow another name convention for User model and AccessToken model#26
clark0x wants to merge 1 commit into
strongloop-archive:masterfrom
clark0x:fix/built-in-models-not-found

Conversation

@clark0x

Copy link
Copy Markdown
Contributor

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 clark.wangs@gmail.com

@slnode

Copy link
Copy Markdown

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
Copy Markdown
Contributor

@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
Copy Markdown
Contributor

ok to test

Comment threadtest/test.js Outdated

Copy link
Copy Markdown
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
Copy Markdown

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

@clark0x

Copy link
Copy Markdown
ContributorAuthor

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

Comment threadlib/helpers.js

Copy link
Copy Markdown

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
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@justin-lauthis 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
Copy Markdown
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 <clark.wangs@gmail.com>
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 <clark.wangs@gmail.com>
refactor according to @ritch's notes
Signed-off-by: Clark Wang <clark.wangs@gmail.com>
use a simple approach
Signed-off-by: Clark Wang <clark.wangs@gmail.com>
remove unused dep
Signed-off-by: Clark Wang <clark.wangs@gmail.com>
pass tests
Signed-off-by: Clark Wang <clark.wangs@gmail.com>
fix missing dep
upgrade to use lodash string util methods
Signed-off-by: Clark Wang <clark.wangs@gmail.com>
@clark0x
clark0xforce-pushed the fix/built-in-models-not-found branch from 8a472d8 to 63a1727CompareMarch 18, 2015 02:28
@clark0xclark0x changed the title Fix built-in models not foundAllow another name convention for User model and AccessToken modelMar 18, 2015
@clark0x

Copy link
Copy Markdown
ContributorAuthor

@ritch could you please review and land?

@zero5100

Copy link
Copy Markdown
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
Copy Markdown
Contributor

See #57 for a potential fix for this.

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

Development

Successfully merging this pull request may close these issues.

7 participants

@clark0x@slnode@bajtos@ritch@guanbo@zero5100@justin-lau