fix(instance): changed state from null & custom setter (#10316)

lib/instance.js

@@ -159,7 +159,7 @@ Instance.prototype.getDataValue = function(key) {
  */
 Instance.prototype.setDataValue = function(key, value) {
   var originalValue = this._previousDataValues[key];
-  if (!Utils.isPrimitive(value) || value !== originalValue) {
+  if ((!Utils.isPrimitive(value) && value !== null) || value !== originalValue) {
     this.changed(key, true);
   }
 
@@ -311,7 +311,9 @@ Instance.prototype.set = function(key, value, options) { // testhint options:non
     // If not raw, and there's a customer setter
     if (!options.raw && this._customSetters[key]) {
       this._customSetters[key].call(this, value, key);
-      if (!Utils.isPrimitive(value) && value !== null || value !== originalValue) {
+      // custom setter should have changed value, get that changed value
+      var newValue = this.dataValues[key];
+      if (!Utils.isPrimitive(newValue) && newValue !== null || newValue !== originalValue) {
         this._previousDataValues[key] = originalValue;
         this.changed(key, true);
       }

test/unit/instance/changed.test.js

@@ -150,5 +150,41 @@ describe(Support.getTestDialectTeaser('Instance'), function() {
       user.set('meta.address', { street: 'Main street', number: '40' } );
       expect(user.changed('meta')).to.equal(false);
     });
+
+    it('should return false when changed from null to null', function() {
+      var attributes = {};
+      var attr;
+
+      for (attr in this.User.rawAttributes) {
+        attributes[attr] = null;
+      }
+      var user = this.User.build(attributes, {
+        isNewRecord: false,
+        raw: true
+      });
+      for (attr in this.User.rawAttributes) {
+        user.set(attr, null);
+      }
+      for (attr in this.User.rawAttributes) {
+        expect(user.changed(attr)).to.equal(false);
+      }
+    });
+
+    describe('setDataValue', function() {
+      it('should return falsy for unchanged primitive', function() {
+        var user = this.User.build({
+          name: 'a',
+          meta: null
+        }, {
+          isNewRecord: false,
+          raw: true
+        });
+
+        user.setDataValue('name', 'a');
+        user.setDataValue('meta', null);
+        expect(user.changed('name')).to.equal(false);
+        expect(user.changed('meta')).to.equal(false);
+      });
+    });
   });
 });

test/unit/instance/set.test.js

@@ -5,7 +5,9 @@ var chai = require('chai')
   , expect = chai.expect
   , Support   = require(__dirname + '/../support')
   , DataTypes = require(__dirname + '/../../../lib/data-types')
-  , current   = Support.sequelize;
+  , current   = Support.sequelize
+  , Promise   = current.Promise
+  , sinon     = require('sinon');
 
 describe(Support.getTestDialectTeaser('Instance'), function() {
   describe('set', function () {
@@ -75,5 +77,81 @@ describe(Support.getTestDialectTeaser('Instance'), function() {
       expect(user.get('date')).to.be.an.instanceof(Date);
       expect(user.get('date')).not.to.be.NaN;
     });
+
+    describe('custom setter', function() {
+      before(function() {
+        this.stubCreate = sinon.stub(current.getQueryInterface(), 'insert', function(instance) {
+          return Promise.resolve(instance);
+        });
+      });
+
+      after(function() {
+        this.stubCreate.restore();
+      });
+
+      var User = current.define('User', {
+        phoneNumber: {
+          type: DataTypes.STRING,
+          set: function(val) {
+            if (typeof val === 'object' && val !== null) {
+              val = '00' + val.country + val.area + val.local;
+            }
+            if (typeof val === 'string') {
+              // Canonicalize phone number
+              val = val.replace(/^\+/, '00').replace(/\(0\)|[\s+\/.\-\(\)]/g, '');
+            }
+            this.setDataValue('phoneNumber', val);
+          }
+        }
+      });
+
+      it('does not set field to changed if field is set to the same value with custom setter using primitive value', function() {
+        var user = User.build({
+          phoneNumber: '+1 234 567'
+        });
+        return user.save().then(function() {
+          expect(user.changed('phoneNumber')).to.be.false;
+
+          user.set('phoneNumber', '+1 (0) 234567'); // Canonical equivalent of existing phone number
+          expect(user.changed('phoneNumber')).to.be.false;
+        });
+      });
+
+      it('sets field to changed if field is set to the another value with custom setter using primitive value', function() {
+        var user = User.build({
+          phoneNumber: '+1 234 567'
+        });
+        return user.save().then(function() {
+          expect(user.changed('phoneNumber')).to.be.false;
+
+          user.set('phoneNumber', '+1 (0) 765432'); // Canonical non-equivalent of existing phone number
+          expect(user.changed('phoneNumber')).to.be.true;
+        });
+      });
+
+      it('does not set field to changed if field is set to the same value with custom setter using object', function() {
+        var user = User.build({
+          phoneNumber: '+1 234 567'
+        });
+        return user.save().then(function() {
+          expect(user.changed('phoneNumber')).to.be.false;
+
+          user.set('phoneNumber', { country: '1', area: '234', local: '567' }); // Canonical equivalent of existing phone number
+          expect(user.changed('phoneNumber')).to.be.false;
+        });
+      });
+
+      it('sets field to changed if field is set to the another value with custom setter using object', function() {
+        var user = User.build({
+          phoneNumber: '+1 234 567'
+        });
+        return user.save().then(function() {
+          expect(user.changed('phoneNumber')).to.be.false;
+
+          user.set('phoneNumber', { country: '1', area: '765', local: '432' }); // Canonical non-equivalent of existing phone number
+          expect(user.changed('phoneNumber')).to.be.true;
+        });
+      });
+    });
   });
 });