diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py index 380bc30c5f..636aae326e 100644 --- a/common/djangoapps/course_groups/cohorts.py +++ b/common/djangoapps/course_groups/cohorts.py @@ -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 ) diff --git a/common/test/acceptance/fixtures/course.py b/common/test/acceptance/fixtures/course.py index e8cbe500bc..560c6a460f 100644 --- a/common/test/acceptance/fixtures/course.py +++ b/common/test/acceptance/fixtures/course.py @@ -10,8 +10,6 @@ from collections import namedtuple from path import path from lazy import lazy -from xmodule.modulestore. - from . import STUDIO_BASE_URL diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 8debf9eeb0..16ce53f112 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -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] diff --git a/lms/djangoapps/bulk_email/tests/test_err_handling.py b/lms/djangoapps/bulk_email/tests/test_err_handling.py index 27a9fa4e10..96d4dd039f 100644 --- a/lms/djangoapps/bulk_email/tests/test_err_handling.py +++ b/lms/djangoapps/bulk_email/tests/test_err_handling.py @@ -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 diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index bbce3ac6e1..6642aca5ba 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -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! diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index 9366122e6c..f7d9c70209 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/lms/djangoapps/dashboard/sysadmin.py @@ -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'

{0}

'.format( _('Courses loaded in the modulestore')) self.msg += u'
    ' - for (cdir, course) in courses.items(): + for course in self.get_courses(): self.msg += u'
  1. {0} ({1})
    2. '.format( - escape(cdir), course.location.to_deprecated_string()) + escape(course.id.to_deprecated_string()), course.location.to_deprecated_string()) self.msg += u'
' 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}
{1}
').format( - course_id, escape(str(err)) - ) + self.msg += _( + 'Error - cannot get course with ID {0}
{1}
' + ).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, diff --git a/lms/djangoapps/dashboard/tests/__init__.py b/lms/djangoapps/dashboard/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/instructor_task/models.py b/lms/djangoapps/instructor_task/models.py index d279028379..884c25f16c 100644 --- a/lms/djangoapps/instructor_task/models.py +++ b/lms/djangoapps/instructor_task/models.py @@ -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 diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index e5e0be9397..41423ca94e 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -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. diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index e04a5b3d8d..af9ffe1de0 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -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)