-
Notifications
You must be signed in to change notification settings - Fork 27
feat: include learning package metadata in the backup dump file #402
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 1 commit
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,14 +6,20 @@ | |
| from typing import Any, Dict | ||
|
|
||
| import tomlkit | ||
| from django.conf import settings | ||
|
|
||
| from openedx_learning.apps.authoring.collections.models import Collection | ||
| from openedx_learning.apps.authoring.publishing import api as publishing_api | ||
| from openedx_learning.apps.authoring.publishing.models import PublishableEntity, PublishableEntityVersion | ||
| from openedx_learning.apps.authoring.publishing.models.learning_package import LearningPackage | ||
|
|
||
|
|
||
| def toml_learning_package(learning_package: LearningPackage, timestamp: datetime) -> str: | ||
| def toml_learning_package( | ||
| learning_package: LearningPackage, | ||
| timestamp: datetime, | ||
| format_version: int = 1, | ||
| user: str | None = None | ||
| ) -> str: | ||
| """ | ||
| Create a TOML representation of the learning package. | ||
|
|
||
|
|
@@ -28,13 +34,23 @@ def toml_learning_package(learning_package: LearningPackage, timestamp: datetime | |
| updated = 2025-09-03T17:50:59.536190Z | ||
| """ | ||
| doc = tomlkit.document() | ||
| doc.add(tomlkit.comment(f"Datetime of the export: {timestamp}")) | ||
|
|
||
| # Learning package main info | ||
| section = tomlkit.table() | ||
| section.add("title", learning_package.title) | ||
| section.add("key", learning_package.key) | ||
| section.add("description", learning_package.description) | ||
| section.add("created", learning_package.created) | ||
| section.add("updated", learning_package.updated) | ||
|
|
||
| # Learning package metadata | ||
| metadata = tomlkit.table() | ||
| metadata.add("format_version", format_version) | ||
| metadata.add("created_by", user or "unknown") | ||
|
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. If we don't have user information, let's just omit the field. The string "unknown" is a highly unlikely, but technically legal username. Omitting the field is more or less the TOML version of making null.
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. Got it. Done. Thanks. |
||
| metadata.add("created_at", timestamp) | ||
| metadata.add("origin_server", getattr(settings, "CMS_BASE", "unknown")) | ||
|
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. Argh. I forgot that we'd need to pipe this value from edx-platform settings if we were to include it. We don't want to start the precedent of openedx-learning knowing about edx-platform specific settings. Let's just make this an optional param on the function for now, instead of reading from the settings. Libraries code in edx-platform can pass it in later. This is another one of those fields that we can omit if there's no value passed in.
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 see. The value needs to be provided from edx-platform. Changes applied. |
||
|
|
||
| doc.add("meta", metadata) | ||
| doc.add("learning_package", section) | ||
| return tomlkit.dumps(doc) | ||
|
|
||
|
|
||
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.
This should take an actual User object, so that we can add more info about the user later if we decide it's prudent, e.g. their email.
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.
Makes sense. Changes have been applied.