diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index d87f893708..cce7a1785d 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1821,12 +1821,12 @@ def activate_account(request, key): # enroll student in any pending CCXs he/she may have if auto_enroll flag is set if settings.FEATURES.get('CUSTOM_COURSES_EDX'): from ccx.models import CcxMembership, CcxFutureMembership - pfms = CcxFutureMembership.objects.filter( + ccxfms = CcxFutureMembership.objects.filter( email=student[0].email ) - for pfm in pfms: - if pfm.auto_enroll: - CcxMembership.auto_enroll(student[0], pfm) + for ccxfm in ccxfms: + if ccxfm.auto_enroll: + CcxMembership.auto_enroll(student[0], ccxfm) resp = render_to_response( "registration/activation_complete.html", diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index b522d51a0f..0628743910 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -1225,25 +1225,17 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): all_descriptors = [] graded_sections = {} - def yield_descendents(module): - for child in module.get_children(): + def yield_descriptor_descendents(module_descriptor): + for child in module_descriptor.get_children(): yield child - for module_descriptor in yield_descendents(child): + for module_descriptor in yield_descriptor_descendents(child): yield module_descriptor -<<<<<<< HEAD for chapter in self.get_children(): for section in chapter.get_children(): if section.graded: xmoduledescriptors = list(yield_descriptor_descendents(section)) xmoduledescriptors.append(section) -======= - for c in module.get_children(): - for s in c.get_children(): - if s.graded: - xmoduledescriptors = list(yield_descendents(s)) - xmoduledescriptors.append(s) ->>>>>>> Hide course blocks not in the CCX from view for coaches and students # The xmoduledescriptors included here are only the ones that have scores. section_description = { diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 2b271de1e3..a28432918a 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -377,7 +377,7 @@ class DiscussionTab(EnrolledOrStaffTab): def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): if settings.FEATURES.get('CUSTOM_COURSES_EDX', False): - from ccx.overrides import get_current_ccx + from ccx.overrides import get_current_ccx # pylint: disable=import-error if get_current_ccx(): return False super_can_display = super(DiscussionTab, self).can_display( @@ -752,11 +752,16 @@ class CcxCoachTab(CourseTab): ) def can_display(self, course, settings, *args, **kw): + """ + Since we don't get the user here, we use a thread local defined in the ccx + overrides to get it, then use the course to get the coach role and find out if + the user is one. + """ user_is_coach = False if settings.FEATURES.get('CUSTOM_COURSES_EDX', False): from opaque_keys.edx.locations import SlashSeparatedCourseKey - from student.roles import CourseCcxCoachRole - from ccx.overrides import get_current_request + from student.roles import CourseCcxCoachRole # pylint: disable=import-error + from ccx.overrides import get_current_request # pylint: disable=import-error course_id = course.id.to_deprecated_string() course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) role = CourseCcxCoachRole(course_key) @@ -766,7 +771,7 @@ class CcxCoachTab(CourseTab): super_can_display = super(CcxCoachTab, self).can_display( course, settings, *args, **kw ) - return (user_is_coach and super_can_display) + return user_is_coach and super_can_display class CourseTabList(List): diff --git a/lms/djangoapps/ccx/__init__.py b/lms/djangoapps/ccx/__init__.py index cf6e925092..3584f0006a 100644 --- a/lms/djangoapps/ccx/__init__.py +++ b/lms/djangoapps/ccx/__init__.py @@ -1 +1,4 @@ +""" +we use this to mark the active ccx, for use by ccx middleware and some views +""" ACTIVE_CCX_KEY = '_ccx_id' diff --git a/lms/djangoapps/ccx/models.py b/lms/djangoapps/ccx/models.py index 835c5403d4..048b63d9c2 100644 --- a/lms/djangoapps/ccx/models.py +++ b/lms/djangoapps/ccx/models.py @@ -1,8 +1,11 @@ +""" +Models for the custom course feature +""" from django.contrib.auth.models import User from django.db import models -from student.models import CourseEnrollment, AlreadyEnrolledError -from xmodule_django.models import CourseKeyField, LocationKeyField +from student.models import CourseEnrollment, AlreadyEnrolledError # pylint: disable=import-error +from xmodule_django.models import CourseKeyField, LocationKeyField # pylint: disable=import-error class CustomCourseForEdX(models.Model): @@ -45,6 +48,9 @@ class CcxMembership(models.Model): @classmethod def memberships_for_user(cls, user, active=True): + """ + active memberships for a user + """ return cls.objects.filter(student=user, active__exact=active) @@ -65,7 +71,7 @@ class CcxFieldOverride(models.Model): location = LocationKeyField(max_length=255, db_index=True) field = models.CharField(max_length=255) - class Meta: + class Meta: # pylint: disable=missing-docstring,old-style-class unique_together = (('ccx', 'location', 'field'),) value = models.TextField(default='null') diff --git a/lms/djangoapps/ccx/overrides.py b/lms/djangoapps/ccx/overrides.py index aced5410f8..752de4bca6 100644 --- a/lms/djangoapps/ccx/overrides.py +++ b/lms/djangoapps/ccx/overrides.py @@ -1,6 +1,6 @@ """ API related to providing field overrides for individual students. This is used -by the individual due dates feature. +by the individual custom courses feature. """ import json import threading @@ -9,8 +9,8 @@ from contextlib import contextmanager from django.db import transaction, IntegrityError -from courseware.field_overrides import FieldOverrideProvider -from ccx import ACTIVE_CCX_KEY +from courseware.field_overrides import FieldOverrideProvider # pylint: disable=import-error +from ccx import ACTIVE_CCX_KEY # pylint: disable=import-error from .models import CcxMembership, CcxFieldOverride @@ -22,6 +22,9 @@ class CustomCoursesForEdxOverrideProvider(FieldOverrideProvider): overrides to be made on a per user basis. """ def get(self, block, name, default): + """ + Just call the get_override_for_ccx method if there is a ccx + """ ccx = get_current_ccx() if ccx: return get_override_for_ccx(ccx, block, name, default) @@ -58,15 +61,18 @@ def get_current_ccx(): """ Return the ccx that is active for this request. """ - ccx = _CCX_CONTEXT.ccx - if ccx: - return ccx + return _CCX_CONTEXT.ccx def get_current_request(): + """ + Return the active request, so that we can get context information in places + where it is limited, like in the tabs. + """ request = _CCX_CONTEXT.request return request + def get_override_for_ccx(ccx, block, name, default=None): """ Gets the value of the overridden field for the `ccx`. `block` and `name` @@ -74,11 +80,11 @@ def get_override_for_ccx(ccx, block, name, default=None): overridden for the given ccx, returns `default`. """ if not hasattr(block, '_ccx_overrides'): - block._ccx_overrides = {} - overrides = block._ccx_overrides.get(ccx.id) + block._ccx_overrides = {} # pylint: disable=protected-access + overrides = block._ccx_overrides.get(ccx.id) # pylint: disable=protected-access if overrides is None: overrides = _get_overrides_for_ccx(ccx, block) - block._ccx_overrides[ccx.id] = overrides + block._ccx_overrides[ccx.id] = overrides # pylint: disable=protected-access return overrides.get(name, default) @@ -110,20 +116,20 @@ def override_field_for_ccx(ccx, block, name, value): value = json.dumps(field.to_json(value)) try: override = CcxFieldOverride.objects.create( - ccx=ccx, - location=block.location, - field=name, - value=value) + ccx=ccx, + location=block.location, + field=name, + value=value) except IntegrityError: transaction.commit() override = CcxFieldOverride.objects.get( - ccx=ccx, - location=block.location, - field=name) + ccx=ccx, + location=block.location, + field=name) override.value = value override.save() if hasattr(block, '_ccx_overrides'): - del block._ccx_overrides[ccx.id] + del block._ccx_overrides[ccx.id] # pylint: disable=protected-access def clear_override_for_ccx(ccx, block, name): @@ -140,7 +146,7 @@ def clear_override_for_ccx(ccx, block, name): field=name).delete() if hasattr(block, '_ccx_overrides'): - del block._ccx_overrides[ccx.id] + del block._ccx_overrides[ccx.id] # pylint: disable=protected-access except CcxFieldOverride.DoesNotExist: pass @@ -169,7 +175,7 @@ class CcxMiddleware(object): _CCX_CONTEXT.request = request - def process_response(self, request, response): + def process_response(self, request, response): # pylint: disable=unused-argument """ Clean up afterwards. """ diff --git a/lms/djangoapps/ccx/tests/factories.py b/lms/djangoapps/ccx/tests/factories.py index 694a547b1a..36c976970f 100644 --- a/lms/djangoapps/ccx/tests/factories.py +++ b/lms/djangoapps/ccx/tests/factories.py @@ -1,18 +1,22 @@ +""" +Dummy factories for tests +""" from factory.django import DjangoModelFactory -from ccx.models import CustomCourseForEdX -from ccx.models import CcxMembership -from ccx.models import CcxFutureMembership +from ccx.models import CustomCourseForEdX # pylint: disable=import-error +from ccx.models import CcxMembership # pylint: disable=import-error +from ccx.models import CcxFutureMembership # pylint: disable=import-error -class CcxFactory(DjangoModelFactory): +class CcxFactory(DjangoModelFactory): # pylint: disable=missing-docstring FACTORY_FOR = CustomCourseForEdX display_name = "Test CCX" + id = None # pylint: disable=redefined-builtin, invalid-name -class CcxMembershipFactory(DjangoModelFactory): +class CcxMembershipFactory(DjangoModelFactory): # pylint: disable=missing-docstring FACTORY_FOR = CcxMembership active = False -class CcxFutureMembershipFactory(DjangoModelFactory): +class CcxFutureMembershipFactory(DjangoModelFactory): # pylint: disable=missing-docstring FACTORY_FOR = CcxFutureMembership diff --git a/lms/djangoapps/ccx/tests/test_models.py b/lms/djangoapps/ccx/tests/test_models.py index 9c164c1fc3..49689c4133 100644 --- a/lms/djangoapps/ccx/tests/test_models.py +++ b/lms/djangoapps/ccx/tests/test_models.py @@ -1,6 +1,9 @@ -from student.models import CourseEnrollment -from student.roles import CourseCcxCoachRole -from student.tests.factories import ( +""" +tests for the models +""" +from student.models import CourseEnrollment # pylint: disable=import-error +from student.roles import CourseCcxCoachRole # pylint: disable=import-error +from student.tests.factories import ( # pylint: disable=import-error AdminFactory, CourseEnrollmentFactory, UserFactory, @@ -10,7 +13,6 @@ from xmodule.modulestore.tests.factories import CourseFactory from .factories import ( CcxFactory, - CcxMembershipFactory, CcxFutureMembershipFactory, ) from ..models import ( @@ -36,6 +38,9 @@ class TestCcxMembership(ModuleStoreTestCase): self.unenrolled_user = UserFactory.create() def create_future_enrollment(self, user, auto_enroll=True): + """ + utility method to create future enrollment + """ pfm = CcxFutureMembershipFactory.create( ccx=self.ccx, email=user.email, @@ -44,24 +49,36 @@ class TestCcxMembership(ModuleStoreTestCase): return pfm def has_course_enrollment(self, user): + """ + utility method to create future enrollment + """ enrollment = CourseEnrollment.objects.filter( user=user, course_id=self.course.id ) return enrollment.exists() def has_ccx_membership(self, user): + """ + verify ccx membership + """ membership = CcxMembership.objects.filter( student=user, ccx=self.ccx, active=True ) return membership.exists() def has_ccx_future_membership(self, user): + """ + verify future ccx membership + """ future_membership = CcxFutureMembership.objects.filter( email=user.email, ccx=self.ccx ) return future_membership.exists() - def call_MUT(self, student, future_membership): + def call_mut(self, student, future_membership): + """ + Call the method undser test + """ CcxMembership.auto_enroll(student, future_membership) def test_ccx_auto_enroll_unregistered_user(self): @@ -75,7 +92,7 @@ class TestCcxMembership(ModuleStoreTestCase): self.assertTrue(self.has_ccx_future_membership(user)) self.assertFalse(self.has_course_enrollment(user)) # auto_enroll user - self.call_MUT(user, pfm) + self.call_mut(user, pfm) self.assertTrue(self.has_course_enrollment(user)) self.assertTrue(self.has_ccx_membership(user)) @@ -89,7 +106,7 @@ class TestCcxMembership(ModuleStoreTestCase): self.assertTrue(self.has_ccx_future_membership(user)) self.assertTrue(self.has_course_enrollment(user)) - self.call_MUT(user, pfm) + self.call_mut(user, pfm) self.assertTrue(self.has_course_enrollment(user)) self.assertTrue(self.has_ccx_membership(user)) @@ -103,7 +120,7 @@ class TestCcxMembership(ModuleStoreTestCase): self.assertTrue(self.has_ccx_future_membership(user)) self.assertFalse(self.has_course_enrollment(user)) - self.assertRaises(ValueError, self.call_MUT, user, pfm) + self.assertRaises(ValueError, self.call_mut, user, pfm) self.assertFalse(self.has_course_enrollment(user)) self.assertFalse(self.has_ccx_membership(user)) diff --git a/lms/djangoapps/ccx/tests/test_overrides.py b/lms/djangoapps/ccx/tests/test_overrides.py index c3e624a769..f6edd44a43 100644 --- a/lms/djangoapps/ccx/tests/test_overrides.py +++ b/lms/djangoapps/ccx/tests/test_overrides.py @@ -1,10 +1,13 @@ +""" +tests for overrides +""" import datetime import mock import pytz -from courseware.field_overrides import OverrideFieldData +from courseware.field_overrides import OverrideFieldData # pylint: disable=import-error from django.test.utils import override_settings -from student.tests.factories import AdminFactory +from student.tests.factories import AdminFactory # pylint: disable=import-error from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -13,6 +16,7 @@ from ..overrides import override_field_for_ccx from .test_views import flatten, iter_blocks + @override_settings(FIELD_OVERRIDE_PROVIDERS=( 'ccx.overrides.CustomCoursesForEdxOverrideProvider',)) class TestFieldOverrides(ModuleStoreTestCase): @@ -39,7 +43,7 @@ class TestFieldOverrides(ModuleStoreTestCase): verticals = flatten([ [ItemFactory.create(due=due, parent=sequential) for _ in xrange(2)] for sequential in sequentials]) - blocks = flatten([ + blocks = flatten([ # pylint: disable=unused-variable [ItemFactory.create(parent=vertical) for _ in xrange(2)] for vertical in verticals]) @@ -59,12 +63,14 @@ class TestFieldOverrides(ModuleStoreTestCase): # just inject the override field storage in this brute force manner. OverrideFieldData.provider_classes = None for block in iter_blocks(course): - block._field_data = OverrideFieldData.wrap( # pylint: disable=protected-access - AdminFactory.create(), block._field_data) # pylint: disable=protected-access + block._field_data = OverrideFieldData.wrap( # pylint: disable=protected-access + AdminFactory.create(), block._field_data) # pylint: disable=protected-access - # and after everything is done, clean up by un-doing the change to the - # OverrideFieldData object that is done during the wrap method. def cleanup_provider_classes(): + """ + After everything is done, clean up by un-doing the change to the + OverrideFieldData object that is done during the wrap method. + """ OverrideFieldData.provider_classes = None self.addCleanup(cleanup_provider_classes) diff --git a/lms/djangoapps/ccx/tests/test_utils.py b/lms/djangoapps/ccx/tests/test_utils.py index d9c21152f0..cd44efe458 100644 --- a/lms/djangoapps/ccx/tests/test_utils.py +++ b/lms/djangoapps/ccx/tests/test_utils.py @@ -1,14 +1,17 @@ -from ccx.models import ( +""" +test utils +""" +from ccx.models import ( # pylint: disable=import-error CcxMembership, CcxFutureMembership, ) -from ccx.tests.factories import ( +from ccx.tests.factories import ( # pylint: disable=import-error CcxFactory, CcxMembershipFactory, CcxFutureMembershipFactory, ) -from student.roles import CourseCcxCoachRole -from student.tests.factories import ( +from student.roles import CourseCcxCoachRole # pylint: disable=import-error +from student.tests.factories import ( # pylint: disable=import-error AdminFactory, UserFactory, CourseEnrollmentFactory, @@ -53,7 +56,7 @@ class TestEmailEnrollmentState(ModuleStoreTestCase): def create_one(self, email=None): """Create a single EmailEnrollmentState object and return it """ - from ccx.utils import EmailEnrollmentState + from ccx.utils import EmailEnrollmentState # pylint: disable=import-error if email is None: email = self.user.email return EmailEnrollmentState(self.ccx, email) @@ -138,17 +141,20 @@ class TestGetEmailParams(ModuleStoreTestCase): self.url_keys = [k for k in self.all_keys if 'url' in k] self.course_keys = [k for k in self.url_keys if 'course' in k] - def call_FUT(self, auto_enroll=False, secure=False): - from ccx.utils import get_email_params + def call_fut(self, auto_enroll=False, secure=False): + """ + call function under test + """ + from ccx.utils import get_email_params # pylint: disable=import-error return get_email_params(self.ccx, auto_enroll, secure) def test_params_have_expected_keys(self): - params = self.call_FUT() + params = self.call_fut() self.assertFalse(set(params.keys()) - set(self.all_keys)) def test_ccx_id_in_params(self): expected_course_id = self.ccx.course_id.to_deprecated_string() - params = self.call_FUT() + params = self.call_fut() self.assertEqual(params['course'], self.ccx) for url_key in self.url_keys: self.assertTrue('http://' in params[url_key]) @@ -156,17 +162,17 @@ class TestGetEmailParams(ModuleStoreTestCase): self.assertTrue(expected_course_id in params[url_key]) def test_security_respected(self): - secure = self.call_FUT(secure=True) + secure = self.call_fut(secure=True) for url_key in self.url_keys: self.assertTrue('https://' in secure[url_key]) - insecure = self.call_FUT(secure=False) + insecure = self.call_fut(secure=False) for url_key in self.url_keys: self.assertTrue('http://' in insecure[url_key]) def test_auto_enroll_passed_correctly(self): - not_auto = self.call_FUT(auto_enroll=False) + not_auto = self.call_fut(auto_enroll=False) self.assertFalse(not_auto['auto_enroll']) - auto = self.call_FUT(auto_enroll=True) + auto = self.call_fut(auto_enroll=True) self.assertTrue(auto['auto_enroll']) @@ -234,14 +240,15 @@ class TestEnrollEmail(ModuleStoreTestCase): self.assertEqual(member, state.member) self.assertEqual(user, state.user) - def call_FUT( - self, - student_email=None, - auto_enroll=False, - email_students=False, - email_params=None + def call_fut( + self, + student_email=None, + auto_enroll=False, + email_students=False, + email_params=None ): - from ccx.utils import enroll_email + """Call function under test""" + from ccx.utils import enroll_email # pylint: disable=import-error if student_email is None: student_email = self.user.email before, after = enroll_email( @@ -255,7 +262,7 @@ class TestEnrollEmail(ModuleStoreTestCase): # ensure no emails are in the outbox now self.assertEqual(self.outbox, []) test_email = "nobody@nowhere.com" - before, after = self.call_FUT( + before, after = self.call_fut( student_email=test_email, email_students=True ) @@ -274,7 +281,7 @@ class TestEnrollEmail(ModuleStoreTestCase): self.create_user() # ensure no emails are in the outbox now self.assertEqual(self.outbox, []) - before, after = self.call_FUT(email_students=True) + before, after = self.call_fut(email_students=True) # there should be a membership set for this email address now self.check_membership(email=self.user.email) @@ -291,7 +298,7 @@ class TestEnrollEmail(ModuleStoreTestCase): self.register_user_in_ccx() # ensure no emails are in the outbox now self.assertEqual(self.outbox, []) - before, after = self.call_FUT(email_students=True) + before, after = self.call_fut(email_students=True) # there should be a membership set for this email address now self.check_membership(email=self.user.email) @@ -308,7 +315,7 @@ class TestEnrollEmail(ModuleStoreTestCase): # ensure no emails are in the outbox now self.assertEqual(self.outbox, []) test_email = "nobody@nowhere.com" - before, after = self.call_FUT( + before, after = self.call_fut( student_email=test_email, email_students=False ) @@ -324,7 +331,7 @@ class TestEnrollEmail(ModuleStoreTestCase): self.create_user() # ensure no emails are in the outbox now self.assertEqual(self.outbox, []) - before, after = self.call_FUT(email_students=False) + before, after = self.call_fut(email_students=False) # there should be a membership set for this email address now self.check_membership(email=self.user.email) @@ -339,7 +346,7 @@ class TestEnrollEmail(ModuleStoreTestCase): self.register_user_in_ccx() # ensure no emails are in the outbox now self.assertEqual(self.outbox, []) - before, after = self.call_FUT(email_students=False) + before, after = self.call_fut(email_students=False) # there should be a membership set for this email address now self.check_membership(email=self.user.email) @@ -363,6 +370,7 @@ class TestUnenrollEmail(ModuleStoreTestCase): role.add_users(coach) self.ccx = CcxFactory(course_id=course.id, coach=coach) self.outbox = self.get_outbox() + self.email = "nobody@nowhere.com" def get_outbox(self): """Return the django mail outbox""" @@ -385,7 +393,6 @@ class TestUnenrollEmail(ModuleStoreTestCase): def make_ccx_future_membership(self): """create future registration for email in self.ccx""" - self.email = "nobody@nowhere.com" CcxFutureMembershipFactory.create( ccx=self.ccx, email=self.email ) @@ -402,6 +409,9 @@ class TestUnenrollEmail(ModuleStoreTestCase): self.assertEqual(user, state.user) def check_membership(self, future=False): + """ + check membership + """ if future: membership = CcxFutureMembership.objects.filter( ccx=self.ccx, email=self.email @@ -412,8 +422,9 @@ class TestUnenrollEmail(ModuleStoreTestCase): ) return membership.exists() - def call_FUT(self, email_students=False): - from ccx.utils import unenroll_email + def call_fut(self, email_students=False): + """call function under test""" + from ccx.utils import unenroll_email # pylint: disable=import-error email = getattr(self, 'user', None) and self.user.email or self.email return unenroll_email(self.ccx, email, email_students=email_students) @@ -425,7 +436,7 @@ class TestUnenrollEmail(ModuleStoreTestCase): self.assertTrue(self.check_membership(future=True)) self.assertEqual(self.outbox, []) # unenroll the student - before, after = self.call_FUT(email_students=True) + before, after = self.call_fut(email_students=True) # assert that membership is now gone self.assertFalse(self.check_membership(future=True)) @@ -444,7 +455,7 @@ class TestUnenrollEmail(ModuleStoreTestCase): self.assertTrue(self.check_membership()) self.assertEqual(self.outbox, []) # unenroll the student - before, after = self.call_FUT(email_students=True) + before, after = self.call_fut(email_students=True) # assert that membership is now gone self.assertFalse(self.check_membership()) @@ -464,7 +475,7 @@ class TestUnenrollEmail(ModuleStoreTestCase): self.assertTrue(self.check_membership(future=True)) self.assertEqual(self.outbox, []) # unenroll the student - before, after = self.call_FUT() + before, after = self.call_fut() # assert that membership is now gone self.assertFalse(self.check_membership(future=True)) @@ -482,7 +493,7 @@ class TestUnenrollEmail(ModuleStoreTestCase): self.assertTrue(self.check_membership()) self.assertEqual(self.outbox, []) # unenroll the student - before, after = self.call_FUT() + before, after = self.call_fut() # assert that membership is now gone self.assertFalse(self.check_membership()) @@ -516,34 +527,36 @@ class TestUserCCXList(ModuleStoreTestCase): CcxMembershipFactory(ccx=self.ccx, student=self.user, active=active) def get_course_title(self): - from courseware.courses import get_course_about_section + """Get course title""" + from courseware.courses import get_course_about_section # pylint: disable=import-error return get_course_about_section(self.course, 'title') - def call_FUT(self, user): - from ccx.utils import get_all_ccx_for_user + def call_fut(self, user): + """Call function under test""" + from ccx.utils import get_all_ccx_for_user # pylint: disable=import-error return get_all_ccx_for_user(user) def test_anonymous_sees_no_ccx(self): - memberships = self.call_FUT(self.anonymous) + memberships = self.call_fut(self.anonymous) self.assertEqual(memberships, []) def test_unenrolled_sees_no_ccx(self): - memberships = self.call_FUT(self.user) + memberships = self.call_fut(self.user) self.assertEqual(memberships, []) def test_enrolled_inactive_sees_no_ccx(self): self.register_user_in_ccx() - memberships = self.call_FUT(self.user) + memberships = self.call_fut(self.user) self.assertEqual(memberships, []) def test_enrolled_sees_a_ccx(self): self.register_user_in_ccx(active=True) - memberships = self.call_FUT(self.user) + memberships = self.call_fut(self.user) self.assertEqual(len(memberships), 1) def test_data_structure(self): self.register_user_in_ccx(active=True) - memberships = self.call_FUT(self.user) + memberships = self.call_fut(self.user) this_membership = memberships[0] self.assertTrue(this_membership) # structure contains the expected keys diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index 3f03288859..a86c681d36 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -1,3 +1,6 @@ +""" +test views +""" import datetime import json import re @@ -5,14 +8,14 @@ import pytz from mock import patch from capa.tests.response_xml_factory import StringResponseXMLFactory -from courseware.field_overrides import OverrideFieldData -from courseware.tests.factories import StudentModuleFactory -from courseware.tests.helpers import LoginEnrollmentTestCase +from courseware.field_overrides import OverrideFieldData # pylint: disable=import-error +from courseware.tests.factories import StudentModuleFactory # pylint: disable=import-error +from courseware.tests.helpers import LoginEnrollmentTestCase # pylint: disable=import-error from django.core.urlresolvers import reverse from django.test.utils import override_settings -from edxmako.shortcuts import render_to_response -from student.roles import CourseCcxCoachRole -from student.tests.factories import ( +from edxmako.shortcuts import render_to_response # pylint: disable=import-error +from student.roles import CourseCcxCoachRole # pylint: disable=import-error +from student.tests.factories import ( # pylint: disable=import-error AdminFactory, CourseEnrollmentFactory, UserFactory, @@ -23,7 +26,6 @@ from xmodule.modulestore.tests.factories import ( CourseFactory, ItemFactory, ) -from ccx import ACTIVE_CCX_KEY from ..models import ( CustomCourseForEdX, CcxMembership, @@ -75,29 +77,41 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): chapters = [ItemFactory.create(start=start, parent=course) for _ in xrange(2)] sequentials = flatten([ - [ItemFactory.create(parent=chapter) for _ in xrange(2)] - for chapter in chapters] - ) + [ + ItemFactory.create(parent=chapter) for _ in xrange(2) + ] for chapter in chapters + ]) verticals = flatten([ - [ItemFactory.create( - due=due, parent=sequential, graded=True, format='Homework') - for _ in xrange(2)] - for sequential in sequentials] - ) - blocks = flatten([ - [ItemFactory.create(parent=vertical) for _ in xrange(2)] - for vertical in verticals] - ) + [ + ItemFactory.create( + due=due, parent=sequential, graded=True, format='Homework' + ) for _ in xrange(2) + ] for sequential in sequentials + ]) + blocks = flatten([ # pylint: disable=unused-variable + [ + ItemFactory.create(parent=vertical) for _ in xrange(2) + ] for vertical in verticals + ]) def make_coach(self): + """ + create coach user + """ role = CourseCcxCoachRole(self.course.id) role.add_users(self.coach) def make_ccx(self): + """ + create ccx + """ ccx = CcxFactory(course_id=self.course.id, coach=self.coach) return ccx def get_outbox(self): + """ + get fake outbox + """ from django.core import mail return mail.outbox @@ -142,7 +156,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): kwargs={'course_id': self.course.id.to_deprecated_string()}) response = self.client.post(url, {'name': 'New CCX'}) self.assertEqual(response.status_code, 302) - url = response.get('location') + url = response.get('location') # pylint: disable=no-member response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertTrue(re.search('id="ccx-schedule"', response.content)) @@ -159,7 +173,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): 'ccx_coach_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()}) response = self.client.get(url) - schedule = json.loads(response.mako_context['schedule']) + schedule = json.loads(response.mako_context['schedule']) # pylint: disable=no-member self.assertEqual(len(schedule), 2) self.assertEqual(schedule[0]['hidden'], True) self.assertEqual(schedule[0]['start'], None) @@ -231,7 +245,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): ) data = { 'enrollment-button': 'Enroll', - 'student-ids': u','.join([student.email, ]), + 'student-ids': u','.join([student.email, ]), # pylint: disable=no-member 'email-students': 'Notify-students-by-email', } response = self.client.post(url, data=data, follow=True) @@ -240,7 +254,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertEqual(len(response.redirect_chain), 1) self.assertTrue(302 in response.redirect_chain[0]) self.assertEqual(len(outbox), 1) - self.assertTrue(student.email in outbox[0].recipients()) + self.assertTrue(student.email in outbox[0].recipients()) # pylint: disable=no-member # a CcxMembership exists for this student self.assertTrue( CcxMembership.objects.filter(ccx=ccx, student=student).exists() @@ -264,7 +278,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): ) data = { 'enrollment-button': 'Unenroll', - 'student-ids': u','.join([student.email, ]), + 'student-ids': u','.join([student.email, ]), # pylint: disable=no-member 'email-students': 'Notify-students-by-email', } response = self.client.post(url, data=data, follow=True) @@ -273,7 +287,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertEqual(len(response.redirect_chain), 1) self.assertTrue(302 in response.redirect_chain[0]) self.assertEqual(len(outbox), 1) - self.assertTrue(student.email in outbox[0].recipients()) + self.assertTrue(student.email in outbox[0].recipients()) # pylint: disable=no-member # the membership for this student is gone self.assertFalse( CcxMembership.objects.filter(ccx=ccx, student=student).exists() @@ -359,7 +373,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): ) data = { 'student-action': 'add', - 'student-id': u','.join([student.email, ]), + 'student-id': u','.join([student.email, ]), # pylint: disable=no-member } response = self.client.post(url, data=data, follow=True) self.assertEqual(response.status_code, 200) @@ -390,7 +404,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): ) data = { 'student-action': 'revoke', - 'student-id': u','.join([student.email, ]), + 'student-id': u','.join([student.email, ]), # pylint: disable=no-member } response = self.client.post(url, data=data, follow=True) self.assertEqual(response.status_code, 200) @@ -463,14 +477,16 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): # just inject the override field storage in this brute force manner. OverrideFieldData.provider_classes = None for block in iter_blocks(course): - block._field_data = OverrideFieldData.wrap( # pylint: disable=protected-access - coach, block._field_data) # pylint: disable=protected-access - block._field_data_cache = {} + block._field_data = OverrideFieldData.wrap( # pylint: disable=protected-access + coach, block._field_data) # pylint: disable=protected-access + block._field_data_cache = {} # pylint: disable=protected-access visible_children(block) - # and after everything is done, clean up by un-doing the change to the - # OverrideFieldData object that is done during the wrap method. def cleanup_provider_classes(): + """ + After everything is done, clean up by un-doing the change to the + OverrideFieldData object that is done during the wrap method. + """ OverrideFieldData.provider_classes = None self.addCleanup(cleanup_provider_classes) @@ -500,7 +516,7 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): ) response = self.client.get(url) self.assertEqual(response.status_code, 200) - student_info = response.mako_context['students'][0] + student_info = response.mako_context['students'][0] # pylint: disable=no-member self.assertEqual(student_info['grade_summary']['percent'], 0.5) self.assertEqual( student_info['grade_summary']['grade_breakdown'][0]['percent'], @@ -535,7 +551,7 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): self.client.login(username=self.student.username, password="test") session = self.client.session - session[ACTIVE_CCX_KEY] = self.ccx.id + session[ACTIVE_CCX_KEY] = self.ccx.id # pylint: disable=no-member session.save() self.client.session.get(ACTIVE_CCX_KEY) url = reverse( @@ -544,7 +560,7 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): ) response = self.client.get(url) self.assertEqual(response.status_code, 200) - grades = response.mako_context['grade_summary'] + grades = response.mako_context['grade_summary'] # pylint: disable=no-member self.assertEqual(grades['percent'], 0.5) self.assertEqual(grades['grade_breakdown'][0]['percent'], 0.5) self.assertEqual(len(grades['section_breakdown']), 4) @@ -574,13 +590,16 @@ class TestSwitchActiveCCX(ModuleStoreTestCase, LoginEnrollmentTestCase): CcxMembershipFactory(ccx=self.ccx, student=self.user, active=active) def revoke_ccx_registration(self): - from ..models import CcxMembership + """ + delete membership + """ membership = CcxMembership.objects.filter( ccx=self.ccx, student=self.user ) membership.delete() - def verify_active_ccx(self, request, id=None): + def verify_active_ccx(self, request, id=None): # pylint: disable=redefined-builtin, invalid-name + """verify that we have the correct active ccx""" if id: id = str(id) self.assertEqual(id, request.session.get(ACTIVE_CCX_KEY, None)) @@ -610,7 +629,7 @@ class TestSwitchActiveCCX(ModuleStoreTestCase, LoginEnrollmentTestCase): ) response = self.client.get(switch_url) self.assertEqual(response.status_code, 302) - self.assertTrue(response.get('Location', '').endswith(self.target_url)) + self.assertTrue(response.get('Location', '').endswith(self.target_url)) # pylint: disable=no-member # if the ccx were active, we'd need to pass the ID of the ccx here. self.verify_active_ccx(self.client) @@ -623,7 +642,7 @@ class TestSwitchActiveCCX(ModuleStoreTestCase, LoginEnrollmentTestCase): ) response = self.client.get(switch_url) self.assertEqual(response.status_code, 302) - self.assertTrue(response.get('Location', '').endswith(self.target_url)) + self.assertTrue(response.get('Location', '').endswith(self.target_url)) # pylint: disable=no-member self.verify_active_ccx(self.client, self.ccx.id) def test_enrolled_user_can_select_mooc(self): @@ -639,7 +658,7 @@ class TestSwitchActiveCCX(ModuleStoreTestCase, LoginEnrollmentTestCase): ) response = self.client.get(switch_url) self.assertEqual(response.status_code, 302) - self.assertTrue(response.get('Location', '').endswith(self.target_url)) + self.assertTrue(response.get('Location', '').endswith(self.target_url)) # pylint: disable=no-member self.verify_active_ccx(self.client) def test_unenrolled_user_cannot_select_ccx(self): @@ -650,7 +669,7 @@ class TestSwitchActiveCCX(ModuleStoreTestCase, LoginEnrollmentTestCase): ) response = self.client.get(switch_url) self.assertEqual(response.status_code, 302) - self.assertTrue(response.get('Location', '').endswith(self.target_url)) + self.assertTrue(response.get('Location', '').endswith(self.target_url)) # pylint: disable=no-member # if the ccx were active, we'd need to pass the ID of the ccx here. self.verify_active_ccx(self.client) @@ -666,7 +685,7 @@ class TestSwitchActiveCCX(ModuleStoreTestCase, LoginEnrollmentTestCase): ) response = self.client.get(switch_url) self.assertEqual(response.status_code, 302) - self.assertTrue(response.get('Location', '').endswith(self.target_url)) + self.assertTrue(response.get('Location', '').endswith(self.target_url)) # pylint: disable=no-member # we tried to select the ccx but are not registered, so we are switched # back to the mooc view self.verify_active_ccx(self.client) @@ -684,7 +703,7 @@ class TestSwitchActiveCCX(ModuleStoreTestCase, LoginEnrollmentTestCase): ) response = self.client.get(switch_url) self.assertEqual(response.status_code, 302) - self.assertTrue(response.get('Location', '').endswith(expected_url)) + self.assertTrue(response.get('Location', '').endswith(expected_url)) # pylint: disable=no-member # the mooc should be active self.verify_active_ccx(self.client) @@ -696,11 +715,11 @@ class TestSwitchActiveCCX(ModuleStoreTestCase, LoginEnrollmentTestCase): args=[self.course.id.to_deprecated_string(), self.ccx.id] ) # delete the ccx - self.ccx.delete() + self.ccx.delete() # pylint: disable=no-member response = self.client.get(switch_url) self.assertEqual(response.status_code, 302) - self.assertTrue(response.get('Location', '').endswith(self.target_url)) + self.assertTrue(response.get('Location', '').endswith(self.target_url)) # pylint: disable=no-member # we tried to select the ccx it doesn't exist anymore, so we are # switched back to the mooc view self.verify_active_ccx(self.client) @@ -734,6 +753,7 @@ def iter_blocks(course): Returns an iterator over all of the blocks in a course. """ def visit(block): + """ get child blocks """ yield block for child in block.get_children(): for descendant in visit(child): # wish they'd backport yield from @@ -742,12 +762,15 @@ def iter_blocks(course): def visible_children(block): + """ + Return only visible children + """ block_get_children = block.get_children - def get_children(): - def iter_children(): + def get_children(): # pylint: disable=missing-docstring + def iter_children(): # pylint: disable=missing-docstring for child in block_get_children(): - child._field_data_cache = {} + child._field_data_cache = {} # pylint: disable=protected-access if not child.visible_to_staff_only: yield child return list(iter_children()) diff --git a/lms/djangoapps/ccx/utils.py b/lms/djangoapps/ccx/utils.py index 8dca532dd4..3ebf9e4d0b 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -4,14 +4,14 @@ CCX Enrollment operations for use by Coach APIs. Does not include any access control, be sure to check access before calling. """ import logging -from courseware.courses import get_course_about_section -from courseware.courses import get_course_by_id +from courseware.courses import get_course_about_section # pylint: disable=import-error +from courseware.courses import get_course_by_id # pylint: disable=import-error from django.contrib.auth.models import User from django.conf import settings from django.core.urlresolvers import reverse from django.core.mail import send_mail -from edxmako.shortcuts import render_to_string -from microsite_configuration import microsite +from edxmako.shortcuts import render_to_string # pylint: disable=import-error +from microsite_configuration import microsite # pylint: disable=import-error from xmodule.modulestore.django import modulestore from xmodule.error_module import ErrorDescriptor @@ -44,7 +44,7 @@ class EmailEnrollmentState(object): self.in_ccx = in_ccx def __repr__(self): - return "{}(user={}, member={}, in_ccx={}".format( + return "{}(user={}, member={}, in_ccx={})".format( self.__class__.__name__, self.user, self.member, @@ -52,6 +52,7 @@ class EmailEnrollmentState(object): ) def to_dict(self): + """ return dict with membership and ccx info """ return { 'user': self.user, 'member': self.member, @@ -60,6 +61,9 @@ class EmailEnrollmentState(object): def enroll_email(ccx, student_email, auto_enroll=False, email_students=False, email_params=None): + """ + Send email to newly enrolled student + """ if email_params is None: email_params = get_email_params(ccx, True) previous_state = EmailEnrollmentState(ccx, student_email) @@ -97,6 +101,9 @@ def enroll_email(ccx, student_email, auto_enroll=False, email_students=False, em def unenroll_email(ccx, student_email, email_students=False, email_params=None): + """ + send email to unenrolled students + """ if email_params is None: email_params = get_email_params(ccx, True) previous_state = EmailEnrollmentState(ccx, student_email) @@ -112,8 +119,7 @@ def unenroll_email(ccx, student_email, email_students=False, email_params=None): send_mail_to_student(student_email, email_params) else: if CcxFutureMembership.objects.filter( - ccx=ccx, email=student_email - ).exists(): + ccx=ccx, email=student_email).exists(): CcxFutureMembership.objects.get( ccx=ccx, email=student_email ).delete() @@ -128,6 +134,9 @@ def unenroll_email(ccx, student_email, email_students=False, email_params=None): def get_email_params(ccx, auto_enroll, secure=True): + """ + get parameters for enrollment emails + """ protocol = 'https' if secure else 'http' course_id = ccx.course_id @@ -172,6 +181,9 @@ def get_email_params(ccx, auto_enroll, secure=True): def send_mail_to_student(student, param_dict): + """ + Check parameters, set text template and send email to student + """ if 'course' in param_dict: param_dict['course_name'] = param_dict['course'].display_name @@ -266,6 +278,7 @@ def get_all_ccx_for_user(user): }) return memberships + def get_ccx_membership_triplets(user, course_org_filter, org_filter_out_set): """ Get the relevant set of (CustomCourseForEdX, CcxMembership, Course) @@ -289,6 +302,6 @@ def get_ccx_membership_triplets(user, course_org_filter, org_filter_out_set): yield (ccx, membership, course) else: - log.error("User {0} enrolled in {2} course {1}".format( + log.error("User {0} enrolled in {2} course {1}".format( # pylint: disable=logging-format-interpolation user.username, ccx.course_id, "broken" if course else "non-existent" )) diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index a42018639f..339b21256f 100644 --- a/lms/djangoapps/ccx/views.py +++ b/lms/djangoapps/ccx/views.py @@ -22,22 +22,23 @@ from django.core.validators import validate_email from django.shortcuts import redirect from django.utils.translation import ugettext as _ from django.views.decorators.cache import cache_control -from django_future.csrf import ensure_csrf_cookie +from django_future.csrf import ensure_csrf_cookie # pylint: disable=import-error from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User -from courseware.courses import get_course_by_id -from courseware.field_overrides import disable_overrides -from courseware.grades import iterate_grades_for -from courseware.model_data import FieldDataCache -from courseware.module_render import get_module_for_descriptor -from edxmako.shortcuts import render_to_response -from opaque_keys.edx.keys import CourseKey -from student.roles import CourseCcxCoachRole +from courseware.courses import get_course_by_id # pylint: disable=import-error -from instructor.offline_gradecalc import student_grades -from instructor.views.api import _split_input_list -from instructor.views.tools import get_student_from_identifier +from courseware.field_overrides import disable_overrides # pylint: disable=import-error +from courseware.grades import iterate_grades_for # pylint: disable=import-error +from courseware.model_data import FieldDataCache # pylint: disable=import-error +from courseware.module_render import get_module_for_descriptor # pylint: disable=import-error +from edxmako.shortcuts import render_to_response # pylint: disable=import-error +from opaque_keys.edx.keys import CourseKey +from student.roles import CourseCcxCoachRole # pylint: disable=import-error + +from instructor.offline_gradecalc import student_grades # pylint: disable=import-error +from instructor.views.api import _split_input_list # pylint: disable=import-error +from instructor.views.tools import get_student_from_identifier # pylint: disable=import-error from .models import CustomCourseForEdX, CcxMembership from .overrides import ( @@ -50,7 +51,7 @@ from .utils import ( enroll_email, unenroll_email, ) -from ccx import ACTIVE_CCX_KEY +from ccx import ACTIVE_CCX_KEY # pylint: disable=import-error log = logging.getLogger(__name__) @@ -181,7 +182,7 @@ def save_ccx(request, course): clear_override_for_ccx(ccx, block, 'due') if not unit['hidden'] and block.graded: - graded[block.format] = graded.get(block.format, 0) + 1 + graded[block.format] = graded.get(block.format, 0) + 1 children = unit.get('children', None) if children: @@ -232,7 +233,9 @@ def set_grading_policy(request, course): def validate_date(year, month, day, hour, minute): - # avoid corrupting db if bad dates come in + """ + avoid corrupting db if bad dates come in + """ valid = True if year < 0: valid = False @@ -320,6 +323,9 @@ def get_ccx_schedule(course, ccx): @cache_control(no_cache=True, no_store=True, must_revalidate=True) @coach_dashboard def ccx_schedule(request, course): + """ + get json representation of ccx schedule + """ ccx = get_ccx_for_coach(course, request.user) schedule = get_ccx_schedule(course, ccx) json_schedule = json.dumps(schedule, indent=4) @@ -512,7 +518,7 @@ def switch_active_ccx(request, course_id, ccx_id=None): requested_ccx = CustomCourseForEdX.objects.get(pk=ccx_id) assert unicode(requested_ccx.course_id) == course_id if not CcxMembership.objects.filter( - ccx=requested_ccx, student=request.user, active=True + ccx=requested_ccx, student=request.user, active=True ).exists(): ccx_id = None except CustomCourseForEdX.DoesNotExist: diff --git a/lms/djangoapps/courseware/field_overrides.py b/lms/djangoapps/courseware/field_overrides.py index e7b0c8e121..7d58836024 100644 --- a/lms/djangoapps/courseware/field_overrides.py +++ b/lms/djangoapps/courseware/field_overrides.py @@ -20,7 +20,6 @@ from abc import ABCMeta, abstractmethod from contextlib import contextmanager from django.conf import settings from xblock.field_data import FieldData -from xmodule.modulestore.django import modulestore from xmodule.modulestore.inheritance import InheritanceMixin diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index a17ff997ed..a1444be4c8 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -27,7 +27,6 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey - log = logging.getLogger("edx.courseware") diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 0b5e650677..43832bfaa7 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -20,7 +20,7 @@ from django.dispatch import receiver from model_utils.models import TimeStampedModel -from xmodule_django.models import CourseKeyField, LocationKeyField, BlockTypeKeyField +from xmodule_django.models import CourseKeyField, LocationKeyField, BlockTypeKeyField # pylint: disable=import-error class StudentModule(models.Model): @@ -37,8 +37,7 @@ class StudentModule(models.Model): ('course', 'course'), ('chapter', 'Section'), ('sequential', 'Subsection'), - ('library_content', 'Library Content'), - ) + ('library_content', 'Library Content')) ## These three are the key for the object module_type = models.CharField(max_length=32, choices=MODULE_TYPES, default='problem', db_index=True) @@ -88,7 +87,7 @@ class StudentModule(models.Model): return 'StudentModule<%r>' % ({ 'course_id': self.course_id, 'module_type': self.module_type, - 'student': self.student.username, + 'student': self.student.username, # pylint: disable=no-member 'module_state_key': self.module_state_key, 'state': str(self.state)[:20], },) @@ -244,7 +243,7 @@ class StudentFieldOverride(TimeStampedModel): location = LocationKeyField(max_length=255, db_index=True) student = models.ForeignKey(User, db_index=True) - class Meta: # pylint: disable=missing-docstring + class Meta(object): # pylint: disable=missing-docstring unique_together = (('course_id', 'field', 'location', 'student'),) field = models.CharField(max_length=255) diff --git a/lms/djangoapps/courseware/student_field_overrides.py b/lms/djangoapps/courseware/student_field_overrides.py index 9b9134b2aa..afbc10ac79 100644 --- a/lms/djangoapps/courseware/student_field_overrides.py +++ b/lms/djangoapps/courseware/student_field_overrides.py @@ -25,11 +25,11 @@ def get_override_for_user(user, block, name, default=None): overridden for the given user, returns `default`. """ if not hasattr(block, '_student_overrides'): - block._student_overrides = {} - overrides = block._student_overrides.get(user.id) + block._student_overrides = {} # pylint: disable=protected-access + overrides = block._student_overrides.get(user.id) # pylint: disable=protected-access if overrides is None: overrides = _get_overrides_for_user(user, block) - block._student_overrides[user.id] = overrides + block._student_overrides[user.id] = overrides # pylint: disable=protected-access return overrides.get(name, default) diff --git a/lms/djangoapps/courseware/tests/test_field_overrides.py b/lms/djangoapps/courseware/tests/test_field_overrides.py index 4d0af91970..76cd636c91 100644 --- a/lms/djangoapps/courseware/tests/test_field_overrides.py +++ b/lms/djangoapps/courseware/tests/test_field_overrides.py @@ -26,6 +26,7 @@ class OverrideFieldDataTests(TestCase): """ def setUp(self): + super(OverrideFieldDataTests, self).setUp() OverrideFieldData.provider_classes = None def tearDown(self): diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 1c43499bc7..628ed638ee 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -720,7 +720,7 @@ def modify_access(request, course_id): rolename = request.GET.get('rolename') action = request.GET.get('action') - if not rolename in ROLES: + if rolename not in ROLES: error = strip_tags("unknown rolename '{}'".format(rolename)) log.error(error) return HttpResponseBadRequest(error) @@ -783,7 +783,7 @@ def list_course_role_members(request, course_id): rolename = request.GET.get('rolename') - if not rolename in ROLES: + if rolename not in ROLES: return HttpResponseBadRequest() def extract_user_info(user):