From e45a9f6b415cb01adcd5b672e39004ab7063dbf4 Mon Sep 17 00:00:00 2001 From: Rob McGuinness Date: Wed, 3 Jan 2018 20:29:49 -0500 Subject: [PATCH] fix(api-core): remove/delete should not assume data payloads BREAKING CHANGE: previously remove/delete would assume that data was being passed in the body of the request if the first param of the message signature was NOT a string or number. Now, the method assumes a config object is passed in instead of data. This allows the developers to pass in params or data as they see fit. --- packages/api-core/src/api.js | 14 +++-- .../api-core/src/resources/notifications.js | 4 +- ...fication.test.js => notifications.test.js} | 4 +- packages/api-core/src/tests/api.test.js | 54 ++++++++----------- 4 files changed, 31 insertions(+), 45 deletions(-) rename packages/api-core/src/resources/tests/{notification.test.js => notifications.test.js} (83%) diff --git a/packages/api-core/src/api.js b/packages/api-core/src/api.js index d0545ed3e..7cd636a60 100644 --- a/packages/api-core/src/api.js +++ b/packages/api-core/src/api.js @@ -234,30 +234,28 @@ export default class AvApi { return this.request(config, this.afterUpdate || this.afterPut); } - put(id, data, config) { - return this.update(id, data, config); + put(...args) { + return this.update(args); } // delete request remove(id, config) { - let data; if (typeof id !== 'string' && typeof id !== 'number') { - data = id; + config = id; id = ''; } config = this.config(config); config.method = 'DELETE'; config.url = this.getUrl(config, id); - config.data = data; const beforeFunc = this.beforeRemove || this.beforeDelete; if (beforeFunc) { - config.data = beforeFunc(config.data); + config = beforeFunc(config); } return this.request(config, this.afterRemove || this.afterDelete); } - delete(id, config) { - return this.remove(id, config); + delete(...args) { + return this.remove(args); } } diff --git a/packages/api-core/src/resources/notifications.js b/packages/api-core/src/resources/notifications.js index 34744e9e3..151287c81 100644 --- a/packages/api-core/src/resources/notifications.js +++ b/packages/api-core/src/resources/notifications.js @@ -1,6 +1,6 @@ import AvApi from '../api'; -export default class AvNotification extends AvApi { +export default class AvNotifications extends AvApi { constructor(http, promise, config = {}) { const options = Object.assign( { @@ -14,6 +14,6 @@ export default class AvNotification extends AvApi { deleteByTopic(topic) { const params = Object.assign({}, { topicId: topic }); - return this.remove(Object.assign({}, { params })); + return this.remove({ params }); } } diff --git a/packages/api-core/src/resources/tests/notification.test.js b/packages/api-core/src/resources/tests/notifications.test.js similarity index 83% rename from packages/api-core/src/resources/tests/notification.test.js rename to packages/api-core/src/resources/tests/notifications.test.js index c6a62154e..c1a9ed951 100644 --- a/packages/api-core/src/resources/tests/notification.test.js +++ b/packages/api-core/src/resources/tests/notifications.test.js @@ -1,4 +1,4 @@ -import AvNotification from '../notification'; +import AvNotification from '../notifications'; const mockHttp = jest.fn(() => Promise.resolve({})); @@ -15,7 +15,7 @@ describe('AvNotification', () => { expect(api).toBeDefined(); }); - test('deleteByTopic() should call query with topic added to params.topicId', () => { + test('deleteByTopic() should call remove with topic added to params.topicId', () => { api = new AvNotification(mockHttp, Promise, {}); api.remove = jest.fn(); diff --git a/packages/api-core/src/tests/api.test.js b/packages/api-core/src/tests/api.test.js index 5928a4fbb..aa205f86b 100644 --- a/packages/api-core/src/tests/api.test.js +++ b/packages/api-core/src/tests/api.test.js @@ -719,46 +719,34 @@ describe('AvApi', () => { expect(api.request).toHaveBeenLastCalledWith(expectedConfig, undefined); }); - test('remove() should set first argument as data if not string or number', () => { + test('remove() should set first argument as config if not string or number', () => { const config = { - testValue: 'test', - }; - const data = { - testData: 'data', - }; - const expectedConfig = Object.assign( - { - method: 'DELETE', - url: testUrl, + method: 'DELETE', + url: testUrl, + data: { + a: '1', + b: '2', }, - config, - { data } - ); - api.remove(data, config); - expect(api.getUrl).toHaveBeenLastCalledWith(expectedConfig, ''); - expect(api.request).toHaveBeenLastCalledWith(expectedConfig, undefined); + }; + api.remove(config); + expect(api.getUrl).toHaveBeenLastCalledWith(config, ''); + expect(api.request).toHaveBeenLastCalledWith(config, undefined); }); - test('remove() should run data through beforeRemove if defined', () => { + test('remove() should call beforeRemove() if defined', () => { const config = { - testValue: 'test', - }; - const data = { - testData: 'data', - }; - const expectedConfig = Object.assign( - { - method: 'DELETE', - url: testUrl, + method: 'DELETE', + url: testUrl, + data: { + a: '1', + b: '2', }, - config, - { data } - ); + }; api.beforeRemove = jest.fn(thisData => thisData); - api.remove(data, config); - expect(api.getUrl).toHaveBeenLastCalledWith(expectedConfig, ''); - expect(api.request).toHaveBeenLastCalledWith(expectedConfig, undefined); - expect(api.beforeRemove).toHaveBeenLastCalledWith(data); + api.remove(config); + expect(api.getUrl).toHaveBeenLastCalledWith(config, ''); + expect(api.request).toHaveBeenLastCalledWith(config, undefined); + expect(api.beforeRemove).toHaveBeenLastCalledWith(config); }); }); });