From ecfb6e850d70a279618631f29117984556ba0cab Mon Sep 17 00:00:00 2001 From: Isaac Lee <124631592+ilee2u@users.noreply.github.com> Date: Thu, 31 Oct 2024 11:18:30 -0400 Subject: [PATCH 01/22] feat: GET endpoint to retrieve message history (#119) * feat: GET endpoint to retrieve message history * fix: added message count as query parameter * chore: rename to MESSAGE_COUNT * chore: lint * chore: isort * fix: corrected query parameter - some docstring nits - logic to ensure no index error w/ message_count in api - serialize data before returning * test: add tests * test: fix view tests * chore: lint * chore: lint * chore: LINT * fix: actually correct query param * chore: function param lint * chore: nits --- CHANGELOG.rst | 4 + learning_assistant/__init__.py | 2 +- learning_assistant/api.py | 13 ++- learning_assistant/text_utils.py | 2 +- learning_assistant/urls.py | 7 +- learning_assistant/views.py | 73 +++++++++++++++- tests/test_api.py | 145 ++++++++++++++++++++++++++++++- tests/test_plugins_api.py | 1 + tests/test_views.py | 123 +++++++++++++++++++++++++- 9 files changed, 361 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fb53e08..5fffeaa 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,10 @@ Unreleased ********** * Add LearningAssistantMessage model +4.4.0 - 2024-10-30 +****************** +* Add new GET endpoint to retrieve a user's message history in a given course. + 4.3.3 - 2024-10-15 ****************** * Use `LEARNING_ASSISTANT_PROMPT_TEMPLATE` for prompt diff --git a/learning_assistant/__init__.py b/learning_assistant/__init__.py index f654ddd..f8f5d1c 100644 --- a/learning_assistant/__init__.py +++ b/learning_assistant/__init__.py @@ -2,6 +2,6 @@ Plugin for a learning assistant backend, intended for use within edx-platform. """ -__version__ = '4.3.3' +__version__ = '4.4.0' default_app_config = 'learning_assistant.apps.LearningAssistantConfig' # pylint: disable=invalid-name diff --git a/learning_assistant/api.py b/learning_assistant/api.py index 3e0b5e0..0790059 100644 --- a/learning_assistant/api.py +++ b/learning_assistant/api.py @@ -11,7 +11,7 @@ from learning_assistant.constants import ACCEPTED_CATEGORY_TYPES, CATEGORY_TYPE_MAP from learning_assistant.data import LearningAssistantCourseEnabledData -from learning_assistant.models import LearningAssistantCourseEnabled +from learning_assistant.models import LearningAssistantCourseEnabled, LearningAssistantMessage from learning_assistant.platform_imports import ( block_get_children, block_leaf_filter, @@ -187,3 +187,14 @@ def get_course_id(course_run_id): course_data = get_cache_course_run_data(course_run_id, ['course']) course_key = course_data['course'] return course_key + + +def get_message_history(course_id, user, message_count): + """ + Given a course run id (str), user (User), and message count (int), return the associated message history. + + Returns a number of messages equal to the message_count value. + """ + message_history = LearningAssistantMessage.objects.filter( + course_id=course_id, user=user).order_by('-created')[:message_count] + return message_history diff --git a/learning_assistant/text_utils.py b/learning_assistant/text_utils.py index b97872c..a1795e9 100644 --- a/learning_assistant/text_utils.py +++ b/learning_assistant/text_utils.py @@ -20,7 +20,7 @@ def cleanup_text(text): return stripped -class _HTMLToTextHelper(HTMLParser): # lint-amnesty, pylint: disable=abstract-method +class _HTMLToTextHelper(HTMLParser): # lint-amnesty """ Helper function for html_to_text below. """ diff --git a/learning_assistant/urls.py b/learning_assistant/urls.py index a5d3bbe..b0dfb48 100644 --- a/learning_assistant/urls.py +++ b/learning_assistant/urls.py @@ -4,7 +4,7 @@ from django.urls import re_path from learning_assistant.constants import COURSE_ID_PATTERN -from learning_assistant.views import CourseChatView, LearningAssistantEnabledView +from learning_assistant.views import CourseChatView, LearningAssistantEnabledView, LearningAssistantMessageHistoryView app_name = 'learning_assistant' @@ -19,4 +19,9 @@ LearningAssistantEnabledView.as_view(), name='enabled', ), + re_path( + fr'learning_assistant/v1/course_id/{COURSE_ID_PATTERN}/history', + LearningAssistantMessageHistoryView.as_view(), + name='message-history', + ), ] diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 7739240..68b1c0a 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -20,7 +20,12 @@ except ImportError: pass -from learning_assistant.api import get_course_id, learning_assistant_enabled, render_prompt_template +from learning_assistant.api import ( + get_course_id, + get_message_history, + learning_assistant_enabled, + render_prompt_template, +) from learning_assistant.serializers import MessageSerializer from learning_assistant.utils import get_chat_response, user_role_is_staff @@ -149,3 +154,69 @@ def get(self, request, course_run_id): } 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_key, which is provided in the URL. + + Accepts: [GET] + + Path: /learning_assistant/v1/course_id/{course_key}/history + + Parameters: + * course_key: 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 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 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.'} + ) + + course_id = get_course_id(course_run_id) + user = request.user + + message_count = int(request.GET.get('message_count', 50)) + message_history = get_message_history(course_id, user, message_count) + data = MessageSerializer(message_history, many=True).data + return Response(status=http_status.HTTP_200_OK, data=data) diff --git a/tests/test_api.py b/tests/test_api.py index 5cb2a43..3ca365d 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -6,6 +6,7 @@ import ddt from django.conf import settings +from django.contrib.auth import get_user_model from django.core.cache import cache from django.test import TestCase, override_settings from opaque_keys import InvalidKeyError @@ -16,16 +17,19 @@ _get_children_contents, _leaf_filter, get_block_content, + get_message_history, learning_assistant_available, learning_assistant_enabled, render_prompt_template, set_learning_assistant_enabled, ) from learning_assistant.data import LearningAssistantCourseEnabledData -from learning_assistant.models import LearningAssistantCourseEnabled +from learning_assistant.models import LearningAssistantCourseEnabled, LearningAssistantMessage fake_transcript = 'This is the text version from the transcript' +User = get_user_model() + class FakeChild: """Fake child block for testing""" @@ -49,6 +53,7 @@ def get_html(self): class FakeBlock: "Fake block for testing, returns given children" + def __init__(self, children): self.children = children self.scope_ids = lambda: None @@ -236,6 +241,7 @@ class LearningAssistantCourseEnabledApiTests(TestCase): """ Test suite for learning_assistant_available, learning_assistant_enabled, and set_learning_assistant_enabled. """ + def setUp(self): super().setUp() self.course_key = CourseKey.from_string('course-v1:edx+fake+1') @@ -305,3 +311,140 @@ def test_learning_assistant_available(self, learning_assistant_available_setting expected_value = learning_assistant_available_setting_value self.assertEqual(return_value, expected_value) + + +@ddt.ddt +class GetMessageHistoryTests(TestCase): + """ + Test suite for get_message_history. + """ + + def setUp(self): + super().setUp() + self.course_id = 'course-v1:edx+fake+1' + self.course_key = CourseKey.from_string(self.course_id) + self.user = User(username='tester', email='tester@test.com') + self.user.save() + + self.role = 'verified' + + def test_get_message_history(self): + message_count = 5 + for i in range(1, message_count + 1): + LearningAssistantMessage.objects.create( + course_id=self.course_id, + user=self.user, + role=self.role, + content=f'Content of message {i}', + ) + + return_value = get_message_history(self.course_id, self.user, message_count) + + expected_value = LearningAssistantMessage.objects.filter( + course_id=self.course_id, user=self.user).order_by('-created')[:message_count] + + # Ensure same number of entries + self.assertEqual(len(return_value), len(expected_value)) + + # Ensure values are as expected for all LearningAssistantMessage instances + for i, return_value in enumerate(return_value): + self.assertEqual(return_value.course_id, expected_value[i].course_id) + self.assertEqual(return_value.user, expected_value[i].user) + self.assertEqual(return_value.role, expected_value[i].role) + self.assertEqual(return_value.content, expected_value[i].content) + + @ddt.data( + 0, 1, 5, 10, 50 + ) + def test_get_message_history_message_count(self, actual_message_count): + for i in range(1, actual_message_count + 1): + LearningAssistantMessage.objects.create( + course_id=self.course_id, + user=self.user, + role=self.role, + content=f'Content of message {i}', + ) + + message_count_parameter = 5 + return_value = get_message_history(self.course_id, self.user, message_count_parameter) + + expected_value = LearningAssistantMessage.objects.filter( + course_id=self.course_id, user=self.user).order_by('-created')[:message_count_parameter] + + # Ensure same number of entries + self.assertEqual(len(return_value), len(expected_value)) + + def test_get_message_history_user_difference(self): + # Default Message + LearningAssistantMessage.objects.create( + course_id=self.course_id, + user=self.user, + role=self.role, + content='Expected content of message', + ) + + # New message w/ new user + new_user = User(username='not_tester', email='not_tester@test.com') + new_user.save() + LearningAssistantMessage.objects.create( + course_id=self.course_id, + user=new_user, + role=self.role, + content='Expected content of message', + ) + + message_count = 2 + return_value = get_message_history(self.course_id, self.user, message_count) + + expected_value = LearningAssistantMessage.objects.filter( + course_id=self.course_id, user=self.user).order_by('-created')[:message_count] + + # Ensure we filtered one of the two present messages + self.assertNotEqual(len(return_value), LearningAssistantMessage.objects.count()) + + # Ensure same number of entries + self.assertEqual(len(return_value), len(expected_value)) + + # Ensure values are as expected for all LearningAssistantMessage instances + for i, return_value in enumerate(return_value): + self.assertEqual(return_value.course_id, expected_value[i].course_id) + self.assertEqual(return_value.user, expected_value[i].user) + self.assertEqual(return_value.role, expected_value[i].role) + self.assertEqual(return_value.content, expected_value[i].content) + + def test_get_message_course_id_differences(self): + # Default Message + LearningAssistantMessage.objects.create( + course_id=self.course_id, + user=self.user, + role=self.role, + content='Expected content of message', + ) + + # New message + wrong_course_id = 'course-v1:wrong+id+1' + LearningAssistantMessage.objects.create( + course_id=wrong_course_id, + user=self.user, + role=self.role, + content='Expected content of message', + ) + + message_count = 2 + return_value = get_message_history(self.course_id, self.user, message_count) + + expected_value = LearningAssistantMessage.objects.filter( + course_id=self.course_id, user=self.user).order_by('-created')[:message_count] + + # Ensure we filtered one of the two present messages + self.assertNotEqual(len(return_value), LearningAssistantMessage.objects.count()) + + # Ensure same number of entries + self.assertEqual(len(return_value), len(expected_value)) + + # Ensure values are as expected for all LearningAssistantMessage instances + for i, return_value in enumerate(return_value): + self.assertEqual(return_value.course_id, expected_value[i].course_id) + self.assertEqual(return_value.user, expected_value[i].user) + self.assertEqual(return_value.role, expected_value[i].role) + self.assertEqual(return_value.content, expected_value[i].content) diff --git a/tests/test_plugins_api.py b/tests/test_plugins_api.py index 076335d..4e30539 100644 --- a/tests/test_plugins_api.py +++ b/tests/test_plugins_api.py @@ -19,6 +19,7 @@ class PluginApiTests(TestCase): """ Test suite for the plugins_api module. """ + def setUp(self): super().setUp() self.course_key = CourseKey.from_string('course-v1:edx+fake+1') diff --git a/tests/test_views.py b/tests/test_views.py index 7ff440e..492a10d 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -14,6 +14,8 @@ from django.test.client import Client from django.urls import reverse +from learning_assistant.models import LearningAssistantMessage + User = get_user_model() @@ -21,6 +23,7 @@ class TestClient(Client): """ Allows for 'fake logins' of a user so we don't need to expose a 'login' HTTP endpoint. """ + def login_user(self, user): """ Login as specified user. @@ -64,7 +67,7 @@ def setUp(self): """ super().setUp() self.client = TestClient() - self.user = User(username='tester', email='tester@test.com') + self.user = User(username='tester', email='tester@test.com', is_staff=True) self.user.save() self.client.login_user(self.user) @@ -210,12 +213,12 @@ def setUp(self): ) @ddt.unpack @patch('learning_assistant.views.learning_assistant_enabled') - def test_learning_assistant_enabled(self, mock_value, expected_value, mock_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': expected_value}) + self.assertEqual(response.data, {'enabled': message}) @patch('learning_assistant.views.learning_assistant_enabled') def test_invalid_course_id(self, mock_learning_assistant_enabled): @@ -223,3 +226,117 @@ def test_invalid_course_id(self, mock_learning_assistant_enabled): response = self.client.get(reverse('enabled', kwargs={'course_run_id': self.course_id+'+invalid'})) self.assertEqual(response.status_code, 400) + + +@ddt.ddt +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_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) + + @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.learning_assistant_enabled') + @patch('learning_assistant.views.get_user_role') + @patch('learning_assistant.views.CourseEnrollment.get_enrollment') + @patch('learning_assistant.views.CourseMode') + def test_user_no_enrollment_not_staff(self, mock_mode, mock_enrollment, mock_role, mock_waffle): + mock_waffle.return_value = True + mock_role.return_value = 'student' + mock_mode.VERIFIED_MODES = ['verified'] + mock_enrollment.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.learning_assistant_enabled') + @patch('learning_assistant.views.get_user_role') + @patch('learning_assistant.views.CourseEnrollment.get_enrollment') + @patch('learning_assistant.views.CourseMode') + def test_user_audit_enrollment_not_staff(self, mock_mode, mock_enrollment, mock_role, mock_waffle): + mock_waffle.return_value = True + mock_role.return_value = 'student' + mock_mode.VERIFIED_MODES = ['verified'] + mock_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.learning_assistant_enabled') + @patch('learning_assistant.views.get_user_role') + @patch('learning_assistant.views.CourseEnrollment.get_enrollment') + @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_role, + mock_waffle + ): + mock_waffle.return_value = True + mock_role.return_value = 'student' + mock_mode.VERIFIED_MODES = ['verified'] + mock_enrollment.return_value = MagicMock(mode='verified') + + LearningAssistantMessage.objects.create( + course_id=self.course_id, + user=self.user, + role='staff', + content='Expected content of message', + ) + message_count = 1 + 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={message_count}', + content_type='application/json' + ) + data = response.data + + # Ensure same number of entries + actual_message_count = LearningAssistantMessage.objects.count() + self.assertEqual(len(data), actual_message_count) + + # Ensure values are as expected + actual_message = LearningAssistantMessage.objects.get(course_id=self.course_id) + self.assertEqual(data[0]['role'], actual_message.role) + self.assertEqual(data[0]['content'], actual_message.content) From 9a4f62d194b7f259628efdd6ae62eab2d06ba00b Mon Sep 17 00:00:00 2001 From: Alison Langston <46360176+alangsto@users.noreply.github.com> Date: Thu, 31 Oct 2024 13:21:31 -0400 Subject: [PATCH 02/22] feat: add management command to remove expired messages (#122) * feat: add management command to remove expired messages * fix: update for lint --- CHANGELOG.rst | 1 + .../commands/retire_user_messages.py | 68 +++++++++++++++++++ .../tests/test_retire_user_messages.py | 68 +++++++++++++++++++ tests/__init__.py | 0 4 files changed, 137 insertions(+) create mode 100644 learning_assistant/management/commands/retire_user_messages.py create mode 100644 learning_assistant/management/commands/tests/test_retire_user_messages.py create mode 100644 tests/__init__.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5fffeaa..a3882fd 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,7 @@ Change Log Unreleased ********** * Add LearningAssistantMessage model +* Add management command to remove expired messages 4.4.0 - 2024-10-30 ****************** diff --git a/learning_assistant/management/commands/retire_user_messages.py b/learning_assistant/management/commands/retire_user_messages.py new file mode 100644 index 0000000..8b90082 --- /dev/null +++ b/learning_assistant/management/commands/retire_user_messages.py @@ -0,0 +1,68 @@ +"""" +Django management command to remove LearningAssistantMessage objects +if they have reached their expiration date. +""" +import logging +import time +from datetime import datetime, timedelta + +from django.conf import settings +from django.core.management.base import BaseCommand + +from learning_assistant.models import LearningAssistantMessage + +log = logging.getLogger(__name__) + + +class Command(BaseCommand): + """ + Django Management command to remove expired messages. + """ + + def add_arguments(self, parser): + parser.add_argument( + '--batch_size', + action='store', + dest='batch_size', + type=int, + default=300, + help='Maximum number of messages to remove. ' + 'This helps avoid overloading the database while updating large amount of data.' + ) + parser.add_argument( + '--sleep_time', + action='store', + dest='sleep_time', + type=int, + default=10, + help='Sleep time in seconds between update of batches' + ) + + def handle(self, *args, **options): + """ + Management command entry point. + """ + batch_size = options['batch_size'] + sleep_time = options['sleep_time'] + + expiry_date = datetime.now() - timedelta(days=getattr(settings, 'LEARNING_ASSISTANT_MESSAGES_EXPIRY', 30)) + + total_deleted = 0 + deleted_count = None + + while deleted_count != 0: + ids_to_delete = LearningAssistantMessage.objects.filter( + created__lte=expiry_date + ).values_list('id', flat=True)[:batch_size] + + ids_to_delete = list(ids_to_delete) + delete_queryset = LearningAssistantMessage.objects.filter( + id__in=ids_to_delete + ) + deleted_count, _ = delete_queryset.delete() + + total_deleted += deleted_count + log.info(f'{deleted_count} messages deleted.') + time.sleep(sleep_time) + + log.info(f'Job completed. {total_deleted} messages deleted.') diff --git a/learning_assistant/management/commands/tests/test_retire_user_messages.py b/learning_assistant/management/commands/tests/test_retire_user_messages.py new file mode 100644 index 0000000..afd720f --- /dev/null +++ b/learning_assistant/management/commands/tests/test_retire_user_messages.py @@ -0,0 +1,68 @@ +""" +Tests for the retire_user_messages management command +""" +from datetime import datetime, timedelta + +from django.contrib.auth import get_user_model +from django.core.management import call_command +from django.test import TestCase + +from learning_assistant.models import LearningAssistantMessage + +User = get_user_model() + + +class RetireUserMessagesTests(TestCase): + """ + Tests for the retire_user_messages command. + """ + + def setUp(self): + """ + Build up test data + """ + super().setUp() + self.user = User(username='tester', email='tester@test.com') + self.user.save() + + self.course_id = 'course-v1:edx+test+23' + + LearningAssistantMessage.objects.create( + user=self.user, + course_id=self.course_id, + role='user', + content='Hello', + created=datetime.now() - timedelta(days=60) + ) + + LearningAssistantMessage.objects.create( + user=self.user, + course_id=self.course_id, + role='user', + content='Hello', + created=datetime.now() - timedelta(days=2) + ) + + LearningAssistantMessage.objects.create( + user=self.user, + course_id=self.course_id, + role='user', + content='Hello', + created=datetime.now() - timedelta(days=4) + ) + + def test_run_command(self): + """ + Run the management command + """ + current_messages = LearningAssistantMessage.objects.filter() + self.assertEqual(len(current_messages), 3) + + call_command( + 'retire_user_messages', + batch_size=2, + sleep_time=0, + ) + + current_messages = LearningAssistantMessage.objects.filter() + self.assertEqual(len(current_messages), 2) diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 From dbcaa394e40f18d7c125652ec01819c26f3a427b Mon Sep 17 00:00:00 2001 From: Alison Langston <46360176+alangsto@users.noreply.github.com> Date: Thu, 31 Oct 2024 13:38:04 -0400 Subject: [PATCH 03/22] feat: update release version (#123) --- CHANGELOG.rst | 5 ++++- learning_assistant/__init__.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a3882fd..baed526 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,11 +13,14 @@ Change Log Unreleased ********** -* Add LearningAssistantMessage model + +4.4.1 - 2024-10-31 +****************** * Add management command to remove expired messages 4.4.0 - 2024-10-30 ****************** +* Add LearningAssistantMessage model * Add new GET endpoint to retrieve a user's message history in a given course. 4.3.3 - 2024-10-15 diff --git a/learning_assistant/__init__.py b/learning_assistant/__init__.py index f8f5d1c..fb90252 100644 --- a/learning_assistant/__init__.py +++ b/learning_assistant/__init__.py @@ -2,6 +2,6 @@ Plugin for a learning assistant backend, intended for use within edx-platform. """ -__version__ = '4.4.0' +__version__ = '4.4.1' default_app_config = 'learning_assistant.apps.LearningAssistantConfig' # pylint: disable=invalid-name From 99ccaa22046beeb7d7c925d6c41b7b7966c2ecf0 Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 30 Oct 2024 09:40:54 -0300 Subject: [PATCH 04/22] feat: Adding chat messages to the DB --- learning_assistant/models.py | 10 +++++++++- learning_assistant/serializers.py | 4 +++- learning_assistant/views.py | 28 ++++++++++++++++++++++++++++ tests/test_views.py | 15 ++++++++++++++- 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/learning_assistant/models.py b/learning_assistant/models.py index c890087..482bfe2 100644 --- a/learning_assistant/models.py +++ b/learning_assistant/models.py @@ -35,7 +35,15 @@ class LearningAssistantMessage(TimeStampedModel): .. pii_retirement: third_party """ + USER_ROLE = 'user' + ASSISTANT_ROLE = 'assistant' + + Roles = ( + (USER_ROLE, USER_ROLE), + (ASSISTANT_ROLE, ASSISTANT_ROLE), + ) + course_id = CourseKeyField(max_length=255, db_index=True) user = models.ForeignKey(USER_MODEL, db_index=True, on_delete=models.CASCADE) - role = models.CharField(max_length=64) + role = models.CharField(choices=Roles, max_length=64) content = models.TextField() diff --git a/learning_assistant/serializers.py b/learning_assistant/serializers.py index a212654..62141ef 100644 --- a/learning_assistant/serializers.py +++ b/learning_assistant/serializers.py @@ -3,6 +3,8 @@ """ from rest_framework import serializers +from learning_assistant.models import LearningAssistantMessage + class MessageSerializer(serializers.Serializer): # pylint: disable=abstract-method """ @@ -16,7 +18,7 @@ def validate_role(self, value): """ Validate that role is one of two acceptable values. """ - valid_roles = ['user', 'assistant'] + valid_roles = [LearningAssistantMessage.USER_ROLE, LearningAssistantMessage.ASSISTANT_ROLE] if value not in valid_roles: raise serializers.ValidationError('Must be valid role.') return value diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 68b1c0a..aa9dc26 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -4,6 +4,7 @@ import logging from django.conf import settings +from django.contrib.auth import get_user_model from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey @@ -26,10 +27,12 @@ learning_assistant_enabled, render_prompt_template, ) +from learning_assistant.models import LearningAssistantMessage from learning_assistant.serializers import MessageSerializer from learning_assistant.utils import get_chat_response, user_role_is_staff log = logging.getLogger(__name__) +User = get_user_model() class CourseChatView(APIView): @@ -81,6 +84,15 @@ def post(self, request, course_run_id): unit_id = request.query_params.get('unit_id') message_list = request.data + + # Check that the last message in the list corresponds to a user + new_user_message = message_list[-1] + if new_user_message['role'] != LearningAssistantMessage.USER_ROLE: + return Response( + status=http_status.HTTP_400_BAD_REQUEST, + data={'detail': "Expects user role on last message."} + ) + serializer = MessageSerializer(data=message_list, many=True) # serializer will not be valid in the case that the message list contains any roles other than @@ -108,6 +120,22 @@ def post(self, request, course_run_id): ) status_code, message = get_chat_response(prompt_template, message_list) + user = User.objects.get(id=request.user.id) # Based on the previous code, user exists. + + # Save the user message to the database. + LearningAssistantMessage.objects.create( + user=user, + role=LearningAssistantMessage.USER_ROLE, + content=new_user_message['content'], + ) + + # Save the assistant response to the database. + LearningAssistantMessage.objects.create( + user=user, + role=LearningAssistantMessage.ASSISTANT_ROLE, + content=message['content'], + ) + return Response(status=status_code, data=message) diff --git a/tests/test_views.py b/tests/test_views.py index 492a10d..35e90d3 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -173,7 +173,8 @@ def test_chat_response_default( test_data = [ {'role': 'user', 'content': 'What is 2+2?'}, - {'role': 'assistant', 'content': 'It is 4'} + {'role': 'assistant', 'content': 'It is 4'}, + {'role': 'user', 'content': 'And what else?'}, ] response = self.client.post( @@ -181,8 +182,20 @@ def test_chat_response_default( data=json.dumps(test_data), content_type='application/json' ) + self.assertEqual(response.status_code, 200) + last_rows = LearningAssistantMessage.objects.all().order_by('-created').values()[:2][::-1] + + user_msg = last_rows[0] + assistant_msg = last_rows[1] + + self.assertEqual(user_msg['role'], LearningAssistantMessage.USER_ROLE) + self.assertEqual(user_msg['content'], test_data[2]['content']) + + self.assertEqual(assistant_msg['role'], LearningAssistantMessage.ASSISTANT_ROLE) + self.assertEqual(assistant_msg['content'], 'Something else') + render_args = mock_render.call_args.args self.assertIn(test_unit_id, render_args) self.assertIn('This is the default template', render_args) From c419fdefefd96045d68f610956e6cf91462adcce Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 30 Oct 2024 12:23:49 -0300 Subject: [PATCH 05/22] chore: Moved code to save the message to its own method --- learning_assistant/views.py | 39 ++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/learning_assistant/views.py b/learning_assistant/views.py index aa9dc26..792aee3 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -43,6 +43,27 @@ class CourseChatView(APIView): authentication_classes = (SessionAuthentication, JwtAuthentication,) permission_classes = (IsAuthenticated,) + def __save_user_interaction(self, user_id, user_message, assistant_message): + """ + Saves the last question/response to the database. + """ + user = User.objects.get(id=user_id) + + # Save the user message to the database. + LearningAssistantMessage.objects.create( + user=user, + role=LearningAssistantMessage.USER_ROLE, + content=user_message, + ) + + # Save the assistant response to the database. + LearningAssistantMessage.objects.create( + user=user, + role=LearningAssistantMessage.ASSISTANT_ROLE, + content=assistant_message, + ) + + def post(self, request, course_run_id): """ Given a course run ID, retrieve a chat response for that course. @@ -120,20 +141,10 @@ def post(self, request, course_run_id): ) status_code, message = get_chat_response(prompt_template, message_list) - user = User.objects.get(id=request.user.id) # Based on the previous code, user exists. - - # Save the user message to the database. - LearningAssistantMessage.objects.create( - user=user, - role=LearningAssistantMessage.USER_ROLE, - content=new_user_message['content'], - ) - - # Save the assistant response to the database. - LearningAssistantMessage.objects.create( - user=user, - role=LearningAssistantMessage.ASSISTANT_ROLE, - content=message['content'], + self.__save_user_interaction( + user_id=request.user.id, + user_message=new_user_message['content'], + assistant_message=message['content'] ) return Response(status=status_code, data=message) From 79917a5863ee026ff794f2c4cbf0cc744fe3c82f Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 30 Oct 2024 15:35:04 -0300 Subject: [PATCH 06/22] feat: Added learning_assistant.enable_chat_history waffle flag --- learning_assistant/toggles.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/learning_assistant/toggles.py b/learning_assistant/toggles.py index 88d5ae6..61d515e 100644 --- a/learning_assistant/toggles.py +++ b/learning_assistant/toggles.py @@ -14,6 +14,16 @@ # .. toggle_tickets: COSMO-80 ENABLE_COURSE_CONTENT = 'enable_course_content' +# .. toggle_name: learning_assistant.enable_chat_history +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Waffle flag to enable the chat history with the learning assistant +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2024-10-30 +# .. toggle_target_removal_date: 2024-12-31 +# .. toggle_tickets: COSMO-436 +ENABLE_CHAT_HISTORY = 'enable_chat_history' + def _is_learning_assistant_waffle_flag_enabled(flag_name, course_key): """ @@ -32,3 +42,10 @@ def course_content_enabled(course_key): Return whether the learning_assistant.enable_course_content WaffleFlag is on. """ return _is_learning_assistant_waffle_flag_enabled(ENABLE_COURSE_CONTENT, course_key) + + +def chat_history_enabled(course_key): + """ + Return whether the learning_assistant.enable_chat_history WaffleFlag is on. + """ + return _is_learning_assistant_waffle_flag_enabled(ENABLE_CHAT_HISTORY, course_key) From dae5df8a781347a5df4080823fa7fb695c02df08 Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 30 Oct 2024 15:36:05 -0300 Subject: [PATCH 07/22] feat: Added save_chat_message() to API --- learning_assistant/api.py | 20 ++++++++++++++++++++ tests/test_api.py | 26 +++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/learning_assistant/api.py b/learning_assistant/api.py index 0790059..7ddaa4b 100644 --- a/learning_assistant/api.py +++ b/learning_assistant/api.py @@ -4,6 +4,7 @@ import logging from django.conf import settings +from django.contrib.auth import get_user_model from django.core.cache import cache from edx_django_utils.cache import get_cache_key from jinja2 import BaseLoader, Environment @@ -24,6 +25,7 @@ from learning_assistant.text_utils import html_to_text log = logging.getLogger(__name__) +User = get_user_model() def _extract_block_contents(child, category): @@ -188,6 +190,24 @@ def get_course_id(course_run_id): course_key = course_data['course'] return course_key +def save_chat_message(user_id, chat_role, message): + """ + Saves the chat message to the database. + """ + + user = None + try: + user = User.objects.get(id=user_id) + except User.DoesNotExist: + raise Exception("User does not exists.") + + # Save the user message to the database. + LearningAssistantMessage.objects.create( + user=user, + role=chat_role, + content=message, + ) + def get_message_history(course_id, user, message_count): """ diff --git a/tests/test_api.py b/tests/test_api.py index 3ca365d..a2f51c3 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -21,13 +21,13 @@ learning_assistant_available, learning_assistant_enabled, render_prompt_template, + save_chat_message, set_learning_assistant_enabled, ) from learning_assistant.data import LearningAssistantCourseEnabledData from learning_assistant.models import LearningAssistantCourseEnabled, LearningAssistantMessage fake_transcript = 'This is the text version from the transcript' - User = get_user_model() @@ -235,6 +235,30 @@ def test_render_prompt_template_invalid_unit_key(self, mock_get_content): self.assertNotIn('The following text is useful.', prompt_text) +@ddt.ddt +class TestLearningAssistantCourseEnabledApi(TestCase): + """ + Test suite for save_chat_message. + """ + def setUp(self): + super().setUp() + + self.test_user = User.objects.create(username='username', password='password') + + @ddt.data( + (LearningAssistantMessage.USER_ROLE, 'What is the meaning of life, the universe and everything?'), + (LearningAssistantMessage.ASSISTANT_ROLE, '42'), + ) + @ddt.unpack + def test_save_chat_message(self, chat_role, message): + save_chat_message(self.test_user.id, chat_role, message) + + row = LearningAssistantMessage.objects.all().last() + + self.assertEqual(row.role, chat_role) + self.assertEqual(row.content, message) + + @ddt.ddt class LearningAssistantCourseEnabledApiTests(TestCase): From 23fd0131484c975cb8fab81b1276010820213534 Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 30 Oct 2024 17:53:09 -0300 Subject: [PATCH 08/22] chore: Fixed coverage for CourseChatView --- learning_assistant/api.py | 8 +++--- learning_assistant/views.py | 40 ++++++++--------------------- tests/test_api.py | 2 +- tests/test_views.py | 50 ++++++++++++++++++++++++------------- 4 files changed, 48 insertions(+), 52 deletions(-) diff --git a/learning_assistant/api.py b/learning_assistant/api.py index 7ddaa4b..e6fa870 100644 --- a/learning_assistant/api.py +++ b/learning_assistant/api.py @@ -190,16 +190,16 @@ def get_course_id(course_run_id): course_key = course_data['course'] return course_key + def save_chat_message(user_id, chat_role, message): """ - Saves the chat message to the database. + Save the chat message to the database. """ - user = None try: user = User.objects.get(id=user_id) - except User.DoesNotExist: - raise Exception("User does not exists.") + except User.DoesNotExist as exc: + raise Exception("User does not exists.") from exc # Save the user message to the database. LearningAssistantMessage.objects.create( diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 792aee3..44eb8d0 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -4,7 +4,6 @@ import logging from django.conf import settings -from django.contrib.auth import get_user_model from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey @@ -26,13 +25,14 @@ get_message_history, learning_assistant_enabled, render_prompt_template, + save_chat_message, ) from learning_assistant.models import LearningAssistantMessage from learning_assistant.serializers import MessageSerializer +from learning_assistant.toggles import chat_history_enabled from learning_assistant.utils import get_chat_response, user_role_is_staff log = logging.getLogger(__name__) -User = get_user_model() class CourseChatView(APIView): @@ -43,27 +43,6 @@ class CourseChatView(APIView): authentication_classes = (SessionAuthentication, JwtAuthentication,) permission_classes = (IsAuthenticated,) - def __save_user_interaction(self, user_id, user_message, assistant_message): - """ - Saves the last question/response to the database. - """ - user = User.objects.get(id=user_id) - - # Save the user message to the database. - LearningAssistantMessage.objects.create( - user=user, - role=LearningAssistantMessage.USER_ROLE, - content=user_message, - ) - - # Save the assistant response to the database. - LearningAssistantMessage.objects.create( - user=user, - role=LearningAssistantMessage.ASSISTANT_ROLE, - content=assistant_message, - ) - - def post(self, request, course_run_id): """ Given a course run ID, retrieve a chat response for that course. @@ -114,6 +93,12 @@ def post(self, request, course_run_id): data={'detail': "Expects user role on last message."} ) + course_id = get_course_id(course_run_id) + user_id = request.user.id + + if chat_history_enabled(course_id): + save_chat_message(user_id, LearningAssistantMessage.USER_ROLE, new_user_message['content']) + serializer = MessageSerializer(data=message_list, many=True) # serializer will not be valid in the case that the message list contains any roles other than @@ -132,8 +117,6 @@ def post(self, request, course_run_id): } ) - course_id = get_course_id(course_run_id) - template_string = getattr(settings, 'LEARNING_ASSISTANT_PROMPT_TEMPLATE', '') prompt_template = render_prompt_template( @@ -141,11 +124,8 @@ def post(self, request, course_run_id): ) status_code, message = get_chat_response(prompt_template, message_list) - self.__save_user_interaction( - user_id=request.user.id, - user_message=new_user_message['content'], - assistant_message=message['content'] - ) + if chat_history_enabled(course_id): + save_chat_message(user_id, LearningAssistantMessage.ASSISTANT_ROLE, message['content']) return Response(status=status_code, data=message) diff --git a/tests/test_api.py b/tests/test_api.py index a2f51c3..6969af8 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -235,6 +235,7 @@ def test_render_prompt_template_invalid_unit_key(self, mock_get_content): self.assertNotIn('The following text is useful.', prompt_text) + @ddt.ddt class TestLearningAssistantCourseEnabledApi(TestCase): """ @@ -259,7 +260,6 @@ def test_save_chat_message(self, chat_role, message): self.assertEqual(row.content, message) - @ddt.ddt class LearningAssistantCourseEnabledApiTests(TestCase): """ diff --git a/tests/test_views.py b/tests/test_views.py index 35e90d3..c521a30 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -4,7 +4,7 @@ import json import sys from importlib import import_module -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, call, patch import ddt from django.conf import settings @@ -19,7 +19,7 @@ User = get_user_model() -class TestClient(Client): +class FakeClient(Client): """ Allows for 'fake logins' of a user so we don't need to expose a 'login' HTTP endpoint. """ @@ -66,14 +66,14 @@ def setUp(self): Setup for tests. """ super().setUp() - self.client = TestClient() + self.client = FakeClient() self.user = User(username='tester', email='tester@test.com', is_staff=True) self.user.save() self.client.login_user(self.user) @ddt.ddt -class CourseChatViewTests(LoggedInTestCase): +class TestCourseChatView(LoggedInTestCase): """ Test for the CourseChatView """ @@ -153,15 +153,27 @@ def test_invalid_messages(self, mock_role, mock_waffle, mock_render): ) self.assertEqual(response.status_code, 400) + @ddt.data(True, False) # TODO: Fix this - See below. @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.get_enrollment') @patch('learning_assistant.views.CourseMode') + @patch('learning_assistant.api.save_chat_message') + @patch('learning_assistant.toggles.chat_history_enabled') @override_settings(LEARNING_ASSISTANT_PROMPT_TEMPLATE='This is the default template') def test_chat_response_default( - self, mock_mode, mock_enrollment, mock_role, mock_waffle, mock_chat_response, mock_render + self, + enabled_flag, + mock_chat_history_enabled, + mock_save_chat_message, + mock_mode, + mock_enrollment, + mock_role, + mock_waffle, + mock_chat_response, + mock_render, ): mock_waffle.return_value = True mock_role.return_value = 'student' @@ -171,6 +183,14 @@ def test_chat_response_default( mock_render.return_value = 'Rendered template mock' test_unit_id = 'test-unit-id' + # TODO: Fix this... + # For some reason this only works the first time. The 2nd time (enabled_flag = False) + # Doesn't actually work since the mocked chat_history_enabled() will return False no matter what. + # Swap the order of the @ddt.data() above by: @ddt.data(False, True) and watch it fail. + # The value for enabled_flag is corrct on this scope, but the mocked method doesn't update. + # It even happens if we split the test cases into two different methods. + mock_chat_history_enabled.return_value = enabled_flag + test_data = [ {'role': 'user', 'content': 'What is 2+2?'}, {'role': 'assistant', 'content': 'It is 4'}, @@ -182,20 +202,8 @@ def test_chat_response_default( data=json.dumps(test_data), content_type='application/json' ) - self.assertEqual(response.status_code, 200) - last_rows = LearningAssistantMessage.objects.all().order_by('-created').values()[:2][::-1] - - user_msg = last_rows[0] - assistant_msg = last_rows[1] - - self.assertEqual(user_msg['role'], LearningAssistantMessage.USER_ROLE) - self.assertEqual(user_msg['content'], test_data[2]['content']) - - self.assertEqual(assistant_msg['role'], LearningAssistantMessage.ASSISTANT_ROLE) - self.assertEqual(assistant_msg['content'], 'Something else') - render_args = mock_render.call_args.args self.assertIn(test_unit_id, render_args) self.assertIn('This is the default template', render_args) @@ -205,6 +213,14 @@ def test_chat_response_default( test_data, ) + if enabled_flag: + mock_save_chat_message.assert_has_calls([ + call(self.user.id, LearningAssistantMessage.USER_ROLE, test_data[-1]['content']), + call(self.user.id, LearningAssistantMessage.ASSISTANT_ROLE, 'Something else') + ]) + else: + mock_save_chat_message.assert_not_called() + @ddt.ddt class LearningAssistantEnabledViewTests(LoggedInTestCase): From 352340ea71c5ec7fae32cee3346827755e25a828 Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 31 Oct 2024 13:25:21 -0300 Subject: [PATCH 09/22] fix: Swapped course id for course key --- learning_assistant/views.py | 7 ++++--- tests/test_views.py | 12 +++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 44eb8d0..3e136fe 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -93,10 +93,9 @@ def post(self, request, course_run_id): data={'detail': "Expects user role on last message."} ) - course_id = get_course_id(course_run_id) user_id = request.user.id - if chat_history_enabled(course_id): + if chat_history_enabled(courserun_key): save_chat_message(user_id, LearningAssistantMessage.USER_ROLE, new_user_message['content']) serializer = MessageSerializer(data=message_list, many=True) @@ -117,6 +116,8 @@ def post(self, request, course_run_id): } ) + course_id = get_course_id(course_run_id) + template_string = getattr(settings, 'LEARNING_ASSISTANT_PROMPT_TEMPLATE', '') prompt_template = render_prompt_template( @@ -124,7 +125,7 @@ def post(self, request, course_run_id): ) status_code, message = get_chat_response(prompt_template, message_list) - if chat_history_enabled(course_id): + if chat_history_enabled(courserun_key): save_chat_message(user_id, LearningAssistantMessage.ASSISTANT_ROLE, message['content']) return Response(status=status_code, data=message) diff --git a/tests/test_views.py b/tests/test_views.py index c521a30..4e791b7 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -184,11 +184,13 @@ def test_chat_response_default( test_unit_id = 'test-unit-id' # TODO: Fix this... - # For some reason this only works the first time. The 2nd time (enabled_flag = False) - # Doesn't actually work since the mocked chat_history_enabled() will return False no matter what. - # Swap the order of the @ddt.data() above by: @ddt.data(False, True) and watch it fail. - # The value for enabled_flag is corrct on this scope, but the mocked method doesn't update. - # It even happens if we split the test cases into two different methods. + # For some reason this assignment only works the first iteration. The 2nd time onwards the return value is + # always falsy. Swap the order of the @ddt.data() above by: @ddt.data(False, True) to see it fail. + # I'm leaving it like this because we are testing the False return in the second iteration, but it's important + # to consider whenever this test needs to be updated. + # It even happens if we split the test cases into two different methods (instead of @ddt.data()), so there's + # probably some scoping issues in how the test is set up. + # Note: There's a similar test for LearningAssistantEnabledView in this file that works just fine. mock_chat_history_enabled.return_value = enabled_flag test_data = [ From 7cdc76331a83a515f91ad6b1b2a88810775fa570 Mon Sep 17 00:00:00 2001 From: Marcos Date: Mon, 4 Nov 2024 11:16:58 -0300 Subject: [PATCH 10/22] chore: Fixed patching on the CourseChatView unit tests --- tests/test_views.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/tests/test_views.py b/tests/test_views.py index 4e791b7..9e07e55 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -153,15 +153,15 @@ def test_invalid_messages(self, mock_role, mock_waffle, mock_render): ) self.assertEqual(response.status_code, 400) - @ddt.data(True, False) # TODO: Fix this - See below. + @ddt.data(False, True) @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.get_enrollment') @patch('learning_assistant.views.CourseMode') - @patch('learning_assistant.api.save_chat_message') - @patch('learning_assistant.toggles.chat_history_enabled') + @patch('learning_assistant.views.save_chat_message') + @patch('learning_assistant.views.chat_history_enabled') @override_settings(LEARNING_ASSISTANT_PROMPT_TEMPLATE='This is the default template') def test_chat_response_default( self, @@ -183,14 +183,6 @@ def test_chat_response_default( mock_render.return_value = 'Rendered template mock' test_unit_id = 'test-unit-id' - # TODO: Fix this... - # For some reason this assignment only works the first iteration. The 2nd time onwards the return value is - # always falsy. Swap the order of the @ddt.data() above by: @ddt.data(False, True) to see it fail. - # I'm leaving it like this because we are testing the False return in the second iteration, but it's important - # to consider whenever this test needs to be updated. - # It even happens if we split the test cases into two different methods (instead of @ddt.data()), so there's - # probably some scoping issues in how the test is set up. - # Note: There's a similar test for LearningAssistantEnabledView in this file that works just fine. mock_chat_history_enabled.return_value = enabled_flag test_data = [ From f0b0cf8d3ec4393d4b1aeae7bdd9519c94a65864 Mon Sep 17 00:00:00 2001 From: Marcos Date: Mon, 4 Nov 2024 11:57:09 -0300 Subject: [PATCH 11/22] fix: Added migration after updating role in model --- ...0008_alter_learningassistantmessage_role.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 learning_assistant/migrations/0008_alter_learningassistantmessage_role.py diff --git a/learning_assistant/migrations/0008_alter_learningassistantmessage_role.py b/learning_assistant/migrations/0008_alter_learningassistantmessage_role.py new file mode 100644 index 0000000..bd699b3 --- /dev/null +++ b/learning_assistant/migrations/0008_alter_learningassistantmessage_role.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.14 on 2024-11-04 08:52 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('learning_assistant', '0007_learningassistantmessage'), + ] + + operations = [ + migrations.AlterField( + model_name='learningassistantmessage', + name='role', + field=models.CharField(choices=[('user', 'user'), ('assistant', 'assistant')], max_length=64), + ), + ] From 4fa9bf5148bc68dc6962bb935ea39f33ecb0b181 Mon Sep 17 00:00:00 2001 From: Marcos Date: Mon, 4 Nov 2024 15:23:31 -0300 Subject: [PATCH 12/22] fix: Added course run key to save_chat_message() --- learning_assistant/api.py | 4 +++- learning_assistant/views.py | 4 ++-- tests/test_api.py | 4 +++- tests/test_views.py | 6 ++++-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/learning_assistant/api.py b/learning_assistant/api.py index e6fa870..eaf4b10 100644 --- a/learning_assistant/api.py +++ b/learning_assistant/api.py @@ -191,7 +191,7 @@ def get_course_id(course_run_id): return course_key -def save_chat_message(user_id, chat_role, message): +def save_chat_message(courserun_key, user_id, chat_role, message): """ Save the chat message to the database. """ @@ -203,9 +203,11 @@ def save_chat_message(user_id, chat_role, message): # Save the user message to the database. LearningAssistantMessage.objects.create( + course_id=courserun_key, user=user, role=chat_role, content=message, + ) diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 3e136fe..583aed1 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -96,7 +96,7 @@ def post(self, request, course_run_id): user_id = request.user.id if chat_history_enabled(courserun_key): - save_chat_message(user_id, LearningAssistantMessage.USER_ROLE, new_user_message['content']) + save_chat_message(courserun_key, user_id, LearningAssistantMessage.USER_ROLE, new_user_message['content']) serializer = MessageSerializer(data=message_list, many=True) @@ -126,7 +126,7 @@ def post(self, request, course_run_id): status_code, message = get_chat_response(prompt_template, message_list) if chat_history_enabled(courserun_key): - save_chat_message(user_id, LearningAssistantMessage.ASSISTANT_ROLE, message['content']) + save_chat_message(courserun_key, user_id, LearningAssistantMessage.ASSISTANT_ROLE, message['content']) return Response(status=status_code, data=message) diff --git a/tests/test_api.py b/tests/test_api.py index 6969af8..0470e32 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -245,6 +245,7 @@ def setUp(self): super().setUp() self.test_user = User.objects.create(username='username', password='password') + self.course_run_key = CourseKey.from_string('course-v1:edx+test+23') @ddt.data( (LearningAssistantMessage.USER_ROLE, 'What is the meaning of life, the universe and everything?'), @@ -252,10 +253,11 @@ def setUp(self): ) @ddt.unpack def test_save_chat_message(self, chat_role, message): - save_chat_message(self.test_user.id, chat_role, message) + save_chat_message(self.course_run_key, self.test_user.id, chat_role, message) row = LearningAssistantMessage.objects.all().last() + self.assertEqual(row.course_id, self.course_run_key) self.assertEqual(row.role, chat_role) self.assertEqual(row.content, message) diff --git a/tests/test_views.py b/tests/test_views.py index 9e07e55..5125d27 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -13,6 +13,7 @@ from django.test import TestCase, override_settings from django.test.client import Client from django.urls import reverse +from opaque_keys.edx.keys import CourseKey from learning_assistant.models import LearningAssistantMessage @@ -85,6 +86,7 @@ class TestCourseChatView(LoggedInTestCase): def setUp(self): super().setUp() 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', @@ -209,8 +211,8 @@ def test_chat_response_default( if enabled_flag: mock_save_chat_message.assert_has_calls([ - call(self.user.id, LearningAssistantMessage.USER_ROLE, test_data[-1]['content']), - call(self.user.id, LearningAssistantMessage.ASSISTANT_ROLE, 'Something else') + call(self.course_run_key, self.user.id, LearningAssistantMessage.USER_ROLE, test_data[-1]['content']), + call(self.course_run_key, self.user.id, LearningAssistantMessage.ASSISTANT_ROLE, 'Something else') ]) else: mock_save_chat_message.assert_not_called() From 7b8425d2a9ee033da96279681730285e85ed94a4 Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 6 Nov 2024 13:03:10 -0300 Subject: [PATCH 13/22] fix: Fixed package version --- learning_assistant/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/learning_assistant/__init__.py b/learning_assistant/__init__.py index fb90252..ea4266b 100644 --- a/learning_assistant/__init__.py +++ b/learning_assistant/__init__.py @@ -2,6 +2,6 @@ Plugin for a learning assistant backend, intended for use within edx-platform. """ -__version__ = '4.4.1' +__version__ = '4.4.3' default_app_config = 'learning_assistant.apps.LearningAssistantConfig' # pylint: disable=invalid-name From d2d5808c61b892570f26b7c7b81dae7477a64a3c Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 6 Nov 2024 16:37:48 -0300 Subject: [PATCH 14/22] fix: Fixed Couseware History fetching --- learning_assistant/api.py | 6 +++--- learning_assistant/views.py | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/learning_assistant/api.py b/learning_assistant/api.py index eaf4b10..272e201 100644 --- a/learning_assistant/api.py +++ b/learning_assistant/api.py @@ -211,12 +211,12 @@ def save_chat_message(courserun_key, user_id, chat_role, message): ) -def get_message_history(course_id, user, message_count): +def get_message_history(courserun_key, user, message_count): """ - Given a course run id (str), user (User), and message count (int), return the associated message history. + Given a courserun key (CourseKey), user (User), and message count (int), return the associated message history. Returns a number of messages equal to the message_count value. """ message_history = LearningAssistantMessage.objects.filter( - course_id=course_id, user=user).order_by('-created')[:message_count] + course_id=courserun_key, user=user).order_by('-created')[:message_count] return message_history diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 583aed1..b1b5c03 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -233,10 +233,9 @@ def get(self, request, course_run_id): data={'detail': 'Must be staff or have valid enrollment.'} ) - course_id = get_course_id(course_run_id) user = request.user message_count = int(request.GET.get('message_count', 50)) - message_history = get_message_history(course_id, user, message_count) + 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) From b56f1461c721e571ffb7294b41a626f81924a32c Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 6 Nov 2024 16:51:05 -0300 Subject: [PATCH 15/22] chore: Updated tests --- tests/test_api.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 0470e32..8b66845 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -347,8 +347,7 @@ class GetMessageHistoryTests(TestCase): def setUp(self): super().setUp() - self.course_id = 'course-v1:edx+fake+1' - self.course_key = CourseKey.from_string(self.course_id) + self.course_key = CourseKey.from_string('course-v1:edx+fake+1') self.user = User(username='tester', email='tester@test.com') self.user.save() @@ -358,16 +357,16 @@ def test_get_message_history(self): message_count = 5 for i in range(1, message_count + 1): LearningAssistantMessage.objects.create( - course_id=self.course_id, + course_id=self.course_key, user=self.user, role=self.role, content=f'Content of message {i}', ) - return_value = get_message_history(self.course_id, self.user, message_count) + return_value = get_message_history(self.course_key, self.user, message_count) expected_value = LearningAssistantMessage.objects.filter( - course_id=self.course_id, user=self.user).order_by('-created')[:message_count] + course_id=self.course_key, user=self.user).order_by('-created')[:message_count] # Ensure same number of entries self.assertEqual(len(return_value), len(expected_value)) @@ -385,17 +384,17 @@ def test_get_message_history(self): def test_get_message_history_message_count(self, actual_message_count): for i in range(1, actual_message_count + 1): LearningAssistantMessage.objects.create( - course_id=self.course_id, + course_id=self.course_key, user=self.user, role=self.role, content=f'Content of message {i}', ) message_count_parameter = 5 - return_value = get_message_history(self.course_id, self.user, message_count_parameter) + return_value = get_message_history(self.course_key, self.user, message_count_parameter) expected_value = LearningAssistantMessage.objects.filter( - course_id=self.course_id, user=self.user).order_by('-created')[:message_count_parameter] + course_id=self.course_key, user=self.user).order_by('-created')[:message_count_parameter] # Ensure same number of entries self.assertEqual(len(return_value), len(expected_value)) @@ -403,7 +402,7 @@ def test_get_message_history_message_count(self, actual_message_count): def test_get_message_history_user_difference(self): # Default Message LearningAssistantMessage.objects.create( - course_id=self.course_id, + course_id=self.course_key, user=self.user, role=self.role, content='Expected content of message', @@ -413,17 +412,17 @@ def test_get_message_history_user_difference(self): new_user = User(username='not_tester', email='not_tester@test.com') new_user.save() LearningAssistantMessage.objects.create( - course_id=self.course_id, + course_id=self.course_key, user=new_user, role=self.role, content='Expected content of message', ) message_count = 2 - return_value = get_message_history(self.course_id, self.user, message_count) + return_value = get_message_history(self.course_key, self.user, message_count) expected_value = LearningAssistantMessage.objects.filter( - course_id=self.course_id, user=self.user).order_by('-created')[:message_count] + course_id=self.course_key, user=self.user).order_by('-created')[:message_count] # Ensure we filtered one of the two present messages self.assertNotEqual(len(return_value), LearningAssistantMessage.objects.count()) @@ -441,7 +440,7 @@ def test_get_message_history_user_difference(self): def test_get_message_course_id_differences(self): # Default Message LearningAssistantMessage.objects.create( - course_id=self.course_id, + course_id=self.course_key, user=self.user, role=self.role, content='Expected content of message', @@ -457,10 +456,10 @@ def test_get_message_course_id_differences(self): ) message_count = 2 - return_value = get_message_history(self.course_id, self.user, message_count) + return_value = get_message_history(self.course_key, self.user, message_count) expected_value = LearningAssistantMessage.objects.filter( - course_id=self.course_id, user=self.user).order_by('-created')[:message_count] + course_id=self.course_key, user=self.user).order_by('-created')[:message_count] # Ensure we filtered one of the two present messages self.assertNotEqual(len(return_value), LearningAssistantMessage.objects.count()) From 510303bfea08a675b57ed1ff608b74ec363729b7 Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 6 Nov 2024 17:50:01 -0300 Subject: [PATCH 16/22] feat: Added timestamp to Learning Assistant History response --- learning_assistant/serializers.py | 13 +++++++++++++ tests/test_views.py | 1 + 2 files changed, 14 insertions(+) diff --git a/learning_assistant/serializers.py b/learning_assistant/serializers.py index 62141ef..1896182 100644 --- a/learning_assistant/serializers.py +++ b/learning_assistant/serializers.py @@ -13,6 +13,19 @@ class MessageSerializer(serializers.Serializer): # pylint: disable=abstract-met role = serializers.CharField(required=True) content = serializers.CharField(required=True) + timestamp = serializers.DateTimeField(required=False, source='created') + + class Meta: + """ + Serializer metadata. + """ + + model = LearningAssistantMessage + fields = ( + 'role', + 'content', + 'timestamp', + ) def validate_role(self, value): """ diff --git a/tests/test_views.py b/tests/test_views.py index 5125d27..22b1cf0 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -365,3 +365,4 @@ def test_learning_message_history_view_get( actual_message = LearningAssistantMessage.objects.get(course_id=self.course_id) self.assertEqual(data[0]['role'], actual_message.role) self.assertEqual(data[0]['content'], actual_message.content) + self.assertEqual(data[0]['timestamp'], actual_message.created.isoformat()) From 27e4f8b8e58c397abebbe7e8ad278c7b1790b6ad Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 7 Nov 2024 12:24:27 -0300 Subject: [PATCH 17/22] chore: Release bump to v4.4.4 --- CHANGELOG.rst | 13 +++++++++++++ learning_assistant/__init__.py | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index baed526..ad32abb 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,19 @@ Change Log Unreleased ********** +4.4.4 - 2024-11-06 +****************** +* Fixed Learning Assistant History endpoint +* Added timestamp to the Learning Assistant History payload + +4.4.3 - 2024-11-06 +****************** +* Fixed package version + +4.4.2 - 2024-11-04 +****************** +* Added chat messages to the DB + 4.4.1 - 2024-10-31 ****************** * Add management command to remove expired messages diff --git a/learning_assistant/__init__.py b/learning_assistant/__init__.py index ea4266b..27b6a0f 100644 --- a/learning_assistant/__init__.py +++ b/learning_assistant/__init__.py @@ -2,6 +2,6 @@ Plugin for a learning assistant backend, intended for use within edx-platform. """ -__version__ = '4.4.3' +__version__ = '4.4.4' default_app_config = 'learning_assistant.apps.LearningAssistantConfig' # pylint: disable=invalid-name From cc51613ac08fd328e1811e78322d80fa540a6e8a Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 8 Nov 2024 12:57:23 -0300 Subject: [PATCH 18/22] fix: Updated Learning Assistant History payload to return in asc order --- CHANGELOG.rst | 1 + learning_assistant/views.py | 2 +- tests/test_views.py | 30 +++++++++++++++++++++--------- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ad32abb..910051e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,7 @@ Change Log Unreleased ********** +* Updated Learning Assistant History payload to return in ascending order 4.4.4 - 2024-11-06 ****************** diff --git a/learning_assistant/views.py b/learning_assistant/views.py index b1b5c03..02c3fca 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -236,6 +236,6 @@ def get(self, request, course_run_id): user = request.user message_count = int(request.GET.get('message_count', 50)) - message_history = get_message_history(courserun_key, user, message_count) + message_history = reversed(list(get_message_history(courserun_key, user, message_count))) # Reversing order data = MessageSerializer(message_history, many=True).data return Response(status=http_status.HTTP_200_OK, data=data) diff --git a/tests/test_views.py b/tests/test_views.py index 22b1cf0..42ad0b3 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1,6 +1,7 @@ """ Tests for the learning assistant views. """ +import datetime import json import sys from importlib import import_module @@ -347,22 +348,33 @@ def test_learning_message_history_view_get( course_id=self.course_id, user=self.user, role='staff', - content='Expected content of message', + content='Older message', + created=datetime.date(2024, 10, 1) ) - message_count = 1 + + LearningAssistantMessage.objects.create( + course_id=self.course_id, + user=self.user, + role='staff', + content='Newer message', + created=datetime.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={message_count}', + 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 - actual_message_count = LearningAssistantMessage.objects.count() - self.assertEqual(len(data), actual_message_count) + self.assertEqual(len(data), db_messages_count) # Ensure values are as expected - actual_message = LearningAssistantMessage.objects.get(course_id=self.course_id) - self.assertEqual(data[0]['role'], actual_message.role) - self.assertEqual(data[0]['content'], actual_message.content) - self.assertEqual(data[0]['timestamp'], actual_message.created.isoformat()) + for i, expected in enumerate(db_messages): + self.assertEqual(expected.role, data[i]['role']) + self.assertEqual(expected.content, data[i]['content']) + self.assertEqual(expected.created.isoformat(), data[i]['timestamp']) From 2f93c7ee2f9af9830d03f14b3ca2a45ffbaeed08 Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 8 Nov 2024 17:10:04 -0300 Subject: [PATCH 19/22] chore: Updated the place where the list is reversed --- learning_assistant/api.py | 4 ++-- learning_assistant/views.py | 2 +- tests/test_api.py | 4 ++-- tests/test_views.py | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/learning_assistant/api.py b/learning_assistant/api.py index 272e201..2006b26 100644 --- a/learning_assistant/api.py +++ b/learning_assistant/api.py @@ -217,6 +217,6 @@ def get_message_history(courserun_key, user, message_count): Returns a number of messages equal to the message_count value. """ - message_history = LearningAssistantMessage.objects.filter( - course_id=courserun_key, user=user).order_by('-created')[:message_count] + message_history = list(LearningAssistantMessage.objects.filter( + course_id=courserun_key, user=user).order_by('-created')[:message_count])[::-1] return message_history diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 02c3fca..b1b5c03 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -236,6 +236,6 @@ def get(self, request, course_run_id): user = request.user message_count = int(request.GET.get('message_count', 50)) - message_history = reversed(list(get_message_history(courserun_key, user, message_count))) # Reversing order + 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) diff --git a/tests/test_api.py b/tests/test_api.py index 8b66845..1344dea 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -365,8 +365,8 @@ def test_get_message_history(self): return_value = get_message_history(self.course_key, self.user, message_count) - expected_value = LearningAssistantMessage.objects.filter( - course_id=self.course_key, user=self.user).order_by('-created')[:message_count] + expected_value = list(LearningAssistantMessage.objects.filter( + course_id=self.course_key, user=self.user).order_by('-created')[:message_count])[::-1] # Ensure same number of entries self.assertEqual(len(return_value), len(expected_value)) diff --git a/tests/test_views.py b/tests/test_views.py index 42ad0b3..1d567b8 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -374,7 +374,7 @@ def test_learning_message_history_view_get( self.assertEqual(len(data), db_messages_count) # Ensure values are as expected - for i, expected in enumerate(db_messages): - self.assertEqual(expected.role, data[i]['role']) - self.assertEqual(expected.content, data[i]['content']) - self.assertEqual(expected.created.isoformat(), data[i]['timestamp']) + 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()) From 726bfdaa295c4fc64deb98e43b48d9aee9c498a7 Mon Sep 17 00:00:00 2001 From: Marcos Date: Tue, 12 Nov 2024 10:44:45 -0300 Subject: [PATCH 20/22] chore: Updated to use as 4.4.5 release --- CHANGELOG.rst | 3 +++ learning_assistant/__init__.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 910051e..19aa431 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,9 @@ Change Log Unreleased ********** + +4.4.5 - 2024-11-12 +****************** * Updated Learning Assistant History payload to return in ascending order 4.4.4 - 2024-11-06 diff --git a/learning_assistant/__init__.py b/learning_assistant/__init__.py index 27b6a0f..8ef32fa 100644 --- a/learning_assistant/__init__.py +++ b/learning_assistant/__init__.py @@ -2,6 +2,6 @@ Plugin for a learning assistant backend, intended for use within edx-platform. """ -__version__ = '4.4.4' +__version__ = '4.4.5' default_app_config = 'learning_assistant.apps.LearningAssistantConfig' # pylint: disable=invalid-name From 4e1ac79e2d2ca1d11044275896b67210d2bc67ce Mon Sep 17 00:00:00 2001 From: Marcos Date: Tue, 12 Nov 2024 11:23:43 -0300 Subject: [PATCH 21/22] chore: Added comment on get_message_history() explaining the double inversion. --- learning_assistant/api.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/learning_assistant/api.py b/learning_assistant/api.py index 2006b26..55c73c1 100644 --- a/learning_assistant/api.py +++ b/learning_assistant/api.py @@ -217,6 +217,10 @@ def get_message_history(courserun_key, user, message_count): Returns a number of messages equal to the message_count value. """ + # Explanation over the double reverse: This fetches the last message_count elements ordered by creating order DESC. + # Slicing the list in the model is an equivalent of adding LIMIT on the query. + # The result is the last chat messages for that user and course but in inversed order, so in order to flip them + # its first turn into a list and then reversed. message_history = list(LearningAssistantMessage.objects.filter( course_id=courserun_key, user=user).order_by('-created')[:message_count])[::-1] return message_history From e3709235304bed34ddac3918497791ed517e2fb7 Mon Sep 17 00:00:00 2001 From: Isaac Lee <124631592+ilee2u@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:03:07 -0500 Subject: [PATCH 22/22] feat: add LearningAssistantAuditTrial model (#132) * feat: add LearningAssistantAuditTrial model * chore: Add no_pii annotation * fix: make start_time non-nullable * fix: also revise the migration * fix: replace ghost tests/__init__.py --- .../0009_learningassistantaudittrial.py | 31 +++++++++++++++++++ learning_assistant/models.py | 15 +++++++++ 2 files changed, 46 insertions(+) create mode 100644 learning_assistant/migrations/0009_learningassistantaudittrial.py diff --git a/learning_assistant/migrations/0009_learningassistantaudittrial.py b/learning_assistant/migrations/0009_learningassistantaudittrial.py new file mode 100644 index 0000000..30068a2 --- /dev/null +++ b/learning_assistant/migrations/0009_learningassistantaudittrial.py @@ -0,0 +1,31 @@ +# Generated by Django 4.2.16 on 2024-11-14 13:55 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('learning_assistant', '0008_alter_learningassistantmessage_role'), + ] + + operations = [ + migrations.CreateModel( + name='LearningAssistantAuditTrial', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('start_date', models.DateTimeField()), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL, unique=True)), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/learning_assistant/models.py b/learning_assistant/models.py index 482bfe2..bd05ceb 100644 --- a/learning_assistant/models.py +++ b/learning_assistant/models.py @@ -47,3 +47,18 @@ class LearningAssistantMessage(TimeStampedModel): user = models.ForeignKey(USER_MODEL, db_index=True, on_delete=models.CASCADE) role = models.CharField(choices=Roles, max_length=64) content = models.TextField() + + +class LearningAssistantAuditTrial(TimeStampedModel): + """ + This model stores the trial period for an audit learner using the learning assistant. + + A LearningAssistantAuditTrial instance will be created on a per user basis, + when an audit learner first sends a message using Xpert LA. + + .. no_pii: This model has no PII. + """ + + # Unique constraint since each user should only have one trial + user = models.ForeignKey(USER_MODEL, db_index=True, on_delete=models.CASCADE, unique=True) + start_date = models.DateTimeField()