From 0c637cdca9c7650060c2a25d89f33e94c4561c93 Mon Sep 17 00:00:00 2001 From: Giovanni Di Milia Date: Mon, 11 Jan 2016 20:44:15 -0500 Subject: [PATCH] Added extra field to CCX model for Course Models REST APIs modified --- lms/djangoapps/ccx/api/v0/serializers.py | 9 + lms/djangoapps/ccx/api/v0/tests/test_views.py | 182 ++++++++++++++++-- lms/djangoapps/ccx/api/v0/views.py | 112 ++++++++++- .../0002_customcourseforedx_structure_json.py | 19 ++ lms/djangoapps/ccx/models.py | 15 +- lms/djangoapps/ccx/tests/test_models.py | 36 +++- lms/djangoapps/ccx/tests/test_utils.py | 48 ++++- lms/djangoapps/ccx/utils.py | 27 +++ 8 files changed, 415 insertions(+), 33 deletions(-) create mode 100644 lms/djangoapps/ccx/migrations/0002_customcourseforedx_structure_json.py diff --git a/lms/djangoapps/ccx/api/v0/serializers.py b/lms/djangoapps/ccx/api/v0/serializers.py index 623223da3a..b376ef3fc6 100644 --- a/lms/djangoapps/ccx/api/v0/serializers.py +++ b/lms/djangoapps/ccx/api/v0/serializers.py @@ -17,6 +17,7 @@ class CCXCourseSerializer(serializers.ModelSerializer): start = serializers.CharField(allow_blank=True) due = serializers.CharField(allow_blank=True) max_students_allowed = serializers.IntegerField(source='max_student_enrollments_allowed') + course_modules = serializers.SerializerMethodField() class Meta(object): model = CustomCourseForEdX @@ -28,6 +29,7 @@ class CCXCourseSerializer(serializers.ModelSerializer): "start", "due", "max_students_allowed", + "course_modules", ) read_only_fields = ( "ccx_course_id", @@ -42,3 +44,10 @@ class CCXCourseSerializer(serializers.ModelSerializer): Getter for the CCX Course ID """ return unicode(CCXLocator.from_course_locator(obj.course.id, obj.id)) + + @staticmethod + def get_course_modules(obj): + """ + Getter for the Course Modules. The list is stored in a compressed field. + """ + return obj.structure or [] diff --git a/lms/djangoapps/ccx/api/v0/tests/test_views.py b/lms/djangoapps/ccx/api/v0/tests/test_views.py index a48193ad35..d308de985d 100644 --- a/lms/djangoapps/ccx/api/v0/tests/test_views.py +++ b/lms/djangoapps/ccx/api/v0/tests/test_views.py @@ -38,6 +38,7 @@ from lms.djangoapps.ccx.api.v0 import views from lms.djangoapps.ccx.models import CcxFieldOverride, CustomCourseForEdX from lms.djangoapps.ccx.overrides import override_field_for_ccx from lms.djangoapps.ccx.tests.utils import CcxTestCase +from lms.djangoapps.ccx.utils import get_course_chapters from opaque_keys.edx.keys import CourseKey from student.roles import ( CourseInstructorRole, @@ -85,6 +86,8 @@ class CcxRestApiTest(CcxTestCase, APITestCase): self.course.enable_ccx = True self.mstore.update_item(self.course, self.coach.id) self.auth = self.get_auth_token() + # making the master course chapters easily available + self.master_course_chapters = get_course_chapters(self.master_course_key) def get_auth_token(self): """ @@ -465,11 +468,38 @@ class CcxListTest(CcxRestApiTest): }, {'max_students_allowed': 'invalid_max_students_allowed'} ), + ( + { + 'max_students_allowed': 10, + 'display_name': 'CCX Title', + 'coach_email': 'email@test.com', + 'course_modules': {'foo': 'bar'} + }, + {'course_modules': 'invalid_course_module_list'} + ), + ( + { + 'max_students_allowed': 10, + 'display_name': 'CCX Title', + 'coach_email': 'email@test.com', + 'course_modules': 'block-v1:org.0+course_0+Run_0+type@chapter+block@chapter_1' + }, + {'course_modules': 'invalid_course_module_list'} + ), + ( + { + 'max_students_allowed': 10, + 'display_name': 'CCX Title', + 'coach_email': 'email@test.com', + 'course_modules': ['foo', 'bar'] + }, + {'course_modules': 'invalid_course_module_keys'} + ), ) @ddt.unpack def test_post_list_wrong_input_data(self, data, expected_errors): """ - Test for various post requests with wrong master course string + Test for various post requests with wrong input data """ # add the master_course_key_str to the request data data['master_course_id'] = self.master_course_key_str @@ -489,6 +519,40 @@ class CcxListTest(CcxRestApiTest): resp = self.client.post(self.list_url, data, format='json', HTTP_AUTHORIZATION=self.auth) self.expect_error(status.HTTP_404_NOT_FOUND, 'coach_user_does_not_exist', resp) + def test_post_list_wrong_modules(self): + """ + Specific test for the case when the input data is valid but the + course modules do not belong to the master course + """ + data = { + 'master_course_id': self.master_course_key_str, + 'max_students_allowed': 111, + 'display_name': 'CCX Title', + 'coach_email': self.coach.email, + 'course_modules': [ + 'block-v1:org.0+course_0+Run_0+type@chapter+block@chapter_foo', + 'block-v1:org.0+course_0+Run_0+type@chapter+block@chapter_bar' + ] + } + resp = self.client.post(self.list_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_module_list_not_belonging_to_master_course', resp) + + def test_post_list_mixed_wrong_and_valid_modules(self): + """ + Specific test for the case when the input data is valid but some of + the course modules do not belong to the master course + """ + modules = self.master_course_chapters[0:1] + ['block-v1:org.0+course_0+Run_0+type@chapter+block@chapter_foo'] + data = { + 'master_course_id': self.master_course_key_str, + 'max_students_allowed': 111, + 'display_name': 'CCX Title', + 'coach_email': self.coach.email, + 'course_modules': modules + } + resp = self.client.post(self.list_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_module_list_not_belonging_to_master_course', resp) + def test_post_list(self): """ Test the creation of a CCX @@ -498,7 +562,8 @@ class CcxListTest(CcxRestApiTest): 'master_course_id': self.master_course_key_str, 'max_students_allowed': 111, 'display_name': 'CCX Test Title', - 'coach_email': self.coach.email + 'coach_email': self.coach.email, + 'course_modules': self.master_course_chapters[0:1] } resp = self.client.post(self.list_url, data, format='json', HTTP_AUTHORIZATION=self.auth) self.assertEqual(resp.status_code, status.HTTP_201_CREATED) @@ -525,6 +590,23 @@ class CcxListTest(CcxRestApiTest): self.assertEqual(len(outbox), 1) self.assertIn(self.coach.email, outbox[0].recipients()) # pylint: disable=no-member + def test_post_list_duplicated_modules(self): + """ + Test the creation of a CCX, but with duplicated modules + """ + chapters = self.master_course_chapters[0:1] + duplicated_chapters = chapters * 3 + data = { + 'master_course_id': self.master_course_key_str, + 'max_students_allowed': 111, + 'display_name': 'CCX Test Title', + 'coach_email': self.coach.email, + 'course_modules': duplicated_chapters + } + resp = self.client.post(self.list_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.assertEqual(resp.status_code, status.HTTP_201_CREATED) + self.assertEqual(resp.data.get('course_modules'), chapters) # pylint: disable=no-member + @attr('shard_1') @ddt.ddt @@ -554,6 +636,8 @@ class CcxDetailTest(CcxRestApiTest): creation of ccx courses """ ccx = super(CcxDetailTest, self).make_ccx(max_students_allowed=max_students_allowed) + ccx.structure_json = json.dumps(self.master_course_chapters) + ccx.save() today = datetime.datetime.today() start = today.replace(tzinfo=pytz.UTC) @@ -745,6 +829,7 @@ class CcxDetailTest(CcxRestApiTest): ) self.assertEqual(resp.data.get('coach_email'), self.ccx.coach.email) # pylint: disable=no-member self.assertEqual(resp.data.get('master_course_id'), unicode(self.ccx.course_id)) # pylint: disable=no-member + self.assertEqual(resp.data.get('course_modules'), self.master_course_chapters) # pylint: disable=no-member def test_delete_detail(self): """ @@ -787,29 +872,29 @@ class CcxDetailTest(CcxRestApiTest): } ), ( - { - 'max_students_allowed': 10, - 'display_name': 'CCX Title', - 'coach_email': 'this is not an email@test.com' - }, + {'coach_email': 'this is not an email@test.com'}, {'coach_email': 'invalid_coach_email'} ), ( - { - 'max_students_allowed': 10, - 'display_name': '', - 'coach_email': 'email@test.com' - }, + {'display_name': ''}, {'display_name': 'invalid_display_name'} ), ( - { - 'max_students_allowed': 'a', - 'display_name': 'CCX Title', - 'coach_email': 'email@test.com' - }, + {'max_students_allowed': 'a'}, {'max_students_allowed': 'invalid_max_students_allowed'} ), + ( + {'course_modules': {'foo': 'bar'}}, + {'course_modules': 'invalid_course_module_list'} + ), + ( + {'course_modules': 'block-v1:org.0+course_0+Run_0+type@chapter+block@chapter_1'}, + {'course_modules': 'invalid_course_module_list'} + ), + ( + {'course_modules': ['foo', 'bar']}, + {'course_modules': 'invalid_course_module_keys'} + ), ) @ddt.unpack def test_patch_detail_wrong_input_data(self, data, expected_errors): @@ -826,12 +911,14 @@ class CcxDetailTest(CcxRestApiTest): display_name = self.ccx.display_name max_students_allowed = self.ccx.max_student_enrollments_allowed # pylint: disable=no-member coach_email = self.ccx.coach.email # pylint: disable=no-member + ccx_structure = self.ccx.structure # pylint: disable=no-member resp = self.client.patch(self.detail_url, {}, format='json', HTTP_AUTHORIZATION=self.auth) self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) ccx = CustomCourseForEdX.objects.get(id=self.ccx.id) self.assertEqual(display_name, ccx.display_name) self.assertEqual(max_students_allowed, ccx.max_student_enrollments_allowed) self.assertEqual(coach_email, ccx.coach.email) + self.assertEqual(ccx_structure, ccx.structure) def test_patch_detail_coach_does_not_exist(self): """ @@ -845,6 +932,32 @@ class CcxDetailTest(CcxRestApiTest): resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) self.expect_error(status.HTTP_404_NOT_FOUND, 'coach_user_does_not_exist', resp) + def test_patch_detail_wrong_modules(self): + """ + Specific test for the case when the input data is valid but the + course modules do not belong to the master course + """ + data = { + 'course_modules': [ + 'block-v1:org.0+course_0+Run_0+type@chapter+block@chapter_foo', + 'block-v1:org.0+course_0+Run_0+type@chapter+block@chapter_bar' + ] + } + resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_module_list_not_belonging_to_master_course', resp) + + def test_patch_detail_mixed_wrong_and_valid_modules(self): + """ + Specific test for the case when the input data is valid but some of + the course modules do not belong to the master course + """ + modules = self.master_course_chapters[0:1] + ['block-v1:org.0+course_0+Run_0+type@chapter+block@chapter_foo'] + data = { + 'course_modules': modules + } + resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_module_list_not_belonging_to_master_course', resp) + def test_patch_detail(self): """ Test for successful patch @@ -874,3 +987,38 @@ class CcxDetailTest(CcxRestApiTest): # check that an email has been sent to the coach self.assertEqual(len(outbox), 1) self.assertIn(new_coach.email, outbox[0].recipients()) # pylint: disable=no-member + + def test_patch_detail_modules(self): + """ + Specific test for successful patch of the course modules + """ + data = {'course_modules': self.master_course_chapters[0:1]} + resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) + ccx_from_db = CustomCourseForEdX.objects.get(id=self.ccx.id) + self.assertEqual(ccx_from_db.structure, data['course_modules']) + + data = {'course_modules': []} + resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) + ccx_from_db = CustomCourseForEdX.objects.get(id=self.ccx.id) + self.assertEqual(ccx_from_db.structure, []) + + data = {'course_modules': self.master_course_chapters} + resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) + ccx_from_db = CustomCourseForEdX.objects.get(id=self.ccx.id) + self.assertEqual(ccx_from_db.structure, self.master_course_chapters) + + data = {'course_modules': None} + resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) + ccx_from_db = CustomCourseForEdX.objects.get(id=self.ccx.id) + self.assertEqual(ccx_from_db.structure, None) + + chapters = self.master_course_chapters[0:1] + data = {'course_modules': chapters * 3} + resp = self.client.patch(self.detail_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) + ccx_from_db = CustomCourseForEdX.objects.get(id=self.ccx.id) + self.assertEqual(ccx_from_db.structure, chapters) diff --git a/lms/djangoapps/ccx/api/v0/views.py b/lms/djangoapps/ccx/api/v0/views.py index 9a4207fbfb..32eebc51fd 100644 --- a/lms/djangoapps/ccx/api/v0/views.py +++ b/lms/djangoapps/ccx/api/v0/views.py @@ -1,6 +1,7 @@ """ API v0 views. """ import datetime +import json import logging import pytz @@ -21,7 +22,7 @@ from instructor.enrollment import ( get_email_params, ) from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.keys import CourseKey, UsageKey from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api import permissions from student.models import CourseEnrollment @@ -35,6 +36,7 @@ from lms.djangoapps.ccx.overrides import ( from lms.djangoapps.ccx.utils import ( assign_coach_role_to_ccx, is_email, + get_course_chapters, ) from .paginators import CCXAPIPagination from .serializers import CCXCourseSerializer @@ -156,9 +158,46 @@ def get_valid_input(request_data, ignore_missing=False): field_errors['max_students_allowed'] = {'error_code': 'invalid_max_students_allowed'} elif 'max_students_allowed' in request_data: field_errors['max_students_allowed'] = {'error_code': 'null_field_max_students_allowed'} + + course_modules = request_data.get('course_modules') + if course_modules is not None: + if isinstance(course_modules, list): + # de-duplicate list of modules + course_modules = list(set(course_modules)) + for course_module_id in course_modules: + try: + UsageKey.from_string(course_module_id) + except InvalidKeyError: + field_errors['course_modules'] = {'error_code': 'invalid_course_module_keys'} + break + else: + valid_input['course_modules'] = course_modules + else: + field_errors['course_modules'] = {'error_code': 'invalid_course_module_list'} + elif 'course_modules' in request_data: + # case if the user actually passed null as input + valid_input['course_modules'] = None + return valid_input, field_errors +def valid_course_modules(course_module_list, master_course_key): + """ + Function to validate that each element in the course_module_list belongs + to the master course structure. + Args: + course_module_list (list): A list of strings representing Block Usage Keys + master_course_key (CourseKey): An object representing the master course key id + + Returns: + bool: whether or not all the course module strings belong to the master course + """ + course_chapters = get_course_chapters(master_course_key) + if course_chapters is None: + return False + return set(course_module_list).intersection(set(course_chapters)) == set(course_module_list) + + def make_user_coach(user, master_course_key): """ Makes an user coach on the master course. @@ -190,7 +229,12 @@ class CCXListView(GenericAPIView): "master_course_id": "course-v1:Organization+EX101+RUN-FALL2099", "display_name": "CCX example title", "coach_email": "john@example.com", - "max_students_allowed": 123 + "max_students_allowed": 123, + "course_modules" : [ + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week1", + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week4", + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week5" + ] } @@ -220,6 +264,8 @@ class CCXListView(GenericAPIView): * max_students_allowed: An integer representing he maximum number of students that can be enrolled in the CCX Course. + * course_modules: Optional. A list of course modules id keys. + **GET Response Values** If the request for information about the course is successful, an HTTP 200 "OK" response @@ -242,6 +288,8 @@ class CCXListView(GenericAPIView): * max_students_allowed: An integer representing he maximum number of students that can be enrolled in the CCX Course. + * course_modules: A list of course modules id keys. + * count: An integer representing the total number of records that matched the request parameters. * next: A string representing the URL where to retrieve the next page of results. This can be `null` @@ -263,7 +311,12 @@ class CCXListView(GenericAPIView): "coach_email": "john@example.com", "start": "2019-01-01", "due": "2019-06-01", - "max_students_allowed": 123 + "max_students_allowed": 123, + "course_modules" : [ + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week1", + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week4", + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week5" + ] }, { ... } } @@ -289,6 +342,8 @@ class CCXListView(GenericAPIView): * max_students_allowed: An integer representing he maximum number of students that can be enrolled in the CCX Course. + * course_modules: A list of course modules id keys. + **Example POST Response** { @@ -297,8 +352,13 @@ class CCXListView(GenericAPIView): "coach_email": "john@example.com", "start": "2019-01-01", "due": "2019-06-01", - "max_students_allowed": 123 - } + "max_students_allowed": 123, + "course_modules" : [ + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week1", + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week4", + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week5" + ] + } """ authentication_classes = (OAuth2Authentication, SessionAuthentication,) permission_classes = (IsAuthenticated, permissions.IsMasterCourseStaffInstructor) @@ -383,11 +443,24 @@ class CCXListView(GenericAPIView): } ) + if valid_input.get('course_modules'): + if not valid_course_modules(valid_input['course_modules'], master_course_key): + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + 'error_code': 'course_module_list_not_belonging_to_master_course' + } + ) + # prepare the course_modules to be stored in a json stringified field + course_modules_json = json.dumps(valid_input.get('course_modules')) + with transaction.atomic(): ccx_course_object = CustomCourseForEdX( course_id=master_course_object.id, coach=coach, - display_name=valid_input['display_name']) + display_name=valid_input['display_name'], + structure_json=course_modules_json + ) ccx_course_object.save() # Make sure start/due are overridden for entire course @@ -459,7 +532,12 @@ class CCXDetailView(GenericAPIView): "display_name": "CCX example title modified", "coach_email": "joe@example.com", - "max_students_allowed": 111 + "max_students_allowed": 111, + "course_modules" : [ + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week1", + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week4", + "block-v1:Organization+EX101+RUN-FALL2099+type@chapter+block@week5" + ] } DELETE /api/ccx/v0/ccx/{ccx_course_id} @@ -483,6 +561,8 @@ class CCXDetailView(GenericAPIView): * max_students_allowed: Optional. An integer representing he maximum number of students that can be enrolled in the CCX Course. + * course_modules: Optional. A list of course modules id keys. + **GET Response Values** If the request for information about the CCX course is successful, an HTTP 200 "OK" response @@ -503,6 +583,8 @@ class CCXDetailView(GenericAPIView): * max_students_allowed: An integer representing he maximum number of students that can be enrolled in the CCX Course. + * course_modules: A list of course modules id keys. + **PATCH and DELETE Response Values** If the request for modification or deletion of a CCX course is successful, an HTTP 204 "No Content" @@ -606,6 +688,9 @@ class CCXDetailView(GenericAPIView): } ) + # get the master course key and master course object + master_course_object, master_course_key, _, _ = get_valid_course(unicode(ccx_course_object.course_id)) + with transaction.atomic(): # update the display name if 'display_name' in valid_input: @@ -625,6 +710,17 @@ class CCXDetailView(GenericAPIView): if ccx_course_object.coach.id != coach.id: old_coach = ccx_course_object.coach ccx_course_object.coach = coach + if 'course_modules' in valid_input: + if valid_input.get('course_modules'): + if not valid_course_modules(valid_input['course_modules'], master_course_key): + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + 'error_code': 'course_module_list_not_belonging_to_master_course' + } + ) + # course_modules to be stored in a json stringified field + ccx_course_object.structure_json = json.dumps(valid_input.get('course_modules')) ccx_course_object.save() # update the overridden field for the maximum amount of students if 'max_students_allowed' in valid_input: @@ -636,8 +732,6 @@ class CCXDetailView(GenericAPIView): ) # if the coach has changed, update the permissions if old_coach is not None: - # get the master course key and master course object - master_course_object, master_course_key, _, _ = get_valid_course(unicode(ccx_course_object.course_id)) # make the new ccx coach a coach on the master course make_user_coach(coach, master_course_key) # enroll the coach in the ccx diff --git a/lms/djangoapps/ccx/migrations/0002_customcourseforedx_structure_json.py b/lms/djangoapps/ccx/migrations/0002_customcourseforedx_structure_json.py new file mode 100644 index 0000000000..0ca713e4a5 --- /dev/null +++ b/lms/djangoapps/ccx/migrations/0002_customcourseforedx_structure_json.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('ccx', '0001_initial'), + ] + + operations = [ + migrations.AddField( + model_name='customcourseforedx', + name='structure_json', + field=models.TextField(null=True, verbose_name=b'Structure JSON', blank=True), + ), + ] diff --git a/lms/djangoapps/ccx/models.py b/lms/djangoapps/ccx/models.py index 2baf72ee88..1daec6e9dd 100644 --- a/lms/djangoapps/ccx/models.py +++ b/lms/djangoapps/ccx/models.py @@ -1,8 +1,9 @@ """ Models for the custom course feature """ -from datetime import datetime +import json import logging +from datetime import datetime from django.contrib.auth.models import User from django.db import models @@ -24,6 +25,9 @@ class CustomCourseForEdX(models.Model): course_id = CourseKeyField(max_length=255, db_index=True) display_name = models.CharField(max_length=255) coach = models.ForeignKey(User, db_index=True) + # if not empty, this field contains a json serialized list of + # the master course modules + structure_json = models.TextField(verbose_name='Structure JSON', blank=True, null=True) class Meta(object): app_label = 'ccx' @@ -107,6 +111,15 @@ class CustomCourseForEdX(models.Model): value += u' UTC' return value + @property + def structure(self): + """ + Deserializes a course structure JSON object + """ + if self.structure_json: + return json.loads(self.structure_json) + return None + class CcxFieldOverride(models.Model): """ diff --git a/lms/djangoapps/ccx/tests/test_models.py b/lms/djangoapps/ccx/tests/test_models.py index 2a1c975092..ccc5ede839 100644 --- a/lms/djangoapps/ccx/tests/test_models.py +++ b/lms/djangoapps/ccx/tests/test_models.py @@ -1,6 +1,7 @@ """ tests for the models """ +import json from datetime import datetime, timedelta from django.utils.timezone import UTC from mock import patch @@ -30,11 +31,11 @@ class TestCCX(ModuleStoreTestCase): def setUp(self): """common setup for all tests""" super(TestCCX, self).setUp() - self.course = course = CourseFactory.create() - coach = AdminFactory.create() - role = CourseCcxCoachRole(course.id) - role.add_users(coach) - self.ccx = CcxFactory(course_id=course.id, coach=coach) + self.course = CourseFactory.create() + self.coach = AdminFactory.create() + role = CourseCcxCoachRole(self.course.id) + role.add_users(self.coach) + self.ccx = CcxFactory(course_id=self.course.id, coach=self.coach) def set_ccx_override(self, field, value): """Create a field override for the test CCX on with """ @@ -209,3 +210,28 @@ class TestCCX(ModuleStoreTestCase): self.set_ccx_override('max_student_enrollments_allowed', expected) actual = self.ccx.max_student_enrollments_allowed # pylint: disable=no-member self.assertEqual(expected, actual) + + def test_structure_json_default_empty(self): + """ + By default structure_json does not contain anything + """ + self.assertEqual(self.ccx.structure_json, None) # pylint: disable=no-member + self.assertEqual(self.ccx.structure, None) # pylint: disable=no-member + + def test_structure_json(self): + """ + Test a json stored in the structure_json + """ + dummy_struct = [ + "block-v1:Organization+CN101+CR-FALL15+type@chapter+block@Unit_4", + "block-v1:Organization+CN101+CR-FALL15+type@chapter+block@Unit_5", + "block-v1:Organization+CN101+CR-FALL15+type@chapter+block@Unit_11" + ] + json_struct = json.dumps(dummy_struct) + ccx = CcxFactory( + course_id=self.course.id, + coach=self.coach, + structure_json=json_struct + ) + self.assertEqual(ccx.structure_json, json_struct) # pylint: disable=no-member + self.assertEqual(ccx.structure, dummy_struct) # pylint: disable=no-member diff --git a/lms/djangoapps/ccx/tests/test_utils.py b/lms/djangoapps/ccx/tests/test_utils.py index 837913ab6e..503e39739d 100644 --- a/lms/djangoapps/ccx/tests/test_utils.py +++ b/lms/djangoapps/ccx/tests/test_utils.py @@ -1,9 +1,9 @@ """ test utils """ +import mock from nose.plugins.attrib import attr -from lms.djangoapps.ccx.tests.factories import CcxFactory from student.roles import CourseCcxCoachRole from student.tests.factories import ( AdminFactory, @@ -12,7 +12,11 @@ from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE) from xmodule.modulestore.tests.factories import CourseFactory +from opaque_keys.edx.keys import CourseKey +from lms.djangoapps.ccx import utils +from lms.djangoapps.ccx.tests.factories import CcxFactory +from lms.djangoapps.ccx.tests.utils import CcxTestCase from ccx_keys.locator import CCXLocator @@ -47,3 +51,45 @@ class TestGetCCXFromCCXLocator(ModuleStoreTestCase): course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) result = self.call_fut(course_key) self.assertEqual(result, ccx) + + +@attr('shard_1') +class TestGetCourseChapters(CcxTestCase): + """ + Tests for the `get_course_chapters` util function + """ + def setUp(self): + """ + Set up tests + """ + super(TestGetCourseChapters, self).setUp() + self.course_key = self.course.location.course_key + + def test_get_structure_non_existing_key(self): + """ + Test to get the course structure + """ + self.assertEqual(utils.get_course_chapters(None), None) + # build a fake key + fake_course_key = CourseKey.from_string('course-v1:FakeOrg+CN1+CR-FALLNEVER1') + self.assertEqual(utils.get_course_chapters(fake_course_key), None) + + @mock.patch('openedx.core.djangoapps.content.course_structures.models.CourseStructure.structure', + new_callable=mock.PropertyMock) + def test_wrong_course_structure(self, mocked_attr): + """ + Test the case where the course has an unexpected structure. + """ + mocked_attr.return_value = {'foo': 'bar'} + self.assertEqual(utils.get_course_chapters(self.course_key), []) + + def test_get_chapters(self): + """ + Happy path + """ + course_chapters = utils.get_course_chapters(self.course_key) + self.assertEqual(len(course_chapters), 2) + self.assertEqual( + sorted(course_chapters), + sorted([unicode(child) for child in self.course.children]) + ) diff --git a/lms/djangoapps/ccx/utils.py b/lms/djangoapps/ccx/utils.py index 4508828df1..93e394674d 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -23,6 +23,7 @@ from instructor.enrollment import ( from instructor.access import allow_access from instructor.views.tools import get_student_from_identifier from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.content.course_structures.models import CourseStructure from student.models import CourseEnrollment from student.roles import CourseCcxCoachRole @@ -284,3 +285,29 @@ def is_email(identifier): except ValidationError: return False return True + + +def get_course_chapters(course_key): + """ + Extracts the chapters from a course structure. + If the course does not exist returns None. + If the structure does not contain 1st level children, + it returns an empty list. + + Args: + course_key (CourseLocator): the course key + Returns: + list (string): a list of string representing the chapters modules + of the course + """ + if course_key is None: + return + try: + course_obj = CourseStructure.objects.get(course_id=course_key) + except CourseStructure.DoesNotExist: + return + course_struct = course_obj.structure + try: + return course_struct['blocks'][course_struct['root']].get('children', []) + except KeyError: + return []