Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion common/djangoapps/student/tests/test_activate_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from uuid import uuid4

from django.conf import settings
from django.contrib.auth.models import User
from django.test import TestCase, override_settings
from django.urls import reverse
from mock import patch
Expand Down Expand Up @@ -103,6 +104,10 @@ def test_account_activation_message(self):
response = self.client.get(reverse('dashboard'))
self.assertNotContains(response, expected_message)

def _assert_user_active_state(self, expected_active_state):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to Reviewers: I updated these tests in order to merge test functionality that was in AccountActivationAndPasswordChangeTest in openedx/core/djangoapps/user_api/accounts/tests/test_api.py.

user = User.objects.get(username=self.user.username)
self.assertEqual(user.is_active, expected_active_state)

def test_account_activation_notification_on_logistration(self):
"""
Verify that logistration page displays success/error/info messages
Expand All @@ -112,15 +117,19 @@ def test_account_activation_notification_on_logistration(self):
login_url=reverse('signin_user'),
redirect_url=reverse('dashboard'),
)
self._assert_user_active_state(expected_active_state=False)

# Access activation link, message should say that account has been activated.
response = self.client.get(reverse('activate', args=[self.registration.activation_key]), follow=True)
self.assertRedirects(response, login_page_url)
self.assertContains(response, 'Success! You have activated your account.')
self._assert_user_active_state(expected_active_state=True)

# Access activation link again, message should say that account is already active.
response = self.client.get(reverse('activate', args=[self.registration.activation_key]), follow=True)
self.assertRedirects(response, login_page_url)
self.assertContains(response, 'This account has already been activated.')
self._assert_user_active_state(expected_active_state=True)

# Open account activation page with an invalid activation link,
# there should be an error message displayed.
Expand All @@ -137,4 +146,4 @@ def test_account_activation_prevent_auth_user_writes(self):
response = self.client.get(reverse('activate', args=[self.registration.activation_key]), follow=True)
self.assertRedirects(response, login_page_url)
self.assertContains(response, SYSTEM_MAINTENANCE_MSG)
assert not self.user.is_active
self._assert_user_active_state(expected_active_state=False)
1 change: 0 additions & 1 deletion common/djangoapps/student/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
url(r'^accounts/disable_account_ajax$', views.disable_account_ajax, name="disable_account_ajax"),
url(r'^accounts/manage_user_standing', views.manage_user_standing, name='manage_user_standing'),

url(r'^change_setting$', views.change_setting, name='change_setting'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to Reviewers: I double-checked in New Relic that this API has been called 0 times since 6 weeks ago on Stage, Prod, and Edge. I believe it's a much older endpoint since user_api was created.

I'd like to move the remaining (email and activation) account-related views to user_api in a subsequent PR. Then, student will only contain functionality for the student dashboard and roles.

url(r'^change_email_settings$', views.change_email_settings, name='change_email_settings'),

url(r'^course_run/{}/refund_status$'.format(settings.COURSE_ID_PATTERN),
Expand Down
18 changes: 0 additions & 18 deletions common/djangoapps/student/views/management.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,24 +468,6 @@ def disable_account_ajax(request):
return JsonResponse(context)


@login_required
@ensure_csrf_cookie
def change_setting(request):
"""
JSON call to change a profile setting: Right now, location
"""
# TODO (vshnayder): location is no longer used
u_prof = UserProfile.objects.get(user=request.user) # request.user.profile_cache
if 'location' in request.POST:
u_prof.location = request.POST['location']
u_prof.save()

return JsonResponse({
"success": True,
"location": u_prof.location,
})


@receiver(post_save, sender=User)
def user_signup_handler(sender, **kwargs): # pylint: disable=unused-argument
"""
Expand Down
29 changes: 0 additions & 29 deletions openedx/core/djangoapps/user_api/accounts/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,35 +331,6 @@ def _send_email_change_requests_if_needed(data, user):
)


@helpers.intercept_errors(errors.UserAPIInternalError, ignore_errors=[errors.UserAPIRequestError])
def activate_account(activation_key):
"""Activate a user's account.

Args:
activation_key (unicode): The activation key the user received via email.

Returns:
None

Raises:
errors.UserNotAuthorized
errors.UserAPIInternalError: the operation failed due to an unexpected error.

"""
# TODO: Confirm this `activate_account` is only used for tests. If so, this should not be used for tests, and we
# should instead use the `activate_account` used for /activate.
set_custom_metric('user_api_activate_account', 'True')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to Reviewers: Thanks to @robrap's metric here, we've verified that activate_account is used only in tests and not in production.

if waffle().is_enabled(PREVENT_AUTH_USER_WRITES):
raise errors.UserAPIInternalError(SYSTEM_MAINTENANCE_MSG)
try:
registration = Registration.objects.get(activation_key=activation_key)
except Registration.DoesNotExist:
raise errors.UserNotAuthorized
else:
# This implicitly saves the registration
registration.activate()


def get_name_validation_error(name):
"""Get the built-in validation error message for when
the user's real name is invalid in some way (we wonder how).
Expand Down
62 changes: 0 additions & 62 deletions openedx/core/djangoapps/user_api/accounts/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from openedx.core.djangoapps.user_api.accounts import PRIVATE_VISIBILITY, USERNAME_MAX_LENGTH
from openedx.core.djangoapps.user_api.accounts.api import (
activate_account,
get_account_settings,
update_account_settings
)
Expand Down Expand Up @@ -530,64 +529,3 @@ def test_normalize_password(self):

expected_user_password = make_password(unicodedata.normalize('NFKC', u'Ṗŕệṿïệẅ Ṯệẍt'), salt_val)
self.assertEqual(expected_user_password, user.password)


@ddt.ddt
class AccountActivationAndPasswordChangeTest(CreateAccountMixin, TestCase):
"""
Test cases to cover the account initialization workflow
"""
USERNAME = u'claire-underwood'
PASSWORD = u'ṕáśśẃőŕd'
EMAIL = u'claire+underwood@example.com'

IS_SECURE = False

def get_activation_key(self, user):
registration = Registration.objects.get(user=user)
return registration.activation_key

@skip_unless_lms
def test_activate_account(self):
# Create the account, which is initially inactive
self.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
user = User.objects.get(username=self.USERNAME)
activation_key = self.get_activation_key(user)

request = RequestFactory().get("/api/user/v1/accounts/")
request.user = user
account = get_account_settings(request)[0]
self.assertEqual(self.USERNAME, account["username"])
self.assertEqual(self.EMAIL, account["email"])
self.assertFalse(account["is_active"])

# Activate the account and verify that it is now active
activate_account(activation_key)
account = get_account_settings(request)[0]
self.assertTrue(account['is_active'])

def test_activate_account_invalid_key(self):
with pytest.raises(UserNotAuthorized):
activate_account(u'invalid')

def test_activate_account_prevent_auth_user_writes(self):
self.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
user = User.objects.get(username=self.USERNAME)
activation_key = self.get_activation_key(user)

with pytest.raises(UserAPIInternalError, message=SYSTEM_MAINTENANCE_MSG):
with waffle().override(PREVENT_AUTH_USER_WRITES, True):
activate_account(activation_key)

def _assert_is_datetime(self, timestamp):
"""
Internal helper to validate the type of the provided timestamp
"""
if not timestamp:
return False
try:
parse_datetime(timestamp)
except ValueError:
return False
else:
return True
Loading