feat: ACADEMIC-16210: Several updates based on PR comments

ai_aside/block.py

@@ -9,9 +9,10 @@
 from webob import Response
 from xblock.core import XBlock, XBlockAside
 
-from ai_aside.api.api import is_summary_enabled
+from ai_aside.config_api.api import is_summary_enabled
 from ai_aside.platform_imports import get_block, get_text_transcript
 from ai_aside.text_utils import html_to_text
+from ai_aside.waffle import summaries_configuration_enabled as ff_is_summary_config_enabled
 from ai_aside.waffle import summary_enabled as ff_summary_enabled
 from ai_aside.waffle import summary_staff_only as ff_summary_staff_only
 
@@ -280,7 +281,10 @@ def _should_apply_can_throw(cls, block):
         if _staff_user(block) and ff_summary_staff_only(course_key):
             return True
 
-        if not ff_summary_enabled(course_key):
-            return False
+        if ff_is_summary_config_enabled(course_key):
+            return is_summary_enabled(course_key, unit_key)
+
+        if ff_summary_enabled(course_key):
+            return True
 
-        return is_summary_enabled(course_key, unit_key)
+        return False

ai_aside/api/api.py → ai_aside/config_api/api.py

@@ -1,41 +1,18 @@
 """
 Implements an API for updating unit and course settings.
 """
+from ai_aside.config_api.internal import NotFoundError, _get_course, _get_unit
 from ai_aside.models import AIAsideCourseEnabled, AIAsideUnitEnabled
 from ai_aside.waffle import summaries_configuration_enabled
 
 
-class NotFoundError(Exception):
-    "Raised when the course/unit is not found in the database"
-
-
-def _get_course(course_key):
-    "Private method that gets a course based on an id"
-    try:
-        record = AIAsideCourseEnabled.objects.get(
-            course_key=course_key,
-        )
-    except AIAsideCourseEnabled.DoesNotExist as exc:
-        raise NotFoundError from exc
-
-    return record
-
-
-def _get_unit(course_key, unit_key):
-    "Private method that gets a unit based on an id"
-    try:
-        record = AIAsideUnitEnabled.objects.get(
-            course_key=course_key,
-            unit_key=unit_key,
-        )
-    except AIAsideUnitEnabled.DoesNotExist as exc:
-        raise NotFoundError from exc
-
-    return record
-
-
 def get_course_settings(course_key):
-    "Gets the settings of a course"
+    """
+    Gets the settings of a course.
+
+    Returns: dictionary of the form:
+        `{'enabled': bool}`
+    """
     record = _get_course(course_key)
     fields = {
         'enabled': record.enabled
@@ -45,8 +22,14 @@ def get_course_settings(course_key):
 
 
 def set_course_settings(course_key, settings):
-    "Sets the settings of a course"
+    """
+    Sets the settings of a course.
+
+    Expects: settings to be a dictionary of the form:
+        `{'enabled': bool}`
 
+    Raises NotFoundError if the settings are not found.
+    """
     enabled = settings['enabled']
 
     if not isinstance(enabled, bool):
@@ -59,19 +42,24 @@ def set_course_settings(course_key, settings):
         defaults=update,
     )
 
-    return True
-
 
 def delete_course_settings(course_key):
-    "Deletes the settings of a course"
+    """
+    Deletes the settings of a course.
+
+    Raises NotFoundError if the settings are not found.
+    """
     record = _get_course(course_key)
     record.delete()
 
-    return True
-
 
 def get_unit_settings(course_key, unit_key):
-    "Gets the settings of a course's unit"
+    """
+    Gets the settings of a unit.
+
+    Returns: dictionary of the form:
+        `{'enabled': bool}`
+    """
     record = _get_unit(course_key, unit_key)
 
     fields = {
@@ -82,31 +70,37 @@ def get_unit_settings(course_key, unit_key):
 
 
 def set_unit_settings(course_key, unit_key, settings):
-    "Sets the settings of a course's unit"
+    """
+    Sets the settings of a course's unit.
+
+    Expects: settings as a dictionary of the form:
+        `{'enabled': bool}`
 
+    Raises NotFoundError if the settings are not found.
+    """
     enabled = settings['enabled']
 
     if not isinstance(enabled, bool):
         raise TypeError
 
-    update = {'enabled': enabled}
+    settings = {'enabled': enabled}
 
     AIAsideUnitEnabled.objects.update_or_create(
         course_key=course_key,
         unit_key=unit_key,
-        defaults=update,
+        defaults=settings,
     )
 
-    return True
-
 
 def delete_unit_settings(course_key, unit_key):
-    "Deletes the settings of a course's unit"
+    """
+    Deletes the settings of a unit.
+
+    Raises NotFoundError if the settings are not found.
+    """
     record = _get_unit(course_key, unit_key)
     record.delete()
 
-    return True
-
 
 def is_summary_config_enabled(course_key):
     """

ai_aside/config_api/internal.py

@@ -0,0 +1,33 @@
+"""
+Implements an API for updating unit and course settings.
+"""
+from ai_aside.models import AIAsideCourseEnabled, AIAsideUnitEnabled
+
+
+class NotFoundError(Exception):
+    "Raised when the course/unit is not found in the database"
+
+
+def _get_course(course_key):
+    "Private method that gets a course based on an id"
+    try:
+        record = AIAsideCourseEnabled.objects.get(
+            course_key=course_key,
+        )
+    except AIAsideCourseEnabled.DoesNotExist as exc:
+        raise NotFoundError from exc
+
+    return record
+
+
+def _get_unit(course_key, unit_key):
+    "Private method that gets a unit based on an id"
+    try:
+        record = AIAsideUnitEnabled.objects.get(
+            course_key=course_key,
+            unit_key=unit_key,
+        )
+    except AIAsideUnitEnabled.DoesNotExist as exc:
+        raise NotFoundError from exc
+
+    return record

ai_aside/api/urls.py → ai_aside/config_api/urls.py

@@ -3,14 +3,14 @@
 """
 from django.urls import re_path
 
-from ai_aside.api.views import CourseEnabledAPIView, CourseSummaryConfigEnabledAPIView, UnitEnabledAPIView
+from ai_aside.config_api.views import CourseEnabledAPIView, CourseSummaryConfigEnabledAPIView, UnitEnabledAPIView
 from ai_aside.constants import COURSE_ID_PATTERN, UNIT_ID_PATTERN
 
 urlpatterns = [
     re_path(r'^v1/{course_id}/?$'.format(
         course_id=COURSE_ID_PATTERN
     ), CourseEnabledAPIView.as_view(), name='api-course-settings'),
-    re_path(r'^v1/{course_id}/configurable?$'.format(
+    re_path(r'^v1/{course_id}/configurable/?$'.format(
         course_id=COURSE_ID_PATTERN
     ), CourseSummaryConfigEnabledAPIView.as_view(), name='api-course-configurable'),
     re_path(r'^v1/{course_id}/{unit_id}/?$'.format(

ai_aside/api/views.py → ai_aside/config_api/views.py

@@ -21,7 +21,7 @@
 from rest_framework.response import Response
 from rest_framework.views import APIView
 
-from ai_aside.api.api import (
+from ai_aside.config_api.api import (
     NotFoundError,
     delete_course_settings,
     delete_unit_settings,

ai_aside/urls.py

@@ -4,7 +4,7 @@
 from django.urls import path, re_path  # pylint: disable=unused-import
 from django.views.generic import TemplateView  # pylint: disable=unused-import
 
-from ai_aside.api.urls import urlpatterns as apipatterns
+from ai_aside.config_api.urls import urlpatterns as apipatterns
 
 app_name = 'ai_aside'
 

requirements.txt

@@ -0,0 +1,80 @@
+#
+# This file is autogenerated by pip-compile with Python 3.8
+# by the following command:
+#
+#    pip-compile
+#
+appdirs==1.4.4
+    # via fs
+asgiref==3.7.2
+    # via django
+backports-zoneinfo==0.2.1
+    # via django
+cffi==1.15.1
+    # via pynacl
+click==8.1.6
+    # via edx-django-utils
+django==4.2.3
+    # via
+    #   ai-aside (setup.py)
+    #   django-crum
+    #   django-waffle
+    #   djangorestframework
+    #   edx-django-utils
+django-crum==0.7.9
+    # via edx-django-utils
+django-waffle==4.0.0
+    # via edx-django-utils
+djangorestframework==3.14.0
+    # via ai-aside (setup.py)
+edx-django-utils==5.6.0
+    # via ai-aside (setup.py)
+edx-opaque-keys==2.3.0
+    # via ai-aside (setup.py)
+fs==2.4.16
+    # via xblock
+lxml==4.9.3
+    # via xblock
+markupsafe==2.1.3
+    # via xblock
+newrelic==8.9.0
+    # via edx-django-utils
+pbr==5.11.1
+    # via stevedore
+psutil==5.9.5
+    # via edx-django-utils
+pycparser==2.21
+    # via cffi
+pymongo==3.13.0
+    # via edx-opaque-keys
+pynacl==1.5.0
+    # via edx-django-utils
+python-dateutil==2.8.2
+    # via xblock
+pytz==2023.3
+    # via
+    #   djangorestframework
+    #   xblock
+pyyaml==6.0.1
+    # via xblock
+six==1.16.0
+    # via
+    #   fs
+    #   python-dateutil
+sqlparse==0.4.4
+    # via django
+stevedore==5.1.0
+    # via
+    #   edx-django-utils
+    #   edx-opaque-keys
+typing-extensions==4.7.1
+    # via asgiref
+web-fragments==2.0.0
+    # via xblock
+webob==1.8.7
+    # via xblock
+xblock==1.6.2
+    # via ai-aside (setup.py)
+
+# The following packages are considered to be unsafe in a requirements file:
+# setuptools

tests/api/test_api.py

@@ -6,7 +6,7 @@
 from django.test import TestCase
 from opaque_keys.edx.keys import CourseKey, UsageKey
 
-from ai_aside.api.api import (
+from ai_aside.config_api.api import (
     NotFoundError,
     delete_course_settings,
     delete_unit_settings,
@@ -35,9 +35,8 @@ class TestApiMethods(TestCase):
     """API Endpoint Method tests"""
     def test_set_course_settings(self):
         course_key = course_keys[0]
-        response = set_course_settings(course_key, {'enabled': True})
 
-        self.assertEqual(response, True)
+        set_course_settings(course_key, {'enabled': True})
 
         res = AIAsideCourseEnabled.objects.filter(course_key=course_key)
 
@@ -52,9 +51,7 @@ def test_set_course_settings_duplicates(self):
             enabled=True
         )
 
-        response = set_course_settings(course_key, {'enabled': False})
-
-        self.assertEqual(response, True)
+        set_course_settings(course_key, {'enabled': False})
 
         res = AIAsideCourseEnabled.objects.filter(course_key=course_key)
 
@@ -109,9 +106,7 @@ def test_set_unit_settings(self):
         course_key = course_keys[0]
         unit_key = unit_keys[0]
 
-        response = set_unit_settings(course_key, unit_key, {'enabled': True})
-
-        self.assertEqual(response, True)
+        set_unit_settings(course_key, unit_key, {'enabled': True})
 
         res = AIAsideUnitEnabled.objects.filter(
             course_key=course_key,
@@ -186,9 +181,7 @@ def test_unit_delete(self):
             enabled=True,
         )
 
-        response = delete_unit_settings(course_key, unit_key)
-
-        self.assertTrue(response)
+        delete_unit_settings(course_key, unit_key)
 
         res = AIAsideUnitEnabled.objects.filter(
             course_key=course_key,
@@ -205,7 +198,7 @@ def test_unit_delete_not_found(self):
         with self.assertRaises(NotFoundError):
             delete_unit_settings(course_key, unit_key)
 
-    @patch('ai_aside.api.api.summaries_configuration_enabled')
+    @patch('ai_aside.config_api.api.summaries_configuration_enabled')
     def test_is_summary_enabled_course(self, mock_enabled):
         mock_enabled.return_value = True
         course_key_true = course_keys[0]
@@ -226,7 +219,7 @@ def test_is_summary_enabled_course(self, mock_enabled):
         course_key_non_existent = course_keys[2]
         self.assertFalse(is_summary_enabled(course_key_non_existent))
 
-    @patch('ai_aside.api.api.summaries_configuration_enabled')
+    @patch('ai_aside.config_api.api.summaries_configuration_enabled')
     def test_is_summary_enabled_unit(self, mock_enabled):
         mock_enabled.return_value = True
         course_key = course_keys[0]
@@ -250,7 +243,7 @@ def test_is_summary_enabled_unit(self, mock_enabled):
         unit_key_non_existent = unit_keys[2]
         self.assertFalse(is_summary_enabled(course_key, unit_key_non_existent))
 
-    @patch('ai_aside.api.api.summaries_configuration_enabled')
+    @patch('ai_aside.config_api.api.summaries_configuration_enabled')
     def test_is_summary_enabled_fallback(self, mock_enabled):
         mock_enabled.return_value = True
         course_key_true = course_keys[0]