Skip to content

Issue#17#18

Open
dipanshu231099 wants to merge 4 commits intoKamandPrompt:masterfrom
dipanshu231099:issue#17
Open

Issue#17#18
dipanshu231099 wants to merge 4 commits intoKamandPrompt:masterfrom
dipanshu231099:issue#17

Conversation

@dipanshu231099
Copy link
Copy Markdown
Collaborator

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

@kpbot kpbot added the size: L label Jul 6, 2019
@Varunvaruns9
Copy link
Copy Markdown
Member

This isn't the solution for #17 although it is a good addition. The original issue talks about the page you visit when you click the link to activate your account from the email. Here is the view that renders it:

def activate(request, uidb64, token):

Copy link
Copy Markdown
Member

@Varunvaruns9 Varunvaruns9 left a 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.

Comment thread carpool/views.py Outdated
if request.user.is_authenticated:
return HttpResponseRedirect('/')
if request.method == 'POST':
elif request.method == 'POST':
Copy link
Copy Markdown
Member

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.

Comment thread carpool/views.py
# email.send()
else:
form = SignUpForm()
return render(request, 'signup.html', {'form': form, 'error': error, 'done': done, })
Copy link
Copy Markdown
Member

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.

Comment thread carpool/views.py Outdated
to_email = form.cleaned_data.get('email')
send_mail(mail_subject, message, 'Hackweek@example.com', [to_email], fail_silently=False)
done=True
HttpResponseRedirect("{% url 'polls.signupcomp' %}")
Copy link
Copy Markdown
Member

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.

@vsvipul
Copy link
Copy Markdown
Member

vsvipul commented Jul 9, 2019

@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.

@dipanshu231099
Copy link
Copy Markdown
Collaborator Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants