warn if forgot to pass options.where (#6150)

lib/model.js

@@ -1277,10 +1277,7 @@ class Model {
     if (options !== undefined && !_.isPlainObject(options)) {
     if (options !== undefined && !_.isPlainObject(options)) {
       throw new Error('The argument passed to findAll must be an options object, use findById if you wish to pass a single primary key value');
       throw new Error('The argument passed to findAll must be an options object, use findById if you wish to pass a single primary key value');
     }
     }
-    // TODO: Remove this in the next major version (4.0)
-    if (arguments.length > 1) {
-      throw new Error('Please note that find* was refactored and uses only one options object from now on.');
-    }
+
     const tableNames = {};
     const tableNames = {};
     let originalOptions;
     let originalOptions;
 
 
@@ -1289,9 +1286,16 @@ class Model {
 
 
     _.defaults(options, { hooks: true, rejectOnEmpty: this.options.rejectOnEmpty });
     _.defaults(options, { hooks: true, rejectOnEmpty: this.options.rejectOnEmpty });
 
 
-      //set rejectOnEmpty option from model config
+    // set rejectOnEmpty option from model config
     options.rejectOnEmpty = options.rejectOnEmpty || this.options.rejectOnEmpty;
     options.rejectOnEmpty = options.rejectOnEmpty || this.options.rejectOnEmpty;
 
 
+    // forgot to pass options.where ?
+    if (!options.where) {
+      let commonKeys =  _.intersection(_.keys(options), _.keys(this.rawAttributes));
+      // jshint -W030 
+      commonKeys.length && Utils.warn(`Model attributes (${commonKeys.join(',')}) found in finder method options but options.where object is empty. Did you forget to use options.where?`);
+    }
+
     return Promise.try(() => {
     return Promise.try(() => {
       this._conformOptions(options, this);
       this._conformOptions(options, this);
       this.$injectScope(options);
       this.$injectScope(options);

lib/utils.js

@@ -16,6 +16,7 @@ exports.Promise = Promise;
 exports._ = _;
 exports._ = _;
 exports.debug = logger.debug.bind(logger);
 exports.debug = logger.debug.bind(logger);
 exports.deprecate = logger.deprecate.bind(logger);
 exports.deprecate = logger.deprecate.bind(logger);
+exports.warn = logger.warn.bind(logger);
 exports.getLogger = () => ( logger );
 exports.getLogger = () => ( logger );
 
 
 function useInflection(_inflection) {
 function useInflection(_inflection) {

lib/utils/logger.js

@@ -32,6 +32,10 @@ class Logger {
     this.config.debug && this.debug(message);
     this.config.debug && this.debug(message);
   }
   }
 
 
+  warn(message) {
+    console.warn(`(${this.config.context}) Warning: ${message}`);
+  }
+
   debugContext(childContext) {
   debugContext(childContext) {
     if (!childContext) {
     if (!childContext) {
       throw new Error('No context supplied to debug');
       throw new Error('No context supplied to debug');

test/integration/associations/belongs-to.test.js

@@ -671,7 +671,7 @@ describe(Support.getTestDialectTeaser('BelongsTo'), function() {
         return User.create({ username: 'bob' }).then(function(newUser) {
         return User.create({ username: 'bob' }).then(function(newUser) {
           return Task.create({ title: 'some task' }).then(function(newTask) {
           return Task.create({ title: 'some task' }).then(function(newTask) {
             return newTask.setUser(newUser).then(function() {
             return newTask.setUser(newUser).then(function() {
-              return Task.findOne({title: 'some task'}).then(function (foundTask) {
+              return Task.findOne({ where: { title: 'some task' } }).then(function (foundTask) {
                 return foundTask.getUser().then(function (foundUser) {
                 return foundTask.getUser().then(function (foundUser) {
                   expect(foundUser.username).to.equal('bob');
                   expect(foundUser.username).to.equal('bob');
                 });
                 });
@@ -700,7 +700,7 @@ describe(Support.getTestDialectTeaser('BelongsTo'), function() {
         return User.create({ username: 'bob' }).then(function(newUser) {
         return User.create({ username: 'bob' }).then(function(newUser) {
           return Task.create({ title: 'some task' }).then(function(newTask) {
           return Task.create({ title: 'some task' }).then(function(newTask) {
             return newTask.setUser(newUser).then(function() {
             return newTask.setUser(newUser).then(function() {
-              return Task.findOne({title: 'some task'}).then(function (foundTask) {
+              return Task.findOne({ where: { title: 'some task' } }).then(function (foundTask) {
                 return foundTask.getUser().then(function (foundUser) {
                 return foundTask.getUser().then(function (foundUser) {
                   expect(foundUser.username).to.equal('bob');
                   expect(foundUser.username).to.equal('bob');
                 });
                 });
@@ -727,7 +727,7 @@ describe(Support.getTestDialectTeaser('BelongsTo'), function() {
         return User.create({ username: 'bob' }).then(function(newUser) {
         return User.create({ username: 'bob' }).then(function(newUser) {
           return Task.create({ title: 'some task' }).then(function(newTask) {
           return Task.create({ title: 'some task' }).then(function(newTask) {
             return newTask.setUser(newUser).then(function() {
             return newTask.setUser(newUser).then(function() {
-              return Task.findOne({title: 'some task'}).then(function (foundTask) {
+              return Task.findOne({ where: { title: 'some task'} }).then(function (foundTask) {
                 return foundTask.getUser().then(function (foundUser) {
                 return foundTask.getUser().then(function (foundUser) {
                   expect(foundUser.username).to.equal('bob');
                   expect(foundUser.username).to.equal('bob');
                 });
                 });

test/integration/data-types.test.js

@@ -322,7 +322,7 @@ describe(Support.getTestDialectTeaser('DataTypes'), function() {
           real: -Infinity
           real: -Infinity
         });
         });
       }).then(function () {
       }).then(function () {
-        return Model.find({id: 1});
+        return Model.find({ where:{ id: 1 } });
       }).then(function (user) {
       }).then(function (user) {
         expect(user.get('float')).to.be.NaN;
         expect(user.get('float')).to.be.NaN;
         expect(user.get('double')).to.eq(Infinity);
         expect(user.get('double')).to.eq(Infinity);

test/integration/include/find.test.js

@@ -257,7 +257,7 @@ describe(Support.getTestDialectTeaser('Include'), function() {
           return Task.create({ title: 'some task' }).then(function(newTask) {
           return Task.create({ title: 'some task' }).then(function(newTask) {
             return newTask.setUser(newUser).then(function() {
             return newTask.setUser(newUser).then(function() {
               return Task.find({
               return Task.find({
-                title: 'some task',
+                where: { title: 'some task' },
                 include: [ { model: User } ]
                 include: [ { model: User } ]
               })
               })
                 .then(function (foundTask) {
                 .then(function (foundTask) {

test/integration/instance.test.js

@@ -1167,12 +1167,12 @@ describe(Support.getTestDialectTeaser('Instance'), function() {
       it('does not update timestamps', function() {
       it('does not update timestamps', function() {
         var self = this;
         var self = this;
         return self.User.create({ username: 'John' }).then(function() {
         return self.User.create({ username: 'John' }).then(function() {
-          return self.User.findOne({ username: 'John' }).then(function(user) {
+          return self.User.findOne({ where: { username: 'John' } }).then(function(user) {
             var updatedAt = user.updatedAt;
             var updatedAt = user.updatedAt;
             self.clock.tick(2000);
             self.clock.tick(2000);
             return user.save().then(function(newlySavedUser) {
             return user.save().then(function(newlySavedUser) {
               expect(newlySavedUser.updatedAt).to.equalTime(updatedAt);
               expect(newlySavedUser.updatedAt).to.equalTime(updatedAt);
-              return self.User.findOne({ username: 'John' }).then(function(newlySavedUser) {
+              return self.User.findOne({ where: { username: 'John' } }).then(function(newlySavedUser) {
                 expect(newlySavedUser.updatedAt).to.equalTime(updatedAt);
                 expect(newlySavedUser.updatedAt).to.equalTime(updatedAt);
               });
               });
             });
             });

test/integration/model/attributes.test.js

@@ -54,14 +54,14 @@ describe(Support.getTestDialectTeaser('Model'), function() {
             return student.addCourse(course, {score: 98, test_value: 1000});
             return student.addCourse(course, {score: 98, test_value: 1000});
           }).then(function() {
           }).then(function() {
             expect(self.callCount).to.equal(1);
             expect(self.callCount).to.equal(1);
-            return self.Score.find({StudentId: 1, CourseId: 100}).then(function(score) {
+            return self.Score.find({ where: { StudentId: 1, CourseId: 100 } }).then(function(score) {
               expect(score.test_value).to.equal(1001);
               expect(score.test_value).to.equal(1001);
             });
             });
           })
           })
           .then(function() {
           .then(function() {
             return Promise.join(
             return Promise.join(
               self.Student.build({no: 1}).getCourses({where: {no: 100}}),
               self.Student.build({no: 1}).getCourses({where: {no: 100}}),
-              self.Score.find({StudentId: 1, CourseId: 100})
+              self.Score.find({ where: { StudentId: 1, CourseId: 100 } })
             );
             );
           })
           })
           .spread(function(courses, score) {
           .spread(function(courses, score) {

test/integration/model/create.test.js

@@ -625,9 +625,11 @@ describe(Support.getTestDialectTeaser('Model'), function() {
           expect(updatedAt.getTime()).to.equal(user.get('updatedAt').getTime());
           expect(updatedAt.getTime()).to.equal(user.get('updatedAt').getTime());
 
 
           return User.findOne({
           return User.findOne({
+            where: {
               updatedAt: {
               updatedAt: {
                 ne: null
                 ne: null
               }
               }
+            }
           }).then(function (user) {
           }).then(function (user) {
             expect(createdAt.getTime()).to.equal(user.get('createdAt').getTime());
             expect(createdAt.getTime()).to.equal(user.get('createdAt').getTime());
             expect(updatedAt.getTime()).to.equal(user.get('updatedAt').getTime());
             expect(updatedAt.getTime()).to.equal(user.get('updatedAt').getTime());
@@ -1402,9 +1404,11 @@ describe(Support.getTestDialectTeaser('Model'), function() {
           silent: true
           silent: true
         }).then(function () {
         }).then(function () {
           return User.findAll({
           return User.findAll({
+            where: {
               updatedAt: {
               updatedAt: {
                 ne: null
                 ne: null
               }
               }
+            }
           }).then(function (users) {
           }).then(function (users) {
             users.forEach(function (user) {
             users.forEach(function (user) {
               expect(createdAt.getTime()).to.equal(user.get('createdAt').getTime());
               expect(createdAt.getTime()).to.equal(user.get('createdAt').getTime());

test/integration/model/findAll.test.js

@@ -1158,7 +1158,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
 
 
         return User.sync({ force: true }).then(function() {
         return User.sync({ force: true }).then(function() {
           return User.create({Login: 'foo'}).then(function() {
           return User.create({Login: 'foo'}).then(function() {
-            return User.findAll({ID: 1}).then(function(user) {
+            return User.findAll({ where: { ID: 1 } }).then(function(user) {
               expect(user).to.be.instanceof(Array);
               expect(user).to.be.instanceof(Array);
               expect(user).to.have.length(1);
               expect(user).to.have.length(1);
             });
             });

test/integration/model/upsert.test.js

@@ -261,7 +261,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
             expect(created).not.to.be.ok;
             expect(created).not.to.be.ok;
           }
           }
 
 
-          return this.ModelWithFieldPK.findOne({ userId: 42 });
+          return this.ModelWithFieldPK.findOne({ where: { userId: 42 } });
         }).then(function(instance) {
         }).then(function(instance) {
           expect(instance.foo).to.equal('second');
           expect(instance.foo).to.equal('second');
         });
         });