-
Notifications
You must be signed in to change notification settings - Fork 27.4k
Add hasBody to ngResource action configuration #12181
Changes from 6 commits
745d347
c6f2687
9e23624
be8aeeb
c8212ad
cf811f5
36ce9f0
9139302
77d9cfa
4e2a874
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,6 +97,72 @@ describe('basic usage', function() { | |
$httpBackend.flush(); | ||
}); | ||
|
||
it('should include a request body when calling custom method with hasBody is true', function() { | ||
var instant = {name: 'info.txt', value: 'V2hlbiB0aGUgdGltZSBlbmRzLg=='}; | ||
var fid = 42; | ||
var created = {fid: fid, filname: 'fooresource', value: 'V2hlbiB0aGUgdGltZSBlbmRzLg=='}; | ||
var condition = {at: '2038-01-19 03:14:08'}; | ||
$httpBackend.expect('CREATE', '/fooresource', instant).respond(created); | ||
$httpBackend.expect('DELETE', '/fooresource', condition).respond({}); | ||
|
||
var r = $resource('/fooresource', {}, { | ||
create: {method: 'CREATE', hasBody: true}, | ||
delete: {method: 'DELETE', hasBody: true} | ||
}); | ||
|
||
var creationResponse = r.create(instant); | ||
var deleteResponse = r.delete(condition); | ||
|
||
$httpBackend.flush(); | ||
|
||
expect(creationResponse.fid).toBe(fid); | ||
expect(deleteResponse.$resolved).toBe(true); | ||
}); | ||
|
||
it('should not include a request body if hasBody is false on POST, PUT and PATCH', function() { | ||
$httpBackend.expect('POST', '/foo').respond({}); | ||
$httpBackend.expect('PUT', '/foo').respond({}); | ||
$httpBackend.expect('PATCH', '/foo').respond({}); | ||
|
||
var R = $resource('/foo', {}, { | ||
post: {method: 'POST', hasBody: false}, | ||
put: {method: 'PUT', hasBody: false}, | ||
patch: {method: 'PATCH', hasBody: false} | ||
}); | ||
|
||
var postResponse = R.post(); | ||
var putResponse = R.put(); | ||
var patchResponse = R.patch(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would these 3 calls fail if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No they don't fail if hasBody is not set. This is a bug or a feature of ngResource not enforcing the described api. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't found a way to distinguish between request parameters and the request body in httpBackend. Did I miss something? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you pass two params (e.g. For example, the following test currently fails, because $httpBackend.expectPOST('/foo').respond(function(method, url, data) {
expect(data).not.toBeUndefined();
return [200, {id: 42}];
});
var R = $resource('/foo');
var response = R.save(null, {id: -1});
$httpBackend.flush();
expect(response.id).toBe(42); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sry for the delayed response on my side. I have merged your proposed test case into the test method with hasBody true. The only problem for me was that I couldn't reproduce the failing of the test case. (Even when I used your version.) Can you see my error or give me the error that you get? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was probably not clear. What I meant is that my test fails with the code currently in master (i.e. without your PR), which is expected, because we don't support the |
||
|
||
$httpBackend.flush(); | ||
|
||
expect(postResponse.$resolved).toBe(true); | ||
expect(putResponse.$resolved).toBe(true); | ||
expect(patchResponse.$resolved).toBe(true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, change these two lines to |
||
}); | ||
|
||
it('should expect a body if hasBody is true', function() { | ||
var username = 'yathos'; | ||
var loginRequest = {name: username, password: 'Smile'}; | ||
var user = {id: 1, name: username}; | ||
|
||
$httpBackend.expect('LOGIN', '/user/me', loginRequest).respond(user); | ||
|
||
$httpBackend.expect('LOGOUT', '/user/me', null).respond(null); | ||
|
||
var UserService = $resource('/user/me', {}, { | ||
login: {method: 'LOGIN', hasBody: true}, | ||
logout: {method: 'LOGOUT', hasBody: false} | ||
}); | ||
|
||
var loginResponse = UserService.login(loginRequest); | ||
var logoutResponse = UserService.logout(); | ||
|
||
$httpBackend.flush(); | ||
|
||
expect(loginResponse.id).toBe(user.id); | ||
expect(logoutResponse.$resolved).toBe(true); | ||
}); | ||
|
||
it('should build resource', function() { | ||
expect(typeof CreditCard).toBe('function'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: You don't really need these 2 extra variables (
fid
,created
). You could inline them (and only keep the necessary props) to avoid visual noise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have cleaned up those variables.