Move updating user validation out of validate_and_update_broadcast_message_status

As only 1 of 2 functions calling it needs that check, it's better
to perform it inside that 1 function.
This commit is contained in:
Pea Tyczynska
2022-01-19 16:06:20 +00:00
parent a4c20e8ba6
commit c2a389e81a
2 changed files with 11 additions and 12 deletions

View File

@@ -42,15 +42,7 @@ def _parse_nullable_datetime(dt):
return dt return dt
def validate_and_update_broadcast_message_status(broadcast_message, new_status, updating_user, from_api=False): def validate_and_update_broadcast_message_status(broadcast_message, new_status, updating_user):
if updating_user not in broadcast_message.service.users:
# we allow platform admins to cancel broadcasts, and we don't check user if request was done via API
if not from_api and not (new_status == BroadcastStatusType.CANCELLED and updating_user.platform_admin):
raise InvalidRequest(
f'User {updating_user.id} cannot update broadcast_message {broadcast_message.id} from other service',
status_code=400
)
if new_status not in BroadcastStatusType.ALLOWED_STATUS_TRANSITIONS[broadcast_message.status]: if new_status not in BroadcastStatusType.ALLOWED_STATUS_TRANSITIONS[broadcast_message.status]:
raise InvalidRequest( raise InvalidRequest(
f'Cannot move broadcast_message {broadcast_message.id} from {broadcast_message.status} to {new_status}', f'Cannot move broadcast_message {broadcast_message.id} from {broadcast_message.status} to {new_status}',
@@ -206,6 +198,14 @@ def update_broadcast_message_status(service_id, broadcast_message_id):
new_status = data['status'] new_status = data['status']
updating_user = get_user_by_id(data['created_by']) updating_user = get_user_by_id(data['created_by'])
if updating_user not in broadcast_message.service.users:
# we allow platform admins to cancel broadcasts, and we don't check user if request was done via API
if not (new_status == BroadcastStatusType.CANCELLED and updating_user.platform_admin):
raise InvalidRequest(
f'User {updating_user.id} cannot update broadcast_message {broadcast_message.id} from other service',
status_code=400
)
validate_and_update_broadcast_message_status(broadcast_message, new_status, updating_user) validate_and_update_broadcast_message_status(broadcast_message, new_status, updating_user)
return jsonify(broadcast_message.serialize()), 200 return jsonify(broadcast_message.serialize()), 200

View File

@@ -111,7 +111,7 @@ def _cancel_or_reject_broadcast(references_to_original_broadcast, service_id):
message="Broadcast message reference and service id didn't match with any existing broadcasts", message="Broadcast message reference and service id didn't match with any existing broadcasts",
status_code=404, status_code=404,
) )
# do we need to check if service is active?
if broadcast_message.status == BroadcastStatusType.PENDING_APPROVAL: if broadcast_message.status == BroadcastStatusType.PENDING_APPROVAL:
new_status = BroadcastStatusType.REJECTED new_status = BroadcastStatusType.REJECTED
else: else:
@@ -119,8 +119,7 @@ def _cancel_or_reject_broadcast(references_to_original_broadcast, service_id):
validate_and_update_broadcast_message_status( validate_and_update_broadcast_message_status(
broadcast_message, broadcast_message,
new_status, new_status,
updating_user=None, updating_user=None
from_api=True
) )
return broadcast_message return broadcast_message