Skip to content

Commit 1ac164a

Browse files
authored
Prevent test courses from being overwritten (#2262)
* unsure runs for test courses are published and ensure they dont get pruned * adding test for loader * excluding test mode courses from getting pruned and removed from index * adding test * rename test * making sure newer runs will also process * moving setting published to section before creating object. added/reformatted test logic
1 parent cd344b8 commit 1ac164a

File tree

3 files changed

+82
-6
lines changed

3 files changed

+82
-6
lines changed

learning_resources/etl/loaders.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from django.contrib.auth import get_user_model
66
from django.db import transaction
7+
from django.db.models import Q
78

89
from learning_resources.constants import (
910
LearningResourceDelivery,
@@ -70,7 +71,11 @@ def update_index(learning_resource, newly_created):
7071
learning resource (LearningResource): a learning resource
7172
newly_created (bool): whether the learning resource has just been created
7273
"""
73-
if not newly_created and not learning_resource.published:
74+
if (
75+
not newly_created
76+
and not learning_resource.published
77+
and not learning_resource.test_mode
78+
):
7479
resource_unpublished_actions(learning_resource)
7580
elif learning_resource.published:
7681
resource_upserted_actions(learning_resource, percolate=False)
@@ -293,6 +298,7 @@ def load_run(
293298
LearningResourceRun: the created/updated resource run
294299
"""
295300
run_id = run_data.pop("run_id")
301+
296302
image_data = run_data.pop("image", None)
297303
status = run_data.pop("status", None)
298304
instructors_data = run_data.pop("instructors", [])
@@ -305,6 +311,9 @@ def load_run(
305311
run_data["prices"] = []
306312
resource_prices = []
307313

314+
if learning_resource.test_mode:
315+
run_data["published"] = True
316+
308317
with transaction.atomic():
309318
(
310319
learning_resource_run,
@@ -314,7 +323,6 @@ def load_run(
314323
run_id=run_id,
315324
defaults={**run_data},
316325
)
317-
318326
load_instructors(learning_resource_run, instructors_data)
319327
load_prices(learning_resource_run, resource_prices)
320328
load_image(learning_resource_run, image_data)
@@ -498,7 +506,8 @@ def load_course(
498506
# The course ETL should be the ultimate source of truth for
499507
# courses and their runs.
500508
for run in learning_resource.runs.exclude(
501-
run_id__in=run_ids_to_update_or_create
509+
Q(run_id__in=run_ids_to_update_or_create)
510+
| Q(learning_resource__test_mode=True)
502511
).filter(published=True):
503512
run.published = False
504513
run.save()
@@ -548,7 +557,10 @@ def load_courses(
548557
if courses and config.prune:
549558
for learning_resource in LearningResource.objects.filter(
550559
etl_source=etl_source, resource_type=LearningResourceType.course.name
551-
).exclude(id__in=[learning_resource.id for learning_resource in courses]):
560+
).exclude(
561+
Q(id__in=[learning_resource.id for learning_resource in courses])
562+
| Q(test_mode=True)
563+
):
552564
learning_resource.published = False
553565
learning_resource.save()
554566
resource_unpublished_actions(learning_resource)
@@ -613,7 +625,8 @@ def load_program(
613625
if config.prune:
614626
# mark runs no longer included here as unpublished
615627
for run in learning_resource.runs.exclude(
616-
run_id__in=run_ids_to_update_or_create
628+
Q(run_id__in=run_ids_to_update_or_create)
629+
| Q(learning_resource__test_mode=True)
617630
).filter(published=True):
618631
run.published = False
619632
run.save()

learning_resources/etl/loaders_test.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
PlatformType,
2222
RunStatus,
2323
)
24+
from learning_resources.etl import loaders
2425
from learning_resources.etl.constants import (
2526
CourseLoaderConfig,
2627
ETLSource,
@@ -143,6 +144,28 @@ def mock_upsert_tasks(mocker):
143144
)
144145

145146

147+
@pytest.mark.parametrize("published", [False, True])
148+
@pytest.mark.parametrize("test_mode", [False, True])
149+
@pytest.mark.parametrize("newly_created", [False, True])
150+
def test_update_index_test_mode_behavior(
151+
mocker,
152+
published,
153+
test_mode,
154+
newly_created,
155+
):
156+
"""Test update_index does not remove test_mode content files from index"""
157+
resource_unpublished_actions = mocker.patch(
158+
"learning_resources.etl.loaders.resource_unpublished_actions"
159+
)
160+
lr = LearningResourceFactory.create(published=published, test_mode=test_mode)
161+
162+
loaders.update_index(lr, newly_created)
163+
if test_mode:
164+
resource_unpublished_actions.assert_not_called()
165+
elif not published and not newly_created:
166+
resource_unpublished_actions.assert_called_once()
167+
168+
146169
@pytest.fixture
147170
def learning_resource_offeror():
148171
"""Return a LearningResourceOfferer"""
@@ -276,6 +299,43 @@ def test_load_program( # noqa: PLR0913
276299
mock_upsert_tasks.upsert_learning_resource_immutable_signature.assert_not_called()
277300

278301

302+
@pytest.mark.django_db
303+
def test_load_run_sets_test_resource_run_to_published(mocker):
304+
"""
305+
Test that load_run sets the test_mode run to published
306+
"""
307+
308+
mock_qs = mocker.patch.object(
309+
loaders.LearningResourceRun.objects, "filter", autospec=True
310+
)
311+
mock_qs.return_value.exists.return_value = True
312+
313+
test_mode_learning_resource = LearningResourceFactory.create(test_mode=True)
314+
315+
test_mode_run_id = "test_run_id"
316+
test_mode_run_data = {"run_id": test_mode_run_id, "published": False}
317+
318+
LearningResourceRunFactory.create(
319+
learning_resource=test_mode_learning_resource,
320+
run_id=test_mode_run_id,
321+
published=False,
322+
)
323+
324+
result = loaders.load_run(test_mode_learning_resource, test_mode_run_data)
325+
assert result.published
326+
regular_learning_resource = LearningResourceFactory.create(test_mode=False)
327+
regular_run_id = "test_run_id"
328+
regular_run_data = {"run_id": regular_run_id, "published": False}
329+
LearningResourceRunFactory.create(
330+
learning_resource=regular_learning_resource,
331+
run_id=regular_run_id,
332+
published=False,
333+
)
334+
335+
result = loaders.load_run(regular_learning_resource, regular_run_data)
336+
assert not result.published
337+
338+
279339
def test_load_program_bad_platform(mocker):
280340
"""A bad platform should log an exception and not create the program"""
281341
mock_log = mocker.patch("learning_resources.etl.loaders.log.exception")

learning_resources/management/commands/mixins.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from learning_resources.models import LearningResource
1+
from learning_resources.models import LearningResource, LearningResourceRun
22

33

44
class TestResourceConfigurationMixin:
@@ -13,6 +13,9 @@ def configure_test_resources(self, options):
1313
)
1414
if options.get("test_ids"):
1515
test_ids = options["test_ids"].split(",")
16+
LearningResourceRun.objects.filter(
17+
learning_resource__id__in=test_ids
18+
).update(published=True)
1619
LearningResource.objects.filter(id__in=test_ids).update(
1720
test_mode=True, published=False
1821
)

0 commit comments

Comments
 (0)