-
Notifications
You must be signed in to change notification settings - Fork 18
Issue#17 #18
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
base: master
Are you sure you want to change the base?
Issue#17 #18
Conversation
Varunvaruns9
left a comment
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.
A review for these changes.
carpool/views.py
Outdated
| if request.user.is_authenticated: | ||
| return HttpResponseRedirect('/') | ||
| if request.method == 'POST': | ||
| elif request.method == 'POST': |
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.
elif is not required here as the first if has a return at its end.
| # email.send() | ||
| else: | ||
| form = SignUpForm() | ||
| return render(request, 'signup.html', {'form': form, 'error': error, 'done': done, }) |
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.
It is better to have a combined render rather than using the same code three times, just remove the done variable.
carpool/views.py
Outdated
| to_email = form.cleaned_data.get('email') | ||
| send_mail(mail_subject, message, '[email protected]', [to_email], fail_silently=False) | ||
| done=True | ||
| HttpResponseRedirect("{% url 'polls.signupcomp' %}") |
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.
Add a return before the function. Also, this URL syntax doesn't work. Use HttpResponseRedirect("/signupcomp/") instead.
|
@dipanshu231099 Are you still working on this? Please do better PR names from now on. Putting an issue number in you PR name is not good. |
next time onwards will take care of that @vsvipul |
the present pr will open a new page to show the user a page where he will be told to go to his email address for the conformation link. However in my app addition of a new user is not working and it is always showing the error for
elif not form.is_valid