Import files for all active runs (#2292)

Author: abeglova

learning_resources/etl/edx_shared.py

@@ -16,7 +16,6 @@
     transform_content_files,
 )
 from learning_resources.models import LearningResourceRun
-from learning_resources.utils import content_files_loaded_actions
 
 log = logging.getLogger(__name__)
 
@@ -147,17 +146,7 @@ def sync_edx_course_files(
 
         if not run:
             continue
-        resource = run.learning_resource
-        if run != (
-            resource.next_run
-            or resource.runs.filter(
-                Q(published=True) | Q(learning_resource__test_mode=True)
-            )
-            .order_by("-start_date")
-            .first()
-        ):
-            log.info("Skipping %s, not the next / most recent run", run.run_id)
-            continue
+
         with TemporaryDirectory() as export_tempdir:
             course_tarpath = Path(export_tempdir, key.split("/")[-1])
             log.info("course tarpath for run %s is %s", run.run_id, course_tarpath)
@@ -169,8 +158,6 @@ def sync_edx_course_files(
                 continue
             if run.checksum == checksum and not overwrite:
                 log.info("Checksums match for %s, skipping load", key)
-                # Ensure any content files for other runs in the course are deindexed
-                content_files_loaded_actions(run=run, deindex_only=True)
                 continue
             try:
                 load_content_files(

learning_resources/etl/edx_shared_test.py

@@ -98,8 +98,8 @@ def test_sync_edx_course_files(  # noqa: PLR0913
     sync_edx_course_files(
         source, [course.id for course in courses], keys, s3_prefix=s3_prefix
     )
-    assert mock_transform.call_count == (2 if published else 0)
-    assert mock_load_content_files.call_count == (2 if published else 0)
+    assert mock_transform.call_count == (4 if published else 0)
+    assert mock_load_content_files.call_count == (4 if published else 0)
     if published:
         for course in courses:
             mock_load_content_files.assert_any_call(course.next_run, fake_data)
@@ -114,7 +114,7 @@ def test_sync_edx_course_files_matching_checksum(
     run = LearningResourceFactory.create(
         is_course=True, create_runs=True, etl_source=ETLSource.mitxonline.name
     ).next_run
-    other_run = run.learning_resource.runs.exclude(id=run.id).first()
+    run.learning_resource.runs.exclude(id=run.id).first()
     run.checksum = "123"
     run.save()
     mocker.patch(
@@ -123,9 +123,6 @@ def test_sync_edx_course_files_matching_checksum(
     mock_index = mocker.patch(
         "learning_resources_search.plugins.tasks.index_run_content_files"
     )
-    mock_deindex = mocker.patch(
-        "learning_resources_search.plugins.tasks.deindex_run_content_files.si"
-    )
     mock_log = mocker.patch("learning_resources.etl.edx_shared.log.info")
     mock_load = mocker.patch("learning_resources.etl.edx_shared.load_content_files")
 
@@ -142,7 +139,6 @@ def test_sync_edx_course_files_matching_checksum(
     )
     sync_edx_course_files("mitxonline", [run.learning_resource.id], [key])
     mock_log.assert_any_call("Checksums match for %s, skipping load", key)
-    mock_deindex.assert_called_once_with(other_run.id, unpublished_only=False)
     mock_load.assert_not_called()
     mock_index.assert_not_called()
 

learning_resources/etl/loaders.py

@@ -801,7 +801,7 @@ def load_content_files(
 
         if calc_completeness:
             calculate_completeness(course_run, content_tags=content_tags)
-        content_files_loaded_actions(run=course_run, deindex_only=False)
+        content_files_loaded_actions(run=course_run)
 
         return content_files_ids
     return None

learning_resources/etl/loaders_test.py

@@ -967,22 +967,19 @@ def test_load_programs(mocker, mock_blocklist, mock_duplicates):
 
 
 @pytest.mark.parametrize("is_published", [True, False])
-@pytest.mark.parametrize("extra_run", [True, False])
 @pytest.mark.parametrize("calc_score", [True, False])
-def test_load_content_files(mocker, is_published, extra_run, calc_score):
+def test_load_content_files(mocker, is_published, calc_score):
     """Test that load_content_files calls the expected functions"""
     course = LearningResourceFactory.create(is_course=True, create_runs=False)
     course_run = LearningResourceRunFactory.create(
         published=is_published, learning_resource=course
     )
-    if extra_run:
-        LearningResourceRunFactory.create(
-            published=is_published,
-            learning_resource=course,
-            start_date=now_in_utc() - timedelta(days=365),
-        )
-    run_count = 2 if extra_run else 1
-    assert course.runs.count() == run_count
+    LearningResourceRunFactory.create(
+        published=is_published,
+        learning_resource=course,
+        start_date=now_in_utc() - timedelta(days=365),
+    )
+    assert course.runs.count() == 2
 
     deleted_content_file = ContentFileFactory.create(run=course_run)
     returned_content_file_id = deleted_content_file.id + 1
@@ -1011,9 +1008,7 @@ def test_load_content_files(mocker, is_published, extra_run, calc_score):
     load_content_files(course_run, content_data, calc_completeness=calc_score)
     assert mock_load_content_file.call_count == len(content_data)
     assert mock_bulk_index.call_count == (1 if is_published else 0)
-    assert mock_bulk_delete.call_count == (
-        run_count if not is_published else run_count - 1
-    )
+    assert mock_bulk_delete.call_count == 0 if is_published else 1
     assert mock_calc_score.call_count == (1 if calc_score else 0)
     deleted_content_file.refresh_from_db()
     assert not deleted_content_file.published
@@ -1045,10 +1040,6 @@ def test_load_test_mode_resource_content_files(
         autospec=True,
         return_value=[],
     )
-
-    mocker.patch(
-        "learning_resources.etl.edx_shared.content_files_loaded_actions",
-    )
     mocker.patch(
         "learning_resources_search.plugins.tasks.deindex_run_content_files",
         autospec=True,

learning_resources/hooks.py

@@ -81,7 +81,7 @@ def offeror_delete(self, offeror):
         """Trigger actions to delete a learning resource offeror"""
 
     @hookspec
-    def content_files_loaded(self, run, deindex_only):
+    def content_files_loaded(self, run):
         """Trigger actions after content files are loaded for a run"""
 
 

learning_resources/utils.py

@@ -322,13 +322,13 @@ def bulk_resources_unpublished_actions(resource_ids: list[int], resource_type: s
     )
 
 
-def content_files_loaded_actions(run: LearningResourceRun, deindex_only):
+def content_files_loaded_actions(run: LearningResourceRun):
     """
     Trigger plugins when content files are loaded for a LearningResourceRun
     """
     pm = get_plugin_manager()
     hook = pm.hook
-    hook.content_files_loaded(run=run, deindex_only=deindex_only)
+    hook.content_files_loaded(run=run)
 
 
 def resource_run_unpublished_actions(run: LearningResourceRun):

learning_resources_search/plugins.py

@@ -187,36 +187,29 @@ def resource_run_delete(self, run):
         run.delete()
 
     @hookimpl
-    def content_files_loaded(self, run, deindex_only):
+    def content_files_loaded(self, run):
         """
         Upsert a created/modified run's content files
 
          Args:
              run(LearningResourceRun): The LearningResourceRun that was upserted
-             deindex_only(bool): Only deindex the other runs' content files
         """
-        course_runs_to_deindex = run.learning_resource.runs.all()
         if run.published:
-            # Only one run per course should have contentfiles at any given time
-            course_runs_to_deindex = course_runs_to_deindex.exclude(id=run.id)
-            if not deindex_only:
-                index_tasks = []
-                if django_settings.QDRANT_ENABLE_INDEXING_PLUGIN_HOOKS:
-                    index_tasks.append(
-                        vector_tasks.embed_run_content_files.si(run.id),
-                    )
+            index_tasks = []
+            if django_settings.QDRANT_ENABLE_INDEXING_PLUGIN_HOOKS:
                 index_tasks.append(
-                    tasks.index_run_content_files.si(run.id),
+                    vector_tasks.embed_run_content_files.si(run.id),
                 )
-                try_with_retry_as_task(chain(*index_tasks))
-        for course_run in course_runs_to_deindex:
+            index_tasks.append(
+                tasks.index_run_content_files.si(run.id),
+            )
+            try_with_retry_as_task(chain(*index_tasks))
+        else:
             deindex_tasks = [
-                tasks.deindex_run_content_files.si(
-                    course_run.id, unpublished_only=False
-                ),
+                tasks.deindex_run_content_files.si(run.id, unpublished_only=False),
             ]
             if django_settings.QDRANT_ENABLE_INDEXING_PLUGIN_HOOKS:
                 deindex_tasks.append(
-                    vector_tasks.remove_run_content_files.si(course_run.id),
+                    vector_tasks.remove_run_content_files.si(run.id),
                 )
             try_with_retry_as_task(chain(*deindex_tasks))