-
Notifications
You must be signed in to change notification settings - Fork 27.4k
feat(ngResource): make stripping of trailing slashes configurable for $resource
#5560
Conversation
I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let me know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
It's signed now. |
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
Fixed commit message format. |
Added some more options for configuring this. First, this now uses a flat object configuration, similar to This adds a fourth optional argument to the Now, both of these ways of configuring this is supported:
or per instance:
|
Docs and tests are added, too. Please let me know if I need to improve anything. |
@@ -74,6 +74,18 @@ function shallowClearAndCopy(src, dst) { | |||
* | |||
* Requires the {@link ngResource `ngResource`} module to be installed. | |||
* | |||
* By default, trailing slashes will be stripped from the calculated URLs, | |||
* which can pose problems with server backends that do not expect that | |||
* behaviour. This can be disabled by configuring the `$resourceProvider` like |
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.
'behavior' - we use US english in our docs (or at least try to :-) )
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.
Addressed in 906642a.
otherwise this looks ok. since it's a feature I'm going to triage it for 1.3 milestone |
First, this now uses a flat object configuration, similar to `$httpBackend`. This should make configuring this provider much more familiar. This adds a fourth optional argument to the `$resource()` constructor, supporting overriding global `$resourceProvider` configuration. Now, both of these ways of configuring this is supported: app.config(function($resourceProvider) { $resourceProvider.defaults.stripTrailingSlashes = false; }); or per instance: var CreditCard = $resource('/some/:url/', ..., ..., { stripTrailingSlashes: false });
I've squashed my commits and rebased that on |
Looks like this is failing the e2e tests with this error: "Some of your tests did a full page reload!" |
I'm not sure what is causing that and how I should address this. Can you give me a little direction here? |
@nvie It looks like a Travis/Sauce Labs hiccup. I restarted the job, let's see if it succeeds this time. |
@nvie You need to rebase your branch onto the latest master. Your branch still tries to test on IE8 which was already removed. Not sure if that's the problem but sth similar might be. |
@nvie ping? |
I rebased it myself. Landed at 3878be5, thanks! |
Awesome, thanks @mzgol! |
…ash. This change required an update to AngularJS ~1.3.0 which currently resolves 1.3.0-rc.2. This is the thing we needed: angular/angular.js#5560 The feature is documented in the AngularJS API reference: https://docs.angularjs.org/api/ngResource/service/$resource
the trailing slash issue has now been fixed in angular 1.3 angular/angular.js#5560
the trailing slash issue has now been fixed in angular 1.3 angular/angular.js#5560
By default, trailing slashes will be stripped from the calculated URLs, which can pose problems with server backends that do not expect that behavior. This patch aims to make this behavior configurable via:
or per instance, using a new, fourth, optional argument to the
$resource()
constructor:This is targeted to be a fix for #992. Please let me know if anything is missing from this, or you need me to change anything to make the implementation more idiomatic.
PS: I'm sorry for the seemingly large patch, but most of it is just indentation (due to rewriting the factory as a provider). I've tried to isolate the whitespace change in a separate commit.