From cc51613ac08fd328e1811e78322d80fa540a6e8a Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 8 Nov 2024 12:57:23 -0300 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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