From 84ed806f0dd4da076f5396816b1242a9a5e68451 Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 3 Aug 2012 20:11:58 -0400 Subject: [PATCH 01/30] start on lms migration path: view for loaded modules, and reload method --- common/lib/xmodule/xmodule/modulestore/xml.py | 35 ++++++++++++------- common/lib/xmodule/xmodule/x_module.py | 2 ++ lms/envs/dev.py | 5 +++ lms/urls.py | 6 ++++ 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 0567e4e7a7..46fcf19469 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -146,19 +146,30 @@ class XMLModuleStore(ModuleStoreBase): os.path.exists(self.data_dir / d / "course.xml")] for course_dir in course_dirs: - try: - # Special-case code here, since we don't have a location for the - # course before it loads. - # So, make a tracker to track load-time errors, then put in the right - # place after the course loads and we have its location - errorlog = make_error_tracker() - course_descriptor = self.load_course(course_dir, errorlog.tracker) - self.courses[course_dir] = course_descriptor - self._location_errors[course_descriptor.location] = errorlog - except: - msg = "Failed to load course '%s'" % course_dir - log.exception(msg) + self.try_load_course(course_dir) + def try_load_course(self,course_dir): + ''' + Load a course, keeping track of errors as we go along. + ''' + try: + # Special-case code here, since we don't have a location for the + # course before it loads. + # So, make a tracker to track load-time errors, then put in the right + # place after the course loads and we have its location + errorlog = make_error_tracker() + course_descriptor = self.load_course(course_dir, errorlog.tracker) + self.courses[course_dir] = course_descriptor + self._location_errors[course_descriptor.location] = errorlog + except: + msg = "Failed to load course '%s'" % course_dir + log.exception(msg) + + def __unicode__(self): + ''' + String representation - for debugging + ''' + return 'data_dir=%s, %d courses, %d modules' % (self.data_dir,len(self.courses),len(self.modules)) def load_course(self, course_dir, tracker): """ diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index f6a43f2612..60670767f7 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -204,6 +204,8 @@ class XModule(HTMLSnippet): ''' return self.metadata.get('display_name', self.url_name.replace('_', ' ')) + def __unicode__(self): + return '' % (self.name, self.category, self.id) def get_children(self): ''' diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 813471fb54..a4655bf763 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -58,6 +58,11 @@ CACHE_TIMEOUT = 0 # Dummy secret key for dev SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' +################################ LMS Migration ################################# +MITX_FEATURES['ENABLE_LMS_MIGRATION'] = True + +LMS_MIGRATION_ALLOWED_IPS = ['any'] + ################################ OpenID Auth ################################# MITX_FEATURES['AUTH_USE_OPENID'] = True MITX_FEATURES['BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'] = True diff --git a/lms/urls.py b/lms/urls.py index 78198b2dfb..c74e92ea6e 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -169,6 +169,12 @@ if settings.MITX_FEATURES.get('AUTH_USE_OPENID'): url(r'^openid/logo.gif$', 'django_openid_auth.views.logo', name='openid-logo'), ) +if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): + urlpatterns += ( + url(r'^migrate/modules$', 'lms_migration.migrate.manage_modulestores'), + url(r'^migrate/reload/(?P[^/]+)$', 'lms_migration.migrate.manage_modulestores'), + ) + urlpatterns = patterns(*urlpatterns) if settings.DEBUG: From f1ba26b007a181fb2962d493ddd7f1a825bf47ed Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 3 Aug 2012 20:36:17 -0400 Subject: [PATCH 02/30] require login and enrollment in course to be able to view its courseware --- lms/djangoapps/courseware/views.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 18b710e108..831e8ced29 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -150,6 +150,7 @@ def render_accordion(request, course, chapter, section): return render_to_string('accordion.html', context) +@login_required @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) def index(request, course_id, chapter=None, section=None, @@ -172,6 +173,10 @@ def index(request, course_id, chapter=None, section=None, - HTTPresponse ''' course = check_course(course_id) + registered = registered_for_course(course, request.user) + if not registered: + log.debug('User %s tried to view course %s but is not enrolled' % (request.user,course.location.url())) + return redirect('/') try: context = { @@ -266,14 +271,18 @@ def course_info(request, course_id): return render_to_response('info.html', {'course': course}) +def registered_for_course(course, user): + '''Return CourseEnrollment if user is registered for course, else False''' + if user is None: + return False + if user.is_authenticated(): + return CourseEnrollment.objects.filter(user=user, course_id=course.id).exists() + else: + return False + @ensure_csrf_cookie @cache_if_anonymous def course_about(request, course_id): - def registered_for_course(course, user): - if user.is_authenticated(): - return CourseEnrollment.objects.filter(user=user, course_id=course.id).exists() - else: - return False course = check_course(course_id, course_must_be_open=False) registered = registered_for_course(course, request.user) return render_to_response('portal/course_about.html', {'course': course, 'registered': registered}) From 30922fb4491d752a46f269150446618b7abdb444 Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 3 Aug 2012 21:39:23 -0400 Subject: [PATCH 03/30] add ACCESS_REQUIRE_STAFF_FOR_COURSE feature for enrollment check --- common/djangoapps/student/views.py | 9 +++++++++ lms/djangoapps/courseware/courses.py | 20 ++++++++++++++++++++ lms/envs/dev.py | 3 ++- lms/templates/portal/course_about.html | 3 +++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 35ce225011..ace1f8f576 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -37,6 +37,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from models import Registration, UserProfile, PendingNameChange, PendingEmailChange, CourseEnrollment from datetime import date from collections import namedtuple +from courseware.courses import course_staff_group_name, has_staff_access_to_course log = logging.getLogger("mitx.student") Article = namedtuple('Article', 'title url author image deck publication publish_date') @@ -184,6 +185,14 @@ def change_enrollment(request): .format(user.username, enrollment.course_id)) return {'success': False, 'error': 'The course requested does not exist.'} + if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'): + # require that user be in the staff_* group (or be an overall admin) to be able to enroll + # eg staff_6.002x or staff_6.00x + if not has_staff_access_to_course(user,course): + staff_group = course_staff_group_name(course) + log.debug('user %s denied enrollment to %s ; not in %s' % (user,course.location.url(),staff_group)) + return {'success': False, 'error' : '%s membership required to access course.' % staff_group} + enrollment, created = CourseEnrollment.objects.get_or_create(user=user, course_id=course.id) return {'success': True} diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 19eef3ee80..e11fb566f4 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -114,3 +114,23 @@ def get_course_info_section(course, section_key): return "! Info section missing !" raise KeyError("Invalid about key " + str(section_key)) + +def course_staff_group_name(course): + return 'staff_%s' % course.metadata['course'] + +def has_staff_access_to_course(user,course): + ''' + Returns True if the given user has staff access to the course. + This means that user is in the staff_* group, or is an overall admin. + ''' + if user.is_staff: + return True + user_groups = [x[1] for x in user.groups.values_list()] # note this is the Auth group, not UserTestGroup + log.debug('user is in groups %s' % user_groups) + staff_group = course_staff_group_name(course) + if staff_group in user_groups: + return True + return False + + + diff --git a/lms/envs/dev.py b/lms/envs/dev.py index a4655bf763..3e681e6b0d 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -60,12 +60,13 @@ SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' ################################ LMS Migration ################################# MITX_FEATURES['ENABLE_LMS_MIGRATION'] = True +MITX_FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = True LMS_MIGRATION_ALLOWED_IPS = ['any'] ################################ OpenID Auth ################################# MITX_FEATURES['AUTH_USE_OPENID'] = True -MITX_FEATURES['BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'] = True +MITX_FEATURES['BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'] = True # require that user be in the staff_* group to be able to enroll INSTALLED_APPS += ('external_auth',) INSTALLED_APPS += ('django_openid_auth',) diff --git a/lms/templates/portal/course_about.html b/lms/templates/portal/course_about.html index e6359d0542..afecc2f795 100644 --- a/lms/templates/portal/course_about.html +++ b/lms/templates/portal/course_about.html @@ -19,6 +19,8 @@ $(document).delegate('#class_enroll_form', 'ajax:success', function(data, json, xhr) { if(json.success) { location.href="${reverse('dashboard')}"; + }else{ + document.getElementById('register_message').innerHTML = "

" + json.error + "

"; } }); })(this) @@ -63,6 +65,7 @@ You are registered for this course (${course.number}). %else: Register for ${course.number} +
%endif %else: Register for ${course.number} From 10f53d62e3a312b58509d399ea97e2bb7128e4d2 Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 3 Aug 2012 22:38:33 -0400 Subject: [PATCH 04/30] migration views - see modulestore contents, and force reload of course --- lms/djangoapps/lms_migration/__init__.py | 0 lms/djangoapps/lms_migration/migrate.py | 104 +++++++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 lms/djangoapps/lms_migration/__init__.py create mode 100644 lms/djangoapps/lms_migration/migrate.py diff --git a/lms/djangoapps/lms_migration/__init__.py b/lms/djangoapps/lms_migration/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/lms_migration/migrate.py b/lms/djangoapps/lms_migration/migrate.py new file mode 100644 index 0000000000..4da346c657 --- /dev/null +++ b/lms/djangoapps/lms_migration/migrate.py @@ -0,0 +1,104 @@ +# +# migration tools for content team to go from stable-edx4edx to LMS+CMS +# + +import logging +from pprint import pprint +import xmodule.modulestore.django as xmodule_django +from xmodule.modulestore.django import modulestore + +from django.http import HttpResponse +from django.conf import settings + +log = logging.getLogger("mitx.lms_migrate") +LOCAL_DEBUG = True +ALLOWED_IPS = settings.LMS_MIGRATION_ALLOWED_IPS + +def escape(s): + """escape HTML special characters in string""" + return str(s).replace('<','<').replace('>','>') + +def manage_modulestores(request,reload_dir=None): + ''' + Manage the static in-memory modulestores. + + If reload_dir is not None, then instruct the xml loader to reload that course directory. + ''' + html = "" + + def_ms = modulestore() + courses = def_ms.get_courses() + + #---------------------------------------- + # check on IP address of requester + + ip = request.META.get('HTTP_X_REAL_IP','') # nginx reverse proxy + if not ip: + ip = request.META.get('REMOTE_ADDR','None') + + if LOCAL_DEBUG: + html += '

IP address: %s ' % ip + + if not (ip in ALLOWED_IPS or 'any' in ALLOWED_IPS): + html += 'Permission denied' + html += "" + return HttpResponse(html) + + #---------------------------------------- + # reload course if specified + + if reload_dir is not None: + if reload_dir not in def_ms.courses: + html += "

Error: '%s' is not a valid course directory

" % reload_dir + else: + html += "

Reloaded course directory '%s'

" % reload_dir + def_ms.try_load_course(reload_dir) + + #---------------------------------------- + + html += '

Courses loaded in the modulestore

' + html += '
    ' + for cdir, course in def_ms.courses.items(): + html += '
  1. %s (%s)
    2. ' % (settings.MITX_ROOT_URL, + escape(cdir), + escape(cdir), + course.location.url()) + html += '
' + + #---------------------------------------- + + dumpfields = ['definition','location','metadata'] + + for cdir, course in def_ms.courses.items(): + html += '
' + html += '

Course: %s (%s)

' % (course.metadata['display_name'],cdir) + + for field in dumpfields: + data = getattr(course,field) + html += '

%s

' % field + if type(data)==dict: + html += '
    ' + for k,v in data.items(): + html += '
  • %s:%s
    • ' % (escape(k),escape(v)) + html += '
' + else: + html += '
  • %s
' % escape(data) + + + #---------------------------------------- + + html += '
' + html += "courses: 
%s
" % escape(courses) + + ms = xmodule_django._MODULESTORES + html += "modules: 
%s
" % escape(ms) + html += "default modulestore: 
%s
" % escape(unicode(def_ms)) + + #---------------------------------------- + + log.debug('_MODULESTORES=%s' % ms) + log.debug('courses=%s' % courses) + log.debug('def_ms=%s' % unicode(def_ms)) + + html += "" + return HttpResponse(html) From 9de6e28180021c4f0310a981c808635d33f4309b Mon Sep 17 00:00:00 2001 From: ichuang Date: Fri, 3 Aug 2012 22:52:48 -0400 Subject: [PATCH 05/30] limit course reload to localhost or user.is_staff --- lms/djangoapps/lms_migration/migrate.py | 11 ++++++++--- lms/envs/dev.py | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/lms_migration/migrate.py b/lms/djangoapps/lms_migration/migrate.py index 4da346c657..285b26b483 100644 --- a/lms/djangoapps/lms_migration/migrate.py +++ b/lms/djangoapps/lms_migration/migrate.py @@ -38,11 +38,16 @@ def manage_modulestores(request,reload_dir=None): if LOCAL_DEBUG: html += '

IP address: %s ' % ip + log.debug('request from ip=%s' % ip) if not (ip in ALLOWED_IPS or 'any' in ALLOWED_IPS): - html += 'Permission denied' - html += "" - return HttpResponse(html) + if request.user and request.user.is_staff: + log.debug('request allowed because user=%s is staff' % request.user) + else: + html += 'Permission denied' + html += "" + log.debug('request denied, ALLOWED_IPS=%s' % ALLOWED_IPS) + return HttpResponse(html) #---------------------------------------- # reload course if specified diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 3e681e6b0d..d61c2e8b39 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -62,7 +62,7 @@ SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' MITX_FEATURES['ENABLE_LMS_MIGRATION'] = True MITX_FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = True -LMS_MIGRATION_ALLOWED_IPS = ['any'] +LMS_MIGRATION_ALLOWED_IPS = ['127.0.0.1'] ################################ OpenID Auth ################################# MITX_FEATURES['AUTH_USE_OPENID'] = True From c20ebd13c47f6053516fe25b5b15719e6abea5e2 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 08:22:06 -0400 Subject: [PATCH 06/30] in course_about.html make "You are registered..." a link to course --- lms/templates/portal/course_about.html | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lms/templates/portal/course_about.html b/lms/templates/portal/course_about.html index afecc2f795..bdea0a47d1 100644 --- a/lms/templates/portal/course_about.html +++ b/lms/templates/portal/course_about.html @@ -62,7 +62,20 @@
%if user.is_authenticated(): %if registered: + <% + if course.has_started() or settings.MITX_FEATURES['DISABLE_START_DATES']: + course_target = reverse('info', args=[course.id]) + else: + course_target = reverse('about_course', args=[course.id]) + show_link = settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION') + %> + %if show_link: + + %endif You are registered for this course (${course.number}). + %if show_link: + + %endif %else: Register for ${course.number}
From 3f83904c128bf3bd8dcf01bb23039c96cf5c1d61 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 10:19:54 -0400 Subject: [PATCH 07/30] if AUTH_REQUIRE_STAFF_FOR_COURSE then course list = those accessible --- common/djangoapps/student/views.py | 14 +++++--------- lms/djangoapps/courseware/courses.py | 23 ++++++++++++++++++++++- lms/djangoapps/courseware/views.py | 18 +++++------------- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index ace1f8f576..87490786c1 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -8,7 +8,6 @@ import uuid import feedparser import urllib import itertools -from collections import defaultdict from django.conf import settings from django.contrib.auth import logout, authenticate, login @@ -37,7 +36,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from models import Registration, UserProfile, PendingNameChange, PendingEmailChange, CourseEnrollment from datetime import date from collections import namedtuple -from courseware.courses import course_staff_group_name, has_staff_access_to_course +from courseware.courses import course_staff_group_name, has_staff_access_to_course, get_courses_by_university log = logging.getLogger("mitx.student") Article = namedtuple('Article', 'title url author image deck publication publish_date') @@ -65,9 +64,9 @@ def index(request): from external_auth.views import edXauth_ssl_login return edXauth_ssl_login(request) - return main_index() + return main_index(user=request.user) -def main_index(extra_context = {}): +def main_index(extra_context = {}, user=None): ''' Render the edX main page. @@ -89,11 +88,8 @@ def main_index(extra_context = {}): entry.image = soup.img['src'] if soup.img else None entry.summary = soup.getText() - universities = defaultdict(list) - courses = sorted(modulestore().get_courses(), key=lambda course: course.number) - for course in courses: - universities[course.org].append(course) - + # The course selection work is done in courseware.courses. + universities = get_courses_by_university(None) context = {'universities': universities, 'entries': entries} context.update(extra_context) return render_to_response('index.html', context) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index e11fb566f4..c050084fff 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -1,3 +1,4 @@ +from collections import defaultdict from fs.errors import ResourceNotFoundError from functools import wraps import logging @@ -123,6 +124,8 @@ def has_staff_access_to_course(user,course): Returns True if the given user has staff access to the course. This means that user is in the staff_* group, or is an overall admin. ''' + if user is None or (not user.is_authenticated()) or course is None: + return False if user.is_staff: return True user_groups = [x[1] for x in user.groups.values_list()] # note this is the Auth group, not UserTestGroup @@ -132,5 +135,23 @@ def has_staff_access_to_course(user,course): return True return False - +def get_courses_by_university(user): + ''' + Returns dict of lists of courses available, keyed by course.org (ie university). + Courses are sorted by course.number. + + if ACCESS_REQUIRE_STAFF_FOR_COURSE then list only includes those accessible to user. + ''' + # TODO: Clean up how 'error' is done. + # filter out any courses that errored. + courses = [c for c in modulestore().get_courses() + if isinstance(c, CourseDescriptor)] + courses = sorted(courses, key=lambda course: course.number) + universities = defaultdict(list) + for course in courses: + if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): + if not has_staff_access_to_course(user,course): + continue + universities[course.org].append(course) + return universities diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 831e8ced29..41b2101b44 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -1,4 +1,3 @@ -from collections import defaultdict import json import logging import urllib @@ -28,7 +27,7 @@ from xmodule.course_module import CourseDescriptor from util.cache import cache, cache_if_anonymous from student.models import UserTestGroup, CourseEnrollment from courseware import grades -from courseware.courses import check_course +from courseware.courses import check_course, get_courses_by_university log = logging.getLogger("mitx.courseware") @@ -58,19 +57,12 @@ def user_groups(user): @ensure_csrf_cookie @cache_if_anonymous def courses(request): - # TODO: Clean up how 'error' is done. - - # filter out any courses that errored. - courses = [c for c in modulestore().get_courses() - if isinstance(c, CourseDescriptor)] - courses = sorted(courses, key=lambda course: course.number) - universities = defaultdict(list) - for course in courses: - universities[course.org].append(course) - + ''' + Render "find courses" page. The course selection work is done in courseware.courses. + ''' + universities = get_courses_by_university(request.user) return render_to_response("courses.html", {'universities': universities}) - @cache_control(no_cache=True, no_store=True, must_revalidate=True) def gradebook(request, course_id): if 'course_admin' not in user_groups(request.user): From fb7b48e10af14f643ce038459f1d3cf56197414e Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 10:28:05 -0400 Subject: [PATCH 08/30] minor - have migrate also show user when debugging --- lms/djangoapps/lms_migration/migrate.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/lms_migration/migrate.py b/lms/djangoapps/lms_migration/migrate.py index 285b26b483..2bf893507b 100644 --- a/lms/djangoapps/lms_migration/migrate.py +++ b/lms/djangoapps/lms_migration/migrate.py @@ -38,7 +38,8 @@ def manage_modulestores(request,reload_dir=None): if LOCAL_DEBUG: html += '

IP address: %s ' % ip - log.debug('request from ip=%s' % ip) + html += '

User: %s ' % request.user + log.debug('request from ip=%s, user=%s' % (ip,request.user)) if not (ip in ALLOWED_IPS or 'any' in ALLOWED_IPS): if request.user and request.user.is_staff: From d50af5765e009ff9c142f24c9f87b061007d20b0 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 11:03:54 -0400 Subject: [PATCH 09/30] make university profile pages also use get_courses_by_university --- common/lib/xmodule/xmodule/xml_module.py | 1 + lms/djangoapps/courseware/courses.py | 7 ++++++- lms/djangoapps/courseware/views.py | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index b0a289d149..d46304b067 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -41,6 +41,7 @@ class XmlDescriptor(XModuleDescriptor): # to definition_from_xml, and from the xml returned by definition_to_xml metadata_attributes = ('format', 'graceperiod', 'showanswer', 'rerandomize', 'start', 'due', 'graded', 'display_name', 'url_name', 'hide_from_toc', + 'ispublic', # if True, then course is listed for all users; see # VS[compat] Remove once unused. 'name', 'slug') diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index c050084fff..78025c2fae 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -135,6 +135,11 @@ def has_staff_access_to_course(user,course): return True return False +def has_access_to_course(user,course): + if course.metadata.get('ispublic'): + return True + return has_staff_access_to_course(user,course) + def get_courses_by_university(user): ''' Returns dict of lists of courses available, keyed by course.org (ie university). @@ -150,7 +155,7 @@ def get_courses_by_university(user): universities = defaultdict(list) for course in courses: if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): - if not has_staff_access_to_course(user,course): + if not has_access_to_course(user,course): continue universities[course.org].append(course) return universities diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 41b2101b44..5db3bcf91a 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -289,7 +289,7 @@ def university_profile(request, org_id): raise Http404("University Profile not found for {0}".format(org_id)) # Only grab courses for this org... - courses = [c for c in all_courses if c.org == org_id] + courses = get_courses_by_university(request.user)[org_id] context = dict(courses=courses, org_id=org_id) template_file = "university_profile/{0}.html".format(org_id).lower() From b8ae026c2937b0a3bbe66278033fe2fc9fc326e4 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 11:16:47 -0400 Subject: [PATCH 10/30] fail gracefully if course.xml missing metadata in course_staff_group_name --- lms/djangoapps/courseware/courses.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 78025c2fae..133a593ac8 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -117,7 +117,10 @@ def get_course_info_section(course, section_key): raise KeyError("Invalid about key " + str(section_key)) def course_staff_group_name(course): - return 'staff_%s' % course.metadata['course'] + coursename = course.metadata.get('course','') + if not coursename: # Fall 2012: not all course.xml have metadata correct yet + coursename = course.metadata.get('data_dir','UnknownCourseName') + return 'staff_%s' % coursename def has_staff_access_to_course(user,course): ''' From 85af1d88cf46c2279caa4974ba364ebf5bbfb020 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 15:30:36 -0400 Subject: [PATCH 11/30] utility scripts to create new users, and staff_* groups for courses --- utility-scripts/create_groups.py | 32 ++++++++ utility-scripts/create_user.py | 137 +++++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 utility-scripts/create_groups.py create mode 100644 utility-scripts/create_user.py diff --git a/utility-scripts/create_groups.py b/utility-scripts/create_groups.py new file mode 100644 index 0000000000..841242fd54 --- /dev/null +++ b/utility-scripts/create_groups.py @@ -0,0 +1,32 @@ +#!/usr/bin/python +# +# File: create_groups.py +# Date: 04-Aug-12 +# Author: I. Chuang +# +# Create all staff_* groups for classes in data directory. + +import os, sys, string, re + +sys.path.append(os.path.abspath('.')) +os.environ['DJANGO_SETTINGS_MODULE'] = 'lms.envs.dev' +from lms.envs.dev import * + +from django.conf import settings +from django.contrib.auth.models import User, Group +from path import path + +data_dir = settings.DATA_DIR +print "data_dir = %s" % data_dir + +for course_dir in os.listdir(data_dir): + # print course_dir + if not os.path.isdir(path(data_dir) / course_dir): + continue + gname = 'staff_%s' % course_dir + if Group.objects.filter(name=gname): + print "group exists for %s" % gname + continue + g = Group(name=gname) + g.save() + print "created group %s" % gname diff --git a/utility-scripts/create_user.py b/utility-scripts/create_user.py new file mode 100644 index 0000000000..19bdb6e743 --- /dev/null +++ b/utility-scripts/create_user.py @@ -0,0 +1,137 @@ +#!/usr/bin/python +# +# File: create_user.py +# Date: 04-Aug-12 +# Author: I. Chuang +# +# Create user. Prompt for groups and ExternalAuthMap + +import os, sys, string, re +import datetime +from getpass import getpass +import json +import readline + +sys.path.append(os.path.abspath('.')) +os.environ['DJANGO_SETTINGS_MODULE'] = 'lms.envs.dev' +from lms.envs.dev import * + +from student.models import UserProfile, Registration +from external_auth.models import ExternalAuthMap +from django.contrib.auth.models import User, Group +from random import choice + +class MyCompleter(object): # Custom completer + + def __init__(self, options): + self.options = sorted(options) + + def complete(self, text, state): + if state == 0: # on first trigger, build possible matches + if text: # cache matches (entries that start with entered text) + self.matches = [s for s in self.options + if s and s.startswith(text)] + else: # no text entered, all matches possible + self.matches = self.options[:] + + # return match indexed by state + try: + return self.matches[state] + except IndexError: + return None + +def GenPasswd(length=8, chars=string.letters + string.digits): + return ''.join([choice(chars) for i in range(length)]) + +#----------------------------------------------------------------------------- +# main + +while True: + uname = raw_input('username: ') + if User.objects.filter(username=uname): + print "username %s already taken" % uname + else: + break + +while True: + email = raw_input('email: ') + if User.objects.filter(email=email): + print "email %s already taken" % email + else: + break + +name = raw_input('Full name: ') + +make_eamap = False +if raw_input('Create MIT ExternalAuth? [n] ').lower()=='y': + if not email.endswith('@MIT.EDU'): + print "Failed - email must be @MIT.EDU" + sys.exit(-1) + mit_domain = 'ssl:MIT' + if ExternalAuthMap.objects.filter(external_id = email, external_domain = mit_domain): + print "Failed - email %s already exists as external_id" % email + sys.exit(-1) + make_eamap = True + password = GenPasswd(12) +else: + while True: + password = getpass() + password2 = getpass() + if password == password2: + break + print "Oops, passwords do not match, please retry" + +user = User(username=uname, email=email, is_active=True) +user.set_password(password) +try: + user.save() +except IntegrityError: + print "Oops, failed to create user %s, IntegrityError" % user + raise + +r = Registration() +r.register(user) + +up = UserProfile(user=user) +up.name = name +up.save() + +if make_eamap: + credentials = "/C=US/ST=Massachusetts/O=Massachusetts Institute of Technology/OU=Client CA v1/CN=%s/emailAddress=%s" % (name,email) + eamap = ExternalAuthMap(external_id = email, + external_email = email, + external_domain = mit_domain, + external_name = name, + internal_password = password, + external_credentials = json.dumps(credentials), + ) + eamap.user = user + eamap.dtsignup = datetime.datetime.now() + eamap.save() + +print "User %s created successfully!" % user + +if not raw_input('Add user %s to any groups? [n] ' % user).lower()=='y': + sys.exit(0) + +print "Here are the groups available:" + +groups = [str(g.name) for g in Group.objects.all()] +print groups + +completer = MyCompleter(groups) +readline.set_completer(completer.complete) +readline.parse_and_bind('tab: complete') + +while True: + gname = raw_input("Add group (tab to autocomplete, empty line to end): ") + if not gname: + break + if not gname in groups: + print "Unknown group %s" % gname + continue + g = Group.objects.get(name=gname) + user.groups.add(g) + print "Added %s to group %s" % (user,g) + +print "Done!" From 7fe75030cc64050e8997d011bab39174aa9c24ad Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 17:52:14 -0400 Subject: [PATCH 12/30] fix staff edit link in module content display (goes to github) --- common/djangoapps/xmodule_modifiers.py | 35 +++++++++++++++------- common/lib/xmodule/xmodule/html_module.py | 10 ++++++- common/lib/xmodule/xmodule/xml_module.py | 11 +++++-- lms/djangoapps/courseware/courses.py | 8 ++++- lms/djangoapps/courseware/module_render.py | 5 +++- lms/templates/staff_problem_info.html | 8 ++--- 6 files changed, 57 insertions(+), 20 deletions(-) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 4d412000ec..082c5f5122 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -1,9 +1,15 @@ +import re import json +import logging + from django.conf import settings from functools import wraps from static_replace import replace_urls from mitxmako.shortcuts import render_to_string +from xmodule.seq_module import SequenceModule +from xmodule.vertical_module import VerticalModule +log = logging.getLogger("mitx.xmodule_modifiers") def wrap_xmodule(get_html, module, template): """ @@ -69,27 +75,33 @@ def add_histogram(get_html, module): the output of the old get_html function with additional information for admin users only, including a histogram of student answers and the definition of the xmodule + + Does nothing if module is a SequenceModule """ @wraps(get_html) def _get_html(): + + if type(module) in [SequenceModule, VerticalModule]: # TODO: make this more general, eg use an XModule attribute instead + return get_html() + module_id = module.id histogram = grade_histogram(module_id) render_histogram = len(histogram) > 0 - # TODO: fixme - no filename in module.xml in general (this code block - # for edx4edx) the following if block is for summer 2012 edX course - # development; it will change when the CMS comes online - if settings.MITX_FEATURES.get('DISPLAY_EDIT_LINK') and settings.DEBUG and module_xml.get('filename') is not None: - coursename = multicourse_settings.get_coursename_from_request(request) - github_url = multicourse_settings.get_course_github_url(coursename) - fn = module_xml.get('filename') - if module_xml.tag=='problem': fn = 'problems/' + fn # grrr - edit_link = (github_url + '/tree/master/' + fn) if github_url is not None else None - if module_xml.tag=='problem': edit_link += '.xml' # grrr + # TODO (ichuang): Remove after fall 2012 LMS migration done + if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): + filename = module.definition.get('filename','') + log.debug('filename = %s' % filename) + data_dir = module.system.filestore.root_path.rsplit('/')[-1] + edit_link = "https://github.com/MITx/%s/tree/master/%s" % (data_dir,filename) + log.debug('edit_link = %s' % edit_link) + log.debug('module = %s' % dir(module)) + log.debug('module type = %s' % type(module)) + log.debug('location = %s' % str(module.location)) else: edit_link = False - staff_context = {'definition': json.dumps(module.definition, indent=4), + staff_context = {'definition': module.definition.get('data'), 'metadata': json.dumps(module.metadata, indent=4), 'element_id': module.location.html_id(), 'edit_link': edit_link, @@ -99,3 +111,4 @@ def add_histogram(get_html, module): return render_to_string("staff_problem_info.html", staff_context) return _get_html + diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 7a09004e33..8af1a9c5ba 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -94,7 +94,15 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): msg = "Couldn't parse html in {0}.".format(filepath) log.warning(msg) system.error_tracker("Warning: " + msg) - return {'data' : html} + + definition = {'data' : html} + + # TODO (ichuang): remove this after migration + # for Fall 2012 LMS migration: keep filename + definition['filename'] = filepath + + return definition + except (ResourceNotFoundError) as err: msg = 'Unable to load file contents at path {0}: {1} '.format( filepath, err) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index d46304b067..dee87921d9 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -9,7 +9,7 @@ from fs.errors import ResourceNotFoundError import os import sys -log = logging.getLogger(__name__) +log = logging.getLogger('mitx.' + __name__) _AttrMapBase = namedtuple('_AttrMap', 'metadata_key to_metadata from_metadata') @@ -110,6 +110,7 @@ class XmlDescriptor(XModuleDescriptor): filename = xml_object.get('filename') if filename is None: definition_xml = copy.deepcopy(xml_object) + filepath = '' else: filepath = cls._format_filepath(xml_object.tag, filename) @@ -137,7 +138,13 @@ class XmlDescriptor(XModuleDescriptor): raise Exception, msg, sys.exc_info()[2] cls.clean_metadata_from_xml(definition_xml) - return cls.definition_from_xml(definition_xml, system) + definition = cls.definition_from_xml(definition_xml, system) + + # TODO (ichuang): remove this after migration + # for Fall 2012 LMS migration: keep filename + definition['filename'] = filepath + + return definition @classmethod diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 133a593ac8..e568f97f56 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -117,7 +117,13 @@ def get_course_info_section(course, section_key): raise KeyError("Invalid about key " + str(section_key)) def course_staff_group_name(course): - coursename = course.metadata.get('course','') + ''' + course should be either a CourseDescriptor instance, or a string (the .course entry of a Location) + ''' + if type(course)==str: + coursename = course + else: + coursename = course.metadata.get('course','') if not coursename: # Fall 2012: not all course.xml have metadata correct yet coursename = course.metadata.get('data_dir','UnknownCourseName') return 'staff_%s' % coursename diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 9260e15c61..cdb9dc40f3 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -15,6 +15,8 @@ from xmodule.exceptions import NotFoundError from xmodule.x_module import ModuleSystem from xmodule_modifiers import replace_static_urls, add_histogram, wrap_xmodule +from courseware.courses import has_staff_access_to_course + log = logging.getLogger("mitx.courseware") @@ -188,7 +190,8 @@ def get_module(user, request, location, student_module_cache, position=None): module.metadata['data_dir'] ) - if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and user.is_staff: + if (settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and + (user.is_staff or has_staff_access_to_course(user, module.location.course))): module.get_html = add_histogram(module.get_html, module) # If StudentModule for this instance wasn't already in the database, diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index f9fa999ae9..c9b92c51db 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -1,11 +1,11 @@ ${module_content} -
-definition = ${definition | h} -metadata = ${metadata | h} -
%if edit_link: % endif +
+definition = 
${definition | h}
+metadata = ${metadata | h} +
%if render_histogram:
%endif From 23669f5aa1fc23b049f53032fa073f171e2aec89 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 17:56:32 -0400 Subject: [PATCH 13/30] add some error handling to utility scripts --- utility-scripts/create_groups.py | 7 ++++++- utility-scripts/create_user.py | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/utility-scripts/create_groups.py b/utility-scripts/create_groups.py index 841242fd54..063d2ae392 100644 --- a/utility-scripts/create_groups.py +++ b/utility-scripts/create_groups.py @@ -10,7 +10,12 @@ import os, sys, string, re sys.path.append(os.path.abspath('.')) os.environ['DJANGO_SETTINGS_MODULE'] = 'lms.envs.dev' -from lms.envs.dev import * + +try: + from lms.envs.dev import * +except Exception as err: + print "Run this script from the top-level mitx directory (mitx_all/mitx), not a subdirectory." + sys.exit(-1) from django.conf import settings from django.contrib.auth.models import User, Group diff --git a/utility-scripts/create_user.py b/utility-scripts/create_user.py index 19bdb6e743..c9708f537d 100644 --- a/utility-scripts/create_user.py +++ b/utility-scripts/create_user.py @@ -14,7 +14,12 @@ import readline sys.path.append(os.path.abspath('.')) os.environ['DJANGO_SETTINGS_MODULE'] = 'lms.envs.dev' -from lms.envs.dev import * + +try: + from lms.envs.dev import * +except Exception as err: + print "Run this script from the top-level mitx directory (mitx_all/mitx), not a subdirectory." + sys.exit(-1) from student.models import UserProfile, Registration from external_auth.models import ExternalAuthMap From ebe6bf4888611364f8f36a2b38632bf4f29121a1 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 21:10:50 -0400 Subject: [PATCH 14/30] remove some unnecessary debugging lines in xmodule_modifiers --- common/djangoapps/xmodule_modifiers.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 082c5f5122..221ad31116 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -91,13 +91,8 @@ def add_histogram(get_html, module): # TODO (ichuang): Remove after fall 2012 LMS migration done if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): filename = module.definition.get('filename','') - log.debug('filename = %s' % filename) data_dir = module.system.filestore.root_path.rsplit('/')[-1] edit_link = "https://github.com/MITx/%s/tree/master/%s" % (data_dir,filename) - log.debug('edit_link = %s' % edit_link) - log.debug('module = %s' % dir(module)) - log.debug('module type = %s' % type(module)) - log.debug('location = %s' % str(module.location)) else: edit_link = False From 9db88b0b52c23513b757b30bc47ef828e99bde8a Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 21:13:43 -0400 Subject: [PATCH 15/30] fix comment in dev.py --- lms/envs/dev.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/envs/dev.py b/lms/envs/dev.py index d61c2e8b39..50062e0513 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -60,13 +60,13 @@ SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' ################################ LMS Migration ################################# MITX_FEATURES['ENABLE_LMS_MIGRATION'] = True -MITX_FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = True +MITX_FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = True # require that user be in the staff_* group to be able to enroll LMS_MIGRATION_ALLOWED_IPS = ['127.0.0.1'] ################################ OpenID Auth ################################# MITX_FEATURES['AUTH_USE_OPENID'] = True -MITX_FEATURES['BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'] = True # require that user be in the staff_* group to be able to enroll +MITX_FEATURES['BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'] = True INSTALLED_APPS += ('external_auth',) INSTALLED_APPS += ('django_openid_auth',) From 1ff49aa3f9da856c2ca213f108f11bc6eb3dd3fe Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 4 Aug 2012 21:15:51 -0400 Subject: [PATCH 16/30] remove unnecessary comments from util-scripts/* --- utility-scripts/create_groups.py | 2 -- utility-scripts/create_user.py | 2 -- 2 files changed, 4 deletions(-) diff --git a/utility-scripts/create_groups.py b/utility-scripts/create_groups.py index 063d2ae392..33c563127f 100644 --- a/utility-scripts/create_groups.py +++ b/utility-scripts/create_groups.py @@ -1,8 +1,6 @@ #!/usr/bin/python # # File: create_groups.py -# Date: 04-Aug-12 -# Author: I. Chuang # # Create all staff_* groups for classes in data directory. diff --git a/utility-scripts/create_user.py b/utility-scripts/create_user.py index c9708f537d..e5cb5aed2c 100644 --- a/utility-scripts/create_user.py +++ b/utility-scripts/create_user.py @@ -1,8 +1,6 @@ #!/usr/bin/python # # File: create_user.py -# Date: 04-Aug-12 -# Author: I. Chuang # # Create user. Prompt for groups and ExternalAuthMap From 3c23235885d3d78a2530bde95c4e7aa893456ef7 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 5 Aug 2012 11:39:56 -0400 Subject: [PATCH 17/30] fix for some broken github edit links - avoids symlinks --- common/djangoapps/xmodule_modifiers.py | 9 ++++++--- common/lib/xmodule/xmodule/html_module.py | 4 ++-- common/lib/xmodule/xmodule/xml_module.py | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 221ad31116..843d2eaa38 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -90,9 +90,12 @@ def add_histogram(get_html, module): # TODO (ichuang): Remove after fall 2012 LMS migration done if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): - filename = module.definition.get('filename','') - data_dir = module.system.filestore.root_path.rsplit('/')[-1] - edit_link = "https://github.com/MITx/%s/tree/master/%s" % (data_dir,filename) + [filepath, filename] = module.definition.get('filename','') + osfs = module.system.filestore + if osfs.exists(filename): + filepath = filename # if original, unmangled filename exists then use it (github doesn't like symlinks) + data_dir = osfs.root_path.rsplit('/')[-1] + edit_link = "https://github.com/MITx/%s/tree/master/%s" % (data_dir,filepath) else: edit_link = False diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 8af1a9c5ba..260b84278b 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -98,8 +98,8 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): definition = {'data' : html} # TODO (ichuang): remove this after migration - # for Fall 2012 LMS migration: keep filename - definition['filename'] = filepath + # for Fall 2012 LMS migration: keep filename (and unmangled filename) + definition['filename'] = [ filepath, filename ] return definition diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index dee87921d9..fbb17fd236 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -141,8 +141,8 @@ class XmlDescriptor(XModuleDescriptor): definition = cls.definition_from_xml(definition_xml, system) # TODO (ichuang): remove this after migration - # for Fall 2012 LMS migration: keep filename - definition['filename'] = filepath + # for Fall 2012 LMS migration: keep filename (and unmangled filename) + definition['filename'] = [ filepath, filename ] return definition From 3ee224e3994fb66709dcab2a460d77329d3d4a6f Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 5 Aug 2012 12:39:13 -0400 Subject: [PATCH 18/30] improve create_user script slightly, to auto-grab fullname for MIT users --- utility-scripts/create_user.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/utility-scripts/create_user.py b/utility-scripts/create_user.py index e5cb5aed2c..3ce9ce0ecf 100644 --- a/utility-scripts/create_user.py +++ b/utility-scripts/create_user.py @@ -56,17 +56,9 @@ while True: else: break -while True: - email = raw_input('email: ') - if User.objects.filter(email=email): - print "email %s already taken" % email - else: - break - -name = raw_input('Full name: ') - make_eamap = False if raw_input('Create MIT ExternalAuth? [n] ').lower()=='y': + email = '%s@MIT.EDU' % uname if not email.endswith('@MIT.EDU'): print "Failed - email must be @MIT.EDU" sys.exit(-1) @@ -76,6 +68,13 @@ if raw_input('Create MIT ExternalAuth? [n] ').lower()=='y': sys.exit(-1) make_eamap = True password = GenPasswd(12) + + # get name from kerberos + kname = os.popen("finger %s | grep 'name:'" % email).read().strip().split('name: ')[1].strip() + name = raw_input('Full name: [%s] ' % kname).strip() + if name=='': + name = kname + print "name = %s" % name else: while True: password = getpass() @@ -84,6 +83,16 @@ else: break print "Oops, passwords do not match, please retry" + while True: + email = raw_input('email: ') + if User.objects.filter(email=email): + print "email %s already taken" % email + else: + break + + name = raw_input('Full name: ') + + user = User(username=uname, email=email, is_active=True) user.set_password(password) try: From c42960c172604472ae8a8a98349b2cdd986717ba Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 5 Aug 2012 16:32:58 -0400 Subject: [PATCH 19/30] add feature ENABLE_SQL_TRACKING_LOGS and url view /event_logs --- common/djangoapps/track/models.py | 20 +++++++++++++++++++- common/djangoapps/track/views.py | 29 +++++++++++++++++++++++++++-- lms/envs/dev.py | 1 + lms/urls.py | 5 +++++ 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/track/models.py b/common/djangoapps/track/models.py index 71a8362390..401fa2832f 100644 --- a/common/djangoapps/track/models.py +++ b/common/djangoapps/track/models.py @@ -1,3 +1,21 @@ from django.db import models -# Create your models here. +from django.db import models + +class TrackingLog(models.Model): + dtcreated = models.DateTimeField('creation date',auto_now_add=True) + username = models.CharField(max_length=32,blank=True) + ip = models.CharField(max_length=32,blank=True) + event_source = models.CharField(max_length=32) + event_type = models.CharField(max_length=32,blank=True) + event = models.TextField(blank=True) + agent = models.CharField(max_length=256,blank=True) + page = models.CharField(max_length=32,blank=True,null=True) + time = models.DateTimeField('event time') + + def __unicode__(self): + s = "[%s] %s@%s: %s | %s | %s | %s" % (self.time, self.username, self.ip, self.event_source, + self.event_type, self.page, self.event) + return s + + diff --git a/common/djangoapps/track/views.py b/common/djangoapps/track/views.py index a60d8bef28..1123b8d30a 100644 --- a/common/djangoapps/track/views.py +++ b/common/djangoapps/track/views.py @@ -2,19 +2,32 @@ import json import logging import os import datetime +import dateutil.parser -# Create your views here. +from django.contrib.auth.decorators import login_required from django.http import HttpResponse from django.http import Http404 +from django.shortcuts import redirect from django.conf import settings +from mitxmako.shortcuts import render_to_response + +from django_future.csrf import ensure_csrf_cookie +from track.models import TrackingLog log = logging.getLogger("tracking") +LOGFIELDS = ['username','ip','event_source','event_type','event','agent','page','time'] def log_event(event): event_str = json.dumps(event) log.info(event_str[:settings.TRACK_MAX_EVENT]) - + if settings.MITX_FEATURES.get('ENABLE_SQL_TRACKING_LOGS'): + event['time'] = dateutil.parser.parse(event['time']) + tldat = TrackingLog(**dict([(x,event[x]) for x in LOGFIELDS])) + try: + tldat.save() + except Exception as err: + log.debug(err) def user_track(request): try: # TODO: Do the same for many of the optional META parameters @@ -70,4 +83,16 @@ def server_track(request, event_type, event, page=None): "page": page, "time": datetime.datetime.utcnow().isoformat(), } + + if event_type=="/event_logs" and request.user.is_staff: # don't log + return log_event(event) + +@login_required +@ensure_csrf_cookie +def view_tracking_log(request): + if not request.user.is_staff: + return redirect('/') + record_instances = TrackingLog.objects.all().order_by('-time')[0:100] + return render_to_response('tracking_log.html',{'records':record_instances}) + diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 50062e0513..204fcec04b 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -14,6 +14,7 @@ DEBUG = True TEMPLATE_DEBUG = True MITX_FEATURES['DISABLE_START_DATES'] = True +MITX_FEATURES['ENABLE_SQL_TRACKING_LOGS'] = True WIKI_ENABLED = True diff --git a/lms/urls.py b/lms/urls.py index c74e92ea6e..9dc317039e 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -175,6 +175,11 @@ if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): url(r'^migrate/reload/(?P[^/]+)$', 'lms_migration.migrate.manage_modulestores'), ) +if settings.MITX_FEATURES.get('ENABLE_SQL_TRACKING_LOGS'): + urlpatterns += ( + url(r'^event_logs$', 'track.views.view_tracking_log'), + ) + urlpatterns = patterns(*urlpatterns) if settings.DEBUG: From 190f1f8f892e352ad88a13f0f16aa1331906e014 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 5 Aug 2012 16:35:32 -0400 Subject: [PATCH 20/30] tracking_log template --- lms/templates/tracking_log.html | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 lms/templates/tracking_log.html diff --git a/lms/templates/tracking_log.html b/lms/templates/tracking_log.html new file mode 100644 index 0000000000..66d375c2f3 --- /dev/null +++ b/lms/templates/tracking_log.html @@ -0,0 +1,14 @@ + +

Tracking Log

+ +% for rec in records: + + + + + + + +% endfor +
datetimeusernameipaddrsourcetype
${rec.time}${rec.username}${rec.ip}${rec.event_source}${rec.event_type}
+ \ No newline at end of file From 0347eb498c40efb45637045c3ec8e6e50deb1e99 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 5 Aug 2012 20:24:00 -0400 Subject: [PATCH 21/30] add MITX_FEATURES flags to enable textbook and discussion, and modify course_navigation correspondingly --- lms/envs/common.py | 11 +++ lms/envs/dev_ike.py | 107 +-------------------------- lms/templates/course_navigation.html | 6 ++ 3 files changed, 21 insertions(+), 103 deletions(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index d89e6760a7..83a4bd4181 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -48,6 +48,17 @@ MITX_FEATURES = { ## DO NOT SET TO True IN THIS FILE ## Doing so will cause all courses to be released on production 'DISABLE_START_DATES': False, # When True, all courses will be active, regardless of start date + + 'ENABLE_TEXTBOOK' : True, + 'ENABLE_DISCUSSION' : True, + + 'ENABLE_SQL_TRACKING_LOGS': False, + 'ENABLE_LMS_MIGRATION': False, + + # extrernal access methods + 'ACCESS_REQUIRE_STAFF_FOR_COURSE': False, + 'AUTH_USE_OPENID': False, + 'AUTH_USE_MIT_CERTIFICATES' : False, } # Used for A/B testing diff --git a/lms/envs/dev_ike.py b/lms/envs/dev_ike.py index fb7d980550..b6cd67dfd8 100644 --- a/lms/envs/dev_ike.py +++ b/lms/envs/dev_ike.py @@ -9,108 +9,9 @@ sessions. Assumes structure: """ from .common import * from .logsettings import get_logger_config +from .dev import * -DEBUG = True -TEMPLATE_DEBUG = True +WIKI_ENABLED = False +MITX_FEATURES['ENABLE_TEXTBOOK'] = False +MITX_FEATURES['ENABLE_DISCUSSION'] = False -MITX_FEATURES['DISABLE_START_DATES'] = True - -WIKI_ENABLED = True - -LOGGING = get_logger_config(ENV_ROOT / "log", - logging_env="dev", - tracking_filename="tracking.log", - debug=True) - -DATABASES = { - 'default': { - 'ENGINE': 'django.db.backends.sqlite3', - 'NAME': ENV_ROOT / "db" / "mitx.db", - } -} - -CACHES = { - # This is the cache used for most things. Askbot will not work without a - # functioning cache -- it relies on caching to load its settings in places. - # In staging/prod envs, the sessions also live here. - 'default': { - 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', - 'LOCATION': 'mitx_loc_mem_cache', - 'KEY_FUNCTION': 'util.memcache.safe_key', - }, - - # The general cache is what you get if you use our util.cache. It's used for - # things like caching the course.xml file for different A/B test groups. - # We set it to be a DummyCache to force reloading of course.xml in dev. - # In staging environments, we would grab VERSION from data uploaded by the - # push process. - 'general': { - 'BACKEND': 'django.core.cache.backends.dummy.DummyCache', - 'KEY_PREFIX': 'general', - 'VERSION': 4, - 'KEY_FUNCTION': 'util.memcache.safe_key', - } -} - -# Dummy secret key for dev -SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' - -################################ OpenID Auth ################################# -MITX_FEATURES['AUTH_USE_OPENID'] = True - -INSTALLED_APPS += ('external_auth',) -INSTALLED_APPS += ('django_openid_auth',) -#INSTALLED_APPS += ('ssl_auth',) - -#MIDDLEWARE_CLASSES += ( -# #'ssl_auth.ssl_auth.NginxProxyHeaderMiddleware', # ssl authentication behind nginx proxy -# ) - -#AUTHENTICATION_BACKENDS = ( -# 'django_openid_auth.auth.OpenIDBackend', -# 'django.contrib.auth.backends.ModelBackend', -# ) - -OPENID_CREATE_USERS = False -OPENID_UPDATE_DETAILS_FROM_SREG = True -OPENID_SSO_SERVER_URL = 'https://www.google.com/accounts/o8/id' -OPENID_USE_AS_ADMIN_LOGIN = False -#import external_auth.views as edXauth -#OPENID_RENDER_FAILURE = edXauth.edXauth_openid - -################################ DEBUG TOOLBAR ################################# -INSTALLED_APPS += ('debug_toolbar',) -MIDDLEWARE_CLASSES += ('debug_toolbar.middleware.DebugToolbarMiddleware',) -INTERNAL_IPS = ('127.0.0.1',) - -DEBUG_TOOLBAR_PANELS = ( - 'debug_toolbar.panels.version.VersionDebugPanel', - 'debug_toolbar.panels.timer.TimerDebugPanel', - 'debug_toolbar.panels.settings_vars.SettingsVarsDebugPanel', - 'debug_toolbar.panels.headers.HeaderDebugPanel', - 'debug_toolbar.panels.request_vars.RequestVarsDebugPanel', - 'debug_toolbar.panels.sql.SQLDebugPanel', - 'debug_toolbar.panels.signals.SignalDebugPanel', - 'debug_toolbar.panels.logger.LoggingPanel', - -# Enabling the profiler has a weird bug as of django-debug-toolbar==0.9.4 and -# Django=1.3.1/1.4 where requests to views get duplicated (your method gets -# hit twice). So you can uncomment when you need to diagnose performance -# problems, but you shouldn't leave it on. -# 'debug_toolbar.panels.profiling.ProfilingDebugPanel', -) - -############################ FILE UPLOADS (ASKBOT) ############################# -DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage' -MEDIA_ROOT = ENV_ROOT / "uploads" -MEDIA_URL = "/static/uploads/" -STATICFILES_DIRS.append(("uploads", MEDIA_ROOT)) -FILE_UPLOAD_TEMP_DIR = ENV_ROOT / "uploads" -FILE_UPLOAD_HANDLERS = ( - 'django.core.files.uploadhandler.MemoryFileUploadHandler', - 'django.core.files.uploadhandler.TemporaryFileUploadHandler', -) - -########################### PIPELINE ################################# - -PIPELINE_SASS_ARGUMENTS = '-r {proj_dir}/static/sass/bourbon/lib/bourbon.rb'.format(proj_dir=PROJECT_ROOT) diff --git a/lms/templates/course_navigation.html b/lms/templates/course_navigation.html index 8bda22148d..84b0c04ca0 100644 --- a/lms/templates/course_navigation.html +++ b/lms/templates/course_navigation.html @@ -14,10 +14,16 @@ def url_class(url):
  • Courseware
  • Course Info
    • % if user.is_authenticated(): +% if settings.MITX_FEATURES.get('ENABLE_TEXTBOOK'):
  • Textbook
    • +% endif +% if settings.MITX_FEATURES.get('ENABLE_DISCUSSION'):
  • Discussion
    • +% endif % endif +% if settings.WIKI_ENABLED:
  • Wiki
    • +% endif % if user.is_authenticated():
  • Profile
    • % endif From 553f7046b470c1716619f6d359ca16a55d76b709 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 5 Aug 2012 21:12:56 -0400 Subject: [PATCH 22/30] suggested username for ssl auth is conjoined name with no spaces --- common/djangoapps/external_auth/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index d00a0a7182..0425f3e158 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -150,7 +150,7 @@ def edXauth_signup(request, eamap=None): context = {'has_extauth_info': True, 'show_signup_immediately' : True, 'extauth_email': eamap.external_email, - 'extauth_username' : eamap.external_name.split(' ')[0], + 'extauth_username' : eamap.external_name.replace(' ',''), # default - conjoin name, no spaces 'extauth_name': eamap.external_name, } From 76074442869a533ab90ea767db33018640226b8a Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 5 Aug 2012 23:26:31 -0400 Subject: [PATCH 23/30] fix bug: course staff group based on dir_name, not course number --- lms/djangoapps/courseware/courses.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index e568f97f56..31ae3e7fda 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -123,9 +123,9 @@ def course_staff_group_name(course): if type(course)==str: coursename = course else: - coursename = course.metadata.get('course','') - if not coursename: # Fall 2012: not all course.xml have metadata correct yet coursename = course.metadata.get('data_dir','UnknownCourseName') + if not coursename: # Fall 2012: not all course.xml have metadata correct yet + coursename = course.metadata.get('course','') return 'staff_%s' % coursename def has_staff_access_to_course(user,course): @@ -138,8 +138,8 @@ def has_staff_access_to_course(user,course): if user.is_staff: return True user_groups = [x[1] for x in user.groups.values_list()] # note this is the Auth group, not UserTestGroup - log.debug('user is in groups %s' % user_groups) staff_group = course_staff_group_name(course) + log.debug('course %s user %s groups %s' % (staff_group, user, user_groups)) if staff_group in user_groups: return True return False From a46a37d1c0445bbdec154077b0a976119e818e77 Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 6 Aug 2012 13:58:42 -0400 Subject: [PATCH 24/30] log.debug -> log.exception; revert log change in xml_module --- common/djangoapps/track/views.py | 2 +- common/lib/xmodule/xmodule/xml_module.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/track/views.py b/common/djangoapps/track/views.py index 1123b8d30a..31878bee26 100644 --- a/common/djangoapps/track/views.py +++ b/common/djangoapps/track/views.py @@ -27,7 +27,7 @@ def log_event(event): try: tldat.save() except Exception as err: - log.debug(err) + log.exception(err) def user_track(request): try: # TODO: Do the same for many of the optional META parameters diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index fbb17fd236..7a12ed869d 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -9,7 +9,7 @@ from fs.errors import ResourceNotFoundError import os import sys -log = logging.getLogger('mitx.' + __name__) +log = logging.getLogger(__name__) _AttrMapBase = namedtuple('_AttrMap', 'metadata_key to_metadata from_metadata') From 3484f5382cb593386f3871848060de7e29fab6ee Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 6 Aug 2012 14:10:00 -0400 Subject: [PATCH 25/30] isinstance instead of type --- lms/djangoapps/courseware/courses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 31ae3e7fda..8193988d67 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -120,7 +120,7 @@ def course_staff_group_name(course): ''' course should be either a CourseDescriptor instance, or a string (the .course entry of a Location) ''' - if type(course)==str: + if isinstance(course,str): coursename = course else: coursename = course.metadata.get('data_dir','UnknownCourseName') From 871ed954be3638326c8cd472bd5f573973ef5790 Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 6 Aug 2012 14:16:11 -0400 Subject: [PATCH 26/30] ACCESS_REQUIRE_STAFF_FOR_COURSE default False in lms.envs.dev --- lms/envs/dev.py | 2 +- lms/envs/dev_ike.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 204fcec04b..bc5b621b32 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -61,7 +61,7 @@ SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' ################################ LMS Migration ################################# MITX_FEATURES['ENABLE_LMS_MIGRATION'] = True -MITX_FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = True # require that user be in the staff_* group to be able to enroll +MITX_FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = False # require that user be in the staff_* group to be able to enroll LMS_MIGRATION_ALLOWED_IPS = ['127.0.0.1'] diff --git a/lms/envs/dev_ike.py b/lms/envs/dev_ike.py index b6cd67dfd8..2256decb46 100644 --- a/lms/envs/dev_ike.py +++ b/lms/envs/dev_ike.py @@ -14,4 +14,5 @@ from .dev import * WIKI_ENABLED = False MITX_FEATURES['ENABLE_TEXTBOOK'] = False MITX_FEATURES['ENABLE_DISCUSSION'] = False +MITX_FEATURES['ACCESS_REQUIRE_STAFF_FOR_COURSE'] = True # require that user be in the staff_* group to be able to enroll From 6f894c816cef3f7c6ee7541ffd1ffd31a77a8cce Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 6 Aug 2012 14:19:45 -0400 Subject: [PATCH 27/30] use jquery for error msg in course_about --- lms/templates/portal/course_about.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/templates/portal/course_about.html b/lms/templates/portal/course_about.html index bdea0a47d1..c2c1e3b747 100644 --- a/lms/templates/portal/course_about.html +++ b/lms/templates/portal/course_about.html @@ -20,7 +20,7 @@ if(json.success) { location.href="${reverse('dashboard')}"; }else{ - document.getElementById('register_message').innerHTML = "

    " + json.error + "

    "; + $('#register_message).html("

    " + json.error + "

    ") } }); })(this) From b1ddff838c15f85d3616582b94e2e8a8725ad466 Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 6 Aug 2012 14:23:55 -0400 Subject: [PATCH 28/30] add comment about course start date logic --- lms/templates/portal/course_about.html | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/templates/portal/course_about.html b/lms/templates/portal/course_about.html index c2c1e3b747..a3bf8dd755 100644 --- a/lms/templates/portal/course_about.html +++ b/lms/templates/portal/course_about.html @@ -63,6 +63,7 @@ %if user.is_authenticated(): %if registered: <% + ## TODO: move this logic into a view if course.has_started() or settings.MITX_FEATURES['DISABLE_START_DATES']: course_target = reverse('info', args=[course.id]) else: From 8a1747770a1bae1d3274692aee856288425d1067 Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 6 Aug 2012 14:30:53 -0400 Subject: [PATCH 29/30] redirect to course_about page if hit internal course page unregistered for --- lms/djangoapps/courseware/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 5db3bcf91a..f014e3fcb5 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -168,7 +168,7 @@ def index(request, course_id, chapter=None, section=None, registered = registered_for_course(course, request.user) if not registered: log.debug('User %s tried to view course %s but is not enrolled' % (request.user,course.location.url())) - return redirect('/') + return redirect(reverse('about_course', args=[course.id])) try: context = { From 9805ed89620f6567c5237ae2ab8b7688cf04903c Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 6 Aug 2012 14:37:17 -0400 Subject: [PATCH 30/30] cleanup syntax, split long if into two lines --- common/djangoapps/track/views.py | 2 +- lms/djangoapps/courseware/module_render.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/track/views.py b/common/djangoapps/track/views.py index 31878bee26..b5f9c54665 100644 --- a/common/djangoapps/track/views.py +++ b/common/djangoapps/track/views.py @@ -23,7 +23,7 @@ def log_event(event): log.info(event_str[:settings.TRACK_MAX_EVENT]) if settings.MITX_FEATURES.get('ENABLE_SQL_TRACKING_LOGS'): event['time'] = dateutil.parser.parse(event['time']) - tldat = TrackingLog(**dict([(x,event[x]) for x in LOGFIELDS])) + tldat = TrackingLog(**dict( (x,event[x]) for x in LOGFIELDS )) try: tldat.save() except Exception as err: diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index cdb9dc40f3..b6ba381a26 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -190,9 +190,9 @@ def get_module(user, request, location, student_module_cache, position=None): module.metadata['data_dir'] ) - if (settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and - (user.is_staff or has_staff_access_to_course(user, module.location.course))): - module.get_html = add_histogram(module.get_html, module) + if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF'): + if has_staff_access_to_course(user, module.location.course): + module.get_html = add_histogram(module.get_html, module) # If StudentModule for this instance wasn't already in the database, # and this isn't a guest user, create it.