1602 / jugglingdb

Multi-database ORM for nodejs: redis, mongodb, mysql, sqlite3, postgresql, arango, in-memory...
http://1602.github.io/jugglingdb/
2.05k stars 241 forks source link

upsert method doesn't trigger any hooks for mysql adapter #461

Open bahung1221 opened 5 years ago

bahung1221 commented 5 years ago

Hi,

I found that the upsert method won't trigger any hook if the adapter support updateOrCreate method (mysql in my case). I debugged and found the reason in below code:

https://github.com/1602/jugglingdb/blob/04adcdbd13c9ad9ddb0b9bb0a18e4620c17f56fc/lib/model.js#L336

if (this.schema.adapter.updateOrCreate) {
        const inst = new Model(data);
        this.schema.callAdapter('updateOrCreate', Model.modelName, stripUndefined(inst.toObject(true)), (err, data) => {
            if (data) {
                return inst.reload(callback);
            }
            callback(err, null);
        });
    } else {
    ...
}

I think it should trigger save/update hooks like other methods, If you need, i can create a PR for it,

Thank you very much for a nice job!

1602 commented 5 years ago

Yep, that seems like a missing call, I'd appreciate PR, thank you!

bahung1221 commented 5 years ago

Hi,

I fixed it by make it work same behavior for all adapter, It will make the workflow of upsert (createOrUpdate) method more consistency. I think it is the best way because if we call updateOrCreate method of adapter, we can't know that it was create or update. Pull request: https://github.com/1602/jugglingdb/pull/462

Thank you

1602 commented 5 years ago

Merged. One test is failing, though. I'll try to find some time to investigate why.