Skip to content
This repository has been archived by the owner on Apr 18, 2020. It is now read-only.

User model present in model-config.json #103

Closed
1 of 2 tasks
greaterweb opened this issue Feb 20, 2017 · 11 comments
Closed
1 of 2 tasks

User model present in model-config.json #103

greaterweb opened this issue Feb 20, 2017 · 11 comments

Comments

@greaterweb
Copy link

Bug or feature request

  • Bug
  • Feature request

Additional information (Node.js version, LoopBack version, etc)

Both the base User model and a custom user model (which extends User) are present in the model-config.json. I don't believe this is intentional and the base User model should be removed from the config.

At the minimum it is confusing for files such as common/models/user.js, it is not immediately clear which model this would apply to.

To resolve, the base User model should be removed or the custom user model should be renamed to something else such as Account.

@superkhau
Copy link
Contributor

superkhau commented Feb 20, 2017

@greaterweb It is intentional. User is a builtin model and user is a user-defined model. We really should just rename it Customer (or Account like you suggested something to prevent confusion). It's been there for a long time -- if you would like to contribute a fix, I can help you get it landed.

Note that User is private while user is public in the model-config.

@superkhau
Copy link
Contributor

cc @crandmck -- we really should get this fixed lol still going around

@greaterweb
Copy link
Author

Thank you for the prompt response.

Could you please explain why you would need the builtin User model if you've created a custom model which extends it? I've got a fair amount of experience in using Loopback v2 and many times using the built in User model as the base for my custom user/account model.

Is something different in v3 which has this as the recommended approach? I don't have any instances where the builtin model was required in addition to the model I created. It's not entirely clear in this repo why you would need both and I can't find anything in the docs supporting the need to do so.

I realize the issue system isn't intended to be a support mechanism but in this case this ticket is actually secondary to a bug I'm was about to submit to the main loopback repo.

@superkhau
Copy link
Contributor

superkhau commented Feb 21, 2017

@greaterweb The builtin User is used for various parts of the LB system -- ie. ACLs, Roles, AccessToken, etc. So it needs to be part of a default LB scaffold -- that said, it is restricting in many ways so we recommend extending it to enable full customization.

Is something different in v3 which has this as the recommended approach? I don't have any instances where the builtin model was required in addition to the model I created. It's not entirely clear in this repo why you would need both and I can't find anything in the docs supporting the need to do so.

I don't believe anything is different in LB with regards to the usage of the builtin User model -- if you remove it, you'll have to hook up your own to make up for the parts LoopBack is expecting OOB.

@greaterweb
Copy link
Author

@superkhau you may wish to follow the discussions in strongloop/loopback#3215

I guess what we are really looking at is the larger topic on whether or not it is required to include the User model reference in model-config.json if I'm extending it. Prior to v3.3.0 it wasn't required to do so to get the "out of the box" benefits of the User model, with the latest loopback release that appears to have changed.

@superkhau
Copy link
Contributor

Yes, I saw your comment with @raymondfeng (especially this one strongloop/loopback#3215 (comment)) which is correct. I wasn't sure if you could've removed User in versions <3.3.0 -- did you try reproducing on a fresh 2.x project vs 3.x to prove it? Otherwise, it's a breaking change that should've never landed since it wasn't mentioned in the 3.x docs IMO. Anyways, lets confirm first.

@crandmck
Copy link
Contributor

crandmck commented Feb 21, 2017

we really should get this fixed lol still going around

The general concept of extending the User model for your own purposes is laid out pretty clearly in
http://loopback.io/doc/en/lb3/Managing-users.html. However, it doesn't say you have to leave the entry for User in model-config.json. I always assumed that to extend a model, you would leave the base model "as is". LMK if I should clarify.

Also, if something did change wrt this between v2 & v3, then we definitely need to document it. I certainly was not aware of it! Please advise. Even if it's a bug that we fix, we should say in the RNs which v3 versions are affected.

@superkhau
Copy link
Contributor

superkhau commented Feb 22, 2017

The bug is being handled by @miroslav at strongloop/loopback#3215 (comment) already. No doc changes are required ATM.

What I meant was change the user to Customer or something to prevent confusion in the example instead of User to user remember? This hasn't changed for a long time now. ;)

@crandmck
Copy link
Contributor

What I meant was change the user to Customer or something to prevent confusion in the example instead of User to user remember? This hasn't changed for a long time now. ;)

Oh, right... It would be a good change, and should be relatively straightforward. Perhaps something for @siddhipai ?

@superkhau
Copy link
Contributor

@crandmck Good idea. :)

@superkhau
Copy link
Contributor

superkhau commented Feb 22, 2017

@siddhipai See #73, reassigning this to you. Also try to go through his example when you have time too.

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

No branches or pull requests

4 participants