master:fix-previous-value

[nemisj]

Pull Request check-list

• Does npm run test or npm run test-DIALECT pass with this change (including linting)?
• Does your issue contain a link to existing issue (Closes #[issue]) or a description of the issue you are solving?
• Have you added new tests to prevent regressions?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
• Have you added an entry under Future in the changelog?

Description of change

Previous method of the instance was working incorrectly

[sushantdhiman]

Needs changelog, otherwise LGTM

[codecov-io]

Codecov Report

Merging #7189 into master will increase coverage by <.01%.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c2f20b...09804cc. Read the comment docs.

[nemisj]

@sushantdhiman , added

[sushantdhiman]

Space before opening brackets function ()

[nemisj]

done

[BorisKozo]

This fix broke our application :D
Reminded me of this: https://xkcd.com/1172/

[barriber]

Previous function doesn't return the correct values now,

return Promise.map(instances, function(instance) {
          // Record updates in instances dataValues
          Utils._.extend(instance.dataValues, values);
          // Set the changed fields on the instance
          Utils._.forIn(valuesUse, function(newValue, attr) {
            if (newValue !== instance._previousDataValues[attr]) {
              instance.setDataValue(attr, newValue);
            }
          });

instance.dataValues override with the new values in Utils._.extend(instance.dataValues, values);
and in instance.setDataValue(attr, newValue);

Instance.prototype.setDataValue = function(key, value) {
  var originalValue = this.dataValues[key];
  if (!Utils.isPrimitive(value) || value !== originalValue) {
    this.changed(key, true);
  }

  this._previousDataValues[key] = originalValue;
  this.dataValues[key] = value;
};

the _previousDataValues overrided with same new data value

[nemisj]

@barriber so actually we also have to change that code which sets dataValues directly I guess, into something like:

return Promise.map(instances, function(instance) {
          // Set the changed fields on the instance
          Utils._.forIn(valuesUse, function(newValue, attr) {
              instance.setDataValue(attr, newValue);
          });

in order to guarantee the same data flow into the instance

[barriber]

@janmeier could you please comment if this is the correct solution?

[nemisj]

@barriber i see that there is a note in changelog.md saying that from 2.0.0. version _previousDataValues must be updated after afterUpdate. But I don't understand this comment completely why it's like that. So i guess this solution is also not good and should be done differently.

[verdier]

This does't work if you call setDataValue multiple times... previous should return the original value (ie. the value in the DB when the record was fetched), am I wrong?

[verdier]

Ok that was reverted... sorry ^^