Skip to content

fix: allow restoring content with blank titles #610

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.

Sign up for GitHub

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

Merged
ormsbee merged 2 commits into from
Jun 25, 2026
Merged

fix: allow restoring content with blank titles #610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08fcf3a
fix: allow restoring content with blank titles
kdmccormick Jun 22, 2026
9ed3676
docs: Add a comment to the TOML with a blank unit title
kdmccormick Jun 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
4 changes: 3 additions & 1 deletion src/openedx_content/applets/backup_restore/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ class EntityVersionSerializer(serializers.Serializer): # pylint: disable=abstra
"""
Serializer for publishable entity versions.
"""
title = serializers.CharField(required=True)
# We allow_blank because empty unit titles are legal and common.

@kdmccormick kdmccormick Jun 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question for reviewers:
This allows empty titles in all entities. Should we only allow it in Units instead?

@ormsbee ormsbee Jun 23, 2026

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.

Allow for all. It happens for components as well, especially on older courses.

@ormsbee ormsbee Jun 23, 2026 •
edited
Loading

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.

Sorry, I wrote that comment in a hurry and I should give more context. We try to force titles for all the components at the editor level these days, because they were being displayed individually by the mobile app (though I think this has since moved back to Unit-level display?). Regardless, for a long while prior to that, some course authors would put titles only at the Unit level, because otherwise it can look goofy from the student view, especially when a Video and HTML block are all on the same topic, and the component title is either directly repeating the Unit title "Intro to Foo" -> "Intro to Foo" or is a useless description like "Video".

Also of note is that whitespace is legal as a title in modulestore, i.e. it is legal to store " " as the display_name, but it gets substituted with the XBlock default display name depending on type. So the field data really is " ", but it's currently displayed as "problem" to the user in the LMS. I think it's okay to strip this whitespace on save, even if that's technically modifying it.

@kdmccormick kdmccormick Jun 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, ty!

title = serializers.CharField(required=True, allow_blank=True)

created = serializers.DateTimeField(required=True, default_timezone=timezone.utc)
version_num = serializers.IntegerField(required=True)

Expand Down
2 changes: 1 addition & 1 deletion ...openedx_content/applets/backup_restore/fixtures/library_backup/entities/unit1-b7eafb.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ version_num = 2
# ### Versions

[[version]]
title = "Unit1"
title = "" # Intentionally blank in order to test that empty Unit names work
version_num = 2

[version.container]
Expand Down
23 changes: 23 additions & 0 deletions tests/openedx_content/applets/backup_restore/test_restore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,29 @@ def test_error_no_metadata_section(self):
expected_error = "Errors encountered during restore:\npackage.toml meta section: {'non_field_errors': [Er"
assert expected_error in log_content

def test_restore_with_blank_unit_title(self):
"""
Restoring should succeed when a container version has a blank title.

Blank titles are legal and common -- content imported from courses
(e.g. via the modulestore migrator) frequently has untitled units, and
such content can be backed up. Restoring that same archive must work.

The ``library_backup`` fixture's ``unit1`` deliberately has a blank
title to exercise this path.
"""
result = LearningPackageUnzipper(self.zip_file, package_ref="lib-xx:WGU:LIB_C001").load()

assert result["status"] == "success", f"Restore failed: {result['log_file_error']}"
assert result["log_file_error"] is None

lp = publishing_api.LearningPackage.objects.get(package_ref="lib-xx:WGU:LIB_C001")
unit = containers_api.get_containers(learning_package_id=lp.id).get(
publishable_entity__entity_ref="unit1-b7eafb"
)
draft_version = publishing_api.get_draft_version(unit.publishable_entity.id)
assert draft_version.title == ""

def test_success_metadata_using_user_context(self):
"""Test that metadata is correctly extracted from learning_package.toml."""
restore_result = LearningPackageUnzipper(self.zip_file, user=self.user).load()
Expand Down