Skip to content

Commit d2ec251

Browse files
committed
fix: Include start/end dates for external course run
exit
1 parent c8c0755 commit d2ec251

File tree

4 files changed

+97
-20
lines changed

4 files changed

+97
-20
lines changed

course_discovery/apps/api/tests/test_utils.py

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -116,16 +116,19 @@ def make_studio_data(self, run, add_pacing=True, add_schedule=True, team=None, a
116116
}
117117
if add_pacing:
118118
data['pacing_type'] = run.pacing_type
119-
if add_schedule:
120-
data['schedule'] = {
121-
'start': serialize_datetime(run.start),
122-
'end': serialize_datetime(run.end),
123-
}
124-
if add_enrollment_dates:
125-
data['schedule'] = {
126-
'enrollment_start': serialize_datetime(run.enrollment_start),
127-
'enrollment_end': serialize_datetime(run.enrollment_end),
128-
}
119+
120+
if add_schedule or add_enrollment_dates:
121+
data['schedule'] = {}
122+
if add_schedule:
123+
data['schedule'].update({
124+
'start': serialize_datetime(run.start),
125+
'end': serialize_datetime(run.end),
126+
})
127+
if add_enrollment_dates:
128+
data['schedule'].update({
129+
'enrollment_start': serialize_datetime(run.enrollment_start),
130+
'enrollment_end': serialize_datetime(run.enrollment_end),
131+
})
129132
return data
130133

131134
def test_create_rerun(self):
@@ -194,7 +197,7 @@ def test_generate_data_for_studio_api__external_course_enrollment_dates(self):
194197
exec_ed_type = CourseTypeFactory(slug=CourseType.EXECUTIVE_EDUCATION_2U)
195198
run = CourseRunFactory(course=CourseFactory(type=exec_ed_type))
196199
with mock.patch('course_discovery.apps.api.utils.logger') as mock_logger:
197-
expected_data = self.make_studio_data(run, add_pacing=False, add_schedule=False, add_enrollment_dates=True)
200+
expected_data = self.make_studio_data(run, add_pacing=False, add_schedule=True, add_enrollment_dates=True)
198201
output_data = StudioAPI.generate_data_for_studio_api(run, False)
199202
assert output_data == expected_data
200203
mock_logger.info.assert_called_with(
@@ -208,14 +211,35 @@ def test_generate_data_for_studio_api__external_course_missing_enrollment_dates(
208211
exec_ed_type = CourseTypeFactory(slug=CourseType.EXECUTIVE_EDUCATION_2U)
209212
run = CourseRunFactory(course=CourseFactory(type=exec_ed_type), enrollment_start=None, enrollment_end=None)
210213
with mock.patch('course_discovery.apps.api.utils.logger') as mock_logger:
211-
expected_data = self.make_studio_data(run, add_pacing=False, add_schedule=False)
214+
expected_data = self.make_studio_data(run, add_pacing=False, add_schedule=True, add_enrollment_dates=False)
212215
output_data = StudioAPI.generate_data_for_studio_api(run, False)
213216
assert output_data == expected_data
214217

215218
with self.assertRaises(AssertionError):
216-
mock_logger.info.assert_called_with(
217-
f'Enrollment information added to data {output_data} for course run {run.key}'
218-
)
219+
mock_logger.info.assert_called()
220+
221+
def test_generate_data_for_studio_api__non_external_course_no_start_end_on_update(self):
222+
"""Test that start/end are NOT included when updating a non-external course."""
223+
run = CourseRunFactory() # Default: non-external
224+
expected_data = self.make_studio_data(run, add_pacing=False, add_schedule=False, add_enrollment_dates=True)
225+
with mock.patch('course_discovery.apps.api.utils.logger') as mock_logger:
226+
output_data = StudioAPI.generate_data_for_studio_api(run, creating=False)
227+
self.assertEqual(output_data, expected_data)
228+
mock_logger.info.assert_called_with(
229+
f'Enrollment information added to data {output_data} for course run {run.key}'
230+
)
231+
232+
def test_generate_data_for_studio_api__external_course_start_end_on_update(self):
233+
"""Test that start/end are included for external course on update."""
234+
exec_ed_type = CourseTypeFactory(slug=CourseType.EXECUTIVE_EDUCATION_2U)
235+
run = CourseRunFactory(course=CourseFactory(type=exec_ed_type))
236+
expected_data = self.make_studio_data(run, add_pacing=False, add_schedule=True, add_enrollment_dates=True)
237+
with mock.patch('course_discovery.apps.api.utils.logger') as mock_logger:
238+
output_data = StudioAPI.generate_data_for_studio_api(run, creating=False)
239+
self.assertEqual(output_data, expected_data)
240+
mock_logger.info.assert_called_with(
241+
f'Enrollment information added to data {output_data} for course run {run.key}'
242+
)
219243

220244
def test_calculate_course_run_key_run_value_with_multiple_runs_per_trimester(self):
221245
start = datetime.datetime(2017, 2, 1)

course_discovery/apps/api/utils.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -253,10 +253,11 @@ def calculate_course_run_key_run_value(cls, course_num, start):
253253
def generate_data_for_studio_api(cls, course_run, creating, user=None):
254254
editors = [editor.user for editor in course_run.course.editors.all()]
255255
key = CourseKey.from_string(course_run.key)
256-
256+
is_external = course_run.course.is_external_course
257257
# start, end, and pacing are not sent on updates - Studio is where users edit them
258-
start = course_run.start if creating else None
259-
end = course_run.end if creating else None
258+
# Include start and end only if creating or if it's an external course during update
259+
start = course_run.start if creating or is_external else None
260+
end = course_run.end if creating or is_external else None
260261
pacing = course_run.pacing_type if creating else None
261262
enrollment_start = course_run.enrollment_start
262263
enrollment_end = course_run.enrollment_end
@@ -300,10 +301,12 @@ def generate_data_for_studio_api(cls, course_run, creating, user=None):
300301
# But when the course run is created, in Studio or Discovery, the enrollment dates are not taken as input.
301302
# It is better to keep the flow consistent across places.
302303
# Allow sending enrollment start and end dates as part of Update only.
303-
data['schedule'] = {
304+
# Using setdefault + update avoids overwriting and prevents KeyErrors.
305+
data.setdefault('schedule', {})
306+
data['schedule'].update({
304307
'enrollment_start': serialize_datetime(course_run.enrollment_start),
305308
'enrollment_end': serialize_datetime(course_run.enrollment_end),
306-
}
309+
})
307310
logger.info(f"Enrollment information added to data {data} for course run {course_run.key}")
308311

309312
return data

course_discovery/apps/course_metadata/data_loaders/csv_loader.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,8 @@ def update_course_run_api_request_data(self, course_run_data, course_run, course
481481
'prices': self.get_pricing_representation(course_run_data['verified_price'], course_type),
482482
'staff': staff_uuids,
483483
'draft': is_draft,
484+
'start': self.get_formatted_datetime_string(f"{course_run_data['start_date']} {course_run_data['start_time']}"), # pylint: disable=line-too-long
485+
'end': self.get_formatted_datetime_string(f"{course_run_data['end_date']} {course_run_data['end_time']}"),
484486

485487
'weeks_to_complete': course_run_data['length'],
486488
'min_effort': course_run_data['minimum_effort'],

course_discovery/apps/course_metadata/data_loaders/tests/test_csv_loader.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from tempfile import NamedTemporaryFile
88
from unittest import mock
99

10+
import pytz
1011
import responses
1112
from ddt import data, ddt, unpack
1213
from edx_toggles.toggles.testutils import override_waffle_switch
@@ -159,6 +160,53 @@ def test_invalid_course_run_type(self, jwt_decode_patch): # pylint: disable=unu
159160
assert Course.objects.count() == 0
160161
assert CourseRun.objects.count() == 0
161162

163+
@responses.activate
164+
def test_course_run_update_start_and_end_dates(self, jwt_decode_patch): # pylint: disable=unused-argument
165+
"""
166+
Verify that 'start_date' and 'end_date' from CSV correctly update
167+
the published CourseRun after CSV ingestion.
168+
"""
169+
self._setup_prerequisites(self.partner)
170+
self.mock_studio_calls(self.partner)
171+
self.mock_ecommerce_publication(self.partner)
172+
self.mock_image_response()
173+
course = CourseFactory(
174+
key=self.COURSE_KEY,
175+
partner=self.partner,
176+
type=self.course_type,
177+
draft=True
178+
)
179+
CourseRunFactory(
180+
course=course,
181+
key=self.COURSE_RUN_KEY,
182+
type=self.course_run_type,
183+
status='published',
184+
draft=True,
185+
start=datetime.datetime(2030, 1, 1, tzinfo=pytz.UTC),
186+
end=datetime.datetime(2030, 12, 31, tzinfo=pytz.UTC)
187+
)
188+
csv_data = copy.deepcopy(mock_data.VALID_COURSE_AND_COURSE_RUN_CSV_DICT)
189+
csv_data.update({
190+
"start_date": "01/01/2035",
191+
"start_time": "09:30",
192+
"end_date": "12/31/2035",
193+
"end_time": "17:45"
194+
})
195+
expected_start = datetime.datetime(2035, 1, 1, 9, 30, tzinfo=pytz.UTC)
196+
expected_end = datetime.datetime(2035, 12, 31, 17, 45, tzinfo=pytz.UTC)
197+
with NamedTemporaryFile() as csv_file:
198+
csv_file = self._write_csv(csv_file, [csv_data])
199+
with mock.patch.object(CSVDataLoader, 'call_course_api', self.mock_call_course_api):
200+
loader = CSVDataLoader(
201+
self.partner,
202+
csv_path=csv_file.name,
203+
product_source=self.source.slug
204+
)
205+
loader.ingest()
206+
course_run = CourseRun.objects.get(key=self.COURSE_RUN_KEY, draft=False)
207+
assert course_run.start == expected_start, f"Expected start {expected_start}, got {course_run.start}"
208+
assert course_run.end == expected_end, f"Expected end {expected_end}, got {course_run.end}"
209+
162210
@responses.activate
163211
def test_image_download_failure(self, jwt_decode_patch): # pylint: disable=unused-argument
164212
"""

0 commit comments

Comments
 (0)