Merge pull request #20164 from edx/hammad/WL-1905

WL-1905 |  Some learners not showing up in report | Missing Data Sharing Consent Redirection

lms/envs/common.py

@@ -3322,6 +3322,8 @@ ENTERPRISE_READONLY_ACCOUNT_FIELDS = [
 ENTERPRISE_CUSTOMER_COOKIE_NAME = 'enterprise_customer_uuid'
 BASE_COOKIE_DOMAIN = 'localhost'
 
+DATA_CONSENT_SHARE_CACHE_TIMEOUT = None  # Never expire
+
 ############## Settings for Course Enrollment Modes ######################
 # The min_price key refers to the minimum price allowed for an instance
 # of a particular type of course enrollment mode. This is not to be confused

openedx/features/enterprise_support/api.py

@@ -12,12 +12,13 @@ from django.shortcuts import redirect
 from django.template.loader import render_to_string
 from django.utils.http import urlencode
 from django.utils.translation import ugettext as _
+from edx_django_utils.cache import TieredCache
 from edx_rest_api_client.client import EdxRestApiClient
 from slumber.exceptions import HttpClientError, HttpNotFoundError, HttpServerError
 
 from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
-from openedx.features.enterprise_support.utils import get_cache_key
+from openedx.features.enterprise_support.utils import get_cache_key, get_data_consent_share_cache_key
 from third_party_auth.pipeline import get as get_partial_pipeline
 from third_party_auth.provider import Registry
 
@@ -450,9 +451,9 @@ def consent_needed_for_course(request, user, course_id, enrollment_exists=False)
     Wrap the enterprise app check to determine if the user needs to grant
     data sharing permissions before accessing a course.
     """
-    consent_key = ('data_sharing_consent_needed', course_id)
-
-    if request.session.get(consent_key) is False:
+    consent_cache_key = get_data_consent_share_cache_key(user.id, course_id)
+    data_sharing_consent_needed_cache = TieredCache.get_cached_response(consent_cache_key)
+    if data_sharing_consent_needed_cache.is_found and data_sharing_consent_needed_cache.value is 0:
         return False
 
     enterprise_learner_details = get_enterprise_learner_data(user)
@@ -470,9 +471,9 @@ def consent_needed_for_course(request, user, course_id, enrollment_exists=False)
             for learner in enterprise_learner_details
         )
     if not consent_needed:
-        # Set an ephemeral item in the user's session to prevent us from needing
+        # Set an ephemeral item in the cache to prevent us from needing
         # to make a Consent API request every time this function is called.
-        request.session[consent_key] = False
+        TieredCache.set_all_tiers(consent_cache_key, 0, settings.DATA_CONSENT_SHARE_CACHE_TIMEOUT)
 
     return consent_needed
 

openedx/features/enterprise_support/signals.py

@@ -6,9 +6,11 @@ from django.dispatch import receiver
 from django.db.models.signals import post_save
 from django.contrib.auth.models import User
 
-from enterprise.models import EnterpriseCustomerUser
+from enterprise.models import EnterpriseCustomerUser, EnterpriseCourseEnrollment
 from email_marketing.tasks import update_user
 
+from openedx.features.enterprise_support.utils import clear_data_consent_share_cache
+
 
 @receiver(post_save, sender=EnterpriseCustomerUser)
 def update_email_marketing_user_with_enterprise_vars(sender, instance, **kwargs):  # pylint: disable=unused-argument, invalid-name
@@ -25,3 +27,14 @@ def update_email_marketing_user_with_enterprise_vars(sender, instance, **kwargs)
         },
         email=user.email
     )
+
+
+@receiver(post_save, sender=EnterpriseCourseEnrollment)
+def update_data_consent_share_cache(sender, instance, **kwargs):  # pylint: disable=unused-argument
+    """
+        clears data_sharing_consent_needed cache after Enterprise Course Enrollment
+    """
+    clear_data_consent_share_cache(
+        instance.enterprise_customer_user.user_id,
+        instance.course_id
+    )

openedx/features/enterprise_support/tests/test_api.py

@@ -31,7 +31,7 @@ from openedx.features.enterprise_support.api import (
 from openedx.features.enterprise_support.tests import FEATURES_WITH_ENTERPRISE_ENABLED
 from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerUserFactory
 from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseServiceMockMixin
-from openedx.features.enterprise_support.utils import get_cache_key
+from openedx.features.enterprise_support.utils import get_cache_key, clear_data_consent_share_cache
 from student.tests.factories import UserFactory
 
 
@@ -168,15 +168,27 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
     def test_consent_needed_for_course(self):
         user = UserFactory(username='janedoe')
         request = mock.MagicMock(session={}, user=user)
+        ec_uuid = 'cf246b88-d5f6-4908-a522-fc307e0b0c59'
+        course_id = 'fake-course'
         self.mock_enterprise_learner_api()
-        self.mock_consent_missing(user.username, 'fake-course', 'cf246b88-d5f6-4908-a522-fc307e0b0c59')
-        self.assertTrue(consent_needed_for_course(request, user, 'fake-course'))
-        self.mock_consent_get(user.username, 'fake-course', 'cf246b88-d5f6-4908-a522-fc307e0b0c59')
-        self.assertFalse(consent_needed_for_course(request, user, 'fake-course'))
-        # Test that the result is cached when false (remove the HTTP mock so if the result
-        # isn't cached, we'll fail spectacularly.)
-        httpretty.reset()
-        self.assertFalse(consent_needed_for_course(request, user, 'fake-course'))
+
+        # test not required consent for example non enterprise customer
+        self.mock_consent_not_required(user.username, course_id, ec_uuid)
+        self.assertFalse(consent_needed_for_course(request, user, course_id))
+
+        # test required and missing consent for example now he becomes a enterprise customer
+        self.mock_consent_missing(user.username, course_id, ec_uuid)
+        # still result should be False as it has been stored in cache "Not to show consent", so it will confirm that
+        # cache is working fine
+        self.assertFalse(consent_needed_for_course(request, user, course_id))
+        # Removing cache
+        clear_data_consent_share_cache(user.id, course_id)
+        # Now test again
+        self.assertTrue(consent_needed_for_course(request, user, course_id))
+
+        # test after consent permission is granted
+        self.mock_consent_get(user.username, course_id, ec_uuid)
+        self.assertFalse(consent_needed_for_course(request, user, course_id))
 
     @httpretty.activate
     @mock.patch('enterprise.models.EnterpriseCustomer.catalog_contains_course')

openedx/features/enterprise_support/utils.py

@@ -8,6 +8,7 @@ from django.conf import settings
 from django.utils.translation import ugettext as _
 
 import third_party_auth
+from edx_django_utils.cache import TieredCache
 from third_party_auth import pipeline
 from enterprise.models import EnterpriseCustomerUser
 
@@ -40,6 +41,22 @@ def get_cache_key(**kwargs):
     return hashlib.md5(key).hexdigest()
 
 
+def get_data_consent_share_cache_key(user_id, course_id):
+    """
+        Returns cache key for data sharing consent needed against user_id and course_id
+    """
+
+    return get_cache_key(type='data_sharing_consent_needed', user_id=user_id, course_id=course_id)
+
+
+def clear_data_consent_share_cache(user_id, course_id):
+    """
+        clears data_sharing_consent_needed cache
+    """
+    consent_cache_key = get_data_consent_share_cache_key(user_id, course_id)
+    TieredCache.delete_all_tiers(consent_cache_key)
+
+
 def update_logistration_context_for_enterprise(request, context, enterprise_customer):
     """
     Take the processed context produced by the view, determine if it's relevant