Merge pull request #3446 from alphagov/fix-invite-errors

Catch errors when user register from invite

app/main/views/verify.py

@@ -93,5 +93,5 @@ def _add_invited_user_to_service(invited_user):
     invitation = InvitedUser(invited_user)
     user = User.from_id(session['user_id'])
     service_id = invited_user['service']
-    user_api_client.add_user_to_service(service_id, user.id, invitation.permissions, invitation.folder_permissions)
+    user.add_to_service(service_id, invitation.permissions, invitation.folder_permissions)
     return service_id

app/models/user.py

@@ -407,12 +407,18 @@ class User(JSONModel, UserMixin):
         session['current_session_id'] = self.current_session_id
 
     def add_to_service(self, service_id, permissions, folder_permissions):
-        user_api_client.add_user_to_service(
+        try:
-            service_id,
+            user_api_client.add_user_to_service(
-            self.id,
+                service_id,
-            permissions,
+                self.id,
-            folder_permissions,
+                permissions,
-        )
+                folder_permissions,
+            )
+        except HTTPError as exception:
+            if exception.status_code == 400 and 'already part of service' in exception.message:
+                pass
+            else:
+                raise exception
 
     def add_to_organisation(self, organisation_id):
         user_api_client.add_user_to_organisation(

tests/app/main/views/test_accept_invite.py

@@ -11,6 +11,7 @@ from tests.conftest import (
     USER_ONE_ID,
     create_active_caseworking_user,
     create_active_user_with_permissions,
+    create_api_user_active,
     normalize_spaces,
 )
 
@@ -196,6 +197,37 @@ def test_existing_user_of_service_get_redirected_to_signin(
     assert mock_accept_invite.call_count == 1
 
 
+def test_accept_invite_redirects_if_api_raises_an_error_that_they_are_already_part_of_the_service(
+    client,
+    mocker,
+    api_user_active,
+    sample_invite,
+    mock_accept_invite,
+    mock_get_service,
+    mock_get_users_by_service
+):
+    sample_invite['email_address'] = api_user_active['email_address']
+
+    # This mock needs to return a user with a different ID to the invited user so that
+    # `existing_user in Users(invited_user.service)` returns False and the right code path is tested
+    mocker.patch('app.user_api_client.get_user_by_email', return_value=create_api_user_active(with_unique_id=True))
+    mocker.patch('app.invite_api_client.check_token', return_value=sample_invite)
+
+    mocker.patch('app.user_api_client.add_user_to_service', side_effect=HTTPError(
+        response=Mock(
+            status_code=400,
+            json={
+                "result": "error",
+                "message": {f"User id: {api_user_active['id']} already part of service id: {SERVICE_ONE_ID}"}
+            },
+        ),
+        message=f"User id: {api_user_active['id']} already part of service id: {SERVICE_ONE_ID}"
+    ))
+
+    response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=False)
+    assert response.location == url_for('main.service_dashboard', service_id=SERVICE_ONE_ID, _external=True)
+
+
 def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(
     client,
     service_one,

tests/app/main/views/test_verify.py

@@ -1,10 +1,14 @@
 import json
 import uuid
+from unittest.mock import Mock
 
 from bs4 import BeautifulSoup
+from flask import session as flask_session
 from flask import url_for
 from itsdangerous import SignatureExpired
+from notifications_python_client.errors import HTTPError
 
+from app.main.views.verify import activate_user
 from tests.conftest import normalize_spaces
 
 
@@ -165,3 +169,32 @@ def test_verify_redirects_to_sign_in_if_not_logged_in(
 
     assert response.location == url_for('main.sign_in', _external=True)
     assert response.status_code == 302
+
+
+def test_activate_user_redirects_to_service_dashboard_if_user_already_belongs_to_service(
+    mocker,
+    client,
+    service_one,
+    sample_invite,
+    api_user_active,
+    mock_login,
+):
+    mocker.patch('app.user_api_client.add_user_to_service', side_effect=HTTPError(
+        response=Mock(
+            status_code=400,
+            json={
+                "result": "error",
+                "message": {f"User id: {api_user_active['id']} already part of service id: {service_one['id']}"}
+            },
+        ),
+        message=f"User id: {api_user_active['id']} already part of service id: {service_one['id']}"
+    ))
+
+    # Can't use `with client.session_transaction()...` here since activate_session is not a view function
+    flask_session['invited_user'] = sample_invite
+
+    response = activate_user(api_user_active['id'])
+
+    assert response.location == url_for('main.service_dashboard', service_id=service_one['id'])
+
+    flask_session.pop('invited_user')