diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 94fe36eed9..38b87c0925 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -51,10 +51,10 @@ from ratelimitbackend import admin import analytics -unenroll_done = Signal(providing_args=["course_enrollment"]) +UNENROLL_DONE = Signal(providing_args=["course_enrollment"]) log = logging.getLogger(__name__) AUDIT_LOG = logging.getLogger("audit") -SessionStore = import_module(settings.SESSION_ENGINE).SessionStore +SessionStore = import_module(settings.SESSION_ENGINE).SessionStore # pylint: disable=invalid-name class AnonymousUserId(models.Model): @@ -133,7 +133,7 @@ def anonymous_id_for_user(user, course_id, save=True): return digest -def user_by_anonymous_id(id): +def user_by_anonymous_id(uid): """ Return user by anonymous_user_id using AnonymousUserId lookup table. @@ -142,11 +142,11 @@ def user_by_anonymous_id(id): because this function will be used inside xmodule w/o django access. """ - if id is None: + if uid is None: return None try: - return User.objects.get(anonymoususerid__anonymous_user_id=id) + return User.objects.get(anonymoususerid__anonymous_user_id=uid) except ObjectDoesNotExist: return None @@ -192,7 +192,7 @@ class UserProfile(models.Model): MITx fall prototype. """ - class Meta: + class Meta: # pylint: disable=missing-docstring db_table = "auth_userprofile" # CRITICAL TODO/SECURITY @@ -250,7 +250,7 @@ class UserProfile(models.Model): goals = models.TextField(blank=True, null=True) allow_certificate = models.BooleanField(default=1) - def get_meta(self): + def get_meta(self): # pylint: disable=missing-docstring js_str = self.meta if not js_str: js_str = dict() @@ -259,8 +259,8 @@ class UserProfile(models.Model): return js_str - def set_meta(self, js): - self.meta = json.dumps(js) + def set_meta(self, meta_json): # pylint: disable=missing-docstring + self.meta = json.dumps(meta_json) def set_login_session(self, session_id=None): """ @@ -487,8 +487,8 @@ class PasswordHistory(models.Model): return True if user.is_staff and cls.is_staff_password_reuse_restricted(): - min_diff_passwords_required = \ - settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE'] + min_diff_passwords_required = \ + settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE'] elif cls.is_student_password_reuse_restricted(): min_diff_passwords_required = \ settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE'] @@ -723,7 +723,7 @@ class CourseEnrollment(models.Model): ) else: - unenroll_done.send(sender=None, course_enrollment=self) + UNENROLL_DONE.send(sender=None, course_enrollment=self) self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED) @@ -961,12 +961,12 @@ class CourseEnrollment(models.Model): # Unfortunately, Django's "group by"-style queries look super-awkward query = use_read_replica_if_available(cls.objects.filter(course_id=course_id, is_active=True).values('mode').order_by().annotate(Count('mode'))) total = 0 - d = defaultdict(int) + enroll_dict = defaultdict(int) for item in query: - d[item['mode']] = item['mode__count'] + enroll_dict[item['mode']] = item['mode__count'] total += item['mode__count'] - d['total'] = total - return d + enroll_dict['total'] = total + return enroll_dict def is_paid_course(self): """ @@ -999,7 +999,7 @@ class CourseEnrollment(models.Model): a verified certificate and the deadline for refunds has not yet passed. """ # In order to support manual refunds past the deadline, set can_refund on this object. - # On unenrolling, the "unenroll_done" signal calls CertificateItem.refund_cert_callback(), + # On unenrolling, the "UNENROLL_DONE" signal calls CertificateItem.refund_cert_callback(), # which calls this method to determine whether to refund the order. # This can't be set directly because refunds currently happen as a side-effect of unenrolling. # (side-effects are bad) @@ -1031,7 +1031,7 @@ class CourseEnrollmentAllowed(models.Model): created = models.DateTimeField(auto_now_add=True, null=True, db_index=True) - class Meta: + class Meta: # pylint: disable=missing-docstring unique_together = (('email', 'course_id'),) def __unicode__(self): @@ -1055,7 +1055,7 @@ class CourseAccessRole(models.Model): course_id = CourseKeyField(max_length=255, db_index=True, blank=True) role = models.CharField(max_length=64, db_index=True) - class Meta: + class Meta: # pylint: disable=missing-docstring unique_together = ('user', 'org', 'course_id', 'role') @property @@ -1071,7 +1071,7 @@ class CourseAccessRole(models.Model): Overriding eq b/c the django impl relies on the primary key which requires fetch. sometimes we just want to compare roles w/o doing another fetch. """ - return type(self) == type(other) and self._key == other._key + return type(self) == type(other) and self._key == other._key # pylint: disable=protected-access def __hash__(self): return hash(self._key) @@ -1080,7 +1080,7 @@ class CourseAccessRole(models.Model): """ Lexigraphic sort """ - return self._key < other._key + return self._key < other._key # pylint: disable=protected-access def __unicode__(self): return "[CourseAccessRole] user: {} role: {} org: {} course: {}".format(self.user.username, self.role, self.org, self.course_id) @@ -1107,32 +1107,32 @@ def get_user_by_username_or_email(username_or_email): def get_user(email): - u = User.objects.get(email=email) - up = UserProfile.objects.get(user=u) - return u, up + user = User.objects.get(email=email) + u_prof = UserProfile.objects.get(user=user) + return user, u_prof def user_info(email): - u, up = get_user(email) - print "User id", u.id - print "Username", u.username - print "E-mail", u.email - print "Name", up.name - print "Location", up.location - print "Language", up.language - return u, up + user, u_prof = get_user(email) + print "User id", user.id + print "Username", user.username + print "E-mail", user.email + print "Name", u_prof.name + print "Location", u_prof.location + print "Language", u_prof.language + return user, u_prof def change_email(old_email, new_email): - u = User.objects.get(email=old_email) - u.email = new_email - u.save() + user = User.objects.get(email=old_email) + user.email = new_email + user.save() def change_name(email, new_name): - u, up = get_user(email) - up.name = new_name - up.save() + _user, u_prof = get_user(email) + u_prof.name = new_name + u_prof.save() def user_count(): @@ -1163,10 +1163,12 @@ def remove_user_from_group(user, group): utg.users.remove(User.objects.get(username=user)) utg.save() -default_groups = {'email_future_courses': 'Receive e-mails about future MITx courses', - 'email_helpers': 'Receive e-mails about how to help with MITx', - 'mitx_unenroll': 'Fully unenrolled -- no further communications', - '6002x_unenroll': 'Took and dropped 6002x'} +DEFAULT_GROUPS = { + 'email_future_courses': 'Receive e-mails about future MITx courses', + 'email_helpers': 'Receive e-mails about how to help with MITx', + 'mitx_unenroll': 'Fully unenrolled -- no further communications', + '6002x_unenroll': 'Took and dropped 6002x' +} def add_user_to_default_group(user, group): @@ -1175,7 +1177,7 @@ def add_user_to_default_group(user, group): except UserTestGroup.DoesNotExist: utg = UserTestGroup() utg.name = group - utg.description = default_groups[group] + utg.description = DEFAULT_GROUPS[group] utg.save() utg.users.add(User.objects.get(username=user)) utg.save() @@ -1188,11 +1190,12 @@ def create_comments_service_user(user): try: cc_user = cc.User.from_django_user(user) cc_user.save() - except Exception as e: - log = logging.getLogger("edx.discussion") + except Exception: # pylint: disable=broad-except + log = logging.getLogger("edx.discussion") # pylint: disable=redefined-outer-name log.error( "Could not create comments service user with id {}".format(user.id), - exc_info=True) + exc_info=True + ) # Define login and logout handlers here in the models file, instead of the views file, # so that they are more likely to be loaded when a Studio user brings up the Studio admin @@ -1201,7 +1204,7 @@ def create_comments_service_user(user): @receiver(user_logged_in) -def log_successful_login(sender, request, user, **kwargs): +def log_successful_login(sender, request, user, **kwargs): # pylint: disable=unused-argument """Handler to log when logins have occurred successfully.""" if settings.FEATURES['SQUELCH_PII_IN_LOGS']: AUDIT_LOG.info(u"Login success - user.id: {0}".format(user.id)) @@ -1210,7 +1213,7 @@ def log_successful_login(sender, request, user, **kwargs): @receiver(user_logged_out) -def log_successful_logout(sender, request, user, **kwargs): +def log_successful_logout(sender, request, user, **kwargs): # pylint: disable=unused-argument """Handler to log when logouts have occurred successfully.""" if settings.FEATURES['SQUELCH_PII_IN_LOGS']: AUDIT_LOG.info(u"Logout - user.id: {0}".format(request.user.id)) @@ -1220,7 +1223,7 @@ def log_successful_logout(sender, request, user, **kwargs): @receiver(user_logged_in) @receiver(user_logged_out) -def enforce_single_login(sender, request, user, signal, **kwargs): +def enforce_single_login(sender, request, user, signal, **kwargs): # pylint: disable=unused-argument """ Sets the current session id in the user profile, to prevent concurrent logins. diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 49df99d4b5..34e216dc1b 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -46,8 +46,7 @@ from student.models import ( Registration, UserProfile, PendingNameChange, PendingEmailChange, CourseEnrollment, unique_id_for_user, CourseEnrollmentAllowed, UserStanding, LoginFailures, - create_comments_service_user, PasswordHistory, UserSignupSource, - anonymous_id_for_user + create_comments_service_user, PasswordHistory, UserSignupSource ) from student.forms import PasswordResetFormNoActive @@ -103,27 +102,29 @@ AUDIT_LOG = logging.getLogger("audit") ReverifyInfo = namedtuple('ReverifyInfo', 'course_id course_name course_number date status display') # pylint: disable=C0103 + def csrf_token(context): """A csrf token that can be included in a form.""" - csrf_token = context.get('csrf_token', '') - if csrf_token == 'NOTPROVIDED': + token = context.get('csrf_token', '') + if token == 'NOTPROVIDED': return '' return (u'
' % (csrf_token)) + ' name="csrfmiddlewaretoken" value="%s" />' % (token)) # NOTE: This view is not linked to directly--it is called from # branding/views.py:index(), which is cached for anonymous users. # This means that it should always return the same thing for anon # users. (in particular, no switching based on query params allowed) -def index(request, extra_context={}, user=AnonymousUser()): +def index(request, extra_context=None, user=AnonymousUser()): """ Render the edX main page. extra_context is used to allow immediate display of certain modal windows, eg signup, as used by external_auth. """ - + if extra_context is None: + extra_context = {} # The course selection work is done in courseware.courses. domain = settings.FEATURES.get('FORCE_UNIVERSITY_DOMAIN') # normally False # do explicit check, because domain=None is valid @@ -259,8 +260,8 @@ def get_course_enrollment_pairs(user, course_org_filter, org_filter_out_set): yield (course, enrollment) else: log.error("User {0} enrolled in {2} course {1}".format( - user.username, enrollment.course_id, "broken" if course else "non-existent" - )) + user.username, enrollment.course_id, "broken" if course else "non-existent" + )) def _cert_info(user, course, cert_status): @@ -294,18 +295,20 @@ def _cert_info(user, course, cert_status): status = template_state.get(cert_status['status'], default_status) - d = {'status': status, - 'show_download_url': status == 'ready', - 'show_disabled_download_button': status == 'generating', - 'mode': cert_status.get('mode', None)} + status_dict = { + 'status': status, + 'show_download_url': status == 'ready', + 'show_disabled_download_button': status == 'generating', + 'mode': cert_status.get('mode', None) + } if (status in ('generating', 'ready', 'notpassing', 'restricted') and course.end_of_course_survey_url is not None): - d.update({ + status_dict.update({ 'show_survey_button': True, 'survey_url': process_survey_link(course.end_of_course_survey_url, user)}) else: - d['show_survey_button'] = False + status_dict['show_survey_button'] = False if status == 'ready': if 'download_url' not in cert_status: @@ -313,7 +316,7 @@ def _cert_info(user, course, cert_status): user.username, course.id) return default_info else: - d['download_url'] = cert_status['download_url'] + status_dict['download_url'] = cert_status['download_url'] if status in ('generating', 'ready', 'notpassing', 'restricted'): if 'grade' not in cert_status: @@ -322,9 +325,9 @@ def _cert_info(user, course, cert_status): # We can add a log.warning here once we think it shouldn't happen. return default_info else: - d['grade'] = cert_status['grade'] + status_dict['grade'] = cert_status['grade'] - return d + return status_dict @ensure_csrf_cookie @@ -446,6 +449,7 @@ def is_course_blocked(request, redeemed_registration_codes, course_key): return blocked + @login_required @ensure_csrf_cookie def dashboard(request): @@ -511,7 +515,6 @@ def dashboard(request): block_courses = frozenset(course.id for course, enrollment in course_enrollment_pairs if is_course_blocked(request, CourseRegistrationCode.objects.filter(course_id=course.id, registrationcoderedemption__redeemed_by=request.user), course.id)) - enrolled_courses_either_paid = frozenset(course.id for course, _enrollment in course_enrollment_pairs if _enrollment.is_paid_course()) # get info w.r.t ExternalAuthMap @@ -608,8 +611,8 @@ def try_change_enrollment(request): # will return redirect_urls. if enrollment_response.status_code == 200 and enrollment_response.content != '': return enrollment_response.content - except Exception, e: - log.exception("Exception automatically enrolling after login: {0}".format(str(e))) + except Exception as exc: # pylint: disable=broad-except + log.exception("Exception automatically enrolling after login: %s", exc) @require_POST @@ -771,7 +774,6 @@ def change_enrollment(request, auto_register=False): return HttpResponse() - elif action == "add_to_cart": # Pass the request handling to shoppingcart.views # The view in shoppingcart.views performs error handling and logs different errors. But this elif clause @@ -885,12 +887,17 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un username=username, backend_name=backend_name)) return HttpResponseBadRequest( _("You've successfully logged into your {provider_name} account, but this account isn't linked with an {platform_name} account yet.").format( - platform_name=settings.PLATFORM_NAME, provider_name=requested_provider.NAME) - + "

" + _("Use your {platform_name} username and password to log into {platform_name} below, " + platform_name=settings.PLATFORM_NAME, provider_name=requested_provider.NAME + ) + + "

" + + _("Use your {platform_name} username and password to log into {platform_name} below, " "and then link your {platform_name} account with {provider_name} from your dashboard.").format( - platform_name=settings.PLATFORM_NAME, provider_name=requested_provider.NAME) - + "

" + _("If you don't have an {platform_name} account yet, click Register Now at the top of the page.").format( - platform_name=settings.PLATFORM_NAME), + platform_name=settings.PLATFORM_NAME, provider_name=requested_provider.NAME + ) + + "

" + + _("If you don't have an {platform_name} account yet, click Register Now at the top of the page.").format( + platform_name=settings.PLATFORM_NAME + ), content_type="text/plain", status=401 ) @@ -1002,7 +1009,7 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un }, context={ 'Google Analytics': { - 'clientId': tracking_context.get('client_id') + 'clientId': tracking_context.get('client_id') } } ) @@ -1018,10 +1025,10 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un log.debug("Setting user session to never expire") else: request.session.set_expiry(0) - except Exception as e: + except Exception as exc: # pylint: disable=broad-except AUDIT_LOG.critical("Login failed - Could not create session. Is memcached running?") log.critical("Login failed - Could not create session. Is memcached running?") - log.exception(e) + log.exception(exc) raise redirect_url = try_change_enrollment(request) @@ -1170,14 +1177,14 @@ def disable_account_ajax(request): def change_setting(request): """JSON call to change a profile setting: Right now, location""" # TODO (vshnayder): location is no longer used - up = UserProfile.objects.get(user=request.user) # request.user.profile_cache + u_prof = UserProfile.objects.get(user=request.user) # request.user.profile_cache if 'location' in request.POST: - up.location = request.POST['location'] - up.save() + u_prof.location = request.POST['location'] + u_prof.save() return JsonResponse({ "success": True, - "location": up.location, + "location": u_prof.location, }) @@ -1226,12 +1233,12 @@ def _do_create_account(post_vars, extended_profile=None): raise AccountValidationError( _("An account with the Public Username '{username}' already exists.").format(username=post_vars['username']), field="username" - ) + ) elif len(User.objects.filter(email=post_vars['email'])) > 0: raise AccountValidationError( _("An account with the Email '{email}' already exists.").format(email=post_vars['email']), field="email" - ) + ) else: raise @@ -1263,7 +1270,7 @@ def _do_create_account(post_vars, extended_profile=None): profile.year_of_birth = None try: profile.save() - except Exception: + except Exception: # pylint: disable=broad-except log.exception("UserProfile creation failed for user {id}.".format(id=user.id)) raise @@ -1295,8 +1302,8 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many # if doing signup for an external authorization, then get email, password, name from the eamap # don't use the ones from the form, since the user could have hacked those # unless originally we didn't get a valid email or name from the external auth - DoExternalAuth = 'ExternalAuthMap' in request.session - if DoExternalAuth: + do_external_auth = 'ExternalAuthMap' in request.session + if do_external_auth: eamap = request.session['ExternalAuthMap'] try: validate_email(eamap.external_email) @@ -1313,15 +1320,15 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many log.debug(u'In create_account with external_auth: user = %s, email=%s', name, email) # Confirm we have a properly formed request - for a in ['username', 'email', 'password', 'name']: - if a not in post_vars: - js['value'] = _("Error (401 {field}). E-mail us.").format(field=a) - js['field'] = a + for req_field in ['username', 'email', 'password', 'name']: + if req_field not in post_vars: + js['value'] = _("Error (401 {field}). E-mail us.").format(field=req_field) + js['field'] = req_field return JsonResponse(js, status=400) if extra_fields.get('honor_code', 'required') == 'required' and \ post_vars.get('honor_code', 'false') != u'true': - js['value'] = _("To enroll, you must follow the honor code.").format(field=a) + js['value'] = _("To enroll, you must follow the honor code.") js['field'] = 'honor_code' return JsonResponse(js, status=400) @@ -1329,7 +1336,7 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many tos_required = ( not settings.FEATURES.get("AUTH_USE_SHIB") or not settings.FEATURES.get("SHIB_DISABLE_TOS") or - not DoExternalAuth or + not do_external_auth or not eamap.external_domain.startswith( external_auth.views.SHIBBOLETH_DOMAIN_PREFIX ) @@ -1337,7 +1344,7 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many if tos_required: if post_vars.get('terms_of_service', 'false') != u'true': - js['value'] = _("You must accept the terms of service.").format(field=a) + js['value'] = _("You must accept the terms of service.") js['field'] = 'terms_of_service' return JsonResponse(js, status=400) @@ -1390,8 +1397,8 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many if field_name in ('email', 'username') and len(post_vars[field_name]) > max_length: error_str = { - 'username': _('Username cannot be more than {0} characters long').format(max_length), - 'email': _('Email cannot be more than {0} characters long').format(max_length) + 'username': _('Username cannot be more than {num} characters long').format(num=max_length), + 'email': _('Email cannot be more than {num} characters long').format(num=max_length) } js['value'] = error_str[field_name] js['field'] = field_name @@ -1400,20 +1407,20 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many try: validate_email(post_vars['email']) except ValidationError: - js['value'] = _("Valid e-mail is required.").format(field=a) + js['value'] = _("Valid e-mail is required.") js['field'] = 'email' return JsonResponse(js, status=400) try: validate_slug(post_vars['username']) except ValidationError: - js['value'] = _("Username should only consist of A-Z and 0-9, with no spaces.").format(field=a) + js['value'] = _("Username should only consist of A-Z and 0-9, with no spaces.") js['field'] = 'username' return JsonResponse(js, status=400) # enforce password complexity as an optional feature # but not if we're doing ext auth b/c those pws never get used and are auto-generated so might not pass validation - if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False) and not DoExternalAuth: + if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False) and not do_external_auth: try: password = post_vars['password'] @@ -1449,8 +1456,8 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many try: with transaction.commit_on_success(): ret = _do_create_account(post_vars, extended_profile) - except AccountValidationError as e: - return JsonResponse({'success': False, 'value': e.message, 'field': e.field}, status=400) + except AccountValidationError as exc: + return JsonResponse({'success': False, 'value': exc.message, 'field': exc.field}, status=400) (user, profile, registration) = ret @@ -1469,14 +1476,14 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many registration_course_id = request.session.get('registration_course_id') analytics.track( user.id, - "edx.bi.user.account.registered", + "edx.bi.user.account.registered", { "category": "conversion", "label": registration_course_id }, context={ 'Google Analytics': { - 'clientId': tracking_context.get('client_id') + 'clientId': tracking_context.get('client_id') } } ) @@ -1520,17 +1527,17 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many # Immediately after a user creates an account, we log them in. They are only # logged in until they close the browser. They can't log in again until they click # the activation link from the email. - login_user = authenticate(username=post_vars['username'], password=post_vars['password']) - login(request, login_user) + new_user = authenticate(username=post_vars['username'], password=post_vars['password']) + login(request, new_user) request.session.set_expiry(0) # TODO: there is no error checking here to see that the user actually logged in successfully, # and is not yet an active user. - if login_user is not None: - AUDIT_LOG.info(u"Login success on new account creation - {0}".format(login_user.username)) + if new_user is not None: + AUDIT_LOG.info(u"Login success on new account creation - {0}".format(new_user.username)) - if DoExternalAuth: - eamap.user = login_user + if do_external_auth: + eamap.user = new_user eamap.dtsignup = datetime.datetime.now(UTC) eamap.save() AUDIT_LOG.info("User registered with external_auth %s", post_vars['username']) @@ -1538,9 +1545,9 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many if settings.FEATURES.get('BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'): log.info('bypassing activation email') - login_user.is_active = True - login_user.save() - AUDIT_LOG.info(u"Login activated on extauth account - {0} ({1})".format(login_user.username, login_user.email)) + new_user.is_active = True + new_user.save() + AUDIT_LOG.info(u"Login activated on extauth account - {0} ({1})".format(new_user.username, new_user.email)) dog_stats_api.increment("common.student.account_created") redirect_url = try_change_enrollment(request) @@ -1623,7 +1630,7 @@ def auto_auth(request): # If successful, this will return a tuple containing # the new user object. try: - user, profile, reg = _do_create_account(post_data) + user, _profile, reg = _do_create_account(post_data) except AccountValidationError: # Attempt to retrieve the existing user. user = User.objects.get(username=username) @@ -1669,16 +1676,16 @@ def auto_auth(request): @ensure_csrf_cookie def activate_account(request, key): """When link in activation e-mail is clicked""" - r = Registration.objects.filter(activation_key=key) - if len(r) == 1: + regs = Registration.objects.filter(activation_key=key) + if len(regs) == 1: user_logged_in = request.user.is_authenticated() already_active = True - if not r[0].user.is_active: - r[0].activate() + if not regs[0].user.is_active: + regs[0].activate() already_active = False # Enroll student in any pending courses he/she may have if auto_enroll flag is set - student = User.objects.filter(id=r[0].user_id) + student = User.objects.filter(id=regs[0].user_id) if student: ceas = CourseEnrollmentAllowed.objects.filter(email=student[0].email) for cea in ceas: @@ -1693,7 +1700,7 @@ def activate_account(request, key): } ) return resp - if len(r) == 0: + if len(regs) == 0: return render_to_response( "registration/activation_invalid.html", {'csrf': csrf(request)['csrf_token']} @@ -1928,8 +1935,9 @@ def change_email_request(request): @ensure_csrf_cookie @transaction.commit_manually -def confirm_email_change(request, key): - """ User requested a new e-mail. This is called when the activation +def confirm_email_change(request, key): # pylint: disable=unused-argument + """ + User requested a new e-mail. This is called when the activation link is clicked. We confirm with the old e-mail, and update """ try: @@ -1954,17 +1962,17 @@ def confirm_email_change(request, key): subject = render_to_string('emails/email_change_subject.txt', address_context) subject = ''.join(subject.splitlines()) message = render_to_string('emails/confirm_email_change.txt', address_context) - up = UserProfile.objects.get(user=user) - meta = up.get_meta() + u_prof = UserProfile.objects.get(user=user) + meta = u_prof.get_meta() if 'old_emails' not in meta: meta['old_emails'] = [] meta['old_emails'].append([user.email, datetime.datetime.now(UTC).isoformat()]) - up.set_meta(meta) - up.save() + u_prof.set_meta(meta) + u_prof.save() # Send it to the old email... try: user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL) - except Exception: + except Exception: # pylint: disable=broad-except log.warning('Unable to send confirmation email to old address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': user.email}) transaction.rollback() @@ -1976,7 +1984,7 @@ def confirm_email_change(request, key): # And send it to the new email... try: user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL) - except Exception: + except Exception: # pylint: disable=broad-except log.warning('Unable to send confirmation email to new address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': pec.new_email}) transaction.rollback() @@ -1985,7 +1993,7 @@ def confirm_email_change(request, key): response = render_to_response("email_change_successful.html", address_context) transaction.commit() return response - except Exception: + except Exception: # pylint: disable=broad-except # If we get an unexpected exception, be sure to rollback the transaction transaction.rollback() raise @@ -2058,27 +2066,31 @@ def reject_name_change(request): return JsonResponse({"success": True}) -def accept_name_change_by_id(id): +def accept_name_change_by_id(uid): + """ + Accepts the pending name change request for the user represented + by user id `uid`. + """ try: - pnc = PendingNameChange.objects.get(id=id) + pnc = PendingNameChange.objects.get(id=uid) except PendingNameChange.DoesNotExist: return JsonResponse({ "success": False, "error": _('Invalid ID'), }) # TODO: this should be status code 400 # pylint: disable=fixme - u = pnc.user - up = UserProfile.objects.get(user=u) + user = pnc.user + u_prof = UserProfile.objects.get(user=user) # Save old name - meta = up.get_meta() + meta = u_prof.get_meta() if 'old_names' not in meta: meta['old_names'] = [] - meta['old_names'].append([up.name, pnc.rationale, datetime.datetime.now(UTC).isoformat()]) - up.set_meta(meta) + meta['old_names'].append([u_prof.name, pnc.rationale, datetime.datetime.now(UTC).isoformat()]) + u_prof.set_meta(meta) - up.name = pnc.new_name - up.save() + u_prof.name = pnc.new_name + u_prof.save() pnc.delete() return JsonResponse({"success": True}) diff --git a/common/lib/capa/capa/tests/test_html_render.py b/common/lib/capa/capa/tests/test_html_render.py index 7bdc477247..f1b5249338 100644 --- a/common/lib/capa/capa/tests/test_html_render.py +++ b/common/lib/capa/capa/tests/test_html_render.py @@ -26,8 +26,9 @@ class CapaHtmlRenderTest(unittest.TestCase): problem = new_loncapa_problem(xml_str) # Render the HTML - rendered_html = etree.XML(problem.get_html()) + etree.XML(problem.get_html()) # expect that we made it here without blowing up + self.assertTrue(True) def test_include_html(self): # Create a test file to include @@ -123,17 +124,17 @@ class CapaHtmlRenderTest(unittest.TestCase): # Render the HTML rendered_html = etree.XML(problem.get_html()) - # expect the javascript is still present in the rendered html self.assertTrue("" in etree.tostring(rendered_html)) - def test_render_response_xml(self): # Generate some XML for a string response - kwargs = {'question_text': "Test question", - 'explanation_text': "Test explanation", - 'answer': 'Test answer', - 'hints': [('test prompt', 'test_hint', 'test hint text')]} + kwargs = { + 'question_text': "Test question", + 'explanation_text': "Test explanation", + 'answer': 'Test answer', + 'hints': [('test prompt', 'test_hint', 'test hint text')] + } xml_str = StringResponseXMLFactory().build_xml(**kwargs) # Mock out the template renderer @@ -186,18 +187,21 @@ class CapaHtmlRenderTest(unittest.TestCase): expected_solution_context = {'id': '1_solution_1'} - expected_calls = [mock.call('textline.html', expected_textline_context), - mock.call('solutionspan.html', expected_solution_context), - mock.call('textline.html', expected_textline_context), - mock.call('solutionspan.html', expected_solution_context)] - - self.assertEqual(the_system.render_template.call_args_list, - expected_calls) + expected_calls = [ + mock.call('textline.html', expected_textline_context), + mock.call('solutionspan.html', expected_solution_context), + mock.call('textline.html', expected_textline_context), + mock.call('solutionspan.html', expected_solution_context) + ] + self.assertEqual( + the_system.render_template.call_args_list, + expected_calls + ) def test_render_response_with_overall_msg(self): # CustomResponse script that sets an overall_message - script=textwrap.dedent(""" + script = textwrap.dedent(""" def check_func(*args): msg = '

Test message 1

Test message 2

' return {'overall_message': msg, @@ -205,19 +209,18 @@ class CapaHtmlRenderTest(unittest.TestCase): """) # Generate some XML for a CustomResponse - kwargs = {'script':script, 'cfn': 'check_func'} + kwargs = {'script': script, 'cfn': 'check_func'} xml_str = CustomResponseXMLFactory().build_xml(**kwargs) # Create the problem and render the html problem = new_loncapa_problem(xml_str) # Grade the problem - correctmap = problem.grade_answers({'1_2_1': 'test'}) + problem.grade_answers({'1_2_1': 'test'}) # Render the html rendered_html = etree.XML(problem.get_html()) - # Expect that there is a
within the response
# with css class response_message msg_div_element = rendered_html.find(".//div[@class='response_message']") @@ -232,7 +235,6 @@ class CapaHtmlRenderTest(unittest.TestCase): self.assertEqual(msg_p_elements[1].tag, "p") self.assertEqual(msg_p_elements[1].text, "Test message 2") - def test_substitute_python_vars(self): # Generate some XML with Python variables defined in a script # and used later as attributes diff --git a/common/lib/i18n/tests/test_extract_and_generate.py b/common/lib/i18n/tests/test_extract_and_generate.py index 6101101514..f5791e4b17 100644 --- a/common/lib/i18n/tests/test_extract_and_generate.py +++ b/common/lib/i18n/tests/test_extract_and_generate.py @@ -1,7 +1,10 @@ +""" +This test tests that i18n extraction (`paver i18n_extract -v`) works properly. +""" from datetime import datetime, timedelta import os import sys -import string +import string # pylint: disable=deprecated-module import random import re @@ -65,12 +68,14 @@ class TestGenerate(TestCase): generate.main(verbosity=0, strict=False) for locale in CONFIGURATION.translated_locales: for filename in ('django', 'djangojs'): - mofile = filename+'.mo' + mofile = filename + '.mo' path = os.path.join(CONFIGURATION.get_messages_dir(locale), mofile) exists = os.path.exists(path) self.assertTrue(exists, msg='Missing file in locale %s: %s' % (locale, mofile)) - self.assertTrue(datetime.fromtimestamp(os.path.getmtime(path), UTC) >= self.start_time, - msg='File not recently modified: %s' % path) + self.assertTrue( + datetime.fromtimestamp(os.path.getmtime(path), UTC) >= self.start_time, + msg='File not recently modified: %s' % path + ) # Segmenting means that the merge headers don't work they way they # used to, so don't make this check for now. I'm not sure if we'll # get the merge header back eventually, or delete this code eventually. @@ -88,12 +93,14 @@ class TestGenerate(TestCase): """ path = os.path.join(CONFIGURATION.get_messages_dir(locale), 'django.po') - po = pofile(path) + pof = pofile(path) pattern = re.compile('^#-#-#-#-#', re.M) - match = pattern.findall(po.header) - self.assertEqual(len(match), 3, - msg="Found %s (should be 3) merge comments in the header for %s" % \ - (len(match), path)) + match = pattern.findall(pof.header) + self.assertEqual( + len(match), + 3, + msg="Found %s (should be 3) merge comments in the header for %s" % (len(match), path) + ) def random_name(size=6): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 2a5b4617ad..8208ef588c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -134,8 +134,6 @@ def xml_store_config(data_dir): return store - - class ModuleStoreTestCase(TestCase): """ Subclass for any test case that uses a ModuleStore. @@ -282,35 +280,39 @@ class ModuleStoreTestCase(TestCase): # Call superclass implementation super(ModuleStoreTestCase, self)._post_teardown() - def create_sample_course(self, org, course, run, block_info_tree=default_block_info_tree, course_fields=None): + def create_sample_course(self, org, course, run, block_info_tree=None, course_fields=None): """ create a course in the default modulestore from the collection of BlockInfo records defining the course tree Returns: course_loc: the CourseKey for the created course """ + if block_info_tree is None: + block_info_tree = default_block_info_tree + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, None): # with self.store.bulk_operations(self.store.make_course_key(org, course, run)): - course = self.store.create_course(org, course, run, self.user.id, fields=course_fields) - self.course_loc = course.location + course = self.store.create_course(org, course, run, self.user.id, fields=course_fields) + self.course_loc = course.location # pylint: disable=attribute-defined-outside-init - def create_sub_tree(parent_loc, block_info): - block = self.store.create_child( - self.user.id, - # TODO remove version_agnostic() when we impl the single transaction - parent_loc.version_agnostic(), - block_info.category, block_id=block_info.block_id, - fields=block_info.fields, - ) - for tree in block_info.sub_tree: - create_sub_tree(block.location, tree) - setattr(self, block_info.block_id, block.location.version_agnostic()) + def create_sub_tree(parent_loc, block_info): + """Recursively creates a sub_tree on this parent_loc with this block.""" + block = self.store.create_child( + self.user.id, + # TODO remove version_agnostic() when we impl the single transaction + parent_loc.version_agnostic(), + block_info.category, block_id=block_info.block_id, + fields=block_info.fields, + ) + for tree in block_info.sub_tree: + create_sub_tree(block.location, tree) + setattr(self, block_info.block_id, block.location.version_agnostic()) - for tree in block_info_tree: - create_sub_tree(self.course_loc, tree) + for tree in block_info_tree: + create_sub_tree(self.course_loc, tree) - # remove version_agnostic when bulk write works - self.store.publish(self.course_loc.version_agnostic(), self.user.id) + # remove version_agnostic when bulk write works + self.store.publish(self.course_loc.version_agnostic(), self.user.id) return self.course_loc.course_key.version_agnostic() def create_toy_course(self, org='edX', course='toy', run='2012_Fall'): @@ -318,43 +320,43 @@ class ModuleStoreTestCase(TestCase): Create an equivalent to the toy xml course """ # with self.store.bulk_operations(self.store.make_course_key(org, course, run)): - self.toy_loc = self.create_sample_course( + self.toy_loc = self.create_sample_course( # pylint: disable=attribute-defined-outside-init org, course, run, TOY_BLOCK_INFO_TREE, { - "textbooks" : [["Textbook", "https://s3.amazonaws.com/edx-textbooks/guttag_computation_v3/"]], - "wiki_slug" : "toy", - "display_name" : "Toy Course", - "graded" : True, - "tabs" : [ - CoursewareTab(), - CourseInfoTab(), - StaticTab(name="Syllabus", url_slug="syllabus"), - StaticTab(name="Resources", url_slug="resources"), - DiscussionTab(), - WikiTab(), - ProgressTab(), + "textbooks": [["Textbook", "https://s3.amazonaws.com/edx-textbooks/guttag_computation_v3/"]], + "wiki_slug": "toy", + "display_name": "Toy Course", + "graded": True, + "tabs": [ + CoursewareTab(), + CourseInfoTab(), + StaticTab(name="Syllabus", url_slug="syllabus"), + StaticTab(name="Resources", url_slug="resources"), + DiscussionTab(), + WikiTab(), + ProgressTab(), ], - "discussion_topics" : {"General" : {"id" : "i4x-edX-toy-course-2012_Fall"}}, - "graceperiod" : datetime.timedelta(days=2, seconds=21599), - "start" : datetime.datetime(2015, 07, 17, 12, tzinfo=pytz.utc), - "xml_attributes" : {"filename" : ["course/2012_Fall.xml", "course/2012_Fall.xml"]}, - "pdf_textbooks" : [ + "discussion_topics": {"General": {"id": "i4x-edX-toy-course-2012_Fall"}}, + "graceperiod": datetime.timedelta(days=2, seconds=21599), + "start": datetime.datetime(2015, 07, 17, 12, tzinfo=pytz.utc), + "xml_attributes": {"filename": ["course/2012_Fall.xml", "course/2012_Fall.xml"]}, + "pdf_textbooks": [ { - "tab_title" : "Sample Multi Chapter Textbook", - "id" : "MyTextbook", - "chapters" : [ - {"url" : "/static/Chapter1.pdf", "title" : "Chapter 1"}, - {"url" : "/static/Chapter2.pdf", "title" : "Chapter 2"} + "tab_title": "Sample Multi Chapter Textbook", + "id": "MyTextbook", + "chapters": [ + {"url": "/static/Chapter1.pdf", "title": "Chapter 1"}, + {"url": "/static/Chapter2.pdf", "title": "Chapter 2"} ] - } + } ], - "course_image" : "just_a_test.jpg", + "course_image": "just_a_test.jpg", } ) with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.toy_loc): self.store.create_item( self.user.id, self.toy_loc, "about", block_id="short_description", - fields={"data" : "A course about toys."} + fields={"data": "A course about toys."} ) self.store.create_item( self.user.id, self.toy_loc, "about", block_id="effort", diff --git a/common/lib/xmodule/xmodule/modulestore/tests/sample_courses.py b/common/lib/xmodule/xmodule/modulestore/tests/sample_courses.py index aa634cc09b..752c61eada 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/sample_courses.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/sample_courses.py @@ -7,8 +7,8 @@ The data type and use of it for declaratively creating test courses. # fields is a dictionary of keys and values. sub_tree is a collection of BlockInfo from collections import namedtuple import datetime -BlockInfo = namedtuple('BlockInfo', 'block_id, category, fields, sub_tree') -default_block_info_tree = [ +BlockInfo = namedtuple('BlockInfo', 'block_id, category, fields, sub_tree') # pylint: disable=invalid-name +default_block_info_tree = [ # pylint: disable=invalid-name BlockInfo( 'chapter_x', 'chapter', {}, [ BlockInfo( @@ -44,7 +44,7 @@ default_block_info_tree = [ # equivalent to toy course in xml TOY_BLOCK_INFO_TREE = [ BlockInfo( - 'Overview', "chapter", {"display_name" : "Overview"}, [ + 'Overview', "chapter", {"display_name": "Overview"}, [ BlockInfo( "Toy_Videos", "videosequence", { "xml_attributes": {"filename": ["", None]}, "display_name": "Toy Videos", "format": "Lecture Sequence" @@ -52,24 +52,24 @@ TOY_BLOCK_INFO_TREE = [ BlockInfo( "secret:toylab", "html", { "data": "Lab 2A: Superposition Experiment\n\n<<<<<<< Updated upstream\n

Isn't the toy course great?

\n\n

Let's add some markup that uses non-ascii characters.\nFor example, we should be able to write words like encyclopædia, or foreign words like français.\nLooking beyond latin-1, we should handle math symbols: πr² ≤ ∞.\nAnd it shouldn't matter if we use entities or numeric codes — Ω ≠ π ≡ Ω ≠ π.\n

\n=======\n

Isn't the toy course great? — ≤

\n>>>>>>> Stashed changes\n", - "xml_attributes": { "filename" : [ "html/secret/toylab.xml", "html/secret/toylab.xml" ] }, - "display_name" : "Toy lab" + "xml_attributes": {"filename": ["html/secret/toylab.xml", "html/secret/toylab.xml"]}, + "display_name": "Toy lab" }, [] ), BlockInfo( "toyjumpto", "html", { - "data" : "This is a link to another page and some Chinese 四節比分和七年前

Some more Chinese 四節比分和七年前

\n", - "xml_attributes": { "filename" : [ "html/toyjumpto.xml", "html/toyjumpto.xml" ] } + "data": "This is a link to another page and some Chinese 四節比分和七年前

Some more Chinese 四節比分和七年前

\n", + "xml_attributes": {"filename": ["html/toyjumpto.xml", "html/toyjumpto.xml"]} }, []), BlockInfo( "toyhtml", "html", { - "data" : "Sample", - "xml_attributes" : { "filename" : [ "html/toyhtml.xml", "html/toyhtml.xml" ] } + "data": "Sample", + "xml_attributes": {"filename": ["html/toyhtml.xml", "html/toyhtml.xml"]} }, []), BlockInfo( "nonportable", "html", { "data": "link\n", - "xml_attributes" : { "filename" : [ "html/nonportable.xml", "html/nonportable.xml" ] } + "xml_attributes": {"filename": ["html/nonportable.xml", "html/nonportable.xml"]} }, []), BlockInfo( "nonportable_link", "html", { @@ -79,7 +79,7 @@ TOY_BLOCK_INFO_TREE = [ BlockInfo( "badlink", "html", { "data": "\n", - "xml_attributes" : { "filename" : [ "html/badlink.xml", "html/badlink.xml" ] } + "xml_attributes": {"filename": ["html/badlink.xml", "html/badlink.xml"]} }, []), BlockInfo( "with_styling", "html", { @@ -89,11 +89,11 @@ TOY_BLOCK_INFO_TREE = [ BlockInfo( "just_img", "html", { "data": "", - "xml_attributes": {"filename": [ "html/just_img.xml", "html/just_img.xml" ] } + "xml_attributes": {"filename": ["html/just_img.xml", "html/just_img.xml"]} }, []), BlockInfo( "Video_Resources", "video", { - "youtube_id_1_0" : "1bK-WdDi6Qw", "display_name" : "Video Resources" + "youtube_id_1_0": "1bK-WdDi6Qw", "display_name": "Video Resources" }, []), ]), BlockInfo( @@ -109,7 +109,7 @@ TOY_BLOCK_INFO_TREE = [ ), BlockInfo( "secret:magic", "chapter", { - "xml_attributes": {"filename": [ "chapter/secret/magic.xml", "chapter/secret/magic.xml"]} + "xml_attributes": {"filename": ["chapter/secret/magic.xml", "chapter/secret/magic.xml"]} }, [ BlockInfo( "toyvideo", "video", {"youtube_id_1_0": "OEoXaMPEzfMA", "display_name": "toyvideo"}, [] @@ -117,18 +117,18 @@ TOY_BLOCK_INFO_TREE = [ ] ), BlockInfo( - "poll_test", "chapter", {}, [ + "poll_test", "chapter", {}, [ BlockInfo( "T1_changemind_poll_foo", "poll_question", { "question": "

Have you changed your mind? ’

", "answers": [{"text": "Yes", "id": "yes"}, {"text": "No", "id": "no"}], "xml_attributes": {"reset": "false", "filename": ["", None]}, "display_name": "Change your answer" - }, []) ] + }, [])] ), BlockInfo( - "vertical_container", "chapter", { - "xml_attributes": {"filename": ["chapter/vertical_container.xml", "chapter/vertical_container.xml"]} + "vertical_container", "chapter", { + "xml_attributes": {"filename": ["chapter/vertical_container.xml", "chapter/vertical_container.xml"]} }, [ BlockInfo("vertical_sequential", "sequential", {}, [ BlockInfo("vertical_test", "vertical", { @@ -163,7 +163,7 @@ TOY_BLOCK_INFO_TREE = [ "T1_changemind_poll_foo_2", "poll_question", { "question": "

Have you changed your mind?

", "answers": [{"text": "Yes", "id": "yes"}, {"text": "No", "id": "no"}], - "xml_attributes": {"reset": "false", "filename": [ "", None]}, + "xml_attributes": {"reset": "false", "filename": ["", None]}, "display_name": "Change your answer" }, []), ]), @@ -174,8 +174,8 @@ TOY_BLOCK_INFO_TREE = [ ] ), BlockInfo( - "handout_container", "chapter", { - "xml_attributes" : {"filename" : ["chapter/handout_container.xml", "chapter/handout_container.xml"]} + "handout_container", "chapter", { + "xml_attributes": {"filename": ["chapter/handout_container.xml", "chapter/handout_container.xml"]} }, [ BlockInfo( "html_7e5578f25f79", "html", { diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 2ed87a592d..1dd81e863a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -72,54 +72,54 @@ class SplitModuleTest(unittest.TestCase): "user_id": "test@edx.org", "fields": { "tabs": [ - { - "type": "courseware" - }, - { - "type": "course_info", - "name": "Course Info" - }, - { - "type": "discussion", - "name": "Discussion" - }, - { - "type": "wiki", - "name": "Wiki" - } - ], + { + "type": "courseware" + }, + { + "type": "course_info", + "name": "Course Info" + }, + { + "type": "discussion", + "name": "Discussion" + }, + { + "type": "wiki", + "name": "Wiki" + } + ], "start": _date_field.from_json("2013-02-14T05:00"), "display_name": "The Ancient Greek Hero", "grading_policy": { "GRADER": [ - { - "min_count": 5, - "weight": 0.15, - "type": "Homework", - "drop_count": 1, - "short_label": "HWa" - }, - { - "short_label": "", - "min_count": 2, - "type": "Lab", - "drop_count": 0, - "weight": 0.15 - }, - { - "short_label": "Midterm", - "min_count": 1, - "type": "Midterm Exam", - "drop_count": 0, - "weight": 0.3 - }, - { - "short_label": "Final", - "min_count": 1, - "type": "Final Exam", - "drop_count": 0, - "weight": 0.4 - } + { + "min_count": 5, + "weight": 0.15, + "type": "Homework", + "drop_count": 1, + "short_label": "HWa" + }, + { + "short_label": "", + "min_count": 2, + "type": "Lab", + "drop_count": 0, + "weight": 0.15 + }, + { + "short_label": "Midterm", + "min_count": 1, + "type": "Midterm Exam", + "drop_count": 0, + "weight": 0.3 + }, + { + "short_label": "Final", + "min_count": 1, + "type": "Final Exam", + "drop_count": 0, + "weight": 0.4 + } ], "GRADE_CUTOFFS": { "Pass": 0.75 @@ -132,31 +132,31 @@ class SplitModuleTest(unittest.TestCase): "head12345": { "end": _date_field.from_json("2013-04-13T04:30"), "tabs": [ - { - "type": "courseware" - }, - { - "type": "course_info", - "name": "Course Info" - }, - { - "type": "discussion", - "name": "Discussion" - }, - { - "type": "wiki", - "name": "Wiki" - }, - { - "type": "static_tab", - "name": "Syllabus", - "url_slug": "01356a17b5924b17a04b7fc2426a3798" - }, - { - "type": "static_tab", - "name": "Advice for Students", - "url_slug": "57e9991c0d794ff58f7defae3e042e39" - } + { + "type": "courseware" + }, + { + "type": "course_info", + "name": "Course Info" + }, + { + "type": "discussion", + "name": "Discussion" + }, + { + "type": "wiki", + "name": "Wiki" + }, + { + "type": "static_tab", + "name": "Syllabus", + "url_slug": "01356a17b5924b17a04b7fc2426a3798" + }, + { + "type": "static_tab", + "name": "Advice for Students", + "url_slug": "57e9991c0d794ff58f7defae3e042e39" + } ], "graceperiod": _time_delta_field.from_json("2 hours 0 minutes 0 seconds"), "grading_policy": { @@ -195,7 +195,7 @@ class SplitModuleTest(unittest.TestCase): } }, }} - }, + }, {"user_id": "testassist@edx.org", "update": {"head12345": { @@ -239,50 +239,50 @@ class SplitModuleTest(unittest.TestCase): "enrollment_end": _date_field.from_json("2013-03-02T05:00"), "advertised_start": "Fall 2013", }}, - "create": [ - { - "id": "chapter1", - "parent": "head12345", - "category": "chapter", - "fields": { - "display_name": "Hercules" - }, - }, - { - "id": "chapter2", - "parent": "head12345", - "category": "chapter", - "fields": { - "display_name": "Hera heckles Hercules" - }, - }, - { - "id": "chapter3", - "parent": "head12345", - "category": "chapter", - "fields": { - "display_name": "Hera cuckolds Zeus" - }, - }, - { - "id": "problem1", - "parent": "chapter3", - "category": "problem", - "fields": { - "display_name": "Problem 3.1", - "graceperiod": _time_delta_field.from_json("4 hours 0 minutes 0 seconds"), - }, - }, - { - "id": "problem3_2", - "parent": "chapter3", - "category": "problem", - "fields": { - "display_name": "Problem 3.2" - }, - } - ] - }, + "create": [ + { + "id": "chapter1", + "parent": "head12345", + "category": "chapter", + "fields": { + "display_name": "Hercules" + }, + }, + { + "id": "chapter2", + "parent": "head12345", + "category": "chapter", + "fields": { + "display_name": "Hera heckles Hercules" + }, + }, + { + "id": "chapter3", + "parent": "head12345", + "category": "chapter", + "fields": { + "display_name": "Hera cuckolds Zeus" + }, + }, + { + "id": "problem1", + "parent": "chapter3", + "category": "problem", + "fields": { + "display_name": "Problem 3.1", + "graceperiod": _time_delta_field.from_json("4 hours 0 minutes 0 seconds"), + }, + }, + { + "id": "problem3_2", + "parent": "chapter3", + "category": "problem", + "fields": { + "display_name": "Problem 3.2" + }, + } + ] + }, ] }, "testx.wonderful": { @@ -451,7 +451,7 @@ class SplitModuleTest(unittest.TestCase): } @staticmethod - def bootstrapDB(split_store): + def bootstrapDB(split_store): # pylint: disable=invalid-name ''' Sets up the initial data into the db ''' @@ -517,7 +517,7 @@ class SplitModuleTest(unittest.TestCase): SplitModuleTest.modulestore = None super(SplitModuleTest, self).tearDown() - def findByIdInResult(self, collection, _id): + def findByIdInResult(self, collection, _id): # pylint: disable=invalid-name """ Result is a collection of descriptors. Find the one whose block id matches the _id. @@ -716,6 +716,7 @@ class SplitModuleCourseTests(SplitModuleTest): self.assertEqual(len(result.children[0].children), 1) self.assertEqual(result.children[0].children[0].locator.version_guid, versions[0]) + class SplitModuleItemTests(SplitModuleTest): ''' Item read tests including inheritance @@ -946,6 +947,10 @@ class SplitModuleItemTests(SplitModuleTest): def version_agnostic(children): + """ + children: list of descriptors + Returns the `children` list with each member version-agnostic + """ return [child.version_agnostic() for child in children] @@ -1035,7 +1040,6 @@ class TestItemCrud(SplitModuleTest): self.assertIn(new_module.location.version_agnostic(), version_agnostic(parent.children)) self.assertEqual(new_module.definition_locator.definition_id, original.definition_locator.definition_id) - def test_unique_naming(self): """ Check that 2 modules of same type get unique block_ids. Also check that if creation provides @@ -1760,6 +1764,6 @@ def modulestore(): return SplitModuleTest.modulestore -# pylint: disable=W0613 +# pylint: disable=unused-argument, missing-docstring def render_to_template_mock(*args): pass diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index 0c6ab792c1..e28561a6e1 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -33,7 +33,7 @@ class PeerGradingFields(object): use_for_single_location = Boolean( display_name=_("Show Single Problem"), help=_('When True, only the single problem specified by "Link to Problem Location" is shown. ' - 'When False, a panel is displayed with all problems available for peer grading.'), + 'When False, a panel is displayed with all problems available for peer grading.'), default=False, scope=Scope.settings ) @@ -54,9 +54,9 @@ class PeerGradingFields(object): scope=Scope.settings) extended_due = Date( help=_("Date that this problem is due by for a particular student. This " - "can be set by an instructor, and will override the global due " - "date if it is set to a date that is later than the global due " - "date."), + "can be set by an instructor, and will override the global due " + "date if it is set to a date that is later than the global due " + "date."), default=None, scope=Scope.user_state, ) @@ -86,6 +86,7 @@ class PeerGradingFields(object): scope=Scope.content ) + class InvalidLinkLocation(Exception): """ Exception for the case in which a peer grading module tries to link to an invalid location. @@ -150,7 +151,8 @@ class PeerGradingModule(PeerGradingFields, XModule): try: self.student_data_for_location = json.loads(self.student_data_for_location) - except Exception: + except Exception: # pylint: disable=broad-except + # OK with this broad exception because we just want to continue on any error pass @property @@ -218,9 +220,9 @@ class PeerGradingModule(PeerGradingFields, XModule): # This is a dev_facing_error return json.dumps({'error': 'Error handling action. Please try again.', 'success': False}) - d = handlers[dispatch](data) + data_dict = handlers[dispatch](data) - return json.dumps(d, cls=ComplexEncoder) + return json.dumps(data_dict, cls=ComplexEncoder) def query_data_for_location(self, location): student_id = self.system.anonymous_student_id @@ -229,13 +231,12 @@ class PeerGradingModule(PeerGradingFields, XModule): try: response = self.peer_gs.get_data_for_location(location, student_id) - count_graded = response['count_graded'] - count_required = response['count_required'] + _count_graded = response['count_graded'] + _count_required = response['count_required'] success = True except GradingServiceError: # This is a dev_facing_error - log.exception("Error getting location data from controller for location {0}, student {1}" - .format(location, student_id)) + log.exception("Error getting location data from controller for location %s, student %s", location, student_id) return success, response @@ -322,8 +323,7 @@ class PeerGradingModule(PeerGradingFields, XModule): return response except GradingServiceError: # This is a dev_facing_error - log.exception("Error getting next submission. server url: {0} location: {1}, grader_id: {2}" - .format(self.peer_gs.url, location, grader_id)) + log.exception("Error getting next submission. server url: %s location: %s, grader_id: %s", self.peer_gs.url, location, grader_id) # This is a student_facing_error return {'success': False, 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR} @@ -355,7 +355,7 @@ class PeerGradingModule(PeerGradingFields, XModule): if not success: return self._err_response(message) - data_dict = {k:data.get(k) for k in required} + data_dict = {k: data.get(k) for k in required} if 'rubric_scores[]' in required: data_dict['rubric_scores'] = data.getall('rubric_scores[]') data_dict['grader_id'] = self.system.anonymous_student_id @@ -365,15 +365,14 @@ class PeerGradingModule(PeerGradingFields, XModule): success, location_data = self.query_data_for_location(data_dict['location']) #Don't check for success above because the response = statement will raise the same Exception as the one #that will cause success to be false. - response.update({'required_done' : False}) - if 'count_graded' in location_data and 'count_required' in location_data and int(location_data['count_graded'])>=int(location_data['count_required']): + response.update({'required_done': False}) + if 'count_graded' in location_data and 'count_required' in location_data and int(location_data['count_graded']) >= int(location_data['count_required']): response['required_done'] = True return response except GradingServiceError: # This is a dev_facing_error - log.exception("""Error saving grade to open ended grading service. server url: {0}""" - .format(self.peer_gs.url) - ) + log.exception("Error saving grade to open ended grading service. server url: %s", self.peer_gs.url) + # This is a student_facing_error return { 'success': False, @@ -411,8 +410,7 @@ class PeerGradingModule(PeerGradingFields, XModule): return response except GradingServiceError: # This is a dev_facing_error - log.exception("Error from open ended grading service. server url: {0}, grader_id: {0}, location: {1}" - .format(self.peer_gs.url, grader_id, location)) + log.exception("Error from open ended grading service. server url: %s, grader_id: %s, location: %s", self.peer_gs.url, grader_id, location) # This is a student_facing_error return { 'success': False, @@ -456,8 +454,7 @@ class PeerGradingModule(PeerGradingFields, XModule): return response except GradingServiceError: # This is a dev_facing_error - log.exception("Error from open ended grading service. server url: {0}, location: {0}" - .format(self.peer_gs.url, location)) + log.exception("Error from open ended grading service. server url: %s, location: %s", self.peer_gs.url, location) # This is a student_facing_error return {'success': False, 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR} @@ -492,7 +489,7 @@ class PeerGradingModule(PeerGradingFields, XModule): if not success: return self._err_response(message) - data_dict = {k:data.get(k) for k in required} + data_dict = {k: data.get(k) for k in required} data_dict['rubric_scores'] = data.getall('rubric_scores[]') data_dict['student_id'] = self.system.anonymous_student_id data_dict['calibration_essay_id'] = data_dict['submission_id'] @@ -619,7 +616,7 @@ class PeerGradingModule(PeerGradingFields, XModule): elif data.get('location') is not None: problem_location = self.course_id.make_usage_key_from_deprecated_string(data.get('location')) - module = self._find_corresponding_module_for_location(problem_location) + module = self._find_corresponding_module_for_location(problem_location) # pylint: disable-unused-variable ajax_url = self.ajax_url html = self.system.render_template('peer_grading/peer_grading_problem.html', { diff --git a/common/lib/xmodule/xmodule/tests/test_lti_unit.py b/common/lib/xmodule/xmodule/tests/test_lti_unit.py index 6a95ad816e..13de6ceef9 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti_unit.py @@ -54,25 +54,29 @@ class LTIModuleTest(LogicTest): self.user_id = self.xmodule.runtime.anonymous_student_id self.lti_id = self.xmodule.lti_id - self.unquoted_resource_link_id= u'{}-i4x-2-3-lti-31de800015cf4afb973356dbe81496df'.format(self.xmodule.runtime.hostname) + self.unquoted_resource_link_id = u'{}-i4x-2-3-lti-31de800015cf4afb973356dbe81496df'.format(self.xmodule.runtime.hostname) - sourcedId = u':'.join(urllib.quote(i) for i in (self.lti_id, self.unquoted_resource_link_id, self.user_id)) + sourced_id = u':'.join(urllib.quote(i) for i in (self.lti_id, self.unquoted_resource_link_id, self.user_id)) - self.DEFAULTS = { - 'namespace' : "http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0", - 'sourcedId': sourcedId, + self.defaults = { + 'namespace': "http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0", + 'sourcedId': sourced_id, 'action': 'replaceResultRequest', 'grade': 0.5, 'messageIdentifier': '528243ba5241b', } - def get_request_body(self, params={}): - data = copy(self.DEFAULTS) + def get_request_body(self, params=None): + """Fetches the body of a request specified by params""" + if params is None: + params = {} + data = copy(self.defaults) data.update(params) return self.request_body_xml_template.format(**data) def get_response_values(self, response): + """Gets the values from the given response""" parser = etree.XMLParser(ns_clean=True, recover=True, encoding='utf-8') root = etree.fromstring(response.body.strip(), parser=parser) lti_spec_namespace = "http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0" @@ -80,23 +84,23 @@ class LTIModuleTest(LogicTest): code_major = root.xpath("//def:imsx_codeMajor", namespaces=namespaces)[0].text description = root.xpath("//def:imsx_description", namespaces=namespaces)[0].text - messageIdentifier = root.xpath("//def:imsx_messageIdentifier", namespaces=namespaces)[0].text - imsx_POXBody = root.xpath("//def:imsx_POXBody", namespaces=namespaces)[0] + message_identifier = root.xpath("//def:imsx_messageIdentifier", namespaces=namespaces)[0].text + imsx_pox_body = root.xpath("//def:imsx_POXBody", namespaces=namespaces)[0] try: - action = imsx_POXBody.getchildren()[0].tag.replace('{'+lti_spec_namespace+'}', '') - except Exception: + action = imsx_pox_body.getchildren()[0].tag.replace('{' + lti_spec_namespace + '}', '') + except Exception: # pylint: disable=broad-except action = None return { 'code_major': code_major, 'description': description, - 'messageIdentifier': messageIdentifier, + 'messageIdentifier': message_identifier, 'action': action } @patch('xmodule.lti_module.LTIModule.get_client_key_secret', return_value=('test_client_key', u'test_client_secret')) - def test_authorization_header_not_present(self, get_key_secret): + def test_authorization_header_not_present(self, _get_key_secret): """ Request has no Authorization header. @@ -110,14 +114,14 @@ class LTIModuleTest(LogicTest): 'action': None, 'code_major': 'failure', 'description': 'OAuth verification error: Malformed authorization header', - 'messageIdentifier': self.DEFAULTS['messageIdentifier'], + 'messageIdentifier': self.defaults['messageIdentifier'], } self.assertEqual(response.status_code, 200) self.assertDictEqual(expected_response, real_response) @patch('xmodule.lti_module.LTIModule.get_client_key_secret', return_value=('test_client_key', u'test_client_secret')) - def test_authorization_header_empty(self, get_key_secret): + def test_authorization_header_empty(self, _get_key_secret): """ Request Authorization header has no value. @@ -132,7 +136,7 @@ class LTIModuleTest(LogicTest): 'action': None, 'code_major': 'failure', 'description': 'OAuth verification error: Malformed authorization header', - 'messageIdentifier': self.DEFAULTS['messageIdentifier'], + 'messageIdentifier': self.defaults['messageIdentifier'], } self.assertEqual(response.status_code, 200) self.assertDictEqual(expected_response, real_response) @@ -152,7 +156,7 @@ class LTIModuleTest(LogicTest): 'action': None, 'code_major': 'failure', 'description': 'User not found.', - 'messageIdentifier': self.DEFAULTS['messageIdentifier'], + 'messageIdentifier': self.defaults['messageIdentifier'], } self.assertEqual(response.status_code, 200) self.assertDictEqual(expected_response, real_response) @@ -207,7 +211,7 @@ class LTIModuleTest(LogicTest): 'action': None, 'code_major': 'unsupported', 'description': 'Target does not support the requested operation.', - 'messageIdentifier': self.DEFAULTS['messageIdentifier'], + 'messageIdentifier': self.defaults['messageIdentifier'], } self.assertEqual(response.status_code, 200) self.assertDictEqual(expected_response, real_response) @@ -222,20 +226,20 @@ class LTIModuleTest(LogicTest): request.body = self.get_request_body() response = self.xmodule.grade_handler(request, '') description_expected = 'Score for {sourcedId} is now {score}'.format( - sourcedId=self.DEFAULTS['sourcedId'], - score=self.DEFAULTS['grade'], - ) + sourcedId=self.defaults['sourcedId'], + score=self.defaults['grade'], + ) real_response = self.get_response_values(response) expected_response = { 'action': 'replaceResultResponse', 'code_major': 'success', 'description': description_expected, - 'messageIdentifier': self.DEFAULTS['messageIdentifier'], + 'messageIdentifier': self.defaults['messageIdentifier'], } self.assertEqual(response.status_code, 200) self.assertDictEqual(expected_response, real_response) - self.assertEqual(self.xmodule.module_score, float(self.DEFAULTS['grade'])) + self.assertEqual(self.xmodule.module_score, float(self.defaults['grade'])) def test_user_id(self): expected_user_id = unicode(urllib.quote(self.xmodule.runtime.anonymous_student_id)) @@ -255,27 +259,27 @@ class LTIModuleTest(LogicTest): self.assertEqual(real_outcome_service_url, mock_url_prefix + test_service_name) def test_resource_link_id(self): - with patch('xmodule.lti_module.LTIModule.location', new_callable=PropertyMock) as mock_location: + with patch('xmodule.lti_module.LTIModule.location', new_callable=PropertyMock): self.xmodule.location.html_id = lambda: 'i4x-2-3-lti-31de800015cf4afb973356dbe81496df' expected_resource_link_id = unicode(urllib.quote(self.unquoted_resource_link_id)) real_resource_link_id = self.xmodule.get_resource_link_id() self.assertEqual(real_resource_link_id, expected_resource_link_id) def test_lis_result_sourcedid(self): - expected_sourcedId = u':'.join(urllib.quote(i) for i in ( + expected_sourced_id = u':'.join(urllib.quote(i) for i in ( self.system.course_id.to_deprecated_string(), self.xmodule.get_resource_link_id(), self.user_id )) real_lis_result_sourcedid = self.xmodule.get_lis_result_sourcedid() - self.assertEqual(real_lis_result_sourcedid, expected_sourcedId) + self.assertEqual(real_lis_result_sourcedid, expected_sourced_id) def test_client_key_secret(self): """ LTI module gets client key and secret provided. """ #this adds lti passports to system - mocked_course = Mock(lti_passports = ['lti_id:test_client:test_secret']) + mocked_course = Mock(lti_passports=['lti_id:test_client:test_secret']) modulestore = Mock() modulestore.get_course.return_value = mocked_course runtime = Mock(modulestore=modulestore) @@ -293,7 +297,7 @@ class LTIModuleTest(LogicTest): """ #this adds lti passports to system - mocked_course = Mock(lti_passports = ['test_id:test_client:test_secret']) + mocked_course = Mock(lti_passports=['test_id:test_client:test_secret']) modulestore = Mock() modulestore.get_course.return_value = mocked_course runtime = Mock(modulestore=modulestore) @@ -301,7 +305,7 @@ class LTIModuleTest(LogicTest): #set another lti_id self.xmodule.lti_id = "another_lti_id" key_secret = self.xmodule.get_client_key_secret() - expected = ('','') + expected = ('', '') self.assertEqual(expected, key_secret) def test_bad_client_key_secret(self): @@ -311,7 +315,7 @@ class LTIModuleTest(LogicTest): There are key and secret provided in wrong format. """ #this adds lti passports to system - mocked_course = Mock(lti_passports = ['test_id_test_client_test_secret']) + mocked_course = Mock(lti_passports=['test_id_test_client_test_secret']) modulestore = Mock() modulestore.get_course.return_value = mocked_course runtime = Mock(modulestore=modulestore) @@ -348,11 +352,11 @@ class LTIModuleTest(LogicTest): Tests that xml body was parsed successfully. """ mocked_request = self.get_signed_grade_mock_request() - messageIdentifier, sourcedId, grade, action = self.xmodule.parse_grade_xml_body(mocked_request.body) - self.assertEqual(self.DEFAULTS['messageIdentifier'], messageIdentifier) - self.assertEqual(self.DEFAULTS['sourcedId'], sourcedId) - self.assertEqual(self.DEFAULTS['grade'], grade) - self.assertEqual(self.DEFAULTS['action'], action) + message_identifier, sourced_id, grade, action = self.xmodule.parse_grade_xml_body(mocked_request.body) + self.assertEqual(self.defaults['messageIdentifier'], message_identifier) + self.assertEqual(self.defaults['sourcedId'], sourced_id) + self.assertEqual(self.defaults['grade'], grade) + self.assertEqual(self.defaults['action'], action) @patch('xmodule.lti_module.signature.verify_hmac_sha1', Mock(return_value=False)) @patch('xmodule.lti_module.LTIModule.get_client_key_secret', Mock(return_value=('test_client_key', u'test_client_secret'))) @@ -372,7 +376,7 @@ class LTIModuleTest(LogicTest): LTI 1.1 will be used. Otherwise fake namespace will be added to XML. """ mock_request = Mock() - mock_request.headers = { + mock_request.headers = { # pylint: disable=no-space-before-operator 'X-Requested-With': 'XMLHttpRequest', 'Content-Type': 'application/x-www-form-urlencoded', 'Authorization': u'OAuth oauth_nonce="135685044251684026041377608307", \ diff --git a/lms/djangoapps/dashboard/views.py b/lms/djangoapps/dashboard/views.py index 26e35f172c..aca1fdcd9a 100644 --- a/lms/djangoapps/dashboard/views.py +++ b/lms/djangoapps/dashboard/views.py @@ -1,3 +1,4 @@ +"""View functions for the LMS Student dashboard""" from django.http import Http404 from edxmako.shortcuts import render_to_response from django.db import connection @@ -15,15 +16,18 @@ def dictfetchall(cursor): # ensure response from db is a list, not a tuple (which is returned # by MySQL backed django instances) - rows_from_cursor=cursor.fetchall() + rows_from_cursor = cursor.fetchall() table = table + [list(row) for row in rows_from_cursor] return table -def SQL_query_to_list(cursor, query_string): + +def SQL_query_to_list(cursor, query_string): # pylint: disable=invalid-name + """Returns the raw result of the query""" cursor.execute(query_string) - raw_result=dictfetchall(cursor) + raw_result = dictfetchall(cursor) return raw_result + def dashboard(request): """ Slightly less hackish hack to show staff enrollment numbers and other @@ -40,11 +44,11 @@ def dashboard(request): # two types of results: scalars and tables. Scalars should be represented # as "Visible Title": Value and tables should be lists of lists where each # inner list represents a single row of the table - results = {"scalars":{},"tables":{}} + results = {"scalars": {}, "tables": {}} # count how many users we have - results["scalars"]["Unique Usernames"]=User.objects.filter().count() - results["scalars"]["Activated Usernames"]=User.objects.filter(is_active=1).count() + results["scalars"]["Unique Usernames"] = User.objects.filter().count() + results["scalars"]["Activated Usernames"] = User.objects.filter(is_active=1).count() # count how many enrollments we have results["scalars"]["Total Enrollments Across All Courses"] = CourseEnrollment.objects.filter(is_active=1).count() @@ -78,6 +82,6 @@ def dashboard(request): cursor.execute(table_queries[query]) results["tables"][query] = SQL_query_to_list(cursor, table_queries[query]) - context={"results":results} + context = {"results": results} - return render_to_response("admin_dashboard.html",context) + return render_to_response("admin_dashboard.html", context) diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index bd7809f9ff..87cf77838b 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -193,7 +193,7 @@ class SingleThreadTestCase(ModuleStoreTestCase): ) mock_request.assert_called_with( "get", - StringEndsWithMatcher(thread_id), # url + StringEndsWithMatcher(thread_id), # url data=None, params=PartialDictMatcher({"mark_as_read": True, "user_id": 1, "recursive": True}), headers=ANY, @@ -227,7 +227,7 @@ class SingleThreadTestCase(ModuleStoreTestCase): ) mock_request.assert_called_with( "get", - StringEndsWithMatcher(thread_id), # url + StringEndsWithMatcher(thread_id), # url data=None, params=PartialDictMatcher({ "mark_as_read": True, @@ -365,7 +365,7 @@ class UserProfileTestCase(ModuleStoreTestCase): "course_id": self.course.id.to_deprecated_string(), "page": params.get("page", 1), "per_page": views.THREADS_PER_PAGE - }), + }), headers=ANY, timeout=ANY ) @@ -393,7 +393,7 @@ class UserProfileTestCase(ModuleStoreTestCase): self.assertEqual( sorted(response_data.keys()), ["annotated_content_info", "discussion_data", "num_pages", "page"] - ) + ) self.assertEqual(len(response_data['discussion_data']), 1) self.assertEqual(response_data["page"], 1) self.assertEqual(response_data["num_pages"], 1) @@ -444,6 +444,7 @@ class UserProfileTestCase(ModuleStoreTestCase): ) self.assertEqual(response.status_code, 405) + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @patch('requests.request') class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase): @@ -463,8 +464,8 @@ class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase): def assert_all_calls_have_header(self, mock_request, key, value): expected = call( - ANY, # method - ANY, # url + ANY, # method + ANY, # url data=ANY, params=ANY, headers=PartialDictMatcher({key: value}), @@ -537,7 +538,7 @@ class ForumFormDiscussionUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin): mock_request.side_effect = make_mock_request_impl(text) request = RequestFactory().get("dummy_url") request.user = self.student - request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True + request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True response = views.forum_form_discussion(request, self.course.id.to_deprecated_string()) self.assertEqual(response.status_code, 200) @@ -559,7 +560,7 @@ class SingleThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin): mock_request.side_effect = make_mock_request_impl(text, thread_id) request = RequestFactory().get("dummy_url") request.user = self.student - request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True + request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True response = views.single_thread(request, self.course.id.to_deprecated_string(), "dummy_discussion_id", thread_id) self.assertEqual(response.status_code, 200) @@ -580,7 +581,7 @@ class UserProfileUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin): mock_request.side_effect = make_mock_request_impl(text) request = RequestFactory().get("dummy_url") request.user = self.student - request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True + request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True response = views.user_profile(request, self.course.id.to_deprecated_string(), str(self.student.id)) self.assertEqual(response.status_code, 200) @@ -601,7 +602,7 @@ class FollowedThreadsUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin): mock_request.side_effect = make_mock_request_impl(text) request = RequestFactory().get("dummy_url") request.user = self.student - request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True + request.META["HTTP_X_REQUESTED_WITH"] = "XMLHttpRequest" # so request.is_ajax() == True response = views.followed_threads(request, self.course.id.to_deprecated_string(), str(self.student.id)) self.assertEqual(response.status_code, 200) diff --git a/lms/djangoapps/foldit/tests.py b/lms/djangoapps/foldit/tests.py index 60a8c661b1..6943e4d91f 100644 --- a/lms/djangoapps/foldit/tests.py +++ b/lms/djangoapps/foldit/tests.py @@ -1,3 +1,4 @@ +"""Tests for the FoldIt module""" import json import logging from functools import partial @@ -19,7 +20,7 @@ log = logging.getLogger(__name__) class FolditTestCase(TestCase): - + """Tests for various responses of the FoldIt module""" def setUp(self): self.factory = RequestFactory() self.url = reverse('foldit_ops') @@ -37,10 +38,8 @@ class FolditTestCase(TestCase): self.tomorrow = now + timedelta(days=1) self.yesterday = now - timedelta(days=1) - self.user.profile - self.user2.profile - def make_request(self, post_data, user=None): + """Makes a request to foldit_ops with the given post data and user (if specified)""" request = self.factory.post(self.url, post_data) request.user = self.user if not user else user return request @@ -57,6 +56,7 @@ class FolditTestCase(TestCase): user = self.user if not user else user def score_dict(puzzle_id, best_score): + """Returns a valid json-parsable score dict""" return {"PuzzleID": puzzle_id, "ScoreType": "score", "BestScore": best_score, @@ -77,7 +77,7 @@ class FolditTestCase(TestCase): self.assertEqual(response.status_code, 200) return response - def test_SetPlayerPuzzleScores(self): + def test_SetPlayerPuzzleScores(self): # pylint: disable=invalid-name puzzle_id = 994391 best_score = 0.078034 @@ -94,7 +94,7 @@ class FolditTestCase(TestCase): self.assertEqual(len(top_10), 1) self.assertEqual(top_10[0]['score'], Score.display_score(best_score)) - def test_SetPlayerPuzzleScores_many(self): + def test_SetPlayerPuzzleScores_many(self): # pylint: disable=invalid-name response = self.make_puzzle_score_request([1, 2], [0.078034, 0.080000]) @@ -113,15 +113,14 @@ class FolditTestCase(TestCase): }] )) - - def test_SetPlayerPuzzleScores_multiple(self): + def test_SetPlayerPuzzleScores_multiple(self): # pylint: disable=invalid-name """ Check that multiple posts with the same id are handled properly (keep latest for each user, have multiple users work properly) """ orig_score = 0.07 puzzle_id = '1' - response = self.make_puzzle_score_request([puzzle_id], [orig_score]) + self.make_puzzle_score_request([puzzle_id], [orig_score]) # There should now be a score in the db. top_10 = Score.get_tops_n(10, puzzle_id) @@ -130,7 +129,7 @@ class FolditTestCase(TestCase): # Reporting a better score should overwrite better_score = 0.06 - response = self.make_puzzle_score_request([1], [better_score]) + self.make_puzzle_score_request([1], [better_score]) top_10 = Score.get_tops_n(10, puzzle_id) self.assertEqual(len(top_10), 1) @@ -144,7 +143,7 @@ class FolditTestCase(TestCase): # reporting a worse score shouldn't worse_score = 0.065 - response = self.make_puzzle_score_request([1], [worse_score]) + self.make_puzzle_score_request([1], [worse_score]) top_10 = Score.get_tops_n(10, puzzle_id) self.assertEqual(len(top_10), 1) @@ -155,7 +154,7 @@ class FolditTestCase(TestCase): delta=0.5 ) - def test_SetPlayerPuzzleScores_multiplecourses(self): + def test_SetPlayerPuzzleScores_multiple_courses(self): # pylint: disable=invalid-name puzzle_id = "1" player1_score = 0.05 @@ -164,9 +163,8 @@ class FolditTestCase(TestCase): course_list_1 = [self.course_id] course_list_2 = [self.course_id2] - response1 = self.make_puzzle_score_request( - puzzle_id, player1_score, self.user - ) + self.make_puzzle_score_request(puzzle_id, player1_score, self.user) + course_1_top_10 = Score.get_tops_n(10, puzzle_id, course_list_1) course_2_top_10 = Score.get_tops_n(10, puzzle_id, course_list_2) total_top_10 = Score.get_tops_n(10, puzzle_id) @@ -176,9 +174,8 @@ class FolditTestCase(TestCase): self.assertEqual(len(course_2_top_10), 0) self.assertEqual(len(total_top_10), 1) - response2 = self.make_puzzle_score_request( - puzzle_id, player2_score, self.user2 - ) + self.make_puzzle_score_request(puzzle_id, player2_score, self.user2) + course_2_top_10 = Score.get_tops_n(10, puzzle_id, course_list_2) total_top_10 = Score.get_tops_n(10, puzzle_id) @@ -187,7 +184,7 @@ class FolditTestCase(TestCase): self.assertEqual(len(course_2_top_10), 1) self.assertEqual(len(total_top_10), 2) - def test_SetPlayerPuzzleScores_manyplayers(self): + def test_SetPlayerPuzzleScores_many_players(self): # pylint: disable=invalid-name """ Check that when we send scores from multiple users, the correct order of scores is displayed. Note that, before being processed by @@ -196,18 +193,14 @@ class FolditTestCase(TestCase): puzzle_id = ['1'] player1_score = 0.08 player2_score = 0.02 - response1 = self.make_puzzle_score_request( - puzzle_id, player1_score, self.user - ) + self.make_puzzle_score_request(puzzle_id, player1_score, self.user) # There should now be a score in the db. top_10 = Score.get_tops_n(10, puzzle_id) self.assertEqual(len(top_10), 1) self.assertEqual(top_10[0]['score'], Score.display_score(player1_score)) - response2 = self.make_puzzle_score_request( - puzzle_id, player2_score, self.user2 - ) + self.make_puzzle_score_request(puzzle_id, player2_score, self.user2) # There should now be two scores in the db top_10 = Score.get_tops_n(10, puzzle_id) @@ -228,32 +221,36 @@ class FolditTestCase(TestCase): # Top score user should be self.user2.username self.assertEqual(top_10[0]['username'], self.user2.username) - def test_SetPlayerPuzzleScores_error(self): + def test_SetPlayerPuzzleScores_error(self): # pylint: disable=invalid-name - scores = [{"PuzzleID": 994391, - "ScoreType": "score", - "BestScore": 0.078034, - "CurrentScore": 0.080035, - "ScoreVersion": 23}] + scores = [{ + "PuzzleID": 994391, + "ScoreType": "score", + "BestScore": 0.078034, + "CurrentScore": 0.080035, + "ScoreVersion": 23 + }] validation_str = json.dumps(scores) - verify = {"Verify": verify_code(self.user.email, validation_str), - "VerifyMethod": "FoldItVerify"} + verify = { + "Verify": verify_code(self.user.email, validation_str), + "VerifyMethod": "FoldItVerify" + } # change the real string -- should get an error scores[0]['ScoreVersion'] = 22 scores_str = json.dumps(scores) - data = {'SetPlayerPuzzleScoresVerify': json.dumps(verify), - 'SetPlayerPuzzleScores': scores_str} + data = { + 'SetPlayerPuzzleScoresVerify': json.dumps(verify), + 'SetPlayerPuzzleScores': scores_str + } request = self.make_request(data) response = foldit_ops(request) self.assertEqual(response.status_code, 200) - response_data = json.loads(response.content) - self.assertEqual(response.content, json.dumps([{ "OperationID": "SetPlayerPuzzleScores", @@ -261,7 +258,6 @@ class FolditTestCase(TestCase): "ErrorString": "Verification failed", "ErrorCode": "VerifyFailed"}])) - def make_puzzles_complete_request(self, puzzles): """ Make a puzzles complete request, given an array of @@ -272,11 +268,15 @@ class FolditTestCase(TestCase): """ puzzles_str = json.dumps(puzzles) - verify = {"Verify": verify_code(self.user.email, puzzles_str), - "VerifyMethod":"FoldItVerify"} + verify = { + "Verify": verify_code(self.user.email, puzzles_str), + "VerifyMethod": "FoldItVerify" + } - data = {'SetPuzzlesCompleteVerify': json.dumps(verify), - 'SetPuzzlesComplete': puzzles_str} + data = { + 'SetPuzzlesCompleteVerify': json.dumps(verify), + 'SetPuzzlesComplete': puzzles_str + } request = self.make_request(data) @@ -286,56 +286,64 @@ class FolditTestCase(TestCase): @staticmethod def set_puzzle_complete_response(values): - return json.dumps([{"OperationID":"SetPuzzlesComplete", + """Returns a json response of a Puzzle Complete message""" + return json.dumps([{"OperationID": "SetPuzzlesComplete", "Value": values}]) + def test_SetPlayerPuzzlesComplete(self): # pylint: disable=invalid-name - def test_SetPlayerPuzzlesComplete(self): - - puzzles = [ {"PuzzleID": 13, "Set": 1, "SubSet": 2}, - {"PuzzleID": 53524, "Set": 1, "SubSet": 1} ] + puzzles = [ + {"PuzzleID": 13, "Set": 1, "SubSet": 2}, + {"PuzzleID": 53524, "Set": 1, "SubSet": 1} + ] response = self.make_puzzles_complete_request(puzzles) self.assertEqual(response.content, self.set_puzzle_complete_response([13, 53524])) - - - def test_SetPlayerPuzzlesComplete_multiple(self): + def test_SetPlayerPuzzlesComplete_multiple(self): # pylint: disable=invalid-name """Check that state is stored properly""" - puzzles = [ {"PuzzleID": 13, "Set": 1, "SubSet": 2}, - {"PuzzleID": 53524, "Set": 1, "SubSet": 1} ] + puzzles = [ + {"PuzzleID": 13, "Set": 1, "SubSet": 2}, + {"PuzzleID": 53524, "Set": 1, "SubSet": 1} + ] response = self.make_puzzles_complete_request(puzzles) self.assertEqual(response.content, self.set_puzzle_complete_response([13, 53524])) - puzzles = [ {"PuzzleID": 14, "Set": 1, "SubSet": 3}, - {"PuzzleID": 15, "Set": 1, "SubSet": 1} ] + puzzles = [ + {"PuzzleID": 14, "Set": 1, "SubSet": 3}, + {"PuzzleID": 15, "Set": 1, "SubSet": 1} + ] response = self.make_puzzles_complete_request(puzzles) - self.assertEqual(response.content, - self.set_puzzle_complete_response([13, 14, 15, 53524])) + self.assertEqual( + response.content, + self.set_puzzle_complete_response([13, 14, 15, 53524]) + ) - - - def test_SetPlayerPuzzlesComplete_level_complete(self): + def test_SetPlayerPuzzlesComplete_level_complete(self): # pylint: disable=invalid-name """Check that the level complete function works""" - puzzles = [ {"PuzzleID": 13, "Set": 1, "SubSet": 2}, - {"PuzzleID": 53524, "Set": 1, "SubSet": 1} ] + puzzles = [ + {"PuzzleID": 13, "Set": 1, "SubSet": 2}, + {"PuzzleID": 53524, "Set": 1, "SubSet": 1} + ] response = self.make_puzzles_complete_request(puzzles) self.assertEqual(response.content, self.set_puzzle_complete_response([13, 53524])) - puzzles = [ {"PuzzleID": 14, "Set": 1, "SubSet": 3}, - {"PuzzleID": 15, "Set": 1, "SubSet": 1} ] + puzzles = [ + {"PuzzleID": 14, "Set": 1, "SubSet": 3}, + {"PuzzleID": 15, "Set": 1, "SubSet": 1} + ] response = self.make_puzzles_complete_request(puzzles) @@ -350,7 +358,7 @@ class FolditTestCase(TestCase): self.assertTrue(is_complete(1, 2)) self.assertFalse(is_complete(4, 5)) - puzzles = [ {"PuzzleID": 74, "Set": 4, "SubSet": 5} ] + puzzles = [{"PuzzleID": 74, "Set": 4, "SubSet": 5}] response = self.make_puzzles_complete_request(puzzles) @@ -361,28 +369,30 @@ class FolditTestCase(TestCase): self.assertTrue(is_complete(1, 1, due=self.tomorrow)) self.assertFalse(is_complete(1, 1, due=self.yesterday)) + def test_SetPlayerPuzzlesComplete_error(self): # pylint: disable=invalid-name - - def test_SetPlayerPuzzlesComplete_error(self): - - puzzles = [ {"PuzzleID": 13, "Set": 1, "SubSet": 2}, - {"PuzzleID": 53524, "Set": 1, "SubSet": 1} ] + puzzles = [ + {"PuzzleID": 13, "Set": 1, "SubSet": 2}, + {"PuzzleID": 53524, "Set": 1, "SubSet": 1} + ] puzzles_str = json.dumps(puzzles) - verify = {"Verify": verify_code(self.user.email, puzzles_str + "x"), - "VerifyMethod":"FoldItVerify"} + verify = { + "Verify": verify_code(self.user.email, puzzles_str + "x"), + "VerifyMethod": "FoldItVerify" + } - data = {'SetPuzzlesCompleteVerify': json.dumps(verify), - 'SetPuzzlesComplete': puzzles_str} + data = { + 'SetPuzzlesCompleteVerify': json.dumps(verify), + 'SetPuzzlesComplete': puzzles_str + } request = self.make_request(data) response = foldit_ops(request) self.assertEqual(response.status_code, 200) - response_data = json.loads(response.content) - self.assertEqual(response.content, json.dumps([{ "OperationID": "SetPuzzlesComplete", diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 78f89fa7ec..89379d1cc5 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -3,7 +3,7 @@ Instructor Views """ ## NOTE: This is the code for the legacy instructor dashboard ## We are no longer supporting this file or accepting changes into it. - +# pylint: skip-file from contextlib import contextmanager import csv import json @@ -1427,6 +1427,7 @@ def get_student_grade_summary_data(request, course, get_grades=True, get_raw_sco # Gradebook has moved to instructor.api.spoc_gradebook # + @cache_control(no_cache=True, no_store=True, must_revalidate=True) def grade_summary(request, course_key): """Display the grade summary for a course.""" diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index ea33302de5..b54077df7d 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -24,7 +24,7 @@ from xmodule.modulestore.django import modulestore from course_modes.models import CourseMode from edxmako.shortcuts import render_to_string -from student.models import CourseEnrollment, unenroll_done +from student.models import CourseEnrollment, UNENROLL_DONE from util.query import use_read_replica_if_available from xmodule_django.models import CourseKeyField @@ -639,7 +639,7 @@ class CertificateItem(OrderItem): course_enrollment = models.ForeignKey(CourseEnrollment) mode = models.SlugField() - @receiver(unenroll_done) + @receiver(UNENROLL_DONE) def refund_cert_callback(sender, course_enrollment=None, **kwargs): """ When a CourseEnrollment object calls its unenroll method, this function checks to see if that unenrollment diff --git a/lms/lib/comment_client/comment_client.py b/lms/lib/comment_client/comment_client.py index f747d7e294..0562c9ada1 100644 --- a/lms/lib/comment_client/comment_client.py +++ b/lms/lib/comment_client/comment_client.py @@ -1,5 +1,5 @@ -# Import other classes here so they can be imported from here. -# pylint: disable=W0611 +"""Import other classes here so they can be imported from here.""" +# pylint: disable=unused-import from .comment import Comment from .thread import Thread from .user import User diff --git a/lms/lib/comment_client/commentable.py b/lms/lib/comment_client/commentable.py index 05efd70e50..c9930d4781 100644 --- a/lms/lib/comment_client/commentable.py +++ b/lms/lib/comment_client/commentable.py @@ -1,7 +1,7 @@ +"""Provides base Commentable model class""" import models import settings - class Commentable(models.Model): base_url = "{prefix}/commentables".format(prefix=settings.PREFIX)