fix(model): dont mark value changed by custom setters if there is no change (#8389)

lib/model.js

@@ -3247,7 +3247,10 @@ class Model {
       // If not raw, and there's a custom 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
+        // TODO: v5 make setters return new value instead of changing internal store
+        const newValue = this.dataValues[key];
+        if (!Utils.isPrimitive(newValue) && newValue !== null || newValue !== originalValue) {
           this._previousDataValues[key] = originalValue;
           this.changed(key, true);
         }

test/unit/instance/set.test.js

@@ -4,7 +4,9 @@ const 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'), () => {
   describe('set', () => {
@@ -74,5 +76,81 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
       expect(user.get('date')).to.be.an.instanceof(Date);
       expect(user.get('date')).not.to.be.NaN;
     });
+
+    describe('custom setter', () => {
+      before(function() {
+        this.stubCreate = sinon.stub(current.getQueryInterface(), 'insert', instance => {
+          return Promise.resolve([instance, 1]);
+        });
+      });
+
+      after(function() {
+        this.stubCreate.restore();
+      });
+
+      const User = current.define('User', {
+        phoneNumber: {
+          type: DataTypes.STRING,
+          set (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', () => {
+        const user = User.build({
+          phoneNumber: '+1 234 567'
+        });
+        return user.save().then(() => {
+          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', () => {
+        const user = User.build({
+          phoneNumber: '+1 234 567'
+        });
+        return user.save().then(() => {
+          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', () => {
+        const user = User.build({
+          phoneNumber: '+1 234 567'
+        });
+        return user.save().then(() => {
+          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', () => {
+        const user = User.build({
+          phoneNumber: '+1 234 567'
+        });
+        return user.save().then(() => {
+          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;
+        });
+      });
+    });
   });
 });