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

Enable configurable root URL paths #164

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Enable configurable root URL paths #164

wants to merge 1 commit into from

Conversation

blag
Copy link
Contributor

@blag blag commented Sep 18, 2023

Allow this app's URLConf to be parented in more places than just /account/.

Description

This PR removes the /account/ prefix from this app's URLConf, allowing developers to choose for themselves what URL prefixes they would like.

To ensure backwards compatibility, I adjusted the root URLConfs in the tests project and the example project to still use the /account/ prefix. Furthermore, I also adjusted the documentation to instruct users to do the same:

...
urls = [
    path('account/', include('user_sessions.urls', namespace='user_sessions')),
]

Advanced users should hopefully understand that they can adjust that prefix to whatever they'd like, and beginner users will simply mirror the documentation and continue with the previous behavior.

Motivation and Context

The previously behavior isn't necessarily congruous with django-allauth, which uses /accounts/, and other users might want different prefixes altogether.

Closes #43 and #47.

How Has This Been Tested?

I updated the URLConf in the tests project and relied on GHA tests.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes. - N/A
  • All new and existing tests passed.

Assuming that tests for Python 3.7+Django 3.2 pass (currently running), test results for this branch have not changed from test results for master.

@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Merging #164 (7538ae0) into master (eb315b2) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #164   +/-   ##
=======================================
  Coverage   97.67%   97.67%           
=======================================
  Files          17       17           
  Lines         689      689           
  Branches       56       56           
=======================================
  Hits          673      673           
  Misses         10       10           
  Partials        6        6           
Files Coverage Δ
tests/urls.py 100.00% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@WhyNotHugo
Copy link
Member

Needs rebase

Copy link
Member

@WhyNotHugo WhyNotHugo left a comment

Choose a reason for hiding this comment

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

Do you think we should include a changelog entry? This might affect existing usages.

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.

Hard coded URL part
2 participants