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

feat(ngResource): make stripping of trailing slashes configurable for $resource #5560

Closed
wants to merge 1 commit into from

Conversation

nvie
Copy link
Contributor

@nvie nvie commented Dec 29, 2013

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:

app.config(function($resourceProvider) {
  $resourceProvider.defaults.stripTrailingSlashes = false;
});

or per instance, using a new, fourth, optional argument to the $resource() constructor:

var CreditCard = $resource('/some/:url/', ..., ..., {
    stripTrailingSlashes: false
});

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.

@IgorMinar
Copy link
Contributor

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.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@nvie
Copy link
Contributor Author

nvie commented Dec 29, 2013

It's signed now.

@IgorMinar
Copy link
Contributor

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).

@nvie
Copy link
Contributor Author

nvie commented Dec 29, 2013

Fixed commit message format.

@nvie
Copy link
Contributor Author

nvie commented Dec 29, 2013

Added some more options for configuring this.

First, this now uses a flat object configuration, similar to $httpProvider.defaults. 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
});

@nvie
Copy link
Contributor Author

nvie commented Dec 29, 2013

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
Copy link
Contributor

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 :-) )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 906642a.

@IgorMinar
Copy link
Contributor

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
    });
@nvie
Copy link
Contributor Author

nvie commented Jan 3, 2014

I've squashed my commits and rebased that on master. The result is 141c88b.

@nelsonpecora
Copy link

Looks like this is failing the e2e tests with this error: "Some of your tests did a full page reload!"

@nvie
Copy link
Contributor Author

nvie commented Mar 11, 2014

I'm not sure what is causing that and how I should address this. Can you give me a little direction here?

@mgol
Copy link
Member

mgol commented Mar 11, 2014

@nvie It looks like a Travis/Sauce Labs hiccup. I restarted the job, let's see if it succeeds this time.

@mgol
Copy link
Member

mgol commented Mar 11, 2014

@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.

@mgol
Copy link
Member

mgol commented Apr 9, 2014

@nvie ping?

@mgol
Copy link
Member

mgol commented Apr 9, 2014

I rebased it myself. Landed at 3878be5, thanks!

@mgol mgol closed this Apr 9, 2014
@nvie
Copy link
Contributor Author

nvie commented Apr 10, 2014

Awesome, thanks @mzgol!

kwk added a commit to kwk/docker-registry-frontend that referenced this pull request Sep 18, 2014
…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
robbyt added a commit to robbyt/django-angular that referenced this pull request Oct 7, 2014
the trailing slash issue has now been fixed in angular 1.3

angular/angular.js#5560
kimarakov added a commit to kimarakov/django_angular that referenced this pull request Dec 29, 2021
the trailing slash issue has now been fixed in angular 1.3

angular/angular.js#5560
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants