From bfa6980913bd77ae21247f24da0685d83ce3184e Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 9 Nov 2017 14:57:01 +0000 Subject: [PATCH] Revert "replace user PUT with POSTs" --- app/main/views/code_not_received.py | 3 ++- app/notify_client/user_api_client.py | 10 +++++++--- tests/app/main/views/test_accept_invite.py | 2 +- .../app/main/views/test_code_not_received.py | 5 +++-- tests/app/main/views/test_two_factor.py | 4 ++-- tests/app/main/views/test_verify.py | 6 +++--- tests/conftest.py | 19 +++++++++---------- 7 files changed, 27 insertions(+), 22 deletions(-) diff --git a/app/main/views/code_not_received.py b/app/main/views/code_not_received.py index 2bef44abb..cebb73b7e 100644 --- a/app/main/views/code_not_received.py +++ b/app/main/views/code_not_received.py @@ -31,7 +31,8 @@ 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 = user_api_client.update_user_attribute(user.id, mobile_number=form.mobile_number.data) + user.mobile_number = form.mobile_number.data + user_api_client.update_user(user) return redirect(url_for('.verify')) return render_template('views/text-not-received.html', form=form) diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 0df41e045..3e3170689 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -54,6 +54,12 @@ 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 @@ -149,9 +155,7 @@ class UserApiClient(NotifyAdminAPIClient): def activate_user(self, user): if user.state == 'pending': user.state = 'active' - url = "/user/{}".format(user.id) - user_data = self.post(url, data={'state': 'active'}) - return User(user_data['data'], max_failed_login_count=self.max_failed_login_count) + return self.update_user(user) else: return user diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 913ff09d6..fd1f28924 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/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_attribute, + mock_update_user, mock_add_user_to_service, mock_accept_invite, mock_get_service, diff --git a/tests/app/main/views/test_code_not_received.py b/tests/app/main/views/test_code_not_received.py index ad977022d..6f7ad700e 100644 --- a/tests/app/main/views/test_code_not_received.py +++ b/tests/app/main/views/test_code_not_received.py @@ -79,10 +79,11 @@ def test_should_resend_verify_code_and_update_mobile_for_pending_user( client, mocker, api_user_pending, - mock_update_user_attribute, + mock_update_user, 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: @@ -94,7 +95,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_attribute.assert_called_once_with(api_user_pending.id, mobile_number=phone_number_to_register_with) + mock_update_user.assert_called_once_with(api_user_pending) mock_send_verify_code.assert_called_once_with(api_user_pending.id, 'sms', to=phone_number_to_register_with) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index 5d7223709..f325fa8fc 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/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_activate_user, + mock_update_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_activate_user.called + assert mock_update_user.called assert api_user_pending.is_active diff --git a/tests/app/main/views/test_verify.py b/tests/app/main/views/test_verify.py index 84add7de1..5cda6794f 100644 --- a/tests/app/main/views/test_verify.py +++ b/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_attribute, + mock_update_user, 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_activate_user, + mock_update_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_activate_user.called + assert mock_update_user.called def test_should_return_200_when_sms_code_is_wrong( diff --git a/tests/conftest.py b/tests/conftest.py index 82e840cce..e1341994e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1413,6 +1413,14 @@ 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): @@ -1429,15 +1437,6 @@ 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) @@ -1491,7 +1490,7 @@ def mock_get_no_api_keys(mocker): @pytest.fixture(scope='function') -def mock_login(mocker, mock_get_user, mock_update_user_attribute, mock_events): +def mock_login(mocker, mock_get_user, mock_update_user, mock_events): def _verify_code(user_id, code, code_type): return True, ''