Skip to content

Commit bbea207

Browse files
fix: Fixing the issue of deactivating user while seat allocation
1 parent 22ade54 commit bbea207

4 files changed

Lines changed: 211 additions & 0 deletions

File tree

enterprise_access/apps/api_client/lms_client.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,41 @@ def get_enterprise_user(self, enterprise_customer_uuid, learner_id):
607607

608608
return None
609609

610+
def get_enterprise_learner_by_email(self, enterprise_customer_uuid, learner_email):
611+
"""
612+
Return the enterprise customer user record for ``learner_email`` if the user is
613+
actively linked to ``enterprise_customer_uuid``, otherwise return ``None``.
614+
615+
Two LMS API calls are made:
616+
1. ``/api/user/v1/accounts`` to resolve the learner's LMS user ID from their email.
617+
2. ``enterprise-learner/`` to fetch the enterprise link for that user.
618+
619+
Returns ``None`` when the learner has no LMS account, is not linked to the given
620+
enterprise, or any API call fails unexpectedly.
621+
622+
Arguments:
623+
enterprise_customer_uuid (UUID): UUID of the enterprise customer.
624+
learner_email (str): Email address of the learner to check.
625+
"""
626+
try:
627+
user_accounts = self.get_lms_user_account(email=learner_email)
628+
except requests.exceptions.HTTPError:
629+
logger.exception(
630+
'get_enterprise_learner_by_email: failed to fetch LMS account for email %s',
631+
learner_email,
632+
)
633+
return None
634+
635+
if not user_accounts:
636+
return None
637+
638+
# get_lms_user_account returns a list; take the first match.
639+
lms_user_id = user_accounts[0].get('id') if isinstance(user_accounts, list) else user_accounts.get('id')
640+
if not lms_user_id:
641+
return None
642+
643+
return self.get_enterprise_user(enterprise_customer_uuid, lms_user_id)
644+
610645
def create_pending_enterprise_users(self, enterprise_customer_uuid, user_emails):
611646
"""
612647
Creates a pending enterprise user in the given ``enterprise_customer_uuid`` for each of the

enterprise_access/apps/api_client/tests/test_lms_client.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,6 +1282,89 @@ def test_get_lms_user_activation_link(
12821282
if expected_link is None:
12831283
self.assertTrue(mock_logger.error.called or mock_logger.exception.called)
12841284

1285+
@ddt.data(
1286+
# list response → returns enterprise user record
1287+
{
1288+
'mock_user_accounts': [{'id': TEST_USER_ID}],
1289+
'mock_enterprise_user': TEST_USER_RECORD,
1290+
'expected_result': TEST_USER_RECORD,
1291+
'expect_get_enterprise_user_called': True,
1292+
},
1293+
# dict response → also works (not a list)
1294+
{
1295+
'mock_user_accounts': {'id': TEST_USER_ID},
1296+
'mock_enterprise_user': TEST_USER_RECORD,
1297+
'expected_result': TEST_USER_RECORD,
1298+
'expect_get_enterprise_user_called': True,
1299+
},
1300+
# empty list → None
1301+
{
1302+
'mock_user_accounts': [],
1303+
'mock_enterprise_user': None,
1304+
'expected_result': None,
1305+
'expect_get_enterprise_user_called': False,
1306+
},
1307+
# None → None
1308+
{
1309+
'mock_user_accounts': None,
1310+
'mock_enterprise_user': None,
1311+
'expected_result': None,
1312+
'expect_get_enterprise_user_called': False,
1313+
},
1314+
# list entry missing 'id' → None
1315+
{
1316+
'mock_user_accounts': [{'email': 'someone@example.com'}],
1317+
'mock_enterprise_user': None,
1318+
'expected_result': None,
1319+
'expect_get_enterprise_user_called': False,
1320+
},
1321+
# dict missing 'id' → None
1322+
{
1323+
'mock_user_accounts': {'email': 'someone@example.com'},
1324+
'mock_enterprise_user': None,
1325+
'expected_result': None,
1326+
'expect_get_enterprise_user_called': False,
1327+
},
1328+
)
1329+
@ddt.unpack
1330+
def test_get_enterprise_learner_by_email(
1331+
self,
1332+
mock_user_accounts,
1333+
mock_enterprise_user,
1334+
expected_result,
1335+
expect_get_enterprise_user_called,
1336+
):
1337+
"""
1338+
Verify get_enterprise_learner_by_email resolves an email to an enterprise user record,
1339+
returning None for empty accounts, missing IDs, or unexpected responses.
1340+
"""
1341+
client = LmsApiClient()
1342+
learner_email = 'test@example.com'
1343+
1344+
with mock.patch.object(client, 'get_lms_user_account', return_value=mock_user_accounts) as mock_get_account:
1345+
with mock.patch.object(client, 'get_enterprise_user', return_value=mock_enterprise_user) as mock_get_enterprise:
1346+
result = client.get_enterprise_learner_by_email(str(TEST_ENTERPRISE_UUID), learner_email)
1347+
self.assertEqual(result, expected_result)
1348+
mock_get_account.assert_called_once_with(email=learner_email)
1349+
if expect_get_enterprise_user_called:
1350+
mock_get_enterprise.assert_called_once_with(str(TEST_ENTERPRISE_UUID), TEST_USER_ID)
1351+
else:
1352+
mock_get_enterprise.assert_not_called()
1353+
1354+
@mock.patch('enterprise_access.apps.api_client.lms_client.logger')
1355+
def test_get_enterprise_learner_by_email_http_error(self, mock_logger):
1356+
"""
1357+
Verify get_enterprise_learner_by_email returns None and logs when get_lms_user_account raises HTTPError.
1358+
"""
1359+
client = LmsApiClient()
1360+
1361+
with mock.patch.object(
1362+
client, 'get_lms_user_account', side_effect=requests.exceptions.HTTPError('whoops')
1363+
):
1364+
result = client.get_enterprise_learner_by_email(str(TEST_ENTERPRISE_UUID), 'test@example.com')
1365+
self.assertIsNone(result)
1366+
mock_logger.exception.assert_called_once()
1367+
12851368

12861369
class TestLmsUserApiClient(TestCase):
12871370
"""

enterprise_access/apps/content_assignments/tasks.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,24 @@ def create_pending_enterprise_learner_for_assignment_task(learner_content_assign
367367
enterprise_customer_uuid = assignment.assignment_configuration.enterprise_customer_uuid
368368

369369
lms_client = LmsApiClient()
370+
371+
# Skip pending-user creation if the learner is already actively linked to this enterprise.
372+
# Calling create_pending_enterprise_users when the user is already linked can inadvertently
373+
# trigger the LMS serializer's "inactivate other customers" logic and deactivate the user's
374+
# links to other enterprise customers.
375+
existing_link = lms_client.get_enterprise_learner_by_email(
376+
enterprise_customer_uuid, assignment.learner_email
377+
)
378+
if existing_link and existing_link.get('active', False):
379+
assignment.add_successful_linked_action()
380+
logger.info(
381+
'Learner is already actively linked to enterprise %s; '
382+
'skipping pending enterprise user creation for assignment %s',
383+
enterprise_customer_uuid,
384+
assignment.uuid,
385+
)
386+
return
387+
370388
# Could raise HTTPError and trigger task retry. Intentionally ignoring response since success should just not throw
371389
# an exception. Two possible success statuses are 201 (created) and 200 (found), but there's no reason to
372390
# distinguish them for the purpose of this task.

enterprise_access/apps/content_assignments/tests/test_tasks.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,14 @@ def setUp(self):
8787
assignment_configuration=self.assignment_configuration,
8888
)
8989

90+
# By default, the learner is NOT already actively linked. Individual tests override this.
91+
patcher = mock.patch(
92+
'enterprise_access.apps.api_client.lms_client.LmsApiClient.get_enterprise_learner_by_email',
93+
return_value=None,
94+
)
95+
self.mock_get_enterprise_learner_by_email = patcher.start()
96+
self.addCleanup(patcher.stop)
97+
9098
@ddt.data(
9199
# The LMS API did not find an existing PendingEnterpriseLearner, so it created one.
92100
{
@@ -210,6 +218,73 @@ def test_last_retry_success(self, mock_oauth_client):
210218
self.assignment.refresh_from_db()
211219
assert self.assignment.state == LearnerContentAssignmentStateChoices.ALLOCATED
212220

221+
@mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient')
222+
def test_skip_if_already_active_link(self, mock_oauth_client):
223+
"""
224+
If the learner is already actively linked to the enterprise, the task should
225+
record a successful linked action and return early without calling the
226+
pending-enterprise-learner LMS endpoint.
227+
"""
228+
self.mock_get_enterprise_learner_by_email.return_value = {
229+
'enterprise_customer': {'uuid': str(TEST_ENTERPRISE_UUID)},
230+
'user': {'email': TEST_EMAIL},
231+
'active': True,
232+
}
233+
234+
task_result = create_pending_enterprise_learner_for_assignment_task.delay(self.assignment.uuid)
235+
236+
assert task_result.state == celery_states.SUCCESS
237+
238+
# The pending-enterprise-learner POST endpoint must NOT have been called.
239+
mock_oauth_client.return_value.post.assert_not_called()
240+
241+
# The active-link check must have been called with the correct arguments.
242+
self.mock_get_enterprise_learner_by_email.assert_called_once_with(
243+
TEST_ENTERPRISE_UUID,
244+
TEST_EMAIL,
245+
)
246+
247+
# Assignment state stays allocated and a successful linked action is recorded.
248+
self.assignment.refresh_from_db()
249+
assert self.assignment.state == LearnerContentAssignmentStateChoices.ALLOCATED
250+
assert self.assignment.actions.filter(action_type=AssignmentActions.LEARNER_LINKED).exists()
251+
252+
@ddt.data(
253+
# Learner has a link but it is explicitly inactive.
254+
{'active': False},
255+
# Learner has a link record with no 'active' field at all.
256+
{},
257+
)
258+
@mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient')
259+
def test_proceeds_if_link_not_active(self, active_value, mock_oauth_client):
260+
"""
261+
If the learner's enterprise link exists but is not active, the task should
262+
proceed normally and call create_pending_enterprise_users.
263+
"""
264+
self.mock_get_enterprise_learner_by_email.return_value = {
265+
'enterprise_customer': {'uuid': str(TEST_ENTERPRISE_UUID)},
266+
'user': {'email': TEST_EMAIL},
267+
**active_value,
268+
}
269+
mock_oauth_client.return_value.post.return_value = MockResponse(
270+
{'enterprise_customer': str(TEST_ENTERPRISE_UUID), 'user_email': TEST_EMAIL},
271+
status.HTTP_201_CREATED,
272+
)
273+
274+
task_result = create_pending_enterprise_learner_for_assignment_task.delay(self.assignment.uuid)
275+
276+
assert task_result.state == celery_states.SUCCESS
277+
278+
# The pending-enterprise-learner POST endpoint must still have been called.
279+
assert len(mock_oauth_client.return_value.post.call_args_list) == 1
280+
assert mock_oauth_client.return_value.post.call_args.kwargs['json'] == [{
281+
'enterprise_customer': str(self.assignment.assignment_configuration.enterprise_customer_uuid),
282+
'user_email': self.assignment.learner_email,
283+
}]
284+
285+
self.assignment.refresh_from_db()
286+
assert self.assignment.state == LearnerContentAssignmentStateChoices.ALLOCATED
287+
213288

214289
@ddt.ddt
215290
class TestBrazeEmailTasks(APITestWithMocks):

0 commit comments

Comments
 (0)