diff --git a/.coveragerc b/.coveragerc index fdefc7e..a519e2a 100644 --- a/.coveragerc +++ b/.coveragerc @@ -8,3 +8,4 @@ omit = *admin.py *static* *templates* + learning_assistant/plugins.py diff --git a/docs/decisions/0003-courseapp-use.rst b/docs/decisions/0003-courseapp-use.rst index b19b1a7..eedf67e 100644 --- a/docs/decisions/0003-courseapp-use.rst +++ b/docs/decisions/0003-courseapp-use.rst @@ -40,6 +40,10 @@ Consequences Assistant plugin is installed into ``edx-platform``. * It will become slightly more difficult to know which ``CourseApps`` are available simply by reading the code, because this ``CourseApp`` plugin is not stored in the `edx-platform repository`_. +* Because the `CourseApps` API is registered under the CMS application, the Learning Assistant needs to be registered as + a plugin to the CMS as well. Otherwise, the plugin is not included in the CMS application's ``INSTALLED_APPS`` list. + This causes a runtime error, because the Learning Assistant CourseApp plugin will refer to the Learning Assistant's + models, and this are not available in the CMS if the Learning Assistant plugin is not installed. Rejected Alternatives ********************* diff --git a/learning_assistant/__init__.py b/learning_assistant/__init__.py index 23a04a2..c900846 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__ = '3.4.0' +__version__ = '3.5.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 7a41996..b5e4a29 100644 --- a/learning_assistant/api.py +++ b/learning_assistant/api.py @@ -10,6 +10,7 @@ from opaque_keys.edx.keys import CourseKey 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.platform_imports import ( block_get_children, @@ -139,3 +140,25 @@ def learning_assistant_enabled(course_key): enabled = True return learning_assistant_available(course_key) and enabled + + +def set_learning_assistant_enabled(course_key, enabled): + """ + Set whether the Learning Assistant is enabled and return a representation of the created data. + + Arguments: + * course_key: (CourseKey): the course's key + * enabled (bool): whether the Learning Assistant should be enabled + + Returns: + * bool: whether the Learning Assistant is enabled + """ + obj, _ = LearningAssistantCourseEnabled.objects.update_or_create( + course_id=course_key, + defaults={'enabled': enabled} + ) + + return LearningAssistantCourseEnabledData( + course_key=obj.course_id, + enabled=obj.enabled + ) diff --git a/learning_assistant/data.py b/learning_assistant/data.py new file mode 100644 index 0000000..2e89c27 --- /dev/null +++ b/learning_assistant/data.py @@ -0,0 +1,15 @@ +""" +Data classes for the Learning Assistant application. +""" +from attrs import field, frozen, validators +from opaque_keys.edx.keys import CourseKey + + +@frozen +class LearningAssistantCourseEnabledData: + """ + Data class representing whether Learning Assistant is enabled in a course. + """ + + course_key: CourseKey = field(validator=validators.instance_of(CourseKey)) + enabled: bool = field(validator=validators.instance_of(bool)) diff --git a/learning_assistant/platform_imports.py b/learning_assistant/platform_imports.py index 2c98d3d..d9958fd 100644 --- a/learning_assistant/platform_imports.py +++ b/learning_assistant/platform_imports.py @@ -77,3 +77,19 @@ def learning_assistant_available(course_key): # pylint: disable=import-error, import-outside-toplevel from lms.djangoapps.courseware.toggles import learning_assistant_is_active return learning_assistant_is_active(course_key) + + +def get_user_role(user, course_key): + """ + Return the role of the user on the edX platform. + + Arguments: + * user (User): the user who's role to get + * course_key (CourseKey): the key of the course in which to get the user's role + + Returns: + * str: the user's role + """ + # pylint: disable=import-error, import-outside-toplevel + from lms.djangoapps.courseware.access import get_user_role as platform_get_user_role + return platform_get_user_role(user, course_key) diff --git a/learning_assistant/plugins.py b/learning_assistant/plugins.py new file mode 100644 index 0000000..c2f3bfe --- /dev/null +++ b/learning_assistant/plugins.py @@ -0,0 +1,90 @@ +""" +Plugins for the Learning Assistant application. +""" +# pylint: disable=import-error +from openedx.core.djangoapps.course_apps.plugins import CourseApp + +from learning_assistant import plugins_api + + +class LearningAssistantCourseApp(CourseApp): + """ + A CourseApp plugin representing the Learning Assistant feature. + + Please see the associated ADR for more details. + """ + + app_id = 'learning_assistant' + name = 'Learning Assistant' + description = 'Use generative AI to power a Learning Assistant using course content.' + documentation_links = { + 'learn_more_openai': 'https://openai.com/', + 'learn_more_openai_data_privacy': 'https://openai.com/api-data-privacy', + } + + @classmethod + def is_available(cls, course_key): + """ + Return a boolean indicating this course app's availability for a given course. + + If an app is not available, it will not show up in the UI at all for that course, + and it will not be possible to enable/disable/configure it. + + Args: + course_key (CourseKey): Course key for course whose availability is being checked. + + Returns: + bool: Availability status of app. + """ + return plugins_api.is_available(course_key) + + @classmethod + def is_enabled(cls, course_key): + """ + Return if this course app is enabled for the provided course. + + Args: + course_key (CourseKey): The course key for the course you + want to check the status of. + + Returns: + bool: The status of the course app for the specified course. + """ + return plugins_api.is_enabled(course_key) + + @classmethod + def set_enabled(cls, course_key, enabled, user): + """ + Update the status of this app for the provided course and return the new status. + + Args: + course_key (CourseKey): The course key for the course for which the app should be enabled. + enabled (bool): The new status of the app. + user (User): The user performing this operation. + + Returns: + bool: The new status of the course app. + """ + return plugins_api.set_enabled(course_key, enabled, user) + + @classmethod + def get_allowed_operations(cls, course_key, user=None): + """ + Return a dictionary of available operations for this app. + + Not all apps will support being configured, and some may support + other operations via the UI. This will list, the minimum whether + the app can be enabled/disabled and whether it can be configured. + + Args: + course_key (CourseKey): The course key for a course. + user (User): The user for which the operation is to be tested. + + Returns: + A dictionary that has keys like 'enable', 'configure' etc + with values indicating whether those operations are allowed. + + get_allowed_operations: function that returns a dictionary of the form + {'enable': , 'configure': }. + """ + return plugins_api.get_allowed_operations(course_key, user) diff --git a/learning_assistant/plugins_api.py b/learning_assistant/plugins_api.py new file mode 100644 index 0000000..eb334c9 --- /dev/null +++ b/learning_assistant/plugins_api.py @@ -0,0 +1,88 @@ +""" +Concrete implementations of abstract methods of the CourseApp plugin ABC, for use by the LearningAssistantCourseApp. + +Because the LearningAssistantCourseApp plugin inherits from the CourseApp class, which is imported from the +edx-platform, we cannot test that plugin directly, because pytest will run outside the platform context. +Instead, the CourseApp abstract methods are implemented here and +imported into and used by the LearningAssistantCourseApp. This way, these implementations can be tested. +""" + +from learning_assistant.api import learning_assistant_enabled, set_learning_assistant_enabled +from learning_assistant.platform_imports import get_user_role, learning_assistant_available +from learning_assistant.utils import user_role_is_staff + + +def is_available(course_key): + """ + Return a boolean indicating this course app's availability for a given course. + + If an app is not available, it will not show up in the UI at all for that course, + and it will not be possible to enable/disable/configure it. + + Args: + course_key (CourseKey): Course key for course whose availability is being checked. + + Returns: + bool: Availability status of app. + """ + return learning_assistant_available(course_key) + + +def is_enabled(course_key): + """ + Return if this course app is enabled for the provided course. + + Args: + course_key (CourseKey): The course key for the course you + want to check the status of. + + Returns: + bool: The status of the course app for the specified course. + """ + return learning_assistant_enabled(course_key) + + +# pylint: disable=unused-argument +def set_enabled(course_key, enabled, user): + """ + Update the status of this app for the provided course and return the new status. + + Args: + course_key (CourseKey): The course key for the course for which the app should be enabled. + enabled (bool): The new status of the app. + user (User): The user performing this operation. + + Returns: + bool: The new status of the course app. + """ + obj = set_learning_assistant_enabled(course_key, enabled) + + return obj.enabled + + +def get_allowed_operations(course_key, user=None): + """ + Return a dictionary of available operations for this app. + + Not all apps will support being configured, and some may support + other operations via the UI. This will list, the minimum whether + the app can be enabled/disabled and whether it can be configured. + + Args: + course_key (CourseKey): The course key for a course. + user (User): The user for which the operation is to be tested. + + Returns: + A dictionary that has keys like 'enable', 'configure' etc + with values indicating whether those operations are allowed. + + get_allowed_operations: function that returns a dictionary of the form + {'enable': , 'configure': }. + """ + if not user: + return {'configure': False, 'enable': False} + else: + user_role = get_user_role(user, course_key) + is_staff = user_role_is_staff(user_role) + + return {'configure': False, 'enable': is_staff} diff --git a/learning_assistant/utils.py b/learning_assistant/utils.py index 0acc04c..518fb01 100644 --- a/learning_assistant/utils.py +++ b/learning_assistant/utils.py @@ -123,3 +123,16 @@ def get_chat_response(prompt_template, message_list, courserun_id): chat = 'Completion endpoint is not defined.' return response_status, chat + + +def user_role_is_staff(role): + """ + Return whether the user role parameter represents that of a staff member. + + Arguments: + * role (str): the user's role + + Returns: + * bool: whether the user's role is that of a staff member + """ + return role in ('staff', 'instructor') diff --git a/learning_assistant/views.py b/learning_assistant/views.py index 360a5a2..5db882a 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -21,7 +21,7 @@ from learning_assistant.api import learning_assistant_enabled, render_prompt_template from learning_assistant.serializers import MessageSerializer -from learning_assistant.utils import get_chat_response +from learning_assistant.utils import get_chat_response, user_role_is_staff log = logging.getLogger(__name__) @@ -65,7 +65,7 @@ def post(self, request, course_id): enrollment_mode = enrollment_object.mode if enrollment_object else None if ( (enrollment_mode not in CourseMode.VERIFIED_MODES) - and user_role not in ('staff', 'instructor') + and not user_role_is_staff(user_role) ): return Response( status=http_status.HTTP_403_FORBIDDEN, diff --git a/requirements/base.in b/requirements/base.in index 52fe316..0fc7801 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -1,6 +1,7 @@ # Core requirements for using this application -c constraints.txt +attrs Django # Web application framework django-model-utils djangorestframework diff --git a/requirements/base.txt b/requirements/base.txt index f9c9994..f6d14c2 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -6,6 +6,8 @@ # asgiref==3.7.2 # via django +attrs==23.2.0 + # via -r requirements/base.in certifi==2023.11.17 # via requests cffi==1.16.0 diff --git a/requirements/dev.txt b/requirements/dev.txt index fc302b2..1473fcf 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -13,6 +13,8 @@ astroid==3.0.2 # -r requirements/quality.txt # pylint # pylint-celery +attrs==23.2.0 + # via -r requirements/quality.txt build==1.0.3 # via # -r requirements/pip-tools.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index 0d69262..39f0fc2 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -10,6 +10,8 @@ asgiref==3.7.2 # via # -r requirements/test.txt # django +attrs==23.2.0 + # via -r requirements/test.txt babel==2.14.0 # via sphinx build==1.0.3 diff --git a/requirements/quality.txt b/requirements/quality.txt index cca8464..6bfae66 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -12,6 +12,8 @@ astroid==3.0.2 # via # pylint # pylint-celery +attrs==23.2.0 + # via -r requirements/test.txt certifi==2023.11.17 # via # -r requirements/test.txt diff --git a/requirements/test.txt b/requirements/test.txt index 0e03b66..33b6add 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -8,6 +8,8 @@ asgiref==3.7.2 # via # -r requirements/base.txt # django +attrs==23.2.0 + # via -r requirements/base.txt certifi==2023.11.17 # via # -r requirements/base.txt diff --git a/setup.py b/setup.py index 80b41a2..817fad1 100755 --- a/setup.py +++ b/setup.py @@ -131,6 +131,12 @@ def is_requirement(line): entry_points={ 'lms.djangoapp': [ "learning_assistant = learning_assistant.apps:LearningAssistantConfig" + ], + 'cms.djangoapp': [ + "learning_assistant = learning_assistant.apps:LearningAssistantConfig" + ], + 'openedx.course_app': [ + "learning_assistant = learning_assistant.plugins:LearningAssistantCourseApp", ] } ) diff --git a/tests/test_api.py b/tests/test_api.py index 1ddab3f..3dbadc4 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,6 +1,7 @@ """ Test cases for the learning-assistant api module. """ +import itertools from unittest.mock import MagicMock, patch import ddt @@ -15,7 +16,9 @@ get_block_content, learning_assistant_enabled, render_prompt_template, + set_learning_assistant_enabled, ) +from learning_assistant.data import LearningAssistantCourseEnabledData from learning_assistant.models import LearningAssistantCourseEnabled fake_transcript = 'This is the text version from the transcript' @@ -232,12 +235,34 @@ def test_learning_assistant_enabled( learning_assistant_available_mock.return_value = learning_assistant_available_value if obj_exists: - LearningAssistantCourseEnabled.objects.update_or_create( - course_id=self.course_key, - defaults={'enabled': obj_value} - ) + set_learning_assistant_enabled(self.course_key, obj_value) self.assertEqual( learning_assistant_enabled(self.course_key), expected_value ) + + @ddt.idata(itertools.product((True, False), (True, False))) + @ddt.unpack + def test_set_learning_assistant_enabled(self, obj_exists, obj_value): + if obj_exists: + LearningAssistantCourseEnabled.objects.create( + course_id=self.course_key, + # Set the opposite of the desired end value to test that it is changed properly. + enabled=not obj_value, + ) + + expected_value = LearningAssistantCourseEnabledData( + self.course_key, + obj_value, + ) + + return_value = set_learning_assistant_enabled(self.course_key, obj_value) + + self.assertEqual( + return_value, + expected_value, + ) + + obj = LearningAssistantCourseEnabled.objects.get(course_id=self.course_key) + self.assertEqual(obj.enabled, obj_value) diff --git a/tests/test_plugins_api.py b/tests/test_plugins_api.py new file mode 100644 index 0000000..f7f14a2 --- /dev/null +++ b/tests/test_plugins_api.py @@ -0,0 +1,85 @@ +""" +Test cases for the learning-assistant plugins module. +""" +from unittest.mock import patch + +import ddt +from django.contrib.auth import get_user_model +from django.test import TestCase +from opaque_keys.edx.keys import CourseKey + +from learning_assistant.models import LearningAssistantCourseEnabled +from learning_assistant.plugins_api import get_allowed_operations, is_available, is_enabled, set_enabled + +User = get_user_model() + + +@ddt.ddt +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') + self.user = User(username='tester', email='tester@test.com') + + @ddt.data(True, False) + @patch('learning_assistant.plugins_api.learning_assistant_available') + def test_is_available(self, is_available_value, learning_assistant_available_mock): + """ + Test the is_available function of the plugins_api module. + """ + learning_assistant_available_mock.return_value = is_available_value + self.assertEqual(is_available(self.course_key), is_available_value) + + @ddt.data(True, False) + @patch('learning_assistant.plugins_api.learning_assistant_enabled') + def test_is_enabled(self, is_enabled_value, learning_assistant_enabled_mock): + """ + Test the is_enabled function of the plugins_api module. + """ + learning_assistant_enabled_mock.return_value = is_enabled_value + self.assertEqual(is_enabled(self.course_key), is_enabled_value) + + @ddt.data(True, False) + def test_set_enabled_create(self, enabled_value): + """ + Test the set_enabled function of the plugins_api module when a create should occur. + """ + self.assertEqual(set_enabled(self.course_key, enabled_value, self.user), enabled_value) + + @ddt.data(True, False) + def test_set_enabled_update(self, enabled_value): + """ + Test the set_enabled function of the plugins_api module when an update should occur. + """ + LearningAssistantCourseEnabled.objects.create( + course_id=self.course_key, + enabled=enabled_value + ) + + self.assertEqual(set_enabled(self.course_key, enabled_value, self.user), enabled_value) + + def test_get_allowed_operations_no_user(self): + """ + Test the get_allowed_operations function of the plugins_api module when no user is passed as an argument. + """ + self.assertEqual( + get_allowed_operations(self.course_key), + {'configure': False, 'enable': False} + ) + + @ddt.unpack + @ddt.data(('instructor', True), ('staff', True), ('student', False)) + @patch('learning_assistant.plugins_api.get_user_role') + def test_get_allowed_operations(self, role_value, is_staff_value, get_user_role_mock): + """ + Test the get_allowed_operations function of the plugins_api module. + """ + get_user_role_mock.return_value = role_value + + self.assertEqual( + get_allowed_operations(self.course_key, self.user), + {'configure': False, 'enable': is_staff_value} + ) diff --git a/tests/test_utils.py b/tests/test_utils.py index 4adcf76..a410c1f 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -10,7 +10,7 @@ from django.test import TestCase, override_settings from requests.exceptions import ConnectTimeout -from learning_assistant.utils import get_chat_response, get_reduced_message_list +from learning_assistant.utils import get_chat_response, get_reduced_message_list, user_role_is_staff @ddt.ddt @@ -142,3 +142,14 @@ def test_message_list(self): reduced_message_list = get_reduced_message_list(self.prompt_template, self.message_list) self.assertEqual(len(reduced_message_list), 2) self.assertEqual(reduced_message_list, self.message_list) + + +@ddt.ddt +class UserRoleIsStaffTests(TestCase): + """ + Tests for the user_role_is_staff helper function. + """ + @ddt.data(('instructor', True), ('staff', True), ('student', False)) + @ddt.unpack + def test_user_role_is_staff(self, role, expected_value): + self.assertEqual(user_role_is_staff(role), expected_value)