Merge pull request #3642 from cpennington/opaque-keys-return-course
Fix some tests on the opaque keys branch
This commit is contained in:
@@ -246,7 +246,7 @@ def add_user_to_cohort(cohort, username_or_email):
|
||||
previous_cohort = None
|
||||
|
||||
course_cohorts = CourseUserGroup.objects.filter(
|
||||
course_id=cohort.course_key,
|
||||
course_id=cohort.course_id,
|
||||
users__id=user.id,
|
||||
group_type=CourseUserGroup.COHORT
|
||||
)
|
||||
|
||||
@@ -10,8 +10,6 @@ from collections import namedtuple
|
||||
from path import path
|
||||
from lazy import lazy
|
||||
|
||||
from xmodule.modulestore.
|
||||
|
||||
from . import STUDIO_BASE_URL
|
||||
|
||||
|
||||
|
||||
@@ -161,7 +161,7 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name)
|
||||
# code that doesn't need the entry_id.
|
||||
if course_id != entry.course_id:
|
||||
format_msg = u"Course id conflict: explicit value %r does not match task value %r"
|
||||
log.warning("Task %s: " + format_msg, task_id, course_id, entry.course_id)
|
||||
log.warning(u"Task %s: " + format_msg, task_id, course_id, entry.course_id)
|
||||
raise ValueError(format_msg % (course_id, entry.course_id))
|
||||
|
||||
# Fetch the CourseEmail.
|
||||
@@ -171,7 +171,7 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name)
|
||||
except CourseEmail.DoesNotExist:
|
||||
# The CourseEmail object should be committed in the view function before the task
|
||||
# is submitted and reaches this point.
|
||||
log.warning("Task %s: Failed to get CourseEmail with id %s", task_id, email_id)
|
||||
log.warning(u"Task %s: Failed to get CourseEmail with id %s", task_id, email_id)
|
||||
raise
|
||||
|
||||
# Check to see if email batches have already been defined. This seems to
|
||||
@@ -182,21 +182,22 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name)
|
||||
# So we just return right away. We don't raise an exception, because we want
|
||||
# the current task to be marked with whatever it had been marked with before.
|
||||
if len(entry.subtasks) > 0 and len(entry.task_output) > 0:
|
||||
log.warning("Task %s has already been processed for email %s! InstructorTask = %s", task_id, email_id, entry)
|
||||
log.warning(u"Task %s has already been processed for email %s! InstructorTask = %s", task_id, email_id, entry)
|
||||
progress = json.loads(entry.task_output)
|
||||
return progress
|
||||
|
||||
# Sanity check that course for email_obj matches that of the task referencing it.
|
||||
if course_id != email_obj.course_id:
|
||||
format_msg = u"Course id conflict: explicit value %r does not match email value %r"
|
||||
log.warning("Task %s: " + format_msg, task_id, course_id, email_obj.course_id)
|
||||
log.warning(u"Task %s: " + format_msg, task_id, course_id, email_obj.course_id)
|
||||
raise ValueError(format_msg % (course_id, email_obj.course_id))
|
||||
|
||||
|
||||
# Fetch the course object.
|
||||
course = get_course(course_id)
|
||||
|
||||
if course is None:
|
||||
msg = "Task %s: course not found: %s"
|
||||
msg = u"Task %s: course not found: %s"
|
||||
log.error(msg, task_id, course_id)
|
||||
raise ValueError(msg % (task_id, course_id))
|
||||
|
||||
@@ -282,7 +283,7 @@ def send_course_email(entry_id, email_id, to_list, global_email_context, subtask
|
||||
subtask_status = SubtaskStatus.from_dict(subtask_status_dict)
|
||||
current_task_id = subtask_status.task_id
|
||||
num_to_send = len(to_list)
|
||||
log.info("Preparing to send email %s to %d recipients as subtask %s for instructor task %d: context = %s, status=%s",
|
||||
log.info(u"Preparing to send email %s to %d recipients as subtask %s for instructor task %d: context = %s, status=%s",
|
||||
email_id, num_to_send, current_task_id, entry_id, global_email_context, subtask_status)
|
||||
|
||||
# Check that the requested subtask is actually known to the current InstructorTask entry.
|
||||
@@ -660,7 +661,7 @@ def _submit_for_retry(entry_id, email_id, to_list, global_email_context, current
|
||||
)
|
||||
except RetryTaskError as retry_error:
|
||||
# If the retry call is successful, update with the current progress:
|
||||
log.exception('Task %s: email with id %d caused send_course_email task to retry.',
|
||||
log.exception(u'Task %s: email with id %d caused send_course_email task to retry.',
|
||||
task_id, email_id)
|
||||
return subtask_status, retry_error
|
||||
except Exception as retry_exc:
|
||||
@@ -669,7 +670,7 @@ def _submit_for_retry(entry_id, email_id, to_list, global_email_context, current
|
||||
# (and put it in retry_exc just in case it's different, but it shouldn't be),
|
||||
# and update status as if it were any other failure. That means that
|
||||
# the recipients still in the to_list are counted as failures.
|
||||
log.exception('Task %s: email with id %d caused send_course_email task to fail to retry. To list: %s',
|
||||
log.exception(u'Task %s: email with id %d caused send_course_email task to fail to retry. To list: %s',
|
||||
task_id, email_id, [i['email'] for i in to_list])
|
||||
num_failed = len(to_list)
|
||||
subtask_status.increment(subtask_status, failed=num_failed, state=FAILURE)
|
||||
@@ -680,5 +681,5 @@ def _statsd_tag(course_title):
|
||||
"""
|
||||
Calculate the tag we will use for DataDog.
|
||||
"""
|
||||
tag = u"course_email:{0}".format(course_title)
|
||||
tag = u"course_email:{0}".format(course_title).encode('utf-8')
|
||||
return tag[:200]
|
||||
|
||||
@@ -198,7 +198,7 @@ class TestEmailErrors(ModuleStoreTestCase):
|
||||
"""
|
||||
email = CourseEmail(course_id=self.course.id, to_option=SEND_TO_ALL)
|
||||
email.save()
|
||||
entry = InstructorTask.create("bogus_task_id", "task_type", "task_key", "task_input", self.instructor)
|
||||
entry = InstructorTask.create("bogus/task/id", "task_type", "task_key", "task_input", self.instructor)
|
||||
task_input = {"email_id": email.id} # pylint: disable=E1101
|
||||
with self.assertRaisesRegexp(ValueError, 'does not match task value'):
|
||||
perform_delegate_email_batches(entry.id, self.course.id, task_input, "action_name") # pylint: disable=E1101
|
||||
|
||||
@@ -55,6 +55,7 @@ def get_course(course_id, depth=0):
|
||||
raise ValueError(u"Course not found: {0}".format(course_id))
|
||||
except InvalidLocationError:
|
||||
raise ValueError(u"Invalid location: {0}".format(course_id))
|
||||
return course
|
||||
|
||||
|
||||
# TODO please rename this function to get_course_by_key at next opportunity!
|
||||
|
||||
@@ -78,10 +78,7 @@ class SysadminDashboardView(TemplateView):
|
||||
def get_courses(self):
|
||||
""" Get an iterable list of courses."""
|
||||
|
||||
courses = self.def_ms.get_courses()
|
||||
courses = dict([c.id.to_deprecated_string(), c] for c in courses) # no course directory
|
||||
|
||||
return courses
|
||||
return self.def_ms.get_courses()
|
||||
|
||||
def return_csv(self, filename, header, data):
|
||||
"""
|
||||
@@ -247,7 +244,6 @@ class Users(SysadminDashboardView):
|
||||
"""Returns the datatable used for this view"""
|
||||
|
||||
self.datatable = {}
|
||||
courses = self.get_courses()
|
||||
|
||||
self.datatable = dict(header=[_('Statistic'), _('Value')],
|
||||
title=_('Site statistics'))
|
||||
@@ -257,9 +253,9 @@ class Users(SysadminDashboardView):
|
||||
self.msg += u'<h2>{0}</h2>'.format(
|
||||
_('Courses loaded in the modulestore'))
|
||||
self.msg += u'<ol>'
|
||||
for (cdir, course) in courses.items():
|
||||
for course in self.get_courses():
|
||||
self.msg += u'<li>{0} ({1})</li>'.format(
|
||||
escape(cdir), course.location.to_deprecated_string())
|
||||
escape(course.id.to_deprecated_string()), course.location.to_deprecated_string())
|
||||
self.msg += u'</ol>'
|
||||
|
||||
def get(self, request):
|
||||
@@ -487,11 +483,10 @@ class Courses(SysadminDashboardView):
|
||||
"""Creates course information datatable"""
|
||||
|
||||
data = []
|
||||
courses = self.get_courses()
|
||||
|
||||
for (cdir, course) in courses.items():
|
||||
gdir = cdir.run
|
||||
data.append([course.display_name, cdir]
|
||||
for course in self.get_courses():
|
||||
gdir = course.id.run
|
||||
data.append([course.display_name, course.id.to_deprecated_string()]
|
||||
+ self.git_info_for_course(gdir))
|
||||
|
||||
return dict(header=[_('Course Name'), _('Directory/ID'),
|
||||
@@ -525,7 +520,7 @@ class Courses(SysadminDashboardView):
|
||||
track.views.server_track(request, action, {},
|
||||
page='courses_sysdashboard')
|
||||
|
||||
courses = self.get_courses()
|
||||
courses = {course.id: course for course in self.get_courses()}
|
||||
if action == 'add_course':
|
||||
gitloc = request.POST.get('repo_location', '').strip().replace(' ', '').replace(';', '')
|
||||
branch = request.POST.get('repo_branch', '').strip().replace(' ', '').replace(';', '')
|
||||
@@ -544,10 +539,12 @@ class Courses(SysadminDashboardView):
|
||||
course = get_course_by_id(course_key)
|
||||
course_found = True
|
||||
except Exception, err: # pylint: disable=broad-except
|
||||
self.msg += _('Error - cannot get course with ID '
|
||||
'{0}<br/><pre>{1}</pre>').format(
|
||||
course_id, escape(str(err))
|
||||
)
|
||||
self.msg += _(
|
||||
'Error - cannot get course with ID {0}<br/><pre>{1}</pre>'
|
||||
).format(
|
||||
course_key,
|
||||
escape(str(err))
|
||||
)
|
||||
|
||||
is_xml_course = (modulestore().get_modulestore_type(course_key) == XML_MODULESTORE_TYPE)
|
||||
if course_found and is_xml_course:
|
||||
@@ -599,9 +596,7 @@ class Staffing(SysadminDashboardView):
|
||||
raise Http404
|
||||
data = []
|
||||
|
||||
courses = self.get_courses()
|
||||
|
||||
for (cdir, course) in courses.items(): # pylint: disable=unused-variable
|
||||
for course in self.get_courses(): # pylint: disable=unused-variable
|
||||
datum = [course.display_name, course.id]
|
||||
datum += [CourseEnrollment.objects.filter(
|
||||
course_id=course.id).count()]
|
||||
@@ -635,9 +630,7 @@ class Staffing(SysadminDashboardView):
|
||||
data = []
|
||||
roles = [CourseInstructorRole, CourseStaffRole, ]
|
||||
|
||||
courses = self.get_courses()
|
||||
|
||||
for (cdir, course) in courses.items(): # pylint: disable=unused-variable
|
||||
for course in self.get_courses(): # pylint: disable=unused-variable
|
||||
for role in roles:
|
||||
for user in role(course.id).users_with_role():
|
||||
datum = [course.id, role, user.username, user.email,
|
||||
|
||||
0
lms/djangoapps/dashboard/tests/__init__.py
Normal file
0
lms/djangoapps/dashboard/tests/__init__.py
Normal file
@@ -160,7 +160,7 @@ class InstructorTask(models.Model):
|
||||
Truncation is indicated by adding "..." to the end of the value.
|
||||
"""
|
||||
tag = '...'
|
||||
task_progress = {'exception': type(exception).__name__, 'message': str(exception.message)}
|
||||
task_progress = {'exception': type(exception).__name__, 'message': unicode(exception.message)}
|
||||
if traceback_string is not None:
|
||||
# truncate any traceback that goes into the InstructorTask model:
|
||||
task_progress['traceback'] = traceback_string
|
||||
|
||||
@@ -108,16 +108,16 @@ class BaseInstructorTask(Task):
|
||||
Note that there is no way to record progress made within the task (e.g. attempted,
|
||||
succeeded, etc.) when such failures occur.
|
||||
"""
|
||||
TASK_LOG.debug('Task %s: failure returned', task_id)
|
||||
TASK_LOG.debug(u'Task %s: failure returned', task_id)
|
||||
entry_id = args[0]
|
||||
try:
|
||||
entry = InstructorTask.objects.get(pk=entry_id)
|
||||
except InstructorTask.DoesNotExist:
|
||||
# if the InstructorTask object does not exist, then there's no point
|
||||
# trying to update it.
|
||||
TASK_LOG.error("Task (%s) has no InstructorTask object for id %s", task_id, entry_id)
|
||||
TASK_LOG.error(u"Task (%s) has no InstructorTask object for id %s", task_id, entry_id)
|
||||
else:
|
||||
TASK_LOG.warning("Task (%s) failed: %s %s", task_id, einfo.exception, einfo.traceback)
|
||||
TASK_LOG.warning(u"Task (%s) failed", task_id, exc_info=True)
|
||||
entry.task_output = InstructorTask.create_output_for_failure(einfo.exception, einfo.traceback)
|
||||
entry.task_state = FAILURE
|
||||
entry.save_now()
|
||||
@@ -296,7 +296,7 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta
|
||||
num_attempted += 1
|
||||
# There is no try here: if there's an error, we let it throw, and the task will
|
||||
# be marked as FAILED, with a stack trace.
|
||||
with dog_stats_api.timer('instructor_tasks.module.time.step', tags=['action:{name}'.format(name=action_name)]):
|
||||
with dog_stats_api.timer('instructor_tasks.module.time.step', tags=[u'action:{name}'.format(name=action_name)]):
|
||||
update_status = update_fcn(module_descriptor, module_to_update)
|
||||
if update_status == UPDATE_STATUS_SUCCEEDED:
|
||||
# If the update_fcn returns true, then it performed some kind of work.
|
||||
|
||||
@@ -262,7 +262,7 @@ class PaidCourseRegistrationTest(ModuleStoreTestCase):
|
||||
self.assertEqual(reg1.user, self.user)
|
||||
self.assertEqual(reg1.status, "cart")
|
||||
self.assertTrue(PaidCourseRegistration.contained_in_order(self.cart, self.course_key))
|
||||
self.assertFalse(PaidCourseRegistration.contained_in_order(self.cart, SlashSeparatedCourseKey("MITx", "999", "Robot_Super_Course_abcd"))
|
||||
self.assertFalse(PaidCourseRegistration.contained_in_order(self.cart, SlashSeparatedCourseKey("MITx", "999", "Robot_Super_Course_abcd")))
|
||||
|
||||
self.assertEqual(self.cart.total_cost, self.cost)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user