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

raise ImproperlyConfigured exception if basename is not unique #8438

Merged
merged 5 commits into from Dec 10, 2022

Conversation

dgravitate
Copy link
Contributor

When basename is dynamically generated in BaseRouter.get_default_basename, it could (and does) generate a conflicting basename when multiple routes use the same Model. Additionally, developers could inadvertently use a basename that has already been used elsewhere.

In either case, this can lead to unexpected results when (correctly) using reverse() to get the URL

@stale
Copy link

stale bot commented May 31, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 31, 2022
@auvipy auvipy self-requested a review November 22, 2022 07:10
@stale stale bot removed the stale label Nov 22, 2022
@auvipy
Copy link
Member

auvipy commented Nov 22, 2022

@sevdog can you review this please?

tests/test_routers.py Outdated Show resolved Hide resolved
rest_framework/routers.py Outdated Show resolved Hide resolved
rest_framework/routers.py Outdated Show resolved Hide resolved
@auvipy auvipy added this to the 3.14 milestone Nov 22, 2022
@dgravitate dgravitate marked this pull request as draft December 1, 2022 03:23
@dgravitate dgravitate marked this pull request as ready for review December 1, 2022 04:21
Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

can we add additional tests using DefaultRouter() ?

@dgravitate
Copy link
Contributor Author

can we add additional tests using DefaultRouter() ?

Added additional tests using DefaultRouter

@auvipy
Copy link
Member

auvipy commented Dec 8, 2022

I think this should be accepted as this change add checks which is good for developer experience. And not introduce any API surface or major behavioral changes

rest_framework/routers.py Outdated Show resolved Hide resolved
@auvipy auvipy merged commit 48a21aa into encode:master Dec 10, 2022
kernicPanel added a commit to openfun/joanie that referenced this pull request Mar 18, 2024
DRF verson 3.15 raises a ImproperlyConfigured exception if url basenames are
not uniques.
encode/django-rest-framework#8438
kernicPanel added a commit to openfun/joanie that referenced this pull request Mar 18, 2024
DRF verson 3.15 raises a ImproperlyConfigured exception if url basenames are
not uniques.
encode/django-rest-framework#8438
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants