Skip to content
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

feat: enhance importdemocourse ; add importdemolibraries #998

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions changelog.d/20240110_101228_kyle_importnewdemocourse.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
- [Feature] Several enhancements to the Demo Course (by @kdmccormick):
- The [Open edX Demo Course](https://github.com/openedx/openedx-demo-course) has been re-built from scratch with up-to-date instruction-focused content. Its directory structure has changed.
- In order to support both the old and new structures of the Demo Course's repository, the command `tutor local do importdemocourse` will now auto-determine the course root based on the location of `course.xml`. Use the `--repo-dir` argument to override this behavior.
- The new command `tutor local do importdemolibraries` will import any content libraries defined within the Demo Course repository. At the moment, that is just the "Respiratory System Question Bank", which is an optional but helpful extension to the new Demo Course.
- To try out the new Demo Course now, run: `tutor local do importdemocourse --version master`.
- To try out the demo Respiratory System Question Bank now, run: `tutor local do importdemolibraries --version master`.
- To revert back to an older Demo Course version at any point, run: `tutor local do importdemocourse --version open-release/quince.2`, replacing `quince.2` with your preferred course version.
27 changes: 26 additions & 1 deletion tests/commands/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,32 @@ def test_import_demo_course(self) -> None:
self.assertEqual(0, result.exit_code)
self.assertIn("cms-job", dc_args)
self.assertIn(
"git clone https://github.com/openedx/edx-demo-course", dc_args[-1]
"git clone https://github.com/openedx/openedx-demo-course", dc_args[-1]
)

def test_import_demo_libraries(self) -> None:
with temporary_root() as root:
self.invoke_in_root(root, ["config", "save"])
with patch("tutor.utils.docker_compose") as mock_docker_compose:
result = self.invoke_in_root(
root,
[
"local",
"do",
"importdemolibraries",
"admin",
],
)
dc_args, _dc_kwargs = mock_docker_compose.call_args
self.assertIsNone(result.exception)
self.assertEqual(0, result.exit_code)
self.assertIn("cms-job", dc_args)
self.assertIn(
"git clone https://github.com/openedx/openedx-demo-course", dc_args[-1]
)
self.assertIn(
"./manage.py cms import_content_library /tmp/library.tar.gz admin",
dc_args[-1],
)

def test_set_theme(self) -> None:
Expand Down
76 changes: 72 additions & 4 deletions tutor/commands/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def create_user_template(
@click.option(
"-r",
"--repo",
default="https://github.com/openedx/edx-demo-course",
default="https://github.com/openedx/openedx-demo-course",
show_default=True,
help="Git repository that contains the course to be imported",
)
Expand All @@ -133,7 +133,7 @@ def create_user_template(
"--repo-dir",
default="",
show_default=True,
help="Git relative subdirectory to import data from",
help="Git relative subdirectory to import data from. If unspecified, will default to the directory containing course.xml",
)
@click.option(
"-v",
Expand All @@ -145,15 +145,82 @@ def importdemocourse(
) -> t.Iterable[tuple[str, str]]:
version = version or "{{ OPENEDX_COMMON_VERSION }}"
template = f"""
# Import demo course
# Clone the repo
git clone {repo} --branch {version} --depth 1 /tmp/course
python ./manage.py cms import ../data /tmp/course/{repo_dir}

# Determine root directory for course import. If one is provided, use that.
# Otherwise, use the directory containing course.xml, failing if there isn't exactly one.
if [ -n "{repo_dir}" ] ; then
course_root=/tmp/course/{repo_dir}
else
course_xml_first="$(find /tmp/course -name course.xml | head -n 1)"
course_xml_extra="$(find /tmp/course -name course.xml | tail -n +2)"
Copy link
Contributor

Choose a reason for hiding this comment

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

the force is strong with this bash developer.

echo "INFO: Found course.xml files(s): $course_xml_first $course_xml_extra"
if [ -z "$course_xml_first" ] ; then
echo "ERROR: Could not find course.xml. Are you sure this is the right repository?"
exit 1
fi
if [ -n "$course_xml_extra" ] ; then
echo "ERROR: Found multiple course.xml files--course root is ambiguous!"
echo " Please specify a course root dir (relative to repo root) using --repo-dir."
exit 1
fi
course_root="$(dirname "$course_xml_first")"
fi
echo "INFO: Will import course data at: $course_root" && echo

# Import into CMS
python ./manage.py cms import ../data "$course_root"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to automatically import demo libraries if we detect a library.xml file? There would remain the difficulty of defining the importing user name.

We don't have to decide this now, this would be a new feature that would land in Redwood.

Copy link
Collaborator Author

@kdmccormick kdmccormick Feb 8, 2024

Choose a reason for hiding this comment

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

There would remain the difficulty of defining the importing user name.

That is indeed the difficulty :) In my previous attempt I had made it so the library is imported iff a library owner is provided. It think it was slightly counterintuitive that --library-owner controls whether or not libraries are imported.

I do think the separate importdemolibraries command makes it very clear what is going on, and it is not much more typing for someone to do, so personally I am OK leaving it as-is.


# Re-index courses
./manage.py cms reindex_course --all --setup"""
yield ("cms", template)


@click.command(help="Import the demo content libraries")
@click.argument("owner_username")
@click.option(
"-r",
"--repo",
default="https://github.com/openedx/openedx-demo-course",
show_default=True,
help="Git repository that contains the library/libraries to be imported",
)
@click.option(
"-v",
"--version",
help="Git branch, tag or sha1 identifier. If unspecified, will default to the value of the OPENEDX_COMMON_VERSION setting.",
)
def importdemolibraries(
owner_username: str, repo: str, version: t.Optional[str]
) -> t.Iterable[tuple[str, str]]:
version = version or "{{ OPENEDX_COMMON_VERSION }}"
template = f"""
# Clone the repo
git clone {repo} --branch {version} --depth 1 /tmp/library

# Fail loudly if:
# * there no library.xml files, or
# * any library.xml is not within a directory named "library/" (upstream edx-platform expectation).
if ! find /tmp/library -name library.xml | grep -q "." ; then
echo "ERROR: No library.xml files found in repository. Are you sure this is the right repository and version?"
exit 1
fi

# For every library.xml file, create a tar of its parent directory, and import into CMS.
for lib_root in $(find /tmp/library -name library.xml | xargs dirname) ; do
echo "INFO: Will import library at $lib_root"
if [ "$(basename "$lib_root")" != "library" ] ; then
echo "ERROR: can only import library.xml files that are within a directory named 'library'"
exit 1
fi
rm -rf /tmp/library.tar.gz
( cd "$(dirname "$lib_root")" && tar czvf /tmp/library.tar.gz library )
yes | ./manage.py cms import_content_library /tmp/library.tar.gz {owner_username}
done"""
yield ("cms", template)


@click.command(
name="print-edx-platform-setting",
help="Print the value of an edx-platform Django setting.",
Expand Down Expand Up @@ -324,6 +391,7 @@ def do_callback(service_commands: t.Iterable[tuple[str, str]]) -> None:
[
createuser,
importdemocourse,
importdemolibraries,
initialise,
print_edx_platform_setting,
settheme,
Expand Down
Loading