From 96ff100e9bf10f18ce49508b6bed8dc8caeebd6c Mon Sep 17 00:00:00 2001 From: Saad Ali Date: Tue, 28 Mar 2023 00:20:35 +0500 Subject: [PATCH] chore: Update retire_user management command (#31966) This update brings a django admin interface to upload a CSV file using the file access api. The management command is updated to access the uploaded files to retire users. --- openedx/core/djangoapps/user_api/admin.py | 16 ++++- .../management/commands/retire_user.py | 65 +++++++++++++------ .../management/tests/test_retire_user.py | 6 -- .../0005_bulkuserretirementconfig.py | 31 +++++++++ openedx/core/djangoapps/user_api/models.py | 18 ++++- 5 files changed, 107 insertions(+), 29 deletions(-) create mode 100644 openedx/core/djangoapps/user_api/migrations/0005_bulkuserretirementconfig.py diff --git a/openedx/core/djangoapps/user_api/admin.py b/openedx/core/djangoapps/user_api/admin.py index 20ad83b4b1..c1de6490ed 100644 --- a/openedx/core/djangoapps/user_api/admin.py +++ b/openedx/core/djangoapps/user_api/admin.py @@ -11,7 +11,13 @@ from django.utils.translation import gettext as _ from openedx.core.djangoapps.user_api.accounts.forms import RetirementQueueDeletionForm -from .models import RetirementState, UserRetirementPartnerReportingStatus, UserRetirementRequest, UserRetirementStatus +from .models import ( + BulkUserRetirementConfig, + RetirementState, + UserRetirementPartnerReportingStatus, + UserRetirementRequest, + UserRetirementStatus +) @admin.register(RetirementState) @@ -191,3 +197,11 @@ class UserRetirementPartnerReportingStatusAdmin(admin.ModelAdmin): self.message_user(request, "%s successfully reset." % message_bit) reset_state.short_description = 'Reset is_being_processed to False' + + +@admin.register(BulkUserRetirementConfig) +class BulkUserRetirementConfigurationAdmin(admin.ModelAdmin): + """ + Admin for BulkUserRetirementConfig model. + """ + list_display = ('id', 'enabled', 'changed_by', 'change_date') diff --git a/openedx/core/djangoapps/user_api/management/commands/retire_user.py b/openedx/core/djangoapps/user_api/management/commands/retire_user.py index 7943870617..60a8634a1f 100644 --- a/openedx/core/djangoapps/user_api/management/commands/retire_user.py +++ b/openedx/core/djangoapps/user_api/management/commands/retire_user.py @@ -10,7 +10,7 @@ from social_django.models import UserSocialAuth from common.djangoapps.student.models import AccountRecovery, Registration, get_retired_email_by_email from openedx.core.djangolib.oauth2_retirement_utils import retire_dot_oauth2_models -from ...models import UserRetirementStatus +from ...models import BulkUserRetirementConfig, UserRetirementStatus logger = logging.getLogger(__name__) @@ -48,6 +48,35 @@ class Command(BaseCommand): help='Comma separated file that have username and user_email of the users that needs to be retired' ) + def append_users_lists(self, file_handler, user_model): + """ + Function to extract users list from CSV files. + """ + unknown_users = [] + users = [] + if file_handler is None: + raise CommandError("No user file specified") + + try: + if isinstance(file_handler, str): + userinfo = open(file_handler, 'r') + else: + userinfo = file_handler.open('r') + except Exception as exc: + error_message = f'Error while reading file: {exc}' + logger.error(error_message) + raise CommandError(error_message) # lint-amnesty, pylint: disable=raise-missing-from + + for record in userinfo: + userdata = record.split(',') + username = userdata[0].strip() + user_email = userdata[1].strip() + try: + users.append(User.objects.get(username=username, email=user_email)) + except user_model.DoesNotExist: + unknown_users.append({username: user_email}) + return users, unknown_users + def check_user_exist(self, user_model, userfile, user_name, useremail): """ Function to check if user exists. This function will execute for userfile @@ -63,21 +92,7 @@ class Command(BaseCommand): user = user_name email = useremail if userfile and (not user and not email): - try: - userinfo = open(userfile, 'r') - except Exception as exc: - error_message = f'Error while reading file: {exc}' - logger.error(error_message) - raise CommandError(error_message) # lint-amnesty, pylint: disable=raise-missing-from - - for record in userinfo: - userdata = record.split(',') - username = userdata[0].strip() - user_email = userdata[1].strip() - try: - users.append(User.objects.get(username=username, email=user_email)) - except user_model.DoesNotExist: - unknown_users.append({username: user_email}) + users, unknown_users = self.append_users_lists(userfile, user_model) elif (user and email) and not userfile: try: users.append(User.objects.get(username=user, email=email)) @@ -97,13 +112,21 @@ class Command(BaseCommand): """ Execute the command. """ - userfile = options['user_file'] - user_name = options['username'] - useremail = options['user_email'] - + users = [] + unknown_users = [] user_model = get_user_model() - users, unknown_users = self.check_user_exist(user_model, userfile, user_name, useremail) + if not options['username'] and not options['user_email'] and not options['user_file']: + bulk_user_retirement_config = BulkUserRetirementConfig.current() + file_handle = bulk_user_retirement_config.csv_file if bulk_user_retirement_config.is_enabled() else None + users, unknown_users = self.append_users_lists(file_handle, user_model) + else: + userfile = options['user_file'] + user_name = options['username'] + useremail = options['user_email'] + + users, unknown_users = self.check_user_exist(user_model, userfile, user_name, useremail) + # Raise if found any such user that does not exit if len(unknown_users) > 0: error_message = ( diff --git a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py index c3922f8e0a..023efb54f4 100644 --- a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py +++ b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py @@ -82,12 +82,6 @@ def test_retire_user_with_usename_email_mismatch(setup_retirement_states): # li remove_user_file() -@skip_unless_lms -def test_retire_user_without_usename_email_userfile(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument - with pytest.raises(CommandError, match=r'Please provide user_file or username and user_email '): - call_command('retire_user') - - @skip_unless_lms def test_successful_retire_with_username_email(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument user = UserFactory.create(username='user0', email="user0@example.com") diff --git a/openedx/core/djangoapps/user_api/migrations/0005_bulkuserretirementconfig.py b/openedx/core/djangoapps/user_api/migrations/0005_bulkuserretirementconfig.py new file mode 100644 index 0000000000..af2508dea6 --- /dev/null +++ b/openedx/core/djangoapps/user_api/migrations/0005_bulkuserretirementconfig.py @@ -0,0 +1,31 @@ +# Generated by Django 3.2.18 on 2023-03-22 10:34 + +from django.conf import settings +import django.core.validators +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('user_api', '0004_userretirementpartnerreportingstatus'), + ] + + operations = [ + migrations.CreateModel( + name='BulkUserRetirementConfig', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('csv_file', models.FileField(help_text='Comma separated file that have username and user_email of the users that needs to be retired', upload_to='bulk_user_retirement_files/', validators=[django.core.validators.FileExtensionValidator(allowed_extensions=['csv'])])), + ('changed_by', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL, verbose_name='Changed by')), + ], + options={ + 'ordering': ('-change_date',), + 'abstract': False, + }, + ), + ] diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index d9f91fe3e7..6c1dd832a3 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -4,10 +4,13 @@ Django ORM model specifications for the User API application from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user -from django.core.validators import RegexValidator +from django.core.validators import FileExtensionValidator, RegexValidator from django.db import models from django.db.models.signals import post_delete, post_save, pre_save from django.dispatch import receiver +from django.utils.translation import gettext_lazy as _ + +from config_models.models import ConfigurationModel from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField @@ -420,6 +423,19 @@ class UserRetirementStatus(TimeStampedModel): return f'User: {self.user.id} State: {self.current_state} Last Updated: {self.modified}' +class BulkUserRetirementConfig(ConfigurationModel): + """ + Configuration to store a csv file that will be used in retire_user management command. + """ + # Timeout set to 0 so that the model does not read from cached config in case the config entry is deleted. + cache_timeout = 0 + csv_file = models.FileField( + upload_to="bulk_user_retirement_files/", + validators=[FileExtensionValidator(allowed_extensions=['csv'])], + help_text=_("Comma separated file that have username and user_email of the users that needs to be retired") + ) + + @receiver(models.signals.post_delete, sender=UserRetirementStatus) def remove_pending_retirement_request(sender, instance, **kwargs): # pylint: disable=unused-argument """