Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExtendedDefaultRouter root view is polluted #57

Closed
cancan101 opened this issue Nov 24, 2014 · 11 comments
Closed

ExtendedDefaultRouter root view is polluted #57

cancan101 opened this issue Nov 24, 2014 · 11 comments

Comments

@cancan101
Copy link

I am using ExtendedDefaultRouter with nested routes. Right now my root view is being polluted by the sub items:

GET /api/
{
    "people": "http://127.0.0.1:8000/api/people/", 
    "people/(?P<parent_lookup_person__uuid>[^/.]+)/tests": "http://127.0.0.1:8000/api/tests/",
    "tests": "http://127.0.0.1:8000/api/tests/"
}
@kevin-brown
Copy link
Contributor

This is because your tests and people/(?P<parent_lookup_person__uuid>[^/.]+)/tests views both have the same base_name of test. You can tell because both of them are (incorrectly) pointing to the same url, and the first one is not triggering an error.

The ExtendedDefaultRouter depends on the first one triggering a reverse error, so it can be excluded from the output.

@cancan101
Copy link
Author

I saw that the URL was duplicated. This does not fully explain the issue, however.

In[5]: reverse('test-list')
Out[5]: '/api/tests/'
In[7]:  reverse('test-list', kwargs={'parent_lookup_person__uuid':'alex'})
Out[7]: '/api/people/alex/tests/'

@cancan101
Copy link
Author

Either way, the nested item should not be exposed at the top level (ie root view). The only reason this works when changing the base name is:

                for key, url_name in api_root_dict.items():
                    try:
                        ret[key] = reverse(url_name, request=request, format=format)
                    except NoReverseMatch:
                        pass

where key is people/(?P<parent_lookup_person__uuid>[^/.]+)/tests and url_name is <base_name>-list.

Changing the base_name to something else causes NoReverseMatch to be raised so the item is silently hidden from the root view.

There is no reason that base_name should not be able to be set to the same value for both levels.

Right now I am trying to move to drf-extensions from https://github.com/alanjds/drf-nested-routers and I had been able to use the same base_name without an issue before.

@cancan101
Copy link
Author

The issue here is that register called on NestedRegistryItem adds to the registry of the underlying router rather than keeping a separate list of sub URLs.

@kevin-brown
Copy link
Contributor

This does not fully explain the issue, however...

Yes, multiple routes can share the same name and take in different arguments.

Changing the base_name to something else causes NoReverseMatch to be raised so the item is silently hidden from the root view.

This is intentional and it was added in #14. I don't know of a better way to handle it, but I'm sure a pull request would be reviewed if it came in.

Right now I am trying to move to drf-extensions from https://github.com/alanjds/drf-nested-routers and I had been able to use the same base_name without an issue before.

If I remember correctly, drf-nested-routers did not support the DefaultRouter and using the one provided by DRF should have had the same issue.

@cancan101
Copy link
Author

@kevin-brown I think the root cause for all of this is: #57 (comment)

That is implicitly how drf-extensions deals with the issue: it does NOT add all of the routes to the base router but required you to add those routers individually.

@cancan101
Copy link
Author

@kevin-brown Any more thoughts on this?

@cancan101 cancan101 reopened this Dec 4, 2014
@cancan101
Copy link
Author

@kevin-brown Thoughts?

@kevin-brown
Copy link
Contributor

@cancan101 looks like it's up to @chibisov.

For what it's worth, you'd have this issue without the custom router now that DRF also has the try...except code in place.

@cancan101
Copy link
Author

@kevin-brown:

The issue here is that register called on NestedRegistryItem adds to the registry of the underlying router rather than keeping a separate list of sub URLs.

This is different from how drf-nested-routers works where each nested router keeps track of the URLs (routes) at its level.

@auvipy
Copy link
Collaborator

auvipy commented Dec 11, 2015

closing

@auvipy auvipy closed this as completed Dec 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants