mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-06-05 22:10:44 -04:00
Catch errors when user register from invite
API gives an error if it tries to add a user to a service and that user is already a member of the service. This situation shouldn't occur - admin checks if an invited user is a member of a service before calling API, but we have seen this error occurring when there are two requests processing at the same time. This change catches the errors from API if a user is already a member of a service and redirects the user to the service dashboard so that they don't see an error page.
This commit is contained in:
@@ -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(
|
||||
service_id,
|
||||
self.id,
|
||||
permissions,
|
||||
folder_permissions,
|
||||
)
|
||||
try:
|
||||
user_api_client.add_user_to_service(
|
||||
service_id,
|
||||
self.id,
|
||||
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(
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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')
|
||||
|
||||
Reference in New Issue
Block a user