diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 439cf32c9a415e033e90ba0604d573ca09babe72..865717bf708f1dbe21be262dde9ff6423ec64153 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -160,7 +160,7 @@ def get_certificates_for_user_by_course_keys(user, course_keys): } -def get_recently_modified_certificates(course_keys=None, start_date=None, end_date=None, usernames=None): +def get_recently_modified_certificates(course_keys=None, start_date=None, end_date=None, user_ids=None): """ Returns a QuerySet of GeneratedCertificate objects filtered by the input parameters and ordered by modified_date. @@ -176,8 +176,8 @@ def get_recently_modified_certificates(course_keys=None, start_date=None, end_da if end_date: cert_filter_args['modified_date__lte'] = end_date - if usernames: - cert_filter_args['user__username__in'] = usernames + if user_ids: + cert_filter_args['user__id__in'] = user_ids return GeneratedCertificate.objects.filter(**cert_filter_args).order_by('modified_date') diff --git a/lms/djangoapps/certificates/management/commands/cert_allowlist_generation.py b/lms/djangoapps/certificates/management/commands/cert_allowlist_generation.py index 5e184d9d039a267a5bc49db87adbe5905b1f8025..7843daaea511eed4650bf825ca4e6b8ceb8b93b7 100644 --- a/lms/djangoapps/certificates/management/commands/cert_allowlist_generation.py +++ b/lms/djangoapps/certificates/management/commands/cert_allowlist_generation.py @@ -22,7 +22,7 @@ class Command(BaseCommand): Management command to generate allowlist certificates for one or more users in a given course run. Example usage: - ./manage.py lms cert_allowlist_generation -u edx verified -c course-v1:edX+DemoX+Demo_Course + ./manage.py lms cert_allowlist_generation -u 123 verified -c course-v1:edX+DemoX+Demo_Course """ help = """ @@ -35,7 +35,7 @@ class Command(BaseCommand): nargs="+", metavar='USER', dest='user', - help='user or space-separated list of users for whom to generate allowlist certificates' + help='user_id or space-separated list of user_ids for whom to generate allowlist certificates' ) parser.add_argument( '-c', '--course-key', @@ -80,8 +80,12 @@ class Command(BaseCommand): # Loop over each user, and ask that a cert be generated for them users_str = options['user'] - for user_identifier in users_str: - user = _get_user_from_identifier(user_identifier) + for user_id in users_str: + user = None + try: + user = User.objects.get(id=user_id) + except User.DoesNotExist: + log.warning('User {user} could not be found'.format(user=user_id)) if user is not None: log.info( 'Calling generate_allowlist_certificate_task for {user} : {course}'.format( @@ -89,18 +93,3 @@ class Command(BaseCommand): course=course_key )) generate_allowlist_certificate_task(user, course_key) - - -def _get_user_from_identifier(identifier): - """ - Using the string identifier, fetch the relevant user object from database - """ - try: - if '@' in identifier: - user = User.objects.get(email=identifier) - else: - user = User.objects.get(username=identifier) - return user - except User.DoesNotExist: - log.warning('User {user} could not be found'.format(user=identifier)) - return None diff --git a/lms/djangoapps/certificates/management/commands/tests/test_cert_allowlist_generation.py b/lms/djangoapps/certificates/management/commands/tests/test_cert_allowlist_generation.py index 4228f0815695c4105c9fd2778747bcf639566e72..1274341115dade161c298e95d81c61a4d52c98b3 100644 --- a/lms/djangoapps/certificates/management/commands/tests/test_cert_allowlist_generation.py +++ b/lms/djangoapps/certificates/management/commands/tests/test_cert_allowlist_generation.py @@ -71,7 +71,7 @@ class CertAllowlistGenerationTests(ModuleStoreTestCase): """ Test generation for 1 user """ - call_command("cert_allowlist_generation", "--u", self.user.username, "--c", self.course_run_key) + call_command("cert_allowlist_generation", "--u", self.user.id, "--c", self.course_run_key) def test_successful_generation_multiple_users(self): """ @@ -79,9 +79,8 @@ class CertAllowlistGenerationTests(ModuleStoreTestCase): """ call_command("cert_allowlist_generation", "--u", - self.user.username, - self.user2.email, - "invalid_user", - " ", + self.user.id, + self.user2.id, + "999999", # non-existant userid "--c", self.course_run_key) diff --git a/lms/djangoapps/certificates/migrations/0020_remove_existing_mgmt_cmd_args.py b/lms/djangoapps/certificates/migrations/0020_remove_existing_mgmt_cmd_args.py new file mode 100644 index 0000000000000000000000000000000000000000..10ac8cd52839f0961cd7647ebc61c36d81cd53b2 --- /dev/null +++ b/lms/djangoapps/certificates/migrations/0020_remove_existing_mgmt_cmd_args.py @@ -0,0 +1,21 @@ +# Generated by Django 2.2.18 on 2021-02-18 22:05 + +from django.db import migrations + + +class Migration(migrations.Migration): + """ + Delete all existing AllowListGenerationConfiguration entries because they may contain PII prior to this change + """ + + def remove_existing_configs(apps, schema_editor): + AllowListGenerationConfiguration = apps.get_model('certificates', 'AllowListGenerationConfiguration') + AllowListGenerationConfiguration.objects.all().delete() + + dependencies = [ + ('certificates', '0019_allowlistgenerationconfiguration'), + ] + + operations = [ + migrations.RunPython(remove_existing_configs) + ] diff --git a/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py b/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py index 23fa60dacc407ef0f17e7e4669b0bc652bb81ef4..96f9c8bda80e7d08f020f7f64192c8382dfe87a6 100644 --- a/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py +++ b/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py @@ -162,7 +162,7 @@ class Command(BaseCommand): help='Send program award notifications with course notification tasks', ) parser.add_argument( - '--usernames', + '--user_ids', default=None, nargs='+', help='Run the command for the given user or list of users', @@ -190,7 +190,7 @@ class Command(BaseCommand): log.info( u"notify_credentials starting, dry-run=%s, site=%s, delay=%d seconds, page_size=%d, " - u"from=%s, to=%s, notify_programs=%s, usernames=%s, execution=%s", + u"from=%s, to=%s, notify_programs=%s, user_ids=%s, execution=%s", options['dry_run'], options['site'], options['delay'], @@ -198,7 +198,7 @@ class Command(BaseCommand): options['start_date'] if options['start_date'] else 'NA', options['end_date'] if options['end_date'] else 'NA', options['notify_programs'], - options['usernames'], + options['user_ids'], 'auto' if options['auto'] else 'manual', ) @@ -208,16 +208,16 @@ class Command(BaseCommand): log.error(u'No site configuration found for site %s', options['site']) course_keys = self.get_course_keys(options['courses']) - if not (course_keys or options['start_date'] or options['end_date'] or options['usernames']): - raise CommandError('You must specify a filter (e.g. --courses= or --start-date or --usernames)') + if not (course_keys or options['start_date'] or options['end_date'] or options['user_ids']): + raise CommandError('You must specify a filter (e.g. --courses= or --start-date or --user_ids)') certs = get_recently_modified_certificates( - course_keys, options['start_date'], options['end_date'], options['usernames'] + course_keys, options['start_date'], options['end_date'], options['user_ids'] ) users = None - if options['usernames']: - users = User.objects.filter(username__in=options['usernames']) + if options['user_ids']: + users = User.objects.filter(id__in=options['user_ids']) grades = get_recently_modified_grades( course_keys, options['start_date'], options['end_date'], users ) diff --git a/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py b/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py index 6c540681b6db9b637b38036586923878ff273d54..934ab651d64a05d2233a1ab926af790aa0ddb4ff 100644 --- a/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py +++ b/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py @@ -126,7 +126,7 @@ class TestNotifyCredentials(TestCase): @mock.patch(COMMAND_MODULE + '.Command.send_notifications') def test_username_arg(self, mock_send): call_command( - Command(), '--start-date', '2017-02-01', '--end-date', '2017-02-02', '--usernames', self.user2.username + Command(), '--start-date', '2017-02-01', '--end-date', '2017-02-02', '--user_ids', self.user2.id ) self.assertTrue(mock_send.called) self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert4]) @@ -134,7 +134,7 @@ class TestNotifyCredentials(TestCase): mock_send.reset_mock() call_command( - Command(), '--usernames', self.user2.username + Command(), '--user_ids', self.user2.id ) self.assertTrue(mock_send.called) self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert4]) @@ -142,7 +142,7 @@ class TestNotifyCredentials(TestCase): mock_send.reset_mock() call_command( - Command(), '--start-date', '2017-02-01', '--end-date', '2017-02-02', '--usernames', self.user.username + Command(), '--start-date', '2017-02-01', '--end-date', '2017-02-02', '--user_ids', self.user.id ) self.assertTrue(mock_send.called) self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert2]) @@ -150,7 +150,7 @@ class TestNotifyCredentials(TestCase): mock_send.reset_mock() call_command( - Command(), '--usernames', self.user.username + Command(), '--user_ids', self.user.id ) self.assertTrue(mock_send.called) self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert1, self.cert2, self.cert3]) @@ -158,7 +158,7 @@ class TestNotifyCredentials(TestCase): mock_send.reset_mock() call_command( - Command(), '--usernames', self.user.username, self.user2.username + Command(), '--user_ids', self.user.id, self.user2.id ) self.assertTrue(mock_send.called) self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert1, self.cert2, self.cert4, self.cert3]) diff --git a/openedx/core/djangoapps/credentials/migrations/0005_remove_existing_mgmt_cmd_args.py b/openedx/core/djangoapps/credentials/migrations/0005_remove_existing_mgmt_cmd_args.py new file mode 100644 index 0000000000000000000000000000000000000000..2781bc3b027fc2306d9cb931eaa3236c3ed920a6 --- /dev/null +++ b/openedx/core/djangoapps/credentials/migrations/0005_remove_existing_mgmt_cmd_args.py @@ -0,0 +1,20 @@ +# Generated by Django 2.2.18 on 2021-02-18 22:01 + +from django.db import migrations + + +class Migration(migrations.Migration): + """ + Delete all existing NotifyCredentialsConfig entries because they may contain PII prior to this change + """ + def remove_existing_configs(apps, schema_editor): + NotifyCredentialsConfig = apps.get_model('credentials', 'NotifyCredentialsConfig') + NotifyCredentialsConfig.objects.all().delete() + + dependencies = [ + ('credentials', '0004_notifycredentialsconfig'), + ] + + operations = [ + migrations.RunPython(remove_existing_configs) + ]