Skip to content

Commit

Permalink
fix(lucid): use primary key instead of id
Browse files Browse the repository at this point in the history
model instance operations should use primary key value instead of hardcoded id

Closes #51
  • Loading branch information
thetutlage committed Sep 27, 2016
1 parent d8d3a04 commit f85da85
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/Lucid/Model/Mixins/Persistance.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ Peristance.update = function * () {
if (this.transaction) {
query.transacting(this.transaction)
}
const affected = yield query.where('id', this.$primaryKeyValue).updateAttributes(dirtyValues)
const affected = yield query.where(this.constructor.primaryKey, this.$primaryKeyValue).updateAttributes(dirtyValues)
if (affected > 0) {
_.merge(this.attributes, dirtyValues)
this.original = _.clone(this.attributes)
Expand All @@ -92,7 +92,7 @@ Peristance.update = function * () {
*/
Peristance.delete = function * () {
const deleteHandler = function * () {
const query = this.constructor.query().where('id', this.$primaryKeyValue)
const query = this.constructor.query().where(this.constructor.primaryKey, this.$primaryKeyValue)
if (this.transaction) {
query.transacting(this.transaction)
}
Expand All @@ -117,7 +117,7 @@ Peristance.delete = function * () {
*/
Peristance.restore = function * () {
const restoreHandler = function * () {
const query = this.constructor.query().where('id', this.$primaryKeyValue)
const query = this.constructor.query().where(this.constructor.primaryKey, this.$primaryKeyValue)
if (this.transaction) {
query.transacting(this.transaction)
}
Expand Down
10 changes: 10 additions & 0 deletions test/unit/fixtures/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ module.exports = {
table.timestamps()
table.timestamp('deleted_at').nullable()
}),
knex.schema.createTable('zombies', function (table) {
table.increments('zombie_id')
table.string('username')
table.string('firstname')
table.string('lastname')
table.timestamps()
table.timestamp('deleted_at').nullable()
}),
knex.schema.createTable('accounts', function (table) {
table.increments()
table.string('account_name')
Expand Down Expand Up @@ -48,6 +56,7 @@ module.exports = {
down: function (knex) {
const dropTables = [
knex.schema.dropTable('users'),
knex.schema.dropTable('zombies'),
knex.schema.dropTable('accounts'),
knex.schema.dropTable('profiles'),
knex.schema.dropTable('cars'),
Expand All @@ -59,6 +68,7 @@ module.exports = {
truncate: function (knex) {
const truncateTables = [
knex.table('users').truncate(),
knex.table('zombies').truncate(),
knex.table('accounts').truncate(),
knex.table('profiles').truncate(),
knex.table('cars').truncate(),
Expand Down
72 changes: 72 additions & 0 deletions test/unit/lucid.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('Lucid', function () {

afterEach(function * () {
yield Database.table('users').truncate()
yield Database.table('zombies').truncate()
})

context('Model', function () {
Expand Down Expand Up @@ -1795,6 +1796,77 @@ describe('Lucid', function () {
const reFetchUser = yield User.findBy('username', 'foo')
expect(reFetchUser.lastname).to.equal('foo')
})

it('should perform the update query on the model primary key instead of id', function * () {
let updateQuery = null
class Zombie extends Model {
static get primaryKey () {
return 'zombie_id'
}
static boot () {
super.boot()
this.onQuery((query) => {
updateQuery = query
})
}
}

Zombie.bootIfNotBooted()
const zombie = yield Zombie.create({username: 'foo', lastname: 'bar'})
expect(zombie.zombie_id).to.equal(1)
zombie.lastname = 'baz'
yield zombie.save()
expect(queryHelpers.formatQuery(updateQuery.sql)).to.equal(queryHelpers.formatQuery('update "zombies" set "lastname" = ?, "updated_at" = ? where "zombie_id" = ?'))
yield Zombie.findByOrFail('lastname', 'baz')
})

it('should perform the delete query on the model primary key instead of id', function * () {
let deleteQuery = null
class Zombie extends Model {
static get primaryKey () {
return 'zombie_id'
}
static boot () {
super.boot()
this.onQuery((query) => {
deleteQuery = query
})
}
}

Zombie.bootIfNotBooted()
const zombie = yield Zombie.create({username: 'foo', lastname: 'bar'})
expect(zombie.zombie_id).to.equal(1)
yield zombie.delete()
expect(queryHelpers.formatQuery(deleteQuery.sql)).to.equal(queryHelpers.formatQuery('delete from "zombies" where "zombie_id" = ?'))
})

it('should perform the restore query on the model primary key instead of id', function * () {
let restoreQuery = null
class Zombie extends Model {
static get primaryKey () {
return 'zombie_id'
}

static get deleteTimestamp () {
return 'deleted_at'
}

static boot () {
super.boot()
this.onQuery((query) => {
restoreQuery = query
})
}
}

Zombie.bootIfNotBooted()
const zombie = yield Zombie.create({username: 'foo', lastname: 'bar'})
expect(zombie.zombie_id).to.equal(1)
yield zombie.delete()
yield zombie.restore()
expect(queryHelpers.formatQuery(restoreQuery.sql)).to.equal(queryHelpers.formatQuery('update "zombies" set "deleted_at" = ?, "updated_at" = ? where "zombie_id" = ?'))
})
})

context('Model Transactions', function () {
Expand Down

0 comments on commit f85da85

Please sign in to comment.