sequelize / sequelize

Feature-rich ORM for modern Node.js and TypeScript, it supports PostgreSQL (with JSON and JSONB support), MySQL, MariaDB, SQLite, MS SQL Server, Snowflake, Oracle DB (v6), DB2 and DB2 for IBM i.
https://sequelize.org/
MIT License
29.57k stars 4.28k forks source link

Update instance attribute in "beforeUpdate" hook #10332

Open oktapodia opened 5 years ago

oktapodia commented 5 years ago

Hello,

The beforeUpdate hook on single update (not related to individualHook) does not update the query

Link to https://github.com/sequelize/sequelize/issues/5821 but on single update (with .save())

What are you doing?

      beforeUpdate: (instance) => {
        if (instance.changed('plainPassword')) {
          instance.encryptedPassword = hashPassword(instance.plainPassword);

          //instance.setDataValue('encryptedPassword', hashPassword(instance.plainPassword)); Tried with this syntax as well
        }
      },

What do you expect to happen?

Update the SQL query via the beforeUpdate hook

What is actually happening?

The SQL query executed

[0] Executing (default): UPDATE "user" SET "updated_at"='2019-01-07 15:41:27.684 +00:00' WHERE "id" = 'a70a0948-1061-4074-a7fb-f46fd974bb6d'

Dialect: postgres Dialect version: 7.7.1 Database version: 11 Sequelize version: 4.41.2 Tested with latest release: Yes

Thanks a lot

oktapodia commented 5 years ago

Hum, I've got a beforeValidate hook who's actually updating the instance.encryptedPassword field, commenting it out "resolve" the issue

knoxcard commented 5 years ago

Thanks for posting the solution. Close ticket?

oktapodia commented 5 years ago

No it's not, if the field is updated into the beforeValidate hook, the SQL query does not handle the updated field

sushantdhiman commented 5 years ago

Can you try with master @oktapodia ?

oktapodia commented 5 years ago

Sure,

The latest master release is for the v5 isn't it?

Version master#0f0255eed3d88efb179a47fe7763b7309517428a has been used

It's still the same, and the log output is not resolved anymore ($1, $2 and $3).

Not working behavior

// model.js

//...
    hooks: {
          beforeValidate: (instance) => {
            if (instance.changed('plainPassword')) {
              instance.encryptedPassword = hashPassword(instance.plainPassword);
            }
          },
          beforeUpdate: (instance) => {
            if (instance.changed('plainPassword')) {
              instance.encryptedPassword = hashPassword(instance.plainPassword);
            }
          },
      },
//...
[0] Executing (default): UPDATE "user" SET "token"=$1,"updated_at"=$2 WHERE "id" = $3

Working behavior

// model.js

//...

    hooks: {
      beforeUpdate: (instance) => {
        if (instance.changed('plainPassword')) {
          instance.encryptedPassword = hashPassword(instance.plainPassword);
        }
      },
//...
[0] Executing (default): UPDATE "user" SET "token"=$1,"updated_at"=$2,"encrypted_password"=$3 WHERE "id" = $4
sushantdhiman commented 5 years ago

So beforeValidate and beforeUpdate are not working together

oktapodia commented 5 years ago

Exactly sorry if I wasn't clear enough

markhealey commented 4 years ago

This is still an issue on v5 — we just encountered it where we're have beforeValidate hook code automatically keeping a date field updated when another field value changes. Sequelize will NOT include the update to our date field if we use beforeValidate, only if we use beforeUpdate 😞

markhealey commented 4 years ago

Actually to be clear, we have other code in the beforeValidate hook that is working. So perhaps it's just date fields where allowNull is true?

cbarlow1993 commented 4 years ago

Having similar issues here whereby I only have a beforeUpdate hook and trying to add a field in to update,

ReadyToDispatchTimestamp is set in beforeUpdate hook. At the bottom of the beforeUpdate hook, i've run instance.isChanged() and it returns as [ 'State', 'Note', 'ReadyToDispatchTimestamp' ].

I would expect that ReadyToDispatchTimestamp would also be in the executed SQL statement but I get none.

Executing (default): UPDATE Order SET PaymentType=?,CartTotal=?,State=?,CodiceFiscale=?,CouponCode=?,Note=?,updatedAt=? WHERE Id = ?

I've tried making ReadyToDispatchTimestamp a string field and also a Date field, neither worked.

Lost...