From 31d5e59332fbdef7f590569a4612c20f42ab7d86 Mon Sep 17 00:00:00 2001 From: Shadi Naif Date: Fri, 18 Oct 2024 18:25:59 +0300 Subject: [PATCH] feat: management command to cleanup roles for all users --- .../helpers/management/__init__.py | 0 .../commands/course_access_roles_clean_up.py | 198 ++++++++++++++++++ futurex_openedx_extensions/helpers/roles.py | 11 +- tests/test_helpers/test_management.py | 68 ++++++ tests/test_helpers/test_roles.py | 4 + 5 files changed, 278 insertions(+), 3 deletions(-) create mode 100644 futurex_openedx_extensions/helpers/management/__init__.py create mode 100644 futurex_openedx_extensions/helpers/management/commands/course_access_roles_clean_up.py create mode 100644 tests/test_helpers/test_management.py diff --git a/futurex_openedx_extensions/helpers/management/__init__.py b/futurex_openedx_extensions/helpers/management/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/futurex_openedx_extensions/helpers/management/commands/course_access_roles_clean_up.py b/futurex_openedx_extensions/helpers/management/commands/course_access_roles_clean_up.py new file mode 100644 index 00000000..28ae4529 --- /dev/null +++ b/futurex_openedx_extensions/helpers/management/commands/course_access_roles_clean_up.py @@ -0,0 +1,198 @@ +""" +This command cleans up course access roles for all users, for tenants with the FX Dashboard enabled. +""" +from __future__ import annotations + +import copy +from typing import Dict + +from common.djangoapps.student.models import CourseAccessRole +from django.contrib.auth import get_user_model +from django.core.management import BaseCommand, CommandParser + +from futurex_openedx_extensions.dashboard.serializers import UserRolesSerializer +from futurex_openedx_extensions.helpers import constants as cs +from futurex_openedx_extensions.helpers.exceptions import FXExceptionCodes +from futurex_openedx_extensions.helpers.roles import ( + cache_refresh_course_access_roles, + delete_course_access_roles, + get_user_course_access_roles, + update_course_access_roles, +) +from futurex_openedx_extensions.helpers.tenants import get_all_tenant_ids, get_course_org_filter_list +from futurex_openedx_extensions.helpers.users import is_system_staff_user + + +class Command(BaseCommand): + """ + Creates enrollment codes for courses. + """ + + help = 'Cleans up course access roles for all users' + + def __init__(self, *args, **kwargs): + """Initialize the command.""" + super().__init__(*args, **kwargs) + self.tenant_ids = get_all_tenant_ids() + self.superuser = get_user_model().objects.filter(is_superuser=True, is_active=True).first() + self.all_orgs = get_course_org_filter_list(self.tenant_ids)['course_org_filter_list'] + self.fake_request = type('Request', (object,), { + 'fx_permission_info': { + 'view_allowed_any_access_orgs': self.all_orgs, + 'view_allowed_tenant_ids_any_access': self.tenant_ids, + }, + 'query_params': {}, + }) + self.commit = False + + def add_arguments(self, parser: CommandParser) -> None: + """Add arguments to the command.""" + parser.add_argument( + '--commit', + action='store', + dest='commit', + default='no', + help='Commit changes, default is no (just perform a dry-run).', + type=str, + ) + + def _process_one_user(self, user): + """Process one user.""" + user_id = user.id + invalid_orgs = CourseAccessRole.objects.filter( + user_id=user_id, + ).exclude( + org__in=self.all_orgs, + ).values_list('org', flat=True).distinct() + if invalid_orgs: + print(f'**** User has invalid orgs in the roles: {list(invalid_orgs)}') + print('**** this must be fixed manually..') + if is_system_staff_user(user) or not user.is_active: + user_desc = 'a system staff' if is_system_staff_user(user) else 'not active' + print(f'**** User is {user_desc}, deleting all roles on all tenants..') + delete_course_access_roles( + caller=self.superuser, + tenant_ids=self.tenant_ids, + user=user, + dry_run=not self.commit, + ) + print('**** Done.') + return + + invalid_orgs = CourseAccessRole.objects.filter( + user_id=user_id, + ).exclude( + org__in=self.all_orgs, + ).exclude( + org='', + ).values_list('org', flat=True).distinct() + if invalid_orgs: + print(f'**** User has invalid orgs in the roles: {list(invalid_orgs)}') + print('**** this must be fixed manually..') + + empty_orgs = CourseAccessRole.objects.filter( + user_id=user_id, + org='', + ).exclude( + role__in=cs.COURSE_ACCESS_ROLES_GLOBAL, + ).values_list('org', flat=True).distinct() + if empty_orgs: + print('**** User has roles with no organization!') + print('**** this must be fixed manually..') + + unsupported_roles = CourseAccessRole.objects.filter( + user_id=user_id, + ).exclude( + role__in=cs.COURSE_ACCESS_ROLES_SUPPORTED_READ, + ).values_list('role', flat=True).distinct() + if unsupported_roles: + print(f'**** User has unsupported roles: {list(unsupported_roles)}') + print('**** this must be fixed manually..') + + roles = UserRolesSerializer(user, context={'request': self.fake_request}).data + for tenant_id in roles['tenants']: + tenant_roles = copy.deepcopy(roles['tenants'][tenant_id]) + tenant_roles['tenant_id'] = tenant_id + result = update_course_access_roles( + caller=self.superuser, + user=user, + new_roles_details=tenant_roles, + dry_run=not self.commit, + ) + if result['error_code']: + print(f'**** Failed for user {user_id}:{user.username}:{user.email} for tenant {tenant_id}..') + print(f'**** {result["error_code"]}:{result["error_message"]}') + self.print_helper_action(int(result['error_code'])) + + + def handle(self, *args: list, **options: Dict[str, str]) -> None: + """Handle the command.""" + self.commit = (str(options['commit']).lower() == 'yes') + + user_ids = CourseAccessRole.objects.values_list( + 'user_id', flat=True, + ).exclude( + course_id__startswith='library-v1:', # ignore library roles as they are not supported + ).distinct() + user_ids_to_clean = [] + + print('-' * 80) + print(f'{len(user_ids)} users to process..') + for user_id in user_ids: + cache_refresh_course_access_roles(user_id) + roles = get_user_course_access_roles(user_id) + if roles['useless_entries_exist']: + user_ids_to_clean.append(user_id) + if not user_ids_to_clean: + print('No dirty entries found..') + else: + print(f'Found {len(user_ids_to_clean)} users with dirty entries..') + + for user_id in user_ids_to_clean: + user = get_user_model().objects.get(id=user_id) + print(f'\nCleaning up user {user_id}:{user.username}:{user.email}...') + libraries_queryset = CourseAccessRole.objects.filter( + user_id=user_id, + course_id__startswith='library-v1:', + ) + library_roles = [] + for role_record in libraries_queryset: + library_roles.append(CourseAccessRole( + user_id=role_record.user_id, + role=role_record.role, + org=role_record.org, + course_id=role_record.course_id, + )) + if library_roles: + print(f'**** User {user_id} has library roles: {len(library_roles)}') + print('**** these will be removed before clean up then restored after..') + libraries_queryset.delete() + + try: + self._process_one_user(user) + except Exception as e: + print(f'**** Failed to process user {user_id}: {e}') + + if library_roles: + print('**** Restoring library roles..') + CourseAccessRole.objects.bulk_create(library_roles) + print('**** Restored.') + + if self.commit: + print('Operation completed..') + else: + print('Dry-run completed..') + + print('-' * 80) + + @staticmethod + def print_helper_action(code: int) -> None: + """Print helper action for the given error code.""" + message = None + if code == FXExceptionCodes.INVALID_INPUT.value: + message = ( + 'Please check the input data and try again. Some roles are not supported in the update ' + 'process and need to be removed manually.' + ) + if message: + print(f'**** {message}') diff --git a/futurex_openedx_extensions/helpers/roles.py b/futurex_openedx_extensions/helpers/roles.py index 34ef9e88..92989857 100644 --- a/futurex_openedx_extensions/helpers/roles.py +++ b/futurex_openedx_extensions/helpers/roles.py @@ -777,7 +777,9 @@ def _delete_course_access_roles(tenant_ids: list[int], user: get_user_model) -> cache_refresh_course_access_roles(user.id) -def delete_course_access_roles(caller: get_user_model, tenant_ids: list[int], user: get_user_model) -> None: +def delete_course_access_roles( + caller: get_user_model, tenant_ids: list[int], user: get_user_model, dry_run: bool = False, +) -> None: """ Delete the course access roles for the given tenant IDs and user @@ -787,12 +789,15 @@ def delete_course_access_roles(caller: get_user_model, tenant_ids: list[int], us :type tenant_ids: list :param user: The user to filter on :type user: get_user_model + :param dry_run: True for dry-run, False otherwise + :type dry_run: bool """ _verify_can_delete_course_access_roles(caller, tenant_ids, user) - _delete_course_access_roles(tenant_ids, user) + if not dry_run: + _delete_course_access_roles(tenant_ids, user) - cache_refresh_course_access_roles(user.id) + cache_refresh_course_access_roles(user.id) def _clean_course_access_roles(redundant_hashes: set[DictHashcode], user: get_user_model) -> None: diff --git a/tests/test_helpers/test_management.py b/tests/test_helpers/test_management.py new file mode 100644 index 00000000..22bd67c7 --- /dev/null +++ b/tests/test_helpers/test_management.py @@ -0,0 +1,68 @@ +"""Tests for management commands""" +from unittest.mock import patch + +import pytest +from common.djangoapps.student.models import CourseAccessRole +from django.contrib.auth import get_user_model +from django.core.management import call_command + +from futurex_openedx_extensions.helpers import constants as cs + +COMMAND_PATH = 'futurex_openedx_extensions.helpers.management.commands.course_access_roles_clean_up' + + +@pytest.mark.django_db +@pytest.mark.parametrize('options', [ + ['--commit=yes'], ['--commit=no'], [], +]) +def test_course_access_roles_clean_up_sanity_check_handler(base_data, options): # pylint: disable=unused-argument + """Sanity check for course_access_roles_clean_up command""" + assert CourseAccessRole.objects.filter(user_id=55, org='org1', role='staff').count() == 0 + CourseAccessRole.objects.create(user_id=55, org='org1', role='staff') + CourseAccessRole.objects.create(user_id=55, org='org1', role='staff', course_id='library-v1:the-lib+id') + with patch(f'{COMMAND_PATH}.update_course_access_roles', return_value={'error_code': None}): + call_command('course_access_roles_clean_up', *options) + assert CourseAccessRole.objects.filter(user_id=55, org='org1', role='staff').count() == 2 + + +@pytest.mark.django_db +@pytest.mark.parametrize('update_result', [ + {'error_code': 4001, 'error_message': 'Some error message'}, + {'error_code': 99999, 'error_message': 'Some error message'}, +]) +def test_course_access_roles_clean_up_sanity_check_errors(base_data, update_result): # pylint: disable=unused-argument + """Sanity check for course_access_roles_clean_up command""" + CourseAccessRole.objects.create(user_id=55, org='invalid_org') + get_user_model().objects.filter(id=1).update(is_active=False) + + with patch(f'{COMMAND_PATH}.update_course_access_roles', return_value=update_result): + call_command('course_access_roles_clean_up', '--commit=yes') + + +@pytest.mark.django_db +def test_course_access_roles_clean_up_delete_error(base_data, capfd): # pylint: disable=unused-argument + """Sanity check for course_access_roles_clean_up command""" + get_user_model().objects.filter(id=1).update(is_active=False) + with patch(f'{COMMAND_PATH}.delete_course_access_roles', side_effect=Exception('Some error for testing')): + call_command('course_access_roles_clean_up', '--commit=yes') + out, _ = capfd.readouterr() + assert 'Failed to process user' in out + assert 'Some error for testing' in out + + +@pytest.mark.django_db +def test_course_access_roles_clean_up_sanity_check_cleaning(base_data, capfd): # pylint: disable=unused-argument + """Sanity check for course_access_roles_clean_up command""" + CourseAccessRole.objects.filter(org='').delete() + CourseAccessRole.objects.filter(user_id__in=[1, 2]).delete() + CourseAccessRole.objects.exclude(role__in=cs.COURSE_ACCESS_ROLES_SUPPORTED_READ).delete() + + call_command('course_access_roles_clean_up', '--commit=yes') + out, _ = capfd.readouterr() + assert 'users with dirty entries' in out + assert 'No dirty entries found..' not in out + + call_command('course_access_roles_clean_up', '--commit=yes') + out, _ = capfd.readouterr() + assert 'users with dirty entries' not in out + assert 'No dirty entries found..' in out diff --git a/tests/test_helpers/test_roles.py b/tests/test_helpers/test_roles.py index 074074e4..a35a4962 100644 --- a/tests/test_helpers/test_roles.py +++ b/tests/test_helpers/test_roles.py @@ -1165,6 +1165,10 @@ def test_delete_course_access_roles(roles_authorize_caller, base_data): # pylin user=user70, org='', role=cs.COURSE_ACCESS_ROLES_UNSUPPORTED[0], ) + assert q_user70.count() == 6 + delete_course_access_roles(None, get_all_tenant_ids(), user70, dry_run=True) + assert q_user70.count() == 6 + delete_course_access_roles(None, get_all_tenant_ids(), user70) assert q_user70.count() == 4 for record in q_user70: