From 2c586ddbe4235392ce07f90d2aa668f5042bcf9a Mon Sep 17 00:00:00 2001 From: Chris Rossi Date: Wed, 17 Dec 2014 11:04:00 -0500 Subject: [PATCH] MIT: CCX. Code Quality fixes --- lms/djangoapps/pocs/tests/factories.py | 1 + lms/djangoapps/pocs/tests/test_utils.py | 1 - lms/djangoapps/pocs/tests/test_views.py | 4 +- lms/djangoapps/pocs/views.py | 67 +++++++++++++++++-------- 4 files changed, 49 insertions(+), 24 deletions(-) diff --git a/lms/djangoapps/pocs/tests/factories.py b/lms/djangoapps/pocs/tests/factories.py index 1e98ae8846..7cd54f8501 100644 --- a/lms/djangoapps/pocs/tests/factories.py +++ b/lms/djangoapps/pocs/tests/factories.py @@ -13,5 +13,6 @@ class PocMembershipFactory(DjangoModelFactory): FACTORY_FOR = PocMembership active = False + class PocFutureMembershipFactory(DjangoModelFactory): FACTORY_FOR = PocFutureMembership diff --git a/lms/djangoapps/pocs/tests/test_utils.py b/lms/djangoapps/pocs/tests/test_utils.py index 9094215b9e..0fe9fbeee0 100644 --- a/lms/djangoapps/pocs/tests/test_utils.py +++ b/lms/djangoapps/pocs/tests/test_utils.py @@ -308,7 +308,6 @@ class TestEnrollEmail(ModuleStoreTestCase): # ensure there are still no emails in the outbox now self.assertEqual(len(self.outbox), 0) - def test_enroll_non_member_no_email(self): """register a non-member but send no email""" self.create_user() diff --git a/lms/djangoapps/pocs/tests/test_views.py b/lms/djangoapps/pocs/tests/test_views.py index edf4c6c040..032a5ccb88 100644 --- a/lms/djangoapps/pocs/tests/test_views.py +++ b/lms/djangoapps/pocs/tests/test_views.py @@ -502,7 +502,7 @@ class TestPocGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertEqual(response.status_code, 200) grades = response.mako_context['grade_summary'] self.assertEqual(grades['percent'], 0.5) - self.assertEqual( grades['grade_breakdown'][0]['percent'], 0.5) + self.assertEqual(grades['grade_breakdown'][0]['percent'], 0.5) self.assertEqual(len(grades['section_breakdown']), 4) @@ -524,8 +524,10 @@ def iter_blocks(course): yield descendant return visit(course) + def visible_children(block): block_get_children = block.get_children + def get_children(): def iter_children(): for child in block_get_children(): diff --git a/lms/djangoapps/pocs/views.py b/lms/djangoapps/pocs/views.py index 86317ca6ad..1bcc939fc0 100644 --- a/lms/djangoapps/pocs/views.py +++ b/lms/djangoapps/pocs/views.py @@ -1,3 +1,6 @@ +""" +Views related to the Personal Online Courses feature. +""" import csv import datetime import functools @@ -40,7 +43,7 @@ from .overrides import ( from .utils import enroll_email, unenroll_email log = logging.getLogger(__name__) -today = datetime.datetime.today # for patching in tests +TODAY = datetime.datetime.today # for patching in tests def coach_dashboard(view): @@ -51,6 +54,10 @@ def coach_dashboard(view): """ @functools.wraps(view) def wrapper(request, course_id): + """ + Wraps the view function, performing access check, loading the course, + and modifying the view's call signature. + """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) role = CoursePocCoachRole(course_key) if not role.has_user(request.user): @@ -79,7 +86,7 @@ def dashboard(request, course): 'gradebook_url': reverse('poc_gradebook', kwargs={'course_id': course.id}), 'grades_csv_url': reverse('poc_grades_csv', - kwargs={'course_id': course.id}), + kwargs={'course_id': course.id}), } if not poc: context['create_poc_url'] = reverse( @@ -102,7 +109,7 @@ def create_poc(request, course): poc.save() # Make sure start/due are overridden for entire course - start = today().replace(tzinfo=pytz.UTC) + start = TODAY().replace(tzinfo=pytz.UTC) override_field_for_poc(poc, course, 'start', start) override_field_for_poc(poc, course, 'due', None) @@ -124,11 +131,16 @@ def create_poc(request, course): @coach_dashboard def save_poc(request, course): """ - Save changes to POC + Save changes to POC. """ poc = get_poc_for_coach(course, request.user) def override_fields(parent, data, earliest=None): + """ + Recursively apply POC schedule data to POC by overriding the + `visible_to_staff_only`, `start` and `due` fields for units in the + course. + """ blocks = { str(child.location): child for child in parent.get_children()} @@ -163,16 +175,17 @@ def save_poc(request, course): content_type='application/json') -def parse_date(s): - if s: - try: - date, time = s.split(' ') - year, month, day = map(int, date.split('-')) - hour, minute = map(int, time.split(':')) - return datetime.datetime( - year, month, day, hour, minute, tzinfo=pytz.UTC) - except: - log.warn("Unable to parse date: " + s) +def parse_date(datestring): + """ + Generate a UTC datetime.datetime object from a string of the form + 'YYYY-MM-DD HH:MM'. If string is empty or `None`, returns `None`. + """ + if datestring: + date, time = datestring.split(' ') + year, month, day = map(int, date.split('-')) + hour, minute = map(int, time.split(':')) + return datetime.datetime( + year, month, day, hour, minute, tzinfo=pytz.UTC) return None @@ -192,8 +205,12 @@ def get_poc_for_coach(course, coach): def get_poc_schedule(course, poc): """ + Generate a JSON serializable POC schedule. """ def visit(node, depth=1): + """ + Recursive generator function which yields POC schedule nodes. + """ for child in node.get_children(): start = get_override_for_poc(poc, child, 'start', None) if start: @@ -301,8 +318,11 @@ def poc_gradebook(request, course): poc = get_poc_for_coach(course, request.user) with poc_context(poc): - course._field_data_cache = {} - course.set_grading_policy(course.grading_policy) # this is so awful + # The grading policy for the MOOC is probably already cached. We need + # to make sure we have the POC grading policy loaded. + course._field_data_cache = {} # pylint: disable=protected-access + course.set_grading_policy(course.grading_policy) + enrolled_students = User.objects.filter( pocmembership__poc=poc, pocmembership__active=1 @@ -343,8 +363,11 @@ def poc_grades_csv(request, course): poc = get_poc_for_coach(course, request.user) with poc_context(poc): - course._field_data_cache = {} - course.set_grading_policy(course.grading_policy) # this is so awful + # The grading policy for the MOOC is probably already cached. We need + # to make sure we have the POC grading policy loaded. + course._field_data_cache = {} # pylint: disable=protected-access + course.set_grading_policy(course.grading_policy) + enrolled_students = User.objects.filter( pocmembership__poc=poc, pocmembership__active=1 @@ -353,7 +376,7 @@ def poc_grades_csv(request, course): header = None rows = [] - for student, gradeset, err_msg in grades: + for student, gradeset, __ in grades: if gradeset: # We were able to successfully grade this student for this # course. @@ -374,9 +397,9 @@ def poc_grades_csv(request, course): rows.append([student.id, student.email, student.username, gradeset['percent']] + row_percents) - buffer = StringIO() - writer = csv.writer(buffer) + buf = StringIO() + writer = csv.writer(buf) for row in rows: writer.writerow(row) - return HttpResponse(buffer.getvalue(), content_type='text/plain') + return HttpResponse(buf.getvalue(), content_type='text/plain')