Skip to content

E
edx-platform
Commit b0c2b73d by Sarina Canelake
Options

Merge pull request #5112 from edx/sarina/quality-clean 

Quality cleanup
parents a12c7959 467f2988
Showing with 381 additions and 334 deletions
common/djangoapps/student/models.py
...@@ -51,10 +51,10 @@ from ratelimitbackend import admin ...@@ -51,10 +51,10 @@ from ratelimitbackend import admin
import analytics import analytics
unenroll_done = Signal(providing_args=["course_enrollment"]) UNENROLL_DONE = Signal(providing_args=["course_enrollment"])
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
AUDIT_LOG = logging.getLogger("audit") 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): class AnonymousUserId(models.Model):
...@@ -133,7 +133,7 @@ def anonymous_id_for_user(user, course_id, save=True): ...@@ -133,7 +133,7 @@ def anonymous_id_for_user(user, course_id, save=True):
return digest return digest
def user_by_anonymous_id(id): def user_by_anonymous_id(uid):
""" """
Return user by anonymous_user_id using AnonymousUserId lookup table. Return user by anonymous_user_id using AnonymousUserId lookup table.
...@@ -142,11 +142,11 @@ def user_by_anonymous_id(id): ...@@ -142,11 +142,11 @@ def user_by_anonymous_id(id):
because this function will be used inside xmodule w/o django access. because this function will be used inside xmodule w/o django access.
""" """
if id is None: if uid is None:
return None return None
try: try:
return User.objects.get(anonymoususerid__anonymous_user_id=id) return User.objects.get(anonymoususerid__anonymous_user_id=uid)
except ObjectDoesNotExist: except ObjectDoesNotExist:
return None return None
...@@ -192,7 +192,7 @@ class UserProfile(models.Model): ...@@ -192,7 +192,7 @@ class UserProfile(models.Model):
MITx fall prototype. MITx fall prototype.
""" """
class Meta: class Meta: # pylint: disable=missing-docstring
db_table = "auth_userprofile" db_table = "auth_userprofile"
# CRITICAL TODO/SECURITY # CRITICAL TODO/SECURITY
...@@ -250,7 +250,7 @@ class UserProfile(models.Model): ...@@ -250,7 +250,7 @@ class UserProfile(models.Model):
goals = models.TextField(blank=True, null=True) goals = models.TextField(blank=True, null=True)
allow_certificate = models.BooleanField(default=1) allow_certificate = models.BooleanField(default=1)
def get_meta(self): def get_meta(self): # pylint: disable=missing-docstring
js_str = self.meta js_str = self.meta
if not js_str: if not js_str:
js_str = dict() js_str = dict()
...@@ -259,8 +259,8 @@ class UserProfile(models.Model): ...@@ -259,8 +259,8 @@ class UserProfile(models.Model):
return js_str return js_str
def set_meta(self, js): def set_meta(self, meta_json): # pylint: disable=missing-docstring
self.meta = json.dumps(js) self.meta = json.dumps(meta_json)
def set_login_session(self, session_id=None): def set_login_session(self, session_id=None):
""" """
...@@ -723,7 +723,7 @@ class CourseEnrollment(models.Model): ...@@ -723,7 +723,7 @@ class CourseEnrollment(models.Model):
) )
else: else:
unenroll_done.send(sender=None, course_enrollment=self) UNENROLL_DONE.send(sender=None, course_enrollment=self)
self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED) self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
...@@ -961,12 +961,12 @@ class CourseEnrollment(models.Model): ...@@ -961,12 +961,12 @@ class CourseEnrollment(models.Model):
# Unfortunately, Django's "group by"-style queries look super-awkward # 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'))) 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 total = 0
d = defaultdict(int) enroll_dict = defaultdict(int)
for item in query: for item in query:
d[item['mode']] = item['mode__count'] enroll_dict[item['mode']] = item['mode__count']
total += item['mode__count'] total += item['mode__count']
d['total'] = total enroll_dict['total'] = total
return d return enroll_dict
def is_paid_course(self): def is_paid_course(self):
""" """
...@@ -999,7 +999,7 @@ class CourseEnrollment(models.Model): ...@@ -999,7 +999,7 @@ class CourseEnrollment(models.Model):
a verified certificate and the deadline for refunds has not yet passed. 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. # 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. # 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. # This can't be set directly because refunds currently happen as a side-effect of unenrolling.
# (side-effects are bad) # (side-effects are bad)
...@@ -1031,7 +1031,7 @@ class CourseEnrollmentAllowed(models.Model): ...@@ -1031,7 +1031,7 @@ class CourseEnrollmentAllowed(models.Model):
created = models.DateTimeField(auto_now_add=True, null=True, db_index=True) 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'),) unique_together = (('email', 'course_id'),)
def __unicode__(self): def __unicode__(self):
...@@ -1055,7 +1055,7 @@ class CourseAccessRole(models.Model): ...@@ -1055,7 +1055,7 @@ class CourseAccessRole(models.Model):
course_id = CourseKeyField(max_length=255, db_index=True, blank=True) course_id = CourseKeyField(max_length=255, db_index=True, blank=True)
role = models.CharField(max_length=64, db_index=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') unique_together = ('user', 'org', 'course_id', 'role')
@property @property
...@@ -1071,7 +1071,7 @@ class CourseAccessRole(models.Model): ...@@ -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 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. 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): def __hash__(self):
return hash(self._key) return hash(self._key)
...@@ -1080,7 +1080,7 @@ class CourseAccessRole(models.Model): ...@@ -1080,7 +1080,7 @@ class CourseAccessRole(models.Model):
""" """
Lexigraphic sort Lexigraphic sort
""" """
return self._key < other._key return self._key < other._key # pylint: disable=protected-access
def __unicode__(self): def __unicode__(self):
return "[CourseAccessRole] user: {} role: {} org: {} course: {}".format(self.user.username, self.role, self.org, self.course_id) 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): ...@@ -1107,32 +1107,32 @@ def get_user_by_username_or_email(username_or_email):
def get_user(email): def get_user(email):
u = User.objects.get(email=email) user = User.objects.get(email=email)
up = UserProfile.objects.get(user=u) u_prof = UserProfile.objects.get(user=user)
return u, up return user, u_prof
def user_info(email): def user_info(email):
u, up = get_user(email) user, u_prof = get_user(email)
print "User id", u.id print "User id", user.id
print "Username", u.username print "Username", user.username
print "E-mail", u.email print "E-mail", user.email
print "Name", up.name print "Name", u_prof.name
print "Location", up.location print "Location", u_prof.location
print "Language", up.language print "Language", u_prof.language
return u, up return user, u_prof
def change_email(old_email, new_email): def change_email(old_email, new_email):
u = User.objects.get(email=old_email) user = User.objects.get(email=old_email)
u.email = new_email user.email = new_email
u.save() user.save()
def change_name(email, new_name): def change_name(email, new_name):
u, up = get_user(email) _user, u_prof = get_user(email)
up.name = new_name u_prof.name = new_name
up.save() u_prof.save()
def user_count(): def user_count():
...@@ -1163,10 +1163,12 @@ def remove_user_from_group(user, group): ...@@ -1163,10 +1163,12 @@ def remove_user_from_group(user, group):
utg.users.remove(User.objects.get(username=user)) utg.users.remove(User.objects.get(username=user))
utg.save() utg.save()
default_groups = {'email_future_courses': 'Receive e-mails about future MITx courses', DEFAULT_GROUPS = {
'email_future_courses': 'Receive e-mails about future MITx courses',
'email_helpers': 'Receive e-mails about how to help with MITx', 'email_helpers': 'Receive e-mails about how to help with MITx',
'mitx_unenroll': 'Fully unenrolled -- no further communications', 'mitx_unenroll': 'Fully unenrolled -- no further communications',
'6002x_unenroll': 'Took and dropped 6002x'} '6002x_unenroll': 'Took and dropped 6002x'
}
def add_user_to_default_group(user, group): def add_user_to_default_group(user, group):
...@@ -1175,7 +1177,7 @@ def add_user_to_default_group(user, group): ...@@ -1175,7 +1177,7 @@ def add_user_to_default_group(user, group):
except UserTestGroup.DoesNotExist: except UserTestGroup.DoesNotExist:
utg = UserTestGroup() utg = UserTestGroup()
utg.name = group utg.name = group
utg.description = default_groups[group] utg.description = DEFAULT_GROUPS[group]
utg.save() utg.save()
utg.users.add(User.objects.get(username=user)) utg.users.add(User.objects.get(username=user))
utg.save() utg.save()
...@@ -1188,11 +1190,12 @@ def create_comments_service_user(user): ...@@ -1188,11 +1190,12 @@ def create_comments_service_user(user):
try: try:
cc_user = cc.User.from_django_user(user) cc_user = cc.User.from_django_user(user)
cc_user.save() cc_user.save()
except Exception as e: except Exception: # pylint: disable=broad-except
log = logging.getLogger("edx.discussion") log = logging.getLogger("edx.discussion") # pylint: disable=redefined-outer-name
log.error( log.error(
"Could not create comments service user with id {}".format(user.id), "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, # 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 # 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): ...@@ -1201,7 +1204,7 @@ def create_comments_service_user(user):
@receiver(user_logged_in) @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.""" """Handler to log when logins have occurred successfully."""
if settings.FEATURES['SQUELCH_PII_IN_LOGS']: if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
AUDIT_LOG.info(u"Login success - user.id: {0}".format(user.id)) AUDIT_LOG.info(u"Login success - user.id: {0}".format(user.id))
...@@ -1210,7 +1213,7 @@ def log_successful_login(sender, request, user, **kwargs): ...@@ -1210,7 +1213,7 @@ def log_successful_login(sender, request, user, **kwargs):
@receiver(user_logged_out) @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.""" """Handler to log when logouts have occurred successfully."""
if settings.FEATURES['SQUELCH_PII_IN_LOGS']: if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
AUDIT_LOG.info(u"Logout - user.id: {0}".format(request.user.id)) AUDIT_LOG.info(u"Logout - user.id: {0}".format(request.user.id))
...@@ -1220,7 +1223,7 @@ def log_successful_logout(sender, request, user, **kwargs): ...@@ -1220,7 +1223,7 @@ def log_successful_logout(sender, request, user, **kwargs):
@receiver(user_logged_in) @receiver(user_logged_in)
@receiver(user_logged_out) @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, Sets the current session id in the user profile,
to prevent concurrent logins. to prevent concurrent logins.
......
common/djangoapps/student/views.py
...@@ -46,8 +46,7 @@ from student.models import ( ...@@ -46,8 +46,7 @@ from student.models import (
Registration, UserProfile, PendingNameChange, Registration, UserProfile, PendingNameChange,
PendingEmailChange, CourseEnrollment, unique_id_for_user, PendingEmailChange, CourseEnrollment, unique_id_for_user,
CourseEnrollmentAllowed, UserStanding, LoginFailures, CourseEnrollmentAllowed, UserStanding, LoginFailures,
create_comments_service_user, PasswordHistory, UserSignupSource, create_comments_service_user, PasswordHistory, UserSignupSource
anonymous_id_for_user
) )
from student.forms import PasswordResetFormNoActive from student.forms import PasswordResetFormNoActive
...@@ -103,27 +102,29 @@ AUDIT_LOG = logging.getLogger("audit") ...@@ -103,27 +102,29 @@ AUDIT_LOG = logging.getLogger("audit")
ReverifyInfo = namedtuple('ReverifyInfo', 'course_id course_name course_number date status display') # pylint: disable=C0103 ReverifyInfo = namedtuple('ReverifyInfo', 'course_id course_name course_number date status display') # pylint: disable=C0103
def csrf_token(context): def csrf_token(context):
"""A csrf token that can be included in a form.""" """A csrf token that can be included in a form."""
csrf_token = context.get('csrf_token', '') token = context.get('csrf_token', '')
if csrf_token == 'NOTPROVIDED': if token == 'NOTPROVIDED':
return '' return ''
return (u'<div style="display:none"><input type="hidden"' return (u'<div style="display:none"><input type="hidden"'
' name="csrfmiddlewaretoken" value="%s" /></div>' % (csrf_token)) ' name="csrfmiddlewaretoken" value="%s" /></div>' % (token))
# NOTE: This view is not linked to directly--it is called from # NOTE: This view is not linked to directly--it is called from
# branding/views.py:index(), which is cached for anonymous users. # branding/views.py:index(), which is cached for anonymous users.
# This means that it should always return the same thing for anon # This means that it should always return the same thing for anon
# users. (in particular, no switching based on query params allowed) # 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. Render the edX main page.
extra_context is used to allow immediate display of certain modal windows, eg signup, extra_context is used to allow immediate display of certain modal windows, eg signup,
as used by external_auth. as used by external_auth.
""" """
if extra_context is None:
extra_context = {}
# The course selection work is done in courseware.courses. # The course selection work is done in courseware.courses.
domain = settings.FEATURES.get('FORCE_UNIVERSITY_DOMAIN') # normally False domain = settings.FEATURES.get('FORCE_UNIVERSITY_DOMAIN') # normally False
# do explicit check, because domain=None is valid # do explicit check, because domain=None is valid
...@@ -294,18 +295,20 @@ 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) status = template_state.get(cert_status['status'], default_status)
d = {'status': status, status_dict = {
'status': status,
'show_download_url': status == 'ready', 'show_download_url': status == 'ready',
'show_disabled_download_button': status == 'generating', 'show_disabled_download_button': status == 'generating',
'mode': cert_status.get('mode', None)} 'mode': cert_status.get('mode', None)
}
if (status in ('generating', 'ready', 'notpassing', 'restricted') and if (status in ('generating', 'ready', 'notpassing', 'restricted') and
course.end_of_course_survey_url is not None): course.end_of_course_survey_url is not None):
d.update({ status_dict.update({
'show_survey_button': True, 'show_survey_button': True,
'survey_url': process_survey_link(course.end_of_course_survey_url, user)}) 'survey_url': process_survey_link(course.end_of_course_survey_url, user)})
else: else:
d['show_survey_button'] = False status_dict['show_survey_button'] = False
if status == 'ready': if status == 'ready':
if 'download_url' not in cert_status: if 'download_url' not in cert_status:
...@@ -313,7 +316,7 @@ def _cert_info(user, course, cert_status): ...@@ -313,7 +316,7 @@ def _cert_info(user, course, cert_status):
user.username, course.id) user.username, course.id)
return default_info return default_info
else: else:
d['download_url'] = cert_status['download_url'] status_dict['download_url'] = cert_status['download_url']
if status in ('generating', 'ready', 'notpassing', 'restricted'): if status in ('generating', 'ready', 'notpassing', 'restricted'):
if 'grade' not in cert_status: if 'grade' not in cert_status:
...@@ -322,9 +325,9 @@ def _cert_info(user, course, 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. # We can add a log.warning here once we think it shouldn't happen.
return default_info return default_info
else: else:
d['grade'] = cert_status['grade'] status_dict['grade'] = cert_status['grade']
return d return status_dict
@ensure_csrf_cookie @ensure_csrf_cookie
...@@ -446,6 +449,7 @@ def is_course_blocked(request, redeemed_registration_codes, course_key): ...@@ -446,6 +449,7 @@ def is_course_blocked(request, redeemed_registration_codes, course_key):
return blocked return blocked
@login_required @login_required
@ensure_csrf_cookie @ensure_csrf_cookie
def dashboard(request): def dashboard(request):
...@@ -511,7 +515,6 @@ def dashboard(request): ...@@ -511,7 +515,6 @@ def dashboard(request):
block_courses = frozenset(course.id for course, enrollment in course_enrollment_pairs 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)) 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 enrolled_courses_either_paid = frozenset(course.id for course, _enrollment in course_enrollment_pairs
if _enrollment.is_paid_course()) if _enrollment.is_paid_course())
# get info w.r.t ExternalAuthMap # get info w.r.t ExternalAuthMap
...@@ -608,8 +611,8 @@ def try_change_enrollment(request): ...@@ -608,8 +611,8 @@ def try_change_enrollment(request):
# will return redirect_urls. # will return redirect_urls.
if enrollment_response.status_code == 200 and enrollment_response.content != '': if enrollment_response.status_code == 200 and enrollment_response.content != '':
return enrollment_response.content return enrollment_response.content
except Exception, e: except Exception as exc: # pylint: disable=broad-except
log.exception("Exception automatically enrolling after login: {0}".format(str(e))) log.exception("Exception automatically enrolling after login: %s", exc)
@require_POST @require_POST
...@@ -771,7 +774,6 @@ def change_enrollment(request, auto_register=False): ...@@ -771,7 +774,6 @@ def change_enrollment(request, auto_register=False):
return HttpResponse() return HttpResponse()
elif action == "add_to_cart": elif action == "add_to_cart":
# Pass the request handling to shoppingcart.views # Pass the request handling to shoppingcart.views
# The view in shoppingcart.views performs error handling and logs different errors. But this elif clause # 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 ...@@ -885,12 +887,17 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un
username=username, backend_name=backend_name)) username=username, backend_name=backend_name))
return HttpResponseBadRequest( return HttpResponseBadRequest(
_("You've successfully logged into your {provider_name} account, but this account isn't linked with an {platform_name} account yet.").format( _("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) platform_name=settings.PLATFORM_NAME, provider_name=requested_provider.NAME
+ "<br/><br/>" + _("Use your {platform_name} username and password to log into {platform_name} below, " )
+ "<br/><br/>" +
_("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( "and then link your {platform_name} account with {provider_name} from your dashboard.").format(
platform_name=settings.PLATFORM_NAME, provider_name=requested_provider.NAME) platform_name=settings.PLATFORM_NAME, provider_name=requested_provider.NAME
+ "<br/><br/>" + _("If you don't have an {platform_name} account yet, click <strong>Register Now</strong> at the top of the page.").format( )
platform_name=settings.PLATFORM_NAME), + "<br/><br/>" +
_("If you don't have an {platform_name} account yet, click <strong>Register Now</strong> at the top of the page.").format(
platform_name=settings.PLATFORM_NAME
),
content_type="text/plain", content_type="text/plain",
status=401 status=401
) )
...@@ -1018,10 +1025,10 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un ...@@ -1018,10 +1025,10 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un
log.debug("Setting user session to never expire") log.debug("Setting user session to never expire")
else: else:
request.session.set_expiry(0) 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?") AUDIT_LOG.critical("Login failed - Could not create session. Is memcached running?")
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 raise
redirect_url = try_change_enrollment(request) redirect_url = try_change_enrollment(request)
...@@ -1170,14 +1177,14 @@ def disable_account_ajax(request): ...@@ -1170,14 +1177,14 @@ def disable_account_ajax(request):
def change_setting(request): def change_setting(request):
"""JSON call to change a profile setting: Right now, location""" """JSON call to change a profile setting: Right now, location"""
# TODO (vshnayder): location is no longer used # 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: if 'location' in request.POST:
up.location = request.POST['location'] u_prof.location = request.POST['location']
up.save() u_prof.save()
return JsonResponse({ return JsonResponse({
"success": True, "success": True,
"location": up.location, "location": u_prof.location,
}) })
...@@ -1263,7 +1270,7 @@ def _do_create_account(post_vars, extended_profile=None): ...@@ -1263,7 +1270,7 @@ def _do_create_account(post_vars, extended_profile=None):
profile.year_of_birth = None profile.year_of_birth = None
try: try:
profile.save() profile.save()
except Exception: except Exception: # pylint: disable=broad-except
log.exception("UserProfile creation failed for user {id}.".format(id=user.id)) log.exception("UserProfile creation failed for user {id}.".format(id=user.id))
raise raise
...@@ -1295,8 +1302,8 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many ...@@ -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 # 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 # 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 # unless originally we didn't get a valid email or name from the external auth
DoExternalAuth = 'ExternalAuthMap' in request.session do_external_auth = 'ExternalAuthMap' in request.session
if DoExternalAuth: if do_external_auth:
eamap = request.session['ExternalAuthMap'] eamap = request.session['ExternalAuthMap']
try: try:
validate_email(eamap.external_email) validate_email(eamap.external_email)
...@@ -1313,15 +1320,15 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many ...@@ -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) log.debug(u'In create_account with external_auth: user = %s, email=%s', name, email)
# Confirm we have a properly formed request # Confirm we have a properly formed request
for a in ['username', 'email', 'password', 'name']: for req_field in ['username', 'email', 'password', 'name']:
if a not in post_vars: if req_field not in post_vars:
js['value'] = _("Error (401 {field}). E-mail us.").format(field=a) js['value'] = _("Error (401 {field}). E-mail us.").format(field=req_field)
js['field'] = a js['field'] = req_field
return JsonResponse(js, status=400) return JsonResponse(js, status=400)
if extra_fields.get('honor_code', 'required') == 'required' and \ if extra_fields.get('honor_code', 'required') == 'required' and \
post_vars.get('honor_code', 'false') != u'true': 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' js['field'] = 'honor_code'
return JsonResponse(js, status=400) return JsonResponse(js, status=400)
...@@ -1329,7 +1336,7 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many ...@@ -1329,7 +1336,7 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many
tos_required = ( tos_required = (
not settings.FEATURES.get("AUTH_USE_SHIB") or not settings.FEATURES.get("AUTH_USE_SHIB") or
not settings.FEATURES.get("SHIB_DISABLE_TOS") or not settings.FEATURES.get("SHIB_DISABLE_TOS") or
not DoExternalAuth or not do_external_auth or
not eamap.external_domain.startswith( not eamap.external_domain.startswith(
external_auth.views.SHIBBOLETH_DOMAIN_PREFIX external_auth.views.SHIBBOLETH_DOMAIN_PREFIX
) )
...@@ -1337,7 +1344,7 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many ...@@ -1337,7 +1344,7 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many
if tos_required: if tos_required:
if post_vars.get('terms_of_service', 'false') != u'true': 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' js['field'] = 'terms_of_service'
return JsonResponse(js, status=400) return JsonResponse(js, status=400)
...@@ -1390,8 +1397,8 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many ...@@ -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: if field_name in ('email', 'username') and len(post_vars[field_name]) > max_length:
error_str = { error_str = {
'username': _('Username 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 {0} characters long').format(max_length) 'email': _('Email cannot be more than {num} characters long').format(num=max_length)
} }
js['value'] = error_str[field_name] js['value'] = error_str[field_name]
js['field'] = field_name js['field'] = field_name
...@@ -1400,20 +1407,20 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many ...@@ -1400,20 +1407,20 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many
try: try:
validate_email(post_vars['email']) validate_email(post_vars['email'])
except ValidationError: except ValidationError:
js['value'] = _("Valid e-mail is required.").format(field=a) js['value'] = _("Valid e-mail is required.")
js['field'] = 'email' js['field'] = 'email'
return JsonResponse(js, status=400) return JsonResponse(js, status=400)
try: try:
validate_slug(post_vars['username']) validate_slug(post_vars['username'])
except ValidationError: 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' js['field'] = 'username'
return JsonResponse(js, status=400) return JsonResponse(js, status=400)
# enforce password complexity as an optional feature # 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 # 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: try:
password = post_vars['password'] password = post_vars['password']
...@@ -1449,8 +1456,8 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many ...@@ -1449,8 +1456,8 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many
try: try:
with transaction.commit_on_success(): with transaction.commit_on_success():
ret = _do_create_account(post_vars, extended_profile) ret = _do_create_account(post_vars, extended_profile)
except AccountValidationError as e: except AccountValidationError as exc:
return JsonResponse({'success': False, 'value': e.message, 'field': e.field}, status=400) return JsonResponse({'success': False, 'value': exc.message, 'field': exc.field}, status=400)
(user, profile, registration) = ret (user, profile, registration) = ret
...@@ -1520,17 +1527,17 @@ def create_account(request, post_override=None): # pylint: disable-msg=too-many ...@@ -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 # 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 # logged in until they close the browser. They can't log in again until they click
# the activation link from the email. # the activation link from the email.
login_user = authenticate(username=post_vars['username'], password=post_vars['password']) new_user = authenticate(username=post_vars['username'], password=post_vars['password'])
login(request, login_user) login(request, new_user)
request.session.set_expiry(0) request.session.set_expiry(0)
# TODO: there is no error checking here to see that the user actually logged in successfully, # TODO: there is no error checking here to see that the user actually logged in successfully,
# and is not yet an active user. # and is not yet an active user.
if login_user is not None: if new_user is not None:
AUDIT_LOG.info(u"Login success on new account creation - {0}".format(login_user.username)) AUDIT_LOG.info(u"Login success on new account creation - {0}".format(new_user.username))
if DoExternalAuth: if do_external_auth:
eamap.user = login_user eamap.user = new_user
eamap.dtsignup = datetime.datetime.now(UTC) eamap.dtsignup = datetime.datetime.now(UTC)
eamap.save() eamap.save()
AUDIT_LOG.info("User registered with external_auth %s", post_vars['username']) 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 ...@@ -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'): if settings.FEATURES.get('BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'):
log.info('bypassing activation email') log.info('bypassing activation email')
login_user.is_active = True new_user.is_active = True
login_user.save() new_user.save()
AUDIT_LOG.info(u"Login activated on extauth account - {0} ({1})".format(login_user.username, login_user.email)) 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") dog_stats_api.increment("common.student.account_created")
redirect_url = try_change_enrollment(request) redirect_url = try_change_enrollment(request)
...@@ -1623,7 +1630,7 @@ def auto_auth(request): ...@@ -1623,7 +1630,7 @@ def auto_auth(request):
# If successful, this will return a tuple containing # If successful, this will return a tuple containing
# the new user object. # the new user object.
try: try:
user, profile, reg = _do_create_account(post_data) user, _profile, reg = _do_create_account(post_data)
except AccountValidationError: except AccountValidationError:
# Attempt to retrieve the existing user. # Attempt to retrieve the existing user.
user = User.objects.get(username=username) user = User.objects.get(username=username)
...@@ -1669,16 +1676,16 @@ def auto_auth(request): ...@@ -1669,16 +1676,16 @@ def auto_auth(request):
@ensure_csrf_cookie @ensure_csrf_cookie
def activate_account(request, key): def activate_account(request, key):
"""When link in activation e-mail is clicked""" """When link in activation e-mail is clicked"""
r = Registration.objects.filter(activation_key=key) regs = Registration.objects.filter(activation_key=key)
if len(r) == 1: if len(regs) == 1:
user_logged_in = request.user.is_authenticated() user_logged_in = request.user.is_authenticated()
already_active = True already_active = True
if not r[0].user.is_active: if not regs[0].user.is_active:
r[0].activate() regs[0].activate()
already_active = False already_active = False
# Enroll student in any pending courses he/she may have if auto_enroll flag is set # 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: if student:
ceas = CourseEnrollmentAllowed.objects.filter(email=student[0].email) ceas = CourseEnrollmentAllowed.objects.filter(email=student[0].email)
for cea in ceas: for cea in ceas:
...@@ -1693,7 +1700,7 @@ def activate_account(request, key): ...@@ -1693,7 +1700,7 @@ def activate_account(request, key):
} }
) )
return resp return resp
if len(r) == 0: if len(regs) == 0:
return render_to_response( return render_to_response(
"registration/activation_invalid.html", "registration/activation_invalid.html",
{'csrf': csrf(request)['csrf_token']} {'csrf': csrf(request)['csrf_token']}
...@@ -1928,8 +1935,9 @@ def change_email_request(request): ...@@ -1928,8 +1935,9 @@ def change_email_request(request):
@ensure_csrf_cookie @ensure_csrf_cookie
@transaction.commit_manually @transaction.commit_manually
def confirm_email_change(request, key): def confirm_email_change(request, key): # pylint: disable=unused-argument
""" User requested a new e-mail. This is called when the activation """
User requested a new e-mail. This is called when the activation
link is clicked. We confirm with the old e-mail, and update link is clicked. We confirm with the old e-mail, and update
""" """
try: try:
...@@ -1954,17 +1962,17 @@ def confirm_email_change(request, key): ...@@ -1954,17 +1962,17 @@ def confirm_email_change(request, key):
subject = render_to_string('emails/email_change_subject.txt', address_context) subject = render_to_string('emails/email_change_subject.txt', address_context)
subject = ''.join(subject.splitlines()) subject = ''.join(subject.splitlines())
message = render_to_string('emails/confirm_email_change.txt', address_context) message = render_to_string('emails/confirm_email_change.txt', address_context)
up = UserProfile.objects.get(user=user) u_prof = UserProfile.objects.get(user=user)
meta = up.get_meta() meta = u_prof.get_meta()
if 'old_emails' not in meta: if 'old_emails' not in meta:
meta['old_emails'] = [] meta['old_emails'] = []
meta['old_emails'].append([user.email, datetime.datetime.now(UTC).isoformat()]) meta['old_emails'].append([user.email, datetime.datetime.now(UTC).isoformat()])
up.set_meta(meta) u_prof.set_meta(meta)
up.save() u_prof.save()
# Send it to the old email... # Send it to the old email...
try: try:
user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL) 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) log.warning('Unable to send confirmation email to old address', exc_info=True)
response = render_to_response("email_change_failed.html", {'email': user.email}) response = render_to_response("email_change_failed.html", {'email': user.email})
transaction.rollback() transaction.rollback()
...@@ -1976,7 +1984,7 @@ def confirm_email_change(request, key): ...@@ -1976,7 +1984,7 @@ def confirm_email_change(request, key):
# And send it to the new email... # And send it to the new email...
try: try:
user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL) 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) 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}) response = render_to_response("email_change_failed.html", {'email': pec.new_email})
transaction.rollback() transaction.rollback()
...@@ -1985,7 +1993,7 @@ def confirm_email_change(request, key): ...@@ -1985,7 +1993,7 @@ def confirm_email_change(request, key):
response = render_to_response("email_change_successful.html", address_context) response = render_to_response("email_change_successful.html", address_context)
transaction.commit() transaction.commit()
return response return response
except Exception: except Exception: # pylint: disable=broad-except
# If we get an unexpected exception, be sure to rollback the transaction # If we get an unexpected exception, be sure to rollback the transaction
transaction.rollback() transaction.rollback()
raise raise
...@@ -2058,27 +2066,31 @@ def reject_name_change(request): ...@@ -2058,27 +2066,31 @@ def reject_name_change(request):
return JsonResponse({"success": True}) 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: try:
pnc = PendingNameChange.objects.get(id=id) pnc = PendingNameChange.objects.get(id=uid)
except PendingNameChange.DoesNotExist: except PendingNameChange.DoesNotExist:
return JsonResponse({ return JsonResponse({
"success": False, "success": False,
"error": _('Invalid ID'), "error": _('Invalid ID'),
}) # TODO: this should be status code 400 # pylint: disable=fixme }) # TODO: this should be status code 400 # pylint: disable=fixme
u = pnc.user user = pnc.user
up = UserProfile.objects.get(user=u) u_prof = UserProfile.objects.get(user=user)
# Save old name # Save old name
meta = up.get_meta() meta = u_prof.get_meta()
if 'old_names' not in meta: if 'old_names' not in meta:
meta['old_names'] = [] meta['old_names'] = []
meta['old_names'].append([up.name, pnc.rationale, datetime.datetime.now(UTC).isoformat()]) meta['old_names'].append([u_prof.name, pnc.rationale, datetime.datetime.now(UTC).isoformat()])
up.set_meta(meta) u_prof.set_meta(meta)
up.name = pnc.new_name u_prof.name = pnc.new_name
up.save() u_prof.save()
pnc.delete() pnc.delete()
return JsonResponse({"success": True}) return JsonResponse({"success": True})
......
common/lib/capa/capa/tests/test_html_render.py
...@@ -26,8 +26,9 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -26,8 +26,9 @@ class CapaHtmlRenderTest(unittest.TestCase):
problem = new_loncapa_problem(xml_str) problem = new_loncapa_problem(xml_str)
# Render the HTML # Render the HTML
rendered_html = etree.XML(problem.get_html()) etree.XML(problem.get_html())
# expect that we made it here without blowing up # expect that we made it here without blowing up
self.assertTrue(True)
def test_include_html(self): def test_include_html(self):
# Create a test file to include # Create a test file to include
...@@ -123,17 +124,17 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -123,17 +124,17 @@ class CapaHtmlRenderTest(unittest.TestCase):
# Render the HTML # Render the HTML
rendered_html = etree.XML(problem.get_html()) rendered_html = etree.XML(problem.get_html())
# expect the javascript is still present in the rendered html # expect the javascript is still present in the rendered html
self.assertTrue("<script type=\"text/javascript\">function(){}</script>" in etree.tostring(rendered_html)) self.assertTrue("<script type=\"text/javascript\">function(){}</script>" in etree.tostring(rendered_html))
def test_render_response_xml(self): def test_render_response_xml(self):
# Generate some XML for a string response # Generate some XML for a string response
kwargs = {'question_text': "Test question", kwargs = {
'question_text': "Test question",
'explanation_text': "Test explanation", 'explanation_text': "Test explanation",
'answer': 'Test answer', 'answer': 'Test answer',
'hints': [('test prompt', 'test_hint', 'test hint text')]} 'hints': [('test prompt', 'test_hint', 'test hint text')]
}
xml_str = StringResponseXMLFactory().build_xml(**kwargs) xml_str = StringResponseXMLFactory().build_xml(**kwargs)
# Mock out the template renderer # Mock out the template renderer
...@@ -186,18 +187,21 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -186,18 +187,21 @@ class CapaHtmlRenderTest(unittest.TestCase):
expected_solution_context = {'id': '1_solution_1'} expected_solution_context = {'id': '1_solution_1'}
expected_calls = [mock.call('textline.html', expected_textline_context), expected_calls = [
mock.call('textline.html', expected_textline_context),
mock.call('solutionspan.html', expected_solution_context), mock.call('solutionspan.html', expected_solution_context),
mock.call('textline.html', expected_textline_context), mock.call('textline.html', expected_textline_context),
mock.call('solutionspan.html', expected_solution_context)] mock.call('solutionspan.html', expected_solution_context)
]
self.assertEqual(the_system.render_template.call_args_list,
expected_calls)
self.assertEqual(
the_system.render_template.call_args_list,
expected_calls
)
def test_render_response_with_overall_msg(self): def test_render_response_with_overall_msg(self):
# CustomResponse script that sets an overall_message # CustomResponse script that sets an overall_message
script=textwrap.dedent(""" script = textwrap.dedent("""
def check_func(*args): def check_func(*args):
msg = '<p>Test message 1<br /></p><p>Test message 2</p>' msg = '<p>Test message 1<br /></p><p>Test message 2</p>'
return {'overall_message': msg, return {'overall_message': msg,
...@@ -205,19 +209,18 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -205,19 +209,18 @@ class CapaHtmlRenderTest(unittest.TestCase):
""") """)
# Generate some XML for a CustomResponse # Generate some XML for a CustomResponse
kwargs = {'script':script, 'cfn': 'check_func'} kwargs = {'script': script, 'cfn': 'check_func'}
xml_str = CustomResponseXMLFactory().build_xml(**kwargs) xml_str = CustomResponseXMLFactory().build_xml(**kwargs)
# Create the problem and render the html # Create the problem and render the html
problem = new_loncapa_problem(xml_str) problem = new_loncapa_problem(xml_str)
# Grade the problem # Grade the problem
correctmap = problem.grade_answers({'1_2_1': 'test'}) problem.grade_answers({'1_2_1': 'test'})
# Render the html # Render the html
rendered_html = etree.XML(problem.get_html()) rendered_html = etree.XML(problem.get_html())
# Expect that there is a <div> within the response <div> # Expect that there is a <div> within the response <div>
# with css class response_message # with css class response_message
msg_div_element = rendered_html.find(".//div[@class='response_message']") msg_div_element = rendered_html.find(".//div[@class='response_message']")
...@@ -232,7 +235,6 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -232,7 +235,6 @@ class CapaHtmlRenderTest(unittest.TestCase):
self.assertEqual(msg_p_elements[1].tag, "p") self.assertEqual(msg_p_elements[1].tag, "p")
self.assertEqual(msg_p_elements[1].text, "Test message 2") self.assertEqual(msg_p_elements[1].text, "Test message 2")
def test_substitute_python_vars(self): def test_substitute_python_vars(self):
# Generate some XML with Python variables defined in a script # Generate some XML with Python variables defined in a script
# and used later as attributes # and used later as attributes
......
common/lib/i18n/tests/test_extract_and_generate.py
"""
This test tests that i18n extraction (`paver i18n_extract -v`) works properly.
"""
from datetime import datetime, timedelta from datetime import datetime, timedelta
import os import os
import sys import sys
import string import string # pylint: disable=deprecated-module
import random import random
import re import re
...@@ -65,12 +68,14 @@ class TestGenerate(TestCase): ...@@ -65,12 +68,14 @@ class TestGenerate(TestCase):
generate.main(verbosity=0, strict=False) generate.main(verbosity=0, strict=False)
for locale in CONFIGURATION.translated_locales: for locale in CONFIGURATION.translated_locales:
for filename in ('django', 'djangojs'): for filename in ('django', 'djangojs'):
mofile = filename+'.mo' mofile = filename + '.mo'
path = os.path.join(CONFIGURATION.get_messages_dir(locale), mofile) path = os.path.join(CONFIGURATION.get_messages_dir(locale), mofile)
exists = os.path.exists(path) exists = os.path.exists(path)
self.assertTrue(exists, msg='Missing file in locale %s: %s' % (locale, mofile)) self.assertTrue(exists, msg='Missing file in locale %s: %s' % (locale, mofile))
self.assertTrue(datetime.fromtimestamp(os.path.getmtime(path), UTC) >= self.start_time, self.assertTrue(
msg='File not recently modified: %s' % path) 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 # 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 # 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. # get the merge header back eventually, or delete this code eventually.
...@@ -88,12 +93,14 @@ class TestGenerate(TestCase): ...@@ -88,12 +93,14 @@ class TestGenerate(TestCase):
""" """
path = os.path.join(CONFIGURATION.get_messages_dir(locale), 'django.po') path = os.path.join(CONFIGURATION.get_messages_dir(locale), 'django.po')
po = pofile(path) pof = pofile(path)
pattern = re.compile('^#-#-#-#-#', re.M) pattern = re.compile('^#-#-#-#-#', re.M)
match = pattern.findall(po.header) match = pattern.findall(pof.header)
self.assertEqual(len(match), 3, self.assertEqual(
msg="Found %s (should be 3) merge comments in the header for %s" % \ len(match),
(len(match), path)) 3,
msg="Found %s (should be 3) merge comments in the header for %s" % (len(match), path)
)
def random_name(size=6): def random_name(size=6):
......
common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
...@@ -134,8 +134,6 @@ def xml_store_config(data_dir): ...@@ -134,8 +134,6 @@ def xml_store_config(data_dir):
return store return store
class ModuleStoreTestCase(TestCase): class ModuleStoreTestCase(TestCase):
""" """
Subclass for any test case that uses a ModuleStore. Subclass for any test case that uses a ModuleStore.
...@@ -282,19 +280,23 @@ class ModuleStoreTestCase(TestCase): ...@@ -282,19 +280,23 @@ class ModuleStoreTestCase(TestCase):
# Call superclass implementation # Call superclass implementation
super(ModuleStoreTestCase, self)._post_teardown() 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 create a course in the default modulestore from the collection of BlockInfo
records defining the course tree records defining the course tree
Returns: Returns:
course_loc: the CourseKey for the created course 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.branch_setting(ModuleStoreEnum.Branch.draft_preferred, None):
# with self.store.bulk_operations(self.store.make_course_key(org, course, run)): # 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) course = self.store.create_course(org, course, run, self.user.id, fields=course_fields)
self.course_loc = course.location self.course_loc = course.location # pylint: disable=attribute-defined-outside-init
def create_sub_tree(parent_loc, block_info): 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( block = self.store.create_child(
self.user.id, self.user.id,
# TODO remove version_agnostic() when we impl the single transaction # TODO remove version_agnostic() when we impl the single transaction
...@@ -318,14 +320,14 @@ class ModuleStoreTestCase(TestCase): ...@@ -318,14 +320,14 @@ class ModuleStoreTestCase(TestCase):
Create an equivalent to the toy xml course Create an equivalent to the toy xml course
""" """
# with self.store.bulk_operations(self.store.make_course_key(org, course, run)): # 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, org, course, run, TOY_BLOCK_INFO_TREE,
{ {
"textbooks" : [["Textbook", "https://s3.amazonaws.com/edx-textbooks/guttag_computation_v3/"]], "textbooks": [["Textbook", "https://s3.amazonaws.com/edx-textbooks/guttag_computation_v3/"]],
"wiki_slug" : "toy", "wiki_slug": "toy",
"display_name" : "Toy Course", "display_name": "Toy Course",
"graded" : True, "graded": True,
"tabs" : [ "tabs": [
CoursewareTab(), CoursewareTab(),
CourseInfoTab(), CourseInfoTab(),
StaticTab(name="Syllabus", url_slug="syllabus"), StaticTab(name="Syllabus", url_slug="syllabus"),
...@@ -334,27 +336,27 @@ class ModuleStoreTestCase(TestCase): ...@@ -334,27 +336,27 @@ class ModuleStoreTestCase(TestCase):
WikiTab(), WikiTab(),
ProgressTab(), ProgressTab(),
], ],
"discussion_topics" : {"General" : {"id" : "i4x-edX-toy-course-2012_Fall"}}, "discussion_topics": {"General": {"id": "i4x-edX-toy-course-2012_Fall"}},
"graceperiod" : datetime.timedelta(days=2, seconds=21599), "graceperiod": datetime.timedelta(days=2, seconds=21599),
"start" : datetime.datetime(2015, 07, 17, 12, tzinfo=pytz.utc), "start": datetime.datetime(2015, 07, 17, 12, tzinfo=pytz.utc),
"xml_attributes" : {"filename" : ["course/2012_Fall.xml", "course/2012_Fall.xml"]}, "xml_attributes": {"filename": ["course/2012_Fall.xml", "course/2012_Fall.xml"]},
"pdf_textbooks" : [ "pdf_textbooks": [
{ {
"tab_title" : "Sample Multi Chapter Textbook", "tab_title": "Sample Multi Chapter Textbook",
"id" : "MyTextbook", "id": "MyTextbook",
"chapters" : [ "chapters": [
{"url" : "/static/Chapter1.pdf", "title" : "Chapter 1"}, {"url": "/static/Chapter1.pdf", "title": "Chapter 1"},
{"url" : "/static/Chapter2.pdf", "title" : "Chapter 2"} {"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): with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.toy_loc):
self.store.create_item( self.store.create_item(
self.user.id, self.toy_loc, "about", block_id="short_description", 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.store.create_item(
self.user.id, self.toy_loc, "about", block_id="effort", self.user.id, self.toy_loc, "about", block_id="effort",
......
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. ...@@ -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 # fields is a dictionary of keys and values. sub_tree is a collection of BlockInfo
from collections import namedtuple from collections import namedtuple
import datetime import datetime
BlockInfo = namedtuple('BlockInfo', 'block_id, category, fields, sub_tree') BlockInfo = namedtuple('BlockInfo', 'block_id, category, fields, sub_tree') # pylint: disable=invalid-name
default_block_info_tree = [ default_block_info_tree = [ # pylint: disable=invalid-name
BlockInfo( BlockInfo(
'chapter_x', 'chapter', {}, [ 'chapter_x', 'chapter', {}, [
BlockInfo( BlockInfo(
...@@ -44,7 +44,7 @@ default_block_info_tree = [ ...@@ -44,7 +44,7 @@ default_block_info_tree = [
# equivalent to toy course in xml # equivalent to toy course in xml
TOY_BLOCK_INFO_TREE = [ TOY_BLOCK_INFO_TREE = [
BlockInfo( BlockInfo(
'Overview', "chapter", {"display_name" : "Overview"}, [ 'Overview', "chapter", {"display_name": "Overview"}, [
BlockInfo( BlockInfo(
"Toy_Videos", "videosequence", { "Toy_Videos", "videosequence", {
"xml_attributes": {"filename": ["", None]}, "display_name": "Toy Videos", "format": "Lecture Sequence" "xml_attributes": {"filename": ["", None]}, "display_name": "Toy Videos", "format": "Lecture Sequence"
...@@ -52,24 +52,24 @@ TOY_BLOCK_INFO_TREE = [ ...@@ -52,24 +52,24 @@ TOY_BLOCK_INFO_TREE = [
BlockInfo( BlockInfo(
"secret:toylab", "html", { "secret:toylab", "html", {
"data": "<b>Lab 2A: Superposition Experiment</b>\n\n<<<<<<< Updated upstream\n<p>Isn't the toy course great?</p>\n\n<p>Let's add some markup that uses non-ascii characters.\nFor example, we should be able to write words like encyclop&aelig;dia, or foreign words like fran&ccedil;ais.\nLooking beyond latin-1, we should handle math symbols: &pi;r&sup2 &le; &#8734.\nAnd it shouldn't matter if we use entities or numeric codes &mdash; &Omega; &ne; &pi; &equiv; &#937; &#8800; &#960;.\n</p>\n=======\n<p>Isn't the toy course great? — &le;</p>\n>>>>>>> Stashed changes\n", "data": "<b>Lab 2A: Superposition Experiment</b>\n\n<<<<<<< Updated upstream\n<p>Isn't the toy course great?</p>\n\n<p>Let's add some markup that uses non-ascii characters.\nFor example, we should be able to write words like encyclop&aelig;dia, or foreign words like fran&ccedil;ais.\nLooking beyond latin-1, we should handle math symbols: &pi;r&sup2 &le; &#8734.\nAnd it shouldn't matter if we use entities or numeric codes &mdash; &Omega; &ne; &pi; &equiv; &#937; &#8800; &#960;.\n</p>\n=======\n<p>Isn't the toy course great? — &le;</p>\n>>>>>>> Stashed changes\n",
"xml_attributes": { "filename" : [ "html/secret/toylab.xml", "html/secret/toylab.xml" ] }, "xml_attributes": {"filename": ["html/secret/toylab.xml", "html/secret/toylab.xml"]},
"display_name" : "Toy lab" "display_name": "Toy lab"
}, [] }, []
), ),
BlockInfo( BlockInfo(
"toyjumpto", "html", { "toyjumpto", "html", {
"data" : "<a href=\"/jump_to_id/vertical_test\">This is a link to another page and some Chinese 四節比分和七年前</a> <p>Some more Chinese 四節比分和七年前</p>\n", "data": "<a href=\"/jump_to_id/vertical_test\">This is a link to another page and some Chinese 四節比分和七年前</a> <p>Some more Chinese 四節比分和七年前</p>\n",
"xml_attributes": { "filename" : [ "html/toyjumpto.xml", "html/toyjumpto.xml" ] } "xml_attributes": {"filename": ["html/toyjumpto.xml", "html/toyjumpto.xml"]}
}, []), }, []),
BlockInfo( BlockInfo(
"toyhtml", "html", { "toyhtml", "html", {
"data" : "<a href='/static/handouts/sample_handout.txt'>Sample</a>", "data": "<a href='/static/handouts/sample_handout.txt'>Sample</a>",
"xml_attributes" : { "filename" : [ "html/toyhtml.xml", "html/toyhtml.xml" ] } "xml_attributes": {"filename": ["html/toyhtml.xml", "html/toyhtml.xml"]}
}, []), }, []),
BlockInfo( BlockInfo(
"nonportable", "html", { "nonportable", "html", {
"data": "<a href=\"/static/foo.jpg\">link</a>\n", "data": "<a href=\"/static/foo.jpg\">link</a>\n",
"xml_attributes" : { "filename" : [ "html/nonportable.xml", "html/nonportable.xml" ] } "xml_attributes": {"filename": ["html/nonportable.xml", "html/nonportable.xml"]}
}, []), }, []),
BlockInfo( BlockInfo(
"nonportable_link", "html", { "nonportable_link", "html", {
...@@ -79,7 +79,7 @@ TOY_BLOCK_INFO_TREE = [ ...@@ -79,7 +79,7 @@ TOY_BLOCK_INFO_TREE = [
BlockInfo( BlockInfo(
"badlink", "html", { "badlink", "html", {
"data": "<img src=\"/static//file.jpg\" />\n", "data": "<img src=\"/static//file.jpg\" />\n",
"xml_attributes" : { "filename" : [ "html/badlink.xml", "html/badlink.xml" ] } "xml_attributes": {"filename": ["html/badlink.xml", "html/badlink.xml"]}
}, []), }, []),
BlockInfo( BlockInfo(
"with_styling", "html", { "with_styling", "html", {
...@@ -89,11 +89,11 @@ TOY_BLOCK_INFO_TREE = [ ...@@ -89,11 +89,11 @@ TOY_BLOCK_INFO_TREE = [
BlockInfo( BlockInfo(
"just_img", "html", { "just_img", "html", {
"data": "<img src=\"/static/foo_bar.jpg\" />", "data": "<img src=\"/static/foo_bar.jpg\" />",
"xml_attributes": {"filename": [ "html/just_img.xml", "html/just_img.xml" ] } "xml_attributes": {"filename": ["html/just_img.xml", "html/just_img.xml"]}
}, []), }, []),
BlockInfo( BlockInfo(
"Video_Resources", "video", { "Video_Resources", "video", {
"youtube_id_1_0" : "1bK-WdDi6Qw", "display_name" : "Video Resources" "youtube_id_1_0": "1bK-WdDi6Qw", "display_name": "Video Resources"
}, []), }, []),
]), ]),
BlockInfo( BlockInfo(
...@@ -109,7 +109,7 @@ TOY_BLOCK_INFO_TREE = [ ...@@ -109,7 +109,7 @@ TOY_BLOCK_INFO_TREE = [
), ),
BlockInfo( BlockInfo(
"secret:magic", "chapter", { "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( BlockInfo(
"toyvideo", "video", {"youtube_id_1_0": "OEoXaMPEzfMA", "display_name": "toyvideo"}, [] "toyvideo", "video", {"youtube_id_1_0": "OEoXaMPEzfMA", "display_name": "toyvideo"}, []
...@@ -124,7 +124,7 @@ TOY_BLOCK_INFO_TREE = [ ...@@ -124,7 +124,7 @@ TOY_BLOCK_INFO_TREE = [
"answers": [{"text": "Yes", "id": "yes"}, {"text": "No", "id": "no"}], "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" "display_name": "Change your answer"
}, []) ] }, [])]
), ),
BlockInfo( BlockInfo(
"vertical_container", "chapter", { "vertical_container", "chapter", {
...@@ -163,7 +163,7 @@ TOY_BLOCK_INFO_TREE = [ ...@@ -163,7 +163,7 @@ TOY_BLOCK_INFO_TREE = [
"T1_changemind_poll_foo_2", "poll_question", { "T1_changemind_poll_foo_2", "poll_question", {
"question": "<p>Have you changed your mind?</p>", "question": "<p>Have you changed your mind?</p>",
"answers": [{"text": "Yes", "id": "yes"}, {"text": "No", "id": "no"}], "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" "display_name": "Change your answer"
}, []), }, []),
]), ]),
...@@ -175,7 +175,7 @@ TOY_BLOCK_INFO_TREE = [ ...@@ -175,7 +175,7 @@ TOY_BLOCK_INFO_TREE = [
), ),
BlockInfo( BlockInfo(
"handout_container", "chapter", { "handout_container", "chapter", {
"xml_attributes" : {"filename" : ["chapter/handout_container.xml", "chapter/handout_container.xml"]} "xml_attributes": {"filename": ["chapter/handout_container.xml", "chapter/handout_container.xml"]}
}, [ }, [
BlockInfo( BlockInfo(
"html_7e5578f25f79", "html", { "html_7e5578f25f79", "html", {
......
common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
...@@ -451,7 +451,7 @@ class SplitModuleTest(unittest.TestCase): ...@@ -451,7 +451,7 @@ class SplitModuleTest(unittest.TestCase):
} }
@staticmethod @staticmethod
def bootstrapDB(split_store): def bootstrapDB(split_store): # pylint: disable=invalid-name
''' '''
Sets up the initial data into the db Sets up the initial data into the db
''' '''
...@@ -517,7 +517,7 @@ class SplitModuleTest(unittest.TestCase): ...@@ -517,7 +517,7 @@ class SplitModuleTest(unittest.TestCase):
SplitModuleTest.modulestore = None SplitModuleTest.modulestore = None
super(SplitModuleTest, self).tearDown() 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 Result is a collection of descriptors. Find the one whose block id
matches the _id. matches the _id.
...@@ -716,6 +716,7 @@ class SplitModuleCourseTests(SplitModuleTest): ...@@ -716,6 +716,7 @@ class SplitModuleCourseTests(SplitModuleTest):
self.assertEqual(len(result.children[0].children), 1) self.assertEqual(len(result.children[0].children), 1)
self.assertEqual(result.children[0].children[0].locator.version_guid, versions[0]) self.assertEqual(result.children[0].children[0].locator.version_guid, versions[0])
class SplitModuleItemTests(SplitModuleTest): class SplitModuleItemTests(SplitModuleTest):
''' '''
Item read tests including inheritance Item read tests including inheritance
...@@ -946,6 +947,10 @@ class SplitModuleItemTests(SplitModuleTest): ...@@ -946,6 +947,10 @@ class SplitModuleItemTests(SplitModuleTest):
def version_agnostic(children): 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] return [child.version_agnostic() for child in children]
...@@ -1035,7 +1040,6 @@ class TestItemCrud(SplitModuleTest): ...@@ -1035,7 +1040,6 @@ class TestItemCrud(SplitModuleTest):
self.assertIn(new_module.location.version_agnostic(), version_agnostic(parent.children)) self.assertIn(new_module.location.version_agnostic(), version_agnostic(parent.children))
self.assertEqual(new_module.definition_locator.definition_id, original.definition_locator.definition_id) self.assertEqual(new_module.definition_locator.definition_id, original.definition_locator.definition_id)
def test_unique_naming(self): def test_unique_naming(self):
""" """
Check that 2 modules of same type get unique block_ids. Also check that if creation provides Check that 2 modules of same type get unique block_ids. Also check that if creation provides
...@@ -1760,6 +1764,6 @@ def modulestore(): ...@@ -1760,6 +1764,6 @@ def modulestore():
return SplitModuleTest.modulestore return SplitModuleTest.modulestore
# pylint: disable=W0613 # pylint: disable=unused-argument, missing-docstring
def render_to_template_mock(*args): def render_to_template_mock(*args):
pass pass
common/lib/xmodule/xmodule/peer_grading_module.py
...@@ -86,6 +86,7 @@ class PeerGradingFields(object): ...@@ -86,6 +86,7 @@ class PeerGradingFields(object):
scope=Scope.content scope=Scope.content
) )
class InvalidLinkLocation(Exception): class InvalidLinkLocation(Exception):
""" """
Exception for the case in which a peer grading module tries to link to an invalid location. 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): ...@@ -150,7 +151,8 @@ class PeerGradingModule(PeerGradingFields, XModule):
try: try:
self.student_data_for_location = json.loads(self.student_data_for_location) 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 pass
@property @property
...@@ -218,9 +220,9 @@ class PeerGradingModule(PeerGradingFields, XModule): ...@@ -218,9 +220,9 @@ class PeerGradingModule(PeerGradingFields, XModule):
# This is a dev_facing_error # This is a dev_facing_error
return json.dumps({'error': 'Error handling action. Please try again.', 'success': False}) 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): def query_data_for_location(self, location):
student_id = self.system.anonymous_student_id student_id = self.system.anonymous_student_id
...@@ -229,13 +231,12 @@ class PeerGradingModule(PeerGradingFields, XModule): ...@@ -229,13 +231,12 @@ class PeerGradingModule(PeerGradingFields, XModule):
try: try:
response = self.peer_gs.get_data_for_location(location, student_id) response = self.peer_gs.get_data_for_location(location, student_id)
count_graded = response['count_graded'] _count_graded = response['count_graded']
count_required = response['count_required'] _count_required = response['count_required']
success = True success = True
except GradingServiceError: except GradingServiceError:
# This is a dev_facing_error # This is a dev_facing_error
log.exception("Error getting location data from controller for location {0}, student {1}" log.exception("Error getting location data from controller for location %s, student %s", location, student_id)
.format(location, student_id))
return success, response return success, response
...@@ -322,8 +323,7 @@ class PeerGradingModule(PeerGradingFields, XModule): ...@@ -322,8 +323,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
return response return response
except GradingServiceError: except GradingServiceError:
# This is a dev_facing_error # This is a dev_facing_error
log.exception("Error getting next submission. server url: {0} location: {1}, grader_id: {2}" log.exception("Error getting next submission. server url: %s location: %s, grader_id: %s", self.peer_gs.url, location, grader_id)
.format(self.peer_gs.url, location, grader_id))
# This is a student_facing_error # This is a student_facing_error
return {'success': False, return {'success': False,
'error': EXTERNAL_GRADER_NO_CONTACT_ERROR} 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR}
...@@ -355,7 +355,7 @@ class PeerGradingModule(PeerGradingFields, XModule): ...@@ -355,7 +355,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
if not success: if not success:
return self._err_response(message) 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: if 'rubric_scores[]' in required:
data_dict['rubric_scores'] = data.getall('rubric_scores[]') data_dict['rubric_scores'] = data.getall('rubric_scores[]')
data_dict['grader_id'] = self.system.anonymous_student_id data_dict['grader_id'] = self.system.anonymous_student_id
...@@ -365,15 +365,14 @@ class PeerGradingModule(PeerGradingFields, XModule): ...@@ -365,15 +365,14 @@ class PeerGradingModule(PeerGradingFields, XModule):
success, location_data = self.query_data_for_location(data_dict['location']) 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 #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. #that will cause success to be false.
response.update({'required_done' : 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']): 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 response['required_done'] = True
return response return response
except GradingServiceError: except GradingServiceError:
# This is a dev_facing_error # This is a dev_facing_error
log.exception("""Error saving grade to open ended grading service. server url: {0}""" log.exception("Error saving grade to open ended grading service. server url: %s", self.peer_gs.url)
.format(self.peer_gs.url)
)
# This is a student_facing_error # This is a student_facing_error
return { return {
'success': False, 'success': False,
...@@ -411,8 +410,7 @@ class PeerGradingModule(PeerGradingFields, XModule): ...@@ -411,8 +410,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
return response return response
except GradingServiceError: except GradingServiceError:
# This is a dev_facing_error # This is a dev_facing_error
log.exception("Error from open ended grading service. server url: {0}, grader_id: {0}, location: {1}" log.exception("Error from open ended grading service. server url: %s, grader_id: %s, location: %s", self.peer_gs.url, grader_id, location)
.format(self.peer_gs.url, grader_id, location))
# This is a student_facing_error # This is a student_facing_error
return { return {
'success': False, 'success': False,
...@@ -456,8 +454,7 @@ class PeerGradingModule(PeerGradingFields, XModule): ...@@ -456,8 +454,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
return response return response
except GradingServiceError: except GradingServiceError:
# This is a dev_facing_error # This is a dev_facing_error
log.exception("Error from open ended grading service. server url: {0}, location: {0}" log.exception("Error from open ended grading service. server url: %s, location: %s", self.peer_gs.url, location)
.format(self.peer_gs.url, location))
# This is a student_facing_error # This is a student_facing_error
return {'success': False, return {'success': False,
'error': EXTERNAL_GRADER_NO_CONTACT_ERROR} 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR}
...@@ -492,7 +489,7 @@ class PeerGradingModule(PeerGradingFields, XModule): ...@@ -492,7 +489,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
if not success: if not success:
return self._err_response(message) 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['rubric_scores'] = data.getall('rubric_scores[]')
data_dict['student_id'] = self.system.anonymous_student_id data_dict['student_id'] = self.system.anonymous_student_id
data_dict['calibration_essay_id'] = data_dict['submission_id'] data_dict['calibration_essay_id'] = data_dict['submission_id']
...@@ -619,7 +616,7 @@ class PeerGradingModule(PeerGradingFields, XModule): ...@@ -619,7 +616,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
elif data.get('location') is not None: elif data.get('location') is not None:
problem_location = self.course_id.make_usage_key_from_deprecated_string(data.get('location')) 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 ajax_url = self.ajax_url
html = self.system.render_template('peer_grading/peer_grading_problem.html', { html = self.system.render_template('peer_grading/peer_grading_problem.html', {
......
common/lib/xmodule/xmodule/tests/test_lti_unit.py
...@@ -54,25 +54,29 @@ class LTIModuleTest(LogicTest): ...@@ -54,25 +54,29 @@ class LTIModuleTest(LogicTest):
self.user_id = self.xmodule.runtime.anonymous_student_id self.user_id = self.xmodule.runtime.anonymous_student_id
self.lti_id = self.xmodule.lti_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 = { self.defaults = {
'namespace' : "http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0", 'namespace': "http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0",
'sourcedId': sourcedId, 'sourcedId': sourced_id,
'action': 'replaceResultRequest', 'action': 'replaceResultRequest',
'grade': 0.5, 'grade': 0.5,
'messageIdentifier': '528243ba5241b', 'messageIdentifier': '528243ba5241b',
} }
def get_request_body(self, params={}): def get_request_body(self, params=None):
data = copy(self.DEFAULTS) """Fetches the body of a request specified by params"""
if params is None:
params = {}
data = copy(self.defaults)
data.update(params) data.update(params)
return self.request_body_xml_template.format(**data) return self.request_body_xml_template.format(**data)
def get_response_values(self, response): def get_response_values(self, response):
"""Gets the values from the given response"""
parser = etree.XMLParser(ns_clean=True, recover=True, encoding='utf-8') parser = etree.XMLParser(ns_clean=True, recover=True, encoding='utf-8')
root = etree.fromstring(response.body.strip(), parser=parser) root = etree.fromstring(response.body.strip(), parser=parser)
lti_spec_namespace = "http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0" lti_spec_namespace = "http://www.imsglobal.org/services/ltiv1p1/xsd/imsoms_v1p0"
...@@ -80,23 +84,23 @@ class LTIModuleTest(LogicTest): ...@@ -80,23 +84,23 @@ class LTIModuleTest(LogicTest):
code_major = root.xpath("//def:imsx_codeMajor", namespaces=namespaces)[0].text code_major = root.xpath("//def:imsx_codeMajor", namespaces=namespaces)[0].text
description = root.xpath("//def:imsx_description", namespaces=namespaces)[0].text description = root.xpath("//def:imsx_description", namespaces=namespaces)[0].text
messageIdentifier = root.xpath("//def:imsx_messageIdentifier", namespaces=namespaces)[0].text message_identifier = root.xpath("//def:imsx_messageIdentifier", namespaces=namespaces)[0].text
imsx_POXBody = root.xpath("//def:imsx_POXBody", namespaces=namespaces)[0] imsx_pox_body = root.xpath("//def:imsx_POXBody", namespaces=namespaces)[0]
try: try:
action = imsx_POXBody.getchildren()[0].tag.replace('{'+lti_spec_namespace+'}', '') action = imsx_pox_body.getchildren()[0].tag.replace('{' + lti_spec_namespace + '}', '')
except Exception: except Exception: # pylint: disable=broad-except
action = None action = None
return { return {
'code_major': code_major, 'code_major': code_major,
'description': description, 'description': description,
'messageIdentifier': messageIdentifier, 'messageIdentifier': message_identifier,
'action': action 'action': action
} }
@patch('xmodule.lti_module.LTIModule.get_client_key_secret', return_value=('test_client_key', u'test_client_secret')) @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. Request has no Authorization header.
...@@ -110,14 +114,14 @@ class LTIModuleTest(LogicTest): ...@@ -110,14 +114,14 @@ class LTIModuleTest(LogicTest):
'action': None, 'action': None,
'code_major': 'failure', 'code_major': 'failure',
'description': 'OAuth verification error: Malformed authorization header', 'description': 'OAuth verification error: Malformed authorization header',
'messageIdentifier': self.DEFAULTS['messageIdentifier'], 'messageIdentifier': self.defaults['messageIdentifier'],
} }
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertDictEqual(expected_response, real_response) self.assertDictEqual(expected_response, real_response)
@patch('xmodule.lti_module.LTIModule.get_client_key_secret', return_value=('test_client_key', u'test_client_secret')) @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. Request Authorization header has no value.
...@@ -132,7 +136,7 @@ class LTIModuleTest(LogicTest): ...@@ -132,7 +136,7 @@ class LTIModuleTest(LogicTest):
'action': None, 'action': None,
'code_major': 'failure', 'code_major': 'failure',
'description': 'OAuth verification error: Malformed authorization header', 'description': 'OAuth verification error: Malformed authorization header',
'messageIdentifier': self.DEFAULTS['messageIdentifier'], 'messageIdentifier': self.defaults['messageIdentifier'],
} }
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertDictEqual(expected_response, real_response) self.assertDictEqual(expected_response, real_response)
...@@ -152,7 +156,7 @@ class LTIModuleTest(LogicTest): ...@@ -152,7 +156,7 @@ class LTIModuleTest(LogicTest):
'action': None, 'action': None,
'code_major': 'failure', 'code_major': 'failure',
'description': 'User not found.', 'description': 'User not found.',
'messageIdentifier': self.DEFAULTS['messageIdentifier'], 'messageIdentifier': self.defaults['messageIdentifier'],
} }
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertDictEqual(expected_response, real_response) self.assertDictEqual(expected_response, real_response)
...@@ -207,7 +211,7 @@ class LTIModuleTest(LogicTest): ...@@ -207,7 +211,7 @@ class LTIModuleTest(LogicTest):
'action': None, 'action': None,
'code_major': 'unsupported', 'code_major': 'unsupported',
'description': 'Target does not support the requested operation.', 'description': 'Target does not support the requested operation.',
'messageIdentifier': self.DEFAULTS['messageIdentifier'], 'messageIdentifier': self.defaults['messageIdentifier'],
} }
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertDictEqual(expected_response, real_response) self.assertDictEqual(expected_response, real_response)
...@@ -222,20 +226,20 @@ class LTIModuleTest(LogicTest): ...@@ -222,20 +226,20 @@ class LTIModuleTest(LogicTest):
request.body = self.get_request_body() request.body = self.get_request_body()
response = self.xmodule.grade_handler(request, '') response = self.xmodule.grade_handler(request, '')
description_expected = 'Score for {sourcedId} is now {score}'.format( description_expected = 'Score for {sourcedId} is now {score}'.format(
sourcedId=self.DEFAULTS['sourcedId'], sourcedId=self.defaults['sourcedId'],
score=self.DEFAULTS['grade'], score=self.defaults['grade'],
) )
real_response = self.get_response_values(response) real_response = self.get_response_values(response)
expected_response = { expected_response = {
'action': 'replaceResultResponse', 'action': 'replaceResultResponse',
'code_major': 'success', 'code_major': 'success',
'description': description_expected, 'description': description_expected,
'messageIdentifier': self.DEFAULTS['messageIdentifier'], 'messageIdentifier': self.defaults['messageIdentifier'],
} }
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertDictEqual(expected_response, real_response) 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): def test_user_id(self):
expected_user_id = unicode(urllib.quote(self.xmodule.runtime.anonymous_student_id)) expected_user_id = unicode(urllib.quote(self.xmodule.runtime.anonymous_student_id))
...@@ -255,27 +259,27 @@ class LTIModuleTest(LogicTest): ...@@ -255,27 +259,27 @@ class LTIModuleTest(LogicTest):
self.assertEqual(real_outcome_service_url, mock_url_prefix + test_service_name) self.assertEqual(real_outcome_service_url, mock_url_prefix + test_service_name)
def test_resource_link_id(self): 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' self.xmodule.location.html_id = lambda: 'i4x-2-3-lti-31de800015cf4afb973356dbe81496df'
expected_resource_link_id = unicode(urllib.quote(self.unquoted_resource_link_id)) expected_resource_link_id = unicode(urllib.quote(self.unquoted_resource_link_id))
real_resource_link_id = self.xmodule.get_resource_link_id() real_resource_link_id = self.xmodule.get_resource_link_id()
self.assertEqual(real_resource_link_id, expected_resource_link_id) self.assertEqual(real_resource_link_id, expected_resource_link_id)
def test_lis_result_sourcedid(self): 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.system.course_id.to_deprecated_string(),
self.xmodule.get_resource_link_id(), self.xmodule.get_resource_link_id(),
self.user_id self.user_id
)) ))
real_lis_result_sourcedid = self.xmodule.get_lis_result_sourcedid() 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): def test_client_key_secret(self):
""" """
LTI module gets client key and secret provided. LTI module gets client key and secret provided.
""" """
#this adds lti passports to system #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 = Mock()
modulestore.get_course.return_value = mocked_course modulestore.get_course.return_value = mocked_course
runtime = Mock(modulestore=modulestore) runtime = Mock(modulestore=modulestore)
...@@ -293,7 +297,7 @@ class LTIModuleTest(LogicTest): ...@@ -293,7 +297,7 @@ class LTIModuleTest(LogicTest):
""" """
#this adds lti passports to system #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 = Mock()
modulestore.get_course.return_value = mocked_course modulestore.get_course.return_value = mocked_course
runtime = Mock(modulestore=modulestore) runtime = Mock(modulestore=modulestore)
...@@ -301,7 +305,7 @@ class LTIModuleTest(LogicTest): ...@@ -301,7 +305,7 @@ class LTIModuleTest(LogicTest):
#set another lti_id #set another lti_id
self.xmodule.lti_id = "another_lti_id" self.xmodule.lti_id = "another_lti_id"
key_secret = self.xmodule.get_client_key_secret() key_secret = self.xmodule.get_client_key_secret()
expected = ('','') expected = ('', '')
self.assertEqual(expected, key_secret) self.assertEqual(expected, key_secret)
def test_bad_client_key_secret(self): def test_bad_client_key_secret(self):
...@@ -311,7 +315,7 @@ class LTIModuleTest(LogicTest): ...@@ -311,7 +315,7 @@ class LTIModuleTest(LogicTest):
There are key and secret provided in wrong format. There are key and secret provided in wrong format.
""" """
#this adds lti passports to system #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 = Mock()
modulestore.get_course.return_value = mocked_course modulestore.get_course.return_value = mocked_course
runtime = Mock(modulestore=modulestore) runtime = Mock(modulestore=modulestore)
...@@ -348,11 +352,11 @@ class LTIModuleTest(LogicTest): ...@@ -348,11 +352,11 @@ class LTIModuleTest(LogicTest):
Tests that xml body was parsed successfully. Tests that xml body was parsed successfully.
""" """
mocked_request = self.get_signed_grade_mock_request() mocked_request = self.get_signed_grade_mock_request()
messageIdentifier, sourcedId, grade, action = self.xmodule.parse_grade_xml_body(mocked_request.body) message_identifier, sourced_id, grade, action = self.xmodule.parse_grade_xml_body(mocked_request.body)
self.assertEqual(self.DEFAULTS['messageIdentifier'], messageIdentifier) self.assertEqual(self.defaults['messageIdentifier'], message_identifier)
self.assertEqual(self.DEFAULTS['sourcedId'], sourcedId) self.assertEqual(self.defaults['sourcedId'], sourced_id)
self.assertEqual(self.DEFAULTS['grade'], grade) self.assertEqual(self.defaults['grade'], grade)
self.assertEqual(self.DEFAULTS['action'], action) self.assertEqual(self.defaults['action'], action)
@patch('xmodule.lti_module.signature.verify_hmac_sha1', Mock(return_value=False)) @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'))) @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): ...@@ -372,7 +376,7 @@ class LTIModuleTest(LogicTest):
LTI 1.1 will be used. Otherwise fake namespace will be added to XML. LTI 1.1 will be used. Otherwise fake namespace will be added to XML.
""" """
mock_request = Mock() mock_request = Mock()
mock_request.headers = { mock_request.headers = { # pylint: disable=no-space-before-operator
'X-Requested-With': 'XMLHttpRequest', 'X-Requested-With': 'XMLHttpRequest',
'Content-Type': 'application/x-www-form-urlencoded', 'Content-Type': 'application/x-www-form-urlencoded',
'Authorization': u'OAuth oauth_nonce="135685044251684026041377608307", \ 'Authorization': u'OAuth oauth_nonce="135685044251684026041377608307", \
......
lms/djangoapps/dashboard/views.py
"""View functions for the LMS Student dashboard"""
from django.http import Http404 from django.http import Http404
from edxmako.shortcuts import render_to_response from edxmako.shortcuts import render_to_response
from django.db import connection from django.db import connection
...@@ -15,15 +16,18 @@ def dictfetchall(cursor): ...@@ -15,15 +16,18 @@ def dictfetchall(cursor):
# ensure response from db is a list, not a tuple (which is returned # ensure response from db is a list, not a tuple (which is returned
# by MySQL backed django instances) # 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] table = table + [list(row) for row in rows_from_cursor]
return table 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) cursor.execute(query_string)
raw_result=dictfetchall(cursor) raw_result = dictfetchall(cursor)
return raw_result return raw_result
def dashboard(request): def dashboard(request):
""" """
Slightly less hackish hack to show staff enrollment numbers and other Slightly less hackish hack to show staff enrollment numbers and other
...@@ -40,11 +44,11 @@ def dashboard(request): ...@@ -40,11 +44,11 @@ def dashboard(request):
# two types of results: scalars and tables. Scalars should be represented # two types of results: scalars and tables. Scalars should be represented
# as "Visible Title": Value and tables should be lists of lists where each # as "Visible Title": Value and tables should be lists of lists where each
# inner list represents a single row of the table # inner list represents a single row of the table
results = {"scalars":{},"tables":{}} results = {"scalars": {}, "tables": {}}
# count how many users we have # count how many users we have
results["scalars"]["Unique Usernames"]=User.objects.filter().count() results["scalars"]["Unique Usernames"] = User.objects.filter().count()
results["scalars"]["Activated Usernames"]=User.objects.filter(is_active=1).count() results["scalars"]["Activated Usernames"] = User.objects.filter(is_active=1).count()
# count how many enrollments we have # count how many enrollments we have
results["scalars"]["Total Enrollments Across All Courses"] = CourseEnrollment.objects.filter(is_active=1).count() results["scalars"]["Total Enrollments Across All Courses"] = CourseEnrollment.objects.filter(is_active=1).count()
...@@ -78,6 +82,6 @@ def dashboard(request): ...@@ -78,6 +82,6 @@ def dashboard(request):
cursor.execute(table_queries[query]) cursor.execute(table_queries[query])
results["tables"][query] = SQL_query_to_list(cursor, 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)
lms/djangoapps/django_comment_client/forum/tests.py
...@@ -444,6 +444,7 @@ class UserProfileTestCase(ModuleStoreTestCase): ...@@ -444,6 +444,7 @@ class UserProfileTestCase(ModuleStoreTestCase):
) )
self.assertEqual(response.status_code, 405) self.assertEqual(response.status_code, 405)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@patch('requests.request') @patch('requests.request')
class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase): class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase):
......
lms/djangoapps/foldit/tests.py
"""Tests for the FoldIt module"""
import json import json
import logging import logging
from functools import partial from functools import partial
...@@ -19,7 +20,7 @@ log = logging.getLogger(__name__) ...@@ -19,7 +20,7 @@ log = logging.getLogger(__name__)
class FolditTestCase(TestCase): class FolditTestCase(TestCase):
"""Tests for various responses of the FoldIt module"""
def setUp(self): def setUp(self):
self.factory = RequestFactory() self.factory = RequestFactory()
self.url = reverse('foldit_ops') self.url = reverse('foldit_ops')
...@@ -37,10 +38,8 @@ class FolditTestCase(TestCase): ...@@ -37,10 +38,8 @@ class FolditTestCase(TestCase):
self.tomorrow = now + timedelta(days=1) self.tomorrow = now + timedelta(days=1)
self.yesterday = now - timedelta(days=1) self.yesterday = now - timedelta(days=1)
self.user.profile
self.user2.profile
def make_request(self, post_data, user=None): 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 = self.factory.post(self.url, post_data)
request.user = self.user if not user else user request.user = self.user if not user else user
return request return request
...@@ -57,6 +56,7 @@ class FolditTestCase(TestCase): ...@@ -57,6 +56,7 @@ class FolditTestCase(TestCase):
user = self.user if not user else user user = self.user if not user else user
def score_dict(puzzle_id, best_score): def score_dict(puzzle_id, best_score):
"""Returns a valid json-parsable score dict"""
return {"PuzzleID": puzzle_id, return {"PuzzleID": puzzle_id,
"ScoreType": "score", "ScoreType": "score",
"BestScore": best_score, "BestScore": best_score,
...@@ -77,7 +77,7 @@ class FolditTestCase(TestCase): ...@@ -77,7 +77,7 @@ class FolditTestCase(TestCase):
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
return response return response
def test_SetPlayerPuzzleScores(self): def test_SetPlayerPuzzleScores(self): # pylint: disable=invalid-name
puzzle_id = 994391 puzzle_id = 994391
best_score = 0.078034 best_score = 0.078034
...@@ -94,7 +94,7 @@ class FolditTestCase(TestCase): ...@@ -94,7 +94,7 @@ class FolditTestCase(TestCase):
self.assertEqual(len(top_10), 1) self.assertEqual(len(top_10), 1)
self.assertEqual(top_10[0]['score'], Score.display_score(best_score)) 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]) response = self.make_puzzle_score_request([1, 2], [0.078034, 0.080000])
...@@ -113,15 +113,14 @@ class FolditTestCase(TestCase): ...@@ -113,15 +113,14 @@ class FolditTestCase(TestCase):
}] }]
)) ))
def test_SetPlayerPuzzleScores_multiple(self): # pylint: disable=invalid-name
def test_SetPlayerPuzzleScores_multiple(self):
""" """
Check that multiple posts with the same id are handled properly Check that multiple posts with the same id are handled properly
(keep latest for each user, have multiple users work properly) (keep latest for each user, have multiple users work properly)
""" """
orig_score = 0.07 orig_score = 0.07
puzzle_id = '1' 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. # There should now be a score in the db.
top_10 = Score.get_tops_n(10, puzzle_id) top_10 = Score.get_tops_n(10, puzzle_id)
...@@ -130,7 +129,7 @@ class FolditTestCase(TestCase): ...@@ -130,7 +129,7 @@ class FolditTestCase(TestCase):
# Reporting a better score should overwrite # Reporting a better score should overwrite
better_score = 0.06 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) top_10 = Score.get_tops_n(10, puzzle_id)
self.assertEqual(len(top_10), 1) self.assertEqual(len(top_10), 1)
...@@ -144,7 +143,7 @@ class FolditTestCase(TestCase): ...@@ -144,7 +143,7 @@ class FolditTestCase(TestCase):
# reporting a worse score shouldn't # reporting a worse score shouldn't
worse_score = 0.065 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) top_10 = Score.get_tops_n(10, puzzle_id)
self.assertEqual(len(top_10), 1) self.assertEqual(len(top_10), 1)
...@@ -155,7 +154,7 @@ class FolditTestCase(TestCase): ...@@ -155,7 +154,7 @@ class FolditTestCase(TestCase):
delta=0.5 delta=0.5
) )
def test_SetPlayerPuzzleScores_multiplecourses(self): def test_SetPlayerPuzzleScores_multiple_courses(self): # pylint: disable=invalid-name
puzzle_id = "1" puzzle_id = "1"
player1_score = 0.05 player1_score = 0.05
...@@ -164,9 +163,8 @@ class FolditTestCase(TestCase): ...@@ -164,9 +163,8 @@ class FolditTestCase(TestCase):
course_list_1 = [self.course_id] course_list_1 = [self.course_id]
course_list_2 = [self.course_id2] course_list_2 = [self.course_id2]
response1 = self.make_puzzle_score_request( self.make_puzzle_score_request(puzzle_id, player1_score, self.user)
puzzle_id, player1_score, self.user
)
course_1_top_10 = Score.get_tops_n(10, puzzle_id, course_list_1) 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) course_2_top_10 = Score.get_tops_n(10, puzzle_id, course_list_2)
total_top_10 = Score.get_tops_n(10, puzzle_id) total_top_10 = Score.get_tops_n(10, puzzle_id)
...@@ -176,9 +174,8 @@ class FolditTestCase(TestCase): ...@@ -176,9 +174,8 @@ class FolditTestCase(TestCase):
self.assertEqual(len(course_2_top_10), 0) self.assertEqual(len(course_2_top_10), 0)
self.assertEqual(len(total_top_10), 1) self.assertEqual(len(total_top_10), 1)
response2 = self.make_puzzle_score_request( self.make_puzzle_score_request(puzzle_id, player2_score, self.user2)
puzzle_id, player2_score, self.user2
)
course_2_top_10 = Score.get_tops_n(10, puzzle_id, course_list_2) course_2_top_10 = Score.get_tops_n(10, puzzle_id, course_list_2)
total_top_10 = Score.get_tops_n(10, puzzle_id) total_top_10 = Score.get_tops_n(10, puzzle_id)
...@@ -187,7 +184,7 @@ class FolditTestCase(TestCase): ...@@ -187,7 +184,7 @@ class FolditTestCase(TestCase):
self.assertEqual(len(course_2_top_10), 1) self.assertEqual(len(course_2_top_10), 1)
self.assertEqual(len(total_top_10), 2) 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 Check that when we send scores from multiple users, the correct order
of scores is displayed. Note that, before being processed by of scores is displayed. Note that, before being processed by
...@@ -196,18 +193,14 @@ class FolditTestCase(TestCase): ...@@ -196,18 +193,14 @@ class FolditTestCase(TestCase):
puzzle_id = ['1'] puzzle_id = ['1']
player1_score = 0.08 player1_score = 0.08
player2_score = 0.02 player2_score = 0.02
response1 = self.make_puzzle_score_request( self.make_puzzle_score_request(puzzle_id, player1_score, self.user)
puzzle_id, player1_score, self.user
)
# There should now be a score in the db. # There should now be a score in the db.
top_10 = Score.get_tops_n(10, puzzle_id) top_10 = Score.get_tops_n(10, puzzle_id)
self.assertEqual(len(top_10), 1) self.assertEqual(len(top_10), 1)
self.assertEqual(top_10[0]['score'], Score.display_score(player1_score)) self.assertEqual(top_10[0]['score'], Score.display_score(player1_score))
response2 = self.make_puzzle_score_request( self.make_puzzle_score_request(puzzle_id, player2_score, self.user2)
puzzle_id, player2_score, self.user2
)
# There should now be two scores in the db # There should now be two scores in the db
top_10 = Score.get_tops_n(10, puzzle_id) top_10 = Score.get_tops_n(10, puzzle_id)
...@@ -228,32 +221,36 @@ class FolditTestCase(TestCase): ...@@ -228,32 +221,36 @@ class FolditTestCase(TestCase):
# Top score user should be self.user2.username # Top score user should be self.user2.username
self.assertEqual(top_10[0]['username'], 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, scores = [{
"PuzzleID": 994391,
"ScoreType": "score", "ScoreType": "score",
"BestScore": 0.078034, "BestScore": 0.078034,
"CurrentScore": 0.080035, "CurrentScore": 0.080035,
"ScoreVersion": 23}] "ScoreVersion": 23
}]
validation_str = json.dumps(scores) validation_str = json.dumps(scores)
verify = {"Verify": verify_code(self.user.email, validation_str), verify = {
"VerifyMethod": "FoldItVerify"} "Verify": verify_code(self.user.email, validation_str),
"VerifyMethod": "FoldItVerify"
}
# change the real string -- should get an error # change the real string -- should get an error
scores[0]['ScoreVersion'] = 22 scores[0]['ScoreVersion'] = 22
scores_str = json.dumps(scores) scores_str = json.dumps(scores)
data = {'SetPlayerPuzzleScoresVerify': json.dumps(verify), data = {
'SetPlayerPuzzleScores': scores_str} 'SetPlayerPuzzleScoresVerify': json.dumps(verify),
'SetPlayerPuzzleScores': scores_str
}
request = self.make_request(data) request = self.make_request(data)
response = foldit_ops(request) response = foldit_ops(request)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
response_data = json.loads(response.content)
self.assertEqual(response.content, self.assertEqual(response.content,
json.dumps([{ json.dumps([{
"OperationID": "SetPlayerPuzzleScores", "OperationID": "SetPlayerPuzzleScores",
...@@ -261,7 +258,6 @@ class FolditTestCase(TestCase): ...@@ -261,7 +258,6 @@ class FolditTestCase(TestCase):
"ErrorString": "Verification failed", "ErrorString": "Verification failed",
"ErrorCode": "VerifyFailed"}])) "ErrorCode": "VerifyFailed"}]))
def make_puzzles_complete_request(self, puzzles): def make_puzzles_complete_request(self, puzzles):
""" """
Make a puzzles complete request, given an array of Make a puzzles complete request, given an array of
...@@ -272,11 +268,15 @@ class FolditTestCase(TestCase): ...@@ -272,11 +268,15 @@ class FolditTestCase(TestCase):
""" """
puzzles_str = json.dumps(puzzles) puzzles_str = json.dumps(puzzles)
verify = {"Verify": verify_code(self.user.email, puzzles_str), verify = {
"VerifyMethod":"FoldItVerify"} "Verify": verify_code(self.user.email, puzzles_str),
"VerifyMethod": "FoldItVerify"
}
data = {'SetPuzzlesCompleteVerify': json.dumps(verify), data = {
'SetPuzzlesComplete': puzzles_str} 'SetPuzzlesCompleteVerify': json.dumps(verify),
'SetPuzzlesComplete': puzzles_str
}
request = self.make_request(data) request = self.make_request(data)
...@@ -286,56 +286,64 @@ class FolditTestCase(TestCase): ...@@ -286,56 +286,64 @@ class FolditTestCase(TestCase):
@staticmethod @staticmethod
def set_puzzle_complete_response(values): 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}]) "Value": values}])
def test_SetPlayerPuzzlesComplete(self): # pylint: disable=invalid-name
def test_SetPlayerPuzzlesComplete(self): puzzles = [
{"PuzzleID": 13, "Set": 1, "SubSet": 2},
puzzles = [ {"PuzzleID": 13, "Set": 1, "SubSet": 2}, {"PuzzleID": 53524, "Set": 1, "SubSet": 1}
{"PuzzleID": 53524, "Set": 1, "SubSet": 1} ] ]
response = self.make_puzzles_complete_request(puzzles) response = self.make_puzzles_complete_request(puzzles)
self.assertEqual(response.content, self.assertEqual(response.content,
self.set_puzzle_complete_response([13, 53524])) self.set_puzzle_complete_response([13, 53524]))
def test_SetPlayerPuzzlesComplete_multiple(self): # pylint: disable=invalid-name
def test_SetPlayerPuzzlesComplete_multiple(self):
"""Check that state is stored properly""" """Check that state is stored properly"""
puzzles = [ {"PuzzleID": 13, "Set": 1, "SubSet": 2}, puzzles = [
{"PuzzleID": 53524, "Set": 1, "SubSet": 1} ] {"PuzzleID": 13, "Set": 1, "SubSet": 2},
{"PuzzleID": 53524, "Set": 1, "SubSet": 1}
]
response = self.make_puzzles_complete_request(puzzles) response = self.make_puzzles_complete_request(puzzles)
self.assertEqual(response.content, self.assertEqual(response.content,
self.set_puzzle_complete_response([13, 53524])) self.set_puzzle_complete_response([13, 53524]))
puzzles = [ {"PuzzleID": 14, "Set": 1, "SubSet": 3}, puzzles = [
{"PuzzleID": 15, "Set": 1, "SubSet": 1} ] {"PuzzleID": 14, "Set": 1, "SubSet": 3},
{"PuzzleID": 15, "Set": 1, "SubSet": 1}
]
response = self.make_puzzles_complete_request(puzzles) response = self.make_puzzles_complete_request(puzzles)
self.assertEqual(response.content, self.assertEqual(
self.set_puzzle_complete_response([13, 14, 15, 53524])) 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""" """Check that the level complete function works"""
puzzles = [ {"PuzzleID": 13, "Set": 1, "SubSet": 2}, puzzles = [
{"PuzzleID": 53524, "Set": 1, "SubSet": 1} ] {"PuzzleID": 13, "Set": 1, "SubSet": 2},
{"PuzzleID": 53524, "Set": 1, "SubSet": 1}
]
response = self.make_puzzles_complete_request(puzzles) response = self.make_puzzles_complete_request(puzzles)
self.assertEqual(response.content, self.assertEqual(response.content,
self.set_puzzle_complete_response([13, 53524])) self.set_puzzle_complete_response([13, 53524]))
puzzles = [ {"PuzzleID": 14, "Set": 1, "SubSet": 3}, puzzles = [
{"PuzzleID": 15, "Set": 1, "SubSet": 1} ] {"PuzzleID": 14, "Set": 1, "SubSet": 3},
{"PuzzleID": 15, "Set": 1, "SubSet": 1}
]
response = self.make_puzzles_complete_request(puzzles) response = self.make_puzzles_complete_request(puzzles)
...@@ -350,7 +358,7 @@ class FolditTestCase(TestCase): ...@@ -350,7 +358,7 @@ class FolditTestCase(TestCase):
self.assertTrue(is_complete(1, 2)) self.assertTrue(is_complete(1, 2))
self.assertFalse(is_complete(4, 5)) 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) response = self.make_puzzles_complete_request(puzzles)
...@@ -361,28 +369,30 @@ class FolditTestCase(TestCase): ...@@ -361,28 +369,30 @@ class FolditTestCase(TestCase):
self.assertTrue(is_complete(1, 1, due=self.tomorrow)) self.assertTrue(is_complete(1, 1, due=self.tomorrow))
self.assertFalse(is_complete(1, 1, due=self.yesterday)) self.assertFalse(is_complete(1, 1, due=self.yesterday))
def test_SetPlayerPuzzlesComplete_error(self): # pylint: disable=invalid-name
puzzles = [
def test_SetPlayerPuzzlesComplete_error(self): {"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) puzzles_str = json.dumps(puzzles)
verify = {"Verify": verify_code(self.user.email, puzzles_str + "x"), verify = {
"VerifyMethod":"FoldItVerify"} "Verify": verify_code(self.user.email, puzzles_str + "x"),
"VerifyMethod": "FoldItVerify"
}
data = {'SetPuzzlesCompleteVerify': json.dumps(verify), data = {
'SetPuzzlesComplete': puzzles_str} 'SetPuzzlesCompleteVerify': json.dumps(verify),
'SetPuzzlesComplete': puzzles_str
}
request = self.make_request(data) request = self.make_request(data)
response = foldit_ops(request) response = foldit_ops(request)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
response_data = json.loads(response.content)
self.assertEqual(response.content, self.assertEqual(response.content,
json.dumps([{ json.dumps([{
"OperationID": "SetPuzzlesComplete", "OperationID": "SetPuzzlesComplete",
......
lms/djangoapps/instructor/views/legacy.py
...@@ -3,7 +3,7 @@ Instructor Views ...@@ -3,7 +3,7 @@ Instructor Views
""" """
## NOTE: This is the code for the legacy instructor dashboard ## NOTE: This is the code for the legacy instructor dashboard
## We are no longer supporting this file or accepting changes into it. ## We are no longer supporting this file or accepting changes into it.
# pylint: skip-file
from contextlib import contextmanager from contextlib import contextmanager
import csv import csv
import json import json
...@@ -1427,6 +1427,7 @@ def get_student_grade_summary_data(request, course, get_grades=True, get_raw_sco ...@@ -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 # # Gradebook has moved to instructor.api.spoc_gradebook #
@cache_control(no_cache=True, no_store=True, must_revalidate=True) @cache_control(no_cache=True, no_store=True, must_revalidate=True)
def grade_summary(request, course_key): def grade_summary(request, course_key):
"""Display the grade summary for a course.""" """Display the grade summary for a course."""
......
lms/djangoapps/shoppingcart/models.py
...@@ -24,7 +24,7 @@ from xmodule.modulestore.django import modulestore ...@@ -24,7 +24,7 @@ from xmodule.modulestore.django import modulestore
from course_modes.models import CourseMode from course_modes.models import CourseMode
from edxmako.shortcuts import render_to_string 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 util.query import use_read_replica_if_available
from xmodule_django.models import CourseKeyField from xmodule_django.models import CourseKeyField
...@@ -639,7 +639,7 @@ class CertificateItem(OrderItem): ...@@ -639,7 +639,7 @@ class CertificateItem(OrderItem):
course_enrollment = models.ForeignKey(CourseEnrollment) course_enrollment = models.ForeignKey(CourseEnrollment)
mode = models.SlugField() mode = models.SlugField()
@receiver(unenroll_done) @receiver(UNENROLL_DONE)
def refund_cert_callback(sender, course_enrollment=None, **kwargs): 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 When a CourseEnrollment object calls its unenroll method, this function checks to see if that unenrollment
......
lms/lib/comment_client/comment_client.py
# Import other classes here so they can be imported from here. """Import other classes here so they can be imported from here."""
# pylint: disable=W0611 # pylint: disable=unused-import
from .comment import Comment from .comment import Comment
from .thread import Thread from .thread import Thread
from .user import User from .user import User
......
lms/lib/comment_client/commentable.py
"""Provides base Commentable model class"""
import models import models
import settings import settings
class Commentable(models.Model): class Commentable(models.Model):
base_url = "{prefix}/commentables".format(prefix=settings.PREFIX) base_url = "{prefix}/commentables".format(prefix=settings.PREFIX)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment