feat(scopes): unpack Op.and at the top most level if not needed

src/model.js

@@ -861,20 +861,7 @@ class Model {
     }
 
     if (['where', 'having'].includes(key)) {
-      let objKeysToKeep;
-      if (objValue) {
-        const objKeys = Object.getOwnPropertyNames(objValue).concat(Object.getOwnPropertySymbols(objValue));
-        objKeysToKeep = _.filter(objKeys, key => key !== Op.and);
-      }
-
-      return {
-        [Op.and]: _.compact([
-          ...(objValue?.[Op.and] ?? []),
-          !_.isEmpty(objKeysToKeep) && _.pick(objValue, objKeysToKeep),
-          srcValue,
-        ]),
-      };
-
+      return combineWheresWithAnd(objValue, srcValue);
     } else if (key === 'attributes' && _.isPlainObject(objValue) && _.isPlainObject(srcValue)) {
       return _.assignWith(objValue, srcValue, (objValue, srcValue) => {
         if (Array.isArray(objValue) && Array.isArray(srcValue)) {
@@ -4772,6 +4759,58 @@ class Model {
   static belongsTo(target, options) {}
 }
 
+/**
+ * Unpacks an object that only contains a single Op.and key to the value of Op.and
+ *
+ * Internal method used by {@link combineWheresWithAnd}
+ *
+ * @param {WhereOptions} where The object to unpack
+ * @example `{ [Op.and]: [a, b] }` becomes `[a, b]`
+ * @example `{ [Op.and]: { key: val } }` becomes `{ key: val }`
+ * @example `{ [Op.or]: [a, b] }` remains as `{ [Op.or]: [a, b] }`
+ * @example `{ [Op.and]: [a, b], key: c }` remains as `{ [Op.and]: [a, b], key: c }`
+ * @private
+ */
+function unpackAnd(where) {
+  if (!_.isObject(where)) {
+    return where;
+  }
+
+  const keys = Utils.getComplexKeys(where);
+
+  // object is empty, remove it.
+  if (keys.length === 0) {
+    return;
+  }
+
+  // we have more than just Op.and, keep as-is
+  if (keys.length !== 1 || keys[0] !== Op.and) {
+    return where;
+  }
+
+  const andParts = where[Op.and];
+
+  return andParts;
+}
+
+function combineWheresWithAnd(whereA, whereB) {
+  const unpackedA = unpackAnd(whereA);
+
+  if (unpackedA === undefined) {
+    return whereB;
+  }
+
+  const unpackedB = unpackAnd(whereB);
+
+  if (unpackedB === undefined) {
+    return whereA;
+  }
+
+  return {
+    [Op.and]: [unpackedA, unpackedB].flat(),
+  };
+}
+
 Object.assign(Model, associationsMixin);
 Hooks.applyTo(Model, true);
 

test/support.js

@@ -2,7 +2,7 @@
 
 const fs = require('fs');
 const path = require('path');
-const { isDeepStrictEqual } = require('util');
+const { inspect, isDeepStrictEqual } = require('util');
 const _ = require('lodash');
 
 const Sequelize = require('@sequelize/core');
@@ -18,6 +18,12 @@ chai.use(require('chai-datetime'));
 chai.use(require('chai-as-promised'));
 chai.use(require('sinon-chai'));
 
+// Using util.inspect to correctly assert objects with symbols
+// Because expect.deep.equal does not test non iterator keys such as symbols (https://github.com/chaijs/chai/issues/1054)
+chai.Assertion.addMethod('deepEqual', function (expected, depth = 5) {
+  expect(inspect(this._obj, { depth })).to.deep.equal(inspect(expected, { depth }));
+});
+
 chai.config.includeStack = true;
 chai.should();
 

test/unit/model/include.test.js

@@ -1,23 +1,15 @@
 'use strict';
 
 const chai = require('chai');
-const util = require('util');
 
 const expect = chai.expect;
 const Support   = require('../support');
-const Sequelize = require('@sequelize/core');
-
-const Op = Sequelize.Op;
+const { Sequelize, Op } = require('@sequelize/core');
+const Utils = require('@sequelize/core/lib/utils/index');
 
 const current   = Support.sequelize;
 
 describe(Support.getTestDialectTeaser('Model'), () => {
-  // Using util.inspect to correctly assert objects with symbols
-  // Because expect.deep.equal does not test non iterator keys such as symbols (https://github.com/chaijs/chai/issues/1054)
-  chai.Assertion.addMethod('deepEquals', function (expected, depth = 5) {
-    expect(util.inspect(this._obj, { depth })).to.deep.equal(util.inspect(expected, { depth }));
-  });
-
   describe('all', () => {
     const Referral = current.define('referal');
 
@@ -188,7 +180,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
           include: [{ model: this.Project }],
         });
 
-        expect(options.include[0]).to.have.property('where').which.deepEquals({ [Op.and]: [{ active: true }] });
+        expect(options.include[0]).to.have.property('where').which.deep.equal({ active: true  });
       });
 
       it('adds the where from a scoped model', function () {
@@ -197,7 +189,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
           include: [{ model: this.Project.scope('that') }],
         });
 
-        expect(options.include[0]).to.have.property('where').which.deepEquals({ [Op.and]: [{ that: false }] });
+        expect(options.include[0]).to.have.property('where').which.deep.equal({ that: false });
         expect(options.include[0]).to.have.property('limit').which.equals(12);
       });
 
@@ -207,7 +199,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
           include: [{ model: this.Project.scope('attr') }],
         });
 
-        expect(options.include[0]).to.have.property('attributes').which.deepEquals(['baz']);
+        expect(options.include[0]).to.have.property('attributes').which.deep.equal(['baz']);
       });
 
       it('merges where with the where from a scoped model', function () {
@@ -216,7 +208,9 @@ describe(Support.getTestDialectTeaser('Model'), () => {
           include: [{ where: { active: false }, model: this.Project.scope('that') }],
         });
 
-        expect(options.include[0]).to.have.property('where').which.deepEquals({ [Op.and]: [{ that: false }, { active: false }] });
+        expect(options.include[0]).to.have.property('where');
+        expect(Utils.getComplexKeys(options.include[0].where)).to.deep.equal([Op.and]);
+        expect(options.include[0].where[Op.and]).to.deep.equal([{ that: false }, { active: false }]);
       });
 
       it('add the where from a scoped associated model', function () {
@@ -225,7 +219,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
           include: [{ model: this.Project, as: 'thisProject' }],
         });
 
-        expect(options.include[0]).to.have.property('where').which.deepEquals({ [Op.and]: [{ this: true }] });
+        expect(options.include[0]).to.have.property('where').which.deep.equal({ this: true });
       });
 
       it('handles a scope with an aliased column (.field)', function () {
@@ -234,7 +228,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
           include: [{ model: this.Project.scope('foobar') }],
         });
 
-        expect(options.include[0]).to.have.property('where').which.deepEquals({ [Op.and]: [{ foo: 42 }] });
+        expect(options.include[0]).to.have.property('where').which.deep.equal({ foo: 42 });
       });
     });