Skip to content

Commit 163556a

Browse files
committed
fix: issues
1 parent e11eccb commit 163556a

4 files changed

Lines changed: 28 additions & 193 deletions

File tree

courses/management/commands/test_unenroll_enrollment.py

Lines changed: 0 additions & 149 deletions
This file was deleted.

courses/management/commands/unenroll_learners.py

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def _parse_csv(self, csv_path):
8484
if not reader.fieldnames or not {"user", "courseware_id"}.issubset(
8585
set(reader.fieldnames)
8686
):
87-
raise CommandError( # noqa: TRY301
87+
raise CommandError(
8888
"CSV file must have 'user' and 'courseware_id' columns" # noqa: EM101
8989
)
9090
for row_num, row in enumerate(reader, start=2):
@@ -113,11 +113,7 @@ def _parse_inline_users(self, users_str, courseware_id):
113113
Returns:
114114
list[tuple[str, str]]: List of (user_identifier, courseware_id) tuples
115115
"""
116-
return [
117-
(u.strip(), courseware_id)
118-
for u in users_str.split(",")
119-
if u.strip()
120-
]
116+
return [(u.strip(), courseware_id) for u in users_str.split(",") if u.strip()]
121117

122118
def _dry_run(self, entries):
123119
"""Preview which enrollments would be unenrolled without making changes."""
@@ -136,9 +132,7 @@ def _dry_run(self, entries):
136132
continue
137133

138134
if cw_id not in run_cache:
139-
run_cache[cw_id] = CourseRun.objects.filter(
140-
courseware_id=cw_id
141-
).first()
135+
run_cache[cw_id] = CourseRun.objects.filter(courseware_id=cw_id).first()
142136
course_run = run_cache[cw_id]
143137
if course_run is None:
144138
self.stderr.write(
@@ -196,12 +190,10 @@ def handle(self, *args, **options): # noqa: ARG002
196190
self._dry_run(entries)
197191
return
198192

199-
summary = bulk_unenroll_learners(
200-
entries, keep_failed_enrollments=keep_failed
201-
)
193+
summary = bulk_unenroll_learners(entries, keep_failed_enrollments=keep_failed)
202194

203195
# Print details
204-
for user_id, cw_id, status, message in summary["details"]:
196+
for _user_id, _cw_id, status, message in summary["details"]:
205197
if status == "succeeded":
206198
self.stdout.write(self.style.SUCCESS(f" {message}"))
207199
elif status == "skipped":

courses/management/commands/unenroll_learners_test.py

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@
99
from django.core.management.base import CommandError
1010

1111
from courses.factories import CourseRunEnrollmentFactory, CourseRunFactory
12+
from courses.management.utils import bulk_unenroll_learners
1213
from users.factories import UserFactory
1314

1415

15-
@pytest.fixture()
16+
@pytest.fixture
1617
def mock_bulk_unenroll(mocker):
1718
"""Mock bulk_unenroll_learners to avoid edX API calls"""
1819
return mocker.patch(
@@ -87,7 +88,12 @@ def test_inline_users_mixed_results(self, mock_bulk_unenroll):
8788
"details": [
8889
("a@b.com", "run-1", "succeeded", "Unenrolled: a@b.com from run-1"),
8990
("b@c.com", "run-1", "failed", "Failed to unenroll b@c.com from run-1"),
90-
("c@d.com", "run-1", "skipped", "No active enrollment for c@d.com in run-1"),
91+
(
92+
"c@d.com",
93+
"run-1",
94+
"skipped",
95+
"No active enrollment for c@d.com in run-1",
96+
),
9197
],
9298
}
9399

@@ -135,10 +141,12 @@ def test_csv_success(self, mock_bulk_unenroll):
135141
],
136142
}
137143

138-
csv_path = self._write_csv([
139-
{"user": "a@b.com", "courseware_id": "run-1"},
140-
{"user": "c@d.com", "courseware_id": "run-2"},
141-
])
144+
csv_path = self._write_csv(
145+
[
146+
{"user": "a@b.com", "courseware_id": "run-1"},
147+
{"user": "c@d.com", "courseware_id": "run-2"},
148+
]
149+
)
142150

143151
out = StringIO()
144152
call_command("unenroll_learners", csv=csv_path, stdout=out)
@@ -180,10 +188,12 @@ def test_csv_skips_empty_rows(self, mock_bulk_unenroll):
180188
],
181189
}
182190

183-
csv_path = self._write_csv([
184-
{"user": "", "courseware_id": "course-v1:x+y+z"},
185-
{"user": "a@b.com", "courseware_id": "run-1"},
186-
])
191+
csv_path = self._write_csv(
192+
[
193+
{"user": "", "courseware_id": "course-v1:x+y+z"},
194+
{"user": "a@b.com", "courseware_id": "run-1"},
195+
]
196+
)
187197

188198
out = StringIO()
189199
call_command("unenroll_learners", csv=csv_path, stdout=out)
@@ -257,8 +267,6 @@ class TestBulkUnenrollLearnersUtil:
257267

258268
def test_successful_unenrollment(self, mocker):
259269
"""Should unenroll active enrollments successfully"""
260-
from courses.management.utils import bulk_unenroll_learners
261-
262270
enrollment = CourseRunEnrollmentFactory.create(active=True)
263271
mocker.patch(
264272
"courses.management.utils.deactivate_run_enrollment",
@@ -275,8 +283,6 @@ def test_successful_unenrollment(self, mocker):
275283

276284
def test_user_not_found(self):
277285
"""Should skip when user doesn't exist"""
278-
from courses.management.utils import bulk_unenroll_learners
279-
280286
run = CourseRunFactory.create()
281287
result = bulk_unenroll_learners(
282288
[("nonexistent@example.com", run.courseware_id)]
@@ -287,20 +293,14 @@ def test_user_not_found(self):
287293

288294
def test_course_run_not_found(self):
289295
"""Should skip when course run doesn't exist"""
290-
from courses.management.utils import bulk_unenroll_learners
291-
292296
user = UserFactory.create()
293-
result = bulk_unenroll_learners(
294-
[(user.email, "course-v1:fake+fake+fake")]
295-
)
297+
result = bulk_unenroll_learners([(user.email, "course-v1:fake+fake+fake")])
296298

297299
assert result["skipped"] == 1
298300
assert result["succeeded"] == 0
299301

300302
def test_no_active_enrollment(self):
301303
"""Should skip when enrollment is inactive"""
302-
from courses.management.utils import bulk_unenroll_learners
303-
304304
enrollment = CourseRunEnrollmentFactory.create(active=False)
305305
result = bulk_unenroll_learners(
306306
[(enrollment.user.email, enrollment.run.courseware_id)]
@@ -311,8 +311,6 @@ def test_no_active_enrollment(self):
311311

312312
def test_deactivation_failure(self, mocker):
313313
"""Should count as failed when deactivate_run_enrollment returns None"""
314-
from courses.management.utils import bulk_unenroll_learners
315-
316314
enrollment = CourseRunEnrollmentFactory.create(active=True)
317315
mocker.patch(
318316
"courses.management.utils.deactivate_run_enrollment",
@@ -328,8 +326,6 @@ def test_deactivation_failure(self, mocker):
328326

329327
def test_keep_failed_enrollments_passed(self, mocker):
330328
"""Should pass keep_failed_enrollments to deactivate_run_enrollment"""
331-
from courses.management.utils import bulk_unenroll_learners
332-
333329
enrollment = CourseRunEnrollmentFactory.create(active=True)
334330
mock_deactivate = mocker.patch(
335331
"courses.management.utils.deactivate_run_enrollment",

courses/management/utils.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,7 @@ def unenroll_learner_from_run(user, course_run, *, keep_failed_enrollments=False
8181
enrollment_result is the deactivated enrollment on success, None on failure.
8282
message is a human-readable status string.
8383
"""
84-
enrollment = CourseRunEnrollment.objects.filter(
85-
user=user, run=course_run
86-
).first()
84+
enrollment = CourseRunEnrollment.objects.filter(user=user, run=course_run).first()
8785
if enrollment is None or not enrollment.active:
8886
return (
8987
None,
@@ -142,9 +140,7 @@ def bulk_unenroll_learners(entries, *, keep_failed_enrollments=False):
142140

143141
# Resolve course run (with caching)
144142
if cw_id not in run_cache:
145-
run_cache[cw_id] = CourseRun.objects.filter(
146-
courseware_id=cw_id
147-
).first()
143+
run_cache[cw_id] = CourseRun.objects.filter(courseware_id=cw_id).first()
148144
course_run = run_cache[cw_id]
149145
if course_run is None:
150146
msg = f"Course run not found: {cw_id}"

0 commit comments

Comments
 (0)