-
Notifications
You must be signed in to change notification settings - Fork 27
feat: import sections, subsections, and units from TOML into Learning Core #392
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
fd12fe6
1dcfad6
2a6ba98
d95fa7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,16 +6,32 @@ | |
| from openedx_learning.apps.authoring.components import api as components_api | ||
|
|
||
|
|
||
| class ComponentSerializer(serializers.Serializer): # pylint: disable=abstract-method | ||
| class EntitySerializer(serializers.Serializer): # pylint: disable=abstract-method | ||
| """ | ||
| Serializer for components. | ||
| Contains logic to convert entity_key to component_type and local_key. | ||
| Serializer for publishable entities. | ||
| """ | ||
| can_stand_alone = serializers.BooleanField(required=True) | ||
| key = serializers.CharField(required=True) | ||
| created = serializers.DateTimeField(required=True) | ||
| created_by = serializers.CharField(required=True, allow_null=True) | ||
|
|
||
|
|
||
| class EntityVersionSerializer(serializers.Serializer): # pylint: disable=abstract-method | ||
| """ | ||
| Serializer for publishable entity versions. | ||
| """ | ||
| title = serializers.CharField(required=True) | ||
| entity_key = serializers.CharField(required=True) | ||
| created = serializers.DateTimeField(required=True) | ||
| created_by = serializers.CharField(required=True, allow_null=True) | ||
|
|
||
|
|
||
| class ComponentSerializer(EntitySerializer): # pylint: disable=abstract-method | ||
| """ | ||
| Serializer for components. | ||
| Contains logic to convert entity_key to component_type and local_key. | ||
| """ | ||
|
|
||
| def validate(self, attrs): | ||
| """ | ||
| Custom validation logic: | ||
|
|
@@ -31,12 +47,56 @@ def validate(self, attrs): | |
| return attrs | ||
|
|
||
|
|
||
| class ComponentVersionSerializer(serializers.Serializer): # pylint: disable=abstract-method | ||
| class ComponentVersionSerializer(EntityVersionSerializer): # pylint: disable=abstract-method | ||
| """ | ||
| Serializer for component versions. | ||
| """ | ||
| title = serializers.CharField(required=True) | ||
| entity_key = serializers.CharField(required=True) | ||
| created = serializers.DateTimeField(required=True) | ||
| created_by = serializers.CharField(required=True, allow_null=True) | ||
| content_to_replace = serializers.DictField(child=serializers.CharField(), required=True) | ||
|
|
||
|
|
||
| class ContainerSerializer(EntitySerializer): # pylint: disable=abstract-method | ||
| """ | ||
| Serializer for containers. | ||
| """ | ||
| container = serializers.DictField(child=serializers.DictField(), required=True) | ||
|
|
||
| def validate(self, attrs): | ||
| """ | ||
| Custom validation logic: | ||
| parse the entity_key into (component_type, local_key). | ||
| """ | ||
| try: | ||
| container = attrs["container"] | ||
| container_type = list(container.keys())[0] | ||
| if container_type not in ("section", "subsection", "unit"): | ||
| raise ValueError(f"Invalid container type: {container_type}") | ||
| attrs["container_type"] = container_type | ||
| attrs.pop("container") # Remove the container field after processing | ||
| except ValueError as exc: | ||
| raise serializers.ValidationError({"key": str(exc)}) | ||
| return attrs | ||
|
|
||
|
|
||
| class ContainerVersionSerializer(EntityVersionSerializer): # pylint: disable=abstract-method | ||
| """ | ||
| Serializer for container versions. | ||
| """ | ||
| container = serializers.DictField(child=serializers.ListField(child=serializers.CharField()), required=True) | ||
|
|
||
| def validate(self, attrs): | ||
| """ | ||
| Custom validation logic: | ||
| parse the entity_key into (component_type, local_key). | ||
| """ | ||
| try: | ||
| container = attrs["container"] | ||
| if "children" not in container: | ||
| raise ValueError("Missing 'children' in container") | ||
| children = container["children"] | ||
| if not isinstance(children, list): | ||
| raise ValueError("'children' must be a list") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally, single-field validation is done with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changes applied. Thank you. |
||
| attrs["children"] = children | ||
| attrs.pop("container") # Remove the container field after processing | ||
| except ValueError as exc: | ||
| raise serializers.ValidationError({"key": str(exc)}) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I see, we're aggregating errors and re-raising. The nice thing about using separate
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, good catch. I've added those errors in an array. Thank you. |
||
| return attrs | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,8 @@ def _get_toml_publishable_entity_table( | |
| [entity.published] | ||
| version_num = 1 | ||
|
|
||
| [entity.container.section] | ||
|
|
||
| Note: This function returns a tomlkit.items.Table, which represents | ||
| a string-like TOML fragment rather than a complete TOML document. | ||
| """ | ||
|
|
@@ -83,6 +85,22 @@ def _get_toml_publishable_entity_table( | |
| else: | ||
| published_table.add(tomlkit.comment("unpublished: no published_version_num")) | ||
| entity_table.add("published", published_table) | ||
|
|
||
| if hasattr(entity, "container"): | ||
| container_table = tomlkit.table() | ||
| container_map = { | ||
| "section": "section", # name of the container class : name of the toml table | ||
| "subsection": "subsection", | ||
| "unit": "unit", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not clear what the purpose of this map is, if all the values map to themselves?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought the class name could be different from the TOML table value, but I changed it since it is the same value for now. |
||
| } | ||
|
|
||
| for container_class, container_type in container_map.items(): | ||
| if hasattr(entity.container, container_class): | ||
| container_table.add(container_type, tomlkit.table()) | ||
| break # stop after the first match | ||
|
|
||
| entity_table.add("container", container_table) | ||
|
|
||
| return entity_table | ||
|
|
||
|
|
||
|
|
@@ -118,12 +136,14 @@ def toml_publishable_entity( | |
|
|
||
| [version.container.unit] | ||
| """ | ||
| # Create the TOML representation for the entity itself | ||
| entity_table = _get_toml_publishable_entity_table(entity, draft_version, published_version) | ||
| doc = tomlkit.document() | ||
| doc.add("entity", entity_table) | ||
|
|
||
| # Add versions as an array of tables (AoT) | ||
| doc.add(tomlkit.nl()) | ||
| doc.add(tomlkit.comment("### Versions")) | ||
|
|
||
| for entity_version in versions_to_write: | ||
| version = tomlkit.aot() | ||
| version_table = toml_publishable_entity_version(entity_version) | ||
|
|
@@ -164,9 +184,6 @@ def toml_publishable_entity_version(version: PublishableEntityVersion) -> tomlki | |
| children = publishing_api.get_container_children_entities_keys(version.containerversion) | ||
| container_table.add("children", children) | ||
|
|
||
| unit_table = tomlkit.table() | ||
|
|
||
| container_table.add("unit", unit_table) | ||
| version_table.add("container", container_table) | ||
| return version_table # For use in AoT | ||
|
|
||
|
|
@@ -231,8 +248,4 @@ def parse_publishable_entity_toml(content: str) -> tuple[Dict[str, Any], list]: | |
| raise ValueError("Invalid publishable entity TOML: missing 'entity' section") | ||
| if "version" not in pe_data: | ||
| raise ValueError("Invalid publishable entity TOML: missing 'version' section") | ||
| if "key" not in pe_data["entity"]: | ||
| raise ValueError("Invalid publishable entity TOML: missing 'key' field") | ||
| if "can_stand_alone" not in pe_data["entity"]: | ||
| raise ValueError("Invalid publishable entity TOML: missing 'can_stand_alone' field") | ||
| return pe_data["entity"], pe_data.get("version", []) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should raise
serializers.ValidationError.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Thank yout.