From 62bf231355dfa2cdad56066a720e718cc18dc27b Mon Sep 17 00:00:00 2001 From: Marcos Date: Tue, 11 Feb 2025 09:35:40 -0300 Subject: [PATCH 1/5] chore: Removed unused chat enabled and chat history endpoints --- learning_assistant/urls.py | 12 -- learning_assistant/views.py | 116 ------------------- tests/test_views.py | 216 ------------------------------------ 3 files changed, 344 deletions(-) diff --git a/learning_assistant/urls.py b/learning_assistant/urls.py index 31914b8..11c8171 100644 --- a/learning_assistant/urls.py +++ b/learning_assistant/urls.py @@ -7,8 +7,6 @@ from learning_assistant.views import ( CourseChatView, LearningAssistantChatSummaryView, - LearningAssistantEnabledView, - LearningAssistantMessageHistoryView, ) app_name = 'learning_assistant' @@ -19,16 +17,6 @@ CourseChatView.as_view(), name='chat' ), - re_path( - fr'learning_assistant/v1/course_id/{COURSE_ID_PATTERN}/enabled', - LearningAssistantEnabledView.as_view(), - name='enabled', - ), - re_path( - fr'learning_assistant/v1/course_id/{COURSE_ID_PATTERN}/history', - LearningAssistantMessageHistoryView.as_view(), - name='message-history', - ), re_path( fr'learning_assistant/v1/course_id/{COURSE_ID_PATTERN}/chat-summary', LearningAssistantChatSummaryView.as_view(), diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 2cccfe9..976f06b 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -174,122 +174,6 @@ def post(self, request, course_run_id): ) -class LearningAssistantEnabledView(APIView): - """ - View to retrieve whether the Learning Assistant is enabled for a course. - - This endpoint returns a boolean representing whether the Learning Assistant feature is enabled in a course - represented by the course_run_id, which is provided in the URL. - - Accepts: [GET] - - Path: /learning_assistant/v1/course_id/{course_run_id}/enabled - - Parameters: - * course_run_id: the ID of the course - - Responses: - * 200: OK - * 400: Malformed Request - Course ID is not a valid course ID. - """ - - authentication_classes = (SessionAuthentication, JwtAuthentication,) - permission_classes = (IsAuthenticated,) - - def get(self, request, course_run_id): - """ - Given a course run ID, retrieve whether the Learning Assistant is enabled for the corresponding course. - - The response will be in the following format. - - {'enabled': } - """ - try: - courserun_key = CourseKey.from_string(course_run_id) - except InvalidKeyError: - return Response( - status=http_status.HTTP_400_BAD_REQUEST, - data={'detail': 'Course ID is not a valid course ID.'} - ) - - data = { - 'enabled': learning_assistant_enabled(courserun_key), - } - - return Response(status=http_status.HTTP_200_OK, data=data) - - -class LearningAssistantMessageHistoryView(APIView): - """ - View to retrieve the message history for user in a course. - - This endpoint returns the message history stored in the LearningAssistantMessage table in a course - represented by the course_run_id, which is provided in the URL. - - Accepts: [GET] - - Path: /learning_assistant/v1/course_id/{course_run_id}/history - - Parameters: - * course_run_id: the ID of the course - - Responses: - * 200: OK - * 400: Malformed Request - Course ID is not a valid course ID. - * 403: Forbidden - Learning assistant not enabled for course or learner does not have a valid enrollment or is - not staff. - """ - - authentication_classes = (SessionAuthentication, JwtAuthentication,) - permission_classes = (IsAuthenticated,) - - def get(self, request, course_run_id): - """ - Given a course run ID, retrieve the message history for the corresponding user. - - The response will be in the following format. - - [{'role': 'assistant', 'content': 'something'}] - """ - try: - courserun_key = CourseKey.from_string(course_run_id) - except InvalidKeyError: - return Response( - status=http_status.HTTP_400_BAD_REQUEST, - data={'detail': 'Course ID is not a valid course ID.'} - ) - - if not learning_assistant_enabled(courserun_key): - return Response( - status=http_status.HTTP_403_FORBIDDEN, - data={'detail': 'Learning assistant not enabled for course.'} - ) - - # If chat history is disabled, we return no messages as response. - if not chat_history_enabled(courserun_key): - return Response(status=http_status.HTTP_200_OK, data=[]) - - # If user does not have an enrollment record, or is not staff, they should not have access - user_role = get_user_role(request.user, courserun_key) - enrollment_object = CourseEnrollment.get_enrollment(request.user, courserun_key) - enrollment_mode = enrollment_object.mode if enrollment_object else None - if ( - (enrollment_mode not in CourseMode.VERIFIED_MODES) - and not user_role_is_staff(user_role) - ): - return Response( - status=http_status.HTTP_403_FORBIDDEN, - data={'detail': 'Must be staff or have valid enrollment.'} - ) - - user = request.user - - message_count = int(request.GET.get('message_count', 50)) - message_history = get_message_history(courserun_key, user, message_count) - data = MessageSerializer(message_history, many=True).data - return Response(status=http_status.HTTP_200_OK, data=data) - - class LearningAssistantChatSummaryView(APIView): """ View to retrieve data about a learner's session with the Learning Assistant. diff --git a/tests/test_views.py b/tests/test_views.py index cebd319..4a17578 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -249,222 +249,6 @@ def test_chat_response_default( mock_save_chat_message.assert_not_called() -@ddt.ddt -class LearningAssistantEnabledViewTests(LoggedInTestCase): - """ - Test for the LearningAssistantEnabledView - """ - sys.modules['lms.djangoapps.courseware.access'] = MagicMock() - sys.modules['lms.djangoapps.courseware.toggles'] = MagicMock() - sys.modules['common.djangoapps.course_modes.models'] = MagicMock() - sys.modules['common.djangoapps.student.models'] = MagicMock() - - def setUp(self): - super().setUp() - self.course_id = 'course-v1:edx+test+23' - - @ddt.data( - (True, True), - (False, False), - ) - @ddt.unpack - @patch('learning_assistant.views.learning_assistant_enabled') - def test_learning_assistant_enabled(self, mock_value, message, mock_learning_assistant_enabled): - mock_learning_assistant_enabled.return_value = mock_value - response = self.client.get(reverse('enabled', kwargs={'course_run_id': self.course_id})) - - self.assertEqual(response.status_code, 200) - self.assertEqual(response.data, {'enabled': message}) - - @patch('learning_assistant.views.learning_assistant_enabled') - def test_invalid_course_id(self, mock_learning_assistant_enabled): - mock_learning_assistant_enabled.return_value = True - response = self.client.get(reverse('enabled', kwargs={'course_run_id': self.course_id+'+invalid'})) - - self.assertEqual(response.status_code, 400) - - -class LearningAssistantMessageHistoryViewTests(LoggedInTestCase): - """ - Tests for the LearningAssistantMessageHistoryView - """ - - def setUp(self): - super().setUp() - self.course_id = 'course-v1:edx+test+23' - - @patch('learning_assistant.views.learning_assistant_enabled') - def test_course_waffle_inactive(self, mock_waffle): - mock_waffle.return_value = False - message_count = 5 - response = self.client.get( - reverse('message-history', kwargs={'course_run_id': self.course_id})+f'?message_count={message_count}', - content_type='application/json' - ) - self.assertEqual(response.status_code, 403) - - @patch('learning_assistant.views.learning_assistant_enabled') - def test_learning_assistant_not_enabled(self, mock_learning_assistant_enabled): - mock_learning_assistant_enabled.return_value = False - message_count = 5 - response = self.client.get( - reverse('message-history', kwargs={'course_run_id': self.course_id})+f'?message_count={message_count}', - content_type='application/json' - ) - - self.assertEqual(response.status_code, 403) - - @patch('learning_assistant.views.chat_history_enabled') - @patch('learning_assistant.views.learning_assistant_enabled') - @patch('learning_assistant.views.get_user_role') - @patch('learning_assistant.views.CourseEnrollment') - @patch('learning_assistant.views.CourseMode') - def test_user_no_enrollment_not_staff( - self, - mock_mode, - mock_enrollment, - mock_get_user_role, - mock_assistant_waffle, - mock_history_waffle - ): - mock_assistant_waffle.return_value = True - mock_history_waffle.return_value = True - mock_get_user_role.return_value = 'student' - mock_mode.VERIFIED_MODES = ['verified'] - mock_enrollment.get_enrollment = MagicMock(return_value=None) - - message_count = 5 - response = self.client.get( - reverse('message-history', kwargs={'course_run_id': self.course_id})+f'?message_count={message_count}', - content_type='application/json' - ) - self.assertEqual(response.status_code, 403) - - @patch('learning_assistant.views.chat_history_enabled') - @patch('learning_assistant.views.learning_assistant_enabled') - @patch('learning_assistant.views.get_user_role') - @patch('learning_assistant.views.CourseEnrollment') - @patch('learning_assistant.views.CourseMode') - def test_user_audit_enrollment_not_staff( - self, - mock_mode, - mock_enrollment, - mock_get_user_role, - mock_assistant_waffle, - mock_history_waffle - ): - mock_assistant_waffle.return_value = True - mock_history_waffle.return_value = True - mock_get_user_role.return_value = 'student' - mock_mode.VERIFIED_MODES = ['verified'] - mock_enrollment.get_enrollment.return_value = MagicMock(mode='audit') - - message_count = 5 - response = self.client.get( - reverse('message-history', kwargs={'course_run_id': self.course_id})+f'?message_count={message_count}', - content_type='application/json' - ) - self.assertEqual(response.status_code, 403) - - @patch('learning_assistant.views.chat_history_enabled') - @patch('learning_assistant.views.learning_assistant_enabled') - @patch('learning_assistant.views.get_user_role') - @patch('learning_assistant.views.CourseEnrollment') - @patch('learning_assistant.views.CourseMode') - @patch('learning_assistant.views.get_course_id') - def test_learning_message_history_view_get( - self, - mock_get_course_id, - mock_mode, - mock_enrollment, - mock_get_user_role, - mock_assistant_waffle, - mock_history_waffle, - ): - mock_assistant_waffle.return_value = True - mock_history_waffle.return_value = True - mock_get_user_role.return_value = 'student' - mock_mode.VERIFIED_MODES = ['verified'] - mock_enrollment.get_enrollment.return_value = MagicMock(mode='verified') - - LearningAssistantMessage.objects.create( - course_id=self.course_id, - user=self.user, - role='staff', - content='Older message', - created=date(2024, 10, 1) - ) - - LearningAssistantMessage.objects.create( - course_id=self.course_id, - user=self.user, - role='staff', - content='Newer message', - created=date(2024, 10, 3) - ) - - db_messages = LearningAssistantMessage.objects.all().order_by('created') - db_messages_count = len(db_messages) - - mock_get_course_id.return_value = self.course_id - response = self.client.get( - reverse('message-history', kwargs={'course_run_id': self.course_id})+f'?message_count={db_messages_count}', - content_type='application/json' - ) - data = response.data - - # Ensure same number of entries - self.assertEqual(len(data), db_messages_count) - - # Ensure values are as expected - for i, message in enumerate(data): - self.assertEqual(message['role'], db_messages[i].role) - self.assertEqual(message['content'], db_messages[i].content) - self.assertEqual(message['timestamp'], db_messages[i].created.isoformat()) - - @patch('learning_assistant.views.chat_history_enabled') - @patch('learning_assistant.views.learning_assistant_enabled') - @patch('learning_assistant.views.get_course_id') - def test_learning_message_history_view_get_disabled( - self, - mock_get_course_id, - mock_assistant_waffle, - mock_history_waffle, - ): - mock_assistant_waffle.return_value = True - mock_history_waffle.return_value = False - - LearningAssistantMessage.objects.create( - course_id=self.course_id, - user=self.user, - role='staff', - content='Older message', - created=date(2024, 10, 1) - ) - - LearningAssistantMessage.objects.create( - course_id=self.course_id, - user=self.user, - role='staff', - content='Newer message', - created=date(2024, 10, 3) - ) - - db_messages = LearningAssistantMessage.objects.all().order_by('created') - db_messages_count = len(db_messages) - - mock_get_course_id.return_value = self.course_id - response = self.client.get( - reverse('message-history', kwargs={'course_run_id': self.course_id})+f'?message_count={db_messages_count}', - content_type='application/json' - ) - data = response.data - - # Ensure returning an empty list - self.assertEqual(len(data), 0) - self.assertEqual(data, []) - - @ddt.ddt class LearningAssistantChatSummaryViewTests(LoggedInTestCase): """ From 48cefcfbcdbe98db1efbc2741eda165c976c4a9a Mon Sep 17 00:00:00 2001 From: Marcos Date: Tue, 11 Feb 2025 10:12:22 -0300 Subject: [PATCH 2/5] fix: Added date checks on the chat endpoints --- learning_assistant/urls.py | 5 +---- learning_assistant/views.py | 30 ++++++++++++++++++++++++++++-- tests/test_views.py | 16 ++++++++++------ 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/learning_assistant/urls.py b/learning_assistant/urls.py index 11c8171..59fd0ac 100644 --- a/learning_assistant/urls.py +++ b/learning_assistant/urls.py @@ -4,10 +4,7 @@ from django.urls import re_path from learning_assistant.constants import COURSE_ID_PATTERN -from learning_assistant.views import ( - CourseChatView, - LearningAssistantChatSummaryView, -) +from learning_assistant.views import CourseChatView, LearningAssistantChatSummaryView app_name = 'learning_assistant' diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 976f06b..f8f543d 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -2,6 +2,7 @@ V1 API Views. """ import logging +from datetime import datetime from django.conf import settings from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication @@ -33,6 +34,7 @@ save_chat_message, ) from learning_assistant.models import LearningAssistantMessage +from learning_assistant.platform_imports import get_cache_course_run_data from learning_assistant.serializers import MessageSerializer from learning_assistant.toggles import chat_history_enabled from learning_assistant.utils import get_audit_trial_length_days, get_chat_response, user_role_is_staff @@ -131,7 +133,20 @@ def post(self, request, course_run_id): data={'detail': 'Course ID is not a valid course ID.'} ) - if not learning_assistant_enabled(courserun_key): + course_data = get_cache_course_run_data(course_run_id, ['start', 'end']) + today = datetime.now() + start = course_data.get('start', None) + end = course_data.get('end', None) + + valid_dates = ( + (start <= today if start else True) + and (end >= today if end else True) + ) + + if ( + not valid_dates + or not learning_assistant_enabled(courserun_key) + ): return Response( status=http_status.HTTP_403_FORBIDDEN, data={'detail': 'Learning assistant not enabled for course.'} @@ -264,8 +279,19 @@ def get(self, request, course_run_id): # return no messages in the response. message_history_data = [] + course_data = get_cache_course_run_data(course_run_id, ['start', 'end']) + today = datetime.now() + start = course_data.get('start', None) + end = course_data.get('end', None) + + valid_dates = ( + (start <= today if start else True) + and (end >= today if end else True) + ) + has_trial_access = ( - enrollment_mode in valid_trial_access_modes + valid_dates + and enrollment_mode in valid_trial_access_modes and audit_trial and not audit_trial_is_expired(enrollment_object, audit_trial) ) diff --git a/tests/test_views.py b/tests/test_views.py index 4a17578..06f71c1 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -76,6 +76,16 @@ def setUp(self): self.user = User(username='tester', email='tester@test.com', is_staff=True) self.user.save() self.client.login_user(self.user) + self.patcher = patch( + 'learning_assistant.views.get_cache_course_run_data', + return_value={'course': 'edx+test'} + ) + self.patcher.start() + self.patcher2 = patch( + 'learning_assistant.api.get_cache_course_run_data', + return_value={'course': 'edx+test'} + ) + self.patcher2.start() @ddt.ddt @@ -93,12 +103,6 @@ def setUp(self): self.course_id = 'course-v1:edx+test+23' self.course_run_key = CourseKey.from_string(self.course_id) - self.patcher = patch( - 'learning_assistant.api.get_cache_course_run_data', - return_value={'course': 'edx+test'} - ) - self.patcher.start() - @patch('learning_assistant.views.learning_assistant_enabled') def test_course_waffle_inactive(self, mock_waffle): mock_waffle.return_value = False From f4da43d1fcb8162715a9aae6f45fc98673297ade Mon Sep 17 00:00:00 2001 From: Marcos Date: Tue, 11 Feb 2025 15:55:54 -0300 Subject: [PATCH 3/5] feat: Updated tests to include date gating --- learning_assistant/views.py | 21 +++- tests/test_views.py | 216 ++++++++++++++++++++++++++++++++++-- 2 files changed, 226 insertions(+), 11 deletions(-) diff --git a/learning_assistant/views.py b/learning_assistant/views.py index f8f543d..9328c4c 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -249,11 +249,28 @@ def get(self, request, course_run_id): data={'detail': 'Course ID is not a valid course ID.'} ) - data = {} + data = { + 'enabled': False, + 'message_history': [], + 'audit_trial': {}, + 'audit_trial_length_days': 0, + } user = request.user + course_data = get_cache_course_run_data(course_run_id, ['start', 'end']) + today = datetime.now() + start = course_data.get('start', None) + end = course_data.get('end', None) + valid_dates = ( + (start <= today if start else True) + and (end >= today if end else True) + ) + # Get whether the Learning Assistant is enabled. - data['enabled'] = learning_assistant_enabled(courserun_key) + data['enabled'] = learning_assistant_enabled(courserun_key) and valid_dates + + if not data['enabled']: + return Response(status=http_status.HTTP_200_OK, data=data) # Get message history. # If user does not have a verified enrollment record or is does not have an active audit trial, or is not staff, diff --git a/tests/test_views.py b/tests/test_views.py index 06f71c1..99046ca 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -24,6 +24,26 @@ User = get_user_model() +mocked_today = datetime(2024, 6, 15) + +mocked_course = { + 'course': 'edx+test', + 'start': None, + 'end': None, +} + + +class MockedDatetime(): + """ + Mocks datetime implementation + """ + + def __init__(self, *args): + pass + + def now(self): + return mocked_today + class FakeClient(Client): """ @@ -76,16 +96,27 @@ def setUp(self): self.user = User(username='tester', email='tester@test.com', is_staff=True) self.user.save() self.client.login_user(self.user) - self.patcher = patch( + + self.mocked_datetime = patch( + 'learning_assistant.views.datetime', + MockedDatetime() + ) + self.mocked_datetime.start() + self.addCleanup(self.mocked_datetime.stop) + + self.mocked_get_cache_course_run_data = patch( 'learning_assistant.views.get_cache_course_run_data', - return_value={'course': 'edx+test'} + return_value=mocked_course ) - self.patcher.start() - self.patcher2 = patch( - 'learning_assistant.api.get_cache_course_run_data', - return_value={'course': 'edx+test'} + self.mocked_get_cache_course_run_data.start() + self.addCleanup(self.mocked_get_cache_course_run_data.stop) + + self.mocked_get_course_id = patch( + 'learning_assistant.views.get_course_id', + return_value=mocked_course['course'] ) - self.patcher2.start() + self.mocked_get_course_id.start() + self.addCleanup(self.mocked_get_course_id.stop) @ddt.ddt @@ -135,6 +166,86 @@ def test_invalid_messages(self, mock_mode, mock_enrollment, mock_get_user_role, ) self.assertEqual(response.status_code, 400) + @ddt.data({ + 'start': datetime(2023, 1, 1), # past date + 'end': datetime(2023, 1, 30), # past date + 'expects_fail': True, + }, { + 'start': datetime(2026, 1, 1), # future date + 'end': datetime(2026, 1, 30), # future date + 'expects_fail': True, + }, { + 'start': datetime(2024, 6, 1), # past date + 'end': datetime(2024, 6, 30), # future date + 'expects_fail': False, + }, { + 'start': datetime(2024, 6, 1), # past date + 'end': None, + 'expects_fail': False, + }, { + 'start': None, + 'end': datetime(2024, 6, 30), # future date + 'expects_fail': False, + }) + @patch('learning_assistant.views.get_cache_course_run_data') + @patch('learning_assistant.views.audit_trial_is_expired') + @patch('learning_assistant.views.render_prompt_template') + @patch('learning_assistant.views.get_chat_response') + @patch('learning_assistant.views.learning_assistant_enabled') + @patch('learning_assistant.views.get_user_role') + @patch('learning_assistant.views.CourseEnrollment') + @patch('learning_assistant.views.CourseMode') + @patch('learning_assistant.views.chat_history_enabled') + @override_settings(LEARNING_ASSISTANT_PROMPT_TEMPLATE='This is the default template') + def test_dates_for_chat_response( + self, + dates, + mock_chat_history_enabled, + mock_mode, + mock_enrollment, + mock_get_user_role, + mock_waffle, + mock_chat_response, + mock_render, + mock_trial_expired, + mocked_get_cache_course_run_data, + ): + mocked_get_cache_course_run_data.return_value = { + 'course': mocked_course['course'], + 'start': dates['start'], + 'end': dates['end'], + } + mock_waffle.return_value = True + mock_get_user_role.return_value = 'staff' + mock_mode.VERIFIED_MODES = ['verified'] + mock_mode.CREDIT_MODES = ['credit'] + mock_mode.NO_ID_PROFESSIONAL_MODE = 'no-id' + mock_mode.UPSELL_TO_VERIFIED_MODES = ['audit'] + mock_enrollment.get_enrollment.return_value = MagicMock(mode='staff') + mock_chat_response.return_value = (200, {'role': 'assistant', 'content': 'Something else'}) + mock_render.return_value = 'Rendered template mock' + mock_trial_expired.return_value = False + test_unit_id = 'test-unit-id' + + mock_chat_history_enabled.return_value = True + + test_data = [ + {'role': 'user', 'content': 'What is 2+2?'}, + {'role': 'assistant', 'content': 'It is 4'}, + {'role': 'user', 'content': 'And what else?'}, + ] + + response = self.client.post( + reverse('chat', kwargs={'course_run_id': self.course_id})+f'?unit_id={test_unit_id}', + data=json.dumps(test_data), + content_type='application/json' + ) + + if dates['expects_fail']: + self.assertEqual(response.status_code, 403) + else: + self.assertEqual(response.status_code, 200) + @patch('learning_assistant.views.audit_trial_is_expired') @patch('learning_assistant.views.learning_assistant_enabled') @patch('learning_assistant.views.get_user_role') @@ -369,8 +480,17 @@ def test_chat_summary_with_access_instructor( response = self.client.get(url) - # Assert message history data is correct. + if not learning_assistant_enabled_mock_value: + self.assertEqual(response.data, { + 'enabled': False, + 'message_history': [], + 'audit_trial': {}, + 'audit_trial_length_days': 0, + }) + return + if chat_history_enabled_mock_value: + # Assert message history data is correct. data = response.data['message_history'] # Ensure same number of entries. @@ -486,8 +606,17 @@ def test_chat_summary_with_full_access_student( response = self.client.get(url) - # Assert message history data is correct. + if not learning_assistant_enabled_mock_value: + self.assertEqual(response.data, { + 'enabled': False, + 'message_history': [], + 'audit_trial': {}, + 'audit_trial_length_days': 0, + }) + return + if chat_history_enabled_mock_value: + # Assert message history data is correct. data = response.data['message_history'] # Ensure same number of entries. @@ -603,6 +732,15 @@ def test_chat_summary_with_trial_access_student( response = self.client.get(url) + if not learning_assistant_enabled_mock_value: + self.assertEqual(response.data, { + 'enabled': False, + 'message_history': [], + 'audit_trial': {}, + 'audit_trial_length_days': 0, + }) + return + # Assert message history data is correct. if chat_history_enabled_mock_value and trial_available and not audit_trial_is_expired_mock_value: data = response.data['message_history'] @@ -626,3 +764,63 @@ def test_chat_summary_with_trial_access_student( self.assertEqual(response.data['audit_trial'], expected_trial_data) self.assertEqual(response.data['audit_trial_length_days'], audit_trial_length_days_mock_value) + + @ddt.data({ + 'start': datetime(2023, 1, 1), # past date + 'end': datetime(2023, 1, 30), # past date + 'expects_fail': True, + }, { + 'start': datetime(2026, 1, 1), # future date + 'end': datetime(2026, 1, 30), # future date + 'expects_fail': True, + }, { + 'start': datetime(2024, 6, 1), # past date + 'end': datetime(2024, 6, 30), # future date + 'expects_fail': False, + }, { + 'start': datetime(2024, 6, 1), # past date + 'end': None, + 'expects_fail': False, + }, { + 'start': None, + 'end': datetime(2024, 6, 30), # future date + 'expects_fail': False, + }) + @patch('learning_assistant.views.get_cache_course_run_data') + @patch('learning_assistant.views.chat_history_enabled') + @patch('learning_assistant.views.learning_assistant_enabled') + @patch('learning_assistant.views.get_user_role') + @patch('learning_assistant.views.CourseEnrollment') + @patch('learning_assistant.views.CourseMode') + def test_chat_summary_with_date_gating( + self, + dates, + mock_mode, + mock_enrollment, + mock_get_user_role, + mock_learning_assistant_enabled, + mock_chat_history_enabled, + mocked_get_cache_course_run_data, + ): + mocked_get_cache_course_run_data.return_value = { + 'course': mocked_course['course'], + 'start': dates['start'], + 'end': dates['end'], + } + mock_learning_assistant_enabled.return_value = True + mock_chat_history_enabled.return_value = True + mock_get_user_role.return_value = 'staff' + mock_mode.VERIFIED_MODES = ['verified'] + mock_mode.CREDIT_MODES = ['credit'] + mock_mode.NO_ID_PROFESSIONAL_MODE = 'no-id' + mock_mode.UPSELL_TO_VERIFIED_MODES = ['audit'] + + mock_enrollment.get_enrollment.return_value = MagicMock(mode='staff') + + url_kwargs = {'course_run_id': self.course_id} + url = reverse('chat-summary', kwargs=url_kwargs) + + response = self.client.get(url) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['enabled'], not dates['expects_fail']) From b47a79646465d896d36f14ece807fe6815e8bd86 Mon Sep 17 00:00:00 2001 From: Marcos Date: Tue, 18 Feb 2025 10:37:13 -0300 Subject: [PATCH 4/5] fix: Updated tests based on reviews --- learning_assistant/views.py | 16 +++------------- tests/test_views.py | 24 +++--------------------- 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 9328c4c..002722d 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -140,7 +140,7 @@ def post(self, request, course_run_id): valid_dates = ( (start <= today if start else True) - and (end >= today if end else True) + and (end > today if end else True) ) if ( @@ -263,7 +263,7 @@ def get(self, request, course_run_id): end = course_data.get('end', None) valid_dates = ( (start <= today if start else True) - and (end >= today if end else True) + and (end > today if end else True) ) # Get whether the Learning Assistant is enabled. @@ -296,16 +296,6 @@ def get(self, request, course_run_id): # return no messages in the response. message_history_data = [] - course_data = get_cache_course_run_data(course_run_id, ['start', 'end']) - today = datetime.now() - start = course_data.get('start', None) - end = course_data.get('end', None) - - valid_dates = ( - (start <= today if start else True) - and (end >= today if end else True) - ) - has_trial_access = ( valid_dates and enrollment_mode in valid_trial_access_modes @@ -315,7 +305,7 @@ def get(self, request, course_run_id): if ( ( - (enrollment_mode in valid_full_access_modes) + enrollment_mode in valid_full_access_modes or has_trial_access or user_role_is_staff(user_role) ) diff --git a/tests/test_views.py b/tests/test_views.py index 99046ca..af51440 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -24,8 +24,6 @@ User = get_user_model() -mocked_today = datetime(2024, 6, 15) - mocked_course = { 'course': 'edx+test', 'start': None, @@ -33,18 +31,6 @@ } -class MockedDatetime(): - """ - Mocks datetime implementation - """ - - def __init__(self, *args): - pass - - def now(self): - return mocked_today - - class FakeClient(Client): """ Allows for 'fake logins' of a user so we don't need to expose a 'login' HTTP endpoint. @@ -97,13 +83,6 @@ def setUp(self): self.user.save() self.client.login_user(self.user) - self.mocked_datetime = patch( - 'learning_assistant.views.datetime', - MockedDatetime() - ) - self.mocked_datetime.start() - self.addCleanup(self.mocked_datetime.stop) - self.mocked_get_cache_course_run_data = patch( 'learning_assistant.views.get_cache_course_run_data', return_value=mocked_course @@ -120,6 +99,7 @@ def setUp(self): @ddt.ddt +@freeze_time('2024-06-15') class CourseChatViewTests(LoggedInTestCase): """ Test for the CourseChatView @@ -243,6 +223,7 @@ def test_dates_for_chat_response( if dates['expects_fail']: self.assertEqual(response.status_code, 403) + self.assertEqual(response.data, {'detail': 'Learning assistant not enabled for course.'}) else: self.assertEqual(response.status_code, 200) @@ -365,6 +346,7 @@ def test_chat_response_default( @ddt.ddt +@freeze_time('2024-06-15') class LearningAssistantChatSummaryViewTests(LoggedInTestCase): """ Tests for the LearningAssistantChatSummaryView From 1aa297f93b4aaa1cbd0c38c95940931e07da5566 Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 19 Feb 2025 10:27:55 -0300 Subject: [PATCH 5/5] chore: Rolled back date comparison to be >= --- learning_assistant/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 002722d..2de8b72 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -140,7 +140,7 @@ def post(self, request, course_run_id): valid_dates = ( (start <= today if start else True) - and (end > today if end else True) + and (end >= today if end else True) ) if ( @@ -263,7 +263,7 @@ def get(self, request, course_run_id): end = course_data.get('end', None) valid_dates = ( (start <= today if start else True) - and (end > today if end else True) + and (end >= today if end else True) ) # Get whether the Learning Assistant is enabled.