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

Schrödingers defaultScope: defaultScope exists but at the same time does not #5277

Closed
manuelbieh opened this Issue Jan 21, 2016 · 2 comments

Comments

3 participants
@manuelbieh

manuelbieh commented Jan 21, 2016

I have a model with no defaultScope defined explicitly. So when I do:

Token.scope('defaultScope').findAll()

I get an error:

Error: Invalid scope defaultScope called.
    at W:\dev\node_modules\sequelize\lib\model.js:1214:13

When I try to add a defaultScope using Token.addScope('defaultScope', {}) (without setting the override: true flag) I get another error:

Error: The scope defaultScope already exists. Pass { override: true } as options to silence this error
    at [object Object].Model.addScope (W:\temp\node_modules\sequelize\lib\model.js:1100:11)

So it looks like a defaultScope is not defined thus invalid but at the same time it seems to be defined internally because I get an error that it is already defined (which it is not really).

In my usecase I want to execute database operations based on REST requests. So:
GET /users/2
is mapped to:
Users.scope(req.query.scope || 'defaultScope').findById(req.params.id);

Since defaultScope is not explicitly defined, I get the error above. I think it would be nice (and probably wouldn't cause any problems) if an "empty" defaultScope is always defined (defaulting to {}). Or option 2: allow to set a defaultScope using the addScope() method without being forced to set the override flag.

@manuelbieh

This comment has been minimized.

manuelbieh commented Jan 30, 2016

Thanks! 👍

@codepunkt

This comment has been minimized.

codepunkt commented Mar 24, 2016

@janmeier @mickhansen
I don't think this is a wise choice, as you've now introduced additional magic that is not obvious at first sight.

On the one hand, the addScope documentation now does not reflect the need to pass { override: true } when adding a defaultScope.

I think it's perfectly fine that the scope method errors when you try to set a scope that is not defined (in this case: defaultScope).

@manuelbieh
wouldn't this be an option?

var ScopedUsers = req.query.scope ? Users.scope(req.query.scope) : Users
ScopedUsers.findById(req.params.id)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment