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

Adding scopes after a model has been defined #3963

Closed
bomattin opened this Issue Jun 18, 2015 · 11 comments

Comments

6 participants
@bomattin

bomattin commented Jun 18, 2015

In the models/index.js file generated by Sequelize-CLI, when the files are read and their models are appended, this is done alphabetically:

models/index.js

fs
  .readdirSync(__dirname)
  .filter(function(file) {
    return (file.indexOf(".") !== 0) && (file !== basename);
  })
  .forEach(function(file) {
    var model = sequelize["import"](path.join(__dirname, file));
    db[model.name] = model;
  });

Object.keys(db).forEach(function(modelName) {
  if ("associate" in db[modelName]) {
    db[modelName].associate(db);
  }
});

I suspect this makes for an awkward situation like the following: Say I have 3 models, Group, GroupType and User.

In the defaultScope of User, I can include Group with the following:

include: [{model: Sequelize.models.Group}]

However, if I try to include GroupType in Group's defaultScope, Sequelize fails silently and ignores the include. Why? Well, I'm not sure really. I added a console.log line right before I returned the object for export:

models/group.js

module.exports = function(sequelize, DataTypes) {
    var group = sequelize.define("group", {
        ...
        ...
    }, {
        classMethods: {
            associate: function(models) {

                ...
                group.belongsTo(models.groupType, {
                    foreignKey:"grouptypeid"
                });
            }
        }
    }, {
        defaultScope: {
            include: [
                { model: sequelize.models.groupType }
            ]
        } // Doesn't actually work
    });
    console.log(sequelize.models);
    return group;
};

And sure enough, groupType is NOT in the sequelize.models object. When I get Group models from a finder, Sequelize fails silently and returns just the Groups, without their types, as though the include was never written.

@janmeier

This comment has been minimized.

Member

janmeier commented Jun 19, 2015

Hmm, I agree that includes should probably throw an error if the argument passed is undefined.

I have thought about this before (when re-writing scopes actually) - and there is currently no good solution. Of course you could sort the models in the order that best suits your case - in this specific case making sure that groupType is loaded before group.

A better solution to this would probably be to have an addScope function for these kind of cases. You could then call this function when all models are loaded (in associate or similar)

@bomattin

This comment has been minimized.

bomattin commented Jun 19, 2015

Glad this is truly a bug and not just programmer error!

I agree, I think an addScope function would be the most elegant solution. Adding scopes in something like index.js might seem strange at first, but I think it could potentially be a helpful design pattern.

I'll see if I can help contribute to some documentation for the time being.

@janmeier janmeier changed the title from Models are loaded into Sequelize alphabetically, making includes in defaultScope difficult to Adding scopes after a model has been defined Jun 21, 2015

@janmeier janmeier added feature and removed bug discussion labels Jun 21, 2015

@aarosil

This comment has been minimized.

aarosil commented Jun 30, 2015

@janmeier do you have any tips about implementing this addScope function, how I could do this? I have about 40 models and counting so manually ordering them seems daunting 😧

@janmeier

This comment has been minimized.

Member

janmeier commented Jul 1, 2015

@aarosil This is actually the only code that handles scopes on the model now https://github.com/sequelize/sequelize/blob/master/lib/model.js#L647-L651. So an addScope function should call conformIncludes and add the scope to this.options.scopes

@ghost

This comment has been minimized.

ghost commented Jul 2, 2015

@bomattin I have found a workaround:

module.exports = function(sequelize, DataTypes) {
    var group = sequelize.define("group", {
        ...
        ...
    }, {
        classMethods: {
            associate: function(models) {

                ...
                group.belongsTo(models.groupType, {
                    foreignKey:"grouptypeid"
                });
            }
        }
    }, {
        scopes: {
            includeType: function() {
                return {
                    include: [{association: group.associations.groupType}]
                };

                // or

                var groupType = group.associations.groupType.target;

                return {
                    include: [{model: groupType}]
                };
            }
        }
    });
    console.log(sequelize.models);
    return group;
};

However, this won't work with defaultScope.

@bomattin

This comment has been minimized.

bomattin commented Jul 8, 2015

@irobert91 Workaround works great. Nice find! Thanks.

@janmeier janmeier closed this in c9edd42 Jul 25, 2015

@markdboyd

This comment has been minimized.

markdboyd commented Nov 3, 2015

Not sure if this is the right place to ask, but it seemed like the most relevant issue. I've noticed that, regardless of what order I specify to load my models, sometimes I get issues where models are undefined at the time I try to reference them in scopes. However, if I define the scope as a function instead of a JS object, things work fine, presumably because the evaluation of the scope as a function is deferred. Is that correct? Is there a better way or should I stick with this method for now?

For reference, this works:

scopes: {
  categories: function() {
    return {
      include: [
        {
          model: models.category,
        }
      ]
    };
  }

But this doesn't:

scopes: {
  categories: {
      include: [
        {
          model: models.category,
        }
      ]
    }
  }
@bomattin

This comment has been minimized.

bomattin commented Nov 6, 2015

@markdboyd That's fascinating. I suspect you're correct, it absolutely must be because the function evaluation is deferred.

We do have Model.addScope() now, so it depends on which method you prefer. Personally, I prefer having all my model information in one place, so I'd prefer your method if I weren't already using addScope() in models/index.js.

@markdboyd

This comment has been minimized.

markdboyd commented Nov 9, 2015

Yeah, I prefer keeping all my model information together as well. I guess I'll keep this workaround for now

@aray12

This comment has been minimized.

aray12 commented May 14, 2016

I know this has been closed but I have found a problem with this workaround. I have been setting my scopes and functions that return an object, but the documentation says that defaultScope must be just a plain object. So it fails in that case. Is there another workaround people are using for that scenario?

@alinalexa

This comment has been minimized.

alinalexa commented Nov 24, 2016

how about this method?

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