diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 6bf764268df3cc5209a2ae4ca7b33be356f57536..a17b864e3b1a9b1952ce0283703bc403f69b538c 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -21,7 +21,6 @@ from functools import total_ordering from importlib import import_module from urllib import urlencode -import analytics from config_models.models import ConfigurationModel from django.apps import apps from django.conf import settings @@ -66,7 +65,7 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.xmodule_django.models import NoneToEmptyManager from openedx.core.djangolib.model_mixins import DeletableByUserValue -from track import contexts +from track import contexts, segment from util.milestones_helpers import is_entrance_exams_enabled from util.model_utils import emit_field_changed_events, get_changed_fields_dict from util.query import use_read_replica_if_available @@ -739,7 +738,7 @@ class Registration(models.Model): has_segment_key = getattr(settings, 'LMS_SEGMENT_KEY', None) has_mailchimp_id = hasattr(settings, 'MAILCHIMP_NEW_USER_LIST_ID') if has_segment_key and has_mailchimp_id: - identity_args = [ + segment.identify( self.user.id, # pylint: disable=no-member { 'email': self.user.email, @@ -751,8 +750,7 @@ class Registration(models.Model): "listId": settings.MAILCHIMP_NEW_USER_LIST_ID } } - ] - analytics.identify(*identity_args) + ) class PendingNameChange(DeletableByUserValue, models.Model): @@ -1429,25 +1427,17 @@ class CourseEnrollment(models.Model): 'course_id': text_type(self.course_id), 'mode': self.mode, } - + segment_properties = { + 'category': 'conversion', + 'label': text_type(self.course_id), + 'org': self.course_id.org, + 'course': self.course_id.course, + 'run': self.course_id.run, + 'mode': self.mode, + } with tracker.get_tracker().context(event_name, context): tracker.emit(event_name, data) - - if hasattr(settings, 'LMS_SEGMENT_KEY') and settings.LMS_SEGMENT_KEY: - tracking_context = tracker.get_tracker().resolve_context() - analytics.track(self.user_id, event_name, { - 'category': 'conversion', - 'label': text_type(self.course_id), - 'org': self.course_id.org, - 'course': self.course_id.course, - 'run': self.course_id.run, - 'mode': self.mode, - }, context={ - 'ip': tracking_context.get('ip'), - 'Google Analytics': { - 'clientId': tracking_context.get('client_id') - } - }) + segment.track(self.user_id, event_name, segment_properties) except: # pylint: disable=bare-except if event_name and self.course_id: diff --git a/common/djangoapps/student/tests/test_activate_account.py b/common/djangoapps/student/tests/test_activate_account.py index aacbbbcf21be9818217ae85990bf157f3eff98ed..8ea34465ea41da7cfe6b55f2a979b34b10de3d30 100644 --- a/common/djangoapps/student/tests/test_activate_account.py +++ b/common/djangoapps/student/tests/test_activate_account.py @@ -72,7 +72,7 @@ class TestActivateAccount(TestCase): LMS_SEGMENT_KEY="testkey", MAILCHIMP_NEW_USER_LIST_ID="listid" ) - @patch('student.models.analytics.identify') + @patch('student.models.segment.identify') def test_activation_with_keys(self, mock_segment_identify): expected_segment_payload = { 'email': self.email, @@ -98,16 +98,16 @@ class TestActivateAccount(TestCase): ) @override_settings(LMS_SEGMENT_KEY="testkey") - @patch('student.models.analytics.identify') + @patch('student.models.segment.identify') def test_activation_without_mailchimp_key(self, mock_segment_identify): self.assert_no_tracking(mock_segment_identify) @override_settings(MAILCHIMP_NEW_USER_LIST_ID="listid") - @patch('student.models.analytics.identify') + @patch('student.models.segment.identify') def test_activation_without_segment_key(self, mock_segment_identify): self.assert_no_tracking(mock_segment_identify) - @patch('student.models.analytics.identify') + @patch('student.models.segment.identify') def test_activation_without_keys(self, mock_segment_identify): self.assert_no_tracking(mock_segment_identify) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 4682cdab725bc11043d6b75b38f6f09f85697b85..e18525e4bac3703de194bdac548f94b4f1b8323d 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -66,7 +66,6 @@ from collections import OrderedDict from logging import getLogger from smtplib import SMTPException -import analytics from django.conf import settings from django.contrib.auth.models import User from django.core.mail.message import EmailMessage @@ -79,12 +78,12 @@ from social_core.pipeline import partial from social_core.pipeline.social_auth import associate_by_email from edxmako.shortcuts import render_to_string -from eventtracking import tracker from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_authn import cookies as user_authn_cookies from lms.djangoapps.verify_student.models import SSOVerification from lms.djangoapps.verify_student.utils import earliest_allowed_verification_date from third_party_auth.utils import user_exists +from track import segment from . import provider @@ -657,23 +656,12 @@ def login_analytics(strategy, auth_entry, current_partial=None, *args, **kwargs) elif auth_entry in [AUTH_ENTRY_ACCOUNT_SETTINGS]: event_name = 'edx.bi.user.account.linked' - if event_name is not None and hasattr(settings, 'LMS_SEGMENT_KEY') and settings.LMS_SEGMENT_KEY: - tracking_context = tracker.get_tracker().resolve_context() - analytics.track( - kwargs['user'].id, - event_name, - { - 'category': "conversion", - 'label': None, - 'provider': kwargs['backend'].name - }, - context={ - 'ip': tracking_context.get('ip'), - 'Google Analytics': { - 'clientId': tracking_context.get('client_id') - } - } - ) + if event_name is not None: + segment.track(kwargs['user'].id, event_name, { + 'category': "conversion", + 'label': None, + 'provider': kwargs['backend'].name + }) @partial.partial diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 1cf8965fbc974fb63bf9f0005418746f7251abdc..0a01fddc174a1750c49d7ef5974c76ca946555e9 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -522,13 +522,13 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): def test_canceling_authentication_redirects_to_root_when_auth_entry_not_set(self): self.assert_exception_redirect_looks_correct('/') - def test_full_pipeline_succeeds_for_linking_account(self): + @mock.patch('third_party_auth.pipeline.segment.track') + def test_full_pipeline_succeeds_for_linking_account(self, _mock_segment_track): # First, create, the request and strategy that store pipeline state, # configure the backend, and mock out wire traffic. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) - pipeline.analytics.track = mock.MagicMock() request.user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username(), skip_social_auth=True) @@ -677,13 +677,13 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): self.assert_account_settings_context_looks_correct( account_settings_context(request), duplicate=True, linked=True) - def test_full_pipeline_succeeds_for_signing_in_to_existing_active_account(self): + @mock.patch('third_party_auth.pipeline.segment.track') + def test_full_pipeline_succeeds_for_signing_in_to_existing_active_account(self, _mock_segment_track): # First, create, the request and strategy that store pipeline state, # configure the backend, and mock out wire traffic. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) - pipeline.analytics.track = mock.MagicMock() user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username()) self.assert_social_auth_exists_for_user(user, strategy) diff --git a/common/djangoapps/track/segment.py b/common/djangoapps/track/segment.py new file mode 100644 index 0000000000000000000000000000000000000000..b2824435e6e62ffad7d4f7abfc81281a1af48ff9 --- /dev/null +++ b/common/djangoapps/track/segment.py @@ -0,0 +1,57 @@ +""" +Wrapper methods for emitting events to Segment directly (rather than through tracking log events). + +These take advantage of properties that are extracted from incoming requests by track middleware, +stored in tracking context objects, and extracted here to be passed to Segment as part of context +required by server-side events. + +To use, call "from track import segment", then call segment.track() or segment.identify(). + +""" + +import analytics + +from django.conf import settings +from eventtracking import tracker + + +def track(user_id, event_name, properties=None, context=None): + """Wrapper for emitting Segment track event, including augmenting context information from middleware.""" + + if event_name is not None and hasattr(settings, 'LMS_SEGMENT_KEY') and settings.LMS_SEGMENT_KEY: + properties = properties or {} + segment_context = dict(context) if context else {} + tracking_context = tracker.get_tracker().resolve_context() + + if 'ip' not in segment_context and 'ip' in tracking_context: + segment_context['ip'] = tracking_context.get('ip') + + if ('Google Analytics' not in segment_context or 'clientId' not in segment_context['Google Analytics']) and 'client_id' in tracking_context: + segment_context['Google Analytics'] = { + 'clientId': tracking_context.get('client_id') + } + + if 'userAgent' not in segment_context and 'agent' in tracking_context: + segment_context['userAgent'] = tracking_context.get('agent') + + path = tracking_context.get('path') + referer = tracking_context.get('referer') + page = tracking_context.get('page') + if path is not None or referer is not None or page is not None: + if 'page' not in segment_context: + segment_context['page'] = {} + if path is not None and 'path' not in segment_context['page']: + segment_context['page']['path'] = path + if referer is not None and 'referrer' not in segment_context['page']: + segment_context['page']['referrer'] = referer + if page is not None and 'url' not in segment_context['page']: + segment_context['page']['url'] = page + + analytics.track(user_id, event_name, properties, segment_context) + + +def identify(user_id, properties, context=None): + """Wrapper for emitting Segment identify event.""" + if hasattr(settings, 'LMS_SEGMENT_KEY') and settings.LMS_SEGMENT_KEY: + segment_context = dict(context) if context else {} + analytics.identify(user_id, properties, segment_context) diff --git a/common/djangoapps/track/tests/test_segment.py b/common/djangoapps/track/tests/test_segment.py new file mode 100644 index 0000000000000000000000000000000000000000..8af0a9be08f98402efa4679ab06d79ff66599caa --- /dev/null +++ b/common/djangoapps/track/tests/test_segment.py @@ -0,0 +1,149 @@ +"""Ensure emitted events contain the fields legacy processors expect to find.""" + +import ddt + +from django.test import TestCase +from django.test.utils import override_settings +from eventtracking import tracker +from eventtracking.django import DjangoTracker +from mock import sentinel, patch + +from track import segment + + +@ddt.ddt +class SegmentTrackTestCase(TestCase): + """Ensure emitted events contain the expected context values.""" + + def setUp(self): + super(SegmentTrackTestCase, self).setUp() + self.tracker = DjangoTracker() + tracker.register_tracker(self.tracker) + self.properties = {sentinel.key: sentinel.value} + + patcher = patch('track.segment.analytics.track') + self.mock_segment_track = patcher.start() + self.addCleanup(patcher.stop) + + def test_missing_key(self): + segment.track(sentinel.user_id, sentinel.name, self.properties) + self.assertFalse(self.mock_segment_track.called) + + @override_settings(LMS_SEGMENT_KEY=None) + def test_null_key(self): + segment.track(sentinel.user_id, sentinel.name, self.properties) + self.assertFalse(self.mock_segment_track.called) + + @override_settings(LMS_SEGMENT_KEY="testkey") + def test_missing_name(self): + segment.track(sentinel.user_id, None, self.properties) + self.assertFalse(self.mock_segment_track.called) + + @override_settings(LMS_SEGMENT_KEY="testkey") + def test_track_without_tracking_context(self): + segment.track(sentinel.user_id, sentinel.name, self.properties) + self.assertTrue(self.mock_segment_track.called) + args, kwargs = self.mock_segment_track.call_args + expected_segment_context = {} + self.assertEqual((sentinel.user_id, sentinel.name, self.properties, expected_segment_context), args) + + @ddt.unpack + @ddt.data( + ({'ip': sentinel.ip}, {'ip': sentinel.provided_ip}, {'ip': sentinel.ip}), + ({'agent': sentinel.agent}, {'userAgent': sentinel.provided_agent}, {'userAgent': sentinel.agent}), + ({'path': sentinel.path}, {'page': {'path': sentinel.provided_path}}, {'page': {'path': sentinel.path}}), + ({'referer': sentinel.referer}, {'page': {'referrer': sentinel.provided_referer}}, {'page': {'referrer': sentinel.referer}}), + ({'page': sentinel.page}, {'page': {'url': sentinel.provided_page}}, {'page': {'url': sentinel.page}}), + ({'client_id': sentinel.client_id}, {'Google Analytics': {'clientId': sentinel.provided_client_id}}, {'Google Analytics': {'clientId': sentinel.client_id}}), + ) + @override_settings(LMS_SEGMENT_KEY="testkey") + def test_track_context_with_stuff(self, tracking_context, provided_context, expected_segment_context): + # Test first with tracking and no provided context. + with self.tracker.context('test', tracking_context): + segment.track(sentinel.user_id, sentinel.name, self.properties) + args, kwargs = self.mock_segment_track.call_args + self.assertEqual((sentinel.user_id, sentinel.name, self.properties, expected_segment_context), args) + + # Test with provided context and no tracking context. + segment.track(sentinel.user_id, sentinel.name, self.properties, provided_context) + args, kwargs = self.mock_segment_track.call_args + self.assertEqual((sentinel.user_id, sentinel.name, self.properties, provided_context), args) + + # Test with provided context and also tracking context. + with self.tracker.context('test', tracking_context): + segment.track(sentinel.user_id, sentinel.name, self.properties, provided_context) + self.assertTrue(self.mock_segment_track.called) + args, kwargs = self.mock_segment_track.call_args + self.assertEqual((sentinel.user_id, sentinel.name, self.properties, provided_context), args) + + @override_settings(LMS_SEGMENT_KEY="testkey") + def test_track_with_standard_context(self): + + tracking_context = { + 'accept_language': sentinel.accept_language, + 'referer': sentinel.referer, + 'username': sentinel.username, + 'session': sentinel.session, + 'ip': sentinel.ip, + 'host': sentinel.host, + 'agent': sentinel.agent, + 'path': sentinel.path, + 'user_id': sentinel.user_id, + 'course_id': sentinel.course_id, + 'org_id': sentinel.org_id, + 'client_id': sentinel.client_id, + } + with self.tracker.context('test', tracking_context): + segment.track(sentinel.user_id, sentinel.name, self.properties) + + self.assertTrue(self.mock_segment_track.called) + args, kwargs = self.mock_segment_track.call_args + + expected_segment_context = { + 'ip': sentinel.ip, + 'Google Analytics': { + 'clientId': sentinel.client_id, + }, + 'userAgent': sentinel.agent, + 'page': { + 'path': sentinel.path, + 'referrer': sentinel.referer, + # No URL value. + } + } + self.assertEqual((sentinel.user_id, sentinel.name, self.properties, expected_segment_context), args) + + +class SegmentIdentifyTestCase(TestCase): + """Ensure emitted events contain the fields legacy processors expect to find.""" + + def setUp(self): + super(SegmentIdentifyTestCase, self).setUp() + patcher = patch('track.segment.analytics.identify') + self.mock_segment_identify = patcher.start() + self.addCleanup(patcher.stop) + self.properties = {sentinel.key: sentinel.value} + + def test_missing_key(self): + segment.identify(sentinel.user_id, self.properties) + self.assertFalse(self.mock_segment_identify.called) + + @override_settings(LMS_SEGMENT_KEY=None) + def test_null_key(self): + segment.identify(sentinel.user_id, self.properties) + self.assertFalse(self.mock_segment_identify.called) + + @override_settings(LMS_SEGMENT_KEY="testkey") + def test_normal_call(self): + segment.identify(sentinel.user_id, self.properties) + self.assertTrue(self.mock_segment_identify.called) + args, kwargs = self.mock_segment_identify.call_args + self.assertEqual((sentinel.user_id, self.properties, {}), args) + + @override_settings(LMS_SEGMENT_KEY="testkey") + def test_call_with_context(self): + provided_context = {sentinel.context_key: sentinel.context_value} + segment.identify(sentinel.user_id, self.properties, provided_context) + self.assertTrue(self.mock_segment_identify.called) + args, kwargs = self.mock_segment_identify.call_args + self.assertEqual((sentinel.user_id, self.properties, provided_context), args) diff --git a/lms/djangoapps/course_goals/tests/test_api.py b/lms/djangoapps/course_goals/tests/test_api.py index edca8d9fa506771e878c49c2b7c22758c57d9bb5..2d29d9706764ad181592c835df0d9e5e235513cb 100644 --- a/lms/djangoapps/course_goals/tests/test_api.py +++ b/lms/djangoapps/course_goals/tests/test_api.py @@ -38,13 +38,13 @@ class TestCourseGoalsAPI(EventTrackingTestCase, SharedModuleStoreTestCase): self.apiUrl = reverse('course_goals_api:v0:course_goal-list') - @mock.patch('lms.djangoapps.course_goals.views.update_google_analytics') + @mock.patch('lms.djangoapps.course_goals.views.segment.track') @override_settings(LMS_SEGMENT_KEY="foobar") def test_add_valid_goal(self, ga_call): """ Ensures a correctly formatted post succeeds.""" response = self.post_course_goal(valid=True, goal_key='certify') self.assertEqual(self.get_event(-1)['name'], EVENT_NAME_ADDED) - ga_call.assert_called_with(EVENT_NAME_ADDED, self.user.id) + ga_call.assert_called_with(self.user.id, EVENT_NAME_ADDED) self.assertEqual(response.status_code, 201) current_goals = CourseGoal.objects.filter(user=self.user, course_key=self.course.id) @@ -68,7 +68,7 @@ class TestCourseGoalsAPI(EventTrackingTestCase, SharedModuleStoreTestCase): status_code=400 ) - @mock.patch('lms.djangoapps.course_goals.views.update_google_analytics') + @mock.patch('lms.djangoapps.course_goals.views.segment.track') @override_settings(LMS_SEGMENT_KEY="foobar") def test_update_goal(self, ga_call): """ Ensures that repeated course goal post events do not create new instances of the goal. """ @@ -77,7 +77,7 @@ class TestCourseGoalsAPI(EventTrackingTestCase, SharedModuleStoreTestCase): self.post_course_goal(valid=True, goal_key='unsure') self.assertEqual(self.get_event(-1)['name'], EVENT_NAME_UPDATED) - ga_call.assert_called_with(EVENT_NAME_UPDATED, self.user.id) + ga_call.assert_called_with(self.user.id, EVENT_NAME_UPDATED) current_goals = CourseGoal.objects.filter(user=self.user, course_key=self.course.id) self.assertEqual(len(current_goals), 1) self.assertEqual(current_goals[0].goal_key, 'unsure') diff --git a/lms/djangoapps/course_goals/views.py b/lms/djangoapps/course_goals/views.py index 8a634708250b6ca7569c5a5ee25f484ef815110f..a6dd7b115142049fee73b560777d8cea8264c46b 100644 --- a/lms/djangoapps/course_goals/views.py +++ b/lms/djangoapps/course_goals/views.py @@ -2,7 +2,6 @@ Course Goals Views - includes REST API """ from __future__ import absolute_import -import analytics from django.contrib.auth import get_user_model from django.conf import settings @@ -17,6 +16,8 @@ from rest_framework import permissions, serializers, viewsets, status from rest_framework.authentication import SessionAuthentication from rest_framework.response import Response +from track import segment + from .api import get_course_goal_options from .models import CourseGoal, GOAL_KEY_CHOICES @@ -110,6 +111,7 @@ class CourseGoalViewSet(viewsets.ModelViewSet): @receiver(post_save, sender=CourseGoal, dispatch_uid="emit_course_goals_event") def emit_course_goal_event(sender, instance, **kwargs): + """Emit events for both tracking logs and for Segment.""" name = 'edx.course.goal.added' if kwargs.get('created', False) else 'edx.course.goal.updated' tracker.emit( name, @@ -117,21 +119,4 @@ def emit_course_goal_event(sender, instance, **kwargs): 'goal_key': instance.goal_key, } ) - if settings.LMS_SEGMENT_KEY: - update_google_analytics(name, instance.user.id) - - -def update_google_analytics(name, user_id): - """ Update student course goal for Google Analytics using Segment. """ - tracking_context = tracker.get_tracker().resolve_context() - context = { - 'ip': tracking_context.get('ip'), - 'Google Analytics': { - 'clientId': tracking_context.get('client_id') - } - } - analytics.track( - user_id, - name, - context=context - ) + segment.track(instance.user.id, name) diff --git a/lms/djangoapps/courseware/field_overrides.py b/lms/djangoapps/courseware/field_overrides.py index 2340f01a5ec7b8653943adc22998d48af54849b9..d2c3d93ffa8475d73df817f0fa41012d2be9f64a 100644 --- a/lms/djangoapps/courseware/field_overrides.py +++ b/lms/djangoapps/courseware/field_overrides.py @@ -102,8 +102,9 @@ class FieldOverrideProvider(object): """ __metaclass__ = ABCMeta - def __init__(self, user): + def __init__(self, user, fallback_field_data): self.user = user + self.fallback_field_data = fallback_field_data @abstractmethod def get(self, block, name, default): # pragma no cover @@ -196,7 +197,7 @@ class OverrideFieldData(FieldData): def __init__(self, user, fallback, providers): self.fallback = fallback - self.providers = tuple(provider(user) for provider in providers) + self.providers = tuple(provider(user, fallback) for provider in providers) def get_override(self, block, name): """ diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 383ca77b7902ed9803d4dbd9af22500cffa89057..fda91efde151147f81c9e802d7eafd4691985ed8 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -2191,9 +2191,9 @@ class GenerateUserCertTests(ModuleStoreTestCase): def test_user_with_passing_grade(self, mock_is_course_passed): # If user has above passing grading then json will return cert generating message and # status valid code - # mocking xqueue and analytics + # mocking xqueue and Segment analytics - analytics_patcher = patch('courseware.views.views.analytics') + analytics_patcher = patch('courseware.views.views.segment') mock_tracker = analytics_patcher.start() self.addCleanup(analytics_patcher.stop) @@ -2211,11 +2211,6 @@ class GenerateUserCertTests(ModuleStoreTestCase): 'category': 'certificates', 'label': unicode(self.course.id) }, - - context={ - 'ip': '127.0.0.1', - 'Google Analytics': {'clientId': None} - } ) mock_tracker.reset_mock() diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 0a1cf45d363cf5eaddbb5d19873747e77bd83919..8a718822f979a5dbe181ebd4b559ce2a43a04dc6 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -7,7 +7,6 @@ import urllib from collections import OrderedDict, namedtuple from datetime import datetime -import analytics from django.conf import settings from django.contrib.auth.decorators import login_required from django.contrib.auth.models import AnonymousUser, User @@ -98,6 +97,7 @@ from openedx.features.enterprise_support.api import data_sharing_consent_require from openedx.features.journals.api import get_journals_context from shoppingcart.utils import is_shopping_cart_enabled from student.models import CourseEnrollment, UserTestGroup +from track import segment from util.cache import cache, cache_if_anonymous from util.db import outer_atomic from util.milestones_helpers import get_prerequisite_courses_display @@ -1429,24 +1429,11 @@ def _track_successful_certificate_generation(user_id, course_id): None """ - if settings.LMS_SEGMENT_KEY: - event_name = 'edx.bi.user.certificate.generate' - tracking_context = tracker.get_tracker().resolve_context() - - analytics.track( - user_id, - event_name, - { - 'category': 'certificates', - 'label': text_type(course_id) - }, - context={ - 'ip': tracking_context.get('ip'), - 'Google Analytics': { - 'clientId': tracking_context.get('client_id') - } - } - ) + event_name = 'edx.bi.user.certificate.generate' + segment.track(user_id, event_name, { + 'category': 'certificates', + 'label': text_type(course_id) + }) @require_http_methods(["GET", "POST"]) diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 5473e4c9f8874884cf34ae5b4bc45f5bd3d54192..0c586d422b459572c78a166fc49c0fd43beba979 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -11,7 +11,6 @@ from datetime import datetime, timedelta from decimal import Decimal from io import BytesIO -import analytics import pytz from boto.exception import BotoServerError # this is a super-class of SESError and catches connection errors from config_models.models import ConfigurationModel @@ -37,9 +36,11 @@ from courseware.courses import get_course_by_id from edxmako.shortcuts import render_to_string from eventtracking import tracker from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx.core.djangolib.markup import HTML, Text from shoppingcart.pdf import PDFInvoice from student.models import CourseEnrollment, EnrollStatusChange from student.signals import UNENROLL_DONE +from track import segment from util.query import use_read_replica_if_available from xmodule.modulestore.django import modulestore @@ -520,19 +521,12 @@ class Order(models.Model): """ try: - if settings.LMS_SEGMENT_KEY: - tracking_context = tracker.get_tracker().resolve_context() - analytics.track(self.user.id, event_name, { - 'orderId': self.id, - 'total': str(self.total_cost), - 'currency': self.currency, - 'products': [item.analytics_data() for item in orderitems] - }, context={ - 'ip': tracking_context.get('ip'), - 'Google Analytics': { - 'clientId': tracking_context.get('client_id') - } - }) + segment.track(self.user.id, event_name, { + 'orderId': self.id, + 'total': str(self.total_cost), + 'currency': self.currency, + 'products': [item.analytics_data() for item in orderitems] + }) except Exception: # pylint: disable=broad-except # Capturing all exceptions thrown while tracking analytics events. We do not want @@ -1578,7 +1572,7 @@ class PaidCourseRegistration(OrderItem): item.unit_cost = cost item.list_price = cost item.line_desc = _(u'Registration for Course: {course_name}').format( - course_name=course.display_name_with_default_escaped) + course_name=course.display_name_with_default) item.currency = currency order.currency = currency item.report_comments = item.csv_report_comments @@ -1618,12 +1612,12 @@ class PaidCourseRegistration(OrderItem): Generates instructions when the user has purchased a PaidCourseRegistration. Basically tells the user to visit the dashboard to see their new classes """ - notification = _( + notification = Text(_( u"Please visit your {link_start}dashboard{link_end} " u"to see your new course." - ).format( - link_start=u'<a href="{url}">'.format(url=reverse('dashboard')), - link_end=u'</a>', + )).format( + link_start=HTML(u'<a href="{url}">').format(url=reverse('dashboard')), + link_end=HTML(u'</a>'), ) return self.pk_with_subclass, set([notification]) @@ -1761,7 +1755,7 @@ class CourseRegCodeItem(OrderItem): item.list_price = cost item.qty = qty item.line_desc = _(u'Enrollment codes for Course: {course_name}').format( - course_name=course.display_name_with_default_escaped) + course_name=course.display_name_with_default) item.currency = currency order.currency = currency item.report_comments = item.csv_report_comments diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index ccc51f8051c04bf3201360f6e2072e665960a404..c4a7896b06090141d1806a95aa0f3cc0e2b3744d 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -81,7 +81,7 @@ class OrderTest(ModuleStoreTestCase): self.cost = 40 # Add mock tracker for event testing. - patcher = patch('shoppingcart.models.analytics') + patcher = patch('shoppingcart.models.segment') self.mock_tracker = patcher.start() self.addCleanup(patcher.stop) @@ -288,7 +288,6 @@ class OrderTest(ModuleStoreTestCase): } ] }, - context={'ip': None, 'Google Analytics': {'clientId': None}} ) def test_purchase_item_failure(self): @@ -862,7 +861,7 @@ class CertificateItemTest(ModuleStoreTestCase): self.mock_tracker = patcher.start() self.addCleanup(patcher.stop) - analytics_patcher = patch('shoppingcart.models.analytics') + analytics_patcher = patch('shoppingcart.models.segment') self.mock_analytics_tracker = analytics_patcher.start() self.addCleanup(analytics_patcher.stop) @@ -888,7 +887,6 @@ class CertificateItemTest(ModuleStoreTestCase): } ] }, - context={'ip': None, 'Google Analytics': {'clientId': None}} ) def test_existing_enrollment(self): @@ -912,7 +910,7 @@ class CertificateItemTest(ModuleStoreTestCase): @override_settings(LMS_SEGMENT_KEY="foobar") @patch.dict(settings.FEATURES, {'STORE_BILLING_INFO': True}) - @patch('lms.djangoapps.course_goals.views.update_google_analytics', Mock(return_value=True)) + @patch('lms.djangoapps.course_goals.views.segment.track', Mock(return_value=True)) @patch('student.models.CourseEnrollment.refund_cutoff_date') def test_refund_cert_callback_no_expiration(self, cutoff_date): # When there is no expiration date on a verified mode, the user can always get a refund @@ -951,7 +949,7 @@ class CertificateItemTest(ModuleStoreTestCase): @override_settings(LMS_SEGMENT_KEY="foobar") @patch.dict(settings.FEATURES, {'STORE_BILLING_INFO': True}) - @patch('lms.djangoapps.course_goals.views.update_google_analytics', Mock(return_value=True)) + @patch('lms.djangoapps.course_goals.views.segment.track', Mock(return_value=True)) @patch('student.models.CourseEnrollment.refund_cutoff_date') def test_refund_cert_callback_before_expiration(self, cutoff_date): # If the expiration date has not yet passed on a verified mode, the user can be refunded diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 2a33bb06c1a912485dcfe69108220637243e69b0..27d8e7c1d86638602b56d08ad30cdcb4decf6c65 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -8,7 +8,6 @@ import json import logging import urllib -import analytics from django.conf import settings from django.contrib.auth.decorators import login_required from django.contrib.staticfiles.storage import staticfiles_storage @@ -49,6 +48,7 @@ from openedx.core.lib.log_utils import audit_log from shoppingcart.models import CertificateItem, Order from shoppingcart.processors import get_purchase_endpoint, get_signed_purchase_params from student.models import CourseEnrollment +from track import segment from util.db import outer_atomic from util.json_request import JsonResponse from xmodule.modulestore.django import modulestore @@ -913,7 +913,7 @@ class SubmitPhotosView(View): return response # Submit the attempt - attempt = self._submit_attempt(request.user, face_image, photo_id_image, initial_verification) + self._submit_attempt(request.user, face_image, photo_id_image, initial_verification) self._fire_event(request.user, "edx.bi.verify.submitted", {"category": "verification"}) self._send_confirmation_email(request.user) @@ -1096,15 +1096,7 @@ class SubmitPhotosView(View): Returns: None """ - if settings.LMS_SEGMENT_KEY: - tracking_context = tracker.get_tracker().resolve_context() - context = { - 'ip': tracking_context.get('ip'), - 'Google Analytics': { - 'clientId': tracking_context.get('client_id') - } - } - analytics.track(user.id, event_name, parameters, context=context) + segment.track(user.id, event_name, parameters) @require_POST diff --git a/openedx/core/djangoapps/schedules/signals.py b/openedx/core/djangoapps/schedules/signals.py index e1aed707a1db80bc946e9310f476b539188eaddb..f6d9dedb3d7d8ec7c1dc9e9239fdb1a5067679a4 100644 --- a/openedx/core/djangoapps/schedules/signals.py +++ b/openedx/core/djangoapps/schedules/signals.py @@ -2,7 +2,6 @@ import datetime import logging import random -import analytics from django.db.models.signals import post_save from django.dispatch import receiver @@ -17,6 +16,7 @@ from openedx.core.djangoapps.schedules.models import ScheduleExperience from openedx.core.djangoapps.schedules.content_highlights import course_has_highlights from openedx.core.djangoapps.theming.helpers import get_current_site from student.models import CourseEnrollment +from track import segment from .config import CREATE_SCHEDULE_WAFFLE_FLAG from .models import Schedule, ScheduleConfig from .tasks import update_course_schedules @@ -53,7 +53,7 @@ def create_schedule(sender, **kwargs): # pylint: disable=unused-argument )) -def update_schedules_on_course_start_changed(sender, updated_course_overview, previous_start_date, **kwargs): +def update_schedules_on_course_start_changed(sender, updated_course_overview, previous_start_date, **kwargs): # pylint: disable=unused-argument """ Updates all course schedules start and upgrade_deadline dates based off of the new course overview start date. @@ -136,9 +136,9 @@ def _should_randomly_suppress_schedule_creation( upgrade_deadline_str = None if upgrade_deadline: upgrade_deadline_str = upgrade_deadline.isoformat() - analytics.track( + segment.track( user_id=enrollment.user.id, - event='edx.bi.schedule.suppressed', + event_name='edx.bi.schedule.suppressed', properties={ 'course_id': unicode(enrollment.course_id), 'experience_type': experience_type, diff --git a/openedx/core/djangoapps/schedules/tests/test_signals.py b/openedx/core/djangoapps/schedules/tests/test_signals.py index a6281460c8d4a5b623cbbdd9f9c92b82a41825b8..438c5ab93baf5a22e1b6822943caf5d719b43658 100644 --- a/openedx/core/djangoapps/schedules/tests/test_signals.py +++ b/openedx/core/djangoapps/schedules/tests/test_signals.py @@ -97,7 +97,7 @@ class CreateScheduleTests(SharedModuleStoreTestCase): self.assert_schedule_created(experience_type=ScheduleExperience.EXPERIENCES.course_updates) @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) - @patch('analytics.track') + @patch('openedx.core.djangoapps.schedules.signals.segment.track') @patch('openedx.core.djangoapps.schedules.signals.random.random', return_value=0.2) @ddt.data( (0, True), @@ -121,7 +121,7 @@ class CreateScheduleTests(SharedModuleStoreTestCase): else: self.assert_schedule_not_created() mock_track.assert_called_once() - assert mock_track.call_args[1].get('event') == 'edx.bi.schedule.suppressed' + assert mock_track.call_args[1].get('event_name') == 'edx.bi.schedule.suppressed' @patch('openedx.core.djangoapps.schedules.signals.log.exception') @patch('openedx.core.djangoapps.schedules.signals.Schedule.objects.create') diff --git a/openedx/core/djangoapps/user_api/preferences/api.py b/openedx/core/djangoapps/user_api/preferences/api.py index fa348eb38fbd6f5d9d23927ac457f20c815ed0b1..2cc02ebedf1cd2870ba58f49d7400376d23d358d 100644 --- a/openedx/core/djangoapps/user_api/preferences/api.py +++ b/openedx/core/djangoapps/user_api/preferences/api.py @@ -2,8 +2,6 @@ API for managing user preferences. """ import logging -import analytics -from eventtracking import tracker from django.conf import settings from django.core.exceptions import ObjectDoesNotExist @@ -17,6 +15,7 @@ from pytz import common_timezones, common_timezones_set, country_timezones from six import text_type from student.models import User, UserProfile +from track import segment from ..errors import ( UserAPIInternalError, UserAPIRequestError, UserNotFound, UserNotAuthorized, PreferenceValidationError, PreferenceUpdateError, CountryCodeError @@ -284,21 +283,13 @@ def _track_update_email_opt_in(user_id, organization, opt_in): """ event_name = 'edx.bi.user.org_email.opted_in' if opt_in else 'edx.bi.user.org_email.opted_out' - tracking_context = tracker.get_tracker().resolve_context() - - analytics.track( + segment.track( user_id, event_name, { 'category': 'communication', 'label': organization }, - context={ - 'ip': tracking_context.get('ip'), - 'Google Analytics': { - 'clientId': tracking_context.get('client_id') - } - } ) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 5d173acc77cc47754dcd0fa5a11d21536ec5bad3..7798c7e8bf6f9bbff1594e5fd2059feb4f908160 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -6,7 +6,6 @@ Much of this file was broken out from views.py, previous history can be found th import logging -import analytics from django.conf import settings from django.contrib.auth import authenticate, login as django_login from django.contrib.auth.decorators import login_required @@ -19,7 +18,6 @@ from django.views.decorators.http import require_http_methods from ratelimitbackend.exceptions import RateLimitException from edxmako.shortcuts import render_to_response -from eventtracking import tracker from openedx.core.djangoapps.user_authn.cookies import set_logged_in_cookies, refresh_jwt_cookies from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError import openedx.core.djangoapps.external_auth.views @@ -34,6 +32,7 @@ from student.models import ( ) from student.views import send_reactivation_email_for_user from student.forms import send_password_reset_email_for_user +from track import segment import third_party_auth from third_party_auth import pipeline, provider from util.json_request import JsonResponse @@ -274,37 +273,28 @@ def _track_user_login(user, request): """ Sends a tracking event for a successful login. """ - if hasattr(settings, 'LMS_SEGMENT_KEY') and settings.LMS_SEGMENT_KEY: - tracking_context = tracker.get_tracker().resolve_context() - analytics.identify( - user.id, - { - 'email': request.POST['email'], - 'username': user.username - }, - { - # Disable MailChimp because we don't want to update the user's email - # and username in MailChimp on every page load. We only need to capture - # this data on registration/activation. - 'MailChimp': False - } - ) - - analytics.track( - user.id, - "edx.bi.user.account.authenticated", - { - 'category': "conversion", - 'label': request.POST.get('course_id'), - 'provider': None - }, - context={ - 'ip': tracking_context.get('ip'), - 'Google Analytics': { - 'clientId': tracking_context.get('client_id') - } - } - ) + segment.identify( + user.id, + { + 'email': request.POST.get('email'), + 'username': user.username + }, + { + # Disable MailChimp because we don't want to update the user's email + # and username in MailChimp on every page load. We only need to capture + # this data on registration/activation. + 'MailChimp': False + } + ) + segment.track( + user.id, + "edx.bi.user.account.authenticated", + { + 'category': "conversion", + 'label': request.POST.get('course_id'), + 'provider': None + }, + ) @login_required diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index 66009b957b1d991653ad3c027c33deedebe95a72..802725fb3da1b35fb017e525ec98d0a5047571b2 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -6,7 +6,6 @@ import datetime import json import logging -import analytics import dogstats_wrapper as dog_stats_api from django.conf import settings from django.contrib.auth import login as django_login @@ -17,7 +16,6 @@ from django.db import transaction from django.dispatch import Signal from django.utils.translation import get_language from django.utils.translation import ugettext as _ -from eventtracking import tracker # Note that this lives in LMS, so this dependency should be refactored. from notification_prefs.views import enable_notifications from pytz import UTC @@ -44,6 +42,7 @@ from student.models import ( create_comments_service_user, ) from student.views import compose_and_send_activation_email +from track import segment import third_party_auth from third_party_auth import pipeline, provider from third_party_auth.saml import SAP_SUCCESSFACTORS_SAML_KEY @@ -316,7 +315,6 @@ def _link_user_to_third_party_provider( def _track_user_registration(user, profile, params, third_party_provider): """ Track the user's registration. """ if hasattr(settings, 'LMS_SEGMENT_KEY') and settings.LMS_SEGMENT_KEY: - tracking_context = tracker.get_tracker().resolve_context() identity_args = [ user.id, { @@ -332,7 +330,7 @@ def _track_user_registration(user, profile, params, third_party_provider): 'country': text_type(profile.country), } ] - + # Provide additional context only if needed. if hasattr(settings, 'MAILCHIMP_NEW_USER_LIST_ID'): identity_args.append({ "MailChimp": { @@ -340,9 +338,8 @@ def _track_user_registration(user, profile, params, third_party_provider): } }) - analytics.identify(*identity_args) - - analytics.track( + segment.identify(*identity_args) + segment.track( user.id, "edx.bi.user.account.registered", { @@ -350,12 +347,6 @@ def _track_user_registration(user, profile, params, third_party_provider): 'label': params.get('course_id'), 'provider': third_party_provider.name if third_party_provider else None }, - context={ - 'ip': tracking_context.get('ip'), - 'Google Analytics': { - 'clientId': tracking_context.get('client_id') - } - } ) diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_register.py b/openedx/core/djangoapps/user_authn/views/tests/test_register.py index 98a0d3bfcf9bccc5bac1b1581bc0cdae7993b74c..65b7c27b4e76fbe672ca842bb357ce2c86d1a6d0 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -181,8 +181,8 @@ class TestCreateAccount(SiteMixin, TestCase): "Microsites not implemented in this environment" ) @override_settings(LMS_SEGMENT_KEY="testkey") - @mock.patch('openedx.core.djangoapps.user_authn.views.register.analytics.track') - @mock.patch('openedx.core.djangoapps.user_authn.views.register.analytics.identify') + @mock.patch('openedx.core.djangoapps.user_authn.views.register.segment.track') + @mock.patch('openedx.core.djangoapps.user_authn.views.register.segment.identify') def test_segment_tracking(self, mock_segment_identify, _): year = datetime.now().year year_of_birth = year - 14 diff --git a/openedx/features/content_type_gating/field_override.py b/openedx/features/content_type_gating/field_override.py index c01d0c13aa053069bd2c161af96cac945bdbbe23..42d22a811a58748f53d5f45f00672ead69650786 100644 --- a/openedx/features/content_type_gating/field_override.py +++ b/openedx/features/content_type_gating/field_override.py @@ -4,7 +4,7 @@ students in the Unlocked Group of the ContentTypeGating partition. """ from django.conf import settings -from lms.djangoapps.courseware.field_overrides import FieldOverrideProvider, disable_overrides +from lms.djangoapps.courseware.field_overrides import FieldOverrideProvider from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID from openedx.features.course_duration_limits.config import ( CONTENT_TYPE_GATING_FLAG, @@ -31,9 +31,23 @@ class ContentTypeGatingFieldOverride(FieldOverrideProvider): if not problem_eligible_for_content_gating: return default - # Read the group_access from the fallback field-data service - with disable_overrides(): - original_group_access = block.group_access + # We want to fetch the value set by course authors since it should take precedence. + # We cannot simply call "block.group_access" to fetch that value even if we disable + # field overrides since it will set the group access field to "dirty" with + # the value read from the course content. Since most content does not have any + # value for this field it will usually be the default empty dict. This field + # override changes the value, however, resulting in the LMS thinking that the + # field data needs to be written back out to the store. This doesn't work, + # however, since this is a read-only setting in the LMS context. After this + # call to get() returns, the _dirty_fields dict will be set correctly to contain + # the value from this field override. This prevents the system from attempting + # to save the overridden value when it thinks it has changed when it hasn't. + original_group_access = None + if self.fallback_field_data.has(block, 'group_access'): + raw_value = self.fallback_field_data.get(block, 'group_access') + group_access_field = block.fields.get('group_access') + if group_access_field is not None: + original_group_access = group_access_field.from_json(raw_value) if original_group_access is None: original_group_access = {} diff --git a/openedx/features/content_type_gating/tests/test_access.py b/openedx/features/content_type_gating/tests/test_access.py index 2ad97ffa9ce3b45c760db85f43b728461a2c1685..3fb1a8ea0f1e16c51c975531c3d9f3bd65cf4397 100644 --- a/openedx/features/content_type_gating/tests/test_access.py +++ b/openedx/features/content_type_gating/tests/test_access.py @@ -3,17 +3,17 @@ Test audit user's access to various content based on content-gating features. """ import ddt -from django.http import Http404 +from django.conf import settings from django.test.client import RequestFactory from django.test.utils import override_settings from django.urls import reverse from mock import patch from course_modes.tests.factories import CourseModeFactory -from courseware.access_response import IncorrectPartitionGroupError from lms.djangoapps.courseware.module_render import load_single_xblock from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from openedx.core.lib.url_utils import quote_slashes +from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID from openedx.features.course_duration_limits.config import CONTENT_TYPE_GATING_FLAG from student.tests.factories import TEST_PASSWORD, AdminFactory, CourseEnrollmentFactory, UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -82,14 +82,14 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): cls.graded_score_weight_blocks[(graded, has_score, weight)] = block # add LTI blocks to default course - cls.lti_block = ItemFactory.create( + cls.blocks_dict['lti_block'] = ItemFactory.create( parent=cls.blocks_dict['vertical'], category='lti_consumer', display_name='lti_consumer', has_score=True, graded=True, ) - cls.lti_block_not_scored = ItemFactory.create( + cls.blocks_dict['lti_block_not_scored'] = ItemFactory.create( parent=cls.blocks_dict['vertical'], category='lti_consumer', display_name='lti_consumer_2', @@ -97,19 +97,32 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): ) # add ungraded problem for xblock_handler test - cls.graded_problem = ItemFactory.create( + cls.blocks_dict['graded_problem'] = ItemFactory.create( parent=cls.blocks_dict['vertical'], category='problem', display_name='graded_problem', graded=True, ) - cls.ungraded_problem = ItemFactory.create( + cls.blocks_dict['ungraded_problem'] = ItemFactory.create( parent=cls.blocks_dict['vertical'], category='problem', display_name='ungraded_problem', graded=False, ) + cls.blocks_dict['audit_visible_graded_problem'] = ItemFactory.create( + parent=cls.blocks_dict['vertical'], + category='problem', + display_name='audit_visible_graded_problem', + graded=True, + group_access={ + CONTENT_GATING_PARTITION_ID: [ + settings.CONTENT_TYPE_GATE_GROUP_IDS['limited_access'], + settings.CONTENT_TYPE_GATE_GROUP_IDS['full_access'] + ] + }, + ) + # audit_only course only has an audit track available cls.courses['audit_only'] = cls._create_course( run='audit_only_course_run_1', @@ -205,14 +218,14 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): display_name='Lesson 1 Vertical - Unit 1' ) - for problem_type in component_types: + for component_type in component_types: block = ItemFactory.create( parent=blocks_dict['vertical'], - category=problem_type, - display_name=problem_type, + category=component_type, + display_name=component_type, graded=True, ) - blocks_dict[problem_type] = block + blocks_dict[component_type] = block return { 'course': course, @@ -224,14 +237,8 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): """ Asserts that a block in a specific course is gated for a specific user - This functions asserts whether the passed in block is gated by content type gating. - This is determined by checking whether the has_access method called the IncorrectPartitionGroupError. - This error gets swallowed up and is raised as a 404, which is why we are checking for a 404 being raised. - However, the 404 could also be caused by other errors, which is why the actual assertion is checking - whether the IncorrectPartitionGroupError was called. - Arguments: - block: some soft of xblock descriptor, must implement .scope_ids.usage_id + block: some sort of xblock descriptor, must implement .scope_ids.usage_id is_gated (bool): if True, this user is expected to be gated from this block user_id (int): id of user, if not set will be set to self.audit_user.id course_id (CourseLocator): id of course, if not set will be set to self.course.id @@ -239,61 +246,52 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): fake_request = self.factory.get('') mock_get_current_request.return_value = fake_request - with patch.object(IncorrectPartitionGroupError, '__init__', - wraps=IncorrectPartitionGroupError.__init__) as mock_access_error: - if is_gated: - with self.assertRaises(Http404): - load_single_xblock( - request=fake_request, - user_id=user_id, - course_id=unicode(course_id), - usage_key_string=unicode(block.scope_ids.usage_id), - course=None - ) - # check that has_access raised the IncorrectPartitionGroupError in order to gate the block - self.assertTrue(mock_access_error.called) - else: - load_single_xblock( - request=fake_request, - user_id=user_id, - course_id=unicode(course_id), - usage_key_string=unicode(block.scope_ids.usage_id), - course=None - ) - # check that has_access did not raise the IncorrectPartitionGroupError thereby not gating the block - self.assertFalse(mock_access_error.called) - - def test_lti_audit_access(self): - """ - LTI stands for learning tools interoperability and is a 3rd party iframe that pulls in learning content from - outside sources. This tests that audit users cannot see LTI components with graded content but can see the LTI - components which do not have graded content. - """ - self._assert_block_is_gated( - block=self.lti_block, - user_id=self.audit_user.id, - course_id=self.course.id, - is_gated=True - ) - self._assert_block_is_gated( - block=self.lti_block_not_scored, - user_id=self.audit_user.id, - course_id=self.course.id, - is_gated=False + # Load a block we know will pass access control checks + vertical_xblock = load_single_xblock( + request=fake_request, + user_id=user_id, + course_id=unicode(course_id), + usage_key_string=unicode(self.blocks_dict['vertical'].scope_ids.usage_id), + course=None ) + runtime = vertical_xblock.runtime + + # This method of fetching the block from the descriptor bypassess access checks + problem_block = runtime.get_module(block) + + # Attempt to render the block, this should return different fragments if the content is gated or not. + frag = runtime.render(problem_block, 'student_view') + if is_gated: + assert 'content-paywall' in frag.content + else: + assert 'content-paywall' not in frag.content + @ddt.data( - *PROBLEM_TYPES + ('problem', True), + ('openassessment', True), + ('drag-and-drop-v2', True), + ('done', True), + ('edx_sga', True), + ('lti_block', True), + ('ungraded_problem', False), + ('lti_block_not_scored', False), + ('audit_visible_graded_problem', False), ) - def test_audit_fails_access_graded_problems(self, prob_type): - block = self.blocks_dict[prob_type] - is_gated = True + @ddt.unpack + def test_access_to_problems(self, prob_type, is_gated): self._assert_block_is_gated( - block=block, - user_id=self.audit_user.id, + block=self.blocks_dict[prob_type], + user_id=self.users['audit'].id, course_id=self.course.id, is_gated=is_gated ) + self._assert_block_is_gated( + block=self.blocks_dict[prob_type], + user_id=self.users['verified'].id, + course_id=self.course.id, + is_gated=False + ) @ddt.data( *GRADED_SCORE_WEIGHT_TEST_CASES diff --git a/requirements/edx/base.in b/requirements/edx/base.in index 6ed70056f8ef26c1d96dc708282bd8ee4c966307..30be70acce35aa0d84036f58215ebd25dfdcfe31 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -23,7 +23,7 @@ # 4. If the package is not needed in production, add it to another file such # as development.in or testing.in instead. -analytics-python==1.1.0 # Used for Segment analytics +analytics-python==1.2.9 # Used for Segment analytics attrs # Reduces boilerplate code involving class attributes Babel==1.3 # Internationalization utilities, used for date formatting in a few places bleach==2.1.4 # Allowed-list-based HTML sanitizing library that escapes or strips markup and attributes; used for capa and LTI diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index da96919c725ed799d5fdbdc216a0f45a4d5c1088..a20ad9c0795d572b7996f07426fbdc7e9eaad48a 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -40,7 +40,7 @@ git+https://github.com/edx-solutions/xblock-drag-and-drop-v2@v2.1.6#egg=xblock-d git+https://github.com/open-craft/xblock-poll@add89e14558c30f3c8dc7431e5cd6536fff6d941#egg=xblock-poll==1.5.1 -e common/lib/xmodule amqp==1.4.9 # via kombu -analytics-python==1.1.0 +analytics-python==1.2.9 aniso8601==4.0.1 # via tincan anyjson==0.3.3 # via kombu appdirs==1.4.3 # via fs diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index afacd497615516d3113f08ff2d1d67fcea0f6393..e24d30b9c743b0df2cb06574677d7457a1152b20 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -44,7 +44,7 @@ git+https://github.com/open-craft/xblock-poll@add89e14558c30f3c8dc7431e5cd6536ff -e common/lib/xmodule alabaster==0.7.12 # via sphinx amqp==1.4.9 -analytics-python==1.1.0 +analytics-python==1.2.9 aniso8601==4.0.1 anyjson==0.3.3 apipkg==1.5 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 3b0fbe003f2714d26a2612533166adefa4aeeb64..8013ec0448fd6d84aa5a8c4d24b65d44c9578db3 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -41,7 +41,7 @@ git+https://github.com/edx-solutions/xblock-drag-and-drop-v2@v2.1.6#egg=xblock-d git+https://github.com/open-craft/xblock-poll@add89e14558c30f3c8dc7431e5cd6536fff6d941#egg=xblock-poll==1.5.1 -e common/lib/xmodule amqp==1.4.9 -analytics-python==1.1.0 +analytics-python==1.2.9 aniso8601==4.0.1 anyjson==0.3.3 apipkg==1.5 # via execnet