updates using UNIQUE (but not PRIMARY) key

[bookshelf]

updates using UNIQUE (but not PRIMARY) key

[tgriesser]

I may need to improve the documentation here, but when dropping into the query layer with the intention of continuing to use the current model (or collection) as the context for querying, you'll want to pass either a function or conditions to the query method... so what you'll want is either:

var entity = models.Entity.forge(); // for brevity

entity.query('where', 'uuid', '=', req.params.uuid).save(attributes).then(function(model) {...

// or
entity.query({where: ['uuid', '=', req.params.uuid]}).save(attributes).then(function(model) {...

// or (as of 0.2.5)
entity.query(function(qb) {
  qb.where('uuid', '=', req.params.uuid);
}).save(attributes).then(...

This way when you call save, you're still acting with the context of the model and not the Knex builder chain directly, so the model (or collection) is passed as the value for the promise handler, and the events will be triggered, etc.

This can also be useful for debugging, as adding:

entity.query('debug').save(...

Would effectively be the same as calling .debug() on the Knex query chain to debug an individual query.

The documentation section for Bookshelf.Sync might also be a little misleading, because while it is documented, it's mainly meant to be used internally... and overriding it would really only be useful if you wanted to modify every insert, update, select, etc. used throughout the library.

Also, just incase you'd find this useful, if you set the hasTimestamps property on the model to true, it will do something along the lines of what you look to be after there with the updating event (assuming there's a created_at datetime or timestamp as well).

Let me know if that helps with the questions you have!

[dmalan]

Hi Tim,

Thanks so much for the follow-up. I just tried out .query, as in the below, though I did have to add {method: 'update'} to force an UPDATE instead of an INSERT (presumably because the entity's idAttribute (id) is undefined in this context, and so .save otherwise defaulted to INSERT):

models.Entity
.forge()
.query({where: ['uuid', '=', req.params.uuid]})
.save(attributes, {method: 'update'})
.then(function(model) {
    // success
}, function(err) {
    // error
});

But by forcing the UPDATE query, I fear I forced a SQL query that isn't quite right. In particular, the automatically generated query included id = NULL as well as created_at = '2013-08-21 22:17:51':

--> ComQueryPacket
{ command: 3,
  sql: 'update `collection` set `created_at` = \'2013-08-21 22:17:51\', `name` = \'foo\', `updated_at` = \'2013-08-21 22:17:51\' where `uuid` = \'110ec58a-a0f2-4ac4-8393-c866d813b8d1\' and `id` = NULL' }

As a workaround, we could first .fetch the id of the entity via its uuid, but we'd definitely prefer to avoid the additional SELECT if there's a cleaner way perhaps?

If related, I also tried passing .query a function (after upgrading to 0.2.5), but the call to qb.where actually seemed to hang on me, whereby after never prints:

models.Entity
.forge()
.query(function(qb) {
    console.log('before');
    qb.where('uuid', '=', '110ec58a-a0f2-4ac4-8393-c866d813b8d1');
    console.log('after');
})
.save(attributes, {method: 'update'})
.then(function(model) {
    // success
}, function(err) {
    // error
});

Much obliged!

[tgriesser]

I agree it's ideal to avoid the extra select there - in the upcoming 0.3.0 version there will be an option of to passing {partial: true} as a flag to only save the data passed in the save call.

In the meantime, I changed a bit of the logic dealing with the timestamp to check the method passed, and also provided a check to not set an extra where clause if the idAttribute doesn't exist (which should only happen in the case of a forced update with {method: 'update'} as you are) - these should be available on the latest with some tests (0.2.7). Let me know if you think of anything else that might seem more intuitive for cases like these.

Also, thanks a lot for pointing out the bug in the query function, I forgot to add a test - should've changed it from .apply to .call there after changing the functionality from the original PR. Should also be fixed.

[dmalan]

Thanks very much, both changes seem to be working nicely now!