The automatic breadcrumbs don't work when WSGIScriptAlias isn't "/" #211

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

As per https://code.djangoproject.com/ticket/10328, the Django-relative part of the URL is stored in the requests "path_info" attribute, while the "path" attribute also includes the WSGIScriptAlias portion.

The breadcrumb generator is currently using the wrong one, and hence all of the calls to "resolve()" fail with a 404 and no breadcrumbs are reported.

It turns out the patch in this pull request isn't sufficient - the breadcrumbs it generates are missing the prefix from the returned URLs

To get usable breadcrumbs when serving pages from a location other than the root URL, I had to do the following:

    broken_crumbs = get_breadcrumbs(request.path_info)
    prefix = request.path[:-len(request.path_info)]
    crumbs = [(crumb_name, prefix + crumb_url)
                 for crumb_name, crumb_url in broken_crumbs]
Contributor

mammique commented Aug 17, 2012

I fix this in my 'prod' fork, here: mammique/django-rest-framework@9c160be

Camille.

Collaborator

tomchristie commented Sep 19, 2012

I believe .get_script_prefix() is the proper way to do this.

Collaborator

tomchristie commented Sep 19, 2012

Anyone care to review the pull req I've made?

Collaborator

tomchristie commented Sep 19, 2012

And, uh, apologies for the delay! :)
Getting on top of this all again, now.

Contributor

mammique commented Sep 20, 2012

I'll try to test this tonight, I keep you posted. Thanks.

Collaborator

tomchristie commented Sep 20, 2012

Thanks Camille, appreciated.

On 20 September 2012 13:53, Camille Harang notifications@github.com wrote:

I'll try to test this tonight, I keep you posted. Thanks.


Reply to this email directly or view it on GitHubhttps://github.com/tomchristie/django-rest-framework/pull/211#issuecomment-8727375.

Collaborator

tomchristie commented Sep 20, 2012

Bear in mind that it's against the 2.0 version. You might want to use the
first bit of the tutorial for testing.
Could patch against master as well if helpful?

On 20 September 2012 14:37, Tom Christie tom@tomchristie.com wrote:

Thanks Camille, appreciated.

On 20 September 2012 13:53, Camille Harang notifications@github.comwrote:

I'll try to test this tonight, I keep you posted. Thanks.


Reply to this email directly or view it on GitHubhttps://github.com/tomchristie/django-rest-framework/pull/211#issuecomment-8727375.

Contributor

mammique commented Sep 20, 2012

Ah, OK, it will take a bit longer then. The only instance I have that isn't running under / is in production mode, I cannot make such deep changes now. I should better try to deploy an nginx subpath instance while testing 2.0. I tell you as soon as I can test.

Camille.

Collaborator

tomchristie commented Sep 27, 2012

Also pushed to master.
Will push a 0.4.1 release shortly as the final release prior to the 2.0 launch.

@phobologic phobologic pushed a commit to phobologic/django-rest-framework that referenced this pull request Oct 8, 2012

@tomchristie tomchristie + vagrant Breadcrumbs play nicely when app not installed at root URL. Fixes #211 aba9170
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment