Unverified Commit 319aa877 by Troy Sankey Committed by GitHub

Merge pull request #16487 from edx/pwnage101/courseware_mgmt_cleanup

courseware management commands cleanup for django 1.11
parents f1f5a7dd 150096e3
"""
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'
option_list = BaseCommand.option_list + (
make_option('--nostatic',
help = 'Import the specified data directory into the default ModuleStore.'
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'),
)
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: <data directory> [--nostatic] [<course dir>...]")
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,
......
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.
Usage: clean_xml PATH-TO-COURSE-DIR PATH-TO-OUTPUT-DIR [force]
help = 'Imports specified course, validates it, then exports it in a canonical format.'
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
force = False
if n == 3 and args[2] == 'force':
force = True
clean_xml(args[0], args[1], force)
course_dir = options['course_dir']
output_dir = options['output_dir']
force = options['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')
# 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',
def add_arguments(self, parser):
parser.add_argument('--modulestore',
default='default',
help='Name of the modulestore to use'),
)
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
"""
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 = "<course_id>"
help = dedent(__doc__).strip()
option_list = BaseCommand.option_list + (
make_option('--modulestore',
action='store',
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'),
make_option('--inherited',
help='name of the modulestore')
parser.add_argument('--inherited',
action='store_true',
default=False,
help='Whether to include inherited metadata'),
make_option('--inherited_defaults',
help='include inherited metadata'),
parser.add_argument('--inherited_defaults',
action='store_true',
default=False,
help='Whether to include default values of inherited metadata'),
)
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")
......
"""
A Django command that exports a course to a tar.gz file.
If <filename> 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 = "<course_id> <output_filename>"
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, <underscore>, <period> and <hyphen>.
# 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)
# 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 = "<start_date> <end_date>"
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
'''
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)
# 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()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment