Skip to content

Commit e45a9f6

Browse files
author
Rob McGuinness
committed
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.
1 parent 5cd35bf commit e45a9f6

File tree

4 files changed

+31
-45
lines changed

4 files changed

+31
-45
lines changed

packages/api-core/src/api.js

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -234,30 +234,28 @@ export default class AvApi {
234234
return this.request(config, this.afterUpdate || this.afterPut);
235235
}
236236

237-
put(id, data, config) {
238-
return this.update(id, data, config);
237+
put(...args) {
238+
return this.update(args);
239239
}
240240

241241
// delete request
242242
remove(id, config) {
243-
let data;
244243
if (typeof id !== 'string' && typeof id !== 'number') {
245-
data = id;
244+
config = id;
246245
id = '';
247246
}
248247
config = this.config(config);
249248
config.method = 'DELETE';
250249
config.url = this.getUrl(config, id);
251-
config.data = data;
252250

253251
const beforeFunc = this.beforeRemove || this.beforeDelete;
254252
if (beforeFunc) {
255-
config.data = beforeFunc(config.data);
253+
config = beforeFunc(config);
256254
}
257255
return this.request(config, this.afterRemove || this.afterDelete);
258256
}
259257

260-
delete(id, config) {
261-
return this.remove(id, config);
258+
delete(...args) {
259+
return this.remove(args);
262260
}
263261
}

packages/api-core/src/resources/notifications.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import AvApi from '../api';
22

3-
export default class AvNotification extends AvApi {
3+
export default class AvNotifications extends AvApi {
44
constructor(http, promise, config = {}) {
55
const options = Object.assign(
66
{
@@ -14,6 +14,6 @@ export default class AvNotification extends AvApi {
1414

1515
deleteByTopic(topic) {
1616
const params = Object.assign({}, { topicId: topic });
17-
return this.remove(Object.assign({}, { params }));
17+
return this.remove({ params });
1818
}
1919
}

packages/api-core/src/resources/tests/notification.test.js renamed to packages/api-core/src/resources/tests/notifications.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import AvNotification from '../notification';
1+
import AvNotification from '../notifications';
22

33
const mockHttp = jest.fn(() => Promise.resolve({}));
44

@@ -15,7 +15,7 @@ describe('AvNotification', () => {
1515
expect(api).toBeDefined();
1616
});
1717

18-
test('deleteByTopic() should call query with topic added to params.topicId', () => {
18+
test('deleteByTopic() should call remove with topic added to params.topicId', () => {
1919
api = new AvNotification(mockHttp, Promise, {});
2020
api.remove = jest.fn();
2121

packages/api-core/src/tests/api.test.js

Lines changed: 21 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -719,46 +719,34 @@ describe('AvApi', () => {
719719
expect(api.request).toHaveBeenLastCalledWith(expectedConfig, undefined);
720720
});
721721

722-
test('remove() should set first argument as data if not string or number', () => {
722+
test('remove() should set first argument as config if not string or number', () => {
723723
const config = {
724-
testValue: 'test',
725-
};
726-
const data = {
727-
testData: 'data',
728-
};
729-
const expectedConfig = Object.assign(
730-
{
731-
method: 'DELETE',
732-
url: testUrl,
724+
method: 'DELETE',
725+
url: testUrl,
726+
data: {
727+
a: '1',
728+
b: '2',
733729
},
734-
config,
735-
{ data }
736-
);
737-
api.remove(data, config);
738-
expect(api.getUrl).toHaveBeenLastCalledWith(expectedConfig, '');
739-
expect(api.request).toHaveBeenLastCalledWith(expectedConfig, undefined);
730+
};
731+
api.remove(config);
732+
expect(api.getUrl).toHaveBeenLastCalledWith(config, '');
733+
expect(api.request).toHaveBeenLastCalledWith(config, undefined);
740734
});
741735

742-
test('remove() should run data through beforeRemove if defined', () => {
736+
test('remove() should call beforeRemove() if defined', () => {
743737
const config = {
744-
testValue: 'test',
745-
};
746-
const data = {
747-
testData: 'data',
748-
};
749-
const expectedConfig = Object.assign(
750-
{
751-
method: 'DELETE',
752-
url: testUrl,
738+
method: 'DELETE',
739+
url: testUrl,
740+
data: {
741+
a: '1',
742+
b: '2',
753743
},
754-
config,
755-
{ data }
756-
);
744+
};
757745
api.beforeRemove = jest.fn(thisData => thisData);
758-
api.remove(data, config);
759-
expect(api.getUrl).toHaveBeenLastCalledWith(expectedConfig, '');
760-
expect(api.request).toHaveBeenLastCalledWith(expectedConfig, undefined);
761-
expect(api.beforeRemove).toHaveBeenLastCalledWith(data);
746+
api.remove(config);
747+
expect(api.getUrl).toHaveBeenLastCalledWith(config, '');
748+
expect(api.request).toHaveBeenLastCalledWith(config, undefined);
749+
expect(api.beforeRemove).toHaveBeenLastCalledWith(config);
762750
});
763751
});
764752
});

0 commit comments

Comments
 (0)