Skip to content

Commit

Permalink
Merge branch 'master' into MJG/dynamic-team-partition
Browse files Browse the repository at this point in the history
  • Loading branch information
mariajgrimaldi authored Apr 9, 2024
2 parents d4bff90 + 41953bb commit deb48df
Show file tree
Hide file tree
Showing 71 changed files with 29,885 additions and 33,977 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/js-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
strategy:
matrix:
os: [ ubuntu-20.04 ]
node-version: [ 16 ]
node-version: [ 18 ]
python-version: [ 3.8 ]

steps:
Expand All @@ -28,13 +28,13 @@ jobs:
node-version: ${{ matrix.node-version }}

- name: Setup npm
run: npm i -g npm@8.5.x
run: npm i -g npm@10.5.x

- name: Install Firefox 61.0
- name: Install Firefox 123.0
run: |
sudo apt-get purge firefox
wget "https://ftp.mozilla.org/pub/firefox/releases/61.0/linux-x86_64/en-US/firefox-61.0.tar.bz2"
tar -xjf firefox-61.0.tar.bz2
wget "https://ftp.mozilla.org/pub/firefox/releases/123.0/linux-x86_64/en-US/firefox-123.0.tar.bz2"
tar -xjf firefox-123.0.tar.bz2
sudo mv firefox /opt/firefox
sudo ln -s /opt/firefox/firefox /usr/bin/firefox
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lockfileversion-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ on:

jobs:
version-check:
uses: openedx/.github/.github/workflows/lockfileversion-check.yml@master
uses: openedx/.github/.github/workflows/lockfileversion-check-v3.yml@master
4 changes: 2 additions & 2 deletions .github/workflows/static-assets-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ jobs:
matrix:
os: [ ubuntu-20.04 ]
python-version: [ 3.8 ]
node-version: [ 16 ]
npm-version: [ 8.5.x ]
node-version: [ 18 ]
npm-version: [ 10.5.x ]
mongo-version: ["4.4", "7.0"]

services:
Expand Down
1 change: 1 addition & 0 deletions .nvmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
18
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ RUN pip install -r requirements/pip.txt
RUN pip install -r requirements/edx/base.txt

# Install node and npm
RUN nodeenv /edx/app/edxapp/nodeenv --node=16.14.0 --prebuilt
RUN npm install -g npm@8.5.x
RUN nodeenv /edx/app/edxapp/nodeenv --node=18.19.0 --prebuilt
RUN npm install -g npm@10.5.x

# This script is used by an npm post-install hook.
# We copy it into the image now so that it will be available when we run `npm install` in the next step.
Expand Down
45 changes: 44 additions & 1 deletion cms/djangoapps/api/v1/serializers/course_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.db import transaction
from django.utils.translation import gettext_lazy as _
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from rest_framework import serializers
from rest_framework.fields import empty

Expand Down Expand Up @@ -198,8 +199,50 @@ def update(self, instance, validated_data):
'display_name': instance.display_name
}
fields.update(validated_data)
new_course_run_key = rerun_course(user, course_run_key, course_run_key.org, number, run, fields, False)
new_course_run_key = rerun_course(
user, course_run_key, course_run_key.org, number, run, fields, background=False,
)

course_run = get_course_and_check_access(new_course_run_key, user)
self.update_team(course_run, team)
return course_run


class CourseCloneSerializer(serializers.Serializer): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring
source_course_id = serializers.CharField()
destination_course_id = serializers.CharField()

def validate(self, attrs):
source_course_id = attrs.get('source_course_id')
destination_course_id = attrs.get('destination_course_id')
store = modulestore()
source_key = CourseKey.from_string(source_course_id)
dest_key = CourseKey.from_string(destination_course_id)

# Check if the source course exists
if not store.has_course(source_key):
raise serializers.ValidationError('Source course does not exist.')

# Check if the destination course already exists
if store.has_course(dest_key):
raise serializers.ValidationError('Destination course already exists.')
return attrs

def create(self, validated_data):
source_course_id = validated_data.get('source_course_id')
destination_course_id = validated_data.get('destination_course_id')
user = self.context['request'].user
source_course_key = CourseKey.from_string(source_course_id)
destination_course_key = CourseKey.from_string(destination_course_id)
source_course_run = get_course_and_check_access(source_course_key, user)
fields = {
'display_name': source_course_run.display_name,
}

destination_course_run_key = rerun_course(
user, source_course_key, destination_course_key.org, destination_course_key.course,
destination_course_key.run, fields, background=False,
)

destination_course_run = get_course_and_check_access(destination_course_run_key, user)
return destination_course_run
51 changes: 51 additions & 0 deletions cms/djangoapps/api/v1/tests/test_views/test_course_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,3 +402,54 @@ def test_rerun_invalid_number(self):
assert response.data == {'non_field_errors': [
'Invalid key supplied. Ensure there are no special characters in the Course Number.'
]}

def test_clone_course(self):
course = CourseFactory()
url = reverse('api:v1:course_run-clone')
data = {
'source_course_id': str(course.id),
'destination_course_id': 'course-v1:destination+course+id',
}
response = self.client.post(url, data, format='json')
assert response.status_code == 201
self.assertEqual(response.data, {"message": "Course cloned successfully."})

def test_clone_course_with_missing_source_id(self):
url = reverse('api:v1:course_run-clone')
data = {
'destination_course_id': 'course-v1:destination+course+id',
}
response = self.client.post(url, data, format='json')
assert response.status_code == 400
self.assertEqual(response.data, {'source_course_id': ['This field is required.']})

def test_clone_course_with_missing_dest_id(self):
url = reverse('api:v1:course_run-clone')
data = {
'source_course_id': 'course-v1:source+course+id',
}
response = self.client.post(url, data, format='json')
assert response.status_code == 400
self.assertEqual(response.data, {'destination_course_id': ['This field is required.']})

def test_clone_course_with_nonexistent_source_course(self):
url = reverse('api:v1:course_run-clone')
data = {
'source_course_id': 'course-v1:nonexistent+source+course_id',
'destination_course_id': 'course-v1:destination+course+id',
}
response = self.client.post(url, data, format='json')
assert response.status_code == 400
assert str(response.data.get('non_field_errors')[0]) == 'Source course does not exist.'

def test_clone_course_with_existing_dest_course(self):
url = reverse('api:v1:course_run-clone')
course = CourseFactory()
existing_dest_course = CourseFactory()
data = {
'source_course_id': str(course.id),
'destination_course_id': str(existing_dest_course.id),
}
response = self.client.post(url, data, format='json')
assert response.status_code == 400
assert str(response.data.get('non_field_errors')[0]) == 'Destination course already exists.'
47 changes: 47 additions & 0 deletions cms/djangoapps/api/v1/views/course_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from cms.djangoapps.contentstore.views.course import _accessible_courses_iter, get_course_and_check_access

from ..serializers.course_runs import (
CourseCloneSerializer,
CourseRunCreateSerializer,
CourseRunImageSerializer,
CourseRunRerunSerializer,
Expand Down Expand Up @@ -90,3 +91,49 @@ def rerun(self, request, *args, **kwargs): # lint-amnesty, pylint: disable=miss
new_course_run = serializer.save()
serializer = self.get_serializer(new_course_run)
return Response(serializer.data, status=status.HTTP_201_CREATED)

@action(detail=False, methods=['post'])
def clone(self, request, *args, **kwargs): # lint-amnesty, pylint: disable=unused-argument
"""
**Use Case**
This endpoint can be used for course cloning.
Unlike reruns, cloning a course allows creating a copy of an existing
course under a different organization name and with a different course
name.
**Example Request**
POST /api/v1/course_runs/clone/ {
"source_course_id": "course-v1:edX+DemoX+Demo_Course",
"destination_course_id": "course-v1:newOrg+newDemoX+Demo_Course_Clone"
}
**POST Parameters**
* source_course_id: a full course id of the course that will be
cloned. Has to be an id of an existing course.
* destination_course_id: a full course id of the destination
course. The organization, course name and course run of the
new course will be determined from the provided id. Has to be
an id of a course that doesn't exist yet.
**Response Values**
If the request parameters are valid and a course has been cloned
succesfully, an HTTP 201 "Created" response is returned.
If source course id and/or destination course id are invalid, or
source course doesn't exist, or destination course already exist,
an HTTP 400 "Bad Request" response is returned.
If the user that is making the request doesn't have the access to
either of the courses, an HTTP 401 "Unauthorized" response is
returned.
"""
serializer = CourseCloneSerializer(data=request.data, context=self.get_serializer_context())
serializer.is_valid(raise_exception=True)
new_course_run = serializer.save()
serializer = self.get_serializer(new_course_run)
return Response({"message": "Course cloned successfully."}, status=status.HTTP_201_CREATED)
19 changes: 14 additions & 5 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,15 @@ def _import_xml_node_to_parent(
parent_key = parent_xblock.scope_ids.usage_id
block_type = node.tag

# Modulestore's IdGenerator here is SplitMongoIdManager which is assigned
# by CachingDescriptorSystem Runtime and since we need our custom ImportIdGenerator
# here we are temporaraliy swtiching it.
original_id_generator = runtime.id_generator

# Generate the new ID:
id_generator = ImportIdGenerator(parent_key.context_key)
def_id = id_generator.create_definition(block_type, slug_hint)
usage_id = id_generator.create_usage(def_id)
runtime.id_generator = ImportIdGenerator(parent_key.context_key)
def_id = runtime.id_generator.create_definition(block_type, slug_hint)
usage_id = runtime.id_generator.create_usage(def_id)
keys = ScopeIds(None, block_type, def_id, usage_id)
# parse_xml is a really messy API. We pass both 'keys' and 'id_generator' and, depending on the XBlock, either
# one may be used to determine the new XBlock's usage key, and the other will be ignored. e.g. video ignores
Expand All @@ -348,7 +353,7 @@ def _import_xml_node_to_parent(

if not xblock_class.has_children:
# No children to worry about. The XML may contain child nodes, but they're not XBlocks.
temp_xblock = xblock_class.parse_xml(node, runtime, keys, id_generator)
temp_xblock = xblock_class.parse_xml(node, runtime, keys)
else:
# We have to handle the children ourselves, because there are lots of complex interactions between
# * the vanilla XBlock parse_xml() method, and its lack of API for "create and save a new XBlock"
Expand All @@ -358,8 +363,12 @@ def _import_xml_node_to_parent(
# serialization of a child block, in order. For blocks that don't support children, their XML content/nodes
# could be anything (e.g. HTML, capa)
node_without_children = etree.Element(node.tag, **node.attrib)
temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys, id_generator)
temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys)
child_nodes = list(node)

# Restore the original id_generator
runtime.id_generator = original_id_generator

if xblock_class.has_children and temp_xblock.children:
raise NotImplementedError("We don't yet support pasting XBlocks with children")
temp_xblock.parent = parent_key
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory

from openedx.core.djangoapps.content_tagging.tests.test_objecttag_export_helpers import TaggedCourseMixin


class TestArgParsingCourseExportOlx(unittest.TestCase):
"""
Expand All @@ -31,7 +33,7 @@ def test_no_args(self):
call_command('export_olx')


class TestCourseExportOlx(ModuleStoreTestCase):
class TestCourseExportOlx(TaggedCourseMixin, ModuleStoreTestCase):
"""
Test exporting OLX content from a course or library.
"""
Expand Down Expand Up @@ -61,7 +63,7 @@ def create_dummy_course(self, store_type):
)
return course.id

def check_export_file(self, tar_file, course_key):
def check_export_file(self, tar_file, course_key, with_tags=False):
"""Check content of export file."""
names = tar_file.getnames()
dirname = "{0.org}-{0.course}-{0.run}".format(course_key)
Expand All @@ -71,6 +73,10 @@ def check_export_file(self, tar_file, course_key):
self.assertIn(f"{dirname}/about/overview.html", names)
self.assertIn(f"{dirname}/assets/assets.xml", names)
self.assertIn(f"{dirname}/policies", names)
if with_tags:
self.assertIn(f"{dirname}/tags.csv", names)
else:
self.assertNotIn(f"{dirname}/tags.csv", names)

def test_export_course(self):
test_course_key = self.create_dummy_course(ModuleStoreEnum.Type.split)
Expand Down Expand Up @@ -98,3 +104,11 @@ def __init__(self, bytes_io):
output = output_wrapper.bytes_io.read()
with tarfile.open(fileobj=BytesIO(output), mode="r:gz") as tar_file:
self.check_export_file(tar_file, test_course_key)

def test_export_course_with_tags(self):
tmp_dir = path(mkdtemp())
self.addCleanup(shutil.rmtree, tmp_dir)
filename = tmp_dir / 'test.tar.gz'
call_command('export_olx', '--output', filename, str(self.course.id))
with tarfile.open(filename) as tar_file:
self.check_export_file(tar_file, self.course.id, with_tags=True)
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,15 @@
)


class MessageValidation(serializers.Serializer):
"""
Serializer for representing XBlock error.
"""

text = serializers.CharField()
type = serializers.CharField()


class ChildAncestorSerializer(serializers.Serializer):
"""
Serializer for representing child blocks in the ancestor XBlock.
Expand Down Expand Up @@ -105,6 +114,8 @@ class ChildVerticalContainerSerializer(serializers.Serializer):
user_partition_info = serializers.DictField()
user_partitions = serializers.ListField()
actions = serializers.SerializerMethodField()
validation_messages = MessageValidation(many=True)
render_error = serializers.CharField()

def get_actions(self, obj): # pylint: disable=unused-argument
"""
Expand Down
Loading

0 comments on commit deb48df

Please sign in to comment.