Skip to content

Commit 48c43c2

Browse files
committed
fix tests
1 parent 9ff3d6b commit 48c43c2

File tree

2 files changed

+20
-33
lines changed

2 files changed

+20
-33
lines changed

src/ol_openedx_lti_utilities/ol_openedx_lti_utilities/views.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,12 @@ def post(self, request):
5959
"""
6060
Handle POST request to fix LTI user authentication record.
6161
62-
This endpoint fixes LTI-created users who have random 40-character usernames
63-
by updating their username to a more user-friendly value.
62+
This endpoint fixes LTI-created users who have lti_user_id as usernames
6463
6564
Parameters
6665
----------
6766
request : Request
68-
The HTTP request object containing email and username in request.data
67+
The HTTP request object containing email in the payload.
6968
7069
Returns
7170
-------
@@ -98,7 +97,7 @@ def post(self, request):
9897
return HttpResponseBadRequest(
9998
"User with the given email does not appear to be an LTI-created user."
10099
)
101-
# Fix username of the LTI created user
100+
102101
user = lti_user.edx_user
103102
user.email = user.email.split("@")[0] + "@" + PLACEHOLDER_EMAIL_DOMAIN
104103
user.save()

src/ol_openedx_lti_utilities/tests/test_views.py

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from unittest.mock import Mock, patch
22

3-
from ddt import data, ddt, unpack
3+
from ddt import ddt
44
from django.contrib.auth.models import User
55
from django.test import TestCase
66
from rest_framework import status
@@ -16,10 +16,6 @@ def setUp(self):
1616
self.client = APIClient()
1717
self.url = "/api/lti-user-fix/" # Use direct URL path instead of reverse
1818
self.valid_email = "[email protected]"
19-
self.valid_username = "testuser"
20-
self.lti_username = (
21-
"A1B2C3D4E5F6G7H8I9J0K1L2M3N4O5P6Q7R8S9T0" # pragma: allowlist secret
22-
)
2319

2420
# Create and authenticate a staff user for API access
2521
self.user = User.objects.create_user(
@@ -31,8 +27,10 @@ def setUp(self):
3127
self.client.force_authenticate(user=self.user)
3228

3329
@patch("ol_openedx_lti_utilities.views.LtiUser")
34-
@patch("ol_openedx_lti_utilities.views.User")
35-
def test_successful_lti_user_fix(self, mock_user_model, mock_lti_user_model):
30+
@patch(
31+
"ol_openedx_lti_utilities.views.create_retirement_request_and_deactivate_account"
32+
)
33+
def test_successful_lti_user_fix(self, mock_retirement_user, mock_lti_user_model):
3634
"""Test successful LTI user fix."""
3735
# Mock LTI user with bad username
3836
mock_lti_user = Mock()
@@ -47,29 +45,19 @@ def test_successful_lti_user_fix(self, mock_user_model, mock_lti_user_model):
4745
mock_qs.first.return_value = mock_lti_user
4846
mock_lti_user_model.objects.filter.return_value = mock_qs
4947

50-
# Mock User.objects.get
51-
mock_user = Mock()
52-
mock_user_model.objects.get.return_value = mock_user
53-
54-
payload = {"email": self.valid_email, "username": self.valid_username}
48+
payload = {"email": self.valid_email}
5549

5650
response = self.client.post(self.url, payload)
51+
mock_edx_user.save.assert_called_once()
52+
mock_retirement_user.assert_called_once_with(mock_edx_user)
5753

5854
assert response.status_code == status.HTTP_200_OK
59-
mock_user_model.objects.get.assert_called_once_with(email=self.valid_email)
60-
mock_user.save.assert_called_once()
6155

62-
@data(
63-
({"username": "testuser"}, "email and username are required"),
64-
({"email": "[email protected]"}, "email and username are required"),
65-
({}, "email and username are required"),
66-
)
67-
@unpack
68-
def test_missing_required_fields(self, test_data, expected_message):
56+
def test_missing_required_fields(self):
6957
"""Test request with missing required fields."""
70-
response = self.client.post(self.url, test_data)
58+
response = self.client.post(self.url, {})
7159
assert response.status_code == status.HTTP_400_BAD_REQUEST
72-
assert expected_message in response.content.decode()
60+
assert "email is required" in response.content.decode()
7361

7462
@patch("ol_openedx_lti_utilities.views.LtiUser")
7563
def test_no_lti_user_found(self, mock_lti_user_model):
@@ -78,7 +66,7 @@ def test_no_lti_user_found(self, mock_lti_user_model):
7866
mock_qs.first.return_value = None
7967
mock_lti_user_model.objects.filter.return_value = mock_qs
8068

81-
payload = {"email": self.valid_email, "username": self.valid_username}
69+
payload = {"email": self.valid_email}
8270

8371
response = self.client.post(self.url, payload)
8472

@@ -89,7 +77,7 @@ def test_user_does_not_need_fixing(self, mock_lti_user_model):
8977
"""Test when LTI user has normal username (doesn't need fixing)."""
9078
# Mock LTI user with normal username
9179
mock_edx_user = Mock()
92-
mock_edx_user.username = "normalusername" # Not 40-char random
80+
mock_edx_user.username = "normalusername"
9381
mock_edx_user.email = self.valid_email
9482

9583
mock_lti_user = Mock()
@@ -100,7 +88,7 @@ def test_user_does_not_need_fixing(self, mock_lti_user_model):
10088
mock_qs.first.return_value = mock_lti_user
10189
mock_lti_user_model.objects.filter.return_value = mock_qs
10290

103-
payload = {"email": self.valid_email, "username": self.valid_username}
91+
payload = {"email": self.valid_email}
10492

10593
response = self.client.post(self.url, payload)
10694

@@ -112,7 +100,7 @@ def test_user_does_not_need_fixing(self, mock_lti_user_model):
112100

113101
def test_only_post_method_allowed(self):
114102
"""Test that only POST method is allowed."""
115-
payload = {"email": self.valid_email, "username": self.valid_username}
103+
payload = {"email": self.valid_email}
116104

117105
# Test GET
118106
response = self.client.get(self.url)
@@ -139,7 +127,7 @@ def test_logging_on_missing_fields(self, mock_lti_user_model, mock_log): # noqa
139127
response = self.client.post(self.url, payload)
140128

141129
assert response.status_code == status.HTTP_400_BAD_REQUEST
142-
mock_log.error.assert_called_once_with("email and username are required")
130+
mock_log.error.assert_called_once_with("email is required")
143131

144132
@patch("ol_openedx_lti_utilities.views.log")
145133
@patch("ol_openedx_lti_utilities.views.LtiUser")
@@ -149,7 +137,7 @@ def test_logging_on_no_user_found(self, mock_lti_user_model, mock_log):
149137
mock_qs.first.return_value = None
150138
mock_lti_user_model.objects.filter.return_value = mock_qs
151139

152-
payload = {"email": self.valid_email, "username": self.valid_username}
140+
payload = {"email": self.valid_email}
153141

154142
response = self.client.post(self.url, payload)
155143

0 commit comments

Comments
 (0)