diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index 123c9c4173..e0203e3b00 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -1,10 +1,7 @@ """ Script for importing courseware from XML format """ -from optparse import make_option - from django.core.management.base import BaseCommand - from django_comment_common.utils import are_permissions_roles_seeded, seed_permissions_roles from xmodule.contentstore.django import contentstore from xmodule.modulestore import ModuleStoreEnum @@ -16,25 +13,24 @@ class Command(BaseCommand): """ Import the specified data directory into the default ModuleStore """ - help = 'Import the specified data directory into the default ModuleStore' + help = 'Import the specified data directory into the default ModuleStore.' - option_list = BaseCommand.option_list + ( - make_option('--nostatic', - action='store_true', - help='Skip import of static content'), - ) + def add_arguments(self, parser): + parser.add_argument('data_directory') + parser.add_argument('course_dirs', + nargs='*', + metavar='course_dir') + parser.add_argument('--nostatic', + action='store_true', + help='skip import of static content') def handle(self, *args, **options): - "Execute the command" - if len(args) == 0: - raise CommandError("import requires at least one argument: [--nostatic] [...]") - - data_dir = args[0] - do_import_static = not options.get('nostatic', False) - if len(args) > 1: - source_dirs = args[1:] - else: + data_dir = options['data_directory'] + do_import_static = not options['nostatic'] + source_dirs = options['course_dirs'] + if len(source_dirs) == 0: source_dirs = None + self.stdout.write("Importing. Data_dir={data}, source_dirs={courses}\n".format( data=data_dir, courses=source_dirs, diff --git a/lms/djangoapps/courseware/management/commands/clean_xml.py b/lms/djangoapps/courseware/management/commands/clean_xml.py index 53e9838475..b0180dbca1 100644 --- a/lms/djangoapps/courseware/management/commands/clean_xml.py +++ b/lms/djangoapps/courseware/management/commands/clean_xml.py @@ -1,17 +1,21 @@ +from __future__ import print_function + import os import sys import traceback import lxml.etree + from django.core.management.base import BaseCommand from fs.osfs import OSFS from path import Path as path - from xmodule.modulestore.xml import XMLModuleStore def traverse_tree(course): - """Load every descriptor in course. Return bool success value.""" + """ + Load every descriptor in course. Return bool success value. + """ queue = [course] while len(queue) > 0: node = queue.pop() @@ -21,13 +25,13 @@ def traverse_tree(course): def export(course, export_dir): - """Export the specified course to course_dir. Creates dir if it doesn't exist. - Overwrites files, does not clean out dir beforehand. + """ + Export the specified course to course_dir. Creates dir if it doesn't + exist. Overwrites files, does not clean out dir beforehand. """ fs = OSFS(export_dir, create=True) if not fs.isdirempty('.'): - print ('WARNING: Directory {dir} not-empty.' - ' May clobber/confuse things'.format(dir=export_dir)) + print('WARNING: Directory {dir} not-empty. May clobber/confuse things'.format(dir=export_dir)) try: course.runtime.export_fs = fs @@ -38,7 +42,7 @@ def export(course, export_dir): return True except: - print 'Export failed!' + print('Export failed!') traceback.print_exc() return False @@ -47,7 +51,7 @@ def export(course, export_dir): def import_with_checks(course_dir): all_ok = True - print "Attempting to load '{0}'".format(course_dir) + print('Attempting to load "{}"'.format(course_dir)) course_dir = path(course_dir) data_dir = course_dir.dirname() @@ -69,88 +73,85 @@ def import_with_checks(course_dir): n = len(courses) if n != 1: - print 'ERROR: Expect exactly 1 course. Loaded {n}: {lst}'.format( - n=n, lst=courses) + print('ERROR: Expect exactly 1 course. Loaded {n}: {lst}'.format(n=n, lst=courses)) return (False, None) course = courses[0] errors = modulestore.get_course_errors(course.id) if len(errors) != 0: all_ok = False - print '\n' - print "=" * 40 - print 'ERRORs during import:' - print '\n'.join(map(str_of_err, errors)) - print "=" * 40 - print '\n' + print( + '\n' + + '========================================' + + 'ERRORs during import:' + + '\n'.join(map(str_of_err, errors)) + + '========================================' + + '\n' + ) # print course validators = ( traverse_tree, ) - print "=" * 40 - print "Running validators..." + print('========================================') + print('Running validators...') for validate in validators: - print 'Running {0}'.format(validate.__name__) + print('Running {}'.format(validate.__name__)) all_ok = validate(course) and all_ok if all_ok: - print 'Course passes all checks!' + print('Course passes all checks!') else: - print "Course fails some checks. See above for errors." + print('Course fails some checks. See above for errors.') return all_ok, course def check_roundtrip(course_dir): - """Check that import->export leaves the course the same""" + """ + Check that import->export leaves the course the same + """ - print "====== Roundtrip import =======" + print('====== Roundtrip import =======') (ok, course) = import_with_checks(course_dir) if not ok: - raise Exception("Roundtrip import failed!") + raise Exception('Roundtrip import failed!') - print "====== Roundtrip export =======" - export_dir = course_dir + ".rt" + print('====== Roundtrip export =======') + export_dir = course_dir + '.rt' export(course, export_dir) # dircmp doesn't do recursive diffs. # diff = dircmp(course_dir, export_dir, ignore=[], hide=[]) - print "======== Roundtrip diff: =========" + print('======== Roundtrip diff: =========') sys.stdout.flush() # needed to make diff appear in the right place - os.system("diff -r {0} {1}".format(course_dir, export_dir)) - print "======== ideally there is no diff above this =======" - - -def clean_xml(course_dir, export_dir, force): - (ok, course) = import_with_checks(course_dir) - if ok or force: - if not ok: - print "WARNING: Exporting despite errors" - export(course, export_dir) - check_roundtrip(export_dir) - else: - print "Did NOT export" + os.system('diff -r {} {}'.format(course_dir, export_dir)) + print('======== ideally there is no diff above this =======') class Command(BaseCommand): - help = """Imports specified course.xml, validate it, then exports in - a canonical format. + help = 'Imports specified course, validates it, then exports it in a canonical format.' -Usage: clean_xml PATH-TO-COURSE-DIR PATH-TO-OUTPUT-DIR [force] - -If 'force' is specified as the last argument, exports even if there -were import errors. -""" + def add_arguments(self, parser): + parser.add_argument('course_dir', + help='path to the input course directory') + parser.add_argument('output_dir', + help='path to the output course directory') + parser.add_argument('--force', + action='store_true', + help='export course even if there were import errors') def handle(self, *args, **options): - n = len(args) - if n < 2 or n > 3: - print Command.help - return + course_dir = options['course_dir'] + output_dir = options['output_dir'] + force = options['force'] - force = False - if n == 3 and args[2] == 'force': - force = True - clean_xml(args[0], args[1], force) + (ok, course) = import_with_checks(course_dir) + if ok or force: + if not ok: + print('WARNING: Exporting despite errors') + export(course, output_dir) + check_roundtrip(output_dir) + else: + print('Did NOT export') diff --git a/lms/djangoapps/courseware/management/commands/dump_course_ids.py b/lms/djangoapps/courseware/management/commands/dump_course_ids.py index ff66e4c12c..25dee61594 100644 --- a/lms/djangoapps/courseware/management/commands/dump_course_ids.py +++ b/lms/djangoapps/courseware/management/commands/dump_course_ids.py @@ -1,29 +1,28 @@ -# pylint: disable=missing-docstring +""" +Dump the course_ids available to the lms. + +Output is UTF-8 encoded by default. +""" +from __future__ import unicode_literals from optparse import make_option from textwrap import dedent -from django.core.management.base import BaseCommand +from six import text_type +from django.core.management.base import BaseCommand from openedx.core.djangoapps.content.course_overviews.models import CourseOverview class Command(BaseCommand): - """ - Simple command to dump the course_ids available to the lms. - - Output is UTF-8 encoded by default. - - """ help = dedent(__doc__).strip() - option_list = BaseCommand.option_list + ( - make_option('--modulestore', - action='store', - default='default', - help='Name of the modulestore to use'), - ) + + def add_arguments(self, parser): + parser.add_argument('--modulestore', + default='default', + help='name of the modulestore to use') def handle(self, *args, **options): - output = u'\n'.join(unicode(course_overview.id) for course_overview in CourseOverview.get_all_courses()) + '\n' + output = '\n'.join(text_type(course_overview.id) for course_overview in CourseOverview.get_all_courses()) + '\n' return output diff --git a/lms/djangoapps/courseware/management/commands/dump_course_structure.py b/lms/djangoapps/courseware/management/commands/dump_course_structure.py index f1b58a85da..d272f8d6a2 100644 --- a/lms/djangoapps/courseware/management/commands/dump_course_structure.py +++ b/lms/djangoapps/courseware/management/commands/dump_course_structure.py @@ -1,5 +1,5 @@ """ -A Django command that dumps the structure of a course as a JSON object. +Dump the structure of a course as a JSON object. The resulting JSON object has one entry for each module in the course: @@ -24,7 +24,6 @@ from django.core.management.base import BaseCommand, CommandError from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from xblock.fields import Scope - from xblock_discussion import DiscussionXBlock from xmodule.modulestore.django import modulestore from xmodule.modulestore.inheritance import compute_inherited_metadata, own_metadata @@ -34,30 +33,22 @@ INHERITED_FILTER_LIST = ['children', 'xml_attributes'] class Command(BaseCommand): - """ - Write out to stdout a structural and metadata information for a - course as a JSON object - """ - args = "" help = dedent(__doc__).strip() - option_list = BaseCommand.option_list + ( - make_option('--modulestore', - action='store', - default='default', - help='Name of the modulestore'), - make_option('--inherited', - action='store_true', - default=False, - help='Whether to include inherited metadata'), - make_option('--inherited_defaults', - action='store_true', - default=False, - help='Whether to include default values of inherited metadata'), - ) + + def add_arguments(self, parser): + parser.add_argument('course_id', + help='specifies the course to dump') + parser.add_argument('--modulestore', + default='default', + help='name of the modulestore') + parser.add_argument('--inherited', + action='store_true', + help='include inherited metadata'), + parser.add_argument('--inherited_defaults', + action='store_true', + help='include default values of inherited metadata'), def handle(self, *args, **options): - if len(args) != 1: - raise CommandError("course_id not specified") # Get the modulestore @@ -66,7 +57,7 @@ class Command(BaseCommand): # Get the course data try: - course_key = CourseKey.from_string(args[0]) + course_key = CourseKey.from_string(options['course_id']) except InvalidKeyError: raise CommandError("Invalid course_id") diff --git a/lms/djangoapps/courseware/management/commands/export_course.py b/lms/djangoapps/courseware/management/commands/export_course.py deleted file mode 100644 index 0f6d748fab..0000000000 --- a/lms/djangoapps/courseware/management/commands/export_course.py +++ /dev/null @@ -1,110 +0,0 @@ -""" -A Django command that exports a course to a tar.gz file. - -If is '-', it pipes the file to stdout - -NOTE: This used to be used by Analytics research exports to provide -researchers with course content. It is now DEPRECATED, and -functionality has moved to export_olx.py in -cms/djangoapps/contentstore/management/commands. - -Note: when removing this file, also remove references to it -from test_dump_course. - -""" - -import os -import re -import shutil -import tarfile -from tempfile import mkdtemp, mktemp -from textwrap import dedent - -from django.core.management.base import BaseCommand, CommandError -from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey -from path import Path as path - -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.xml_exporter import export_course_to_xml - - -class Command(BaseCommand): - """ - Export a course to XML. The output is compressed as a tar.gz file - - """ - args = " " - help = dedent(__doc__).strip() - - def handle(self, *args, **options): - course_key, filename, pipe_results = self._parse_arguments(args) - - export_course_to_tarfile(course_key, filename) - - results = self._get_results(filename) if pipe_results else None - - self.stdout.write(results, ending="") - - def _parse_arguments(self, args): - """Parse command line arguments""" - try: - course_key = CourseKey.from_string(args[0]) - filename = args[1] - except InvalidKeyError: - raise CommandError("Unparsable course_id") - except IndexError: - raise CommandError("Insufficient arguments") - - # If filename is '-' save to a temp file - pipe_results = False - if filename == '-': - filename = mktemp() - pipe_results = True - - return course_key, filename, pipe_results - - def _get_results(self, filename): - """Load results from file""" - with open(filename) as f: - results = f.read() - os.remove(filename) - return results - - -def export_course_to_tarfile(course_key, filename): - """Exports a course into a tar.gz file""" - tmp_dir = mkdtemp() - try: - course_dir = export_course_to_directory(course_key, tmp_dir) - compress_directory(course_dir, filename) - finally: - shutil.rmtree(tmp_dir, ignore_errors=True) - - -def export_course_to_directory(course_key, root_dir): - """Export course into a directory""" - store = modulestore() - course = store.get_course(course_key) - if course is None: - raise CommandError("Invalid course_id") - - # The safest characters are A-Z, a-z, 0-9, , and . - # We represent the first four with \w. - # TODO: Once we support courses with unicode characters, we will need to revisit this. - replacement_char = u'-' - course_dir = replacement_char.join([course.id.org, course.id.course, course.id.run]) - course_dir = re.sub(r'[^\w\.\-]', replacement_char, course_dir) - - export_course_to_xml(store, None, course.id, root_dir, course_dir) - - export_dir = path(root_dir) / course_dir - return export_dir - - -def compress_directory(directory, filename): - """Compress a directory into a tar.gz file""" - mode = 'w:gz' - name = path(directory).name - with tarfile.open(filename, mode) as tar_file: - tar_file.add(directory, arcname=name) diff --git a/lms/djangoapps/courseware/management/commands/fix_student_module_newlines.py b/lms/djangoapps/courseware/management/commands/fix_student_module_newlines.py deleted file mode 100644 index d195afec2e..0000000000 --- a/lms/djangoapps/courseware/management/commands/fix_student_module_newlines.py +++ /dev/null @@ -1,364 +0,0 @@ -# pragma: no cover -""" -Fix StudentModule entries that have Course IDs with trailing newlines. - -Due to a bug, many rows in courseware_studentmodule were written with a -course_id that had a trailing newline. This command tries to fix that, and to -merge that data with data that might have been written to the correct course_id. -""" -import json -import logging -from collections import namedtuple -from optparse import make_option -from textwrap import dedent - -from django.core.management.base import BaseCommand, CommandError -from django.db import DatabaseError, transaction - -from courseware.models import StudentModule -from util.query import use_read_replica_if_available - -log = logging.getLogger("fix_student_module_newlines") - -FixResult = namedtuple('FixResult', 'record_trimmed data_copied record_archived capa_merge error') - - -class Command(BaseCommand): - """Fix StudentModule entries that have Course IDs with trailing newlines.""" - args = " " - help = dedent(__doc__).strip() - option_list = BaseCommand.option_list + ( - make_option('--dry_run', - action='store_true', - default=False, - help="Run read queries and say what we're going to do, but don't actually do it."), - ) - - def handle(self, *args, **options): - """Fix newline courses in CSM!""" - if len(args) != 2: - raise CommandError('Must specify start and end dates: e.g. "2016-08-23 16:43:00" "2016-08-24 22:00:00"') - - start, end = args - dry_run = options['dry_run'] - - log.info( - "Starting fix_student_module_newlines in %s mode!", - "dry_run" if dry_run else "real" - ) - - rows_to_fix = use_read_replica_if_available( - # pylint: disable=no-member - StudentModule.objects.raw( - "select * from courseware_studentmodule where modified between %s and %s and course_id like %s", - (start, end, '%\n') - ) - ) - - results = [self.fix_row(row, dry_run=dry_run) for row in rows_to_fix] - log.info( - "Finished fix_student_module_newlines in %s mode!", - "dry_run" if dry_run else "real" - ) - log.info("Stats: %s rows detected", len(results)) - if results: - # Add up all the columns - aggregated_result = FixResult(*[sum(col) for col in zip(*results)]) - log.info("Results: %s", aggregated_result) - - def fix_row(self, read_only_newline_course_row, dry_run=False): - """ - Fix a StudentModule with a trailing newline in the course_id. - - Returns a count of (row modified, ) - - At the end of this method call, the record should no longer have a - trailing newline for the course_id. There are three possible outcomes: - - 1. There was never a conflicting entry: - -> We just update this row. - 2. Conflict and the other row (with correct course_id) wins: - -> We archive this row. - 2. Conflict and this row wins: - -> We copy the data to the conflicting row (the one that has a - correct course_id), and archive this row. - - Even though all the StudentModule entries coming in here have trailing - newlines in the course_id, the deserialization logic will obscure that - (it gets parsed out when read from the database). We will also - automatically strip the newlines when writing back to the database, so - we have to be very careful about violating unique constraints by doing - unintended updates. That's why we only do an update to - newline_course_row if no correct_course_row exists. - """ - # pylint: disable=too-many-statements - # We got the StudentModule from the read replica, so we have to fetch it - # again from our writeable database before making changes - log.info( - "Fixing row %s, course %s, student %s, block %s", - read_only_newline_course_row.id, - read_only_newline_course_row.course_id, - read_only_newline_course_row.student_id, - read_only_newline_course_row.module_state_key, - ) - try: - newline_course_row = StudentModule.objects.get(id=read_only_newline_course_row.id) - except StudentModule.DoesNotExist: - # We're not going to be able to make any corrective writes, so just fail fast - log.exception("Could not find editable CSM row %s", read_only_newline_course_row.id) - return FixResult(record_trimmed=0, data_copied=0, record_archived=0, capa_merge=0, error=1) - - # Find the StudentModule entry with the correct course_id. - try: - correct_course_row = StudentModule.objects.get( - student_id=newline_course_row.student_id, - course_id=newline_course_row.course_id, - module_state_key=newline_course_row.module_state_key, - ) - assert correct_course_row.pk != newline_course_row.pk - except StudentModule.DoesNotExist: - correct_course_row = None - - # If only an entry with the newline course exists, just change the course_id and save. - if correct_course_row is None: - log.info( - "No conflict: Removing trailing newline from course_id in CSM row %s - (%s) %s", - newline_course_row.id, - newline_course_row.module_type, - newline_course_row.module_state_key, - ) - if dry_run: - return FixResult(record_trimmed=1, data_copied=0, record_archived=0, capa_merge=0, error=0) - - try: - with transaction.atomic(): - newline_course_row.save() - return FixResult(record_trimmed=1, data_copied=0, record_archived=0, capa_merge=0, error=0) - except DatabaseError: - log.exception( - "Could not remove newline and update CSM row %s", newline_course_row.id - ) - return FixResult(record_trimmed=0, data_copied=0, record_archived=0, capa_merge=0, error=1) - - # If we're here, then both versions of the row exist. We're going to - # pick a winner. This is handled differently for capa (where we have to - # merge entries) and everything else. - # - # Handle CAPA record merging! - if newline_course_row.module_type == 'problem': - return self.handle_capa_merge(correct_course_row, newline_course_row, dry_run) - - row_to_keep = self.row_to_keep(correct_course_row, newline_course_row) - - # To minimize the chances of conflicts and preserve history, if we - # decided to keep the data from the newline_course_row, we copy that - # data into correct_course_row, and then archive the newline_course_row - # as a cleanup step. - if row_to_keep.id == newline_course_row.id: - log.info( - "Conflict: Choosing data from newline course, copying data from CSM %s to %s - (%s) %s", - newline_course_row.id, - correct_course_row.id, - newline_course_row.module_type, - newline_course_row.module_state_key - ) - correct_course_row.state = newline_course_row.state - correct_course_row.grade = newline_course_row.grade - correct_course_row.max_grade = newline_course_row.max_grade - - if dry_run: - return FixResult(record_trimmed=0, data_copied=1, record_archived=1, capa_merge=0, error=0) - - try: - with transaction.atomic(): - correct_course_row.save() - self.cleanup_row(newline_course_row) - return FixResult(record_trimmed=0, data_copied=1, record_archived=1, capa_merge=0, error=0) - except DatabaseError: - log.exception( - "Failed while trying save CSM row %s and archiving row %s", - correct_course_row.id, - newline_course_row.id - ) - return FixResult(record_trimmed=0, data_copied=0, record_archived=0, capa_merge=0, error=1) - - # If we've reached this point, we just want to keep correct_course_row - # and delete the newline_course_row entry. - log.info( - "Conflict: Choosing data from record with correct course_id (%s) " - "and archiving row with newline in course_id (%s) - (%s) %s", - correct_course_row.id, - newline_course_row.id, - newline_course_row.module_type, - newline_course_row.module_state_key - ) - - if dry_run: - return FixResult(record_trimmed=0, data_copied=0, record_archived=1, capa_merge=0, error=0) - - try: - with transaction.atomic(): - self.cleanup_row(newline_course_row) - return FixResult(record_trimmed=0, data_copied=0, record_archived=1, capa_merge=0, error=0) - except DatabaseError: - log.exception("Could not archive CSM row %s", newline_course_row.id) - return FixResult(record_trimmed=0, data_copied=0, record_archived=0, capa_merge=0, error=1) - - def cleanup_row(self, model): - """Rename the row for archiving purposes""" - new_run = model.course_id.run.strip() + "_FIX_FOR_ECOM-5345" - model.course_id = model.course_id.replace(run=new_run) - model.save() - - def handle_capa_merge(self, correct_course_row, newline_course_row, dry_run): - """ - Merge capa state and grades, if possible. - """ - log.info( - "-> Merging capa grade and state information for CSM rows %s (newline) and %s (correct course_id)", - newline_course_row.id, - correct_course_row.id - ) - state, grade, max_grade = self.capa_state_and_grade( - correct_course_row, newline_course_row - ) - # The derived grade should match at least one of the two records - nl_grade_tuple = (newline_course_row.grade, newline_course_row.max_grade) - cc_grade_tuple = (correct_course_row.grade, correct_course_row.max_grade) - safe_to_change_grade = (grade, max_grade) in [nl_grade_tuple, cc_grade_tuple] - if not safe_to_change_grade: - log.error( - "-> Derived grade %s does not match grades from either %s %s or " - "%s %s -- Archiving newline entry %s and leaving %s alone", - (grade, max_grade), - newline_course_row.id, - nl_grade_tuple, - correct_course_row.id, - cc_grade_tuple, - newline_course_row.id, - correct_course_row.id, - ) - - safe_to_change_result = FixResult(record_trimmed=0, data_copied=0, record_archived=1, capa_merge=1, error=0) - not_safe_to_change_result = FixResult(record_trimmed=0, data_copied=0, record_archived=1, capa_merge=0, error=1) - - if dry_run: - return safe_to_change_result if safe_to_change_grade else not_safe_to_change_result - - try: - with transaction.atomic(): - if safe_to_change_grade: - correct_course_row.state = state - correct_course_row.grade = grade - correct_course_row.max_grade = max_grade - correct_course_row.save() - self.cleanup_row(newline_course_row) - return safe_to_change_result if safe_to_change_grade else not_safe_to_change_result - except DatabaseError: - log.exception("Failed while trying to merge capa CSM row %s (newline) and %s (correct course_id)") - return FixResult(record_trimmed=0, data_copied=0, record_archived=0, capa_merge=0, error=1) - - def capa_state_and_grade(self, correct_course_row, newline_course_row): - """ - Return a tuple that is (state, grade, max_grade) to preserve. - - THIS SHOULD ONLY BE CALLED FOR CAPA PROBLEMS. - - Updates to scores could have gotten crossed with updates to state, so - that the states and scores do not match up to each other (e.g. you might - have correct_course_row with the proper state, but the score - corresponding to that state might live in newline_course_row). So we're - going to re-derive the scores from the state for capa problems. - """ - ccr_grade, ccr_max_grade = self.grade_for_state(correct_course_row.state) - if (ccr_grade, ccr_max_grade) != (correct_course_row.grade, correct_course_row.max_grade): - log.info( - "-> Correct course row %s has grade %s but should have grade %s", - correct_course_row.id, - (correct_course_row.grade, correct_course_row.max_grade), - (ccr_grade, ccr_max_grade) - ) - - ncr_grade, ncr_max_grade = self.grade_for_state(newline_course_row.state) - if (ncr_grade, ncr_max_grade) != (newline_course_row.grade, newline_course_row.max_grade): - log.info( - "-> Newline course row %s has grade %s but should have grade %s", - newline_course_row.id, - (newline_course_row.grade, newline_course_row.max_grade), - (ncr_grade, ncr_max_grade) - ) - - if ncr_grade > ccr_grade: - log.info( - "-> Newline course row %s has higher derived grade for state.", - newline_course_row.id - ) - return newline_course_row.state, ncr_grade, ncr_max_grade - - log.info( - "-> Correct course row %s has higher or equal derived grade for state.", - correct_course_row.id - ) - return correct_course_row.state, ccr_grade, ccr_max_grade - - def grade_for_state(self, state): - """Given unparsed state, return the (grade, max_grade) we should have.""" - parsed_state = json.loads(state) - correct_map = parsed_state.get("correct_map") - if not correct_map: - input_state = parsed_state.get('input_state') - if input_state is not None: - return 0.0, float(len(input_state)) - return None, None - - def item_score(item): - """Return score for an individual correct_map entry.""" - # Partial credit overrides all, even if correctness is 'incorrect'. - # We're inconsistent on whether we say the correctness is - # 'incorrect' or 'partially-correct' in this situation. - partial_points = item.get('npoints') - if partial_points is not None: - return partial_points - - # Otherwise, we're either right or wrong. - correctness = item.get('correctness') - if correctness in ['correct', 'partially-correct']: - return 1.0 - - return 0.0 - - grade = sum(item_score(item) for item in correct_map.values()) - max_grade = float(len(correct_map)) - - return grade, max_grade - - def row_to_keep(self, correct_course_row, newline_course_row): - """Determine which row's data we want to keep.""" - # Rule 1: Take the higher grade. - if newline_course_row.grade > correct_course_row.grade: - log.info( - "-> Newline course record grade %s is higher than correct course record grade %s", - newline_course_row.grade, - correct_course_row.grade - ) - return newline_course_row - elif correct_course_row.grade > newline_course_row.grade: - log.info( - "-> Correct course record grade %s is higher than newline course record grade %s", - correct_course_row.grade, - newline_course_row.grade - ) - return correct_course_row - - # Rule 2: Take the newline course record if they've interacted with it - # more recently. - if newline_course_row.modified > correct_course_row.modified: - log.info( - "-> Newline course record modified %s is later than correct course record modified %s", - newline_course_row.modified, - correct_course_row.modified - ) - return newline_course_row - - # In all other cases, take the correct_course_row - return correct_course_row diff --git a/lms/djangoapps/courseware/management/commands/regrade_partial.py b/lms/djangoapps/courseware/management/commands/regrade_partial.py deleted file mode 100644 index 656fb71e2d..0000000000 --- a/lms/djangoapps/courseware/management/commands/regrade_partial.py +++ /dev/null @@ -1,166 +0,0 @@ -''' -This is a one-off command aimed at fixing a temporary problem encountered where partial credit was awarded for -code problems, but the resulting score (or grade) was mistakenly set to zero because of a bug in -CorrectMap.get_npoints(). -''' - -import json -import logging -from optparse import make_option - -from django.core.management.base import BaseCommand - -from capa.correctmap import CorrectMap -from courseware.models import StudentModule - -LOG = logging.getLogger(__name__) - - -class Command(BaseCommand): - ''' - The fix here is to recalculate the score/grade based on the partial credit. - To narrow down the set of problems that might need fixing, the StudentModule - objects to be checked is filtered down to those: - - created < '2013-03-08 15:45:00' (the problem must have been answered before the fix was installed, - on Prod and Edge) - modified > '2013-03-07 20:18:00' (the problem must have been visited after the bug was introduced) - state like '%"npoints": 0.%' (the problem must have some form of partial credit). - ''' - - num_visited = 0 - num_changed = 0 - - option_list = BaseCommand.option_list + ( - make_option('--save', - action='store_true', - dest='save_changes', - default=False, - help='Persist the changes that were encountered. If not set, no changes are saved.'), ) - - def fix_studentmodules(self, save_changes): - '''Identify the list of StudentModule objects that might need fixing, and then fix each one''' - modules = StudentModule.objects.filter(modified__gt='2013-03-07 20:18:00', - created__lt='2013-03-08 15:45:00', - state__contains='"npoints": 0.') - - for module in modules: - self.fix_studentmodule_grade(module, save_changes) - - def fix_studentmodule_grade(self, module, save_changes): - ''' Fix the grade assigned to a StudentModule''' - module_state = module.state - if module_state is None: - # not likely, since we filter on it. But in general... - LOG.info( - u"No state found for %s module %s for student %s in course %s", - module.module_type, - module.module_state_key, - module.student.username, - module.course_id, - ) - return - - state_dict = json.loads(module_state) - self.num_visited += 1 - - # LoncapaProblem.get_score() checks student_answers -- if there are none, we will return a grade of 0 - # Check that this is the case, but do so sooner, before we do any of the other grading work. - student_answers = state_dict['student_answers'] - if (not student_answers) or len(student_answers) == 0: - # we should not have a grade here: - if module.grade != 0: - log_msg = ( - u"No answer found but grade %(grade)s exists for %(type)s module %(id)s for student %(student)s " + - u"in course %(course_id)s" - ) - - LOG.error(log_msg, { - "grade": module.grade, - "type": module.module_type, - "id": module.module_state_key, - "student": module.student.username, - "course_id": module.course_id, - }) - else: - log_msg = ( - u"No answer and no grade found for %(type)s module %(id)s for student %(student)s " + - u"in course %(course_id)s" - ) - - LOG.debug(log_msg, { - "grade": module.grade, - "type": module.module_type, - "id": module.module_state_key, - "student": module.student.username, - "course_id": module.course_id, - }) - return - - # load into a CorrectMap, as done in LoncapaProblem.__init__(): - correct_map = CorrectMap() - if 'correct_map' in state_dict: - correct_map.set_dict(state_dict['correct_map']) - - # calculate score the way LoncapaProblem.get_score() works, by deferring to - # CorrectMap's get_npoints implementation. - correct = 0 - for key in correct_map: - correct += correct_map.get_npoints(key) - - if module.grade == correct: - # nothing to change - log_msg = u"Grade matches for %(type)s module %(id)s for student %(student)s in course %(course_id)s" - LOG.debug(log_msg, { - "type": module.module_type, - "id": module.module_state_key, - "student": module.student.username, - "course_id": module.course_id, - }) - elif save_changes: - # make the change - log_msg = ( - u"Grade changing from %(grade)s to %(correct)s for %(type)s module " + - u"%(id)s for student %(student)s in course %(course_id)s" - ) - - LOG.debug(log_msg, { - "grade": module.grade, - "correct": correct, - "type": module.module_type, - "id": module.module_state_key, - "student": module.student.username, - "course_id": module.course_id, - }) - - module.grade = correct - module.save() - self.num_changed += 1 - else: - # don't make the change, but log that the change would be made - log_msg = ( - u"Grade would change from %(grade)s to %(correct)s for %(type)s module %(id)s for student " + - u"%(student)s in course %(course_id)s" - ) - - LOG.debug(log_msg, { - "grade": module.grade, - "correct": correct, - "type": module.module_type, - "id": module.module_state_key, - "student": module.student.username, - "course_id": module.course_id, - }) - - self.num_changed += 1 - - def handle(self, **options): - '''Handle management command request''' - - save_changes = options['save_changes'] - - LOG.info("Starting run: save_changes = %s", save_changes) - - self.fix_studentmodules(save_changes) - - LOG.info("Finished run: updating %s of %s modules", self.num_changed, self.num_visited) diff --git a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py index 01812093a6..d292f1fb73 100644 --- a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py +++ b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py @@ -1,19 +1,18 @@ # coding=utf-8 -"""Tests for Django management commands""" +""" +Tests for Django management commands +""" import json -import shutil -import tarfile from StringIO import StringIO -from tempfile import mkdtemp + +from nose.plugins.attrib import attr +from six import text_type import factory from django.conf import settings from django.core.management import call_command -from nose.plugins.attrib import attr -from path import Path as path - from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ( @@ -49,7 +48,9 @@ class CommandsTestBase(SharedModuleStoreTestCase): @classmethod def load_courses(cls): - """Load test courses and return list of ids""" + """ + Load test courses and return list of ids + """ store = modulestore() unique_org = factory.Sequence(lambda n: 'edX.%d' % n) @@ -76,7 +77,9 @@ class CommandsTestBase(SharedModuleStoreTestCase): return [course.id for course in store.get_courses()] def call_command(self, name, *args, **kwargs): - """Call management command and return output""" + """ + Call management command and return output + """ out = StringIO() # To Capture the output of the command call_command(name, *args, stdout=out, **kwargs) out.seek(0) @@ -85,12 +88,12 @@ class CommandsTestBase(SharedModuleStoreTestCase): def test_dump_course_ids(self): output = self.call_command('dump_course_ids') dumped_courses = output.decode('utf-8').strip().split('\n') - course_ids = {unicode(course_id) for course_id in self.loaded_courses} + course_ids = {text_type(course_id) for course_id in self.loaded_courses} dumped_ids = set(dumped_courses) self.assertEqual(course_ids, dumped_ids) def test_correct_course_structure_metadata(self): - course_id = unicode(self.test_course_key) + course_id = text_type(self.test_course_key) args = [course_id] kwargs = {'modulestore': 'default'} @@ -103,7 +106,7 @@ class CommandsTestBase(SharedModuleStoreTestCase): self.assertGreater(len(dump.values()), 0) def test_dump_course_structure(self): - args = [unicode(self.test_course_key)] + args = [text_type(self.test_course_key)] kwargs = {'modulestore': 'default'} output = self.call_command('dump_course_structure', *args, **kwargs) @@ -119,7 +122,7 @@ class CommandsTestBase(SharedModuleStoreTestCase): # Check a few elements in the course dump test_course_key = self.test_course_key - parent_id = unicode(test_course_key.make_usage_key('chapter', 'Overview')) + parent_id = text_type(test_course_key.make_usage_key('chapter', 'Overview')) self.assertEqual(dump[parent_id]['category'], 'chapter') self.assertEqual(len(dump[parent_id]['children']), 3) @@ -127,7 +130,7 @@ class CommandsTestBase(SharedModuleStoreTestCase): self.assertEqual(dump[child_id]['category'], 'videosequence') self.assertEqual(len(dump[child_id]['children']), 2) - video_id = unicode(test_course_key.make_usage_key('video', 'Welcome')) + video_id = text_type(test_course_key.make_usage_key('video', 'Welcome')) self.assertEqual(dump[video_id]['category'], 'video') self.assertItemsEqual( dump[video_id]['metadata'].keys(), @@ -140,7 +143,7 @@ class CommandsTestBase(SharedModuleStoreTestCase): self.assertEqual(len(dump), 17) def test_dump_inherited_course_structure(self): - args = [unicode(self.test_course_key)] + args = [text_type(self.test_course_key)] kwargs = {'modulestore': 'default', 'inherited': True} output = self.call_command('dump_course_structure', *args, **kwargs) dump = json.loads(output) @@ -155,7 +158,7 @@ class CommandsTestBase(SharedModuleStoreTestCase): self.assertNotIn('due', element['inherited_metadata']) def test_dump_inherited_course_structure_with_defaults(self): - args = [unicode(self.test_course_key)] + args = [text_type(self.test_course_key)] kwargs = {'modulestore': 'default', 'inherited': True, 'inherited_defaults': True} output = self.call_command('dump_course_structure', *args, **kwargs) dump = json.loads(output) @@ -170,37 +173,18 @@ class CommandsTestBase(SharedModuleStoreTestCase): self.assertIsNone(element['inherited_metadata']['due']) def test_export_discussion_ids(self): - output = self.call_command('dump_course_structure', unicode(self.course.id)) + output = self.call_command('dump_course_structure', text_type(self.course.id)) dump = json.loads(output) - dumped_id = dump[unicode(self.discussion.location)]['metadata']['discussion_id'] + dumped_id = dump[text_type(self.discussion.location)]['metadata']['discussion_id'] self.assertEqual(dumped_id, self.discussion.discussion_id) def test_export_discussion_id_custom_id(self): - output = self.call_command('dump_course_structure', unicode(self.test_course_key)) + output = self.call_command('dump_course_structure', text_type(self.test_course_key)) dump = json.loads(output) - discussion_key = unicode(self.test_course_key.make_usage_key('discussion', 'custom_id')) - dumped_id = dump[unicode(discussion_key)]['metadata']['discussion_id'] + discussion_key = text_type(self.test_course_key.make_usage_key('discussion', 'custom_id')) + dumped_id = dump[text_type(discussion_key)]['metadata']['discussion_id'] self.assertEqual(dumped_id, "custom") - def test_export_course(self): - tmp_dir = path(mkdtemp()) - self.addCleanup(shutil.rmtree, tmp_dir) - filename = tmp_dir / 'test.tar.gz' - - self.run_export_course(filename) - with tarfile.open(filename) as tar_file: - self.check_export_file(tar_file) - - def test_export_course_stdout(self): - output = self.run_export_course('-') - with tarfile.open(fileobj=StringIO(output)) as tar_file: - self.check_export_file(tar_file) - - def run_export_course(self, filename): # pylint: disable=missing-docstring - args = [unicode(self.test_course_key), filename] - kwargs = {'modulestore': 'default'} - return self.call_command('export_course', *args, **kwargs) - def check_export_file(self, tar_file): # pylint: disable=missing-docstring names = tar_file.getnames()