feat: support delete and update hooks

closes SoftwareBrothers/adminjs#503

Author: Wojciech Krysiak

.eslintrc.js

@@ -52,6 +52,7 @@ module.exports = {
     files: ['./src/**/*.spec.ts', 'spec/*.ts'],
     rules: {
       'no-unused-expressions': 'off',
+      'prefer-arrow-callback': 'off',
       'func-names': 'off',
       'import/no-extraneous-dependencies': 'off',
     },

example-app/package.json

@@ -20,7 +20,7 @@
   },
   "dependencies": {
     "@admin-bro/express": "^3.0.0",
-    "@admin-bro/sequelize": "../",
+    "@admin-bro/sequelize": "1.0.0-beta.5",
     "admin-bro": "^3.1.1",
     "dotenv": "^8.2.0",
     "express": "^4.17.1",

example-app/yarn.lock

@@ -18,8 +18,10 @@
   resolved "https://registry.yarnpkg.com/@admin-bro/express/-/express-3.0.0.tgz#0ac581bbd4511d8854d07409b980e0fff3d9d8e0"
   integrity sha512-Ya/a3F/JT//VanaIjlVBeU6gh1afwcjU/q9qhMcETKwy3M+QbVhfmLdOyD1mFqVxwifnkiGnC6gPkupb51C9Pg==
 
-"@admin-bro/sequelize@../":
-  version "1.0.0-beta.3"
+"@admin-bro/[email protected]":
+  version "1.0.0-beta.5"
+  resolved "https://registry.yarnpkg.com/@admin-bro/sequelize/-/sequelize-1.0.0-beta.5.tgz#662d875ad2fedd781003bb84d303d37be691e0ad"
+  integrity sha512-MpXND4ZheWePaA2rpxhedpeUwzbI0OaLfyKvdpZif/SsFgsDOEzgto3QryLyMSkXtCdzXh54NYwKR9Uku0c0VQ==
   dependencies:
     escape-regexp "0.0.1"
     flat "^5.0.0"

src/resource.spec.ts

@@ -3,84 +3,88 @@ import { ValidationError, Filter, BaseRecord } from 'admin-bro'
 
 import chai, { expect } from 'chai'
 import sinonChai from 'sinon-chai'
+import sinon from 'sinon'
 
-import Resource from './resource'
+import Resource, { ModelType } from './resource'
 import Property from './property'
 
 chai.use(sinonChai)
 
 const config = require('../config/config')[process.env.NODE_ENV as string]
 const db = require('../models/index.js')
 
-describe('Resource', () => {
+describe('Resource', function () {
+  let SequelizeModel: ModelType<any>
+  let resource: Resource
+
   before(function () {
-    this.SequelizeModel = db.sequelize.models.User
-    this.resource = new Resource(this.SequelizeModel)
+    SequelizeModel = db.sequelize.models.User
+    resource = new Resource(SequelizeModel)
   })
 
   afterEach(async function () {
-    await this.SequelizeModel.destroy({ where: {} })
+    await SequelizeModel.destroy({ where: {} })
   })
 
   describe('.isAdapterFor', () => {
     it('returns true when sequelize model is given', function () {
-      expect(Resource.isAdapterFor(this.SequelizeModel)).to.equal(true)
+      expect(Resource.isAdapterFor(SequelizeModel)).to.equal(true)
     })
   })
 
   describe('#database', () => {
     it('returns correct database name', function () {
-      expect(this.resource.databaseName()).to.equal(config.database)
+      expect(resource.databaseName()).to.equal(config.database)
     })
   })
 
   describe('#databaseType', () => {
     it('returns correct database', function () {
-      expect(this.resource.databaseType()).to.equal(config.dialect)
+      expect(resource.databaseType()).to.equal(config.dialect)
     })
   })
 
   describe('#name', () => {
     it('returns correct name', function () {
-      expect(this.resource.name()).to.equal('Users')
+      expect(resource.name()).to.equal('Users')
     })
   })
 
   describe('#id', () => {
     it('returns correct name', function () {
-      expect(this.resource.id()).to.equal('Users')
+      expect(resource.id()).to.equal('Users')
     })
   })
 
   describe('#properties', () => {
     it('returns all properties', function () {
       const length = 8 // there are 8 properties in the User model (5 regular + __v and _id)
-      expect(this.resource.properties()).to.have.lengthOf(length)
+      expect(resource.properties()).to.have.lengthOf(length)
     })
   })
 
   describe('#property', () => {
     it('returns given property', function () {
-      expect(this.resource.property('email')).to.be.an.instanceOf(Property)
+      expect(resource.property('email')).to.be.an.instanceOf(Property)
     })
 
     it('returns null when property doesn\'t exit', function () {
-      expect(this.resource.property('some.imagine.property')).to.be.null
+      expect(resource.property('some.imagine.property')).to.be.null
     })
 
     it('returns nested property for array field', function () {
-      const property = this.resource.property('arrayed.1')
+      const property = resource.property('arrayed.1')
 
       expect(property).to.be.an.instanceOf(Property)
-      expect(property.type()).to.equal('string')
+      expect(property?.type()).to.equal('string')
     })
   })
 
   describe('#findMany', () => {
     it('returns array of BaseRecords', async function () {
-      const params = await this.resource.create({ email: '[email protected]' })
+      const params = await resource.create({ email: '[email protected]' })
 
-      const records = await this.resource.findMany([params.id])
+      const records = await resource.findMany([params.id])
 
       expect(records).to.have.lengthOf(1)
       expect(records[0]).to.be.instanceOf(BaseRecord)
@@ -93,14 +97,14 @@ describe('Resource', () => {
         gender: 'male',
         email: '[email protected]',
       }
-      this.record = await this.resource.create(this.params)
+      this.record = await resource.create(this.params)
     })
 
     it('returns 1 BaseRecord when filtering on ENUMS', async function () {
       const filter = new Filter({
         gender: 'male',
-      }, this.resource)
-      const records = await this.resource.find(filter, { limit: 20, offset: 0, sort: { direction: 'asc', sortBy: 'id' } })
+      }, resource)
+      const records = await resource.find(filter, { limit: 20, offset: 0, sort: { direction: 'asc', sortBy: 'id' } })
 
       expect(records).to.have.lengthOf(1)
       expect(records[0]).to.be.instanceOf(BaseRecord)
@@ -110,18 +114,18 @@ describe('Resource', () => {
     it('returns 0 BaseRecord when filtering on ENUMS', async function () {
       const filter = new Filter({
         gender: 'female',
-      }, this.resource)
-      const records = await this.resource.find(filter, { limit: 20, offset: 0, sort: { direction: 'asc', sortBy: 'id' } })
+      }, resource)
+      const records = await resource.find(filter, { limit: 20, offset: 0, sort: { direction: 'asc', sortBy: 'id' } })
 
       expect(records).to.have.lengthOf(0)
     })
 
     it('returns error when filtering on ENUMS with invalid value', async function () {
       const filter = new Filter({
         gender: 'XXX',
-      }, this.resource)
+      }, resource)
       try {
-        await this.resource.find(filter, { limit: 20, offset: 0, sort: { direction: 'asc', sortBy: 'id' } })
+        await resource.find(filter, { limit: 20, offset: 0, sort: { direction: 'asc', sortBy: 'id' } })
       } catch (error) {
         expect(error.name).to.eq('SequelizeDatabaseError')
       }
@@ -130,30 +134,30 @@ describe('Resource', () => {
 
   describe('#count', () => {
     it('returns 0 when there are none elements', async function () {
-      const count = await this.resource.count(new Filter({} as any, {} as any))
+      const count = await resource.count(new Filter({} as any, {} as any))
       expect(count).to.equal(0)
     })
 
     it('returns given count without filters', async function () {
-      await this.resource.create({ email: '[email protected]' })
-      expect(await this.resource.count(new Filter({} as any, {} as any))).to.equal(1)
+      await resource.create({ email: '[email protected]' })
+      expect(await resource.count(new Filter({} as any, {} as any))).to.equal(1)
     })
 
     it('returns given count for given filters', async function () {
-      await this.resource.create({
+      await resource.create({
         firstName: 'john',
         lastName: 'doe',
         email: '[email protected]',
       })
-      await this.resource.create({
+      await resource.create({
         firstName: 'andrew',
         lastName: 'golota',
         email: '[email protected]',
       })
       const filter = new Filter({
         email: '[email protected]',
-      }, this.resource)
-      expect(await this.resource.count(filter)).to.equal(1)
+      }, resource)
+      expect(await resource.count(filter)).to.equal(1)
     })
   })
 
@@ -165,11 +169,11 @@ describe('Resource', () => {
           lastName: 'doe',
           email: '[email protected]',
         }
-        this.record = await this.resource.create(this.params)
+        this.record = await resource.create(this.params)
       })
 
       it('creates new user when data is valid', async function () {
-        const newCount = await this.resource.count()
+        const newCount = await resource.count(null as any)
         expect(newCount).to.equal(1)
       })
 
@@ -189,7 +193,7 @@ describe('Resource', () => {
 
       it('throws validation error', async function () {
         try {
-          await this.resource.create(this.params)
+          await resource.create(this.params)
         } catch (error) {
           expect(error).to.be.an.instanceOf(ValidationError)
         }
@@ -198,8 +202,8 @@ describe('Resource', () => {
 
     context('record with empty id field', () => {
       beforeEach(function () {
-        this.SequelizeModel = db.sequelize.models.Post
-        this.resource = new Resource(this.SequelizeModel)
+        SequelizeModel = db.sequelize.models.Post
+        resource = new Resource(SequelizeModel)
       })
 
       it('creates record without an error', async function () {
@@ -209,28 +213,82 @@ describe('Resource', () => {
           publishedAt: '2019-12-10 12:00',
           userId: '',
         }
-        this.recordParams = await this.resource.create(this.params)
+        this.recordParams = await resource.create(this.params)
         expect(this.recordParams.userId).to.be.null
       })
     })
   })
 
   describe('#update', () => {
     beforeEach(async function () {
+      SequelizeModel = db.sequelize.models.User
+      resource = new Resource(SequelizeModel)
       this.params = {
         firstName: 'john',
         lastName: 'doe',
         email: '[email protected]',
       }
-      this.record = await this.resource.create(this.params)
+      this.record = await resource.create(this.params)
     })
 
     it('updates the title', async function () {
       this.newEmail = '[email protected]'
-      this.record = await this.resource.update(this.record.id, {
+      const params = await resource.update(this.record.id, {
         email: this.newEmail,
       })
-      expect(this.record.email).to.equal(this.newEmail)
+
+      expect(params.email).to.equal(this.newEmail)
+    })
+
+    it('calls update hooks', async function () {
+      const beforeUpdateSpy = sinon.spy()
+      const afterUpdateSpy = sinon.spy()
+      const beforeBulkUpdateSpy = sinon.spy()
+      SequelizeModel.addHook('beforeUpdate', beforeUpdateSpy)
+      SequelizeModel.addHook('beforeBulkUpdate', beforeBulkUpdateSpy)
+      SequelizeModel.addHook('afterUpdate', afterUpdateSpy)
+
+      await resource.update(this.record.id, { firstName: 'jack' })
+
+      expect(beforeUpdateSpy).to.have.been.called
+      expect(afterUpdateSpy).to.have.been.called
+      expect(beforeBulkUpdateSpy).not.to.have.been.called
+    })
+  })
+
+  describe('#delete', () => {
+    beforeEach(async function () {
+      SequelizeModel = db.sequelize.models.User
+      resource = new Resource(SequelizeModel)
+      this.params = {
+        firstName: 'john',
+        lastName: 'doe',
+        email: '[email protected]',
+      }
+      this.record = await resource.create(this.params)
+    })
+
+    it('deletes the resource', async function () {
+      await resource.delete(this.record.id)
+
+      const newRecord = await resource.findOne(this.record.id)
+
+      expect(newRecord).to.be.null
+    })
+
+    it('calls delete hooks', async function () {
+      const beforeDestroySpy = sinon.spy()
+      const afterDestroySpy = sinon.spy()
+      const beforeDestroyUpdateSpy = sinon.spy()
+      SequelizeModel.addHook('beforeDestroy', beforeDestroySpy)
+      SequelizeModel.addHook('beforeBulkDestroy', beforeDestroyUpdateSpy)
+      SequelizeModel.addHook('afterDestroy', afterDestroySpy)
+
+      await resource.delete(this.record.id)
+
+      expect(beforeDestroySpy).to.have.been.called
+      expect(afterDestroySpy).to.have.been.called
+      expect(beforeDestroyUpdateSpy).not.to.have.been.called
     })
   })
 })

src/resource.ts

@@ -14,7 +14,7 @@ const SEQUELIZE_UNIQUE_ERROR = 'SequelizeUniqueConstraintError'
 // this fixes problem with unbound this when you setup type of Mode as a member of another
 // class: https://stackoverflow.com/questions/55166230/sequelize-typescript-typeof-model
 type Constructor<T> = new (...args: any[]) => T;
-type ModelType<T extends Model<T>> = Constructor<T> & typeof Model;
+export type ModelType<T extends Model<T>> = Constructor<T> & typeof Model;
 
 type FindOptions = {
   limit?: number;
@@ -127,8 +127,11 @@ class Resource extends BaseResource {
     return sequelizeObjects.map((sequelizeObject) => new BaseRecord(sequelizeObject.toJSON(), this))
   }
 
-  async findOne(id) {
+  async findOne(id): Promise<BaseRecord | null> {
     const sequelizeObject = await this.findById(id)
+    if (!sequelizeObject) {
+      return null
+    }
     return new BaseRecord(sequelizeObject.toJSON(), this)
   }
 
@@ -148,7 +151,7 @@ class Resource extends BaseResource {
     return this.SequelizeModel[method](id)
   }
 
-  async create(params) {
+  async create(params): Promise<Record<string, any>> {
     const parsedParams = this.parseParams(params)
     const unflattedParams = unflatten(parsedParams)
     try {
@@ -173,6 +176,8 @@ class Resource extends BaseResource {
         where: {
           [this.primaryKey()]: id,
         },
+        individualHooks: true,
+        hooks: false,
       })
       const record = await this.findById(id)
       return record.toJSON()
@@ -187,12 +192,13 @@ class Resource extends BaseResource {
     }
   }
 
-  async delete(id) {
-    this.SequelizeModel.destroy({
-      where: {
-        [this.primaryKey()]: id,
-      },
-    })
+  async delete(id): Promise<void> {
+    // we find first because we need to invoke destroy on model, so all hooks
+    // instance hooks (not bulk) are called.
+    // We cannot set {individualHooks: true, hooks: false} in this.SequelizeModel.destroy,
+    // as it is in #update method because for some reason it wont delete the record
+    const model = await this.SequelizeModel.findByPk(id)
+    await model.destroy()
   }
 
   /**