From ce50c9e620405b5452220fa0965200ecab67a491 Mon Sep 17 00:00:00 2001 From: bmedx Date: Mon, 30 Oct 2017 14:21:06 -0400 Subject: [PATCH] Student management command cleanup for Django 1.11 --- .../management/commands/add_to_group.py | 46 ++++----- .../commands/anonymized_id_mapping.py | 14 +-- .../management/commands/assigngroups.py | 63 ++++++------ .../commands/bulk_change_enrollment.py | 67 +++++-------- .../management/commands/cert_restriction.py | 99 ++++++++----------- .../management/commands/change_enrollment.py | 84 ++++++++-------- .../commands/create_random_users.py | 23 ++--- .../management/commands/create_user.py | 86 +++++++--------- ...populate_created_on_site_user_attribute.py | 5 +- .../student/management/commands/set_staff.py | 56 +++++------ .../management/commands/set_superuser.py | 32 +++--- .../tests/test_bulk_change_enrollment.py | 62 +++++++----- .../tests/test_change_enrollment.py | 37 ++++--- 13 files changed, 317 insertions(+), 357 deletions(-) diff --git a/common/djangoapps/student/management/commands/add_to_group.py b/common/djangoapps/student/management/commands/add_to_group.py index 66f0f4eaca..28e5b3582b 100644 --- a/common/djangoapps/student/management/commands/add_to_group.py +++ b/common/djangoapps/student/management/commands/add_to_group.py @@ -1,45 +1,39 @@ -from optparse import make_option +from __future__ import print_function from django.core.management.base import BaseCommand, CommandError from django.contrib.auth.models import User, Group class Command(BaseCommand): - option_list = BaseCommand.option_list + ( - make_option('--list', - action='store_true', - dest='list', - default=False, - help='List available groups'), - make_option('--create', - action='store_true', - dest='create', - default=False, - help='Create the group if it does not exist'), - make_option('--remove', - action='store_true', - dest='remove', - default=False, - help='Remove the user from the group instead of adding it'), - ) + def add_arguments(self, parser): + parser.add_argument('name_or_email', + help='Username or email address of the user to add or remove') + parser.add_argument('group_name', + help='Name of the group to change') + parser.add_argument('--list', + action='store_true', + help='List available groups') + parser.add_argument('--create', + action='store_true', + help='Create the group if it does not exist') + parser.add_argument('--remove', + action='store_true', + help='Remove the user from the group instead of adding it') - args = ' ' help = 'Add a user to a group' def print_groups(self): - print 'Groups available:' + print('Groups available:') for group in Group.objects.all().distinct(): - print ' ', group.name + print(' {}'.format(group.name)) def handle(self, *args, **options): if options['list']: self.print_groups() return - if len(args) != 2: - raise CommandError('Usage is add_to_group {0}'.format(self.args)) - - name_or_email, group_name = args + name_or_email = options['name_or_email'] + group_name = options['group_name'] if '@' in name_or_email: user = User.objects.get(email=name_or_email) @@ -60,4 +54,4 @@ class Command(BaseCommand): else: user.groups.add(group) - print 'Success!' + print('Success!') diff --git a/common/djangoapps/student/management/commands/anonymized_id_mapping.py b/common/djangoapps/student/management/commands/anonymized_id_mapping.py index ce08b39446..6315facc65 100644 --- a/common/djangoapps/student/management/commands/anonymized_id_mapping.py +++ b/common/djangoapps/student/management/commands/anonymized_id_mapping.py @@ -20,23 +20,17 @@ from opaque_keys.edx.keys import CourseKey class Command(BaseCommand): """Add our handler to the space where django-admin looks up commands.""" - # TODO: revisit now that rake has been deprecated - # It appears that with the way Rake invokes these commands, we can't - # have more than one arg passed through...annoying. - args = ("course_id", ) - help = """Export a CSV mapping usernames to anonymized ids Exports a CSV document mapping each username in the specified course to the anonymized, unique user ID. """ - def handle(self, *args, **options): - if len(args) != 1: - raise CommandError("Usage: unique_id_mapping %s" % - " ".join(("<%s>" % arg for arg in Command.args))) + def add_arguments(self, parser): + parser.add_argument('course_id') - course_key = CourseKey.from_string(args[0]) + def handle(self, *args, **options): + course_key = CourseKey.from_string(options['course_id']) # Generate the output filename from the course ID. # Change slashes to dashes first, and then append .csv extension. diff --git a/common/djangoapps/student/management/commands/assigngroups.py b/common/djangoapps/student/management/commands/assigngroups.py index 166e5cab16..ed5bf08ef5 100644 --- a/common/djangoapps/student/management/commands/assigngroups.py +++ b/common/djangoapps/student/management/commands/assigngroups.py @@ -1,3 +1,5 @@ +from __future__ import print_function + from django.core.management.base import BaseCommand from django.contrib.auth.models import User @@ -11,22 +13,27 @@ from textwrap import dedent import json from pytz import UTC +# Examples: +# python manage.py assigngroups summary_test:0.3,skip_summary_test:0.7 log.txt "Do previews of future materials help?" +# python manage.py assigngroups skip_capacitor:0.3,capacitor:0.7 log.txt "Do we show capacitor in linearity tutorial?" + def group_from_value(groups, v): - ''' Given group: (('a',0.3),('b',0.4),('c',0.3)) And random value + """ + Given group: (('a',0.3),('b',0.4),('c',0.3)) And random value in [0,1], return the associated group (in the above case, return 'a' if v<0.3, 'b' if 0.3<=v<0.7, and 'c' if v>0.7 -''' - sum = 0 - for (g, p) in groups: - sum = sum + p - if sum > v: - return g - return g # For round-off errors + """ + curr_sum = 0 + for (group, p_value) in groups: + curr_sum = curr_sum + p_value + if curr_sum > v: + return group + return group # For round-off errors class Command(BaseCommand): - help = dedent("""\ + help = dedent(""" Assign users to test groups. Takes a list of groups: a:0.3,b:0.4,c:0.3 file.txt "Testing something" Will assign each user to group a, b, or c with @@ -36,48 +43,49 @@ class Command(BaseCommand): Will log what happened to file.txt. """) + def add_arguments(self, parser): + parser.add_argument('group_and_score') + parser.add_argument('log_name') + parser.add_argument('description') + def handle(self, *args, **options): - if len(args) != 3: - print "Invalid number of options" - sys.exit(-1) - # Extract groups from string - group_strs = [x.split(':') for x in args[0].split(',')] + group_strs = [x.split(':') for x in options['group_and_score'].split(',')] groups = [(group, float(value)) for group, value in group_strs] - print "Groups", groups + print("Groups", groups) - ## Confirm group probabilities add up to 1 + # Confirm group probabilities add up to 1 total = sum(zip(*groups)[1]) - print "Total:", total + print("Total:", total) if abs(total - 1) > 0.01: - print "Total not 1" + print("Total not 1") sys.exit(-1) - ## Confirm groups don't already exist + # Confirm groups don't already exist for group in dict(groups): if UserTestGroup.objects.filter(name=group).count() != 0: - print group, "already exists!" + print(group, "already exists!") sys.exit(-1) group_objects = {} - f = open(args[1], "a+") + f = open(options['log_name'], "a+") - ## Create groups + # Create groups for group in dict(groups): utg = UserTestGroup() utg.name = group - utg.description = json.dumps({"description": args[2]}, + utg.description = json.dumps({"description": options['description']}, {"time": datetime.datetime.now(UTC).isoformat()}) group_objects[group] = utg group_objects[group].save() - ## Assign groups + # Assign groups users = list(User.objects.all()) count = 0 for user in users: if count % 1000 == 0: - print count + print(count) count = count + 1 v = random.uniform(0, 1) group = group_from_value(groups, v) @@ -88,10 +96,7 @@ class Command(BaseCommand): group=group ).encode('utf-8')) - ## Save groups + # Save groups for group in group_objects: group_objects[group].save() f.close() - -# python manage.py assigngroups summary_test:0.3,skip_summary_test:0.7 log.txt "Do previews of future materials help?" -# python manage.py assigngroups skip_capacitor:0.3,capacitor:0.7 log.txt "Do we show capacitor in linearity tutorial?" diff --git a/common/djangoapps/student/management/commands/bulk_change_enrollment.py b/common/djangoapps/student/management/commands/bulk_change_enrollment.py index 569dc78488..9dbda939c0 100644 --- a/common/djangoapps/student/management/commands/bulk_change_enrollment.py +++ b/common/djangoapps/student/management/commands/bulk_change_enrollment.py @@ -6,6 +6,7 @@ from django.db import transaction from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from optparse import make_option +from six import text_type from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from course_modes.models import CourseMode @@ -31,54 +32,36 @@ class Command(BaseCommand): Without the --commit option, the command will have no effect. """ - option_list = BaseCommand.option_list + ( - make_option( - '-f', '--from_mode', - dest='from_mode', - default=None, - help='move from this enrollment mode' - ), - make_option( - '-t', '--to_mode', - dest='to_mode', - default=None, - help='move to this enrollment mode' - ), - make_option( + def add_arguments(self, parser): + group = parser.add_mutually_exclusive_group() + group.add_argument( '-c', '--course', - dest='course', - default=None, - help='the course to change enrollments in' - ), - make_option( + help='The course to change enrollments in') + group.add_argument( '-o', '--org', - dest='org', - default=None, - help='all courses belonging to this org will be selected for changing the enrollments' - ), - make_option( + help='All courses belonging to this org will be selected for changing the enrollments') + + parser.add_argument( + '-f', '--from_mode', + required=True, + help='Move from this enrollment mode') + parser.add_argument( + '-t', '--to_mode', + required=True, + help='Move to this enrollment mode') + parser.add_argument( '--commit', action='store_true', - dest='commit', - default=False, - help='display what will be done without any effect' - ) - ) + help='Save the changes, without this flag only a dry run will be performed and nothing will be changed') def handle(self, *args, **options): - course_id = options.get('course') - org = options.get('org') - from_mode = options.get('from_mode') - to_mode = options.get('to_mode') - commit = options.get('commit') - - if (not course_id and not org) or (course_id and org): - raise CommandError('You must provide either a course ID or an org, but not both.') - - if from_mode is None or to_mode is None: - raise CommandError('Both `from` and `to` course modes must be given.') - + course_id = options['course'] + org = options['org'] + from_mode = options['from_mode'] + to_mode = options['to_mode'] + commit = options['commit'] course_keys = [] + if course_id: try: course_key = CourseKey.from_string(course_id) @@ -111,7 +94,7 @@ class Command(BaseCommand): commit (bool): required to make the change to the database. Otherwise just a count will be displayed. """ - unicode_course_key = unicode(course_key) + unicode_course_key = text_type(course_key) if CourseMode.mode_for_course(course_key, to_mode) is None: logger.info('Mode ({}) does not exist for course ({}).'.format(to_mode, unicode_course_key)) return diff --git a/common/djangoapps/student/management/commands/cert_restriction.py b/common/djangoapps/student/management/commands/cert_restriction.py index c43ff05f3e..f57ccf70d5 100644 --- a/common/djangoapps/student/management/commands/cert_restriction.py +++ b/common/djangoapps/student/management/commands/cert_restriction.py @@ -1,12 +1,14 @@ -from django.core.management.base import BaseCommand, CommandError -import os -from optparse import make_option -from student.models import UserProfile +from __future__ import print_function + import csv +import os + +from django.core.management.base import BaseCommand, CommandError + +from student.models import UserProfile class Command(BaseCommand): - help = """ Sets or gets certificate restrictions for users from embargoed countries. (allow_certificate in @@ -31,79 +33,62 @@ class Command(BaseCommand): """ - option_list = BaseCommand.option_list + ( - make_option('-i', '--import', - metavar='IMPORT_FILE', - dest='import', - default=False, - help='csv file to import, comma delimitted file with ' - 'double-quoted entries'), - make_option('-o', '--output', - metavar='EXPORT_FILE', - dest='output', - default=False, - help='csv file to export'), - make_option('-e', '--enable', - metavar='STUDENT', - dest='enable', - default=False, - help="enable a single student's certificate"), - make_option('-d', '--disable', - metavar='STUDENT', - dest='disable', - default=False, - help="disable a single student's certificate") - ) + def add_arguments(self, parser): + # This command can only take one of these arguments per run, this enforces that. + group = parser.add_mutually_exclusive_group(required=True) + group.add_argument('-i', '--import', + metavar='IMPORT_FILE', + nargs='?', + help='CSV file to import, comma delimitted file with double-quoted entries') + group.add_argument('-o', '--output', + metavar='EXPORT_FILE', + nargs='?', + help='CSV file to export') + group.add_argument('-e', '--enable', + metavar='STUDENT', + nargs='?', + help='Enable a certificate for a single student') + group.add_argument('-d', '--disable', + metavar='STUDENT', + nargs='?', + help='Disable a certificate for a single student') def handle(self, *args, **options): if options['output']: - if os.path.exists(options['output']): - raise CommandError("File {0} already exists".format( - options['output'])) - disabled_users = UserProfile.objects.filter( - allow_certificate=False) + raise CommandError("File {0} already exists".format(options['output'])) + disabled_users = UserProfile.objects.filter(allow_certificate=False) with open(options['output'], 'w') as csvfile: - csvwriter = csv.writer(csvfile, delimiter=',', quotechar='"', - quoting=csv.QUOTE_MINIMAL) + csvwriter = csv.writer(csvfile, delimiter=',', quotechar='"', quoting=csv.QUOTE_MINIMAL) for user in disabled_users: csvwriter.writerow([user.user.username]) + print('{} disabled users written'.format(len(disabled_users))) elif options['import']: - if not os.path.exists(options['import']): - raise CommandError("File {0} does not exist".format( - options['import'])) + raise CommandError("File {0} does not exist".format(options['import'])) - print "Importing students from {0}".format(options['import']) + print("Importing students from {0}".format(options['import'])) - students = None with open(options['import']) as csvfile: - student_list = csv.reader(csvfile, delimiter=',', - quotechar='"') + student_list = csv.reader(csvfile, delimiter=',', quotechar='"') students = [student[0] for student in student_list] + if not students: - raise CommandError( - "Unable to read student data from {0}".format( - options['import'])) - UserProfile.objects.filter(user__username__in=students).update( - allow_certificate=False) + raise CommandError("Unable to read student data from {0}".format(options['import'])) + + update_cnt = UserProfile.objects.filter(user__username__in=students).update(allow_certificate=False) + print('{} user(s) disabled out of {} in CSV file'.format(update_cnt, len(students))) elif options['enable']: - - print "Enabling {0} for certificate download".format( - options['enable']) - cert_allow = UserProfile.objects.get( - user__username=options['enable']) + print("Enabling {0} for certificate download".format(options['enable'])) + cert_allow = UserProfile.objects.get(user__username=options['enable']) cert_allow.allow_certificate = True cert_allow.save() elif options['disable']: - - print "Disabling {0} for certificate download".format( - options['disable']) - cert_allow = UserProfile.objects.get( - user__username=options['disable']) + print("Disabling {0} for certificate download".format(options['disable'])) + cert_allow = UserProfile.objects.get(user__username=options['disable']) cert_allow.allow_certificate = False cert_allow.save() diff --git a/common/djangoapps/student/management/commands/change_enrollment.py b/common/djangoapps/student/management/commands/change_enrollment.py index 38c12d8d0d..efd0e51b6c 100644 --- a/common/djangoapps/student/management/commands/change_enrollment.py +++ b/common/djangoapps/student/management/commands/change_enrollment.py @@ -4,8 +4,8 @@ import logging from django.core.management.base import BaseCommand, CommandError from django.db import transaction +from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey -from optparse import make_option from student.models import CourseEnrollment, User @@ -33,64 +33,60 @@ class Command(BaseCommand): Or - $ ... change_enrollment -e "joe@example.com,frank@example.com,bill@example.com" -c some/course/id --from audit --to honor + $ ... change_enrollment -e "joe@example.com,frank@example.com,..." -c some/course/id --from audit --to honor See what would have been changed from audit to honor without making that change $ ... change_enrollment -u joe,frank,bill -c some/course/id --from audit --to honor -n - """ - option_list = BaseCommand.option_list + ( - make_option('-f', '--from', - metavar='FROM_MODE', - dest='from_mode', - default=False, - help='move from this enrollment mode'), - make_option('-t', '--to', - metavar='TO_MODE', - dest='to_mode', - default=False, - help='move to this enrollment mode'), - make_option('-u', '--usernames', - metavar='USERNAME', - dest='username', - default=False, - help="Comma-separated list of usernames to move in the course"), - make_option('-e', '--emails', - metavar='EMAIL', - dest='email', - default=False, - help="Comma-separated list of email addresses to move in the course"), - make_option('-c', '--course', - metavar='COURSE_ID', - dest='course_id', - default=False, - help="course id to use for transfer"), - make_option('-n', '--noop', - action='store_true', - dest='noop', - default=False, - help="display what will be done but don't actually do anything") + enrollment_modes = ('audit', 'verified', 'honor') - ) + def add_arguments(self, parser): + parser.add_argument('-f', '--from', + metavar='FROM_MODE', + dest='from_mode', + required=True, + choices=self.enrollment_modes, + help='Move from this enrollment mode') + parser.add_argument('-t', '--to', + metavar='TO_MODE', + dest='to_mode', + required=True, + choices=self.enrollment_modes, + help='Move to this enrollment mode') + parser.add_argument('-u', '--username', + metavar='USERNAME', + help='Comma-separated list of usernames to move in the course') + parser.add_argument('-e', '--email', + metavar='EMAIL', + help='Comma-separated list of email addresses to move in the course') + parser.add_argument('-c', '--course', + metavar='COURSE_ID', + dest='course_id', + required=True, + help='Course id to use for transfer') + parser.add_argument('-n', '--noop', + action='store_true', + help='Display what will be done but do not actually do anything') def handle(self, *args, **options): - error_users = [] - success_users = [] + try: + course_key = CourseKey.from_string(options['course_id']) + except InvalidKeyError: + raise CommandError('Invalid or non-existant course id {}'.format(options['course_id'])) - if not options['course_id']: - raise CommandError('You must specify a course id for this command') - if not options['from_mode'] or not options['to_mode']: - raise CommandError('You must specify a "to" and "from" mode as parameters') - - course_key = CourseKey.from_string(options['course_id']) + if not options['username'] and not options['email']: + raise CommandError('You must include usernames (-u) or emails (-e) to select users to update') enrollment_args = dict( course_id=course_key, mode=options['from_mode'] ) + error_users = [] + success_users = [] + if options['username']: self.update_enrollments('username', enrollment_args, options, error_users, success_users) @@ -102,8 +98,10 @@ class Command(BaseCommand): def update_enrollments(self, identifier, enrollment_args, options, error_users, success_users): """ Update enrollments for a specific user identifier (email or username). """ users = options[identifier].split(",") + for identified_user in users: logger.info(identified_user) + try: user_args = { identifier: identified_user diff --git a/common/djangoapps/student/management/commands/create_random_users.py b/common/djangoapps/student/management/commands/create_random_users.py index 7c58f1eb71..79de0a585a 100644 --- a/common/djangoapps/student/management/commands/create_random_users.py +++ b/common/djangoapps/student/management/commands/create_random_users.py @@ -1,6 +1,7 @@ """ A script to create some dummy users """ +from __future__ import print_function import uuid from django.core.management.base import BaseCommand @@ -32,6 +33,7 @@ def create(num, course_key): (user, _, _) = _do_create_account(make_random_form()) if course_key is not None: CourseEnrollment.enroll(user, course_key) + print('Created user {}'.format(user.username)) class Command(BaseCommand): @@ -45,16 +47,15 @@ Examples: create_random_users.py 100 HarvardX/CS50x/2012 """ + def add_arguments(self, parser): + parser.add_argument('num_users', + help='Number of users to create', + type=int) + parser.add_argument('course_key', + help='Add newly created users to this course', + nargs='?') + def handle(self, *args, **options): - if len(args) < 1 or len(args) > 2: - print Command.help - return - - num = int(args[0]) - - if len(args) == 2: - course_key = CourseKey.from_string(args[1]) - else: - course_key = None - + num = options['num_users'] + course_key = CourseKey.from_string(options['course_key']) if options['course_key'] else None create(num, course_key) diff --git a/common/djangoapps/student/management/commands/create_user.py b/common/djangoapps/student/management/commands/create_user.py index c98452ae49..51ecb12043 100644 --- a/common/djangoapps/student/management/commands/create_user.py +++ b/common/djangoapps/student/management/commands/create_user.py @@ -1,8 +1,7 @@ -from optparse import make_option +from __future__ import print_function from django.conf import settings from django.contrib.auth.models import User -from django.core.management.base import BaseCommand from django.utils import translation from opaque_keys.edx.keys import CourseKey @@ -23,56 +22,39 @@ class Command(TrackedCommand): manage.py ... create_user -e test@example.com -p insecure -c edX/Open_DemoX/edx_demo_course -m verified """ - option_list = BaseCommand.option_list + ( - make_option('-m', '--mode', - metavar='ENROLLMENT_MODE', - dest='mode', - default='honor', - choices=('audit', 'verified', 'honor'), - help='Enrollment type for user for a specific course'), - make_option('-u', '--username', - metavar='USERNAME', - dest='username', - default=None, - help='Username, defaults to "user" in the email'), - make_option('-n', '--name', - metavar='NAME', - dest='name', - default=None, - help='Name, defaults to "user" in the email'), - make_option('-p', '--password', - metavar='PASSWORD', - dest='password', - default=None, - help='Password for user'), - make_option('-e', '--email', - metavar='EMAIL', - dest='email', - default=None, - help='Email for user'), - make_option('-c', '--course', - metavar='COURSE_ID', - dest='course', - default=None, - help='course to enroll the user in (optional)'), - make_option('-s', '--staff', - dest='staff', - default=False, - action='store_true', - help='give user the staff bit'), - ) + def add_arguments(self, parser): + parser.add_argument('-m', '--mode', + metavar='ENROLLMENT_MODE', + default='honor', + choices=('audit', 'verified', 'honor'), + help='Enrollment type for user for a specific course, defaults to "honor"') + parser.add_argument('-u', '--username', + metavar='USERNAME', + help='Username, defaults to "user" in the email') + parser.add_argument('-n', '--name', + metavar='NAME', + help='Name, defaults to "user" in the email') + parser.add_argument('-p', '--password', + metavar='PASSWORD', + help='Password for user', + required=True) + parser.add_argument('-e', '--email', + metavar='EMAIL', + help='Email for user', + required=True) + parser.add_argument('-c', '--course', + metavar='COURSE_ID', + help='Course to enroll the user in (optional)') + parser.add_argument('-s', '--staff', + action='store_true', + help='Give user the staff bit, defaults to off') def handle(self, *args, **options): - username = options['username'] - name = options['name'] - if not username: - username = options['email'].split('@')[0] - if not name: - name = options['email'].split('@')[0] + username = options['username'] if options['username'] else options['email'].split('@')[0] + name = options['name'] if options['name'] else options['email'].split('@')[0] # parse out the course into a coursekey - if options['course']: - course = CourseKey.from_string(options['course']) + course = CourseKey.from_string(options['course']) if options['course'] else None form = AccountCreationForm( data={ @@ -83,11 +65,13 @@ class Command(TrackedCommand): }, tos_required=False ) + # django.utils.translation.get_language() will be used to set the new # user's preferred language. This line ensures that the result will # match this installation's default locale. Otherwise, inside a # management command, it will always return "en-us". translation.activate(settings.LANGUAGE_CODE) + try: user, _, reg = _do_create_account(form) if options['staff']: @@ -97,8 +81,10 @@ class Command(TrackedCommand): reg.save() create_comments_service_user(user) except AccountValidationError as e: - print e.message + print(e.message) user = User.objects.get(email=options['email']) - if options['course']: + + if course: CourseEnrollment.enroll(user, course, mode=options['mode']) + translation.deactivate() diff --git a/common/djangoapps/student/management/commands/populate_created_on_site_user_attribute.py b/common/djangoapps/student/management/commands/populate_created_on_site_user_attribute.py index 34a495e16d..78819726ae 100644 --- a/common/djangoapps/student/management/commands/populate_created_on_site_user_attribute.py +++ b/common/djangoapps/student/management/commands/populate_created_on_site_user_attribute.py @@ -14,7 +14,7 @@ class Command(BaseCommand): This command back-populates domain of the site the user account was created on. """ help = """./manage.py lms populate_created_on_site_user_attribute --users ,... - '--activation-keys ,... --site-domain --settings=devstack""" + '--activation-keys ,... --site-domain --settings=devstack_docker""" def add_arguments(self, parser): """ @@ -35,6 +35,7 @@ class Command(BaseCommand): parser.add_argument( '--site-domain', help='Enter an existing site domain.', + required=True ) def handle(self, *args, **options): @@ -42,8 +43,6 @@ class Command(BaseCommand): user_ids = options['users'].split(',') if options['users'] else [] activation_keys = options['activation_keys'].split(',') if options['activation_keys'] else [] - if not site_domain: - raise CommandError('You must provide site-domain argument.') if not user_ids and not activation_keys: raise CommandError('You must provide user ids or activation keys.') diff --git a/common/djangoapps/student/management/commands/set_staff.py b/common/djangoapps/student/management/commands/set_staff.py index 1556923253..cf8134c36a 100644 --- a/common/djangoapps/student/management/commands/set_staff.py +++ b/common/djangoapps/student/management/commands/set_staff.py @@ -1,47 +1,45 @@ -from optparse import make_option +from __future__ import print_function +import re from django.contrib.auth.models import User -from django.core.management.base import BaseCommand, CommandError -import re +from django.core.management.base import BaseCommand class Command(BaseCommand): - option_list = BaseCommand.option_list + ( - make_option('--unset', - action='store_true', - dest='unset', - default=False, - help='Set is_staff to False instead of True'), - ) - args = ' [user|email ...]>' help = """ This command will set is_staff to true for one or more users. Lookup by username or email address, assumes usernames do not look like email addresses. """ + def add_arguments(self, parser): + parser.add_argument('users', + nargs='+', + help='Users to set or unset (with the --unset flag) as superusers') + parser.add_argument('--unset', + action='store_true', + dest='unset', + default=False, + help='Set is_staff to False instead of True') + def handle(self, *args, **options): - if len(args) < 1: - raise CommandError('Usage is set_staff {0}'.format(self.args)) - - for user in args: - if re.match(r'[^@]+@[^@]+\.[^@]+', user): - try: + for user in options['users']: + try: + if re.match(r'[^@]+@[^@]+\.[^@]+', user): v = User.objects.get(email=user) - except: - raise CommandError("User {0} does not exist".format(user)) - else: - try: + else: v = User.objects.get(username=user) - except: - raise CommandError("User {0} does not exist".format(user)) - if options['unset']: - v.is_staff = False - else: - v.is_staff = True + if options['unset']: + v.is_staff = False + else: + v.is_staff = True - v.save() + v.save() + print('Modified {} sucessfully.'.format(user)) - print 'Success!' + except Exception as err: # pylint: disable=broad-except + print("Error modifying user with identifier {}: {}: {}".format(user, type(err).__name__, err.message)) + + print('Complete!') diff --git a/common/djangoapps/student/management/commands/set_superuser.py b/common/djangoapps/student/management/commands/set_superuser.py index 068742bc8c..ee65cbd09f 100644 --- a/common/djangoapps/student/management/commands/set_superuser.py +++ b/common/djangoapps/student/management/commands/set_superuser.py @@ -1,32 +1,31 @@ """Management command to grant or revoke superuser access for one or more users""" +from __future__ import print_function -from optparse import make_option from django.contrib.auth.models import User -from django.core.management.base import BaseCommand, CommandError +from django.core.management.base import BaseCommand class Command(BaseCommand): """Management command to grant or revoke superuser access for one or more users""" - option_list = BaseCommand.option_list + ( - make_option('--unset', - action='store_true', - dest='unset', - default=False, - help='Set is_superuser to False instead of True'), - ) - args = ' [user|email ...]>' help = """ This command will set is_superuser to true for one or more users. Lookup by username or email address, assumes usernames do not look like email addresses. """ - def handle(self, *args, **options): - if len(args) < 1: - raise CommandError('Usage is set_superuser {0}'.format(self.args)) + def add_arguments(self, parser): + parser.add_argument('users', + nargs='+', + help='Users to set or unset (with the --unset flag) as superusers') + parser.add_argument('--unset', + action='store_true', + dest='unset', + default=False, + help='Set is_superuser to False instead of True') - for user in args: + def handle(self, *args, **options): + for user in options['users']: try: if '@' in user: userobj = User.objects.get(email=user) @@ -39,8 +38,9 @@ class Command(BaseCommand): userobj.is_superuser = True userobj.save() + print('Modified {} sucessfully.'.format(user)) except Exception as err: # pylint: disable=broad-except - print "Error modifying user with identifier {}: {}: {}".format(user, type(err).__name__, err.message) + print("Error modifying user with identifier {}: {}: {}".format(user, type(err).__name__, err.message)) - print 'Success!' + print('Complete!') diff --git a/common/djangoapps/student/management/tests/test_bulk_change_enrollment.py b/common/djangoapps/student/management/tests/test_bulk_change_enrollment.py index 00c903631d..7fb1076908 100644 --- a/common/djangoapps/student/management/tests/test_bulk_change_enrollment.py +++ b/common/djangoapps/student/management/tests/test_bulk_change_enrollment.py @@ -1,5 +1,7 @@ """Tests for the bulk_change_enrollment command.""" import ddt +from six import text_type + from django.core.management import call_command from django.core.management.base import CommandError from mock import patch, call @@ -35,12 +37,15 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): # Verify that no users are in the `from` mode yet. self.assertEqual(len(CourseEnrollment.objects.filter(mode=to_mode, course_id=self.course.id)), 0) + args = '--course {course} --from_mode {from_mode} --to_mode {to_mode} --commit'.format( + course=text_type(self.course.id), + from_mode=from_mode, + to_mode=to_mode + ) + call_command( 'bulk_change_enrollment', - course=unicode(self.course.id), - from_mode=from_mode, - to_mode=to_mode, - commit=True, + *args.split(' ') ) # Verify that all users have been moved -- if not, this will @@ -67,12 +72,15 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): self.assertEqual(len(CourseEnrollment.objects.filter(mode=to_mode, course_id=self.course.id)), 0) self.assertEqual(len(CourseEnrollment.objects.filter(mode=to_mode, course_id=course_2.id)), 0) - call_command( - 'bulk_change_enrollment', + args = '--org {org} --from_mode {from_mode} --to_mode {to_mode} --commit'.format( org=self.org, from_mode=from_mode, - to_mode=to_mode, - commit=True, + to_mode=to_mode + ) + + call_command( + 'bulk_change_enrollment', + *args.split(' ') ) # Verify that all users have been moved -- if not, this will @@ -91,7 +99,7 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): call_command( 'bulk_change_enrollment', org=self.org, - course=unicode(self.course.id), + course=text_type(self.course.id), from_mode='audit', to_mode='no-id-professional', commit=True, @@ -114,12 +122,15 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): self.assertEqual(len(CourseEnrollment.objects.filter(mode=to_mode, course_id=self.course.id)), 0) self.assertEqual(len(CourseEnrollment.objects.filter(mode=to_mode, course_id=course_2.id)), 0) - call_command( - 'bulk_change_enrollment', + args = '--org {org} --from_mode {from_mode} --to_mode {to_mode} --commit'.format( org=self.org, from_mode=from_mode, - to_mode=to_mode, - commit=True, + to_mode=to_mode + ) + + call_command( + 'bulk_change_enrollment', + *args.split(' ') ) # Verify that users were not moved for the invalid course/mode combination @@ -139,12 +150,15 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): CourseModeFactory(course_id=self.course.id, mode_slug='no-id-professional') with self.assertRaises(CommandError): - call_command( - 'bulk_change_enrollment', + args = '--org {org} --from_mode {from_mode} --to_mode {to_mode} --commit'.format( org='fakeX', from_mode='audit', to_mode='no-id-professional', - commit=True, + ) + + call_command( + 'bulk_change_enrollment', + *args.split(' ') ) def test_without_commit(self): @@ -152,11 +166,15 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): self._enroll_users(self.course, self.users, 'audit') CourseModeFactory(course_id=self.course.id, mode_slug='honor') + args = '--course {course} --from_mode {from_mode} --to_mode {to_mode}'.format( + course=text_type(self.course.id), + from_mode='audit', + to_mode='honor' + ) + call_command( 'bulk_change_enrollment', - course=unicode(self.course.id), - from_mode='audit', - to_mode='honor', + *args.split(' ') ) # Verify that no users are in the honor mode. @@ -170,7 +188,7 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): with self.assertRaises(CommandError): call_command( 'bulk_change_enrollment', - course=unicode(self.course.id), + course=text_type(self.course.id), from_mode='audit', ) @@ -180,7 +198,7 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): command_options = { 'from_mode': 'audit', 'to_mode': 'honor', - 'course': unicode(self.course.id), + 'course': text_type(self.course.id), } command_options.pop(option) @@ -209,7 +227,7 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): [ call( EVENT_NAME_ENROLLMENT_MODE_CHANGED, - {'course_id': unicode(course.id), 'user_id': user.id, 'mode': to_mode} + {'course_id': text_type(course.id), 'user_id': user.id, 'mode': to_mode} ), ] ) diff --git a/common/djangoapps/student/management/tests/test_change_enrollment.py b/common/djangoapps/student/management/tests/test_change_enrollment.py index 5cfb8d9592..503f8a3d65 100644 --- a/common/djangoapps/student/management/tests/test_change_enrollment.py +++ b/common/djangoapps/student/management/tests/test_change_enrollment.py @@ -2,6 +2,7 @@ import ddt from mock import patch +from six import text_type from django.core.management import call_command from xmodule.modulestore.tests.factories import CourseFactory @@ -53,13 +54,6 @@ class ChangeEnrollmentTests(SharedModuleStoreTestCase): """ The command should update the user's enrollment. """ user_str = ','.join([getattr(user, method) for user in self.users]) user_ids = [u.id for u in self.users] - command_args = { - 'course_id': unicode(self.course.id), - 'to_mode': 'honor', - 'from_mode': 'audit', - 'noop': noop, - method: user_str, - } # Verify users are not in honor mode yet self.assertEqual( @@ -67,11 +61,19 @@ class ChangeEnrollmentTests(SharedModuleStoreTestCase): 0 ) - call_command( - 'change_enrollment', - **command_args + noop = " --noop" if noop else "" + + # Hack around call_command bugs dealing with required options see: + # https://stackoverflow.com/questions/32036562/call-command-argument-is-required + command_args = '--course {course} --to honor --from audit --{method} {user_str}{noop}'.format( + course=text_type(self.course.id), + noop=noop, + method=method, + user_str=user_str ) + call_command('change_enrollment', *command_args.split(' ')) + # Verify correct number of users are now in honor mode self.assertEqual( len(CourseEnrollment.objects.filter(mode='honor', user_id__in=user_ids)), @@ -95,12 +97,6 @@ class ChangeEnrollmentTests(SharedModuleStoreTestCase): all_users.append(fake_user) user_str = ','.join(all_users) real_user_ids = [u.id for u in self.users] - command_args = { - 'course_id': unicode(self.course.id), - 'to_mode': 'honor', - 'from_mode': 'audit', - method: user_str, - } # Verify users are not in honor mode yet self.assertEqual( @@ -108,11 +104,14 @@ class ChangeEnrollmentTests(SharedModuleStoreTestCase): 0 ) - call_command( - 'change_enrollment', - **command_args + command_args = '--course {course} --to honor --from audit --{method} {user_str}'.format( + course=text_type(self.course.id), + method=method, + user_str=user_str ) + call_command('change_enrollment', *command_args.split(' ')) + # Verify correct number of users are now in honor mode self.assertEqual( len(CourseEnrollment.objects.filter(mode='honor', user_id__in=real_user_ids)),