Merge pull request #129 from edx/rijuma/fix-message-history-order

fix: Updated Learning Assistant History payload to return in ascending order

CHANGELOG.rst

@@ -14,6 +14,10 @@ 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
 ******************
 * Fixed Learning Assistant History endpoint

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

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.
     """
-    message_history = LearningAssistantMessage.objects.filter(
-        course_id=courserun_key, user=user).order_by('-created')[:message_count]
+    # 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

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))

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, 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())