diff --git a/enterprise_data/api/v1/serializers.py b/enterprise_data/api/v1/serializers.py index 904b6d12..cd1be3f1 100644 --- a/enterprise_data/api/v1/serializers.py +++ b/enterprise_data/api/v1/serializers.py @@ -29,6 +29,7 @@ class EnterpriseLearnerEnrollmentSerializer(serializers.ModelSerializer): total_learning_time_hours = serializers.SerializerMethodField() enterprise_flex_group_name = serializers.SerializerMethodField() enterprise_flex_group_uuid = serializers.SerializerMethodField() + course_passing_grade = serializers.SerializerMethodField() course_progress = serializers.SerializerMethodField() class Meta: @@ -51,6 +52,7 @@ class Meta: 'enterprise_customer_uuid', 'enterprise_sso_uid', 'created', 'course_api_url', 'total_learning_time_hours', 'is_subsidy', 'course_product_line', 'budget_id', 'enterprise_flex_group_name', 'enterprise_flex_group_uuid', + 'course_passing_grade', 'course_progress', ) @@ -72,6 +74,10 @@ def get_course_progress(self, obj): """Returns learner course progress from selected report data.""" return getattr(obj, 'course_progress', None) + def get_course_passing_grade(self, obj): + """Returns required passing grade for the course, if available.""" + return getattr(obj, 'course_passing_grade', None) + @cache_it() def _get_flex_groups(self, obj): """ diff --git a/enterprise_data/api/v1/views/enterprise_learner.py b/enterprise_data/api/v1/views/enterprise_learner.py index 2b3a002f..2d2473e8 100644 --- a/enterprise_data/api/v1/views/enterprise_learner.py +++ b/enterprise_data/api/v1/views/enterprise_learner.py @@ -6,6 +6,11 @@ from logging import getLogger from uuid import UUID +try: + from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +except ImportError: + CourseOverview = None + from rest_framework import filters, viewsets from rest_framework.decorators import action from rest_framework.exceptions import NotFound @@ -14,12 +19,14 @@ from django.conf import settings from django.core.paginator import Paginator +from django.db import connection from django.db.models import Count, Exists, Max, OuterRef, Prefetch, Q, Subquery, Value from django.db.models.fields import IntegerField from django.db.models.functions import Coalesce from django.http import StreamingHttpResponse from django.utils import timezone +from enterprise_data import cache from enterprise_data.api.v1 import serializers from enterprise_data.clients import EnterpriseApiClient from enterprise_data.exceptions import EnterpriseApiClientException @@ -35,6 +42,7 @@ DEFAULT_LEARNER_CACHE_TIMEOUT = 60 * 10 +DEFAULT_COURSE_PASSING_GRADE_CACHE_TIMEOUT = 60 * 60 * 6 class EnterpriseLearnerEnrollmentViewSet(EnterpriseViewSetMixin, viewsets.ReadOnlyModelViewSet): @@ -70,6 +78,7 @@ class EnterpriseLearnerEnrollmentViewSet(EnterpriseViewSetMixin, viewsets.ReadOn 'enterprise_customer_uuid', 'enterprise_sso_uid', 'created', 'course_api_url', 'total_learning_time_hours', 'is_subsidy', 'course_product_line', 'budget_id', 'enterprise_flex_group_name', 'enterprise_flex_group_uuid', + 'course_passing_grade', 'course_progress', ] @@ -96,15 +105,27 @@ def get_queryset(self): # TODO: Created a ticket ENT0-9531 to add the cache on this viewset - # Add a synthetic placeholder column so the serialized response shape - # always includes `course_progress`; real values are merged in later. - enrollments = EnterpriseLearnerEnrollment.objects.filter( - enterprise_customer_uuid=enterprise_customer_uuid - ).extra(select={'course_progress': 'NULL'}) + # Add synthetic placeholder columns so the serialized response shape + # always includes `course_progress` and `course_passing_grade`. + enrollments = self._get_enrollments_with_course_metadata(enterprise_customer_uuid) + enrollments = self.apply_filters(enrollments) return enrollments + def _get_enrollments_with_course_metadata(self, enterprise_customer_uuid): + """ + Build the enrollments queryset and include synthetic metadata fields. + + Adds placeholders for fields enriched after serialization. Keeping the + base queryset ORM-only avoids a per-request CourseOverview join and + lets the enrichment path use a reusable cache. + """ + enrollments = EnterpriseLearnerEnrollment.objects.filter( + enterprise_customer_uuid=enterprise_customer_uuid + ).extra(select={'course_progress': 'NULL'}) + return enrollments.extra(select={'course_passing_grade': 'NULL'}) + def list(self, request, *args, **kwargs): """ Override the list method to handle streaming CSV download and enrich @@ -123,13 +144,13 @@ def list(self, request, *args, **kwargs): def _enrich_course_progress_rows(self, rows): """ - Enrich serialized enrollment rows with ``course_progress`` fetched from - Snowflake's internal LPR table. + Enrich serialized enrollment rows with cached course metadata. Accepts a list-like collection of serialized row dicts and mutates each matching row in place. Silently skips enrichment on any error so the ORM-backed response is always returned intact. """ + self._enrich_course_passing_grade_rows(rows) try: if not rows: return rows @@ -137,7 +158,7 @@ def _enrich_course_progress_rows(self, rows): progress_map = SnowflakeCourseProgressSource().get_course_progress_map(enterprise_uuid, rows) for row in rows: key = ( - row.get('user_email', '').strip(), + row.get('user_email', '').strip().lower(), row.get('courserun_key', '').strip(), ) if key in progress_map: @@ -147,6 +168,77 @@ def _enrich_course_progress_rows(self, rows): LOGGER.warning('Could not enrich course_progress from Snowflake', exc_info=True) return rows + @staticmethod + def _course_passing_grade_cache_timeout(): + """Return the configurable TTL for CourseOverview passing-grade cache entries.""" + return getattr( + settings, + 'LPR_COURSE_PASSING_GRADE_CACHE_TIMEOUT', + DEFAULT_COURSE_PASSING_GRADE_CACHE_TIMEOUT, + ) + + @staticmethod + def _course_passing_grade_cache_key(courserun_key): + """Return the cache key for a CourseOverview passing grade.""" + return cache.get_key('lpr_course_passing_grade', courserun_key) + + @staticmethod + def _course_overview_table_exists(): + """Return whether CourseOverview can be queried in this runtime.""" + if CourseOverview is None: + return False + return CourseOverview._meta.db_table in connection.introspection.table_names() + + def _enrich_course_passing_grade_rows(self, rows): + """ + Enrich serialized enrollment rows with cached CourseOverview passing grades. + """ + try: + if not rows or not self._course_overview_table_exists(): + return rows + + courserun_keys = sorted({ + row.get('courserun_key', '').strip() + for row in rows + if row.get('courserun_key') + }) + if not courserun_keys: + return rows + + passing_grade_map = {} + missing_courserun_keys = [] + for courserun_key in courserun_keys: + cached_response = cache.get(self._course_passing_grade_cache_key(courserun_key)) + if cached_response.is_found: + passing_grade_map[courserun_key] = cached_response.value + else: + missing_courserun_keys.append(courserun_key) + + if missing_courserun_keys: + fetched_grades = dict( + CourseOverview.objects.filter(id__in=missing_courserun_keys).values_list( + 'id', + 'lowest_passing_grade', + ) + ) + for courserun_key in missing_courserun_keys: + passing_grade = fetched_grades.get(courserun_key) + passing_grade_map[courserun_key] = passing_grade + cache.set( + self._course_passing_grade_cache_key(courserun_key), + passing_grade, + timeout=self._course_passing_grade_cache_timeout(), + ) + + for row in rows: + courserun_key = row.get('courserun_key', '').strip() + if courserun_key in passing_grade_map: + row['course_passing_grade'] = passing_grade_map[courserun_key] + return rows + except Exception: # pylint: disable=broad-exception-caught + LOGGER.warning('Could not enrich course_passing_grade from CourseOverview', exc_info=True) + return rows + def _enrich_course_progress(self, response): """ Enrich each row in the paginated response with ``course_progress`` fetched diff --git a/enterprise_data/api/v1/views/lpr_data_source_base.py b/enterprise_data/api/v1/views/lpr_data_source_base.py index 80ab52d4..74afa86d 100644 --- a/enterprise_data/api/v1/views/lpr_data_source_base.py +++ b/enterprise_data/api/v1/views/lpr_data_source_base.py @@ -31,6 +31,7 @@ class LPRSerializerShapeMixin: 'enterprise_customer_uuid', 'enterprise_sso_uid', 'created', 'course_api_url', 'total_learning_time_hours', 'is_subsidy', 'course_product_line', 'budget_id', 'enterprise_flex_group_name', 'enterprise_flex_group_uuid', + 'course_passing_grade', 'course_progress', ) diff --git a/enterprise_data/api/v1/views/lpr_data_source_snowflake.py b/enterprise_data/api/v1/views/lpr_data_source_snowflake.py index c6c16ba9..dd16ba01 100644 --- a/enterprise_data/api/v1/views/lpr_data_source_snowflake.py +++ b/enterprise_data/api/v1/views/lpr_data_source_snowflake.py @@ -19,6 +19,7 @@ LPR_SNOWFLAKE_INTERNAL_TABLE = 'LEARNER_PROGRESS_REPORT_INTERNAL' LPR_SNOWFLAKE_WAREHOUSE = None (omitted from connection if not set) LPR_SNOWFLAKE_ROLE = None (omitted from connection if not set) + LPR_COURSE_PROGRESS_CACHE_TIMEOUT = 300 (5 minutes) """ from logging import getLogger @@ -26,8 +27,12 @@ from django.conf import settings +from enterprise_data import cache + LOGGER = getLogger(__name__) +DEFAULT_COURSE_PROGRESS_CACHE_TIMEOUT = 60 * 5 + try: import snowflake.connector as snowflake_connector except ImportError: # pragma: no cover - depends on runtime extras @@ -98,6 +103,33 @@ def _internal_table(): table = getattr(settings, 'LPR_SNOWFLAKE_INTERNAL_TABLE', 'LEARNER_PROGRESS_REPORT_INTERNAL') return f'{database}.{schema}.{table}' + @staticmethod + def _normalized_enterprise_uuid(enterprise_customer_uuid): + """Normalize enterprise UUIDs to the Snowflake comparison format.""" + return str(enterprise_customer_uuid).replace('-', '').lower() + + @staticmethod + def _normalized_row_key(user_email, courserun_key): + """Return a stable in-memory key for matching ORM rows to Snowflake rows.""" + return (str(user_email).strip().lower(), str(courserun_key).strip()) + + def _cache_key(self, enterprise_customer_uuid): + """Return the enterprise-scoped cache key for Snowflake course progress.""" + return cache.get_key( + 'lpr_course_progress', + self._internal_table(), + self._normalized_enterprise_uuid(enterprise_customer_uuid), + ) + + @staticmethod + def _cache_timeout(): + """Return the configurable TTL for Snowflake course progress cache entries.""" + return getattr( + settings, + 'LPR_COURSE_PROGRESS_CACHE_TIMEOUT', + DEFAULT_COURSE_PROGRESS_CACHE_TIMEOUT, + ) + # ------------------------------------------------------------------ # Public interface # ------------------------------------------------------------------ @@ -107,6 +139,11 @@ def get_course_progress_map(self, enterprise_customer_uuid, enrollments): Return a ``{(user_email, courserun_key): course_progress}`` mapping for all rows on the current page. + The Snowflake data refreshes roughly daily, so this method keeps an + enterprise-scoped cache of the full Snowflake result set and returns + only the rows requested by the current page. This avoids repeated + Snowflake queries while keeping the public API unchanged. + Args: enterprise_customer_uuid (str): The enterprise UUID to scope the query. enrollments (list[dict]): Serialized enrollment rows from the ORM page. @@ -117,31 +154,59 @@ def get_course_progress_map(self, enterprise_customer_uuid, enrollments): try/except so Snowflake unavailability never breaks the LPR response. """ pairs = [ - (row.get('user_email', ''), row.get('courserun_key', '')) + self._normalized_row_key(row.get('user_email', ''), row.get('courserun_key', '')) for row in enrollments if row.get('user_email') and row.get('courserun_key') ] if not pairs: return {} - table = self._internal_table() - normalized_uuid = str(enterprise_customer_uuid).replace('-', '').lower() + enterprise_progress_map = self._get_enterprise_course_progress_map(enterprise_customer_uuid) + return { + pair: enterprise_progress_map[pair] + for pair in pairs + if pair in enterprise_progress_map + } + + def _get_enterprise_course_progress_map(self, enterprise_customer_uuid): + """ + Return the full enterprise course progress map from cache or Snowflake. + """ + cache_key = self._cache_key(enterprise_customer_uuid) + cached_response = cache.get(cache_key) + if cached_response.is_found: + LOGGER.info( + '[course_progress] Cache hit for enterprise_uuid=%s', + enterprise_customer_uuid, + ) + return cached_response.value - # Build parameterised IN list for (user_email, courserun_key) pairs. - placeholders = ', '.join(['(%s, %s)'] * len(pairs)) - flat_params = [param for pair in pairs for param in pair] + LOGGER.info( + '[course_progress] Cache miss for enterprise_uuid=%s; fetching from Snowflake', + enterprise_customer_uuid, + ) + progress_map = self._fetch_enterprise_course_progress_map(enterprise_customer_uuid) + cache.set(cache_key, progress_map, timeout=self._cache_timeout()) + return progress_map + + def _fetch_enterprise_course_progress_map(self, enterprise_customer_uuid): + """ + Fetch all course progress rows for an enterprise from Snowflake. + """ + + table = self._internal_table() + normalized_uuid = self._normalized_enterprise_uuid(enterprise_customer_uuid) sql = ( f"SELECT USER_EMAIL, COURSERUN_KEY, COURSE_PROGRESS " f"FROM {table} " - f"WHERE LOWER(REPLACE(TO_VARCHAR(ENTERPRISE_CUSTOMER_UUID), '-', '')) = %s " - f" AND (USER_EMAIL, COURSERUN_KEY) IN ({placeholders})" + f"WHERE LOWER(REPLACE(TO_VARCHAR(ENTERPRISE_CUSTOMER_UUID), '-', '')) = %s" ) ctx = self._get_connection() cs = ctx.cursor() try: - cs.execute(sql, [normalized_uuid] + flat_params) + cs.execute(sql, [normalized_uuid]) rows = cs.fetchall() if not rows: LOGGER.warning( @@ -150,7 +215,7 @@ def get_course_progress_map(self, enterprise_customer_uuid, enrollments): enterprise_customer_uuid, ) return { - (row[0], row[1]): row[2] + self._normalized_row_key(row[0], row[1]): row[2] for row in rows } finally: diff --git a/enterprise_data/renderers.py b/enterprise_data/renderers.py index 231f5dd4..b38c7a5e 100644 --- a/enterprise_data/renderers.py +++ b/enterprise_data/renderers.py @@ -28,6 +28,7 @@ class EnrollmentsCSVRenderer(CSVStreamingRenderer): 'user_country_code', 'user_username', 'user_first_name', 'user_last_name', 'enterprise_name', 'enterprise_customer_uuid', 'enterprise_sso_uid', 'created', 'course_api_url', 'total_learning_time_hours', 'is_subsidy', 'course_product_line', 'budget_id', 'enterprise_flex_group_name', 'enterprise_flex_group_uuid', + 'course_passing_grade', 'course_progress', ] diff --git a/enterprise_data/tests/api/v1/test_serializers.py b/enterprise_data/tests/api/v1/test_serializers.py index b66dd8de..da39285f 100644 --- a/enterprise_data/tests/api/v1/test_serializers.py +++ b/enterprise_data/tests/api/v1/test_serializers.py @@ -48,6 +48,15 @@ def test_course_progress_field_present(self): serializer = EnterpriseLearnerEnrollmentSerializer(self.enrollment) assert serializer.data['course_progress'] == 0.42 + def test_course_passing_grade_field_present(self): + self.enrollment.course_passing_grade = 0.7 + serializer = EnterpriseLearnerEnrollmentSerializer(self.enrollment) + assert serializer.data['course_passing_grade'] == 0.7 + + def test_course_passing_grade_field_defaults_to_none(self): + serializer = EnterpriseLearnerEnrollmentSerializer(self.enrollment) + assert serializer.data['course_passing_grade'] is None + def test_csv_renderer_header_matches_serializer_field_order(self): """CSV header must exactly match serializer field order.""" serializer_fields = list(EnterpriseLearnerEnrollmentSerializer.Meta.fields) diff --git a/enterprise_data/tests/api/v1/test_views.py b/enterprise_data/tests/api/v1/test_views.py index 78d627f1..f6109377 100644 --- a/enterprise_data/tests/api/v1/test_views.py +++ b/enterprise_data/tests/api/v1/test_views.py @@ -3,7 +3,10 @@ """ import datetime +import importlib import os +import sys +import types from unittest import mock from uuid import UUID, uuid4 @@ -16,6 +19,7 @@ from django.utils import timezone from enterprise_data.api.v1.serializers import EnterpriseOfferSerializer +from enterprise_data.api.v1.views.enterprise_learner import EnterpriseLearnerEnrollmentViewSet from enterprise_data.models import EnterpriseLearnerEnrollment, EnterpriseOffer from enterprise_data.tests.factories import ( EnterpriseAdminLearnerProgressFactory, @@ -33,6 +37,47 @@ from enterprise_data_roles.models import EnterpriseDataFeatureRole, EnterpriseDataRoleAssignment +def _ensure_course_overview_import_for_tests(): + """ + Provide a minimal ``openedx...course_overviews.models`` shim for local + unit tests when full edx-platform Python dependencies are unavailable. + """ + try: + course_overview_models = importlib.import_module( + 'openedx.core.djangoapps.content.course_overviews.models' + ) + if getattr(course_overview_models, 'CourseOverview', None) is not None: + return + except ImportError: + pass + + module_names = [ + 'openedx', + 'openedx.core', + 'openedx.core.djangoapps', + 'openedx.core.djangoapps.content', + 'openedx.core.djangoapps.content.course_overviews', + ] + for module_name in module_names: + if module_name not in sys.modules: + sys.modules[module_name] = types.ModuleType(module_name) + + models_module_name = 'openedx.core.djangoapps.content.course_overviews.models' + models_module = types.ModuleType(models_module_name) + + class _Meta: + db_table = 'course_overviews_courseoverview' + + class DummyCourseOverview: + _meta = _Meta() + + models_module.CourseOverview = DummyCourseOverview + sys.modules[models_module_name] = models_module + + +_ensure_course_overview_import_for_tests() + + @ddt.ddt @mark.django_db class TestEnterpriseLearnerEnrollmentViewSet(JWTTestMixin, APITransactionTestCase): @@ -292,6 +337,97 @@ def test_stream_serialized_data_enriches_course_progress_from_snowflake(self, mo self.assertIn('course_progress', content) self.assertIn('0.87', content) + def test_course_passing_grade_field_in_response(self): + """Test that course_passing_grade field is included in the API response""" + enterprise_learner = EnterpriseLearnerFactory( + enterprise_customer_uuid=self.enterprise_id, + user_email='student@example.com', + ) + EnterpriseLearnerEnrollmentFactory( + enterprise_customer_uuid=self.enterprise_id, + is_consent_granted=True, + enterprise_user_id=enterprise_learner.enterprise_user_id, + user_email='student@example.com', + courserun_key='course-v1:edX+Demo+2024', + ) + + url = reverse('v1:enterprise-learner-enrollment-list', kwargs={'enterprise_id': self.enterprise_id}) + response = self.client.get(url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + results = response.json()['results'] + self.assertEqual(len(results), 1) + # Verify course_passing_grade field is present in the response + self.assertIn('course_passing_grade', results[0]) + # The field will be null since CourseOverview is mocked in tests + self.assertIsNone(results[0]['course_passing_grade']) + + @mock.patch('enterprise_data.api.v1.views.enterprise_learner.CourseOverview', None) + @mock.patch('enterprise_data.api.v1.views.enterprise_learner.EnterpriseLearnerEnrollment.objects.filter') + def test_get_enrollments_with_course_metadata_without_course_overview(self, mock_filter): + viewset = EnterpriseLearnerEnrollmentViewSet() + enrollments = mock.Mock() + with_progress = mock.Mock() + with_passing_grade = mock.Mock() + + mock_filter.return_value = enrollments + enrollments.extra.return_value = with_progress + with_progress.extra.return_value = with_passing_grade + + result = viewset._get_enrollments_with_course_metadata(self.enterprise_id) # pylint: disable=protected-access + + mock_filter.assert_called_once_with(enterprise_customer_uuid=self.enterprise_id) + enrollments.extra.assert_called_once_with(select={'course_progress': 'NULL'}) + with_progress.extra.assert_called_once_with(select={'course_passing_grade': 'NULL'}) + self.assertEqual(result, with_passing_grade) + + @mock.patch('enterprise_data.api.v1.views.enterprise_learner.cache') + @mock.patch('enterprise_data.api.v1.views.enterprise_learner.CourseOverview') + @mock.patch('enterprise_data.api.v1.views.enterprise_learner.connection.introspection.table_names') + def test_enrich_course_passing_grade_uses_cache( + self, + mock_table_names, + mock_course_overview, + mock_cache, + ): + viewset = EnterpriseLearnerEnrollmentViewSet() + mock_course_overview._meta.db_table = 'course_overviews_courseoverview' + mock_table_names.return_value = ['course_overviews_courseoverview'] + mock_cache.get.return_value = mock.Mock(is_found=True, value=0.7) + rows = [{'courserun_key': 'course-v1:edX+Demo+2024', 'course_passing_grade': None}] + + result = viewset._enrich_course_passing_grade_rows(rows) # pylint: disable=protected-access + + self.assertEqual(result[0]['course_passing_grade'], 0.7) + mock_table_names.assert_called_once_with() + mock_course_overview.objects.filter.assert_not_called() + mock_cache.set.assert_not_called() + + @mock.patch('enterprise_data.api.v1.views.enterprise_learner.cache') + @mock.patch('enterprise_data.api.v1.views.enterprise_learner.connection.introspection.table_names') + @mock.patch('enterprise_data.api.v1.views.enterprise_learner.CourseOverview') + def test_enrich_course_passing_grade_fetches_and_caches_missing_grade( + self, + mock_course_overview, + mock_table_names, + mock_cache, + ): + viewset = EnterpriseLearnerEnrollmentViewSet() + mock_course_overview._meta.db_table = 'course_overviews_courseoverview' + mock_table_names.return_value = ['course_overviews_courseoverview'] + mock_cache.get.return_value = mock.Mock(is_found=False) + mock_course_overview.objects.filter.return_value.values_list.return_value = [ + ('course-v1:edX+Demo+2024', 0.7), + ] + rows = [{'courserun_key': 'course-v1:edX+Demo+2024', 'course_passing_grade': None}] + + result = viewset._enrich_course_passing_grade_rows(rows) # pylint: disable=protected-access + + self.assertEqual(result[0]['course_passing_grade'], 0.7) + mock_table_names.assert_called_once_with() + mock_course_overview.objects.filter.assert_called_once_with(id__in=['course-v1:edX+Demo+2024']) + mock_cache.set.assert_called_once() + @ddt.ddt @mark.django_db diff --git a/enterprise_data/tests/lpr/test_lpr_data_source_snowflake.py b/enterprise_data/tests/lpr/test_lpr_data_source_snowflake.py index 1846f042..6df99282 100644 --- a/enterprise_data/tests/lpr/test_lpr_data_source_snowflake.py +++ b/enterprise_data/tests/lpr/test_lpr_data_source_snowflake.py @@ -5,7 +5,12 @@ import pytest -from enterprise_data.api.v1.views.lpr_data_source_snowflake import SnowflakeCourseProgressSource +from django.test import override_settings + +from enterprise_data.api.v1.views.lpr_data_source_snowflake import ( + DEFAULT_COURSE_PROGRESS_CACHE_TIMEOUT, + SnowflakeCourseProgressSource, +) ENTERPRISE_UUID = 'a1b2c3d4-e5f6-7890-abcd-ef1234567890' NORMALIZED_UUID = 'a1b2c3d4e5f67890abcdef1234567890' @@ -70,6 +75,28 @@ def test_connection_includes_optional_role_and_warehouse(self, mock_settings, mo assert kwargs['role'] == 'ANALYST' +class TestCacheConfiguration: + """Tests for enterprise-scoped course progress cache configuration.""" + + def test_default_cache_timeout_is_five_minutes(self): + assert DEFAULT_COURSE_PROGRESS_CACHE_TIMEOUT == 300 + assert _source()._cache_timeout() == 300 + + @override_settings(LPR_COURSE_PROGRESS_CACHE_TIMEOUT=120) + def test_cache_timeout_is_configurable(self): + assert _source()._cache_timeout() == 120 + + @patch('enterprise_data.api.v1.views.lpr_data_source_snowflake.cache.get_key', return_value='cache-key') + @patch.object(SnowflakeCourseProgressSource, '_internal_table', return_value=DEFAULT_TABLE) + def test_cache_key_scopes_by_table_and_normalized_enterprise(self, _table, mock_get_key): + assert _source()._cache_key(ENTERPRISE_UUID) == 'cache-key' + mock_get_key.assert_called_once_with( + 'lpr_course_progress', + DEFAULT_TABLE, + NORMALIZED_UUID, + ) + + class TestGetCourseProgressMap: """Tests for SQL execution, mapping, and cleanup behavior.""" @@ -77,10 +104,15 @@ def test_returns_empty_dict_when_no_pairs(self): assert _source().get_course_progress_map(ENTERPRISE_UUID, []) == {} assert _source().get_course_progress_map(ENTERPRISE_UUID, [{'user_email': '', 'courserun_key': ''}]) == {} + @patch('enterprise_data.api.v1.views.lpr_data_source_snowflake.cache') @patch.object(SnowflakeCourseProgressSource, '_get_connection') @patch.object(SnowflakeCourseProgressSource, '_internal_table', return_value=DEFAULT_TABLE) - def test_executes_expected_sql_and_params(self, _table, mock_conn_factory): - ctx, cursor = _mock_ctx_and_cursor(fetchall=[('alice@example.com', 'course-v1:Org+Course+Run', 0.8)]) + def test_executes_expected_sql_and_params(self, _table, mock_conn_factory, mock_cache): + mock_cache.get.return_value = MagicMock(is_found=False) + ctx, cursor = _mock_ctx_and_cursor(fetchall=[ + ('alice@example.com', 'course-v1:Org+Course+Run', 0.8), + ('carol@example.com', 'course-v1:Org+Other+Run', 0.4), + ]) mock_conn_factory.return_value = ctx enrollments = [ @@ -94,15 +126,32 @@ def test_executes_expected_sql_and_params(self, _table, mock_conn_factory): assert 'COURSE_PROGRESS' in sql assert 'USER_EMAIL, COURSERUN_KEY' in sql assert NORMALIZED_UUID == params[0] - assert params[1:] == [ - 'alice@example.com', 'course-v1:Org+Course+Run', - 'bob@example.com', 'course-v1:Org+Other+Run', - ] + assert len(params) == 1 + assert result == {('alice@example.com', 'course-v1:Org+Course+Run'): 0.8} + mock_cache.set.assert_called_once() + + @patch('enterprise_data.api.v1.views.lpr_data_source_snowflake.cache') + @patch.object(SnowflakeCourseProgressSource, '_get_connection') + def test_returns_results_from_enterprise_cache(self, mock_conn_factory, mock_cache): + mock_cache.get.return_value = MagicMock( + is_found=True, + value={('alice@example.com', 'course-v1:Org+Course+Run'): 0.8}, + ) + + result = _source().get_course_progress_map( + ENTERPRISE_UUID, + [{'user_email': 'Alice@Example.com', 'courserun_key': 'course-v1:Org+Course+Run'}], + ) + assert result == {('alice@example.com', 'course-v1:Org+Course+Run'): 0.8} + mock_conn_factory.assert_not_called() + mock_cache.set.assert_not_called() + @patch('enterprise_data.api.v1.views.lpr_data_source_snowflake.cache') @patch.object(SnowflakeCourseProgressSource, '_get_connection') @patch.object(SnowflakeCourseProgressSource, '_internal_table', return_value=DEFAULT_TABLE) - def test_cursor_and_connection_closed_on_success(self, _table, mock_conn_factory): + def test_cursor_and_connection_closed_on_success(self, _table, mock_conn_factory, mock_cache): + mock_cache.get.return_value = MagicMock(is_found=False) ctx, cursor = _mock_ctx_and_cursor() mock_conn_factory.return_value = ctx _source().get_course_progress_map( @@ -112,9 +161,11 @@ def test_cursor_and_connection_closed_on_success(self, _table, mock_conn_factory cursor.close.assert_called_once() ctx.close.assert_called_once() + @patch('enterprise_data.api.v1.views.lpr_data_source_snowflake.cache') @patch.object(SnowflakeCourseProgressSource, '_get_connection') @patch.object(SnowflakeCourseProgressSource, '_internal_table', return_value=DEFAULT_TABLE) - def test_cursor_and_connection_closed_on_execute_error(self, _table, mock_conn_factory): + def test_cursor_and_connection_closed_on_execute_error(self, _table, mock_conn_factory, mock_cache): + mock_cache.get.return_value = MagicMock(is_found=False) ctx, cursor = _mock_ctx_and_cursor() cursor.execute.side_effect = RuntimeError('Snowflake error') mock_conn_factory.return_value = ctx