diff --git a/analytics_data_api/insights_snowflake/response_headers.py b/analytics_data_api/insights_snowflake/response_headers.py new file mode 100644 index 00000000..c4317f6a --- /dev/null +++ b/analytics_data_api/insights_snowflake/response_headers.py @@ -0,0 +1,28 @@ +"""Response header helpers for Snowflake-backed Insights endpoints.""" + +DATA_SOURCE_HEADER = 'X-Insights-Data-Source' +DATA_SOURCE_AURORA = 'aurora' +DATA_SOURCE_SNOWFLAKE = 'snowflake' + + +class InsightsDataSourceResponseMixin: + """Add the Insights data source response header when a view sets one.""" + + data_source_header = DATA_SOURCE_HEADER + data_source_aurora = DATA_SOURCE_AURORA + data_source_snowflake = DATA_SOURCE_SNOWFLAKE + insights_data_source = None + + def set_insights_data_source_aurora(self): + """Mark the current response as using Aurora.""" + self.insights_data_source = self.data_source_aurora + + def set_insights_data_source_snowflake(self): + """Mark the current response as using Snowflake.""" + self.insights_data_source = self.data_source_snowflake + + def finalize_response(self, request, response, *args, **kwargs): + response = super().finalize_response(request, response, *args, **kwargs) + if self.insights_data_source: + response[self.data_source_header] = self.insights_data_source + return response diff --git a/analytics_data_api/insights_snowflake/toggles.py b/analytics_data_api/insights_snowflake/toggles.py index 40fbeb2a..39f7dc14 100644 --- a/analytics_data_api/insights_snowflake/toggles.py +++ b/analytics_data_api/insights_snowflake/toggles.py @@ -2,9 +2,18 @@ from waffle import flag_is_active +INSIGHTS_SNOWFLAKE_FLAG = 'insights_snowflake_enabled' COURSE_ACTIVITY_SNOWFLAKE_FLAG = 'insights_snowflake_course_activity' +def is_insights_snowflake_enabled(request): + """Return whether Snowflake-backed Insights endpoints are enabled globally.""" + return flag_is_active(request, INSIGHTS_SNOWFLAKE_FLAG) + + def is_course_activity_snowflake_enabled(request): """Return whether course activity should be read from Snowflake.""" - return flag_is_active(request, COURSE_ACTIVITY_SNOWFLAKE_FLAG) + return ( + is_insights_snowflake_enabled(request) or + flag_is_active(request, COURSE_ACTIVITY_SNOWFLAKE_FLAG) + ) diff --git a/analytics_data_api/tests/test_insights_snowflake.py b/analytics_data_api/tests/test_insights_snowflake.py index e9814b1b..dd956d20 100644 --- a/analytics_data_api/tests/test_insights_snowflake.py +++ b/analytics_data_api/tests/test_insights_snowflake.py @@ -4,14 +4,22 @@ from unittest.mock import Mock, patch from django.test import SimpleTestCase, override_settings +from rest_framework.response import Response from analytics_data_api.insights_snowflake.client import fetch_all, get_qualified_table_name from analytics_data_api.insights_snowflake.mappers.activity import map_course_activity_weekly_rows from analytics_data_api.insights_snowflake.queries.activity import get_course_activity_weekly_rows +from analytics_data_api.insights_snowflake.response_headers import ( + DATA_SOURCE_HEADER, + DATA_SOURCE_SNOWFLAKE, + InsightsDataSourceResponseMixin, +) from analytics_data_api.insights_snowflake.service import get_course_activity_weekly from analytics_data_api.insights_snowflake.toggles import ( COURSE_ACTIVITY_SNOWFLAKE_FLAG, + INSIGHTS_SNOWFLAKE_FLAG, is_course_activity_snowflake_enabled, + is_insights_snowflake_enabled, ) from analytics_data_api.snowflake_client import SnowflakeConfigurationError @@ -192,14 +200,75 @@ def test_get_course_activity_weekly_calls_query_and_mapper(self, mock_get_rows, mock_map_rows.assert_called_once_with(raw_rows) +class BaseTestView: + """Small base class for testing response mixin behavior.""" + + def finalize_response(self, _request, response, *_args, **_kwargs): + return response + + +class InsightsDataSourceTestView(InsightsDataSourceResponseMixin, BaseTestView): + """Test view using the Snowflake data source response mixin.""" + + +class InsightsSnowflakeResponseHeaderTests(SimpleTestCase): + """Cover shared response header helpers.""" + + def test_finalize_response_adds_data_source_header(self): + view = InsightsDataSourceTestView() + view.set_insights_data_source_snowflake() + + response = view.finalize_response(Mock(), Response({})) + + self.assertEqual(response[DATA_SOURCE_HEADER], DATA_SOURCE_SNOWFLAKE) + + def test_finalize_response_skips_header_when_source_is_not_set(self): + view = InsightsDataSourceTestView() + + response = view.finalize_response(Mock(), Response({})) + + self.assertNotIn(DATA_SOURCE_HEADER, response) + + class InsightsSnowflakeToggleTests(SimpleTestCase): """Cover endpoint Waffle flag wrapper.""" @patch('analytics_data_api.insights_snowflake.toggles.flag_is_active') - def test_is_course_activity_snowflake_enabled_uses_endpoint_flag(self, mock_flag_is_active): + def test_is_insights_snowflake_enabled_uses_global_flag(self, mock_flag_is_active): request = Mock() mock_flag_is_active.return_value = True + self.assertTrue(is_insights_snowflake_enabled(request)) + + mock_flag_is_active.assert_called_once_with(request, INSIGHTS_SNOWFLAKE_FLAG) + + @patch('analytics_data_api.insights_snowflake.toggles.flag_is_active') + def test_is_course_activity_snowflake_enabled_uses_global_flag(self, mock_flag_is_active): + request = Mock() + mock_flag_is_active.return_value = True + + self.assertTrue(is_course_activity_snowflake_enabled(request)) + + mock_flag_is_active.assert_called_once_with(request, INSIGHTS_SNOWFLAKE_FLAG) + + @patch('analytics_data_api.insights_snowflake.toggles.flag_is_active') + def test_is_course_activity_snowflake_enabled_uses_endpoint_flag(self, mock_flag_is_active): + request = Mock() + mock_flag_is_active.side_effect = [False, True] + self.assertTrue(is_course_activity_snowflake_enabled(request)) - mock_flag_is_active.assert_called_once_with(request, COURSE_ACTIVITY_SNOWFLAKE_FLAG) + self.assertEqual(mock_flag_is_active.call_count, 2) + self.assertEqual(mock_flag_is_active.call_args_list[0].args, (request, INSIGHTS_SNOWFLAKE_FLAG)) + self.assertEqual(mock_flag_is_active.call_args_list[1].args, (request, COURSE_ACTIVITY_SNOWFLAKE_FLAG)) + + @patch('analytics_data_api.insights_snowflake.toggles.flag_is_active') + def test_is_course_activity_snowflake_enabled_returns_false_when_flags_disabled(self, mock_flag_is_active): + request = Mock() + mock_flag_is_active.return_value = False + + self.assertFalse(is_course_activity_snowflake_enabled(request)) + + self.assertEqual(mock_flag_is_active.call_count, 2) + self.assertEqual(mock_flag_is_active.call_args_list[0].args, (request, INSIGHTS_SNOWFLAKE_FLAG)) + self.assertEqual(mock_flag_is_active.call_args_list[1].args, (request, COURSE_ACTIVITY_SNOWFLAKE_FLAG)) diff --git a/analytics_data_api/v0/views/courses.py b/analytics_data_api/v0/views/courses.py index 880f9eab..ba1e254e 100644 --- a/analytics_data_api/v0/views/courses.py +++ b/analytics_data_api/v0/views/courses.py @@ -14,6 +14,7 @@ from rest_framework.views import APIView from analytics_data_api.constants import enrollment_modes +from analytics_data_api.insights_snowflake.response_headers import InsightsDataSourceResponseMixin from analytics_data_api.insights_snowflake.service import get_course_activity_weekly from analytics_data_api.insights_snowflake.toggles import is_course_activity_snowflake_enabled from analytics_data_api.utils import dictfetchall, get_course_report_download_details @@ -71,7 +72,7 @@ def finalize_response(self, request, response, *args, **kwargs): # pylint: disable=line-too-long -class CourseActivityWeeklyView(BaseCourseView): +class CourseActivityWeeklyView(InsightsDataSourceResponseMixin, BaseCourseView): """ Get counts of users who performed specific activities in a course. @@ -120,10 +121,6 @@ class CourseActivityWeeklyView(BaseCourseView): slug = 'engagement-activity' model = models.CourseActivityWeekly serializer_class = serializers.CourseActivityWeeklySerializer - data_source_header = 'X-Insights-Data-Source' - data_source_aurora = 'aurora' - data_source_snowflake = 'snowflake' - insights_data_source = None def apply_date_filtering(self, queryset): if self.start_date or self.end_date: @@ -141,21 +138,15 @@ def apply_date_filtering(self, queryset): queryset = queryset.filter(interval_end=latest_date) return queryset - def finalize_response(self, request, response, *args, **kwargs): - response = super().finalize_response(request, response, *args, **kwargs) - if self.insights_data_source: - response[self.data_source_header] = self.insights_data_source - return response - def get_queryset(self): if is_course_activity_snowflake_enabled(self.request): - self.insights_data_source = self.data_source_snowflake + self.set_insights_data_source_snowflake() data = get_course_activity_weekly(self.course_id, self.start_date, self.end_date) if data: return data raise Http404 - self.insights_data_source = self.data_source_aurora + self.set_insights_data_source_aurora() queryset = super().get_queryset() queryset = self.format_data(queryset) return queryset