Draft
Conversation
497edf4 to
dac911e
Compare
This was referenced Apr 2, 2026
dac911e to
1f1cb48
Compare
Comment on lines
+131
to
+132
| validators=[ | ||
| RegexValidator( |
Contributor
There was a problem hiding this comment.
Django now supports doing regex validations via database constraints, like:
openedx-core/src/openedx_catalog/models/catalog_course.py
Lines 238 to 239 in 5391abc
This has the advantage that it's enforced at the database level, with perfect consistency, which is much better because Django validators are not normally called on .update() or .save(). But if you define a check constraint, not only does it run at the database level, but Django also runs it as a python-level validator automatically, so you get all the same advantages of a validator.
The only thing is, I'm not sure offhand if there's any way to specify constraints like this at the field level, only at the model level 😞
| # a single LearningPackage through our publishable_entity relation. However, | ||
| # having this foreign key directly allows us to make indexes that efficiently | ||
| # query by other Container fields within a given LearningPackage. | ||
| learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE) |
Contributor
There was a problem hiding this comment.
I know we already have an identical field for Component. I really wish we had some way to enforce that these non-normalized fields are kept up to date.
Also, standardize internal usage of collection_key to collection_code. This helps clarify that Collection.key is *not* an OpaqueKey, but is rather a local slug, which can be combined with other identifiers to form a fully- qualified LibraryCollectionKey instance. BREAKING CHANGE: Collection.key has been renamed to Collection.collection_code. BREAKING CHANGE: Collection.collection_code now validates that its contents matches '[A-Za-z0-9\-\_\.]+'. This was already effectively true, because LibraryCollectionKey can only be built with slug-like parts, but we now we explicitly raise ValiationError from create_collection. Backup now writes both 'key' and 'collection_code' to collection TOML files, so older software (which only knows 'key') can still read new archives. Restore accepts either field, preferring 'collection_code' and falling back to the legacy 'key'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Part of: #322
Renames the Component.local_key field to component_code, switching from key_field() to code_field() (stricter validation: [A-Za-z0-9\-\_\.]+, max_length=255). Updates all API call sites, backup/restore, and tests. Backup/restore now writes an [entity.component] section in each component TOML file containing component_type and component_code explicitly, so that restore does not need to parse the entity key. Old archives (without the [entity.component] section) are still accepted by falling back to the existing entity key parsing. BREAKING CHANGE: Component.local_key has been renamed to Component.component_code. BREAKING CHANGE: Component.component_code now validates against [A-Za-z0-9\-\_\.]+ and has a max_length of 255. Previously local_key used key_field() (no regex validation, max_length=500). BREAKING CHANGE: Function parameters renamed from local_key to component_code in create_component(...) and create_component_and_version(...). BREAKING CHANGE: Functions get_component_by_key(...)/component_exists_by_key(...), renamed to get_component_by_code(...)/component_exists_by_code(...), and parameters renamed from local_key to component_code. Part of: #322 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a container_code field (code_field) and a learning_package FK to the Container model. Also adds a UniqueConstraint on (learning_package, container_code), which is stricter than Component's constraint (no type scoping -- container codes must be unique across all container types within a given LearningPackage). For existing containers, container_code is backfilled from the entity key via a data migration. Future containers will have container_code set explicitly by the caller. Backup/restore now writes container_code into the [entity.container] section (Verawood and later). Archives created in Ulmo (which have no container_code) fall back to using the entity key as the container_code on restore. BREAKING CHANGE: create_container() and create_container_and_version() now require a container_code keyword argument. The same applies to create_unit_and_version(), create_subsection_and_version(), and create_section_and_version(). Part of: #322 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The `{LearningPackage,PublishableEntity}.key` fields were always
meant to be externally-supplied refs whose format does not matter.
However, they were effectively being used as parseable psuedo-keys
for identifying containers and components, with more assumptions to their
structure being baked in over time, both within and outside openedx-core.
This commit renames them to `{package,entity}_ref`, and removes all
parsing of them from the API and from the internals (except for where
required for backcompat with Ulmo ZIP backups). Instead, the pairings
of (component_type,component_code) and (container_type,container_code)
are used to identify components and containers; their respective
entity_refs are derived from those *by conventional default* but also
are customizable by callers.
BREAKING CHANGE: Renamed key_field(...) -> ref_field(...) in openedx_django_lib.
BREAKING CHANGE: Renamed PublishableEntity.key -> PublishableEntityKey.entity_ref
BREAKING CHANGE: Renamed PublishableEntityMixin.key -> PublishableEntityMixin.entity_ref
BREAKING CHANGE: In create_publishable_entity(...), renamed param key -> entity_ref
BREAKING CHANGE: Renamed get_publishable_entity_by_key(...) ->
get_publishable_entity_by_ref(...), and renamed param key -> entity_ref
BREAKING CHANGE: In get_entity_collections(...), renamed param entity_key -> entity_ref
BREAKING CHANGE: In create_component(...)/create_container(...):
* Positional key argument removed
* Optional entity_ref argument added
* Defaults to defaults to "{namespace}:{type_name}:{component_code}" if unspecified
BREAKING CHANGE: Function get_or_create_component_type_by_entity_key() removed.
BREAKING CHANGE: Rename get_container_children_entities_keys(...) ->
get_container_children_entity_refs(...)
BREAKING CHANGE: Renamed get_container_by_key(...) -> get_container_by_ref(..)
and renamed param key -> entity_ref.
BREAKING CHANGE: Renamed get_container_children_entities_keys(...) ->
get_container_children_entity_refs(...)
BRAEKING CHANGE: Renamed LearningPackage.key -> LearningPackage.package_ref.
BREAKING CHANGE: In create_learning_package(...), renamed param key -> package_ref
BREAKING CHANGE: In update_learning_package(...), renamed param key -> package_ref
BREAKING CHANGE: Renamed get_learning_package_by_key(...) ->
get_learning_package_by_ref(...), and renamed param key -> package_ref
BREAKING CHANGE: In learning_package_exists(...), renamed param key -> package_ref
BREAKING CHANGE: In look_up_component_version_media(...):
* Renamed param learning_package_key -> learning_package_ref
* Renamed param component_key -> entity_ref
BREAKING CHANGE: In add_assets_to_component management command:
* Renamed argument learning_package_key -> learning_package_ref
* Renamed argument component_key -> entity_ref
BREAKING CHANGE In load_learning_package:
* Renamed param key -> package_ref
* In return dict, within sub-dict ["lp_restored_data"]:
* Renamed ["key"] -> ["package_ref"]
* Renamed ["archive_lp_key"] -> ["archive_package_ref"]
* Renamed ["archive_org_key"] -> ["archive_org_code"]
* Renamed ["archive_slug"] -> ["archive_package_code"]
* If archive_package_ref cannot be parsed into "{_prefix}:{org_code}:{package_code}", then
archvie_org_code and archive_package_code will both be None. Callers should handle this case.
(Previously, a ValueError would be raised by backup-restore code.)
Backup/Restore format changes (non-breaking):
* In [learning_package], key field is renamed -> package_ref.
* In [entity], key field is renamed -> entity_ref.
* For compatibility with Ulmo instances, both key and {package,entity}_ref
will be written in all new archives.
* For compatibility with Ulmo archives, both key and {package,entity}_ref
will be read, with priority given to the latter.
Part of: #322
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
BREAKING CHANGE: Renamed ComponentVersionMedia.key -> ComponentVersionMedia.path
BREAKING CHANGE: In create_component_version_media(...) and
look_up_component_version_media(...), renamed param key -> path.
Part of: #322
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1f1cb48 to
7594b5d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This applies the guidance about Code, OpaqueKeys, and refs from openedx/openedx-proposals#773. Please see the individual commits for details.