Skip to content

Commit

Permalink
Add errors related to "leave team" feature (#2442)
Browse files Browse the repository at this point in the history
This PR:
- adds a method on the service object get_users_with_permission - this returns a list of active users who belong to the service who have the permission you pass in as an argument
- adds 2 new error responses that are returned when you try to delete a user. SERVICE_CANNOT_HAVE_LT_2_MEMBERS and SERVICE_NEEDS_USER_W_MANAGE_SETTINGS_PERM. This is to support the "leave team" feature in notification admin
  • Loading branch information
smcmurtry authored Feb 12, 2025
1 parent 124e04e commit 75cda65
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 8 deletions.
9 changes: 9 additions & 0 deletions app/dao/permissions_dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,14 @@ def get_permissions_by_user_id_and_service_id(self, user_id, service_id):
self.Meta.model.query.filter_by(user_id=user_id).join(Permission.service).filter_by(active=True, id=service_id).all()
)

def get_team_members_with_permission(self, service_id, permission):
permission_objs = (
self.Meta.model.query.filter_by(service_id=service_id, permission=permission)
.join(Permission.user)
.filter_by(state="active")
.all()
)
return [p.user for p in permission_objs]


permission_dao = PermissionDAO()
2 changes: 1 addition & 1 deletion app/dao/users_dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def verify_within_time(user, age=timedelta(seconds=30)):
return query.count()


def get_user_by_id(user_id=None):
def get_user_by_id(user_id=None) -> User:
if user_id:
return User.query.filter_by(id=user_id).one()
return User.query.filter_by().all()
Expand Down
16 changes: 16 additions & 0 deletions app/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,23 @@ def __str__(self):
return str(self.to_dict())


class CannotRemoveUserError(InvalidRequest):
message = "Cannot remove user from team"

def __init__(self, fields=[], message=None, status_code=400):
# Call parent class __init__ with message and status_code
super().__init__(message=message if message else self.message, status_code=status_code)
self.fields = fields


def register_errors(blueprint):
@blueprint.errorhandler(CannotRemoveUserError)
def cannot_remove_user_error(error):
response = jsonify(error.to_dict())
response.status_code = error.status_code
current_app.logger.info(error.message)
return response

@blueprint.errorhandler(InvalidEmailError)
def invalid_format(error):
# Please not that InvalidEmailError is re-raised for InvalidEmail or InvalidPhone,
Expand Down
7 changes: 7 additions & 0 deletions app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,13 @@ def serialize_for_org_dashboard(self) -> dict:
"research_mode": self.research_mode,
}

def get_users_with_permission(self, permission):
from app.dao.permissions_dao import permission_dao

if permission:
return permission_dao.get_team_members_with_permission(self.id, permission)
return []


class AnnualBilling(BaseModel):
__tablename__ = "annual_billing"
Expand Down
15 changes: 13 additions & 2 deletions app/service/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,12 @@
)
from app.dao.templates_dao import dao_get_template_by_id
from app.dao.users_dao import get_user_by_id
from app.errors import InvalidRequest, register_errors
from app.errors import CannotRemoveUserError, InvalidRequest, register_errors
from app.models import (
EMAIL_TYPE,
KEY_TYPE_NORMAL,
LETTER_TYPE,
MANAGE_SETTINGS,
NOTIFICATION_CANCELLED,
SMS_TYPE,
EmailBranding,
Expand Down Expand Up @@ -490,13 +491,23 @@ def add_user_to_service(service_id, user_id):
def remove_user_from_service(service_id, user_id):
service = dao_fetch_service_by_id(service_id)
user = get_user_by_id(user_id=user_id)
users_with_manage_settings_perm = service.get_users_with_permission(MANAGE_SETTINGS)

if user not in service.users:
error = "User not found"
raise InvalidRequest(error, status_code=404)

elif len(service.users) == 1:
error = "You cannot remove the only user for a service"
raise InvalidRequest(error, status_code=400)
raise CannotRemoveUserError(message=error)

elif len(service.users) == 2:
error = "SERVICE_CANNOT_HAVE_LT_2_MEMBERS"
raise CannotRemoveUserError(message=error)

elif user in users_with_manage_settings_perm and len(users_with_manage_settings_perm) <= 1:
error = "SERVICE_NEEDS_USER_W_MANAGE_SETTINGS_PERM"
raise CannotRemoveUserError(message=error)

dao_remove_user_from_service(service, user)

Expand Down
15 changes: 15 additions & 0 deletions tests/app/dao/test_permissions_dao.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from app.dao.permissions_dao import permission_dao
from app.models import MANAGE_SETTINGS
from tests.app.conftest import create_sample_service


Expand Down Expand Up @@ -27,3 +28,17 @@ def test_get_permissions_by_user_id_returns_only_active_service(notify_db, notif
assert len(permissions) == 8
assert active_service in [i.service for i in permissions]
assert inactive_service not in [i.service for i in permissions]


def test_get_team_members_with_permission(notify_db, notify_db_session, sample_user):
active_service = create_sample_service(notify_db, notify_db_session, service_name="Active service", user=sample_user)

users_w_permission_1 = permission_dao.get_team_members_with_permission(active_service.id, MANAGE_SETTINGS)
assert users_w_permission_1 == [sample_user]

permission_dao.remove_user_service_permissions(user=sample_user, service=active_service)
users_w_permission_2 = permission_dao.get_team_members_with_permission(active_service.id, MANAGE_SETTINGS)
assert users_w_permission_2 == []

users_w_permission_3 = permission_dao.get_team_members_with_permission(active_service.id, None)
assert users_w_permission_3 == []
53 changes: 48 additions & 5 deletions tests/app/service/test_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1482,18 +1482,42 @@ def test_add_unknown_user_to_service_returns404(notify_api, notify_db, notify_db

def test_remove_user_from_service(notify_db, notify_db_session, client, sample_user_service_permission, mocker):
mocked_salesforce_client = mocker.patch("app.service.rest.salesforce_client")
second_user = create_user(email="[email protected]")
third_user = create_user(email="[email protected]")
fourth_user = create_user(email="[email protected]")
# Simulates successfully adding a user to the service
second_permission = create_sample_user_service_permission(notify_db, notify_db_session, user=second_user)
third_permission = create_sample_user_service_permission(notify_db, notify_db_session, user=third_user)
create_sample_user_service_permission(notify_db, notify_db_session, user=fourth_user)
endpoint = url_for(
"service.remove_user_from_service",
service_id=str(second_permission.service.id),
user_id=str(second_permission.user.id),
service_id=str(third_permission.service.id),
user_id=str(third_permission.user.id),
)
auth_header = create_authorization_header()
resp = client.delete(endpoint, headers=[("Content-Type", "application/json"), auth_header])
assert resp.status_code == 204
mocked_salesforce_client.engagement_delete_contact_role.assert_called_with(second_permission.service, second_permission.user)
mocked_salesforce_client.engagement_delete_contact_role.assert_called_with(third_permission.service, third_permission.user)


def test_remove_user_from_service_only_user_with_manage_perm(
notify_api, notify_db, notify_db_session, client, sample_user_service_permission, mocker
):
with notify_api.test_request_context():
with notify_api.test_client() as client:
manage_settings_user = sample_user_service_permission.user
second_user = create_user(email="[email protected]")
third_user = create_user(email="[email protected]")
create_sample_user_service_permission(notify_db, notify_db_session, user=second_user, permission="view_activity")
create_sample_user_service_permission(notify_db, notify_db_session, user=third_user, permission="view_activity")
endpoint = url_for(
"service.remove_user_from_service",
service_id=str(sample_user_service_permission.service.id),
user_id=str(manage_settings_user.id),
)
auth_header = create_authorization_header()
resp = client.delete(endpoint, headers=[("Content-Type", "application/json"), auth_header])
assert resp.status_code == 400
result = resp.json
assert result["message"] == "SERVICE_NEEDS_USER_W_MANAGE_SETTINGS_PERM"


def test_remove_non_existant_user_from_service(client, sample_user_service_permission):
Expand Down Expand Up @@ -1523,6 +1547,25 @@ def test_cannot_remove_only_user_from_service(notify_api, notify_db, notify_db_s
assert result["message"] == "You cannot remove the only user for a service"


def test_remove_user_from_service_with_2_users(
notify_api, notify_db, notify_db_session, client, sample_user_service_permission, mocker
):
with notify_api.test_request_context():
with notify_api.test_client() as client:
second_user = create_user(email="[email protected]")
create_sample_user_service_permission(notify_db, notify_db_session, user=second_user, permission="view_activity")
endpoint = url_for(
"service.remove_user_from_service",
service_id=str(sample_user_service_permission.service.id),
user_id=str(sample_user_service_permission.user.id),
)
auth_header = create_authorization_header()
resp = client.delete(endpoint, headers=[("Content-Type", "application/json"), auth_header])
assert resp.status_code == 400
result = resp.json
assert result["message"] == "SERVICE_CANNOT_HAVE_LT_2_MEMBERS"


# This test is just here verify get_service_and_api_key_history that is a temp solution
# until proper ui is sorted out on admin app
def test_get_service_and_api_key_history(notify_api, notify_db, notify_db_session, sample_service):
Expand Down

0 comments on commit 75cda65

Please sign in to comment.