Skip to content

Commit

Permalink
chore: apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Andrea Williams <[email protected]>
  • Loading branch information
BCerki and andrea-williams committed Jan 22, 2025
1 parent c88d702 commit 63a770f
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,17 @@
)
from registration.tests.utils.helpers import CommonTestSetup, TestUtils
from registration.utils import custom_reverse_lazy
from registration.enums.enums import AccessRequestStates, AccessRequestTypes


class TestUpdateUserOperatorStatusEndpoint(CommonTestSetup):
def test_industry_user_approves_access_request(self, mocker):
operator = operator_baker({"status": Operator.Statuses.APPROVED, "is_new": False})
TestUtils.authorize_current_user_as_operator_user(self, operator=operator)
subsequent_user_operator = baker.make(UserOperator, operator=operator)
# mock_send_operator_access_request_email = mocker.patch(
# "service.user_operator_service.send_operator_access_request_email"
# )
mock_send_operator_access_request_email = mocker.patch(
"service.user_operator_service_v2.send_operator_access_request_email"
)
response = TestUtils.mock_put_with_auth_role(
self,
"industry_user",
Expand All @@ -36,13 +37,13 @@ def test_industry_user_approves_access_request(self, mocker):
),
)
# # Assert that the email notification was called
# mock_send_operator_access_request_email.assert_called_once_with(
# AccessRequestStates.APPROVED,
# AccessRequestTypes.OPERATOR_WITH_ADMIN,
# operator.legal_name,
# subsequent_user_operator.user.get_full_name(),
# subsequent_user_operator.user.email,
# )
mock_send_operator_access_request_email.assert_called_once_with(
AccessRequestStates.APPROVED,
AccessRequestTypes.OPERATOR_WITH_ADMIN,
operator.legal_name,
subsequent_user_operator.user.get_full_name(),
subsequent_user_operator.user.email,
)
assert response.status_code == 200

def test_industry_user_cannot_approve_access_request_from_a_different_operator(
Expand Down Expand Up @@ -78,9 +79,9 @@ def test_cas_analyst_approves_access_request_with_existing_operator(self, mocker

pending_user_operator.user.business_guid = approved_admin_user_operator.user.business_guid

# mock_send_operator_access_request_email = mocker.patch(
# "service.user_operator_service.send_operator_access_request_email"
# )
mock_send_operator_access_request_email = mocker.patch(
"service.user_operator_service_v2.send_operator_access_request_email"
)
response_2 = TestUtils.mock_put_with_auth_role(
self,
"cas_analyst",
Expand All @@ -102,22 +103,22 @@ def test_cas_analyst_approves_access_request_with_existing_operator(self, mocker
assert pending_user_operator.role == UserOperator.Roles.ADMIN
assert pending_user_operator.verified_by == self.user
# Assert that the email notification was called
# mock_send_operator_access_request_email.assert_called_once_with(
# AccessRequestStates.APPROVED,
# AccessRequestTypes.ADMIN,
# operator.legal_name,
# pending_user_operator.user.get_full_name(),
# pending_user_operator.user.email,
# )
mock_send_operator_access_request_email.assert_called_once_with(
AccessRequestStates.APPROVED,
AccessRequestTypes.ADMIN,
approved_admin_user_operator.operator.legal_name,
pending_user_operator.user.get_full_name(),
pending_user_operator.user.email,
)

def test_cas_director_approves_admin_access_request_with_new_operator(self, mocker):
# In this test we are testing the user operator status change and not the operator change,
# so we have to mark the operator as is_new=False and status=APPROVED so we can bypass the below part and can get to the email sending part
operator = operator_baker({'status': Operator.Statuses.APPROVED, 'is_new': False, 'created_by': self.user})
user_operator = user_operator_baker({'operator': operator, 'user': operator.created_by})
# mock_send_operator_access_request_email = mocker.patch(
# "service.user_operator_service.send_operator_access_request_email"
# )
mock_send_operator_access_request_email = mocker.patch(
"service.user_operator_service_v2.send_operator_access_request_email"
)
response_2 = TestUtils.mock_put_with_auth_role(
self,
'cas_director',
Expand All @@ -139,13 +140,13 @@ def test_cas_director_approves_admin_access_request_with_new_operator(self, mock
assert user_operator.role == UserOperator.Roles.ADMIN
assert user_operator.verified_by == self.user
# Assert that the email notification was called (user associated with the user_operator IS the creator of the operator)
# mock_send_operator_access_request_email.assert_called_once_with(
# AccessRequestStates.APPROVED,
# AccessRequestTypes.NEW_OPERATOR_AND_ADMIN,
# operator.legal_name,
# user_operator.user.get_full_name(),
# user_operator.user.email,
# )
mock_send_operator_access_request_email.assert_called_once_with(
AccessRequestStates.APPROVED,
AccessRequestTypes.NEW_OPERATOR_AND_ADMIN,
operator.legal_name,
user_operator.user.get_full_name(),
user_operator.user.email,
)

def test_cas_analyst_declines_access_request(self, mocker):
user = baker.make(User)
Expand All @@ -158,9 +159,9 @@ def test_cas_analyst_declines_access_request(self, mocker):
user_operator.operator = operator
user_operator.save(update_fields=["user_id", "operator_id"])

# mock_send_operator_access_request_email = mocker.patch(
# "service.user_operator_service.send_operator_access_request_email"
# )
mock_send_operator_access_request_email = mocker.patch(
"service.user_operator_service_v2.send_operator_access_request_email"
)
# Now decline the user_operator and make sure the contacts are deleted
response = TestUtils.mock_put_with_auth_role(
self,
Expand All @@ -182,10 +183,10 @@ def test_cas_analyst_declines_access_request(self, mocker):
assert user_operator.role == UserOperator.Roles.PENDING
assert user_operator.verified_by == self.user
# Assert that the email notification was sent
# mock_send_operator_access_request_email.assert_called_once_with(
# AccessRequestStates.DECLINED,
# AccessRequestTypes.ADMIN,
# operator.legal_name,
# user_operator.user.get_full_name(),
# user_operator.user.email,
# )
mock_send_operator_access_request_email.assert_called_once_with(
AccessRequestStates.DECLINED,
AccessRequestTypes.ADMIN,
operator.legal_name,
user_operator.user.get_full_name(),
user_operator.user.email,
)
8 changes: 3 additions & 5 deletions bc_obps/service/user_operator_service_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@
from service.data_access_service.operator_service import OperatorDataAccessService
from registration.models import Operator, User, UserOperator
from django.db import transaction
from zoneinfo import ZoneInfo
from datetime import datetime
from registration.constants import UNAUTHORIZED_MESSAGE
from service.operator_service_v2 import OperatorServiceV2
from registration.schema.v2.user_operator import UserOperatorFilterSchema
from django.db.models import QuerySet
from django.db.models.functions import Lower
from ninja import Query
from registration.utils import set_verification_columns


class UserOperatorServiceV2:
Expand Down Expand Up @@ -121,7 +120,7 @@ def list_user_operators_v2(
def update_status_and_create_contact(
cls, user_operator_id: UUID, payload: UserOperatorStatusUpdate, admin_user_guid: UUID
) -> UserOperator:
"Function to update the user_operator status. If they are being approved, we create a Contact record for them."
"""Function to update the user_operator status. If they are being approved, we create a Contact record for them."""
admin_user: User = UserDataAccessService.get_by_guid(admin_user_guid)
user_operator: UserOperator = UserOperatorDataAccessService.get_user_operator_by_id(user_operator_id)

Expand All @@ -141,8 +140,7 @@ def update_status_and_create_contact(
updated_role = payload.role

if user_operator.status in [UserOperator.Statuses.APPROVED, UserOperator.Statuses.DECLINED]:
user_operator.verified_at = datetime.now(ZoneInfo("UTC"))
user_operator.verified_by_id = admin_user_guid
set_verification_columns(user_operator, admin_user_guid)

if user_operator.status == UserOperator.Statuses.DECLINED:
# Set role to pending for now but we may want to add a new role for declined
Expand Down

0 comments on commit 63a770f

Please sign in to comment.