fix: handle invalid unit ids (#110)

* fix: handle invalid unit ids

* fix: update log level

CHANGELOG.rst

@@ -14,6 +14,10 @@ Change Log
 Unreleased
 **********
 
+4.3.2 - 2024-09-19
+******************
+* Add error handling for invalid unit usage keys
+
 4.3.1 - 2024-09-10
 ******************
 * Remove GPT model field as part of POST request to Xpert backend

learning_assistant/__init__.py

@@ -2,6 +2,6 @@
 Plugin for a learning assistant backend, intended for use within edx-platform.
 """
 
-__version__ = '4.3.1'
+__version__ = '4.3.2'
 
 default_app_config = 'learning_assistant.apps.LearningAssistantConfig'  # pylint: disable=invalid-name

learning_assistant/api.py

@@ -7,7 +7,7 @@
 from django.core.cache import cache
 from edx_django_utils.cache import get_cache_key
 from jinja2 import BaseLoader, Environment
-from opaque_keys.edx.keys import CourseKey
+from opaque_keys import InvalidKeyError
 
 from learning_assistant.constants import ACCEPTED_CATEGORY_TYPES, CATEGORY_TYPE_MAP
 from learning_assistant.data import LearningAssistantCourseEnabledData
@@ -22,7 +22,6 @@
     traverse_block_pre_order,
 )
 from learning_assistant.text_utils import html_to_text
-from learning_assistant.toggles import course_content_enabled
 
 log = logging.getLogger(__name__)
 
@@ -109,9 +108,15 @@ def render_prompt_template(request, user_id, course_run_id, unit_usage_key, cour
     """
     unit_content = ''
 
-    course_run_key = CourseKey.from_string(course_run_id)
-    if unit_usage_key and course_content_enabled(course_run_key):
-        _, unit_content = get_block_content(request, user_id, course_run_id, unit_usage_key)
+    if unit_usage_key:
+        try:
+            _, unit_content = get_block_content(request, user_id, course_run_id, unit_usage_key)
+        except InvalidKeyError:
+            log.warning(
+                'Failed to retrieve course content for course_id=%(course_run_id)s because of '
+                'invalid unit_id=%(unit_usage_key)s',
+                {'course_run_id': course_run_id, 'unit_usage_key': unit_usage_key}
+            )
 
     course_data = get_cache_course_data(course_id, ['skill_names', 'title'])
     skill_names = course_data['skill_names']

tests/test_api.py

@@ -8,6 +8,7 @@
 from django.conf import settings
 from django.core.cache import cache
 from django.test import TestCase, override_settings
+from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey, UsageKey
 
 from learning_assistant.api import (
@@ -178,20 +179,13 @@ def test_get_block_content(self, mock_get_children_contents, mock_get_single_blo
         self.assertEqual(items, content_items)
 
     @ddt.data(
-        ('This is content.', True),
-        ('', True),
-        ('This is content.', False),
-        ('', False),
+        'This is content.',
+        ''
     )
-    @ddt.unpack
     @patch('learning_assistant.api.get_cache_course_data')
-    @patch('learning_assistant.toggles._is_learning_assistant_waffle_flag_enabled')
     @patch('learning_assistant.api.get_block_content')
-    def test_render_prompt_template(
-        self, unit_content, flag_enabled, mock_get_content, mock_is_flag_enabled, mock_cache
-    ):
+    def test_render_prompt_template(self, unit_content, mock_get_content, mock_cache):
         mock_get_content.return_value = (len(unit_content), unit_content)
-        mock_is_flag_enabled.return_value = flag_enabled
         skills_content = ['skills']
         title = 'title'
         mock_cache.return_value = {'skill_names': skills_content, 'title': title}
@@ -209,13 +203,33 @@ def test_render_prompt_template(
             request, user_id, course_run_id, unit_usage_key, course_id, template_string
         )
 
-        if unit_content and flag_enabled:
+        if unit_content:
             self.assertIn(unit_content, prompt_text)
         else:
             self.assertNotIn('The following text is useful.', prompt_text)
         self.assertIn(str(skills_content), prompt_text)
         self.assertIn(title, prompt_text)
 
+    @patch('learning_assistant.api.get_cache_course_data', MagicMock())
+    @patch('learning_assistant.api.get_block_content')
+    def test_render_prompt_template_invalid_unit_key(self, mock_get_content):
+        mock_get_content.side_effect = InvalidKeyError('foo', 'bar')
+
+        # mock arguments that are passed through to `get_block_content` function. the value of these
+        # args does not matter for this test right now, as the `get_block_content` function is entirely mocked.
+        request = MagicMock()
+        user_id = 1
+        course_run_id = self.course_run_id
+        unit_usage_key = 'block-v1:edX+A+B+type@vertical+block@verticalD'
+        course_id = 'edx+test'
+        template_string = getattr(settings, 'LEARNING_ASSISTANT_PROMPT_TEMPLATE', '')
+
+        prompt_text = render_prompt_template(
+            request, user_id, course_run_id, unit_usage_key, course_id, template_string
+        )
+
+        self.assertNotIn('The following text is useful.', prompt_text)
+
 
 @ddt.ddt
 class LearningAssistantCourseEnabledApiTests(TestCase):