Skip to content

Commit

Permalink
Cleanup URLs and route names for reset password, forgot password and …
Browse files Browse the repository at this point in the history
…register.

https://trello.com/c/Yiraeqzs/408-logged-out-form-url-changes-and-redirects

To test:
In /: create new user
In acount/settings: changed email changed password
In stream: logged out and logged in with new password, forgot password and reset it
  • Loading branch information
Sheetal Umesh Kumar committed Aug 3, 2016
1 parent 69a0a2b commit b6a2fc5
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 52 deletions.
42 changes: 21 additions & 21 deletions h/accounts/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ def post(self):
self._send_forgot_password_email(user)

return httpexceptions.HTTPFound(
self.request.route_path('reset_password'))
self.request.route_path('account_reset'))

def _redirect_if_logged_in(self):
if self.request.authenticated_userid is not None:
Expand All @@ -229,14 +229,14 @@ def _send_forgot_password_email(self, user):
serializer = self.request.registry.password_reset_serializer
code = serializer.dumps(user.username)

link = reset_password_link(self.request, code)
message = reset_password_email(user, code, link)
link = account_reset_link(self.request, code)
message = account_reset_email(user, code, link)
mailer.send.delay(**message)


@view_defaults(route_name='reset_password',
renderer='h:templates/accounts/reset_password.html.jinja2')
class ResetPasswordController(object):
@view_defaults(route_name='account_reset',
renderer='h:templates/accounts/reset.html.jinja2')
class ResetController(object):

"""Controller for handling password reset forms."""

Expand All @@ -245,15 +245,15 @@ def __init__(self, request):
self.schema = schemas.ResetPasswordSchema().bind(request=self.request)
self.form = request.create_form(
schema=self.schema,
action=self.request.route_path('reset_password'),
action=self.request.route_path('account_reset'),
buttons=(_('Save'),))

@view_config(request_method='GET')
def get(self):
"""Render the reset password form."""
return {'form': self.form.render(), 'has_code': False}

@view_config(route_name='reset_password_with_code',
@view_config(route_name='account_reset_with_code',
request_method='GET')
def get_with_prefilled_code(self):
"""Render the reset password form with a prefilled code."""
Expand Down Expand Up @@ -310,9 +310,9 @@ def _reset_password(self, user, password):
self.request.registry.notify(PasswordResetEvent(self.request, user))


@view_defaults(route_name='register',
renderer='h:templates/accounts/register.html.jinja2')
class RegisterController(object):
@view_defaults(route_name='signup',
renderer='h:templates/accounts/signup.html.jinja2')
class SignupController(object):

def __init__(self, request):
tos_link = ('<a class="link" href="/terms-of-service">' +
Expand Down Expand Up @@ -353,7 +353,7 @@ def post(self):
except deform.ValidationFailure:
return {'form': self.form.render()}

self._register(username=appstruct['username'],
self._signup(username=appstruct['username'],
email=appstruct['email'],
password=appstruct['password'])

Expand All @@ -364,7 +364,7 @@ def _redirect_if_logged_in(self):
if self.request.authenticated_userid is not None:
raise httpexceptions.HTTPFound(self.request.route_url('stream'))

def _register(self, username, email, password):
def _signup(self, username, email, password):
user = User(username=username, email=email, password=password)
self.request.db.add(user)

Expand Down Expand Up @@ -624,7 +624,7 @@ def activation_email(request, user):
}


def reset_password_email(user, reset_code, reset_link):
def account_reset_email(user, reset_code, reset_link):
"""Return the data for a 'reset your password' email for the given user.
:rtype: dict
Expand All @@ -649,9 +649,9 @@ def reset_password_email(user, reset_code, reset_link):
}


def reset_password_link(request, reset_code):
"""Transform an activation code into a password reset link."""
return request.route_url('reset_password_with_code', code=reset_code)
def account_reset_link(request, reset_code):
"""Transform an activation code into an account reset link."""
return request.route_url('account_reset_with_code', code=reset_code)


# TODO: This can be removed after October 2016, which will be >1 year from the
Expand Down Expand Up @@ -680,11 +680,11 @@ def dismiss_sidebar_tutorial(request):
def includeme(config):
config.add_route('login', '/login')
config.add_route('logout', '/logout')
config.add_route('register', '/register')
config.add_route('signup', '/signup')
config.add_route('activate', '/activate/{id}/{code}')
config.add_route('forgot_password', '/forgot_password')
config.add_route('reset_password', '/reset_password')
config.add_route('reset_password_with_code', '/reset_password/{code}')
config.add_route('forgot_password', '/forgot-password')
config.add_route('account_reset', '/account/reset')
config.add_route('account_reset_with_code', '/account/reset/{code}')
config.add_route('account', '/account/settings')
config.add_route(
'account_notifications', '/account/settings/notifications')
Expand Down
2 changes: 1 addition & 1 deletion h/templates/accounts/forgot_password.html.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Password reset
<div class="form-vertical">
<ul class="nav nav-tabs">
<li><a href="{{ request.route_path('login') }}">Log in</a></li>{#
#}<li><a href="{{ request.route_path('register') }}">Create an account</a></li>{#
#}<li><a href="{{ request.route_path('signup') }}">Create an account</a></li>{#
#}<li class="active"><a href="{{ request.route_path('forgot_password') }}">Password reset</a></li>
</ul>
{{ form }}
Expand Down
4 changes: 2 additions & 2 deletions h/templates/accounts/login.html.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
{{ form }}
<footer class="form-footer">
Don't have a Hypothesis account?
<a class="link" href="{{ request.route_path('register') }}">Sign up</a>
<a class="link" href="{{ request.route_path('signup') }}">Sign up</a>
</footer>
</div>
{% else %}
Expand All @@ -23,7 +23,7 @@
<div class="form-vertical">
<ul class="nav nav-tabs">
<li class="active"><a href="{{ request.route_path('login') }}">Log in</a></li>{#
#}<li><a href="{{ request.route_path('register') }}">Create an account</a></li>
#}<li><a href="{{ request.route_path('signup') }}">Create an account</a></li>
</ul>
{{ form }}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Password reset
<div class="form-vertical">
<ul class="nav nav-tabs">
<li><a href="{{ request.route_path('login') }}">Log in</a></li>{#
#}<li><a href="{{ request.route_path('register') }}">Create an account</a></li>{#
#}<li><a href="{{ request.route_path('signup') }}">Create an account</a></li>{#
#}<li class="active"><a href="">New password</a></li>
</ul>
{% if not has_code %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Create account
<div class="form-vertical">
<ul class="nav nav-tabs">
<li><a href="{{ request.route_path('login') }}">Log in</a></li>{#
#}<li class="active"><a href="{{ request.route_path('register') }}">Create an account</a></li>
#}<li class="active"><a href="{{ request.route_path('signup') }}">Create an account</a></li>
</ul>
{{ form }}
</div>
Expand Down
2 changes: 1 addition & 1 deletion h/templates/home.html.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
<li><a href="{{ request.route_url("logout") }}">Log out</a></li>
{% else %}
<li><a href="{{ base_url }}login">Log in</a></li>
<li><a href="{{ base_url }}register">Create an account</a></li>
<li><a href="{{ base_url }}signup">Create an account</a></li>
{% endif %}
</ul>
</nav>
Expand Down
4 changes: 3 additions & 1 deletion h/tweens.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ def content_security_policy_tween(request):
REDIRECTS = {
'/profile': 'account',
'/profile/notifications': 'account_notifications',
'/profile/developer': 'account_developer'
'/profile/developer': 'account_developer',
'/register': 'signup',
'/forgot_password': 'forgot_password'
}


Expand Down
48 changes: 24 additions & 24 deletions tests/h/accounts/views_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ def test_post_creates_no_activations_when_validation_fails(self,

assert activation_model.call_count == 0

@mock.patch('h.accounts.views.reset_password_link')
@mock.patch('h.accounts.views.account_reset_link')
def test_post_generates_reset_link(self, reset_link, pyramid_request):
pyramid_request.registry.password_reset_serializer = FakeSerializer()
user = FakeUser(username='giraffe', email='giraffe@thezoo.org')
Expand All @@ -302,8 +302,8 @@ def test_post_generates_reset_link(self, reset_link, pyramid_request):

reset_link.assert_called_with(pyramid_request, "faketoken")

@mock.patch('h.accounts.views.reset_password_email')
@mock.patch('h.accounts.views.reset_password_link')
@mock.patch('h.accounts.views.account_reset_email')
@mock.patch('h.accounts.views.account_reset_link')
def test_post_generates_mail(self,
reset_link,
reset_mail,
Expand All @@ -324,7 +324,7 @@ def test_post_generates_mail(self,

reset_mail.assert_called_with(user, "faketoken", "http://example.com")

@mock.patch('h.accounts.views.reset_password_email')
@mock.patch('h.accounts.views.account_reset_email')
def test_post_sends_mail(self, reset_mail, mailer, pyramid_request):
pyramid_request.registry.password_reset_serializer = FakeSerializer()
user = FakeUser(username='giraffe', email='giraffe@thezoo.org')
Expand Down Expand Up @@ -361,15 +361,15 @@ def test_get_redirects_when_logged_in(self, pyramid_config, pyramid_request):
@pytest.fixture
def routes(self, pyramid_config):
pyramid_config.add_route('index', '/index')
pyramid_config.add_route('reset_password', '/reset')
pyramid_config.add_route('reset_password_with_code', '/reset-with-code')
pyramid_config.add_route('account_reset', '/account/reset')
pyramid_config.add_route('account_reset_with_code', '/account/reset-with-code')


@pytest.mark.usefixtures('routes')
class TestResetPasswordController(object):
class TestResetController(object):

def test_post_returns_form_when_validation_fails(self, pyramid_request):
controller = views.ResetPasswordController(pyramid_request)
controller = views.ResetController(pyramid_request)
controller.form = invalid_form()

result = controller.post()
Expand All @@ -378,7 +378,7 @@ def test_post_returns_form_when_validation_fails(self, pyramid_request):

def test_post_sets_user_password_from_form(self, pyramid_request):
elephant = FakeUser(password='password1')
controller = views.ResetPasswordController(pyramid_request)
controller = views.ResetController(pyramid_request)
controller.form = form_validating_to({'user': elephant,
'password': 's3cure!'})

Expand All @@ -389,7 +389,7 @@ def test_post_sets_user_password_from_form(self, pyramid_request):
@mock.patch('h.accounts.views.PasswordResetEvent', autospec=True)
def test_post_emits_event(self, event, notify, pyramid_request):
user = FakeUser(password='password1')
controller = views.ResetPasswordController(pyramid_request)
controller = views.ResetController(pyramid_request)
controller.form = form_validating_to({'user': user,
'password': 's3cure!'})

Expand All @@ -400,7 +400,7 @@ def test_post_emits_event(self, event, notify, pyramid_request):

def test_post_redirects_on_success(self, pyramid_request):
user = FakeUser(password='password1')
controller = views.ResetPasswordController(pyramid_request)
controller = views.ResetController(pyramid_request)
controller.form = form_validating_to({'user': user,
'password': 's3cure!'})

Expand All @@ -412,8 +412,8 @@ def test_post_redirects_on_success(self, pyramid_request):
def routes(self, pyramid_config):
pyramid_config.add_route('index', '/index')
pyramid_config.add_route('login', '/login')
pyramid_config.add_route('reset_password', '/reset')
pyramid_config.add_route('reset_password_with_code', '/reset-with-code')
pyramid_config.add_route('account_reset', '/reset')
pyramid_config.add_route('account_reset_with_code', '/reset-with-code')


@pytest.mark.usefixtures('activation_model',
Expand All @@ -422,18 +422,18 @@ def routes(self, pyramid_config):
'notify',
'routes',
'user_model')
class TestRegisterController(object):
class TestSignupController(object):

def test_post_returns_errors_when_validation_fails(self, pyramid_request):
controller = views.RegisterController(pyramid_request)
controller = views.SignupController(pyramid_request)
controller.form = invalid_form()

result = controller.post()

assert result == {"form": "invalid form"}

def test_post_creates_user_from_form_data(self, pyramid_request, user_model):
controller = views.RegisterController(pyramid_request)
controller = views.SignupController(pyramid_request)
controller.form = form_validating_to({
"username": "bob",
"email": "bob@example.com",
Expand All @@ -448,7 +448,7 @@ def test_post_creates_user_from_form_data(self, pyramid_request, user_model):
password="s3crets")

def test_post_adds_new_user_to_session(self, pyramid_request, user_model):
controller = views.RegisterController(pyramid_request)
controller = views.SignupController(pyramid_request)
controller.form = form_validating_to({
"username": "bob",
"email": "bob@example.com",
Expand All @@ -460,7 +460,7 @@ def test_post_adds_new_user_to_session(self, pyramid_request, user_model):
assert user_model.return_value in pyramid_request.db.added

def test_post_creates_new_activation(self, activation_model, pyramid_request, user_model):
controller = views.RegisterController(pyramid_request)
controller = views.SignupController(pyramid_request)
controller.form = form_validating_to({
"username": "bob",
"email": "bob@example.com",
Expand All @@ -477,7 +477,7 @@ def test_post_generates_activation_email_from_user(self,
activation_email,
pyramid_request,
user_model):
controller = views.RegisterController(pyramid_request)
controller = views.SignupController(pyramid_request)
controller.form = form_validating_to({
"username": "bob",
"email": "bob@example.com",
Expand All @@ -496,7 +496,7 @@ def test_post_generates_activation_email_from_user(self,

@mock.patch('h.accounts.views.activation_email')
def test_post_sends_email(self, activation_email, mailer, pyramid_request):
controller = views.RegisterController(pyramid_request)
controller = views.SignupController(pyramid_request)
controller.form = form_validating_to({
"username": "bob",
"email": "bob@example.com",
Expand All @@ -516,7 +516,7 @@ def test_post_sends_email(self, activation_email, mailer, pyramid_request):

@mock.patch('h.accounts.views.RegistrationEvent')
def test_post_no_event_when_validation_fails(self, event, notify, pyramid_request):
controller = views.RegisterController(pyramid_request)
controller = views.SignupController(pyramid_request)
controller.form = invalid_form()

controller.post()
Expand All @@ -530,7 +530,7 @@ def test_post_event_when_validation_succeeds(self,
notify,
pyramid_request,
user_model):
controller = views.RegisterController(pyramid_request)
controller = views.SignupController(pyramid_request)
controller.form = form_validating_to({
"username": "bob",
"email": "bob@example.com",
Expand All @@ -544,7 +544,7 @@ def test_post_event_when_validation_succeeds(self,
notify.assert_called_with(event.return_value)

def test_post_event_redirects_on_success(self, pyramid_request):
controller = views.RegisterController(pyramid_request)
controller = views.SignupController(pyramid_request)
controller.form = form_validating_to({
"username": "bob",
"email": "bob@example.com",
Expand All @@ -557,7 +557,7 @@ def test_post_event_redirects_on_success(self, pyramid_request):

def test_get_redirects_when_logged_in(self, pyramid_config, pyramid_request):
pyramid_config.testing_securitypolicy("acct:jane@doe.org")
controller = views.RegisterController(pyramid_request)
controller = views.SignupController(pyramid_request)

with pytest.raises(httpexceptions.HTTPRedirection):
controller.get()
Expand Down

0 comments on commit b6a2fc5

Please sign in to comment.