From 14518cd23b325921af5cf7cef8f2d26a3ee1985c Mon Sep 17 00:00:00 2001 From: Kyle D McCormick Date: Thu, 23 Apr 2026 16:51:04 -0400 Subject: [PATCH 01/10] build: Bump version to 1.0.0 --- src/openedx_core/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openedx_core/__init__.py b/src/openedx_core/__init__.py index fb5e265a1..e9bef3c71 100644 --- a/src/openedx_core/__init__.py +++ b/src/openedx_core/__init__.py @@ -6,4 +6,4 @@ """ # The version for the entire repository -__version__ = "0.45.0" +__version__ = "1.0.0" From b443da6735290b0b93b94254fb295d35cbe059bb Mon Sep 17 00:00:00 2001 From: Kyle D McCormick Date: Thu, 23 Apr 2026 17:01:56 -0400 Subject: [PATCH 02/10] refactor: Remove unnecessary AUTH_USER migration deps --- .../migrations/0008_rename_collection_key_to_collection_code.py | 1 - .../migrations/0013_unicode_container_component_codes.py | 1 - 2 files changed, 2 deletions(-) diff --git a/src/openedx_content/migrations/0008_rename_collection_key_to_collection_code.py b/src/openedx_content/migrations/0008_rename_collection_key_to_collection_code.py index 9bdab4915..66db2a3bb 100644 --- a/src/openedx_content/migrations/0008_rename_collection_key_to_collection_code.py +++ b/src/openedx_content/migrations/0008_rename_collection_key_to_collection_code.py @@ -15,7 +15,6 @@ class Migration(migrations.Migration): dependencies = [ ('openedx_content', '0007_publishlogrecord_direct'), - migrations.swappable_dependency(settings.AUTH_USER_MODEL), ] operations = [ diff --git a/src/openedx_content/migrations/0013_unicode_container_component_codes.py b/src/openedx_content/migrations/0013_unicode_container_component_codes.py index e944ce2fd..ad211d909 100644 --- a/src/openedx_content/migrations/0013_unicode_container_component_codes.py +++ b/src/openedx_content/migrations/0013_unicode_container_component_codes.py @@ -14,7 +14,6 @@ class Migration(migrations.Migration): dependencies = [ ('openedx_content', '0012_rename_componentversionmedia_key_to_path'), - migrations.swappable_dependency(settings.AUTH_USER_MODEL), ] operations = [ From d9a1cafe55371801f948426553967bcd5b4b552c Mon Sep 17 00:00:00 2001 From: Kyle D McCormick Date: Thu, 23 Apr 2026 17:04:23 -0400 Subject: [PATCH 03/10] docs: Remove instability warning from README --- README.rst | 5 ----- 1 file changed, 5 deletions(-) diff --git a/README.rst b/README.rst index ac8bd8b40..4fd80efee 100644 --- a/README.rst +++ b/README.rst @@ -64,11 +64,6 @@ The code in this repository is licensed under the AGPL 3.0 unless otherwise note Please see `LICENSE.txt `_ for details. -How To Contribute ------------------ - -This repo is in a very experimental state. Discussion using GitHub Issues is welcome, but you probably don't want to make contributions as everything can shift around drastically with little notice. - Reporting Security Issues ------------------------- From 81658762225078c7ad88c5548d5eac54b468c311 Mon Sep 17 00:00:00 2001 From: Kyle D McCormick Date: Thu, 23 Apr 2026 17:06:17 -0400 Subject: [PATCH 04/10] docs: Remove top-level instability warnings --- src/openedx_content/applets/collections/api.py | 2 +- src/openedx_content/applets/components/api.py | 2 +- src/openedx_content/applets/containers/api.py | 4 +--- src/openedx_content/applets/media/api.py | 2 +- src/openedx_content/applets/publishing/api.py | 2 +- src/openedx_content/applets/sections/api.py | 2 -- src/openedx_content/applets/subsections/api.py | 2 -- src/openedx_content/applets/units/api.py | 2 -- 8 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/openedx_content/applets/collections/api.py b/src/openedx_content/applets/collections/api.py index 6abe5e2b7..0f47df482 100644 --- a/src/openedx_content/applets/collections/api.py +++ b/src/openedx_content/applets/collections/api.py @@ -1,5 +1,5 @@ """ -Collections API (warning: UNSTABLE, in progress API) +Collections API """ from __future__ import annotations diff --git a/src/openedx_content/applets/components/api.py b/src/openedx_content/applets/components/api.py index 4c9f136a1..7b93e3a38 100644 --- a/src/openedx_content/applets/components/api.py +++ b/src/openedx_content/applets/components/api.py @@ -1,5 +1,5 @@ """ -Components API (warning: UNSTABLE, in progress API) +Components API These functions are often going to be simple-looking write operations, but there is bookkeeping logic needed across multiple models to keep state consistent. You diff --git a/src/openedx_content/applets/containers/api.py b/src/openedx_content/applets/containers/api.py index 9f0756a58..e457807c6 100644 --- a/src/openedx_content/applets/containers/api.py +++ b/src/openedx_content/applets/containers/api.py @@ -1,5 +1,5 @@ """ -Containers API (warning: UNSTABLE, in progress API) +Containers API """ from __future__ import annotations @@ -45,8 +45,6 @@ # start with an underscore AND it is not in __all__, that function is considered # to be callable only by other apps in the authoring package. __all__ = [ - # ๐Ÿ›‘ UNSTABLE: All APIs related to containers are unstable until we've figured - # out our approach to dynamic content (randomized, A/B tests, etc.) "ContainerSubclass", "ContainerImplementationMissingError", "create_container", diff --git a/src/openedx_content/applets/media/api.py b/src/openedx_content/applets/media/api.py index 90d0ff801..f36c71a25 100644 --- a/src/openedx_content/applets/media/api.py +++ b/src/openedx_content/applets/media/api.py @@ -1,5 +1,5 @@ """ -Low Level media.api (warning: UNSTABLE, in progress API) +Low Level media.api Please look at the models.py file for more information about the kinds of data are stored in this app. diff --git a/src/openedx_content/applets/publishing/api.py b/src/openedx_content/applets/publishing/api.py index 8889c1f19..e5bd6a900 100644 --- a/src/openedx_content/applets/publishing/api.py +++ b/src/openedx_content/applets/publishing/api.py @@ -1,5 +1,5 @@ """ -Publishing API (warning: UNSTABLE, in progress API) +Publishing API Please look at the models.py file for more information about the kinds of data are stored in this app. diff --git a/src/openedx_content/applets/sections/api.py b/src/openedx_content/applets/sections/api.py index 5449235eb..5d80596a6 100644 --- a/src/openedx_content/applets/sections/api.py +++ b/src/openedx_content/applets/sections/api.py @@ -13,8 +13,6 @@ from ..subsections.models import Subsection, SubsectionVersion from .models import Section, SectionVersion -# ๐Ÿ›‘ UNSTABLE: All APIs related to containers are unstable until we've figured -# out our approach to dynamic content (randomized, A/B tests, etc.) __all__ = [ "get_section", "create_section_and_version", diff --git a/src/openedx_content/applets/subsections/api.py b/src/openedx_content/applets/subsections/api.py index 1d9544990..46f8d8c9f 100644 --- a/src/openedx_content/applets/subsections/api.py +++ b/src/openedx_content/applets/subsections/api.py @@ -13,8 +13,6 @@ from ..units.models import Unit, UnitVersion from .models import Subsection, SubsectionVersion -# ๐Ÿ›‘ UNSTABLE: All APIs related to containers are unstable until we've figured -# out our approach to dynamic content (randomized, A/B tests, etc.) __all__ = [ "get_subsection", "create_subsection_and_version", diff --git a/src/openedx_content/applets/units/api.py b/src/openedx_content/applets/units/api.py index 415ebf291..c1556a32f 100644 --- a/src/openedx_content/applets/units/api.py +++ b/src/openedx_content/applets/units/api.py @@ -13,8 +13,6 @@ from ..publishing.models import LearningPackage from .models import Unit, UnitVersion -# ๐Ÿ›‘ UNSTABLE: All APIs related to containers are unstable until we've figured -# out our approach to dynamic content (randomized, A/B tests, etc.) __all__ = [ "get_unit", "create_unit_and_version", From 58e265ba50bc0d1338730f5b7a61c215fcb79990 Mon Sep 17 00:00:00 2001 From: Kyle D McCormick Date: Thu, 23 Apr 2026 17:10:52 -0400 Subject: [PATCH 05/10] temp: Stable or unstable? -- resolve before merging --- src/openedx_content/applets/containers/api.py | 34 +++++++++---------- src/openedx_content/applets/publishing/api.py | 8 ++--- src/openedx_content/applets/sections/api.py | 4 +-- .../applets/subsections/api.py | 4 +-- src/openedx_content/applets/units/api.py | 4 +-- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/openedx_content/applets/containers/api.py b/src/openedx_content/applets/containers/api.py index e457807c6..ec41e73f7 100644 --- a/src/openedx_content/applets/containers/api.py +++ b/src/openedx_content/applets/containers/api.py @@ -74,7 +74,7 @@ @dataclass(frozen=True) class ContainerEntityListEntry: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Data about a single entity in a container, e.g. a component in a unit. """ @@ -143,7 +143,7 @@ def create_container( can_stand_alone: bool = True, ) -> ContainerModel: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Create a new container. Args: @@ -183,7 +183,7 @@ def create_container( def create_entity_list() -> EntityList: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Create a new entity list. This is an structure that holds a list of entities that will be referenced by the container. @@ -199,7 +199,7 @@ def create_entity_list_with_rows( learning_package_id: LearningPackage.ID | None, ) -> EntityList: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Create new entity list rows for an entity list. Args: @@ -305,7 +305,7 @@ def create_container_version( created_by: int | None, ) -> ContainerVersion: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Create a new container version. Args: @@ -355,7 +355,7 @@ def create_container_and_version( can_stand_alone: bool = True, ) -> tuple[ContainerModel, ContainerVersionModel]: """ - [ ๐Ÿ›‘ UNSTABLE ] Create a new container and its initial version. + [ โ“TODO: STABLE or UNSTABLE? ] Create a new container and its initial version. Args: learning_package_id: The learning package ID. @@ -470,7 +470,7 @@ def create_next_container_version( force_version_num: int | None = None, ) -> ContainerVersion: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Create the next version of a container. A new version of the container is created only when its metadata changes: @@ -543,7 +543,7 @@ def create_next_container_version( def get_container(pk: Container.ID) -> Container: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Get a container by its primary key. This returns the Container, not any specific version. It may not be published, or may have been soft deleted. @@ -559,7 +559,7 @@ def get_container(pk: Container.ID) -> Container: def get_container_version(container_version_pk: int) -> ContainerVersion: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Get a container version by its primary key. Args: @@ -573,7 +573,7 @@ def get_container_version(container_version_pk: int) -> ContainerVersion: def get_container_by_code(learning_package_id: LearningPackage.ID, /, container_code: str) -> Container: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Get a container by its learning package and container code. Args: @@ -639,7 +639,7 @@ def get_containers( include_deleted: bool | None = False, ) -> QuerySet[Container]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Get all containers in the given learning package. Args: @@ -667,7 +667,7 @@ def get_entities_in_container( select_related_version: str | None = None, ) -> list[ContainerEntityListEntry]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Get the list of entities and their versions in the current draft or published version of the given container. @@ -726,7 +726,7 @@ def get_entities_in_container_as_of( publish_log_id: int, ) -> tuple[ContainerVersion | None, list[ContainerEntityListEntry]]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Get the list of entities and their versions in the published version of the given container as of the given PublishLog version (which is essentially a version for the entire learning package). @@ -760,7 +760,7 @@ def get_entities_in_container_as_of( def contains_unpublished_changes(container_or_pk: Container | Container.ID, /) -> bool: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Check recursively if a container has any unpublished changes. Note: I've preserved the API signature for now, but we probably eventually @@ -813,7 +813,7 @@ def get_containers_with_entity( published=False, ) -> QuerySet[Container]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Find all draft containers that directly contain the given entity. They will always be from the same learning package; cross-package containers @@ -854,7 +854,7 @@ def get_container_children_count( published: bool, ): """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Get the count of entities in the current draft or published version of the given container. Args: @@ -892,7 +892,7 @@ def get_container_children_entity_refs(container_version: ContainerVersion) -> l def get_descendant_component_entity_ids(container: Container) -> list[int]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Return the entity IDs of all leaf (non-Container) descendants of ``container``. Intermediate containers (e.g. Subsections, Units) are never included in the diff --git a/src/openedx_content/applets/publishing/api.py b/src/openedx_content/applets/publishing/api.py index e5bd6a900..5fda59772 100644 --- a/src/openedx_content/applets/publishing/api.py +++ b/src/openedx_content/applets/publishing/api.py @@ -575,7 +575,7 @@ def get_entity_draft_history( publishable_entity_or_id: PublishableEntity | int, / ) -> QuerySet[DraftChangeLogRecord]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Return DraftChangeLogRecords for a PublishableEntity since its last publication, ordered from most recent to oldest. @@ -643,7 +643,7 @@ def get_entity_publish_history( publishable_entity_or_id: PublishableEntity | int, / ) -> QuerySet[PublishLogRecord]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Return all PublishLogRecords for a PublishableEntity, ordered most recent first. Edge cases: @@ -678,7 +678,7 @@ def get_entity_publish_history_entries( publish_log_uuid: str, ) -> QuerySet[DraftChangeLogRecord]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Return the DraftChangeLogRecords associated with a specific PublishLog. Finds the PublishLogRecord for the given entity and publish_log_uuid, then @@ -776,7 +776,7 @@ def get_entity_version_contributors( new_version_num: int | None, ) -> QuerySet: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Return distinct User queryset of contributors (changed_by) for DraftChangeLogRecords of a PublishableEntity after old_version_num. diff --git a/src/openedx_content/applets/sections/api.py b/src/openedx_content/applets/sections/api.py index 5d80596a6..91c6aa058 100644 --- a/src/openedx_content/applets/sections/api.py +++ b/src/openedx_content/applets/sections/api.py @@ -91,7 +91,7 @@ def create_next_section_version( @dataclass(frozen=True) class SectionListEntry: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Data about a single subsection in a section. """ @@ -109,7 +109,7 @@ def get_subsections_in_section( published: bool, ) -> list[SectionListEntry]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Get the list of entities and their versions in the draft or published version of the given Section. diff --git a/src/openedx_content/applets/subsections/api.py b/src/openedx_content/applets/subsections/api.py index 46f8d8c9f..2bbcd02cf 100644 --- a/src/openedx_content/applets/subsections/api.py +++ b/src/openedx_content/applets/subsections/api.py @@ -91,7 +91,7 @@ def create_next_subsection_version( @dataclass(frozen=True) class SubsectionListEntry: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Data about a single unit in a subsection. """ @@ -109,7 +109,7 @@ def get_units_in_subsection( published: bool, ) -> list[SubsectionListEntry]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Get the list of entities and their versions in the draft or published version of the given Subsection. diff --git a/src/openedx_content/applets/units/api.py b/src/openedx_content/applets/units/api.py index c1556a32f..41bcbd20c 100644 --- a/src/openedx_content/applets/units/api.py +++ b/src/openedx_content/applets/units/api.py @@ -91,7 +91,7 @@ def create_next_unit_version( @dataclass(frozen=True) class UnitListEntry: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Data about a single entity in a container, e.g. a component in a unit. """ @@ -109,7 +109,7 @@ def get_components_in_unit( published: bool, ) -> list[UnitListEntry]: """ - [ ๐Ÿ›‘ UNSTABLE ] + [ โ“TODO: STABLE or UNSTABLE? ] Get the list of entities and their versions in the draft or published version of the given Unit. From fd907976ee38f516ae07bdf4e986033cd8ab83fa Mon Sep 17 00:00:00 2001 From: Kyle D McCormick Date: Thu, 23 Apr 2026 17:25:09 -0400 Subject: [PATCH 06/10] docs: Stability policy --- README.rst | 5 ++++- setup.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 4fd80efee..55055d452 100644 --- a/README.rst +++ b/README.rst @@ -11,7 +11,10 @@ Overview The ``openedx-core`` project holds Django apps which represent core teaching & learning platform concepts. -Each app exposes stable, public API of Python functions and Django models. Some apps additionally provides REST APIs. These APIs are suitable for use in ``openedx-platform`` as well as in community-developed Open edX plugins. +Each app exposes a public API of Python functions and Django models; some apps also provide REST APIs. These APIs are suitable for use in openedx-platform and in community-developed Open edX plugins. + +APIs marked "UNSTABLE" are subject to change at any time. All other APIs are considered stable, and any breaking changes will be announced through the community DEPR (deprecation and removal) process. + Motivation ---------- diff --git a/setup.py b/setup.py index d451c1d6a..94e328c82 100755 --- a/setup.py +++ b/setup.py @@ -80,7 +80,7 @@ def is_requirement(line): zip_safe=False, keywords='Python edx', classifiers=[ - 'Development Status :: 3 - Alpha', + 'Development Status :: 4 - Beta', 'Framework :: Django', 'Framework :: Django :: 5.2', 'Intended Audience :: Developers', From e93b44c7177913167f748169bb3418f267c3b6ee Mon Sep 17 00:00:00 2001 From: Kyle D McCormick Date: Thu, 23 Apr 2026 17:30:32 -0400 Subject: [PATCH 07/10] docs: Fix links in readme --- README.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.rst b/README.rst index 55055d452..39f39650e 100644 --- a/README.rst +++ b/README.rst @@ -47,10 +47,10 @@ We have a few different identifier types in the schema, and we try to avoid ``_i See Also ~~~~~~~~ -The structure of this repo follows [OEP-0049](https://open-edx-proposals.readthedocs.io/en/latest/architectural-decisions/oep-0049-django-app-patterns.html) where possible, and also borrows inspiration from: +The structure of this repo follows `OEP-0049 `_ where possible, and also borrows inspiration from: -* [Scaling Django to 500 apps](https://2021.djangocon.us/talks/scaling-django-to-500-apps/) (Dan Palmer, DjangoCon US 2021) -* [Django structure for scale and longevity](https://www.youtube.com/watch?v=yG3ZdxBb1oo) (Radoslav Georgiev, EuroPython 2018) +* `Scaling Django to 500 apps `_ (Dan Palmer, DjangoCon US 2021) +* `Django structure for scale and longevity `_ (Radoslav Georgiev, EuroPython 2018) Code Overview ------------- From 2ee25f21192504e2ba03866c085f69c34ab6421f Mon Sep 17 00:00:00 2001 From: Kyle D McCormick Date: Mon, 27 Apr 2026 12:09:05 -0400 Subject: [PATCH 08/10] fix: backup_restore __all__ --- src/openedx_content/applets/backup_restore/api.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/openedx_content/applets/backup_restore/api.py b/src/openedx_content/applets/backup_restore/api.py index 1f768b0b0..49de268d5 100644 --- a/src/openedx_content/applets/backup_restore/api.py +++ b/src/openedx_content/applets/backup_restore/api.py @@ -9,6 +9,17 @@ from .zipper import LearningPackageUnzipper, LearningPackageZipper +# The public API that will be re-exported by openedx_content.api +# is listed in the __all__ entries below. Internal helper functions that are +# private to this module should start with an underscore. If a function does not +# start with an underscore AND it is not in __all__, that function is considered +# to be callable only by other applets in the openedx_content package. +__all__ = [ + "create_zip_file", + "load_learning_package", +] + + def create_zip_file( package_ref: str, path: str, user: UserType | None = None, origin_server: str | None = None ) -> None: From f6cb1b7a0e195602067108d385df61b006b00de0 Mon Sep 17 00:00:00 2001 From: Kyle D McCormick Date: Mon, 27 Apr 2026 12:10:57 -0400 Subject: [PATCH 09/10] docs: fix __all__ comment since restructure --- src/openedx_content/applets/components/api.py | 2 +- src/openedx_content/applets/containers/api.py | 2 +- src/openedx_content/applets/media/api.py | 2 +- src/openedx_content/applets/publishing/api.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/openedx_content/applets/components/api.py b/src/openedx_content/applets/components/api.py index 7b93e3a38..8ebf90558 100644 --- a/src/openedx_content/applets/components/api.py +++ b/src/openedx_content/applets/components/api.py @@ -31,7 +31,7 @@ # is listed in the __all__ entries below. Internal helper functions that are # private to this module should start with an underscore. If a function does not # start with an underscore AND it is not in __all__, that function is considered -# to be callable only by other apps in the authoring package. +# to be callable only by other applets in the openedx_content package. __all__ = [ "get_or_create_component_type", "create_component", diff --git a/src/openedx_content/applets/containers/api.py b/src/openedx_content/applets/containers/api.py index ec41e73f7..56f815f23 100644 --- a/src/openedx_content/applets/containers/api.py +++ b/src/openedx_content/applets/containers/api.py @@ -43,7 +43,7 @@ # is listed in the __all__ entries below. Internal helper functions that are # private to this module should start with an underscore. If a function does not # start with an underscore AND it is not in __all__, that function is considered -# to be callable only by other apps in the authoring package. +# to be callable only by other applets in the openedx_content package. __all__ = [ "ContainerSubclass", "ContainerImplementationMissingError", diff --git a/src/openedx_content/applets/media/api.py b/src/openedx_content/applets/media/api.py index f36c71a25..bf67d4cfe 100644 --- a/src/openedx_content/applets/media/api.py +++ b/src/openedx_content/applets/media/api.py @@ -21,7 +21,7 @@ # is listed in the __all__ entries below. Internal helper functions that are # private to this module should start with an underscore. If a function does not # start with an underscore AND it is not in __all__, that function is considered -# to be callable only by other apps in the authoring package. +# to be callable only by other applets in the openedx_content package. __all__ = [ "get_or_create_media_type", "get_media", diff --git a/src/openedx_content/applets/publishing/api.py b/src/openedx_content/applets/publishing/api.py index 5fda59772..770a049da 100644 --- a/src/openedx_content/applets/publishing/api.py +++ b/src/openedx_content/applets/publishing/api.py @@ -41,7 +41,7 @@ # is listed in the __all__ entries below. Internal helper functions that are # private to this module should start with an underscore. If a function does not # start with an underscore AND it is not in __all__, that function is considered -# to be callable only by other apps in the authoring package. +# to be callable only by other applets in the openedx_content package. __all__ = [ "get_learning_package", "get_learning_package_by_ref", From c8e618704adc128d56685a8ede963800add8c347 Mon Sep 17 00:00:00 2001 From: Kyle D McCormick Date: Tue, 28 Apr 2026 10:49:13 -0400 Subject: [PATCH 10/10] feat!: Make Timestamp and Author params consistent across Publishing (WIP) --- src/openedx_content/applets/publishing/api.py | 266 +++++++++++++++--- 1 file changed, 223 insertions(+), 43 deletions(-) diff --git a/src/openedx_content/applets/publishing/api.py b/src/openedx_content/applets/publishing/api.py index 770a049da..c49c4845a 100644 --- a/src/openedx_content/applets/publishing/api.py +++ b/src/openedx_content/applets/publishing/api.py @@ -9,7 +9,7 @@ from contextlib import nullcontext from datetime import datetime, timezone -from typing import ContextManager, Optional, cast +from typing import ContextManager, Optional, cast, TypeAlias, Literal from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist @@ -43,6 +43,12 @@ # start with an underscore AND it is not in __all__, that function is considered # to be callable only by other applets in the openedx_content package. __all__ = [ + "Timestamp", + "TIMESTAMP_AUTO", + "UserID", + "AUTHOR_AUTO", + "AUTHOR_NONE", + "Author", "get_learning_package", "get_learning_package_by_ref", "create_learning_package", @@ -74,6 +80,65 @@ ] +TIMESTAMP_AUTO: Literal["TIMESTAMP_AUTO"] = "TIMESTAMP_AUTO" +""" +Automatically choose a timestamp for a creation, deletion, edit, or change. + +Within a `bulk_draft_changes_for` context, `TIMESTAMP_AUTO` indicates to use the +context's timestamp. Outside of `bulk_draft_changes_for`, `TIMESTAMP_AUTO` indicates +to the current time (`datetime.now`). + +`TIMESTAMP_AUTO` is a reasonable default for most situations. +""" + + +type Timestamp = datetime | Literal["TIMESTAMP_AUTO"] +""" +How to choose a datetime indicating when an operation occured. + +This is either a specific `datetime`, or it is `TIMESTAMP_AUTO`. +When in doubt, `TIMESTAMP_AUTO` is a reasonable default. +""" + + +UserID: TypeAlias = int +""" +A UserID is an integer. + +A UserID is meant to hold a primary key of a User object, but this is not +enforced by the type checker. Future versions of openedx-core may enforce +this more strictly by making UserID a NewType (subclass) of int. +""" + + +AUTHOR_NONE: Literal["AUTHOR_NONE"] = "AUTHOR_NONE" +""" +The operation cannot be attributed to a particular user. + +Most operations can and should be attributed to a user; AUTHOR_NONE is to be +reserved for special scenarios, such as data backfills. AUTHOR_NONE is not a +reasonable default for standard application API usage--consider AUTO_AUTHOR +instead. +""" + + +AUTHOR_AUTO: Literal["AUTHOR_AUTO"] = "AUTHOR_AUTO" +""" +Attribute the operation to the same user as the enclosing `bulk_draft_changes_for` context. + +Outside of a context, using `AUTHOR_AUTO` is meaningless and will result in a `ValueError`. +""" + + +type Author = UserID | Literal["AUTHOR_AUTO"] | Literal["AUTHOR_NONE"] +""" +How to attribute an operation (creation, edit, deletion, change) to a user. + +This is either the database ID of a `User`, or it is `AUTHOR_NONE`, or it is `AUTHOR_AUTO`. +When in doubt, `AUTHOR_AUTO` is a reasonable default. +""" + + def get_learning_package(learning_package_id: LearningPackage.ID, /) -> LearningPackage: """ Get LearningPackage by ID. @@ -124,7 +189,7 @@ def update_learning_package( package_ref: str | None = None, title: str | None = None, description: str | None = None, - updated: datetime | None = None, + updated: Timestamp = TIMESTAMP_AUTO, ) -> LearningPackage: """ Make an update to LearningPackage metadata. @@ -147,7 +212,7 @@ def update_learning_package( # updated is a bit differentโ€“we auto-generate it if it's not explicitly # passed in. - if updated is None: + if updated is TIMESTAMP_AUTO: updated = datetime.now(tz=timezone.utc) lp.updated = updated @@ -162,13 +227,101 @@ def learning_package_exists(package_ref: str) -> bool: return LearningPackage.objects.filter(package_ref=package_ref).exists() +def resolve_timestamp( + learning_package_id: LearningPackage.ID, + timestamp: Timestamp, + *, + can_override_context: bool = True, +) -> datetime: + """ + Convert a Timestamp specifier into a concerete datetime to be saved to the DB. + + If `timestamp is TIMESTAMP_AUTO`, then return either the datetime of the + enclosing `bulk_draft_changes_for` context, or `datetime.now` if there is + no enclosing context. + + If `can_override_context==False`, then raise ValueError if there's an enclosing + context whose timestamp is incompatible with the supplied timestamp. This is only + appropriate for the the handful of operations which cannot be timestamped differently + than their context + + This function can be used by other openedx_content applets, but it is not part of the public API. + """ + if timestamp is TIMESTAMP_AUTO: + if ctx := DraftChangeLogContext.get_active_draft_change_log(learning_package_id): + return ctx.changed_at + else: + return datetime.now(tz=timezone.utc) + assert isinstance(timestamp, datetime) + # Timestamp is specified as a datetime... + if can_override_context: + return timestamp + # If we get here, then timestamp is specified, but we can't override the context's timestamp. + # So, if there's a context, ensure they match, and raise if they don't. + if ctx := DraftChangeLogContext.get_active_draft_change_log(learning_package_id): + if ctx.changed_at != timestamp: + raise ValueError( + f"Tried to give operation the timestamp {timestamp}, but this operation requires " + f"using the same timestamp as the enclosing context, which is {ctx.changed_at}." + ) + return ctx.changed_at + # No change context -- use the specified timestamp. + return timestamp + + +def resolve_author( + learning_package_id: LearningPackage.ID, + author: Author, + *, + can_override_context: bool = True, +) -> UserID | None: + """ + Convert a Author specifier into a concerete user ID (or None) to be saved to the DB. + + If `author is AUTHOR_AUTO`, then return either the author of the enclosing + `bulk_draft_changes_for` context. Raises `ValueError` if there is no + enclosing context. + + If `author is AUTHOR_NONE`, then return None. + + If `can_override_context==False`, then raise `ValueError` if there's an enclosing + context whose author is incompatible with the supplied author. This is only + appropriate for the the handful of operations which cannot be attributed differently + than their context. + + This function can be used by other openedx_content applets, but it is not part of the public API. + """ + if author is AUTHOR_AUTO: + if ctx := DraftChangeLogContext.get_active_draft_change_log(learning_package_id): + return ctx.changed_by.id if ctx.changed_by else None + raise ValueError( + "An author (other than AUTHOR_AUTO) must be specified when changing content " + "outside of a `bulk_draft_changes_for` context." + ) + author_id: UserID | None = None if author is AUTHOR_NONE else cast(UserID, author) + # author is UserID or None... + if can_override_context: + return author_id + # If we get here, then author is UserID/None, but we can't override the context's user. + # So, if there's a context, ensure they match, and raise if they don't. + if ctx := DraftChangeLogContext.get_active_draft_change_log(learning_package_id): + ctx_author_id = ctx.changed_by.id if ctx.changed_by else None + if ctx_author_id != author_id: + raise ValueError( + f"Tried to attribute operation to author {author_id}, but this operation can only be " + "attributed to the author of the enclosing `bulk_draft_changes_for` context, " + f"which is {ctx_author_id}." + ) + # No change context -- use the specified author. + return author_id + + def create_publishable_entity( learning_package_id: LearningPackage.ID, /, entity_ref: str, - created: datetime, - # User ID who created this - created_by: int | None, + created: Timestamp = TIMESTAMP_AUTO, + created_by: Author = AUTHOR_AUTO, *, can_stand_alone: bool = True, ) -> PublishableEntity: @@ -178,11 +331,12 @@ def create_publishable_entity( You'd typically want to call this right before creating your own content model that points to it. """ + return PublishableEntity.objects.create( learning_package_id=learning_package_id, entity_ref=entity_ref, - created=created, - created_by_id=created_by, + created=resolve_timestamp(learning_package_id, created), + created_by_id=resolve_author(learning_package_id, created_by), can_stand_alone=can_stand_alone, ) @@ -192,8 +346,8 @@ def create_publishable_entity_version( /, version_num: int, title: str, - created: datetime, - created_by: int | None, + created: Timestamp = TIMESTAMP_AUTO, + created_by: Author = AUTHOR_AUTO, *, dependencies: list[int] | None = None, # PublishableEntity IDs ) -> PublishableEntityVersion: @@ -203,13 +357,14 @@ def create_publishable_entity_version( You'd typically want to call this right before creating your own content version model that points to it. """ + learning_package_id = get_publishable_entity(entity_id).learning_package_id with atomic(savepoint=False): version = PublishableEntityVersion.objects.create( entity_id=entity_id, version_num=version_num, title=title, - created=created, - created_by_id=created_by, + created=resolve_timestamp(learning_package_id, created), + created_by_id=resolve_author(learning_package_id, created_by), ) if dependencies: set_version_dependencies(version.id, dependencies) @@ -376,8 +531,8 @@ def publish_all_drafts( learning_package_id: LearningPackage.ID, /, message="", - published_at: datetime | None = None, - published_by: int | None = None + published_at: Timestamp = TIMESTAMP_AUTO, + published_by: Author = AUTHOR_AUTO, ) -> PublishLog: """ Publish everything that is a Draft and is not already published. @@ -430,8 +585,8 @@ def publish_from_drafts( /, draft_qset: QuerySet[Draft], message: str = "", - published_at: datetime | None = None, - published_by: int | None = None, # User.id + published_at: Timestamp = TIMESTAMP_AUTO, + published_by: Author = AUTHOR_AUTO, *, publish_dependencies: bool = True, ) -> PublishLog: @@ -441,9 +596,6 @@ def publish_from_drafts( By default, this will also publish all dependencies (e.g. unpinned children) of the Drafts that are passed in. """ - if published_at is None: - published_at = datetime.now(tz=timezone.utc) - with atomic(): if publish_dependencies: dependency_drafts_qsets = _get_dependencies_with_unpublished_changes(draft_qset) @@ -457,8 +609,8 @@ def publish_from_drafts( publish_log = PublishLog( learning_package_id=learning_package_id, message=message, - published_at=published_at, - published_by_id=published_by, + published_at=resolve_timestamp(learning_package_id, published_at), + published_by_id=resolve_author(learning_package_id, published_by), ) publish_log.full_clean() publish_log.save(force_insert=True) @@ -843,8 +995,8 @@ def set_draft_version( draft_or_id: Draft | PublishableEntity.ID, publishable_entity_version_pk: int | None, /, - set_at: datetime | None = None, - set_by: int | None = None, # User.id + set_at: Timestamp = TIMESTAMP_AUTO, + set_by: Author = AUTHOR_AUTO, ) -> None: """ Modify the Draft of a PublishableEntity to be a PublishableEntityVersion. @@ -869,9 +1021,6 @@ def set_draft_version( containers, as bulk_draft_changes_for will automatically do that when the block exits. """ - if set_at is None: - set_at = datetime.now(tz=timezone.utc) - with atomic(savepoint=False): if isinstance(draft_or_id, Draft): draft = draft_or_id @@ -884,6 +1033,17 @@ def set_draft_version( f"draft_or_id must be a Draft or int, not ({class_name})" ) + # If there is an active DraftChangeLog context, then timestamp & authorship cannot be + # overriden. That's because we have nowhere to put the custom timesetamp & author + # other than the DraftChangeLog model itself! + learning_package_id = draft.entity.learning_package_id + set_at_dt: datetime = resolve_timestamp( + learning_package_id, set_at, can_override_context=False + ) + set_by_id: UserID | None = resolve_author( + learning_package_id, set_by, can_override_context=False + ) + # If the Draft is already pointing at this version, there's nothing to do. old_version_id = draft.version_id if old_version_id == publishable_entity_version_pk: @@ -951,8 +1111,8 @@ def set_draft_version( # DraftChangeLogRecord, because we know it can't exist yet. change_log = DraftChangeLog.objects.create( learning_package_id=learning_package_id, - changed_at=set_at, - changed_by_id=set_by, + changed_at=set_at_dt, + changed_by_id=set_by_id, ) draft.draft_log_record = DraftChangeLogRecord.objects.create( draft_change_log=change_log, @@ -1452,7 +1612,12 @@ def hash_for_log_record( return digest -def soft_delete_draft(publishable_entity_id: PublishableEntity.ID, /, deleted_by: int | None = None) -> None: +def soft_delete_draft( + publishable_entity_id: PublishableEntity.ID, + /, + deleted_at: Timestamp = TIMESTAMP_AUTO, + deleted_by: Author = AUTHOR_AUTO, +) -> None: """ Sets the Draft version to None. @@ -1462,14 +1627,14 @@ def soft_delete_draft(publishable_entity_id: PublishableEntity.ID, /, deleted_by of pointing the Draft back to the most recent ``PublishableEntityVersion`` for a given ``PublishableEntity``. """ - return set_draft_version(publishable_entity_id, None, set_by=deleted_by) + return set_draft_version(publishable_entity_id, None, set_at=deleted_at, set_by=deleted_by) def reset_drafts_to_published( learning_package_id: LearningPackage.ID, /, - reset_at: datetime | None = None, - reset_by: int | None = None, # User.id + reset_at: Timestamp = TIMESTAMP_AUTO, + reset_by: Author = AUTHOR_AUTO, ) -> None: """ Reset all Drafts to point to the most recently Published versions. @@ -1495,9 +1660,6 @@ def reset_drafts_to_published( latest version created for a PublishableEntity (its ``latest`` attribute), rather than basing it off of the version that Draft points to. """ - if reset_at is None: - reset_at = datetime.now(tz=timezone.utc) - # These are all the drafts that are different from the published versions. draft_qset = Draft.objects \ .select_related("entity__published") \ @@ -1518,6 +1680,16 @@ def reset_drafts_to_published( if not draft_qset: return + # If there is an active DraftChangeLog context, then timestamp & authorship cannot be + # overriden. That's because we have nowhere to put the custom timesetamp & author + # other than the DraftChangeLog model itself! + reset_at_dt: datetime = resolve_timestamp( + learning_package_id, reset_at, can_override_context=False + ) + reset_by_id: UserID | None = resolve_author( + learning_package_id, reset_by, can_override_context=False + ) + active_change_log = DraftChangeLogContext.get_active_draft_change_log(learning_package_id) # If there's an active DraftChangeLog, we're already in a transaction, so @@ -1527,7 +1699,7 @@ def reset_drafts_to_published( tx_context = nullcontext() else: tx_context = bulk_draft_changes_for( - learning_package_id, changed_at=reset_at, changed_by=reset_by + learning_package_id, changed_at=reset_at_dt, changed_by=(reset_by_id or AUTHOR_NONE), ) with tx_context: @@ -1617,8 +1789,8 @@ def get_published_version_as_of( def bulk_draft_changes_for( learning_package_id: LearningPackage.ID, - changed_by: int | None = None, - changed_at: datetime | None = None + changed_by: UserID | Literal["AUTHOR_NONE"], # Unlike in other functions, this is required! No AUTHOR_AUTO. + changed_at: Timestamp = TIMESTAMP_AUTO, ) -> DraftChangeLogContext: """ Context manager to do a single batch of Draft changes in. @@ -1634,7 +1806,7 @@ def bulk_draft_changes_for( Example:: - with bulk_draft_changes_for(learning_package.id): + with bulk_draft_changes_for(learning_package.id, changed_by=request.user.id): for section in course: update_section_drafts(learning_package.id, section) @@ -1642,17 +1814,25 @@ def bulk_draft_changes_for( the individual change (and its side effects) will be automatically wrapped in a one-off change context. For example, this:: - update_one_component(component.learning_package, component) + update_one_component(component.learning_package, component, changed_by=request.user.id) is identical to this:: - with bulk_draft_changes_for(component.learning_package.id): + with bulk_draft_changes_for(component.learning_package.id, changed_by=request.user.id): update_one_component(component.learning_package.id, component) """ + if changed_at is TIMESTAMP_AUTO: + changed_at_dt = datetime.now(tz=timezone.utc) + else: + changed_at_dt = cast(datetime, changed_at) + if changed_by is AUTHOR_NONE: + changed_by_id = None + else: + changed_by_id = cast(UserID, changed_by) return DraftChangeLogContext( learning_package_id, - changed_at=changed_at, - changed_by=changed_by, + changed_at=changed_at_dt, + changed_by=changed_by_id, exit_callbacks=[ _create_side_effects_for_change_log, ]