-
Notifications
You must be signed in to change notification settings - Fork 48
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
[urls] creating a new user gave NoReverseMatch (solves #7) #9
[urls] creating a new user gave NoReverseMatch (solves #7) #9
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job @shashwat1002, some minor changes requested, see below
tests/settings.py
Outdated
@@ -25,7 +25,7 @@ | |||
# SECURITY WARNING: don't run with debug turned on in production! | |||
DEBUG = True | |||
|
|||
ALLOWED_HOSTS = [] | |||
ALLOWED_HOSTS = ['testserver', 'localhost',] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nemesisdesign What about this? Shouldn't this do to local_settings.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atb00ker , my test will not pass if I don't add this. Because I'm using django.test.Client which uses a testserver. Since the build won't work without this, I added it to the main settings file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atb00ker thanks for spotting this, it is indeed symptom of an issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once you address the previous issue you should not need this change anymore, although you can keep localhost
019f853
to
1b6da41
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there..
Check your commit message, seems a [
is missing. I'd also simplify it to:
[urls] Fixed NoReverseMatch on user creation #7
Fixes #7
tests/settings.py
Outdated
@@ -25,7 +25,7 @@ | |||
# SECURITY WARNING: don't run with debug turned on in production! | |||
DEBUG = True | |||
|
|||
ALLOWED_HOSTS = [] | |||
ALLOWED_HOSTS = ['testserver', 'localhost',] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once you address the previous issue you should not need this change anymore, although you can keep localhost
@nemesisdesign , which issue are you referring to? |
I tried running tests again, without testserver, it seems to work now. I don't understand why, it was suggesting that I add 'testserver' to allowed hosts before. Nonetheless I'm removing that and committing. |
1b6da41
to
143a499
Compare
143a499
to
da75a6f
Compare
@nemesisdesign , made the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This code effectively adds a test corresponding to this error(issue #7) and includes allauth urls to solve the problem.
Fixes #7