feat: add library roles support for post and update roles

futurex_openedx_extensions/helpers/constants.py

@@ -29,11 +29,15 @@
 
 CLICKHOUSE_QUERY_SLUG_PATTERN = r'[a-z0-9_\-.]+'
 
-COURSE_ID_PART = r'[a-zA-Z0-9_-]+'
+ID_PART = r'[a-zA-Z0-9_-]+'
 COURSE_ID_REGX = \
-    fr'(?P<course_id>course-v1:(?P<org>{COURSE_ID_PART})\+(?P<course>{COURSE_ID_PART})\+(?P<run>{COURSE_ID_PART}))'
+    fr'(?P<course_id>course-v1:(?P<org>{ID_PART})\+(?P<course>{ID_PART})\+(?P<run>{ID_PART}))'
 COURSE_ID_REGX_EXACT = rf'^{COURSE_ID_REGX}$'
 
+LIBRARY_ID_REGX = \
+    fr'(?P<course_id>library-v1:(?P<org>{ID_PART})\+(?P<code>{ID_PART}))'
+LIBRARY_ID_REGX_EXACT = rf'^{LIBRARY_ID_REGX}$'
+
 COURSE_STATUSES = {
     'active': 'active',
     'archived': 'archived',
@@ -57,10 +61,12 @@
 ]
 
 COURSE_ACCESS_ROLES_STAFF_EDITOR = 'staff'
+COURSE_ACCESS_ROLES_LIBRARY_USER = 'library_user'
 
 COURSE_ACCESS_ROLES_TENANT_OR_COURSE = [
     'data_researcher',
     'instructor',
+    COURSE_ACCESS_ROLES_LIBRARY_USER,
     COURSE_ACCESS_ROLES_STAFF_EDITOR,
 ]
 COURSE_ACCESS_ROLES_GLOBAL = [
@@ -78,7 +84,6 @@
 COURSE_ACCESS_ROLES_ACCEPT_COURSE_ID = COURSE_ACCESS_ROLES_COURSE_ONLY + COURSE_ACCESS_ROLES_TENANT_OR_COURSE
 
 COURSE_ACCESS_ROLES_UNSUPPORTED = [
-    'library_user',  # not supported yet, it requires a library ID instead of a course ID
     'sales_admin',  # won't be supported, looks like a deprecated role, there is no useful code for it
 ]
 

futurex_openedx_extensions/helpers/extractors.py

@@ -7,6 +7,7 @@
 from urllib.parse import urlparse
 
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
+from xmodule.modulestore.django import modulestore
 
 from futurex_openedx_extensions.helpers import constants as cs
 
@@ -122,7 +123,7 @@ def verify_course_ids(course_ids: List[str]) -> None:
     for course_id in course_ids:
         if not isinstance(course_id, str):
             raise ValueError(f'course_id must be a string, but got {type(course_id).__name__}')
-        if not re.search(cs.COURSE_ID_REGX_EXACT, course_id):
+        if not re.search(cs.COURSE_ID_REGX_EXACT, course_id) and not re.search(cs.LIBRARY_ID_REGX_EXACT, course_id):
             raise ValueError(f'Invalid course ID format: {course_id}')
 
 
@@ -144,6 +145,12 @@ def get_orgs_of_courses(course_ids: List[str]) -> Dict[str, Any]:
         'courses': {str(course.id): course.org.lower() for course in courses},
     }
 
+    result['courses'].update({
+        str(lib_key): lib_key.org
+        for lib_key in modulestore().get_library_keys()
+        if str(lib_key) in course_ids
+    })
+
     invalid_course_ids = [course_id for course_id in course_ids if course_id not in result['courses']]
     if invalid_course_ids:
         raise ValueError(f'Invalid course IDs provided: {invalid_course_ids}')

futurex_openedx_extensions/helpers/roles.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import logging
+import re
 from copy import deepcopy
 from enum import Enum
 from typing import Any, Dict, List, Tuple
@@ -44,6 +45,19 @@
 logger = logging.getLogger(__name__)
 
 
+def are_all_library_ids(course_ids: List[str] | None) -> bool:
+    """
+    Verify if all given ids are library ids
+    """
+    if not course_ids:
+        return False
+
+    for course_id in course_ids:
+        if not re.match(cs.LIBRARY_ID_REGX, course_id):
+            return False
+    return True
+
+
 def validate_course_access_role(course_access_role: dict) -> None:
     """
     Validate the course access role.
@@ -60,6 +74,11 @@ def check_broken_rule(broken_rule: bool, code: FXExceptionCodes, message: str) -
     org = course_access_role['org'].strip()
     course_id = course_access_role['course_id']
     role = course_access_role['role']
+    check_broken_rule(
+        role == cs.COURSE_ACCESS_ROLES_LIBRARY_USER and course_id and not are_all_library_ids([course_id]),
+        FXExceptionCodes.ROLE_INVALID_ENTRY,
+        f'role {role} is only valid with library_id',
+    )
 
     check_broken_rule(
         role not in cs.COURSE_ACCESS_ROLES_ALL,
@@ -1155,6 +1174,9 @@ def add_course_access_roles(  # pylint: disable=too-many-arguments, too-many-bra
         if (tenant_wide and course_ids) or (not tenant_wide and not course_ids):
             raise ValueError('Conflict between tenant_wide and course_ids')
 
+        if role == cs.COURSE_ACCESS_ROLES_LIBRARY_USER and course_ids and not are_all_library_ids(course_ids):
+            raise ValueError(f'Role: {role} is only valid for library ids.')
+
         if not user_keys:
             raise ValueError('No users provided!')
 

test_utils/edx_platform_mocks/xmodule/modulestore/django.py

@@ -0,0 +1,17 @@
+"""Mock modulestore"""
+from opaque_keys.edx.keys import CourseKey
+
+
+class DummyModuleStore:  # pylint: disable=too-few-public-methods
+    """DUmmy module store class"""
+    def __init__(self):
+        self.ids = ['library-v1:org1+11', 'library-v1:org1+22']
+
+    def get_library_keys(self):
+        """mock modulestore library keys method"""
+        return [CourseKey.from_string(id) for id in self.ids]
+
+
+def modulestore():
+    """mocked modulestore"""
+    return DummyModuleStore()

tests/test_helpers/test_extractors.py

@@ -49,16 +49,22 @@ def test_get_course_id_from_uri(uri, expected_course_id):
     assert get_course_id_from_uri(uri) == expected_course_id
 
 
-def test_verify_course_ids_success():
-    """Verify that verify_course_ids does not raise an error for valid course IDs."""
-    course_ids = ['course-v1:edX+DemoX+Demo_Course', 'course-v1:edX+DemoX+Demo_Course2']
+@pytest.mark.parametrize('course_ids', [
+    (['course-v1:edX+DemoX+Demo_Course', 'course-v1:edX+DemoX+Demo_Course2']),
+    (['library-v1:DemoX+11', 'library-v1:DemoX+22']),
+    (['course-v1:edX+DemoX+Demo_Course', 'library-v1:DemoX+22']),
+])
+def test_verify_course_ids_success(course_ids):
+    """Verify that verify_course_ids does not raise an error for valid course and library IDs."""
     verify_course_ids(course_ids)
 
 
 @pytest.mark.parametrize('course_ids, error_message', [
     (None, 'course_ids must be a list of course IDs, but got None'),
     (['course-v1:edX+DemoX+Demo_Course', 3], 'course_id must be a string, but got int'),
     (['course-v1:edX+DemoX+Demo_Course+extra'], 'Invalid course ID format: course-v1:edX+DemoX+Demo_Course+extra'),
+    (['library-v1:edX+DemoX+extra'], 'Invalid course ID format: library-v1:edX+DemoX+extra'),
+    (['library-v1:invalid'], 'Invalid course ID format: library-v1:invalid'),
 ])
 def test_verify_course_ids_fail(course_ids, error_message):
     """Verify that verify_course_ids raises an error for invalid course IDs."""
@@ -83,11 +89,48 @@ def test_get_orgs_of_courses(base_data):  # pylint: disable=unused-argument
 
 
 @pytest.mark.django_db
-def test_get_orgs_of_courses_invalid_course(base_data):  # pylint: disable=unused-argument
+@pytest.mark.parametrize('course_ids, expected_orgs', [
+    (['course-v1:Org1+1+1', 'course-v1:ORG1+2+2'], {
+        'course-v1:Org1+1+1': 'org1',
+        'course-v1:ORG1+2+2': 'org1',
+    }),
+    (['library-v1:org1+11', 'course-v1:ORG1+5+5'], {
+        'library-v1:org1+11': 'org1',
+        'course-v1:ORG1+5+5': 'org1',
+    }),
+    (['library-v1:org1+11', 'library-v1:org1+22'], {
+        'library-v1:org1+11': 'org1',
+        'library-v1:org1+22': 'org1',
+    }),
+])
+def test_get_orgs_of_courses_for_library_ids(course_ids, expected_orgs, base_data):  # pylint: disable=unused-argument
+    """Verify that get_orgs_of_courses returns the expected organization for library ids."""
+    result = get_orgs_of_courses(course_ids)
+    assert result == {'courses': expected_orgs}
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize('invalid_course_ids, expected_error_message', [
+    (
+        ['course-v1:ORG1+2+99'],
+        'Invalid course IDs provided: [\'course-v1:ORG1+2+99\']'
+    ),
+    (
+        ['library-v1:not_exist+11'],
+        'Invalid course IDs provided: [\'library-v1:not_exist+11\']'
+    ),
+    (
+        ['course-v1:ORG1+2+99', 'library-v1:not_exist+1'],
+        'Invalid course IDs provided: [\'course-v1:ORG1+2+99\', \'library-v1:not_exist+1\']'
+    )
+])
+def test_get_orgs_of_courses_invalid_course(
+    course_ids, expected_error_message, base_data
+):  # pylint: disable=unused-argument
     """Verify that get_orgs_of_courses raises an error for an invalid course ID."""
     with pytest.raises(ValueError) as exc_info:
-        get_orgs_of_courses(['course-v1:ORG1+2+99'])
-    assert str(exc_info.value) == 'Invalid course IDs provided: [\'course-v1:ORG1+2+99\']'
+        get_orgs_of_courses(course_ids)
+    assert str(exc_info.value) == expected_error_message
 
 
 def test_dict_hashcode_init():

tests/test_helpers/test_roles.py

@@ -30,6 +30,7 @@
     add_course_access_roles,
     add_missing_signup_source_record,
     add_org_course_creator,
+    are_all_library_ids,
     cache_name_user_course_access_roles,
     cache_refresh_course_access_roles,
     check_tenant_access,
@@ -65,6 +66,20 @@ def reset_fx_views_with_roles():
     FXViewRoleInfoMetaClass._fx_views_with_roles = {'_all_view_names': {}}  # pylint: disable=protected-access
 
 
+@pytest.mark.parametrize('course_ids, expected_return, usecase', [
+    (None, False, 'course_ids list is None'),
+    ([], False, 'course_ids list is empty'),
+    (['course-v1:ORG1+1+1', 'course-v1:ORG1+1+1'], False, 'course_ids contain courses'),
+    (['library-v1:ORG1+1', 'course-v1:ORG1+1+1'], False, 'course_ids contain libraries and courses'),
+    (['library-v1:ORG1+11', 'library-v1:ORG1+22'], True, 'course_ids contain valid library'),
+    (['library-v1:ORG1+11', 'invalid-library-id'], False, 'course_ids contain invalid library'),
+])
+def test_are_all_library_ids(course_ids, expected_return, usecase):
+    """test are_all_librarary_ids"""
+    return_value = are_all_library_ids(course_ids)
+    assert return_value is expected_return, f'Failed usecase: {usecase}'
+
+
 @pytest.mark.parametrize('update_course_access_role, error_msg, error_code', [
     ({'role': 'bad_role'}, 'invalid role ({role})!', FXExceptionCodes.ROLE_INVALID_ENTRY),
     (
@@ -117,6 +132,11 @@ def reset_fx_views_with_roles():
         'missing course-creator record for {role} role!',
         FXExceptionCodes.ROLE_INVALID_ENTRY,
     ),
+    (
+        {'role': cs.COURSE_ACCESS_ROLES_LIBRARY_USER, 'course_id': 'yes'},
+        'role {role} is only valid with library_id',
+        FXExceptionCodes.ROLE_INVALID_ENTRY
+    ),
 ])
 @pytest.mark.django_db
 def test_validate_course_access_role_invalid(
@@ -132,7 +152,7 @@ def test_validate_course_access_role_invalid(
     }
     bad_course_access_role.update(update_course_access_role)
     if bad_course_access_role['course_id']:
-        bad_course_access_role['course_id'] = CourseKey.from_string('course-v1:ORG1+1+1')
+        bad_course_access_role['course_id'] = 'course-v1:ORG1+1+1'
 
     if error_msg and '{role}' in error_msg:
         error_msg = error_msg.format(role=update_course_access_role['role'])
@@ -1316,6 +1336,15 @@ def test_add_course_access_roles_tenant_wide_role(roles_authorize_caller):  # py
     assert str(exc_info.value) == f'Role ({role}) can only be tenant-wide!'
 
 
+@pytest.mark.django_db
+def test_add_course_access_roles_for_library_user(roles_authorize_caller):  # pylint: disable=unused-argument
+    """Verify that add_course_access_roles raises an error adding library_user role with course_id"""
+    role = cs.COURSE_ACCESS_ROLES_LIBRARY_USER
+    with pytest.raises(FXCodedException) as exc_info:
+        add_course_access_roles(None, [1, 8], ['user1'], role, False, ['course-v1:ORG3+1+1'])
+    assert str(exc_info.value) == f'Role: {role} is only valid for library ids.'
+
+
 @pytest.mark.django_db
 @pytest.mark.parametrize('user_keys, error_message', [
     (None, 'No users provided!'),
@@ -1411,6 +1440,30 @@ def test_add_course_access_roles_success_list_contains_user_key(
     }
 
 
+@pytest.mark.django_db
+@pytest.mark.parametrize('role, library_ids, tenant_wide', [
+    ('library_user', ['library-v1:org1+11'], False),
+    ('library_user', [], True),
+    ('library_user', None, True),
+    ('staff', ['library-v1:org1+11'], False),
+    ('instructor', ['library-v1:org1+11'], False)
+])
+@patch('futurex_openedx_extensions.helpers.users.get_user_by_username_or_email')
+def test_add_course_access_roles_success_for_library_roles(
+    mock_get_user, role, library_ids, tenant_wide, base_data
+):  # pylint: disable=unused-argument
+    """Verify that add_course_access_roles for library roles"""
+    caller = get_user_model().objects.get(username='user1')
+    mock_get_user.return_value = get_user_model().objects.get(id=3)
+    result = add_course_access_roles(caller, [1], ['user3'], role, tenant_wide, library_ids)
+    assert result == {
+        'failed': [],
+        'added': [],
+        'updated': ['user3'],
+        'not_updated': [],
+    }
+
+
 @pytest.mark.django_db
 def test_add_course_access_roles_success_user_key_same_as_id(
     roles_authorize_caller, base_data