Skip to content
Open
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
9e5a1c5
feat: Python api and rest api added for component draft history
ChrisChV Mar 16, 2026
80c9155
feat: publish history and entris functions added
ChrisChV Mar 21, 2026
e20f0a3
feat: Add profile images to contributors list
ChrisChV Mar 24, 2026
0fdbd38
feat: Add blockType to responses
ChrisChV Mar 24, 2026
1688454
feat: get_library_component_creation_entry added
ChrisChV Mar 26, 2026
c6df70f
feat: get_library_container_draft_history function added
ChrisChV Mar 27, 2026
df20d13
Merge branch 'master' into chris/FAL-4330-history-log
ChrisChV Apr 6, 2026
c14bb36
feat: add container publish history endpoint and unify entries endpoint
ChrisChV Apr 13, 2026
1596aae
feat: add Post-Verawood publish history with direct_published_entitie…
ChrisChV Apr 19, 2026
b0df5d3
fix: Broken lints
ChrisChV Apr 19, 2026
e89065d
Merge branch 'master' into chris/FAL-4330-history-log
ChrisChV Apr 20, 2026
bfc0013
feat: Function to get creation entry for containers
ChrisChV Apr 20, 2026
d0b6263
Merge branch 'master' into chris/FAL-4330-history-log
ChrisChV Apr 20, 2026
7cd6457
temp: Update openedx-core version
ChrisChV Apr 20, 2026
cc87727
fix: Broken lints
ChrisChV Apr 21, 2026
e9e9061
refactor: Add `entity__component__component_type` to history log func…
ChrisChV Apr 23, 2026
2518487
Merge branch 'master' into chris/FAL-4330-history-log
ChrisChV Apr 23, 2026
93be604
Merge branch 'master' into chris/FAL-4330-history-log
ChrisChV Apr 23, 2026
2345adf
fix: Small fix in block_metadata about local_key
ChrisChV Apr 23, 2026
8f7a67e
fix: Broken lint and tests
ChrisChV Apr 24, 2026
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
155 changes: 154 additions & 1 deletion openedx/core/djangoapps/content_libraries/api/block_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,28 @@
from __future__ import annotations

from dataclasses import dataclass
from datetime import datetime

from django.contrib.auth import get_user_model
from django.utils.translation import gettext as _ # noqa: F401
from opaque_keys.edx.locator import LibraryUsageLocatorV2
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
from openedx_content.models_api import Component, PublishLogRecord

from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_urls_for_user

from .libraries import PublishableItem, library_component_usage_key

# The public API is only the following symbols:
__all__ = [
"LibraryXBlockMetadata",
"LibraryXBlockStaticFile",
"LibraryHistoryEntry",
"LibraryHistoryContributor",
"LibraryPublishHistoryGroup",
]

User = get_user_model()


@dataclass(frozen=True, kw_only=True)
class LibraryXBlockMetadata(PublishableItem):
Expand Down Expand Up @@ -48,6 +58,7 @@ def from_component(cls, library_key, component, associated_collections=None):
usage_key=usage_key,
display_name=draft.title,
created=component.created,
created_by=component.created_by.username if component.created_by else None,
modified=draft.created,
draft_version_num=draft.version_num,
published_version_num=published.version_num if published else None,
Expand All @@ -63,6 +74,77 @@ def from_component(cls, library_key, component, associated_collections=None):
)


@dataclass(frozen=True)
class LibraryHistoryEntry:
"""
One entry in the history of a library component.
"""
changed_by: LibraryHistoryContributor | None
changed_at: datetime
title: str # title at time of change
item_type: str
action: str # "edited" | "renamed"


@dataclass(frozen=True)
class LibraryHistoryContributor:
"""
A contributor in a publish history group, with profile image URLs.
"""
username: str
profile_image_urls: dict # {"full": str, "large": str, "medium": str, "small": str}

@classmethod
def from_user(cls, user, request=None) -> LibraryHistoryContributor:
return cls(
username=user.username,
profile_image_urls=get_profile_image_urls_for_user(user, request),
)


@dataclass(frozen=True)
class DirectPublishedEntity:
"""
Represents one entity the user directly requested to publish (direct=True).
Each entry carries its own title and entity_type so the frontend can display
the correct label for each directly published item.

Pre-Verawood groups have exactly one entry (approximated from available data).
Post-Verawood groups have one entry per direct=True record in the PublishLog.
"""
entity_key: str # str(usage_key) for components, str(container_key) for containers
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.

Why make this a str instead of a LibraryUsageLocatorV2?

title: str # title of the entity at time of publish
entity_type: str # e.g. "html", "problem" for components; "unit", "section" for containers


@dataclass(frozen=True)
class LibraryPublishHistoryGroup:
"""
Summary of a publish event for a library item.

Each instance represents one or more PublishLogRecords, and includes the
set of contributors who authored draft changes between the previous publish
and this one.

Pre-Verawood (direct=None): one group per entity × publish event.
Post-Verawood (direct!=None): one group per unique PublishLog.
"""
publish_log_uuid: str
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.

Similar above, why not have this be a UUID?

published_by: object # AUTH_USER_MODEL instance or None
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.

@kdmccormick: What's the right way to do this if we want to be respectful of custom user models? I vaguely recall you went down this rabbit hole once.

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.

Actually, I think openedx-platform has too many assumptions about our user model for custom user models to work anyway, so maybe we can just make this a User?

published_at: datetime
contributors: list[LibraryHistoryContributor] # distinct authors of versions in this group
contributors_count: int
# Replaces entity_key, title, block_type. Each element is one entity the
# user directly requested to publish. Pre-Verawood: single approximated entry.
# Post-Verawood: one entry per direct=True record in the PublishLog.
direct_published_entities: list[DirectPublishedEntity]
# Key to pass as scope_entity_key when fetching entries for this group.
# Pre-Verawood: the specific entity key for this group (container or usage key).
# Post-Verawood container groups: None — frontend must use currentContainerKey.
# Component history (all eras): str(usage_key).
scope_entity_key: str | None
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.

Can this be a union of the acceptable key types?



@dataclass(frozen=True)
class LibraryXBlockStaticFile:
"""
Expand All @@ -76,3 +158,74 @@ class LibraryXBlockStaticFile:
url: str
# Size in bytes
size: int


def resolve_contributors(users, request=None) -> list[LibraryHistoryContributor | None]:
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.

At a high level, it seems like the whole function of this is to make sure that whatever users come in do a select_related on profile to avoid n+1 queries, and otherwise you could do the entire thing as:

[
    LibraryHistoryContributor.from_user(user, request) if user else None
    for user in users
]

Am I interpreting that correctly? If so, can we just make sure the original qset uses the right select_related and skip the logic in this function?

"""
Convert an iterable of User objects (possibly containing None) to a list of
LibraryHistoryContributor.
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.

Please put more information in the docstring about how this function is expected to behave, such as:

  1. What ordering guarantees does it make?
  2. Will it remove duplicates?
  3. Will it return None entries in the output?

"""
users_list = list(users)
user_pks = list({user.pk for user in users_list if user is not None})
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.

  1. Please explain that you're doing here (what user_pks is for).
  2. You don't really need to make user_pks a list if the only point is to de-duplicate. You can iterate the set. Not for performance reasons, but just because the list() here implies that order matters, when it doesn't seem to in this case.
  3. It would be clearer to just return early after this if there are no real users in the request.
if not user_pks:
    return []

prefetched = {
user.pk: user
for user in User.objects.filter(pk__in=user_pks).select_related('profile')
} if user_pks else {}
return [
LibraryHistoryContributor.from_user(prefetched.get(user.pk, user), request)
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.

prefetched.get(user.pk, user) is peculiar here. Are we worried that for some reason it won't come back on the prefetch?

if user else None
for user in users_list
]


def resolve_change_action(old_version, new_version) -> str:
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.

Please annotate what types old_version and new_version are supposed to be.

"""
Derive a human-readable action label from a draft history record's versions.

Returns "renamed" when both versions exist and the title changed between
them; otherwise returns "edited" as the default action.
Comment on lines +185 to +186
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.

Are creation and deletion out of scope?

"""
if old_version and new_version and old_version.title != new_version.title:
return "renamed"
return "edited"


def direct_published_entity_from_record(
record: PublishLogRecord,
lib_key: LibraryLocatorV2,
) -> DirectPublishedEntity:
"""
Build a DirectPublishedEntity from a PublishLogRecord.

lib_key is used only to construct locator strings — entity_key is always
derived from record.entity itself, never from an external container key.

Callers must ensure the record is fetched with:
select_related(
'entity__component__component_type',
'entity__container__container_type',
'new_version',
)
"""
# Import here to avoid circular imports (container_metadata imports block_metadata).
from .container_metadata import library_container_locator # noqa: PLC0415

title = record.new_version.title if record.new_version else ""
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.

Does that mean that deletion is going to show up in the log as:

"" was edited

Because the new_version is going to be null?

try:
component = record.entity.component
return DirectPublishedEntity(
entity_key=str(LibraryUsageLocatorV2( # type: ignore[abstract]
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.

Why is ignore[abstract] necessary?

lib_key=lib_key,
block_type=component.component_type.name,
usage_id=component.component_code,
)),
title=title,
entity_type=component.component_type.name,
)
except Component.DoesNotExist:
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.

Please do checks with hasattr() for container vs. component, and branch accordingly. Having the container code path be an error handling fallback of component handling is awkward and will break when we have entities that are neither Components nor Containers.

container = record.entity.container
return DirectPublishedEntity(
entity_key=str(library_container_locator(lib_key, container)),
title=title,
entity_type=container.container_type.type_code,
)
Loading
Loading