Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 17 additions & 12 deletions openedx/core/djangoapps/content_libraries/api/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
from openedx.core.types import User as UserType

from ..models import ContentLibrary
from ..permissions import CAN_EDIT_THIS_CONTENT_LIBRARY
from .exceptions import (
BlockLimitReachedError,
ContentLibraryBlockNotFound,
Expand All @@ -67,7 +66,6 @@
from .libraries import (
library_collection_locator,
library_component_usage_key,
require_permission_for_library_key,
PublishableItem,
)

Expand Down Expand Up @@ -891,18 +889,14 @@ def publish_component_changes(usage_key: LibraryUsageLocatorV2, user: UserType):
"""
Publish all pending changes in a single component.
"""
content_library = require_permission_for_library_key(
usage_key.lib_key,
user,
CAN_EDIT_THIS_CONTENT_LIBRARY
)
component = get_component_from_usage_key(usage_key)
library_key = usage_key.context_key
content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined]
learning_package = content_library.learning_package

assert learning_package
component = get_component_from_usage_key(usage_key)
drafts_to_publish = authoring_api.get_all_drafts(learning_package.id).filter(
entity__key=component.key
)
# The core publishing API is based on draft objects, so find the draft that corresponds to this component:
drafts_to_publish = authoring_api.get_all_drafts(learning_package.id).filter(entity__key=component.key)
# Publish the component and update anything that needs to be updated (e.g. search index):
authoring_api.publish_from_drafts(learning_package.id, draft_qset=drafts_to_publish, published_by=user.id)
LIBRARY_BLOCK_UPDATED.send_event(
library_block=LibraryBlockData(

@pomegranited pomegranited Apr 18, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we also raise LIBRARY_CONTAINER_UPDATED for affected containers here like we do here, then it'll handle this case:

  1. Edit a Component in a Unit:
    • Component + Unit show "unpublished changes" ✔️
  2. Publish the Component only.
    • Component no longer shows "unpublished changes" ✔️
    • Unit no longer shows "unpublished changes" ❌

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with c0972ef.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for identifying and fixing that!

Expand All @@ -911,6 +905,17 @@ def publish_component_changes(usage_key: LibraryUsageLocatorV2, user: UserType):
)
)

# For each container, trigger LIBRARY_CONTAINER_UPDATED signal and set background=True to trigger
# container indexing asynchronously.
affected_containers = get_containers_contains_component(usage_key)
for container in affected_containers:
LIBRARY_CONTAINER_UPDATED.send_event(
library_container=LibraryContainerData(
container_key=container.container_key,
background=True,
)
)


def _component_exists(usage_key: UsageKeyV2) -> bool:
"""
Expand Down
52 changes: 50 additions & 2 deletions openedx/core/djangoapps/content_libraries/api/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,21 @@
from dataclasses import dataclass
from datetime import datetime
from enum import Enum
import logging
from uuid import uuid4

from django.utils.text import slugify
from opaque_keys.edx.keys import UsageKeyV2
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2
from openedx_events.content_authoring.data import ContentObjectChangedData, LibraryCollectionData, LibraryContainerData
from openedx_events.content_authoring.data import (
ContentObjectChangedData,
LibraryBlockData,
LibraryCollectionData,
LibraryContainerData,
)
from openedx_events.content_authoring.signals import (
CONTENT_OBJECT_ASSOCIATIONS_CHANGED,
LIBRARY_BLOCK_UPDATED,
LIBRARY_COLLECTION_UPDATED,
LIBRARY_CONTAINER_CREATED,
LIBRARY_CONTAINER_DELETED,
Expand All @@ -27,7 +34,7 @@

from ..models import ContentLibrary
from .exceptions import ContentLibraryContainerNotFound
from .libraries import LibraryXBlockMetadata, PublishableItem
from .libraries import LibraryXBlockMetadata, PublishableItem, library_component_usage_key

# The public API is only the following symbols:
__all__ = [
Expand All @@ -46,8 +53,11 @@
"restore_container",
"update_container_children",
"get_containers_contains_component",
"publish_container_changes",
]

log = logging.getLogger(__name__)


class ContainerType(Enum):
Unit = "unit"
Expand Down Expand Up @@ -400,3 +410,41 @@ def get_containers_contains_component(
ContainerMetadata.from_container(usage_key.context_key, container)
for container in containers
]


def publish_container_changes(container_key: LibraryContainerLocator, user_id: int | None) -> None:
"""
Publish all unpublished changes in a container and all its child
containers/blocks.
"""
container = get_container_from_key(container_key)
library_key = container_key.library_key
content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined]
learning_package = content_library.learning_package
assert learning_package
# The core publishing API is based on draft objects, so find the draft that corresponds to this container:
drafts_to_publish = authoring_api.get_all_drafts(learning_package.id).filter(entity__pk=container.pk)
# Publish the container, which will also auto-publish any unpublished child components:
publish_log = authoring_api.publish_from_drafts(
learning_package.id,
draft_qset=drafts_to_publish,
published_by=user_id,
)
# Update anything that needs to be updated (e.g. search index):
for record in publish_log.records.select_related("entity", "entity__container", "entity__component").all():
if hasattr(record.entity, "component"):
# This is a child component like an XBLock in a Unit that was published:
usage_key = library_component_usage_key(library_key, record.entity.component)
LIBRARY_BLOCK_UPDATED.send_event(
library_block=LibraryBlockData(library_key=library_key, usage_key=usage_key)
)
elif hasattr(record.entity, "container"):
# This is a child container like a Unit, or is the same "container" we published above.
LIBRARY_CONTAINER_UPDATED.send_event(
library_container=LibraryContainerData(container_key=container_key)
)
else:
log.warning(
f"PublishableEntity {record.entity.pk} / {record.entity.key} was modified during publish operation "
"but is of unknown type."
)
8 changes: 8 additions & 0 deletions openedx/core/djangoapps/content_libraries/rest_api/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,15 @@ class LibraryBlockPublishView(APIView):

@convert_exceptions
def post(self, request, usage_key_str):
"""
Publish the draft changes made to this component.
"""
key = LibraryUsageLocatorV2.from_string(usage_key_str)
api.require_permission_for_library_key(
key.lib_key,
request.user,
permissions.CAN_EDIT_THIS_CONTENT_LIBRARY
)
api.publish_component_changes(key, request.user)
return Response({})

Expand Down
22 changes: 22 additions & 0 deletions openedx/core/djangoapps/content_libraries/rest_api/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,3 +328,25 @@ def patch(self, request: RestRequest, container_key: LibraryContainerLocator) ->
)

return Response({'count': len(collection_keys)})


@method_decorator(non_atomic_requests, name="dispatch")
@view_auth_classes()
class LibraryContainerPublishView(GenericAPIView):
"""
View to publish a container, or revert to last published.
"""
@convert_exceptions
def post(self, request: RestRequest, container_key: LibraryContainerLocator) -> Response:
"""
Publish the container and its children
"""
api.require_permission_for_library_key(
container_key.library_key,
request.user,
permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
)
api.publish_container_changes(container_key, request.user.id)
# If we need to in the future, we could return a list of all the child containers/components that were
# auto-published as a result.
return Response({})
5 changes: 5 additions & 0 deletions openedx/core/djangoapps/content_libraries/tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
URL_LIB_CONTAINER_COMPONENTS = URL_LIB_CONTAINER + 'children/' # Get, add or delete a component in this container
URL_LIB_CONTAINER_RESTORE = URL_LIB_CONTAINER + 'restore/' # Restore a deleted container
URL_LIB_CONTAINER_COLLECTIONS = URL_LIB_CONTAINER + 'collections/' # Handle associated collections
URL_LIB_CONTAINER_PUBLISH = URL_LIB_CONTAINER + 'publish/' # Publish changes to the specified container + children

URL_LIB_LTI_PREFIX = URL_PREFIX + 'lti/1.3/'
URL_LIB_LTI_JWKS = URL_LIB_LTI_PREFIX + 'pub/jwks/'
Expand Down Expand Up @@ -455,3 +456,7 @@ def _patch_container_collections(
{'collection_keys': collection_keys},
expect_response
)

def _publish_container(self, container_key, expect_response=200):
""" Publish all changes in the specified container + children """
return self._api('post', URL_LIB_CONTAINER_PUBLISH.format(container_key=container_key), None, expect_response)
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
import ddt
from freezegun import freeze_time

from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2
from openedx_events.content_authoring.data import LibraryContainerData
from openedx_events.content_authoring.signals import (
LIBRARY_BLOCK_UPDATED,
LIBRARY_CONTAINER_CREATED,
LIBRARY_CONTAINER_DELETED,
LIBRARY_CONTAINER_UPDATED,
Expand Down Expand Up @@ -43,6 +44,7 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest):
break any tests, but backwards-incompatible API changes will.
"""
ENABLED_OPENEDX_EVENTS = [
LIBRARY_BLOCK_UPDATED.event_type,
LIBRARY_CONTAINER_CREATED.event_type,
LIBRARY_CONTAINER_DELETED.event_type,
LIBRARY_CONTAINER_UPDATED.event_type,
Expand Down Expand Up @@ -413,3 +415,88 @@ def test_container_collections(self):

# Verify the collections
assert unit_as_read['collections'] == [{"title": col1.title, "key": col1.key}]

def test_publish_container(self): # pylint: disable=too-many-statements
"""
Test that we can publish the changes to a specific container
"""
lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more")

# Create two containers and add some components
container1 = self._create_container(lib["id"], "unit", display_name="Alpha Unit", slug=None)
container2 = self._create_container(lib["id"], "unit", display_name="Bravo Unit", slug=None)
problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False)
html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False)
html_block2 = self._add_block_to_library(lib["id"], "html", "Html2", can_stand_alone=False)
self._add_container_components(container1["id"], children_ids=[problem_block["id"], html_block["id"]])
self._add_container_components(container2["id"], children_ids=[html_block["id"], html_block2["id"]])
# At first everything is unpublished:
c1_before = self._get_container(container1["id"])
assert c1_before["has_unpublished_changes"]
c1_components_before = self._get_container_components(container1["id"])
assert len(c1_components_before) == 2
assert c1_components_before[0]["id"] == problem_block["id"]
assert c1_components_before[0]["has_unpublished_changes"]
assert c1_components_before[0]["published_by"] is None
assert c1_components_before[1]["id"] == html_block["id"]
assert c1_components_before[1]["has_unpublished_changes"]
assert c1_components_before[1]["published_by"] is None
c2_before = self._get_container(container2["id"])
assert c2_before["has_unpublished_changes"]

# Set up event receivers after the initial mock data setup is complete:
updated_container_receiver = mock.Mock()
updated_block_receiver = mock.Mock()
LIBRARY_CONTAINER_UPDATED.connect(updated_container_receiver)
LIBRARY_BLOCK_UPDATED.connect(updated_block_receiver)

# Now publish only Container 1
self._publish_container(container1["id"])

# Now it is published:
c1_after = self._get_container(container1["id"])
assert c1_after["has_unpublished_changes"] is False
c1_components_after = self._get_container_components(container1["id"])
assert len(c1_components_after) == 2
assert c1_components_after[0]["id"] == problem_block["id"]
assert c1_components_after[0]["has_unpublished_changes"] is False
assert c1_components_after[0]["published_by"] == self.user.username
assert c1_components_after[1]["id"] == html_block["id"]
assert c1_components_after[1]["has_unpublished_changes"] is False
assert c1_components_after[1]["published_by"] == self.user.username

# and container 2 is still unpublished, except for the shared HTML block that is also in container 1:
c2_after = self._get_container(container2["id"])
assert c2_after["has_unpublished_changes"]
c2_components_after = self._get_container_components(container2["id"])
assert len(c2_components_after) == 2
assert c2_components_after[0]["id"] == html_block["id"]
assert c2_components_after[0]["has_unpublished_changes"] is False # published since it's also in container 1
assert c2_components_after[0]["published_by"] == self.user.username
assert c2_components_after[1]["id"] == html_block2["id"]
assert c2_components_after[1]["has_unpublished_changes"] # unaffected
assert c2_components_after[1]["published_by"] is None

# Make sure that the right events were sent out.
# First, there should be one container updated event:
assert len(updated_container_receiver.call_args_list) == 1
self.assertDictContainsSubset(
{
"signal": LIBRARY_CONTAINER_UPDATED,
"library_container": LibraryContainerData(
container_key=LibraryContainerLocator.from_string(container1["id"]),
),
},
updated_container_receiver.call_args_list[0].kwargs,
)

# Second, two XBlock updated events:
assert len(updated_block_receiver.call_args_list) == 2
updated_block_ids = set(
call.kwargs["library_block"].usage_key for call in updated_block_receiver.call_args_list
)
assert updated_block_ids == {
LibraryUsageLocatorV2.from_string(problem_block["id"]),
LibraryUsageLocatorV2.from_string(html_block["id"]),
}
assert all(call.kwargs["signal"] == LIBRARY_BLOCK_UPDATED for call in updated_block_receiver.call_args_list)
3 changes: 2 additions & 1 deletion openedx/core/djangoapps/content_libraries/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@
path('restore/', containers.LibraryContainerRestore.as_view()),
# Update collections for a given container
path('collections/', containers.LibraryContainerCollectionsView.as_view(), name='update-collections-ct'),
# path('publish/', views.LibraryContainerPublishView.as_view()),
# Publish a container (or reset to last published)
path('publish/', containers.LibraryContainerPublishView.as_view()),
])),
re_path(r'^lti/1.3/', include([
path('login/', libraries.LtiToolLoginView.as_view(), name='lti-login'),
Expand Down