loopback-sdk-angular icon indicating copy to clipboard operation
loopback-sdk-angular copied to clipboard

lb-ng: Issue when working with related models

Open markusiseli opened this issue 9 years ago • 11 comments

Given two simple related models: user and answer, defined as follows: user.json:

...
"relations": {
    "answers": {
    "type": "hasMany",
    "model": "answer",
}
...

and answer.json:

...
"relations": {
    "user": {
    "type": "belongsTo",
    "model": "user"
}
...

The client-side code runs fine, does all the user authentication etc. but creating an answer instance, given the user model using User.answers.create({"id": some_user_id}, {some_answer_json}); throws an Uncaught TypeError: Cannot set property 'create' of undefined.

I tracked the error back to lb-ng: lb-ng creates the following code (lb-services.js) for creation of an answer instance, given the user (R in this case is the user resource):

...
R.answers.create = function() {
    var TargetResource = $injector.get("Answer");
    var action = TargetResource["::create::user::answers"];
    return action.apply(R, arguments);
};
...

The issue with this code is that the property answers of R - as in R.answers - was not defined before defining R.answers.create, hence the error. This error occurs with all similar methods on related models, where the relation name is inserted: e.g. R.answers.createMany, etc.

Here is my very temporary fix: In lb-services.js, manually initialize R.answers = {}; before the line R.anwers.create = function() { works and the answer instance gets created with the creatorId in the DB.

If this is a bug, I'd be glad to help fix it, but would need instructions. If this is not a bug, please let me know what is wrong with my understanding.

markusiseli avatar May 03 '16 21:05 markusiseli

@markusiseli not sure if this is the fix, but you may need to explicitly allow methods for these related models in the user ACL (user.json). See https://docs.strongloop.com/display/public/LB/Accessing+related+models#Accessingrelatedmodels-Restrictingaccesstorelatedmodels

example:

"acls": [
    {
      "principalType": "ROLE",
      "principalId": "$owner",
      "permission": "ALLOW",
      "property": [
        "__create__answers"
      ]
    }
  ]

dancingshell avatar May 09 '16 17:05 dancingshell

@dancingshell thanks for this, it pointed me to the solution. For security reasons I had disabled all methods by default using a mixin and had only activated the __create_answers but not the__get__answers method.

Here is an excerpt from my lb-services.js file with my comments:

// If __get__answers method enabled, the code below will be generated
// If not enabled the code is missing, but should be R.answers = {}, since other methods will depend on this, see next code segment!
R.answers = function() {  
    var TargetResource = $injector.get("Answer");
    var action = TargetResource["::get::user::answers"];
    return action.apply(R, arguments);
};

// If __get__answers method enabled, the code below will be generated
// If not enabled the code is missing
// Note that this code depends on the definition of the code above!
R.answers.create = function() {  
      var TargetResource = $injector.get("Answer");
      var action = TargetResource["::create::user::answers"];
      return action.apply(R, arguments);
};

Thus, for my current models, the fix was to expose the __get__answers method. I would say that this is a BUG - probably not too high priority though.

markusiseli avatar May 09 '16 23:05 markusiseli

Hi @markusiseli, thanks for bringing this up, trying to understand the issue, so basically the bug is: when in relations a get method isnt enabled, the whole Resource isnt defined?


this following line might provide some insight to whomever fixing issue: https://github.com/strongloop/loopback-sdk-angular/blob/279c810747d4061c11f126c19c3e34a67812aa8f/lib/services.template.ejs#L266

// sort the names to make sure the get method creating the scope

davidcheung avatar May 10 '16 15:05 davidcheung

@davidcheung yes. I used Model.disableRemoteMethod(methodName, method.isStatic); in a mixin to disable all methods by default and the __get__* method is part of these methods (see: https://github.com/strongloop/loopback/issues/651). Then I enable only the methods I'd like to expose, like __create__*. I can generate a gist, please let me know.

markusiseli avatar May 10 '16 19:05 markusiseli

@markusiseli thanks! that would be great, I think the main goal of fix should be making sure the behaviour is the same as exposed from strong-remoting, hoping to pin point exactly what triggers the relation resource to be undefined so whomever fixing this bug will have an easier time.

davidcheung avatar May 10 '16 21:05 davidcheung

@davidcheung please see following repository (did not succeed in generating gist): https://github.com/markusiseli/loopback-angular-sdk-test I hope this is ok...

markusiseli avatar May 11 '16 01:05 markusiseli

@davidcheung, threw a waiting label on here even though in this case we are waiting for you! :wink:

@markusiseli, we generally prefer a repo over a gist! Makes our lives easier when digging for the root cause of an issue, thanks.

richardpringle avatar May 11 '16 13:05 richardpringle

Thanks @markusiseli! 😄 I probably won't be able to get to it today, but I think it should be enough for me to reproduce the issue

davidcheung avatar May 11 '16 18:05 davidcheung

Thanks @markusiseli, I can reproduce this problem, since explorer can handle this, meaning its bug to the angular-sdk (not issue with strong-remoting) might need to add a logic in buildScopeMethod() to handle this situation since the code is relying on get to handle this

davidcheung avatar May 19 '16 13:05 davidcheung

I have this problem as well without any acls setup

DecentGradient avatar Jul 07 '16 00:07 DecentGradient

I think this is the relevant code that's assuming a get method will be always present in a scope: lib/services.template.ejs#L277-L279:

      // sort the names to make sure the get method creating the scope
      // is emitted first (R.categories before R.categories.create)
      Object.keys(scopeMethods).sort().forEach(function(methodName) {

The fix should be relatively straightforward - check whether scopeMethods contain a top-level get method and if it does not, then emit an empty scope object.

Are there any volunteers for contributing this fix? I am happy to help you along the way, just mention my GH handle in the pull request description.

bajtos avatar Jun 07 '17 12:06 bajtos