Merge pull request #1632 from alphagov/user-activation-stuff-for-real

User activation stuff for real

app/main/views/code_not_received.py

@@ -31,8 +31,7 @@ def check_and_resend_text_code():
     form = TextNotReceivedForm(mobile_number=user.mobile_number)
     if form.validate_on_submit():
         user_api_client.send_verify_code(user.id, 'sms', to=form.mobile_number.data)
-        user.mobile_number = form.mobile_number.data
-        user_api_client.update_user(user)
+        user = user_api_client.update_user_attribute(user.id, mobile_number=form.mobile_number.data)
         return redirect(url_for('.verify'))
 
     return render_template('views/text-not-received.html', form=form)

app/notify_client/user_api_client.py

@@ -54,12 +54,6 @@ class UserApiClient(NotifyAdminAPIClient):
             users.append(User(user, max_failed_login_count=self.max_failed_login_count))
         return users
 
-    def update_user(self, user):
-        data = user.serialize()
-        url = "/user/{}".format(user.id)
-        user_data = self.put(url, data=data)
-        return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
-
     def update_user_attribute(self, user_id, **kwargs):
         data = dict(kwargs)
         disallowed_attributes = set(data.keys()) - ALLOWED_ATTRIBUTES
@@ -154,8 +148,9 @@ class UserApiClient(NotifyAdminAPIClient):
 
     def activate_user(self, user):
         if user.state == 'pending':
-            user.state = 'active'
-            return self.update_user(user)
+            url = "/user/{}/activate".format(user.id)
+            user_data = self.post(url, data=None)
+            return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
         else:
             return user
 

tests/app/main/views/test_accept_invite.py

@@ -324,7 +324,7 @@ def test_new_invited_user_verifies_and_added_to_service(
     mock_send_verify_code,
     mock_check_verify_code,
     mock_get_user,
-    mock_update_user,
+    mock_update_user_attribute,
     mock_add_user_to_service,
     mock_accept_invite,
     mock_get_service,

tests/app/main/views/test_code_not_received.py

@@ -79,11 +79,10 @@ def test_should_resend_verify_code_and_update_mobile_for_pending_user(
     client,
     mocker,
     api_user_pending,
-    mock_update_user,
+    mock_update_user_attribute,
     mock_send_verify_code,
     phone_number_to_register_with,
 ):
-
     mocker.patch('app.user_api_client.get_user_by_email', return_value=api_user_pending)
 
     with client.session_transaction() as session:
@@ -95,7 +94,7 @@ def test_should_resend_verify_code_and_update_mobile_for_pending_user(
     assert response.status_code == 302
     assert response.location == url_for('main.verify', _external=True)
 
-    mock_update_user.assert_called_once_with(api_user_pending)
+    mock_update_user_attribute.assert_called_once_with(api_user_pending.id, mobile_number=phone_number_to_register_with)
     mock_send_verify_code.assert_called_once_with(api_user_pending.id, 'sms', to=phone_number_to_register_with)
 
 

tests/app/main/views/test_two_factor.py

@@ -200,7 +200,7 @@ def test_two_factor_should_activate_pending_user(
     mocker,
     api_user_pending,
     mock_check_verify_code,
-    mock_update_user,
+    mock_activate_user,
 ):
     mocker.patch('app.user_api_client.get_user', return_value=api_user_pending)
     mocker.patch('app.service_api_client.get_services', return_value={'data': []})
@@ -211,5 +211,5 @@ def test_two_factor_should_activate_pending_user(
         }
     client.post(url_for('main.two_factor'), data={'sms_code': '12345'})
 
-    assert mock_update_user.called
+    assert mock_activate_user.called
     assert api_user_pending.is_active

tests/app/main/views/test_verify.py

@@ -30,7 +30,7 @@ def test_should_redirect_to_add_service_when_sms_code_is_correct(
     client,
     api_user_active,
     mocker,
-    mock_update_user,
+    mock_update_user_attribute,
     mock_check_verify_code,
     fake_uuid
 ):
@@ -60,14 +60,14 @@ def test_should_activate_user_after_verify(
     api_user_pending,
     mock_send_verify_code,
     mock_check_verify_code,
-    mock_update_user,
+    mock_activate_user,
 ):
     mocker.patch('app.user_api_client.get_user', return_value=api_user_pending)
     with client.session_transaction() as session:
         session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id}
     client.post(url_for('main.verify'),
                 data={'sms_code': '12345'})
-    assert mock_update_user.called
+    assert mock_activate_user.called
 
 
 def test_should_return_200_when_sms_code_is_wrong(

tests/app/notify_client/test_user_client.py

@@ -33,3 +33,22 @@ def test_client_updates_password_separately(mocker, api_user_active):
 
     client.update_password(api_user_active.id, expected_params['_password'])
     mock_update_password.assert_called_once_with(expected_url, data=expected_params)
+
+
+def test_client_activates_if_pending(mocker, api_user_pending):
+    mock_post = mocker.patch('app.notify_client.user_api_client.UserApiClient.post')
+    client = UserApiClient()
+    client.max_failed_login_count = 1  # doesn't matter for this test
+
+    client.activate_user(api_user_pending)
+
+    mock_post.assert_called_once_with('/user/{}/activate'.format(api_user_pending.id), data=None)
+
+
+def test_client_doesnt_activate_if_already_active(mocker, api_user_active):
+    mock_post = mocker.patch('app.notify_client.user_api_client.UserApiClient.post')
+    client = UserApiClient()
+
+    client.activate_user(api_user_active)
+
+    assert not mock_post.called

tests/conftest.py

@@ -1413,14 +1413,6 @@ def mock_verify_password(mocker):
         side_effect=_verify_password)
 
 
-@pytest.fixture(scope='function')
-def mock_update_user(mocker, api_user_active):
-    def _update(user_id, **kwargs):
-        return api_user_active
-
-    return mocker.patch('app.user_api_client.update_user', side_effect=_update)
-
-
 @pytest.fixture(scope='function')
 def mock_update_user_password(mocker, api_user_active):
     def _update(user_id, **kwargs):
@@ -1437,6 +1429,15 @@ def mock_update_user_attribute(mocker, api_user_active):
     return mocker.patch('app.user_api_client.update_user_attribute', side_effect=_update)
 
 
+@pytest.fixture
+def mock_activate_user(mocker):
+    def _activate(user):
+        user.state = 'active'
+        return user
+
+    return mocker.patch('app.user_api_client.activate_user', side_effect=_activate)
+
+
 @pytest.fixture(scope='function')
 def mock_is_email_unique(mocker):
     return mocker.patch('app.user_api_client.is_email_unique', return_value=True)
@@ -1490,7 +1491,7 @@ def mock_get_no_api_keys(mocker):
 
 
 @pytest.fixture(scope='function')
-def mock_login(mocker, mock_get_user, mock_update_user, mock_events):
+def mock_login(mocker, mock_get_user, mock_update_user_attribute, mock_events):
     def _verify_code(user_id, code, code_type):
         return True, ''