Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9573a5c
fix: search index wasn't updated properly on "Publish All Changes" in…
bradenmacdonald Apr 30, 2025
f88ce1a
test: update tests
bradenmacdonald May 2, 2025
4701a25
fix: flaky test
bradenmacdonald May 2, 2025
6d72525
fix: revert some previous changes
bradenmacdonald May 6, 2025
1bb4fb1
fix: make events more focused
bradenmacdonald May 6, 2025
d8c6a2c
test: more comprehensive tests for events
bradenmacdonald May 6, 2025
f9ca9ed
test: remove redundant tests
bradenmacdonald May 6, 2025
3301645
chore: address lint warnings
bradenmacdonald May 6, 2025
a510557
fix: "[created] received a naive datetime"
bradenmacdonald May 6, 2025
a13bfaa
fix: handle publishing a single component consistently
bradenmacdonald May 6, 2025
19fca02
feat: distinguish PUBLISHED events from other draft CRUD events
bradenmacdonald May 6, 2025
c2d81ad
fix: simplify how we send single-component publish events
bradenmacdonald May 6, 2025
1ed7044
test: more comprehensive tests for events
bradenmacdonald May 6, 2025
cc1ae1e
test: more comprehensive tests for events
bradenmacdonald May 6, 2025
53fbc05
feat: update search handlers for new PUBLISHED events
bradenmacdonald May 6, 2025
47ea4cd
temp: bump version of openedx-events
bradenmacdonald May 6, 2025
345ce52
test: remove unneeded call to enable events isolation
bradenmacdonald May 6, 2025
b69c59e
test: lol, belay that order
bradenmacdonald May 6, 2025
701fd79
feat: Add ready_to_sync field to ContainerLink model
ChrisChV May 6, 2025
ee01108
feat: Create handler to update ready_to_sync in ContainerLink
ChrisChV May 8, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 4.2.20 on 2025-05-06 23:50

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('contentstore', '0010_container_link_models'),
]

operations = [
migrations.AddField(
model_name='containerlink',
name='ready_to_sync',
field=models.BooleanField(default=False, help_text='True if the downstream has an available sync from upstream. It changes to True when the upstream is published, and it changes to False when the sync is declined/accepted. The version number is not used to check this value because containers do not change version if a component is edited/deleted within them. '),
),
]
39 changes: 21 additions & 18 deletions cms/djangoapps/contentstore/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,16 @@ class ContainerLink(EntityLinkBase):
"or were deleted."
)
)
ready_to_sync = models.BooleanField(
default=False,
help_text=_(
"True if the downstream has an available sync from upstream. "
"It changes to True when the upstream is published, and it changes to False "
"when the sync is declined/accepted. "
"The version number is not used to check this value because containers do not change "
"version if a component is edited/deleted within them. "
),
)

class Meta:
verbose_name = _("Container Link")
Expand Down Expand Up @@ -303,24 +313,15 @@ def filter_links(
"""
Get all links along with sync flag, upstream context title and version, with optional filtering.
"""
ready_to_sync = link_filter.pop('ready_to_sync', None)
result = cls.objects.filter(**link_filter).select_related(
"upstream_container__publishable_entity__published__version",
"upstream_container__publishable_entity__learning_package"
).annotate(
ready_to_sync=(
GreaterThan(
Coalesce("upstream_container__publishable_entity__published__version__version_num", 0),
Coalesce("version_synced", 0)
) & GreaterThan(
Coalesce("upstream_container__publishable_entity__published__version__version_num", 0),
Coalesce("version_declined", 0)
)
)
)
if ready_to_sync is not None:
result = result.filter(ready_to_sync=ready_to_sync)
return result

@classmethod
def get_link(cls, downstream_key) -> "EntityLinkBase":
return cls.objects.get(downstream_usage_key=downstream_key)

@classmethod
def summarize_by_downstream_context(cls, downstream_context_key: CourseKey) -> QuerySet:
Expand Down Expand Up @@ -356,12 +357,13 @@ def update_or_create(
cls,
upstream_container: Container | None,
/,
upstream_container_key: LibraryContainerLocator,
upstream_context_key: str,
downstream_usage_key: UsageKey,
downstream_context_key: CourseKey,
version_synced: int,
upstream_container_key: LibraryContainerLocator | None = None,
upstream_context_key: str | None = None,
downstream_context_key: CourseKey | None = None,
version_synced: int | None = None,
version_declined: int | None = None,
ready_to_sync: bool | None = None,
created: datetime | None = None,
) -> "ContainerLink":
"""
Expand All @@ -376,6 +378,7 @@ def update_or_create(
'downstream_context_key': downstream_context_key,
'version_synced': version_synced,
'version_declined': version_declined,
'ready_to_sync': ready_to_sync,
}
if upstream_container:
new_values['upstream_container'] = upstream_container
Expand All @@ -384,7 +387,7 @@ def update_or_create(
has_changes = False
for key, new_value in new_values.items():
prev_value = getattr(link, key)
if prev_value != new_value:
if new_value is not None and prev_value != new_value:
has_changes = True
setattr(link, key, new_value)
if has_changes:
Expand Down
34 changes: 32 additions & 2 deletions cms/djangoapps/contentstore/signals/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,20 @@
from django.db import transaction
from django.dispatch import receiver
from edx_toggles.toggles import SettingToggle
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.keys import CourseKey, UsageKey
from openedx_events.content_authoring.data import (
CourseCatalogData,
CourseData,
CourseScheduleData,
LibraryBlockData,
XBlockData,
LibraryContainerData,
)
from openedx_events.content_authoring.signals import (
COURSE_CATALOG_INFO_CHANGED,
COURSE_IMPORT_COMPLETED,
LIBRARY_BLOCK_DELETED,
LIBRARY_CONTAINER_PUBLISHED,
XBLOCK_CREATED,
XBLOCK_DELETED,
XBLOCK_UPDATED,
Expand All @@ -40,6 +42,7 @@
from openedx.core.djangoapps.content.learning_sequences.api import key_supports_outlines
from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task
from openedx.core.lib.gating import api as gating_api
from openedx.core.djangoapps.content_libraries import api as lib_api
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import SignalHandler, modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
Expand Down Expand Up @@ -264,7 +267,6 @@ def create_or_update_upstream_downstream_link_handler(**kwargs):
if not xblock_info or not isinstance(xblock_info, XBlockData):
log.error("Received null or incorrect data for event")
return

handle_create_or_update_xblock_upstream_link.delay(str(xblock_info.usage_key))


Expand Down Expand Up @@ -314,3 +316,31 @@ def unlink_upstream_block_handler(**kwargs):
return

handle_unlink_upstream_block.delay(str(library_block.usage_key))


@receiver(LIBRARY_CONTAINER_PUBLISHED)
def library_container_published_handler(**kwargs) -> None:
"""
Handle publish a container. Mark all related links to ready to sync.
"""
library_container = kwargs.get("library_container", None)

if not library_container or not isinstance(library_container, LibraryContainerData): # pragma: no cover
log.error("Received null or incorrect data for event")
return
# The PUBLISHED event is sent for any change to the published version including deletes, so check if it exists:
try:
lib_api.get_container(library_container.container_key)
except lib_api.ContentLibraryContainerNotFound:
log.info(f"Observed published deletion of container {str(library_container.container_key)}.")
# The document should already have been deleted from the search index
# via the DELETED handler, so there's nothing to do now.
return

# Get all container links that has the published container as the upstream
link_filter: dict[str, CourseKey | UsageKey | bool] = {}
link_filter["upstream_container_key"] = library_container.container_key
links = ContainerLink.filter_links(**link_filter)

# Mark all links to ready to sync
links.update(ready_to_sync=True)
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG
from cms.djangoapps.contentstore.helpers import StaticFileNotices
from cms.djangoapps.contentstore.models import ContainerLink
from cms.djangoapps.models.settings.course_grading import CourseGradingModel
from cms.lib.ai_aside_summary_config import AiAsideSummaryConfig
from cms.lib.xblock.upstream_sync import BadUpstream, UpstreamLink
Expand Down Expand Up @@ -577,6 +578,14 @@ def sync_library_content(downstream: XBlock, request, store) -> StaticFileNotice
store.delete_item(child.usage_key, user_id=request.user.id)
downstream.children = children
store.update_item(downstream, request.user.id)
# Mark the container link as not ready to sync (synchronized)
# The other data is updated with the XBLOCK_UPDATED signal,
# see: ../signals/handlers.py
ContainerLink.update_or_create(
None,
ready_to_sync=False,
downstream_usage_key=downstream.usage_key
)
static_file_notices = concat_static_file_notices(notices)
return static_file_notices

Expand Down
19 changes: 16 additions & 3 deletions cms/lib/xblock/upstream_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class UpstreamLink:
version_synced: int | None # Version of the upstream to which the downstream was last synced.
version_available: int | None # Latest version of the upstream that's available, or None if it couldn't be loaded.
version_declined: int | None # Latest version which the user has declined to sync with, if any.
force_ready_to_sync: bool # Bypass the verification of versions to be ready to sync. This is used by containers.
error_message: str | None # If link is valid, None. Otherwise, a localized, human-friendly error message.

@property
Expand All @@ -89,9 +90,11 @@ def ready_to_sync(self) -> bool:
"""
return bool(
self.upstream_ref and
self.version_available and
self.version_available > (self.version_synced or 0) and
self.version_available > (self.version_declined or 0)
self.force_ready_to_sync or (
self.version_available and
self.version_available > (self.version_synced or 0) and
self.version_available > (self.version_declined or 0)
)
)

@property
Expand Down Expand Up @@ -141,6 +144,7 @@ def try_get_for_block(cls, downstream: XBlock, log_error: bool = True) -> t.Self
version_synced=getattr(downstream, "upstream_version", None),
version_available=None,
version_declined=None,
force_ready_to_sync=False,
error_message=str(exc),
)

Expand All @@ -159,6 +163,7 @@ def get_for_block(cls, downstream: XBlock) -> t.Self:
"""
# We import this here b/c UpstreamSyncMixin is used by cms/envs, which loads before the djangoapps are ready.
from openedx.core.djangoapps.content_libraries import api as lib_api
from cms.djangoapps.contentstore.models import ContainerLink

if not isinstance(downstream, UpstreamSyncMixin):
raise BadDownstream(_("Downstream is not an XBlock or is missing required UpstreamSyncMixin"))
Expand All @@ -177,6 +182,7 @@ def get_for_block(cls, downstream: XBlock) -> t.Self:
except InvalidKeyError as exc:
raise BadUpstream(_("Reference to linked library item is malformed")) from exc

force_ready_to_sync = False
if isinstance(upstream_key, LibraryUsageLocatorV2):
# The upstream is an XBlock
if downstream.has_children:
Expand All @@ -196,6 +202,12 @@ def get_for_block(cls, downstream: XBlock) -> t.Self:
container_meta = lib_api.get_container(upstream_key)
except lib_api.ContentLibraryContainerNotFound as exc:
raise BadUpstream(_("Linked upstream library container was not found in the system")) from exc
try:
link = ContainerLink.get_link(downstream.usage_key)
except ContainerLink.DoesNotExist as exec:
raise BadUpstream(_("Container link was not found in the system")) from exec

force_ready_to_sync = link.ready_to_sync
expected_downstream_block_type = container_meta.container_type.olx_tag
version_available = container_meta.published_version_num

Expand All @@ -220,6 +232,7 @@ def get_for_block(cls, downstream: XBlock) -> t.Self:
version_synced=downstream.upstream_version,
version_available=version_available,
version_declined=downstream.upstream_version_declined,
force_ready_to_sync=force_ready_to_sync,
error_message=None,
)

Expand Down
23 changes: 0 additions & 23 deletions openedx/core/djangoapps/content/search/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -653,29 +653,6 @@ def _delete_index_doc(doc_id) -> None:
_wait_for_meili_tasks(tasks)


def delete_all_draft_docs_for_library(library_key: LibraryLocatorV2) -> None:
"""
Deletes draft documents for the given XBlocks from the search index
"""
current_rebuild_index_name = _get_running_rebuild_index_name()
client = _get_meilisearch_client()
# Delete all documents where last_published is null i.e. never published before.
delete_filter = [
f'{Fields.context_key}="{library_key}"',
# This field should only be NULL or have a value, but we're also checking IS EMPTY just in case.
# Inner arrays are connected by an OR
[f'{Fields.last_published} IS EMPTY', f'{Fields.last_published} IS NULL'],
]

tasks = []
if current_rebuild_index_name:
# If there is a rebuild in progress, the documents will also be deleted from the new index.
tasks.append(client.index(current_rebuild_index_name).delete_documents(filter=delete_filter))
tasks.append(client.index(STUDIO_INDEX_NAME).delete_documents(filter=delete_filter))

_wait_for_meili_tasks(tasks)


def upsert_library_block_index_doc(usage_key: UsageKey) -> None:
"""
Creates or updates the document for the given Library Block in the search index
Expand Down
Loading