Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(ngResource): Remove request body from $delete #4281

Closed
wants to merge 1 commit into from

Conversation

bourey
Copy link
Contributor

@bourey bourey commented Oct 4, 2013

Prevent the obj.$delete instance method from sending the resource as the request body. This commit uses the existing hasBody boolean to only set httpConfig.data for methods which should have a request body.

Closes #4280

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

Prevent the  obj.$delete instance method from sending the resource as the request body.  This commit uses the existing hasBody boolean to only set httpConfig.data for methods which should have a request body.

Closes angular#4280
@bourey
Copy link
Contributor Author

bourey commented Oct 4, 2013

Signed CLA (as Jennifer Bourey) and updated commit message format.

@ghost ghost assigned jeffbcross Oct 4, 2013
@btford
Copy link
Contributor

btford commented Oct 4, 2013

Thanks @bourey!

I think this was already fixed in master by 0d0330a, though I may be mistaken.

Can you try this recent build from master and see if the issue persists? http://ci.angularjs.org/job/angular.js-angular-master/1659/artifact/build/angular.js

If not, can you please add a test to this file demonstrating the issue: https://github.com/angular/angular.js/blob/master/test/ngResource/resourceSpec.js

@bourey
Copy link
Contributor Author

bourey commented Oct 4, 2013

Unfortunately I don't think the above change fixes this issue. I'm seeing the entire serialized object included in the request, rather than simply an empty string.

Embarrassingly enough I'm not really currently set up to run karma tests normally, but as a quick test that I demonstrated the following test failing in our local build (thanks for the collaboration @jeffbcross!):

it('should not include a request body when calling $delete', function() {
  mockBackend.expect('DELETE', '/fooresource', null).respond({});
  var Resource = $resource('/fooresource');
  var resource = new Resource({ foo: 'bar' });

  resource.$delete();
  mockBackend.flush();
});

@jeffbcross
Copy link
Contributor

That test does the trick, thanks @bourey!

@jeffbcross
Copy link
Contributor

Just waiting for CI to tell me everything's gonna be alright: http://ci.angularjs.org/job/angular.js-jeff/71/

@jeffbcross
Copy link
Contributor

Merged: 8336b3a

@jeffbcross jeffbcross closed this Oct 5, 2013
@btford
Copy link
Contributor

btford commented Oct 5, 2013

👍 great work team

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$resource obj.$delete sends the resource as the request body
4 participants