fix(lucid): use primary key instead of id

model instance operations should use primary key value instead of hardcoded id Closes #51
adonisjs · Jul 16, 2017 · ba39a06 · ba39a06
1 parent 8b85d12
commit ba39a06
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 78 deletions.
diff --git a/src/Lucid/Model/Mixins/Persistance.js b/src/Lucid/Model/Mixins/Persistance.js
@@ -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)
@@ -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)
     }
@@ -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)
     }

diff --git a/test/unit/fixtures/model.js b/test/unit/fixtures/model.js
@@ -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')
@@ -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'),
@@ -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(),

diff --git a/test/unit/lucid.spec.js b/test/unit/lucid.spec.js
@@ -1303,79 +1303,4 @@ test.group('Model', (group) => {
     const fn = () => User.addGlobalScope('foo')
     assert.throw(fn, 'E_INVALID_PARAMETER: Model.addGlobalScope expects a closure as first parameter')
   })
-
-  context('Traits', function () {
-    it('should throw an exception when trait does not have register method', function () {
-      const MyTrait = {
-      }
-      class User extends Model {
-        static boot () {
-          super.boot()
-          this.use(MyTrait)
-        }
-      }
-      const fn = () => User.bootIfNotBooted()
-      expect(fn).to.throw('InvalidArgumentException: E_INVALID_MODEL_TRAIT: Make sure you have defined register method on model')
-    })
-
-    it('should be able to assign trait to the model', function () {
-      const MyTrait = {
-        register (Model) {
-          Model.findByTrait = function () {}
-        }
-      }
-      class User extends Model {
-        static boot () {
-          super.boot()
-          this.use(MyTrait)
-        }
-      }
-      User.bootIfNotBooted()
-      expect(User.findByTrait).to.be.a('function')
-    })
-
-    it('should be able to assign trait class to the model', function () {
-      class MyTrait {
-        get name () {
-          return 'foo'
-        }
-
-        register (Model) {
-          const name = this.name
-          Model.findByTrait = function () {
-            return name
-          }
-        }
-      }
-
-      class User extends Model {
-        static boot () {
-          super.boot()
-          this.use(MyTrait)
-        }
-      }
-
-      User.bootIfNotBooted()
-      expect(User.findByTrait).to.be.a('function')
-      expect(User.findByTrait()).to.equal('foo')
-    })
-
-    it('should be able to assign trait using static traits getters', function () {
-      const MyTrait = {
-        register (Model) {
-          Model.findByTrait = function () {}
-        }
-      }
-      class User extends Model {
-        static get traits () {
-          return [MyTrait]
-        }
-        static boot () {
-          super.boot()
-        }
-      }
-      User.bootIfNotBooted()
-      expect(User.findByTrait).to.be.a('function')
-    })
-  })
 })