Merge pull request #14631 from open-craft/jill/ent-204-consent-wiki-forums

[ENT-204] Require data sharing consent before accessing forum and wiki

common/djangoapps/util/enterprise_helpers.py

@@ -3,9 +3,11 @@ Helpers to access the enterprise app
 """
 import logging
 
+from functools import wraps
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
+from django.shortcuts import redirect
 from django.utils.http import urlencode
 from edx_rest_api_client.client import EdxRestApiClient
 try:
@@ -70,6 +72,36 @@ class EnterpriseApiClient(object):
             raise EnterpriseApiException(message)
 
 
+def data_sharing_consent_required(view_func):
+    """
+    Decorator which makes a view method redirect to the Data Sharing Consent form if:
+
+    * The wrapped method is passed request, course_id as the first two arguments.
+    * Enterprise integration is enabled
+    * Data sharing consent is required before accessing this course view.
+    * The request.user has not yet given data sharing consent for this course.
+
+    After granting consent, the user will be redirected back to the original request.path.
+
+    """
+    @wraps(view_func)
+    def inner(request, course_id, *args, **kwargs):
+        """
+        Redirect to the consent page if the request.user must consent to data sharing before viewing course_id.
+
+        Otherwise, just call the wrapped view function.
+        """
+        # Redirect to the consent URL, if consent is required.
+        consent_url = get_enterprise_consent_url(request, course_id)
+        if consent_url:
+            return redirect(consent_url)
+
+        # Otherwise, drop through to wrapped view
+        return view_func(request, course_id, *args, **kwargs)
+
+    return inner
+
+
 def enterprise_enabled():
     """
     Determines whether the Enterprise app is installed
@@ -87,14 +119,32 @@ def consent_needed_for_course(user, course_id):
     return consent_necessary_for_course(user, course_id)
 
 
-def get_course_specific_consent_url(request, course_id, return_to):
+def get_enterprise_consent_url(request, course_id, user=None, return_to=None):
     """
     Build a URL to redirect the user to the Enterprise app to provide data sharing
     consent for a specific course ID.
+
+    Arguments:
+    * request: Request object
+    * course_id: Course key/identifier string.
+    * user: user to check for consent. If None, uses request.user
+    * return_to: url name label for the page to return to after consent is granted.
+                 If None, return to request.path instead.
     """
+    if user is None:
+        user = request.user
+
+    if not consent_needed_for_course(user, course_id):
+        return None
+
+    if return_to is None:
+        return_path = request.path
+    else:
+        return_path = reverse(return_to, args=(course_id,))
+
     url_params = {
         'course_id': course_id,
-        'next': request.build_absolute_uri(reverse(return_to, args=(course_id,)))
+        'next': request.build_absolute_uri(return_path)
     }
     querystring = urlencode(url_params)
     full_url = reverse('grant_data_sharing_permissions') + '?' + querystring

common/djangoapps/util/tests/mixins/enterprise.py

@@ -2,6 +2,7 @@
 Mixins for the EnterpriseApiClient.
 """
 import json
+import mock
 
 import httpretty
 from django.conf import settings
@@ -55,3 +56,37 @@ class EnterpriseServiceMockMixin(object):
             content_type='application/json',
             status=500
         )
+
+
+class EnterpriseTestConsentRequired(object):
+    """
+    Mixin to help test the data_sharing_consent_required decorator.
+    """
+    def verify_consent_required(self, client, url, status_code=200):
+        """
+        Verify that the given URL redirects to the consent page when consent is required,
+        and doesn't redirect to the consent page when consent is not required.
+
+        Arguments:
+        * self: ignored
+        * client: the TestClient instance to be used
+        * url: URL to test
+        * status_code: expected status code of URL when no data sharing consent is required.
+        """
+        with mock.patch('util.enterprise_helpers.enterprise_enabled', return_value=True):
+            with mock.patch('util.enterprise_helpers.consent_necessary_for_course') as mock_consent_necessary:
+                # Ensure that when consent is necessary, the user is redirected to the consent page.
+                mock_consent_necessary.return_value = True
+                response = client.get(url)
+                assert response.status_code == 302
+                assert 'grant_data_sharing_permissions' in response.url  # pylint: disable=no-member
+
+                # Ensure that when consent is not necessary, the user continues through to the requested page.
+                mock_consent_necessary.return_value = False
+                response = client.get(url)
+                assert response.status_code == status_code
+
+                # If we were expecting a redirect, ensure it's not to the data sharing permission page
+                if status_code == 302:
+                    assert 'grant_data_sharing_permissions' not in response.url  # pylint: disable=no-member
+                return response

common/djangoapps/util/tests/test_enterprise_helpers.py

@@ -4,11 +4,14 @@ Test the enterprise app helpers
 import unittest
 
 from django.conf import settings
+from django.http import HttpResponseRedirect
+from django.test.utils import override_settings
 import mock
 
 from util.enterprise_helpers import (
     enterprise_enabled,
     insert_enterprise_pipeline_elements,
+    data_sharing_consent_required,
     set_enterprise_branding_filter_param,
     get_enterprise_branding_filter_param,
     get_enterprise_customer_logo_url
@@ -21,21 +24,28 @@ class TestEnterpriseHelpers(unittest.TestCase):
     Test enterprise app helpers
     """
 
-    @mock.patch('util.enterprise_helpers.enterprise_enabled')
-    def test_utils_with_enterprise_disabled(self, mock_enterprise_enabled):
+    @override_settings(ENABLE_ENTERPRISE_INTEGRATION=False)
+    def test_utils_with_enterprise_disabled(self):
         """
-        Test that the enterprise app not being available causes
+        Test that disabling the enterprise integration flag causes
         the utilities to return the expected default values.
         """
-        mock_enterprise_enabled.return_value = False
+        self.assertFalse(enterprise_enabled())
         self.assertEqual(insert_enterprise_pipeline_elements(None), None)
 
-    def test_enterprise_enabled(self):
+    @override_settings(ENABLE_ENTERPRISE_INTEGRATION=True)
+    def test_utils_with_enterprise_enabled(self):
         """
-        The test settings inherit from common, which have the enterprise
-        app installed; therefore, it should appear installed here.
+        Test that enabling enterprise integration (which is currently on by default) causes the
+        the utilities to return the expected values.
         """
         self.assertTrue(enterprise_enabled())
+        pipeline = ['abc', 'social.pipeline.social_auth.load_extra_data', 'def']
+        insert_enterprise_pipeline_elements(pipeline)
+        self.assertEqual(pipeline, ['abc',
+                                    'enterprise.tpa_pipeline.handle_enterprise_logistration',
+                                    'social.pipeline.social_auth.load_extra_data',
+                                    'def'])
 
     def test_set_enterprise_branding_filter_param(self):
         """
@@ -51,7 +61,7 @@ class TestEnterpriseHelpers(unittest.TestCase):
         set_enterprise_branding_filter_param(request, provider_id=provider_id)
         self.assertEqual(get_enterprise_branding_filter_param(request), {'provider_id': provider_id})
 
-    @mock.patch('util.enterprise_helpers.enterprise_enabled', mock.Mock(return_value=True))
+    @override_settings(ENABLE_ENTERPRISE_INTEGRATION=True)
     def test_get_enterprise_customer_logo_url(self):
         """
         Test test_get_enterprise_customer_logo_url return the logo url as desired.
@@ -75,7 +85,7 @@ class TestEnterpriseHelpers(unittest.TestCase):
             logo_url = get_enterprise_customer_logo_url(request)
             self.assertEqual(logo_url, '/test/image.png')
 
-    @mock.patch('util.enterprise_helpers.enterprise_enabled', mock.Mock(return_value=False))
+    @override_settings(ENABLE_ENTERPRISE_INTEGRATION=False)
     def test_get_enterprise_customer_logo_url_return_none(self):
         """
         Test get_enterprise_customer_logo_url return 'None' when enterprise application is not installed.
@@ -88,7 +98,7 @@ class TestEnterpriseHelpers(unittest.TestCase):
             logo_url = get_enterprise_customer_logo_url(request)
             self.assertEqual(logo_url, None)
 
-    @mock.patch('util.enterprise_helpers.enterprise_enabled', mock.Mock(return_value=True))
+    @override_settings(ENABLE_ENTERPRISE_INTEGRATION=True)
     @mock.patch('util.enterprise_helpers.get_enterprise_branding_filter_param', mock.Mock(return_value=None))
     def test_get_enterprise_customer_logo_url_return_none_when_param_missing(self):
         """
@@ -101,3 +111,84 @@ class TestEnterpriseHelpers(unittest.TestCase):
         with mock.patch('enterprise.utils.get_enterprise_branding_info_by_provider_id', return_value=branding_info):
             logo_url = get_enterprise_customer_logo_url(request)
             self.assertEqual(logo_url, None)
+
+    def check_data_sharing_consent(self, consent_required=False, consent_url=None):
+        """
+        Used to test the data_sharing_consent_required view decorator.
+        """
+
+        # Test by wrapping a function that has the expected signature
+        @data_sharing_consent_required
+        def view_func(request, course_id, *args, **kwargs):
+            """
+            Return the function arguments, so they can be tested.
+            """
+            return ((request, course_id,) + args, kwargs)
+
+        # Call the wrapped function
+        args = (mock.MagicMock(), 'course-id', 'another arg', 'and another')
+        kwargs = dict(a=1, b=2, c=3)
+        response = view_func(*args, **kwargs)
+
+        # If consent required, then the response should be a redirect to the consent URL, and the view function would
+        # not be called.
+        if consent_required:
+            self.assertIsInstance(response, HttpResponseRedirect)
+            self.assertEquals(response.url, consent_url)  # pylint: disable=no-member
+
+        # Otherwise, the view function should have been called with the expected arguments.
+        else:
+            self.assertEqual(response, (args, kwargs))
+
+    @mock.patch('util.enterprise_helpers.enterprise_enabled')
+    @mock.patch('util.enterprise_helpers.consent_necessary_for_course')
+    def test_data_consent_required_enterprise_disabled(self,
+                                                       mock_consent_necessary,
+                                                       mock_enterprise_enabled):
+        """
+        Verify that the wrapped view is called directly when enterprise integration is disabled,
+        without checking for course consent necessary.
+        """
+        mock_enterprise_enabled.return_value = False
+
+        self.check_data_sharing_consent(consent_required=False)
+
+        mock_enterprise_enabled.assert_called_once()
+        mock_consent_necessary.assert_not_called()
+
+    @mock.patch('util.enterprise_helpers.enterprise_enabled')
+    @mock.patch('util.enterprise_helpers.consent_necessary_for_course')
+    def test_no_course_data_consent_required(self,
+                                             mock_consent_necessary,
+                                             mock_enterprise_enabled):
+
+        """
+        Verify that the wrapped view is called directly when enterprise integration is enabled,
+        and no course consent is required.
+        """
+        mock_enterprise_enabled.return_value = True
+        mock_consent_necessary.return_value = False
+
+        self.check_data_sharing_consent(consent_required=False)
+
+        mock_enterprise_enabled.assert_called_once()
+        mock_consent_necessary.assert_called_once()
+
+    @mock.patch('util.enterprise_helpers.enterprise_enabled')
+    @mock.patch('util.enterprise_helpers.consent_necessary_for_course')
+    @mock.patch('util.enterprise_helpers.get_enterprise_consent_url')
+    def test_data_consent_required(self, mock_get_consent_url, mock_consent_necessary, mock_enterprise_enabled):
+        """
+        Verify that the wrapped function returns a redirect to the consent URL when enterprise integration is enabled,
+        and course consent is required.
+        """
+        mock_enterprise_enabled.return_value = True
+        mock_consent_necessary.return_value = True
+        consent_url = '/abc/def'
+        mock_get_consent_url.return_value = consent_url
+
+        self.check_data_sharing_consent(consent_required=True, consent_url=consent_url)
+
+        mock_get_consent_url.assert_called_once()
+        mock_enterprise_enabled.assert_called_once()
+        mock_consent_necessary.assert_called_once()

lms/djangoapps/course_wiki/middleware.py

@@ -10,6 +10,7 @@ from courseware.courses import get_course_with_access, get_course_overview_with_
 from courseware.access import has_access
 from student.models import CourseEnrollment
 from util.request import course_id_from_url
+from util.enterprise_helpers import get_enterprise_consent_url
 
 
 class WikiAccessMiddleware(object):
@@ -75,6 +76,12 @@ class WikiAccessMiddleware(object):
                     # if a user is logged in, but not authorized to see a page,
                     # we'll redirect them to the course about page
                     return redirect('about_course', course_id.to_deprecated_string())
+
+                # If we need enterprise data sharing consent for this course, then redirect to the form.
+                consent_url = get_enterprise_consent_url(request, course_id)
+                if consent_url:
+                    return redirect(consent_url)
+
             # set the course onto here so that the wiki template can show the course navigation
             request.course = course
         else:

lms/djangoapps/course_wiki/tests/tests.py

@@ -2,6 +2,7 @@ from django.core.urlresolvers import reverse
 from nose.plugins.attrib import attr
 
 from courseware.tests.tests import LoginEnrollmentTestCase
+from util.tests.mixins.enterprise import EnterpriseTestConsentRequired
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
@@ -9,7 +10,7 @@ from mock import patch
 
 
 @attr(shard=1)
-class WikiRedirectTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
+class WikiRedirectTestCase(EnterpriseTestConsentRequired, LoginEnrollmentTestCase, ModuleStoreTestCase):
     """
     Tests for wiki course redirection.
     """
@@ -196,3 +197,25 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
 
         resp = self.client.get(course_wiki_page, follow=True, HTTP_REFERER=referer)
         self.assertEqual(resp.status_code, 200)
+
+    @patch.dict("django.conf.settings.FEATURES", {'ALLOW_WIKI_ROOT_ACCESS': True})
+    def test_consent_required(self):
+        """
+        Test that enterprise data sharing consent is required when enabled for the various courseware views.
+        """
+        # Public wikis can be accessed by non-enrolled users, and so direct access is not gated by the consent page
+        course = CourseFactory.create()
+        course.allow_public_wiki_access = False
+        course.save()
+
+        # However, for private wikis, enrolled users must pass through the consent gate
+        # (Unenrolled users are redirected to course/about)
+        course_id = unicode(course.id)
+        self.login(self.student, self.password)
+        self.enroll(course)
+
+        for (url, status_code) in (
+                (reverse('course_wiki', kwargs={'course_id': course_id}), 302),
+                ('/courses/{}/wiki/'.format(course_id), 200),
+        ):
+            self.verify_consent_required(self.client, url, status_code)

lms/djangoapps/course_wiki/views.py

@@ -16,6 +16,7 @@ from courseware.courses import get_course_by_id
 from course_wiki.utils import course_wiki_slug
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
+from util.enterprise_helpers import data_sharing_consent_required
 
 log = logging.getLogger(__name__)
 
@@ -29,7 +30,8 @@ def root_create(request):  # pylint: disable=unused-argument
     return redirect('wiki:get', path=root.path)
 
 
-def course_wiki_redirect(request, course_id):  # pylint: disable=unused-argument
+@data_sharing_consent_required
+def course_wiki_redirect(request, course_id, wiki_path=""):  # pylint: disable=unused-argument
     """
     This redirects to whatever page on the wiki that the course designates
     as it's home page. A course's wiki must be an article on the root (for
@@ -50,7 +52,7 @@ def course_wiki_redirect(request, course_id):  # pylint: disable=unused-argument
         return redirect("wiki:get", path="")
 
     try:
-        urlpath = URLPath.get_by_path(course_slug, select_related=True)
+        urlpath = URLPath.get_by_path(wiki_path or course_slug, select_related=True)
 
         results = list(Article.objects.filter(id=urlpath.article.id))
         if results:

lms/djangoapps/courseware/tests/test_course_info.py

@@ -60,9 +60,8 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
         resp = self.client.get(url)
         self.assertNotIn("You are not currently enrolled in this course", resp.content)
 
-    @mock.patch('courseware.views.views.get_course_specific_consent_url')
-    @mock.patch('courseware.views.views.consent_needed_for_course')
-    def test_redirection_missing_enterprise_consent(self, mock_consent_needed, mock_get_url):
+    @mock.patch('courseware.views.views.get_enterprise_consent_url')
+    def test_redirection_missing_enterprise_consent(self, mock_get_url):
         """
         Verify that users viewing the course info who are enrolled, but have not provided
         data sharing consent, are first redirected to a consent page, and then, once they've
@@ -70,7 +69,6 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
         """
         self.setup_user()
         self.enroll(self.course)
-        mock_consent_needed.return_value = True
         mock_get_url.return_value = reverse('dashboard')
         url = reverse('info', args=[self.course.id.to_deprecated_string()])
 
@@ -80,8 +78,8 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
             response,
             reverse('dashboard')
         )
-        mock_consent_needed.assert_called_once_with(self.user, unicode(self.course.id))
-        mock_consent_needed.return_value = False
+        mock_get_url.assert_called_once()
+        mock_get_url.return_value = None
         response = self.client.get(url)
         self.assertNotIn("You are not currently enrolled in this course", response.content)
 

lms/djangoapps/courseware/tests/test_view_authentication.py

@@ -197,28 +197,28 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
             )
         )
 
-    @patch('courseware.views.index.get_course_specific_consent_url')
-    @patch('courseware.views.index.consent_needed_for_course')
-    def test_redirection_missing_enterprise_consent(self, mock_consent_needed, mock_get_url):
+    @patch('courseware.views.index.get_enterprise_consent_url')
+    def test_redirection_missing_enterprise_consent(self, mock_get_url):
         """
         Verify that enrolled students are redirected to the Enterprise consent
         URL if a linked Enterprise Customer requires data sharing consent
         and it has not yet been provided.
         """
-        mock_consent_needed.return_value = True
         mock_get_url.return_value = reverse('dashboard')
         self.login(self.enrolled_user)
-        response = self.client.get(
-            reverse(
-                'courseware',
-                kwargs={'course_id': self.course.id.to_deprecated_string()}
-            )
+        url = reverse(
+            'courseware',
+            kwargs={'course_id': self.course.id.to_deprecated_string()}
         )
+        response = self.client.get(url)
         self.assertRedirects(
             response,
             reverse('dashboard')
         )
-        mock_consent_needed.assert_called_once_with(self.enrolled_user, unicode(self.course.id))
+        mock_get_url.assert_called_once()
+        mock_get_url.return_value = None
+        response = self.client.get(url)
+        self.assertNotIn("You are not currently enrolled in this course", response.content)
 
     def test_instructor_page_access_nonstaff(self):
         """

lms/djangoapps/courseware/tests/test_views.py

@@ -54,6 +54,7 @@ from openedx.core.lib.gating import api as gating_api
 from openedx.core.djangoapps.crawlers.models import CrawlersConfig
 from student.models import CourseEnrollment
 from student.tests.factories import AdminFactory, UserFactory, CourseEnrollmentFactory
+from util.tests.mixins.enterprise import EnterpriseTestConsentRequired
 from util.tests.test_date_utils import fake_ugettext, fake_pgettext
 from util.url import reload_django_url_config
 from util.views import ensure_valid_course_key
@@ -2201,3 +2202,28 @@ class TestIndexViewCrawlerStudentStateWrites(SharedModuleStoreTestCase):
         # Make sure we get back an actual 200, and aren't redirected because we
         # messed up the setup somehow (e.g. didn't enroll properly)
         self.assertEqual(response.status_code, 200)
+
+
+@attr(shard=1)
+class EnterpriseConsentTestCase(EnterpriseTestConsentRequired, ModuleStoreTestCase):
+    """
+    Ensure that the Enterprise Data Consent redirects are in place only when consent is required.
+    """
+    def setUp(self):
+        super(EnterpriseConsentTestCase, self).setUp()
+        self.user = UserFactory.create()
+        self.assertTrue(self.client.login(username=self.user.username, password='test'))
+        self.course = CourseFactory.create()
+        CourseEnrollmentFactory(user=self.user, course_id=self.course.id)
+
+    def test_consent_required(self):
+        """
+        Test that enterprise data sharing consent is required when enabled for the various courseware views.
+        """
+        course_id = unicode(self.course.id)
+        for url in (
+                reverse("courseware", kwargs=dict(course_id=course_id)),
+                reverse("progress", kwargs=dict(course_id=course_id)),
+                reverse("student_progress", kwargs=dict(course_id=course_id, student_id=str(self.user.id))),
+        ):
+            self.verify_consent_required(self.client, url)

lms/djangoapps/courseware/views/index.py

@@ -31,7 +31,7 @@ from shoppingcart.models import CourseRegistrationCode
 from student.models import CourseEnrollment
 from student.views import is_course_blocked
 from student.roles import GlobalStaff
-from util.enterprise_helpers import consent_needed_for_course, get_course_specific_consent_url
+from util.enterprise_helpers import get_enterprise_consent_url
 from util.views import ensure_valid_course_key
 from xmodule.modulestore.django import modulestore
 from xmodule.x_module import STUDENT_VIEW
@@ -203,13 +203,14 @@ class CoursewareIndex(View):
         the course, and redirect the user to provide consent if needed.
         """
         course_id = unicode(self.course_key)
-        if consent_needed_for_course(self.real_user, course_id):
+        consent_url = get_enterprise_consent_url(self.request, course_id, user=self.real_user, return_to='courseware')
+        if consent_url:
             log.warning(
                 u'User %s cannot access the course %s because they have not granted consent',
                 self.real_user,
                 course_id,
             )
-            raise Redirect(get_course_specific_consent_url(self.request, course_id, 'courseware'))
+            raise Redirect(consent_url)
 
     def _redirect_if_needed_to_pay_for_course(self):
         """

lms/djangoapps/courseware/views/views.py

@@ -93,7 +93,7 @@ from student.roles import GlobalStaff
 from util.cache import cache, cache_if_anonymous
 from util.date_utils import strftime_localized
 from util.db import outer_atomic
-from util.enterprise_helpers import consent_needed_for_course, get_course_specific_consent_url
+from util.enterprise_helpers import get_enterprise_consent_url
 from util.milestones_helpers import get_prerequisite_courses_display
 from util.views import _record_feedback_in_zendesk
 from util.views import ensure_valid_course_key, ensure_valid_usage_key
@@ -330,8 +330,9 @@ def course_info(request, course_id):
 
         # If the user is sponsored by an enterprise customer, and we still need to get data
         # sharing consent, redirect to do that first.
-        if consent_needed_for_course(user, course_id):
-            return redirect(get_course_specific_consent_url(request, course_id, 'info'))
+        consent_url = get_enterprise_consent_url(request, course_id, user=user, return_to='info')
+        if consent_url:
+            return redirect(consent_url)
 
         # If the user needs to take an entrance exam to access this course, then we'll need
         # to send them to that specific course module before allowing them into other areas
@@ -818,8 +819,9 @@ def _progress(request, course_key, student_id):
 
     # If the user is sponsored by an enterprise customer, and we still need to get data
     # sharing consent, redirect to do that first.
-    if consent_needed_for_course(request.user, unicode(course.id)):
-        return redirect(get_course_specific_consent_url(request, unicode(course.id), 'progress'))
+    consent_url = get_enterprise_consent_url(request, unicode(course.id), return_to='progress')
+    if consent_url:
+        return redirect(consent_url)
 
     # check to see if there is a required survey that must be taken before
     # the user can access the course.

lms/djangoapps/discussion/tests/test_views.py

@@ -23,6 +23,7 @@ from django_comment_client.utils import strip_none
 from lms.djangoapps.discussion import views
 from student.tests.factories import UserFactory, CourseEnrollmentFactory
 from util.testing import UrlResetMixin
+from util.tests.mixins.enterprise import EnterpriseTestConsentRequired
 from openedx.core.djangoapps.util.testing import ContentGroupTestCase
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
@@ -356,16 +357,26 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase):
         # course is outside the context manager that is verifying the number of queries,
         # and with split mongo, that method ends up querying disabled_xblocks (which is then
         # cached and hence not queried as part of call_single_thread).
-        (ModuleStoreEnum.Type.mongo, 1, 5, 3, 13, 1),
-        (ModuleStoreEnum.Type.mongo, 50, 5, 3, 13, 1),
+        (ModuleStoreEnum.Type.mongo, False, 1, 5, 3, 13, 1),
+        (ModuleStoreEnum.Type.mongo, False, 50, 5, 3, 13, 1),
         # split mongo: 3 queries, regardless of thread response size.
-        (ModuleStoreEnum.Type.split, 1, 3, 3, 12, 1),
-        (ModuleStoreEnum.Type.split, 50, 3, 3, 12, 1),
+        (ModuleStoreEnum.Type.split, False, 1, 3, 3, 12, 1),
+        (ModuleStoreEnum.Type.split, False, 50, 3, 3, 12, 1),
+
+        # Enabling Enterprise integration increases the number of (cached and uncached) SQL queries by 1,
+        # because the presence of the user's consent for the course must be checked.
+        # But there should be no effect on the number of mongo queries made.
+        (ModuleStoreEnum.Type.mongo, True, 1, 5, 3, 14, 2),
+        (ModuleStoreEnum.Type.mongo, True, 50, 5, 3, 14, 2),
+        # split mongo: 3 queries, regardless of thread response size.
+        (ModuleStoreEnum.Type.split, True, 1, 3, 3, 13, 2),
+        (ModuleStoreEnum.Type.split, True, 50, 3, 3, 13, 2),
     )
     @ddt.unpack
     def test_number_of_mongo_queries(
             self,
             default_store,
+            enterprise_enabled,
             num_thread_responses,
             num_uncached_mongo_calls,
             num_cached_mongo_calls,
@@ -393,12 +404,13 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase):
             """
             Call single_thread and assert that it returns what we expect.
             """
-            response = views.single_thread(
-                request,
-                course.id.to_deprecated_string(),
-                "dummy_discussion_id",
-                test_thread_id
-            )
+            with override_settings(ENABLE_ENTERPRISE_INTEGRATION=enterprise_enabled):
+                response = views.single_thread(
+                    request,
+                    course.id.to_deprecated_string(),
+                    "dummy_discussion_id",
+                    test_thread_id
+                )
             self.assertEquals(response.status_code, 200)
             self.assertEquals(len(json.loads(response.content)["content"]["children"]), num_thread_responses)
 
@@ -1566,3 +1578,47 @@ class EnrollmentTestCase(ForumsEnableMixin, ModuleStoreTestCase):
         request.user = self.student
         with self.assertRaises(UserNotEnrolled):
             views.forum_form_discussion(request, course_id=self.course.id.to_deprecated_string())
+
+
+@patch('requests.request', autospec=True)
+class EnterpriseConsentTestCase(EnterpriseTestConsentRequired, ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase):
+    """
+    Ensure that the Enterprise Data Consent redirects are in place only when consent is required.
+    """
+    CREATE_USER = False
+
+    @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
+    def setUp(self):
+        # Invoke UrlResetMixin setUp
+        super(EnterpriseConsentTestCase, self).setUp()
+
+        username = "foo"
+        password = "bar"
+
+        self.discussion_id = 'dummy_discussion_id'
+        self.course = CourseFactory.create(discussion_topics={'dummy discussion': {'id': self.discussion_id}})
+        self.student = UserFactory.create(username=username, password=password)
+        CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
+        self.assertTrue(
+            self.client.login(username=username, password=password)
+        )
+
+        self.addCleanup(translation.deactivate)
+
+    def test_consent_required(self, mock_request):
+        """
+        Test that enterprise data sharing consent is required when enabled for the various discussion views.
+        """
+        thread_id = 'dummy'
+        course_id = unicode(self.course.id)
+        mock_request.side_effect = make_mock_request_impl(course=self.course, text='dummy', thread_id=thread_id)
+
+        for url in (
+                reverse('discussion.views.forum_form_discussion',
+                        kwargs=dict(course_id=course_id)),
+                reverse('discussion.views.inline_discussion',
+                        kwargs=dict(course_id=course_id, discussion_id=self.discussion_id)),
+                reverse('discussion.views.single_thread',
+                        kwargs=dict(course_id=course_id, discussion_id=self.discussion_id, thread_id=thread_id)),
+        ):
+            self.verify_consent_required(self.client, url)

lms/djangoapps/discussion/views.py

@@ -47,6 +47,7 @@ from django_comment_client.utils import (
 )
 import django_comment_client.utils as utils
 import lms.lib.comment_client as cc
+from util.enterprise_helpers import data_sharing_consent_required
 
 from opaque_keys.edx.keys import CourseKey
 
@@ -179,6 +180,7 @@ def use_bulk_ops(view_func):
 
 
 @login_required
+@data_sharing_consent_required
 @use_bulk_ops
 def inline_discussion(request, course_key, discussion_id):
     """
@@ -214,6 +216,7 @@ def inline_discussion(request, course_key, discussion_id):
 
 
 @login_required
+@data_sharing_consent_required
 @use_bulk_ops
 def forum_form_discussion(request, course_key):
     """
@@ -256,6 +259,7 @@ def forum_form_discussion(request, course_key):
 
 @require_GET
 @login_required
+@data_sharing_consent_required
 @use_bulk_ops
 def single_thread(request, course_key, discussion_id, thread_id):
     """