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

Multi owner for one model #2441

Closed
upupzealot opened this issue Jun 16, 2016 · 5 comments
Closed

Multi owner for one model #2441

upupzealot opened this issue Jun 16, 2016 · 5 comments
Assignees

Comments

@upupzealot
Copy link

$owner is defined by the method
Role.isOwner = function isOwner(modelClass, modelId, userId, callback)
It will define if the current user is an "owner" of the model instance through key owner or userId or belongsTo relations

The problem is what if a model has two(or more) owners(through relation)?
The codes here says the method only checks the first belongsTo relation.
But what if the user is an owner through another belongsTo?

@upupzealot
Copy link
Author

upupzealot commented Jun 16, 2016

I tried to fix this by change the method like this

Role.isOwner = function isOwner(modelClass, modelId, userId, callback) {
  debug('overrided!');
  assert(modelClass, 'Model class is required');
  debug('isOwner(): %s %s userId: %s', modelClass && modelClass.modelName, modelId, userId);
  // No userId is present
  if (!userId) {
    process.nextTick(function() {
      callback(null, false);
    });
    return;
  }

  // Is the modelClass User or a subclass of User?
  if (isUserClass(modelClass)) {
    process.nextTick(function() {
      callback(null, matches(modelId, userId));
    });
    return;
  }

  modelClass.findById(modelId, function(err, inst) {
    if (err || !inst) {
      debug('Model not found for id %j', modelId);
      if (callback) callback(err, false);
    }
    debug('Model found: %j', inst);
    var ownerId = inst.userId || inst.owner;
    // Ensure ownerId exists and is not a function/relation
    if (ownerId && 'function' !== typeof ownerId) {
      if (callback) callback(null, matches(ownerId, userId));
    } else {
      var rel_tries = 0;
      var matched = false;
      function processRelatedUser(err, user) {
        if(matched) {
          return;
        }
        var checkFinish = function() {
          if(rel_tries === modelClass.relations.length) {
            debug('No matching belongsTo relation found for model %j and user: %j', modelId, userId);
            if (callback) callback(null, false);
          }
        }
        if (!err && user) {
          rel_tries++;
          debug('User found: %j', user.id);
          var result = matches(user.id, userId);
          debug('User matches result: %s !', result);
          if(result) {
            matched = true;
            if (callback) callback(null, matches(user.id, userId));
          }
          checkFinish();
        } else {
          rel_tries++;
          checkFinish();
        }
      }

      // Try to follow belongsTo
      for (var r in modelClass.relations) {
        var rel = modelClass.relations[r];
        if (rel.type === 'belongsTo' && isUserClass(rel.modelTo)) {
          debug('Checking relation %s to %s: %j', r, rel.modelTo.modelName, rel);
          inst[r](processRelatedUser);
        }
      }
    }
  });
};

@0candy
Copy link
Contributor

0candy commented Jun 17, 2016

We don't currently support following relations for multiple owners as you pointed out above. Would you like to submit a PR to add this feature? Thanks!

@0candy 0candy added feature and removed triaging labels Jun 17, 2016
@upupzealot
Copy link
Author

@0candy Yeah I'd like to.
I would do that as soon as I get time.

@upupzealot
Copy link
Author

@0candy #2454

@gvillenave
Copy link

Any update on when the PR will be available in master?

@stale stale bot added the stale label Aug 23, 2017
@stale stale bot closed this as completed Sep 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants