diff --git a/analytics_dashboard/courses/presenters/__init__.py b/analytics_dashboard/courses/presenters/__init__.py index 64bb09696..8da6f3332 100644 --- a/analytics_dashboard/courses/presenters/__init__.py +++ b/analytics_dashboard/courses/presenters/__init__.py @@ -90,6 +90,14 @@ def module_type(self): """ Module type to retrieve structure for. E.g. video, problem. """ pass + @property + def module_graded_type(self): + """ + Property used to filter modules by. True/False will include only modules with + that grade field. Set to None if not filtering by the graded value. + """ + return None + def get_cache_key(self, name): """ Returns sanitized key for caching. """ return sanitize_cache_key(u'{}_{}'.format(self.course_id, name)) @@ -114,7 +122,7 @@ def course_structure(self, section_id=None, subsection_id=None): if not found_structure: structure = self._get_structure() found_structure = CourseStructure.course_structure_to_sections(structure, self.module_type, - graded=False) + graded=self.module_graded_type) cache.set(all_sections_key, found_structure) for section in found_structure: diff --git a/analytics_dashboard/courses/presenters/performance.py b/analytics_dashboard/courses/presenters/performance.py index df38fc325..ef969ac7c 100644 --- a/analytics_dashboard/courses/presenters/performance.py +++ b/analytics_dashboard/courses/presenters/performance.py @@ -368,3 +368,14 @@ def all_sections_key(self): @property def module_type(self): return 'problem' + + @property + def module_graded_type(self): + """ + Get ungraded blocks. + + This is a bit confusing as this presenter is used to show both graded and + ungraded content. The ungraded content uses CourseAPIPresenterMixin::course_structure + which then gets the module grade type for filtering. + """ + return False diff --git a/analytics_dashboard/courses/tests/factories.py b/analytics_dashboard/courses/tests/factories.py index fba89a392..c95733346 100644 --- a/analytics_dashboard/courses/tests/factories.py +++ b/analytics_dashboard/courses/tests/factories.py @@ -309,3 +309,29 @@ def _get_video_segments(self, segment_min, segment_max): 'created': CREATED_DATETIME_STRING }) return segments + + def videos(self): + """ + Mock video data. + """ + videos = [ + { + "pipeline_video_id": "edX/DemoX/Demo_Course|i4x-edX-DemoX-video-7e9b434e6de3435ab99bd3fb25bde807", + "encoded_module_id": "i4x-edX-DemoX-video-7e9b434e6de3435ab99bd3fb25bde807", + "duration": 257, + "segment_length": 5, + "users_at_start": 10, + "users_at_end": 0, + "created": "2015-04-15T214158" + }, + { + "pipeline_video_id": "edX/DemoX/Demo_Course|i4x-edX-DemoX-videoalpha-0b9e39477cf34507a7a48f74be381fdd", + "encoded_module_id": "i4x-edX-DemoX-videoalpha-0b9e39477cf34507a7a48f74be381fdd", + "duration": 195, + "segment_length": 5, + "users_at_start": 55, + "users_at_end": 0, + "created": "2015-04-15T214158" + } + ] + return videos diff --git a/analytics_dashboard/courses/tests/test_presenters.py b/analytics_dashboard/courses/tests/test_presenters.py index b376ef3d9..454996c9c 100644 --- a/analytics_dashboard/courses/tests/test_presenters.py +++ b/analytics_dashboard/courses/tests/test_presenters.py @@ -5,17 +5,19 @@ import analyticsclient.constants.activity_type as AT from django.core.cache import cache from django.core.urlresolvers import reverse -from django.test import TestCase +from django.test import (override_settings, TestCase) import mock from ddt import ddt, data +from common.tests import course_fixtures + from courses.exceptions import NoVideosError from courses.presenters import BasePresenter from courses.presenters.engagement import (CourseEngagementActivityPresenter, CourseEngagementVideoPresenter) from courses.presenters.enrollment import (CourseEnrollmentPresenter, CourseEnrollmentDemographicsPresenter) from courses.presenters.performance import CoursePerformancePresenter -from courses.tests import utils, SwitchMixin -from courses.tests.factories import CourseEngagementDataFactory, CoursePerformanceDataFactory +from courses.tests import (SwitchMixin, utils) +from courses.tests.factories import (CourseEngagementDataFactory, CoursePerformanceDataFactory) class BasePresenterTests(TestCase): @@ -123,6 +125,7 @@ def test_get_summary_and_trend_data_small(self, mock_activity): self.assertSummaryAndTrendsValid(True, self.get_expected_trends_small(True)) +@ddt class CourseEngagementVideoPresenterTests(SwitchMixin, TestCase): SECTION_ID = 'i4x://edX/DemoX/chapter/9fca584977d04885bc911ea76a9ef29e' SUBSECTION_ID = 'i4x://edX/DemoX/sequential/07bc32474380492cb34f76e5f9d9a135' @@ -142,6 +145,65 @@ def test_default_block_data(self): 'start_only_percent': 0, }) + def _create_graded_and_ungraded_course_structure_fixtures(self): + """ + Create graded and ungraded video sections. + """ + chapter_fixture = course_fixtures.ChapterFixture() + # a dictionary to access the fixtures easily + course_structure_fixtures = { + 'chapter': chapter_fixture, + 'course': course_fixtures.CourseFixture(org='this', course='course', run='id') + } + + for grade_status in ['graded', 'ungraded']: + sequential_fixture = course_fixtures.SequentialFixture(graded=grade_status is 'graded').add_children( + course_fixtures.VerticalFixture().add_children( + course_fixtures.VideoFixture() + ) + ) + course_structure_fixtures[grade_status] = { + 'sequential': sequential_fixture, + } + chapter_fixture.add_children(sequential_fixture) + + course_structure_fixtures['course'].add_children(chapter_fixture) + return course_structure_fixtures + + @data('graded', 'ungraded') + @override_settings(CACHES={ + 'default': { + 'BACKEND': 'django.core.cache.backends.dummy.DummyCache', + } + }) + def test_graded_modes(self, grade_status): + """ + Ensure that video structure will be retrieved for both graded and ungraded. + """ + factory = CourseEngagementDataFactory() + course_structure_fixtures = self._create_graded_and_ungraded_course_structure_fixtures() + course_fixture = course_structure_fixtures['course'] + chapter_fixture = course_structure_fixtures['chapter'] + + with mock.patch('slumber.Resource.get', mock.Mock(return_value=course_fixture.course_structure())): + with mock.patch('analyticsclient.course.Course.videos', mock.Mock(return_value=factory.videos())): + # check that we get results for both graded and ungraded + sequential_fixture = course_structure_fixtures[grade_status]['sequential'] + video_id = sequential_fixture.children[0].children[0].id + + actual_videos = self.presenter.subsection_children(chapter_fixture.id, sequential_fixture.id) + expected_url = reverse('courses:engagement:video_timeline', + kwargs={ + 'course_id': self.course_id, + 'section_id': chapter_fixture.id, + 'subsection_id': sequential_fixture.id, + 'video_id': video_id + }) + expected = [{'index': 1, 'name': None, 'end_percent': 0, 'url': expected_url, 'start_only_percent': 0, + 'id': video_id, 'users_at_start': 0, 'start_only_users': 0, 'users_at_end': 0, + 'children': []}] + self.assertListEqual(actual_videos, expected) + def test_module_id_to_data_id(self): opaque_key_id = 'i4x-edX-DemoX-video-0b9e39477cf34507a7a48f74be381fdd' module_id = 'i4x://edX/DemoX/video/0b9e39477cf34507a7a48f74be381fdd' @@ -245,26 +307,8 @@ def test_attach_aggregated_data_to_parent(self): @mock.patch('analyticsclient.course.Course.videos') def test_fetch_course_module_data(self, mock_videos): - videos = [ - { - "pipeline_video_id": "edX/DemoX/Demo_Course|i4x-edX-DemoX-video-7e9b434e6de3435ab99bd3fb25bde807", - "encoded_module_id": "i4x-edX-DemoX-video-7e9b434e6de3435ab99bd3fb25bde807", - "duration": 257, - "segment_length": 5, - "users_at_start": 10, - "users_at_end": 0, - "created": "2015-04-15T214158" - }, - { - "pipeline_video_id": "edX/DemoX/Demo_Course|i4x-edX-DemoX-videoalpha-0b9e39477cf34507a7a48f74be381fdd", - "encoded_module_id": "i4x-edX-DemoX-videoalpha-0b9e39477cf34507a7a48f74be381fdd", - "duration": 195, - "segment_length": 5, - "users_at_start": 55, - "users_at_end": 0, - "created": "2015-04-15T214158" - } - ] + factory = CourseEngagementDataFactory() + videos = factory.videos() mock_videos.return_value = videos self.assertListEqual(self.presenter.fetch_course_module_data(), videos) diff --git a/common/course_structure.py b/common/course_structure.py index 951c09aba..9858d8afc 100644 --- a/common/course_structure.py +++ b/common/course_structure.py @@ -17,8 +17,6 @@ def _filter_children(blocks, key, require_format=False, **kwargs): if block_type: kwargs[u'type'] = block_type - kwargs.setdefault(u'graded', False) - matched = True for name, value in kwargs.iteritems(): matched &= (block.get(name, None) == value) @@ -99,10 +97,14 @@ def _build_sections(blocks, section_id, graded, block_types): if len(block_types) > 0: block_types = list(block_types) block_type = block_types.pop(0) + filter_kwargs = { + 'block_type': block_type, + } + if graded is not None: + filter_kwargs['graded'] = graded structure_sections = CourseStructure._filter_children(blocks, section_id, - graded=graded, - block_type=block_type, - require_format=False) + require_format=False, + **filter_kwargs) for section in structure_sections: children = CourseStructure._build_sections(blocks, section[u'id'], graded, block_types) diff --git a/common/tests/course_fixtures.py b/common/tests/course_fixtures.py new file mode 100644 index 000000000..1862d3eac --- /dev/null +++ b/common/tests/course_fixtures.py @@ -0,0 +1,109 @@ +import uuid + + +class CourseStructureAPIFixtureMixin(object): + """Represents a course that can serialize itself in the form generated by the Course Structure API.""" + # pylint: disable=unused-argument + def __init__(self, *args, **kwargs): + self._uuid = uuid.uuid4().hex + self._type = None + self._display_name = kwargs.get('display_name', '') + self._graded = False + self._assignment_type = None + self._children = [] + # Course data + self._org = None + self._course = None + self._run = None + + @property + def children(self): + return self._children + + def to_dict(self): + """Return a dict representation of this block in the form generated by the Course Structure API.""" + return { + 'id': self.id, + 'type': self._type, + 'display_name': self._display_name, + 'graded': self._graded, + 'format': self._assignment_type, + 'children': [child.id for child in self._children] + } + + def add_children(self, *children): + """Add children to this block""" + self._children += children + return self + + def pre_order(self): + """Return the fixture rooted at `self` as a list visited in pre-order.""" + visited = [self] + for child in self._children: + # Children should inherit and cache org, course, and run for ID generation. + # 'graded' is also inherited. + # pylint: disable=protected-access + child._org = self._org + child._course = self._course + child._run = self._run + child._graded = self._graded + visited += child.pre_order() + return visited + + @property + def id(self): + """Uniquely identifies this block in the format used by the Course Structure API.""" + return 'i4x://{org}/{course}/{type}/{_uuid}'.format( + org=self._org, course=self._course, type=self._type, _uuid=self._uuid + ) + + +class CourseFixture(CourseStructureAPIFixtureMixin): + """Represents a course as returned by the Course Structure API.""" + def __init__(self, org=None, course=None, run=None, *args, **kwargs): + super(CourseFixture, self).__init__(*args, **kwargs) + self._type = 'course' + self._org = org + self._course = course + self._run = run + self._uuid = run # The org/course/run triple uniquely identifies the course + + def course_structure(self): + """Return a dict representing this course in the form generated by the Course Structure API.""" + return { + 'root': self.id, + 'blocks': { + child.id: child.to_dict() + for child in self.pre_order() + } + } + + +class ChapterFixture(CourseStructureAPIFixtureMixin): + """Represents a chapter as returned by the Course Structure API.""" + def __init__(self, *args, **kwargs): + super(ChapterFixture, self).__init__(*args, **kwargs) + self._type = 'chapter' + + +class SequentialFixture(CourseStructureAPIFixtureMixin): + """Represents a sequential as returned by the Course Structure API.""" + def __init__(self, graded=False, assignment_type=None, *args, **kwargs): + super(SequentialFixture, self).__init__(*args, **kwargs) + self._graded = graded + self._assignment_type = assignment_type + self._type = 'sequential' + + +class VerticalFixture(CourseStructureAPIFixtureMixin): + """Represents a vertical as returned by the Course Structure API.""" + def __init__(self, *args, **kwargs): + super(VerticalFixture, self).__init__(*args, **kwargs) + self._type = 'vertical' + + +class VideoFixture(CourseStructureAPIFixtureMixin): + """Represents a video as returned by the Course Structure API.""" + def __init__(self, *args, **kwargs): + super(VideoFixture, self).__init__(*args, **kwargs) + self._type = 'video'