From 4160784d83205dc4bb5e6ee14e50de5707b93562 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 10 Jan 2025 10:01:07 +0530 Subject: [PATCH 01/13] feat: entity linking command --- cms/envs/common.py | 1 + lms/envs/common.py | 1 + .../content/course_overviews/signals.py | 15 ++++ .../core/djangoapps/content_libraries/api.py | 25 +++++++ .../commands/recreate_upstream_links.py | 75 +++++++++++++++++++ .../content_libraries/signal_handlers.py | 37 ++++++++- .../djangoapps/content_libraries/tasks.py | 65 ++++++++++++++++ 7 files changed, 216 insertions(+), 3 deletions(-) create mode 100644 openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py diff --git a/cms/envs/common.py b/cms/envs/common.py index 591247388a9d..e1536141109b 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1887,6 +1887,7 @@ "openedx_learning.apps.authoring.components", "openedx_learning.apps.authoring.contents", "openedx_learning.apps.authoring.publishing", + "openedx_learning.apps.authoring.linking", ] diff --git a/lms/envs/common.py b/lms/envs/common.py index cb7643c3668e..7283081eddbe 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3380,6 +3380,7 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring "openedx_learning.apps.authoring.components", "openedx_learning.apps.authoring.contents", "openedx_learning.apps.authoring.publishing", + "openedx_learning.apps.authoring.linking", ] diff --git a/openedx/core/djangoapps/content/course_overviews/signals.py b/openedx/core/djangoapps/content/course_overviews/signals.py index 4ae2f4bf0f8a..93fcbe6b3603 100644 --- a/openedx/core/djangoapps/content/course_overviews/signals.py +++ b/openedx/core/djangoapps/content/course_overviews/signals.py @@ -10,6 +10,7 @@ from django.dispatch import Signal from django.dispatch.dispatcher import receiver +from openedx.core.djangoapps.content_libraries.tasks import update_course_name_in_upstream_links from openedx.core.djangoapps.signals.signals import COURSE_CERT_DATE_CHANGE from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.django import SignalHandler @@ -89,6 +90,7 @@ def _check_for_course_changes(previous_course_overview, updated_course_overview) _check_for_course_start_date_changes(previous_course_overview, updated_course_overview) _check_for_pacing_changes(previous_course_overview, updated_course_overview) _check_for_cert_date_changes(previous_course_overview, updated_course_overview) + _check_for_display_name_change(previous_course_overview, updated_course_overview) def _check_for_course_start_date_changes(previous_course_overview, updated_course_overview): @@ -216,3 +218,16 @@ def _send_course_cert_date_change_signal(): if send_signal: transaction.on_commit(_send_course_cert_date_change_signal) + + +def _check_for_display_name_change(previous_course_overview, updated_course_overview): + """ + Checks for change in display name of course and triggers task to update course name in upstream->downstream entity + links table. + """ + if previous_course_overview.display_name_with_default == updated_course_overview.display_name_with_default: + return + update_course_name_in_upstream_links.delay( + str(previous_course_overview.id), + updated_course_overview.display_name_with_default + ) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index c51c707fc470..0ed68c36a72a 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1952,3 +1952,28 @@ def import_blocks_create_task(library_key, course_key, use_course_key_as_block_i log.info(f"Import block task created: import_task={import_task} " f"celery_task={result.id}") return import_task + + +def create_or_update_xblock_upstream_link(xblock, course_key: str, course_name: str, created: datetime | None = None): + """ + Create or update upstream->downstream link in database for given xblock. + """ + if not xblock.upstream: + log.info(f"No upstream found for xblock: {xblock.usage_key}") + return None + upstream_usage_key = UsageKeyV2.from_string(xblock.upstream) + try: + lib_component = get_component_from_usage_key(upstream_usage_key) + except ObjectDoesNotExist: + log.exception("Library block not found!") + lib_component = None + authoring_api.update_or_create_entity_link( + lib_component, + upstream_usage_key=xblock.upstream, + downstream_context_key=course_key, + downstream_context_title=course_name, + downstream_usage_key=str(xblock.usage_key), + version_synced=xblock.upstream_version, + version_declined=xblock.upstream_version_declined, + created=created, + ) diff --git a/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py b/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py new file mode 100644 index 000000000000..c24bf5fc4bf9 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py @@ -0,0 +1,75 @@ +""" +Management command to recreate upstream-dowstream links in PublishableEntityLink for course(s). + +This command can be run for all the courses or for given list of courses. +""" + +from __future__ import annotations + +import logging +from datetime import datetime, timezone + +from django.core.management.base import BaseCommand, CommandError +from django.utils.translation import gettext as _ + +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + +from ...tasks import create_or_update_upstream_links + +log = logging.getLogger(__name__) + + +class Command(BaseCommand): + """ + Recreate links for course(s) in PublishableEntityLink table. + + Examples: + # Recreate upstream links for two courses. + $ ./manage.py cms recreate_upstream_links --course course-v1:edX+DemoX.1+2014 \ + --course course-v1:edX+DemoX.2+2015 + # Recreate upstream links for all courses. + $ ./manage.py cms recreate_upstream_links --all + # Force recreate links for all courses including completely processed ones. + $ ./manage.py cms recreate_upstream_links --all + """ + + def add_arguments(self, parser): + parser.add_argument( + '--course', + metavar=_('COURSE_KEY'), + action='append', + help=_('Recreate links for xblocks under given course keys. For eg. course-v1:edX+DemoX.1+2014'), + default=[], + ) + parser.add_argument( + '--all', + action='store_true', + help=_( + 'Recreate links for xblocks under all courses. NOTE: this can take long time depending' + ' on number of course and xblocks' + ), + ) + parser.add_argument( + '--force', + action='store_true', + help=_('Recreate links even for completely processed courses.'), + ) + + def handle(self, *args, **options): + """ + Handle command + """ + courses = options['course'] + should_process_all = options['all'] + force = options['force'] + self.time_now = datetime.now(tz=timezone.utc) + if not courses and not should_process_all: + raise CommandError('Either --course or --all argument should be provided.') + + if should_process_all and courses: + raise CommandError('Only one of --course or --all argument should be provided.') + + if should_process_all: + courses = CourseOverview.get_all_course_keys() + for course in courses: + create_or_update_upstream_links.delay(str(course), force) diff --git a/openedx/core/djangoapps/content_libraries/signal_handlers.py b/openedx/core/djangoapps/content_libraries/signal_handlers.py index 58f45d218e95..95ae965d0b2a 100644 --- a/openedx/core/djangoapps/content_libraries/signal_handlers.py +++ b/openedx/core/djangoapps/content_libraries/signal_handlers.py @@ -5,7 +5,7 @@ import logging from django.conf import settings -from django.db.models.signals import post_save, post_delete, m2m_changed +from django.db.models.signals import m2m_changed, post_delete, post_save from django.dispatch import receiver from opaque_keys import InvalidKeyError @@ -13,21 +13,25 @@ from openedx_events.content_authoring.data import ( ContentObjectChangedData, LibraryCollectionData, + XBlockData, ) from openedx_events.content_authoring.signals import ( CONTENT_OBJECT_ASSOCIATIONS_CHANGED, LIBRARY_COLLECTION_CREATED, LIBRARY_COLLECTION_DELETED, LIBRARY_COLLECTION_UPDATED, + XBLOCK_CREATED, + XBLOCK_DELETED, + XBLOCK_UPDATED, ) -from openedx_learning.api.authoring import get_component, get_components +from openedx_learning.api.authoring import delete_entity_link, get_component, get_components from openedx_learning.api.authoring_models import Collection, CollectionPublishableEntity, Component, PublishableEntity from lms.djangoapps.grades.api import signals as grades_signals from .api import library_component_usage_key from .models import ContentLibrary, LtiGradedResource - +from .tasks import create_or_update_xblock_upstream_link log = logging.getLogger(__name__) @@ -203,3 +207,30 @@ def library_collection_entities_changed(sender, instance, action, pk_set, **kwar for component in components.all(): _library_collection_component_changed(component, library.library_key) + + +@receiver(XBLOCK_CREATED) +@receiver(XBLOCK_UPDATED) +def create_or_update_upstream_downstream_link_handler(**kwargs): + """ + Automatically create or update upstream->downstream link in database. + """ + xblock_info = kwargs.get("xblock_info", None) + if not xblock_info or not isinstance(xblock_info, XBlockData): + log.error("Received null or incorrect data for event") + return + + create_or_update_xblock_upstream_link.delay(str(xblock_info.usage_key)) + + +@receiver(XBLOCK_DELETED) +def delete_upstream_downstream_link_handler(**kwargs): + """ + Delete upstream->downstream link from database on xblock delete. + """ + xblock_info = kwargs.get("xblock_info", None) + if not xblock_info or not isinstance(xblock_info, XBlockData): + log.error("Received null or incorrect data for event") + return + + delete_entity_link(str(xblock_info.usage_key)) diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index f56b4adfe313..0cedc2ba2a46 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -17,18 +17,23 @@ from __future__ import annotations import logging +from datetime import datetime, timezone from celery import shared_task from celery_utils.logged_task import LoggedTask from celery.utils.log import get_task_logger from edx_django_utils.monitoring import set_code_owner_attribute, set_code_owner_attribute_from_module +from opaque_keys.edx.keys import UsageKey from user_tasks.tasks import UserTask, UserTaskStatus from xblock.fields import Scope from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import BlockUsageLocator +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib import ensure_cms +from openedx_learning.api.authoring import get_entity_links, get_or_create_course_link_status +from openedx_learning.api.authoring_models import CourseLinksStatusChoices from xmodule.capa_block import ProblemBlock from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LegacyLibraryContentBlock from xmodule.modulestore import ModuleStoreEnum @@ -169,6 +174,66 @@ def duplicate_children( self.status.fail({'raw_error_msg': str(exception)}) +@shared_task(base=LoggedTask) +@set_code_owner_attribute +def create_or_update_xblock_upstream_link(usage_key): + ensure_cms("create_or_update_xblock_upstream_link may only be executed in a CMS context") + xblock = modulestore().get_item(UsageKey.from_string(usage_key)) + if not xblock.upstream: + TASK_LOGGER.info(f"No upstream found for xblock: {xblock.usage_key}") + return + try: + course_name = CourseOverview.get_from_id(xblock.course_id).display_name_with_default + except CourseOverview.DoesNotExist: + TASK_LOGGER.exception(f'Could not find course: {xblock.course_id}') + return + api.create_or_update_xblock_upstream_link(xblock, str(xblock.course_id), course_name) + + +@shared_task(base=LoggedTask) +@set_code_owner_attribute +def create_or_update_upstream_links(course_key_str: str, force: bool = False): + """ + A Celery task to create or update upstream downstream links in database from course xblock content. + """ + ensure_cms("create_or_update_upstream_links may only be executed in a CMS context") + + created = datetime.now(timezone.utc) + course_status = get_or_create_course_link_status(course_key_str, created) + if course_status.status in [ + CourseLinksStatusChoices.COMPLETED, + CourseLinksStatusChoices.PROCESSING + ] and not force: + return + store = modulestore() + course_key = CourseKey.from_string(course_key_str) + course_status.status = CourseLinksStatusChoices.PROCESSING + course_status.save() + try: + course_name = CourseOverview.get_from_id(course_key).display_name_with_default + except CourseOverview.DoesNotExist: + TASK_LOGGER.exception(f'Could not find course: {course_key_str}') + return + xblocks = store.get_items(course_key, settings={"upstream": lambda x: x is not None}) + for xblock in xblocks: + api.create_or_update_xblock_upstream_link(xblock, course_key_str, course_name, created) + course_status.status = CourseLinksStatusChoices.COMPLETED + course_status.save() + + +@shared_task(base=LoggedTask) +@set_code_owner_attribute +def update_course_name_in_upstream_links(course_key_str: str, new_course_name: str): + """ + Celery task to update course name in upstream->downstream entity links. + """ + updated_time = datetime.now(timezone.utc) + get_entity_links({"downstream_context_key": course_key_str}).update( + downstream_context_title=new_course_name, + updated=updated_time + ) + + def _sync_children( task: LibrarySyncChildrenTask, store: MixedModuleStore, From 610947743fc8f886c9b75193819135369fa0cca7 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 15 Jan 2025 12:05:10 +0530 Subject: [PATCH 02/13] test: recreate_upstream_links management command --- .../commands/recreate_upstream_links.py | 1 + .../tests/test_upstream_downstream_links.py | 76 +++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py diff --git a/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py b/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py index c24bf5fc4bf9..08ceec2b0d07 100644 --- a/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py +++ b/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py @@ -72,4 +72,5 @@ def handle(self, *args, **options): if should_process_all: courses = CourseOverview.get_all_course_keys() for course in courses: + log.info(f"Start processing upstream->dowstream links in course: {course}") create_or_update_upstream_links.delay(str(course), force) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py new file mode 100644 index 000000000000..a3315c6df42f --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py @@ -0,0 +1,76 @@ +""" +Tests for upstream downstream tracking links. +""" + +from io import StringIO +from unittest.mock import patch + +from django.core.management import call_command +from django.core.management.base import CommandError + +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +class TestRecreateUpstreamLinks(ModuleStoreTestCase, CacheIsolationTestCase): + """ + Test recreate_upstream_links management command. + """ + + ENABLED_SIGNALS = ['course_deleted', 'course_published'] + + def call_command(self, *args, **kwargs): + """ + call command with pass args. + """ + out = StringIO() + kwargs['stdout'] = out + call_command('recreate_upstream_links', *args, **kwargs) + return out + + def test_call_with_invalid_args(self): + """ + Test command with invalid args. + """ + with self.assertRaisesRegex(CommandError, 'Either --course or --all argument'): + self.call_command() + with self.assertRaisesRegex(CommandError, 'Only one of --course or --all argument'): + self.call_command('--all', '--course', 'some-course') + + @patch( + 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' + ) + def test_call_for_single_course(self, mock_task): + """ + Test command with single course argument + """ + self.call_command('--course', 'some-course') + mock_task.delay.assert_called_with('some-course', False) + # call with --force + self.call_command('--course', 'some-course', '--force') + mock_task.delay.assert_called_with('some-course', True) + + @patch( + 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' + ) + def test_call_for_multiple_course(self, mock_task): + """ + Test command with multiple course arguments + """ + self.call_command('--course', 'some-course', '--course', 'one-more-course') + mock_task.delay.assert_any_call('some-course', False) + mock_task.delay.assert_any_call('one-more-course', False) + + @patch( + 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' + ) + def test_call_for_all_courses(self, mock_task): + """ + Test command with multiple course arguments + """ + course_key_1 = CourseFactory.create(emit_signals=True).id + course_key_2 = CourseFactory.create(emit_signals=True).id + self.call_command('--all') + mock_task.delay.assert_any_call(str(course_key_1), False) + mock_task.delay.assert_any_call(str(course_key_2), False) From debba3ba8447de0b898714c5cb95ec4f0a724b12 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 15 Jan 2025 19:39:02 +0530 Subject: [PATCH 03/13] test: upstream - downstream link tasks --- .../tests/test_upstream_downstream_links.py | 87 ++++++++++++++++++- 1 file changed, 84 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py index a3315c6df42f..643226264f9b 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py @@ -7,13 +7,16 @@ from django.core.management import call_command from django.core.management.base import CommandError +from django.utils import timezone +from freezegun import freeze_time -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory +from ..tasks import create_or_update_upstream_links, create_or_update_xblock_upstream_link -class TestRecreateUpstreamLinks(ModuleStoreTestCase, CacheIsolationTestCase): + +class TestRecreateUpstreamLinks(ModuleStoreTestCase): """ Test recreate_upstream_links management command. """ @@ -74,3 +77,81 @@ def test_call_for_all_courses(self, mock_task): self.call_command('--all') mock_task.delay.assert_any_call(str(course_key_1), False) mock_task.delay.assert_any_call(str(course_key_2), False) + + +class TestUpstreamLinksTasks(ModuleStoreTestCase): + """ + Test tasks related to managing upstream->downstream links. + """ + + ENABLED_SIGNALS = ['course_deleted', 'course_published'] + + def setUp(self): + super().setUp() + self.now = timezone.now() + freezer = freeze_time(self.now) + freezer.start() + self.addCleanup(freezer.stop) + self.course = course = CourseFactory.create(emit_signals=True) + self.course_key = course_key = self.course.id + self.upstream_1 = "upstream-block-id-1" + self.upstream_2 = "upstream-block-id-2" + with self.store.bulk_operations(course_key): + self.section = BlockFactory.create(parent=course, category="chapter", display_name="Section") + self.sequence = BlockFactory.create(parent=self.section, category="sequential", display_name="Sequence") + self.unit = BlockFactory.create(parent=self.sequence, category="vertical", display_name="Unit") + self.component_1 = BlockFactory.create( + parent=self.unit, + category="html", + display_name="An HTML Block", + upstream=self.upstream_1, + ) + self.component_2 = BlockFactory.create( + parent=self.unit, + category="html", + display_name="Another HTML Block", + upstream=self.upstream_2, + ) + self.component_3 = BlockFactory.create( + parent=self.unit, + category="html", + display_name="Another another HTML Block", + ) + + @patch( + 'openedx.core.djangoapps.content_libraries.api.create_or_update_xblock_upstream_link' + ) + def test_create_or_update_upstream_links_task(self, mock_api): + """ + Test task create_or_update_upstream_links for a course + """ + create_or_update_upstream_links(str(self.course_key), force=False) + expected_calls = [ + (self.component_1.usage_key, str(self.course_key), self.course.display_name_with_default, self.now), + (self.component_2.usage_key, str(self.course_key), self.course.display_name_with_default, self.now), + ] + assert [(x[0][0].usage_key, x[0][1], x[0][2], x[0][3]) for x in mock_api.call_args_list] == expected_calls + mock_api.reset_mock() + # call again with same course, it should not be processed again as its CourseLinksStatusChoices = COMPLETED + create_or_update_upstream_links(str(self.course_key), force=False) + mock_api.assert_not_called() + # again with same course but with force=True, it should be processed now + create_or_update_upstream_links(str(self.course_key), force=True) + assert [(x[0][0].usage_key, x[0][1], x[0][2], x[0][3]) for x in mock_api.call_args_list] == expected_calls + + @patch( + 'openedx.core.djangoapps.content_libraries.api.create_or_update_xblock_upstream_link' + ) + def test_create_or_update_xblock_upstream_link(self, mock_api): + """ + Test task create_or_update_xblock_upstream_link for a course + """ + create_or_update_xblock_upstream_link(str(self.component_1.usage_key)) + expected_calls = [ + (self.component_1.usage_key, str(self.course_key), self.course.display_name_with_default), + ] + assert [(x[0][0].usage_key, x[0][1], x[0][2]) for x in mock_api.call_args_list] == expected_calls + mock_api.reset_mock() + # call for xblock with no upstream + create_or_update_xblock_upstream_link(str(self.component_3.usage_key)) + mock_api.assert_not_called() From a30b00e212d01dc3bc9fae3b9230a669dbaf2874 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 15 Jan 2025 20:16:52 +0530 Subject: [PATCH 04/13] chore: fix lint issues --- .../management/commands/recreate_upstream_links.py | 4 ++-- openedx/core/djangoapps/content_libraries/tasks.py | 8 ++++++-- .../tests/test_upstream_downstream_links.py | 6 +++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py b/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py index 08ceec2b0d07..2d88c26c3017 100644 --- a/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py +++ b/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py @@ -62,7 +62,7 @@ def handle(self, *args, **options): courses = options['course'] should_process_all = options['all'] force = options['force'] - self.time_now = datetime.now(tz=timezone.utc) + time_now = datetime.now(tz=timezone.utc) if not courses and not should_process_all: raise CommandError('Either --course or --all argument should be provided.') @@ -73,4 +73,4 @@ def handle(self, *args, **options): courses = CourseOverview.get_all_course_keys() for course in courses: log.info(f"Start processing upstream->dowstream links in course: {course}") - create_or_update_upstream_links.delay(str(course), force) + create_or_update_upstream_links.delay(str(course), force, created=time_now) diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index 0cedc2ba2a46..b67411c1f2a7 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -177,6 +177,9 @@ def duplicate_children( @shared_task(base=LoggedTask) @set_code_owner_attribute def create_or_update_xblock_upstream_link(usage_key): + """ + Create or update upstream link for a single xblock. + """ ensure_cms("create_or_update_xblock_upstream_link may only be executed in a CMS context") xblock = modulestore().get_item(UsageKey.from_string(usage_key)) if not xblock.upstream: @@ -192,13 +195,14 @@ def create_or_update_xblock_upstream_link(usage_key): @shared_task(base=LoggedTask) @set_code_owner_attribute -def create_or_update_upstream_links(course_key_str: str, force: bool = False): +def create_or_update_upstream_links(course_key_str: str, force: bool = False, created: datetime | None = None): """ A Celery task to create or update upstream downstream links in database from course xblock content. """ ensure_cms("create_or_update_upstream_links may only be executed in a CMS context") - created = datetime.now(timezone.utc) + if not created: + created = datetime.now(timezone.utc) course_status = get_or_create_course_link_status(course_key_str, created) if course_status.status in [ CourseLinksStatusChoices.COMPLETED, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py index 643226264f9b..19734eda092d 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py @@ -42,7 +42,7 @@ def test_call_with_invalid_args(self): self.call_command('--all', '--course', 'some-course') @patch( - 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' + 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' # pylint: disable=line-too-long ) def test_call_for_single_course(self, mock_task): """ @@ -55,7 +55,7 @@ def test_call_for_single_course(self, mock_task): mock_task.delay.assert_called_with('some-course', True) @patch( - 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' + 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' # pylint: disable=line-too-long ) def test_call_for_multiple_course(self, mock_task): """ @@ -66,7 +66,7 @@ def test_call_for_multiple_course(self, mock_task): mock_task.delay.assert_any_call('one-more-course', False) @patch( - 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' + 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' # pylint: disable=line-too-long ) def test_call_for_all_courses(self, mock_task): """ From ccb9f3dfe9584f51019a1c4ef60af30ab040dbf9 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 16 Jan 2025 11:16:25 +0530 Subject: [PATCH 05/13] refactor: fix import issues --- .../content/course_overviews/signals.py | 23 +++++++++++-------- .../content_libraries/signal_handlers.py | 15 +++++++++++- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/openedx/core/djangoapps/content/course_overviews/signals.py b/openedx/core/djangoapps/content/course_overviews/signals.py index 93fcbe6b3603..4de13bcf88d7 100644 --- a/openedx/core/djangoapps/content/course_overviews/signals.py +++ b/openedx/core/djangoapps/content/course_overviews/signals.py @@ -10,7 +10,6 @@ from django.dispatch import Signal from django.dispatch.dispatcher import receiver -from openedx.core.djangoapps.content_libraries.tasks import update_course_name_in_upstream_links from openedx.core.djangoapps.signals.signals import COURSE_CERT_DATE_CHANGE from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.django import SignalHandler @@ -27,6 +26,8 @@ IMPORT_COURSE_DETAILS = Signal() # providing_args=["courserun_key"] DELETE_COURSE_DETAILS = Signal() +# providing_args=["courserun_key", "old_name", "new_name"] +COURSE_NAME_CHANGED = Signal() @receiver(SignalHandler.course_published) @@ -222,12 +223,16 @@ def _send_course_cert_date_change_signal(): def _check_for_display_name_change(previous_course_overview, updated_course_overview): """ - Checks for change in display name of course and triggers task to update course name in upstream->downstream entity - links table. + Checks for change in display name of course and sends COURSE_NAME_CHANGED signal. """ - if previous_course_overview.display_name_with_default == updated_course_overview.display_name_with_default: - return - update_course_name_in_upstream_links.delay( - str(previous_course_overview.id), - updated_course_overview.display_name_with_default - ) + if previous_course_overview.display_name_with_default != updated_course_overview.display_name_with_default: + COURSE_NAME_CHANGED.send( + sender=None, + courserun_key=str(previous_course_overview.id), + old_name=previous_course_overview.display_name_with_default, + new_name=updated_course_overview.display_name_with_default, + ) + # update_course_name_in_upstream_links.delay( + # str(previous_course_overview.id), + # updated_course_overview.display_name_with_default + # ) diff --git a/openedx/core/djangoapps/content_libraries/signal_handlers.py b/openedx/core/djangoapps/content_libraries/signal_handlers.py index 95ae965d0b2a..a16ca60d42a1 100644 --- a/openedx/core/djangoapps/content_libraries/signal_handlers.py +++ b/openedx/core/djangoapps/content_libraries/signal_handlers.py @@ -28,10 +28,11 @@ from openedx_learning.api.authoring_models import Collection, CollectionPublishableEntity, Component, PublishableEntity from lms.djangoapps.grades.api import signals as grades_signals +from openedx.core.djangoapps.content.course_overviews.signals import COURSE_NAME_CHANGED from .api import library_component_usage_key from .models import ContentLibrary, LtiGradedResource -from .tasks import create_or_update_xblock_upstream_link +from .tasks import create_or_update_xblock_upstream_link, update_course_name_in_upstream_links log = logging.getLogger(__name__) @@ -234,3 +235,15 @@ def delete_upstream_downstream_link_handler(**kwargs): return delete_entity_link(str(xblock_info.usage_key)) + + +@receiver(COURSE_NAME_CHANGED, dispatch_uid="update_course_name_in_upstream_links_handler") +def update_course_name_in_upstream_links_handler(courserun_key, old_name, new_name, **kwargs): + """ + Handler to update course names in upstream->downstream links on change. + """ + log.info(f"Updating course name in upstream->downstream links from '{old_name}' to '{new_name}'") + update_course_name_in_upstream_links.delay( + courserun_key, + new_name, + ) From 960a439b2fb1584574b0d8ca13d171a41227190d Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 16 Jan 2025 11:29:50 +0530 Subject: [PATCH 06/13] temp: point openedx-learning to dev branch --- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index f99c41c6947b..f6c004476373 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -131,7 +131,7 @@ optimizely-sdk<5.0 # Date: 2023-09-18 # pinning this version to avoid updates while the library is being developed # Issue for unpinning: https://github.com/openedx/edx-platform/issues/35269 -openedx-learning==0.18.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/link-table # Date: 2023-11-29 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 0587503e31f4..cfde49e06828 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -830,7 +830,7 @@ openedx-filters==1.12.0 # ora2 openedx-forum==0.1.6 # via -r requirements/edx/kernel.in -openedx-learning==0.18.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/link-table # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index c6b76cdde3b1..c2e1497b46ba 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1392,7 +1392,7 @@ openedx-forum==0.1.6 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-learning==0.18.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/link-table # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index fe57b36c59be..9f76e47e09a5 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -1007,7 +1007,7 @@ openedx-filters==1.12.0 # ora2 openedx-forum==0.1.6 # via -r requirements/edx/base.txt -openedx-learning==0.18.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/link-table # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index ce5880269c04..684a0efbe9c0 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1054,7 +1054,7 @@ openedx-filters==1.12.0 # ora2 openedx-forum==0.1.6 # via -r requirements/edx/base.txt -openedx-learning==0.18.1 +openedx-learning @ git+https://github.com/open-craft/openedx-learning@navin/link-table # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From 6c52c9d6af68f708966222e0bb0a256d1ec0ba33 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 16 Jan 2025 12:30:45 +0530 Subject: [PATCH 07/13] refactor: check for upstream_version --- openedx/core/djangoapps/content_libraries/tasks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index b67411c1f2a7..648960051c05 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -182,8 +182,8 @@ def create_or_update_xblock_upstream_link(usage_key): """ ensure_cms("create_or_update_xblock_upstream_link may only be executed in a CMS context") xblock = modulestore().get_item(UsageKey.from_string(usage_key)) - if not xblock.upstream: - TASK_LOGGER.info(f"No upstream found for xblock: {xblock.usage_key}") + if not xblock.upstream or not xblock.upstream_version: + TASK_LOGGER.info(f"No upstream or upstream_version found for xblock: {xblock.usage_key}") return try: course_name = CourseOverview.get_from_id(xblock.course_id).display_name_with_default From d5f23ec5311893132eea21daf16d2d3c1520ba6b Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 16 Jan 2025 12:44:04 +0530 Subject: [PATCH 08/13] test: fix failing tests --- .../tests/test_upstream_downstream_links.py | 24 ++++++++++++++----- .../tests/test_mixed_modulestore.py | 13 +++++++--- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py index 19734eda092d..3787d3cedcb3 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py @@ -10,12 +10,14 @@ from django.utils import timezone from freezegun import freeze_time +from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory from ..tasks import create_or_update_upstream_links, create_or_update_xblock_upstream_link +@skip_unless_cms class TestRecreateUpstreamLinks(ModuleStoreTestCase): """ Test recreate_upstream_links management command. @@ -23,6 +25,13 @@ class TestRecreateUpstreamLinks(ModuleStoreTestCase): ENABLED_SIGNALS = ['course_deleted', 'course_published'] + def setUp(self): + super().setUp() + self.now = timezone.now() + freezer = freeze_time(self.now) + freezer.start() + self.addCleanup(freezer.stop) + def call_command(self, *args, **kwargs): """ call command with pass args. @@ -49,10 +58,10 @@ def test_call_for_single_course(self, mock_task): Test command with single course argument """ self.call_command('--course', 'some-course') - mock_task.delay.assert_called_with('some-course', False) + mock_task.delay.assert_called_with('some-course', False, created=self.now) # call with --force self.call_command('--course', 'some-course', '--force') - mock_task.delay.assert_called_with('some-course', True) + mock_task.delay.assert_called_with('some-course', True, created=self.now) @patch( 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' # pylint: disable=line-too-long @@ -62,8 +71,8 @@ def test_call_for_multiple_course(self, mock_task): Test command with multiple course arguments """ self.call_command('--course', 'some-course', '--course', 'one-more-course') - mock_task.delay.assert_any_call('some-course', False) - mock_task.delay.assert_any_call('one-more-course', False) + mock_task.delay.assert_any_call('some-course', False, created=self.now) + mock_task.delay.assert_any_call('one-more-course', False, created=self.now) @patch( 'openedx.core.djangoapps.content_libraries.management.commands.recreate_upstream_links.create_or_update_upstream_links' # pylint: disable=line-too-long @@ -75,10 +84,11 @@ def test_call_for_all_courses(self, mock_task): course_key_1 = CourseFactory.create(emit_signals=True).id course_key_2 = CourseFactory.create(emit_signals=True).id self.call_command('--all') - mock_task.delay.assert_any_call(str(course_key_1), False) - mock_task.delay.assert_any_call(str(course_key_2), False) + mock_task.delay.assert_any_call(str(course_key_1), False, created=self.now) + mock_task.delay.assert_any_call(str(course_key_2), False, created=self.now) +@skip_unless_cms class TestUpstreamLinksTasks(ModuleStoreTestCase): """ Test tasks related to managing upstream->downstream links. @@ -105,12 +115,14 @@ def setUp(self): category="html", display_name="An HTML Block", upstream=self.upstream_1, + upstream_version=1, ) self.component_2 = BlockFactory.create( parent=self.unit, category="html", display_name="Another HTML Block", upstream=self.upstream_2, + upstream_version=1, ) self.component_3 = BlockFactory.create( parent=self.unit, diff --git a/xmodule/modulestore/tests/test_mixed_modulestore.py b/xmodule/modulestore/tests/test_mixed_modulestore.py index 0928ab253b9c..4d335ceaa554 100644 --- a/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -164,6 +164,13 @@ def setUp(self): self.course_locations = {} self.user_id = ModuleStoreEnum.UserID.test + # mock and ignore create_or_update_xblock_upstream_link task to avoid unnecessary + # errors as it is tested separately + create_or_update_xblock_upstream_link_patch = patch( + 'openedx.core.djangoapps.content_libraries.signal_handlers.create_or_update_xblock_upstream_link' + ) + create_or_update_xblock_upstream_link_patch.start() + self.addCleanup(create_or_update_xblock_upstream_link_patch.stop) def _check_connection(self): """ @@ -1099,7 +1106,7 @@ def test_has_changes_missing_child(self, default_ms, default_branch): # check CONTENT_TAGGING_AUTO CourseWaffleFlag # Find: active_versions, 2 structures (published & draft), definition (unnecessary) # Sends: updated draft and published structures and active_versions - @ddt.data((ModuleStoreEnum.Type.split, 5, 2, 3)) + @ddt.data((ModuleStoreEnum.Type.split, 6, 2, 3)) @ddt.unpack def test_delete_item(self, default_ms, num_mysql, max_find, max_send): """ @@ -1122,7 +1129,7 @@ def test_delete_item(self, default_ms, num_mysql, max_find, max_send): # check CONTENT_TAGGING_AUTO CourseWaffleFlag # find: draft and published structures, definition (unnecessary) # sends: update published (why?), draft, and active_versions - @ddt.data((ModuleStoreEnum.Type.split, 5, 3, 3)) + @ddt.data((ModuleStoreEnum.Type.split, 6, 3, 3)) @ddt.unpack def test_delete_private_vertical(self, default_ms, num_mysql, max_find, max_send): """ @@ -1172,7 +1179,7 @@ def test_delete_private_vertical(self, default_ms, num_mysql, max_find, max_send # check CONTENT_TAGGING_AUTO CourseWaffleFlag # find: structure (cached) # send: update structure and active_versions - @ddt.data((ModuleStoreEnum.Type.split, 5, 1, 2)) + @ddt.data((ModuleStoreEnum.Type.split, 6, 1, 2)) @ddt.unpack def test_delete_draft_vertical(self, default_ms, num_mysql, max_find, max_send): """ From c825d39de30b2da4cf53b8cc51d2499714d443c2 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 17 Jan 2025 11:57:52 +0530 Subject: [PATCH 09/13] feat: create upstream links after course import --- xmodule/modulestore/xml_importer.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/xmodule/modulestore/xml_importer.py b/xmodule/modulestore/xml_importer.py index 5b880b4ade2f..a1546f54a71a 100644 --- a/xmodule/modulestore/xml_importer.py +++ b/xmodule/modulestore/xml_importer.py @@ -40,6 +40,8 @@ from xblock.runtime import DictKeyValueStore, KvsFieldData from common.djangoapps.util.monitoring import monitor_import_failure +from openedx.core.djangoapps.content_libraries.tasks import create_or_update_upstream_links +from openedx.core.djangoapps.content_tagging.api import import_course_tags_from_csv from xmodule.assetstore import AssetMetadata from xmodule.contentstore.content import StaticContent from xmodule.errortracker import make_error_tracker @@ -52,7 +54,6 @@ from xmodule.tabs import CourseTabList from xmodule.util.misc import escape_invalid_characters from xmodule.x_module import XModuleMixin -from openedx.core.djangoapps.content_tagging.api import import_course_tags_from_csv from .inheritance import own_metadata from .store_utilities import rewrite_nonportable_content_links @@ -548,6 +549,11 @@ def depth_first(subtree): # pylint: disable=raise-missing-from raise BlockFailedToImport(leftover.display_name, leftover.location) + def post_course_import(self, dest_id): + """ + Tasks that need to triggered after a course is imported. + """ + def run_imports(self): """ Iterate over the given directories and yield courses. @@ -589,6 +595,7 @@ def run_imports(self): logging.info(f'Course import {dest_id}: No tags.csv file present.') except ValueError as e: logging.info(f'Course import {dest_id}: {str(e)}') + self.post_course_import(dest_id) yield courselike @@ -717,6 +724,12 @@ def import_tags(self, data_path, dest_id): csv_path = path(data_path) / 'tags.csv' import_course_tags_from_csv(csv_path, dest_id) + def post_course_import(self, dest_id): + """ + Trigger celery task to create upstream links for newly imported blocks. + """ + create_or_update_upstream_links.delay(str(dest_id)) + class LibraryImportManager(ImportManager): """ From b93564a66c550fa2e07d889608925030b5d9e96b Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 17 Jan 2025 20:05:42 +0530 Subject: [PATCH 10/13] refactor: rename learning classes --- openedx/core/djangoapps/content_libraries/api.py | 1 + openedx/core/djangoapps/content_libraries/tasks.py | 14 +++++++------- .../tests/test_upstream_downstream_links.py | 3 ++- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 0ed68c36a72a..8cd98f3cdd6a 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1970,6 +1970,7 @@ def create_or_update_xblock_upstream_link(xblock, course_key: str, course_name: authoring_api.update_or_create_entity_link( lib_component, upstream_usage_key=xblock.upstream, + upstream_context_key=str(upstream_usage_key.context_key), downstream_context_key=course_key, downstream_context_title=course_name, downstream_usage_key=str(xblock.usage_key), diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index 648960051c05..ffb088f611b5 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -32,8 +32,8 @@ from opaque_keys.edx.locator import BlockUsageLocator from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib import ensure_cms -from openedx_learning.api.authoring import get_entity_links, get_or_create_course_link_status -from openedx_learning.api.authoring_models import CourseLinksStatusChoices +from openedx_learning.api.authoring import get_entity_links, get_or_create_learning_context_link_status +from openedx_learning.api.authoring_models import LearningContextLinksStatusChoices from xmodule.capa_block import ProblemBlock from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LegacyLibraryContentBlock from xmodule.modulestore import ModuleStoreEnum @@ -203,15 +203,15 @@ def create_or_update_upstream_links(course_key_str: str, force: bool = False, cr if not created: created = datetime.now(timezone.utc) - course_status = get_or_create_course_link_status(course_key_str, created) + course_status = get_or_create_learning_context_link_status(course_key_str, created) if course_status.status in [ - CourseLinksStatusChoices.COMPLETED, - CourseLinksStatusChoices.PROCESSING + LearningContextLinksStatusChoices.COMPLETED, + LearningContextLinksStatusChoices.PROCESSING ] and not force: return store = modulestore() course_key = CourseKey.from_string(course_key_str) - course_status.status = CourseLinksStatusChoices.PROCESSING + course_status.status = LearningContextLinksStatusChoices.PROCESSING course_status.save() try: course_name = CourseOverview.get_from_id(course_key).display_name_with_default @@ -221,7 +221,7 @@ def create_or_update_upstream_links(course_key_str: str, force: bool = False, cr xblocks = store.get_items(course_key, settings={"upstream": lambda x: x is not None}) for xblock in xblocks: api.create_or_update_xblock_upstream_link(xblock, course_key_str, course_name, created) - course_status.status = CourseLinksStatusChoices.COMPLETED + course_status.status = LearningContextLinksStatusChoices.COMPLETED course_status.save() diff --git a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py index 3787d3cedcb3..7215c8576c8d 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py @@ -144,7 +144,8 @@ def test_create_or_update_upstream_links_task(self, mock_api): ] assert [(x[0][0].usage_key, x[0][1], x[0][2], x[0][3]) for x in mock_api.call_args_list] == expected_calls mock_api.reset_mock() - # call again with same course, it should not be processed again as its CourseLinksStatusChoices = COMPLETED + # call again with same course, it should not be processed again + # as its LearningContextLinksStatusChoices = COMPLETED create_or_update_upstream_links(str(self.course_key), force=False) mock_api.assert_not_called() # again with same course but with force=True, it should be processed now From 9f7891ca036d63c47f2e4097c59aa1fbf8ef8f37 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 17 Jan 2025 20:22:44 +0530 Subject: [PATCH 11/13] refactor: apply review suggestions --- openedx/core/djangoapps/content/course_overviews/signals.py | 4 ---- openedx/core/djangoapps/content_libraries/api.py | 3 +-- openedx/core/djangoapps/content_libraries/tasks.py | 1 - 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/content/course_overviews/signals.py b/openedx/core/djangoapps/content/course_overviews/signals.py index 4de13bcf88d7..5412e3ebc013 100644 --- a/openedx/core/djangoapps/content/course_overviews/signals.py +++ b/openedx/core/djangoapps/content/course_overviews/signals.py @@ -232,7 +232,3 @@ def _check_for_display_name_change(previous_course_overview, updated_course_over old_name=previous_course_overview.display_name_with_default, new_name=updated_course_overview.display_name_with_default, ) - # update_course_name_in_upstream_links.delay( - # str(previous_course_overview.id), - # updated_course_overview.display_name_with_default - # ) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 8cd98f3cdd6a..23299a22d53c 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1959,13 +1959,12 @@ def create_or_update_xblock_upstream_link(xblock, course_key: str, course_name: Create or update upstream->downstream link in database for given xblock. """ if not xblock.upstream: - log.info(f"No upstream found for xblock: {xblock.usage_key}") return None upstream_usage_key = UsageKeyV2.from_string(xblock.upstream) try: lib_component = get_component_from_usage_key(upstream_usage_key) except ObjectDoesNotExist: - log.exception("Library block not found!") + log.error(f"Library component not found for {upstream_usage_key}") lib_component = None authoring_api.update_or_create_entity_link( lib_component, diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index ffb088f611b5..55a2329681e0 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -183,7 +183,6 @@ def create_or_update_xblock_upstream_link(usage_key): ensure_cms("create_or_update_xblock_upstream_link may only be executed in a CMS context") xblock = modulestore().get_item(UsageKey.from_string(usage_key)) if not xblock.upstream or not xblock.upstream_version: - TASK_LOGGER.info(f"No upstream or upstream_version found for xblock: {xblock.usage_key}") return try: course_name = CourseOverview.get_from_id(xblock.course_id).display_name_with_default From 259b80c916b50124c5a8ffd89973b5f41113093a Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 20 Jan 2025 10:26:44 +0530 Subject: [PATCH 12/13] refactor: save failed status of course links --- .../djangoapps/content_libraries/tasks.py | 23 ++++++++++++++---- .../tests/test_upstream_downstream_links.py | 24 +++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index 55a2329681e0..fef25f039f46 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -32,7 +32,11 @@ from opaque_keys.edx.locator import BlockUsageLocator from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib import ensure_cms -from openedx_learning.api.authoring import get_entity_links, get_or_create_learning_context_link_status +from openedx_learning.api.authoring import ( + get_entity_links, + get_or_create_learning_context_link_status, + update_learning_context_link_status, +) from openedx_learning.api.authoring_models import LearningContextLinksStatusChoices from xmodule.capa_block import ProblemBlock from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LegacyLibraryContentBlock @@ -210,18 +214,27 @@ def create_or_update_upstream_links(course_key_str: str, force: bool = False, cr return store = modulestore() course_key = CourseKey.from_string(course_key_str) - course_status.status = LearningContextLinksStatusChoices.PROCESSING - course_status.save() + update_learning_context_link_status( + context_key=course_key_str, + status=LearningContextLinksStatusChoices.PROCESSING, + updated=created, + ) try: course_name = CourseOverview.get_from_id(course_key).display_name_with_default except CourseOverview.DoesNotExist: TASK_LOGGER.exception(f'Could not find course: {course_key_str}') + update_learning_context_link_status( + context_key=course_key_str, + status=LearningContextLinksStatusChoices.FAILED, + ) return xblocks = store.get_items(course_key, settings={"upstream": lambda x: x is not None}) for xblock in xblocks: api.create_or_update_xblock_upstream_link(xblock, course_key_str, course_name, created) - course_status.status = LearningContextLinksStatusChoices.COMPLETED - course_status.save() + update_learning_context_link_status( + context_key=course_key_str, + status=LearningContextLinksStatusChoices.COMPLETED, + ) @shared_task(base=LoggedTask) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py index 7215c8576c8d..96a9dc311b33 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_upstream_downstream_links.py @@ -10,6 +10,8 @@ from django.utils import timezone from freezegun import freeze_time +from openedx_learning.api.authoring_models import LearningContextLinksStatus, LearningContextLinksStatusChoices + from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory @@ -137,12 +139,18 @@ def test_create_or_update_upstream_links_task(self, mock_api): """ Test task create_or_update_upstream_links for a course """ + assert not LearningContextLinksStatus.objects.filter(context_key=str(self.course_key)).exists() create_or_update_upstream_links(str(self.course_key), force=False) expected_calls = [ (self.component_1.usage_key, str(self.course_key), self.course.display_name_with_default, self.now), (self.component_2.usage_key, str(self.course_key), self.course.display_name_with_default, self.now), ] assert [(x[0][0].usage_key, x[0][1], x[0][2], x[0][3]) for x in mock_api.call_args_list] == expected_calls + assert LearningContextLinksStatus.objects.filter(context_key=str(self.course_key)).exists() + assert LearningContextLinksStatus.objects.filter( + context_key=str(self.course_key) + ).first().status == LearningContextLinksStatusChoices.COMPLETED + mock_api.reset_mock() # call again with same course, it should not be processed again # as its LearningContextLinksStatusChoices = COMPLETED @@ -168,3 +176,19 @@ def test_create_or_update_xblock_upstream_link(self, mock_api): # call for xblock with no upstream create_or_update_xblock_upstream_link(str(self.component_3.usage_key)) mock_api.assert_not_called() + + @patch( + 'openedx.core.djangoapps.content_libraries.api.create_or_update_xblock_upstream_link' + ) + def test_create_or_update_upstream_links_task_for_invalid_course(self, mock_api): + """ + Test task create_or_update_upstream_links for an invalid course key. + """ + course_key = "course-v1:non+existent+course" + assert not LearningContextLinksStatus.objects.filter(context_key=course_key).exists() + create_or_update_upstream_links(course_key, force=False) + mock_api.assert_not_called() + assert LearningContextLinksStatus.objects.filter(context_key=course_key).exists() + assert LearningContextLinksStatus.objects.filter( + context_key=course_key + ).first().status == LearningContextLinksStatusChoices.FAILED From d3fbc1ec276f13cd7509b3ea41769e833d9fa893 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 21 Jan 2025 10:05:36 +0530 Subject: [PATCH 13/13] docs: update docs --- .../management/commands/recreate_upstream_links.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py b/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py index 2d88c26c3017..b8e0ea7d6329 100644 --- a/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py +++ b/openedx/core/djangoapps/content_libraries/management/commands/recreate_upstream_links.py @@ -27,10 +27,13 @@ class Command(BaseCommand): # Recreate upstream links for two courses. $ ./manage.py cms recreate_upstream_links --course course-v1:edX+DemoX.1+2014 \ --course course-v1:edX+DemoX.2+2015 + # Force recreate upstream links for one or more courses including processed ones. + $ ./manage.py cms recreate_upstream_links --course course-v1:edX+DemoX.1+2014 \ + --course course-v1:edX+DemoX.2+2015 --force # Recreate upstream links for all courses. $ ./manage.py cms recreate_upstream_links --all # Force recreate links for all courses including completely processed ones. - $ ./manage.py cms recreate_upstream_links --all + $ ./manage.py cms recreate_upstream_links --all --force """ def add_arguments(self, parser):