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/learning_assistant/api.py b/learning_assistant/api.py index 0a44a7f..f90f39e 100644 --- a/learning_assistant/api.py +++ b/learning_assistant/api.py @@ -6,12 +6,14 @@ from edx_django_utils.cache import get_cache_key from learning_assistant.constants import ACCEPTED_CATEGORY_TYPES, CATEGORY_TYPE_MAP -from learning_assistant.models import CoursePrompt +from learning_assistant.data import LearningAssistantCourseEnabledData +from learning_assistant.models import CoursePrompt, LearningAssistantCourseEnabled from learning_assistant.platform_imports import ( block_get_children, block_leaf_filter, get_single_block, get_text_transcript, + learning_assistant_available, traverse_block_pre_order, ) from learning_assistant.text_utils import html_to_text @@ -112,3 +114,49 @@ def get_block_content(request, user_id, course_id, unit_usage_key): cache.set(cache_key, cache_data, getattr(settings, 'LEARNING_ASSISTANT_CACHE_TIMEOUT', 360)) return cache_data['content_length'], cache_data['content_items'] + + +def learning_assistant_enabled(course_key): + """ + Return whether the Learning Assistant is enabled in the course represented by the course_key. + + The Learning Assistant is enabled if the feature is available (i.e. appropriate CourseWaffleFlag is enabled) and + either there is no override in the LearningAssistantCourseEnabled table or there is an enabled value in the + LearningAssistantCourseEnabled table. + + Arguments: + * course_key: (CourseKey): the course's key + + Returns: + * bool: whether the Learning Assistant is enabled + """ + try: + obj = LearningAssistantCourseEnabled.objects.get(course_id=course_key) + enabled = obj.enabled + except LearningAssistantCourseEnabled.DoesNotExist: + # Currently, the Learning Assistant defaults to enabled if there is no override. + 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 d29de31..4ef5970 100644 --- a/learning_assistant/platform_imports.py +++ b/learning_assistant/platform_imports.py @@ -45,3 +45,39 @@ def block_get_children(block): # pylint: disable=import-error, import-outside-toplevel from openedx.core.lib.graph_traversals import get_children return get_children(block) + + +def learning_assistant_available(course_key): + """ + Return whether the Learning Assistant is available in the course represented by the course_key. + + Note that this may be different than whether the Learning Assistant is enabled in the course. The value returned by + this fuction represenents whether the Learning Assistant is available in the course and, perhaps, whether it is + enabled. Course teams can disable the Learning Assistant via the LearningAssistantCourseEnabled model, so, in those + cases, the Learning Assistant may be available and disabled. + + Arguments: + * course_key (CourseKey): the course's key + + Returns: + * bool: whether the Learning Assistant feature is available + """ + # 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: + * bool: whether the Learning Assistant feature is available + """ + # 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..fa2d235 --- /dev/null +++ b/learning_assistant/plugins.py @@ -0,0 +1,87 @@ +""" +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 = 'TBD' + documentation_links = {} + + @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 110f449..69f5622 100644 --- a/learning_assistant/utils.py +++ b/learning_assistant/utils.py @@ -84,3 +84,16 @@ def get_chat_response(system_list, message_list): 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 4749008..a3e7497 100644 --- a/learning_assistant/views.py +++ b/learning_assistant/views.py @@ -22,7 +22,7 @@ from learning_assistant.api import get_setup_messages 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__) @@ -59,7 +59,7 @@ def post(self, request, course_id): enrollment_mode = enrollment_object.mode if enrollment_object else None if ( (enrollment_mode not in CourseMode.ALL_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 9f9bb7e..301a868 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 a14fa9a..d17b4cd 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 a977133..c820691 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 65d4147..e1f9193 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 @@ -43,7 +45,6 @@ cryptography==41.0.7 # via # -r requirements/test.txt # pyjwt - # secretstorage ddt==1.7.0 # via -r requirements/test.txt django==3.2.23 @@ -122,10 +123,6 @@ iniconfig==2.0.0 # pytest jaraco-classes==3.3.0 # via keyring -jeepney==0.8.0 - # via - # keyring - # secretstorage jinja2==3.1.2 # via # -r requirements/test.txt @@ -242,8 +239,6 @@ rfc3986==2.0.0 # via twine rich==13.7.0 # via twine -secretstorage==3.3.3 - # via keyring semantic-version==2.10.0 # via # -r requirements/test.txt diff --git a/requirements/quality.txt b/requirements/quality.txt index 672fc8e..5958264 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 bc8ad53..88da15f 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 ad145e8..38b4867 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,12 +1,13 @@ """ Test cases for the learning-assistant api module. """ +import itertools from unittest.mock import MagicMock, patch import ddt from django.core.cache import cache from django.test import TestCase -from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.keys import CourseKey, UsageKey from learning_assistant.api import ( _extract_block_contents, @@ -15,8 +16,11 @@ get_block_content, get_deserialized_prompt_content_by_course_id, get_setup_messages, + learning_assistant_enabled, + set_learning_assistant_enabled, ) -from learning_assistant.models import CoursePrompt +from learning_assistant.data import LearningAssistantCourseEnabledData +from learning_assistant.models import CoursePrompt, LearningAssistantCourseEnabled fake_transcript = 'This is the text version from the transcript' @@ -204,3 +208,68 @@ def test_get_block_content(self, mock_get_children_contents, mock_get_single_blo mock_get_children_contents.assert_called_once() self.assertEqual(length, len(block_content)) self.assertEqual(items, content_items) + + +@ddt.ddt +class LearningAssistantCourseEnabledApiTests(TestCase): + """ + Test suite for the learning_assistant_enabled and set_learning_assistant_enalbed api functions. + """ + def setUp(self): + super().setUp() + self.course_key = CourseKey.from_string('course-v1:edx+fake+1') + + @ddt.data( + (True, True, True, True), + (True, True, False, False), + (True, False, True, False), + (True, False, False, False), + (False, True, True, True), + (False, False, True, True), + (False, True, False, False), + (False, False, False, False), + ) + @ddt.unpack + @patch('learning_assistant.api.learning_assistant_available') + def test_learning_assistant_enabled( + self, + obj_exists, + obj_value, + learning_assistant_available_value, + expected_value, + learning_assistant_available_mock, + ): + learning_assistant_available_mock.return_value = learning_assistant_available_value + + if obj_exists: + 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 27afa8e..70ad9a0 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -11,7 +11,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 @@ -129,3 +129,14 @@ def test_message_list(self): reduced_message_list = get_reduced_message_list(self.system_message, copy.deepcopy(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)