From 4f33f7ffbc3a1352d73d8714eefb1b1b268f6f7d Mon Sep 17 00:00:00 2001 From: ichuang Date: Thu, 3 Jan 2013 00:49:57 +0000 Subject: [PATCH 01/11] turn on tracking (helps with debugging) --- cms/envs/common.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cms/envs/common.py b/cms/envs/common.py index 50f237c374..a83f61d8f9 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -172,6 +172,9 @@ LANGUAGE_CODE = 'en' # http://www.i18nguy.com/unicode/language-identi USE_I18N = True USE_L10N = True +# Tracking +TRACK_MAX_EVENT = 10000 + # Messages MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage' @@ -275,6 +278,10 @@ INSTALLED_APPS = ( 'auth', 'student', # misleading name due to sharing with lms 'course_groups', # not used in cms (yet), but tests run + + # tracking + 'track', + # For asset pipelining 'pipeline', 'staticfiles', From 816b0edfb01f47942dee702d18566869b2b358b8 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 25 Feb 2013 16:00:35 -0500 Subject: [PATCH 02/11] add another command line argument to allow users to only simulate a delete (i.e. show what items would be deleted). This is a handy sanity checker. --- .../management/commands/delete_course.py | 16 ++++++++++++---- .../xmodule/modulestore/store_utilities.py | 14 +++++++++----- rakefile | 5 ++++- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/delete_course.py b/cms/djangoapps/contentstore/management/commands/delete_course.py index bb38e72d44..ef2505b276 100644 --- a/cms/djangoapps/contentstore/management/commands/delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/delete_course.py @@ -21,18 +21,26 @@ class Command(BaseCommand): '''Delete a MongoDB backed course''' def handle(self, *args, **options): - if len(args) != 1: - raise CommandError("delete_course requires one argument: ") + if len(args) != 1 and len(args) != 2: + raise CommandError("delete_course requires one argument: |commit|") loc_str = args[0] + commit = False + if len(args) == 2: + commit = args[1] == 'commit' + + if commit: + print 'Actually going to delete the course from DB....' + ms = modulestore('direct') cs = contentstore() if query_yes_no("Deleting course {0}. Confirm?".format(loc_str), default="no"): if query_yes_no("Are you sure. This action cannot be undone!", default="no"): loc = CourseDescriptor.id_to_location(loc_str) - if delete_course(ms, cs, loc) == True: + if delete_course(ms, cs, loc, commit) == True: print 'removing User permissions from course....' # in the django layer, we need to remove all the user permissions groups associated with this course - _delete_course_group(loc) + if commit: + _delete_course_group(loc) diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index 192b012bef..f08fdd639a 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -92,7 +92,7 @@ def clone_course(modulestore, contentstore, source_location, dest_location, dele return True -def delete_course(modulestore, contentstore, source_location): +def delete_course(modulestore, contentstore, source_location, commit = False): # first check to see if the modulestore is Mongo backed if not isinstance(modulestore, MongoModuleStore): raise Exception("Expected a MongoModuleStore in the runtime. Aborting....") @@ -107,7 +107,8 @@ def delete_course(modulestore, contentstore, source_location): thumb_loc = Location(thumb["_id"]) id = StaticContent.get_id_from_location(thumb_loc) print "Deleting {0}...".format(id) - contentstore.delete(id) + if commit: + contentstore.delete(id) # then delete all of the assets assets = contentstore.get_all_content_for_course(source_location) @@ -115,7 +116,8 @@ def delete_course(modulestore, contentstore, source_location): asset_loc = Location(asset["_id"]) id = StaticContent.get_id_from_location(asset_loc) print "Deleting {0}...".format(id) - contentstore.delete(id) + if commit: + contentstore.delete(id) # then delete all course modules modules = modulestore.get_items([source_location.tag, source_location.org, source_location.course, None, None, None]) @@ -123,10 +125,12 @@ def delete_course(modulestore, contentstore, source_location): for module in modules: if module.category != 'course': # save deleting the course module for last print "Deleting {0}...".format(module.location) - modulestore.delete_item(module.location) + if commit: + modulestore.delete_item(module.location) # finally delete the top-level course module itself print "Deleting {0}...".format(source_location) - modulestore.delete_item(source_location) + if commit: + modulestore.delete_item(source_location) return True diff --git a/rakefile b/rakefile index 5647b5b13a..15692d0d99 100644 --- a/rakefile +++ b/rakefile @@ -415,7 +415,10 @@ end namespace :cms do desc "Delete existing MongoDB based course" task :delete_course do - if ENV['LOC'] + + if ENV['LOC'] and ENV['COMMIT'] + sh(django_admin(:cms, :dev, :delete_course, ENV['LOC'], ENV['COMMIT'])) + elsif ENV['LOC'] sh(django_admin(:cms, :dev, :delete_course, ENV['LOC'])) else raise "You must pass in a LOC parameter" From 9fe305e02b9b00c3c95d33a51f76f288b9d95cc8 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Wed, 27 Feb 2013 13:05:29 -0500 Subject: [PATCH 03/11] Modified ChoiceGroup to display correct/incorrect indicators (check or x) next to the option the user selected (check boxes or radio buttons). --- .../lib/capa/capa/templates/choicegroup.html | 38 ++++++++++++------- .../lib/xmodule/xmodule/css/capa/display.scss | 4 +- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/common/lib/capa/capa/templates/choicegroup.html b/common/lib/capa/capa/templates/choicegroup.html index 457d9e7817..b9d8164321 100644 --- a/common/lib/capa/capa/templates/choicegroup.html +++ b/common/lib/capa/capa/templates/choicegroup.html @@ -1,23 +1,35 @@
-
- % if status == 'unsubmitted': - - % elif status == 'correct': - - % elif status == 'incorrect': - - % elif status == 'incomplete': - - % endif -
% for choice_id, choice_description in choices: - + /> + + ${choice_description} + + + % endfor
diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index d40bdb556e..007178834d 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -227,7 +227,7 @@ section.problem { background: url('../images/correct-icon.png') center center no-repeat; height: 20px; position: relative; - top: 6px; + top: 3px; width: 25px; } @@ -237,7 +237,7 @@ section.problem { height: 20px; width: 20px; position: relative; - top: 6px; + top: 3px; } } From 8a953241c113cbd8b59620f0ea7cb09a7b47ac72 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 1 Mar 2013 10:45:48 -0500 Subject: [PATCH 04/11] first pass --- .../django_comment_client/forum/views.py | 21 ++++++++++--------- lms/templates/discussion/_new_post.html | 17 +++++++-------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index efacbc1b4e..bb1dc00339 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -147,17 +147,18 @@ def inline_discussion(request, course_id, discussion_id): else: #otherwise, just make a dictionary of two - user_cohort = get_cohort(cc_user, course_id) - if user_cohort: - user_cohort_name = user_cohort.name - user_cohort_id = user_cohort.id - else: - user_cohort_name = user_cohort_id = None + #user_cohort = get_cohort(cc_user, course_id) + #if user_cohort: + # user_cohort_name = user_cohort.name + # user_cohort_id = user_cohort.id + #else: + # user_cohort_name = user_cohort_id = None + cohorts_list = None - if user_cohort: - cohorts_list.append({'name':user_cohort_name, 'id':user_cohort_id}) - else: - cohorts_list = None + #if user_cohort: + # cohorts_list.append({'name':user_cohort_name, 'id':user_cohort_id}) + #else: + # cohorts_list = None return utils.JsonResponse({ diff --git a/lms/templates/discussion/_new_post.html b/lms/templates/discussion/_new_post.html index 223b593368..feaabf894c 100644 --- a/lms/templates/discussion/_new_post.html +++ b/lms/templates/discussion/_new_post.html @@ -46,21 +46,18 @@ %elif course.metadata.get("allow_anonymous_to_peers", False): %endif - %if is_course_cohorted: + %if is_course_cohorted and is_moderator:
Make visible to:
%endif From d7c28bfa11103e44db2f852c0781ae4b266e276d Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 1 Mar 2013 15:24:56 -0500 Subject: [PATCH 05/11] Don't usurp ValidatingView's error handling. #219 --- cms/static/js/views/settings/main_settings_view.js | 2 +- cms/static/js/views/settings/settings_grading_view.js | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/cms/static/js/views/settings/main_settings_view.js b/cms/static/js/views/settings/main_settings_view.js index e24492c438..8f998dbf7a 100644 --- a/cms/static/js/views/settings/main_settings_view.js +++ b/cms/static/js/views/settings/main_settings_view.js @@ -97,7 +97,7 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({ } var newVal = new Date(date.getTime() + time * 1000); if (!cacheModel.has(fieldName) || cacheModel.get(fieldName).getTime() !== newVal.getTime()) { - cacheModel.save(fieldName, newVal, { error: CMS.ServerError}); + cacheModel.save(fieldName, newVal); } } }; diff --git a/cms/static/js/views/settings/settings_grading_view.js b/cms/static/js/views/settings/settings_grading_view.js index 5b3e68bf72..a7c8defb43 100644 --- a/cms/static/js/views/settings/settings_grading_view.js +++ b/cms/static/js/views/settings/settings_grading_view.js @@ -90,8 +90,7 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({ setGracePeriod : function(event) { event.data.clearValidationErrors(); var newVal = event.data.model.dateToGracePeriod($(event.currentTarget).timepicker('getTime')); - if (event.data.model.get('grace_period') != newVal) event.data.model.save('grace_period', newVal, - { error : CMS.ServerError}); + if (event.data.model.get('grace_period') != newVal) event.data.model.save('grace_period', newVal); }, updateModel : function(event) { if (!this.selectorToField[event.currentTarget.id]) return; @@ -227,8 +226,7 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({ object[cutoff['designation']] = cutoff['cutoff'] / 100.0; return object; }, - {}), - { error : CMS.ServerError}); + {})); }, addNewGrade: function(e) { From 724c6c9eaab92d51ef698543d42132d667ee4058 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 1 Mar 2013 15:25:36 -0500 Subject: [PATCH 06/11] reapply Ike's pull-request in a new branch. The PR branch got mangled on the rebase by accident. --- cms/djangoapps/contentstore/utils.py | 13 +++++++++---- cms/djangoapps/contentstore/views.py | 7 +++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index cf0c281dd4..cba30131b5 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -80,10 +80,15 @@ def get_lms_link_for_item(location, preview=False, course_id=None): course_id = get_course_id(location) if settings.LMS_BASE is not None: - lms_link = "//{preview}{lms_base}/courses/{course_id}/jump_to/{location}".format( - preview='preview.' if preview else '', - lms_base=settings.LMS_BASE, - course_id= course_id, + if preview: + lms_base = settings.MITX_FEATURES.get('PREVIEW_LMS_BASE', + 'preview.' + settings.LMS_BASE) + else: + lms_base = settings.LMS_BASE + + lms_link = "//{lms_base}/courses/{course_id}/jump_to/{location}".format( + lms_base=lms_base, + course_id=course_id, location=Location(location) ) else: diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index b55dc13e58..5fb65fd18e 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -317,8 +317,11 @@ def edit_unit(request, location): break index = index + 1 - preview_lms_link = '//{preview}{lms_base}/courses/{org}/{course}/{course_name}/courseware/{section}/{subsection}/{index}'.format( - preview='preview.', + preview_lms_base = settings.MITX_FEATURES.get('PREVIEW_LMS_BASE', + 'preview.' + settings.LMS_BASE) + + preview_lms_link = '//{preview_lms_base}/courses/{org}/{course}/{course_name}/courseware/{section}/{subsection}/{index}'.format( + preview_lms_base=preview_lms_base, lms_base=settings.LMS_BASE, org=course.location.org, course=course.location.course, From 20540fbc2d50479622b58e2c9adcce848fc12f24 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 1 Mar 2013 15:52:27 -0500 Subject: [PATCH 07/11] update usage feedback --- .../contentstore/management/commands/delete_course.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/management/commands/delete_course.py b/cms/djangoapps/contentstore/management/commands/delete_course.py index ef2505b276..e3cac7de02 100644 --- a/cms/djangoapps/contentstore/management/commands/delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/delete_course.py @@ -22,7 +22,7 @@ class Command(BaseCommand): def handle(self, *args, **options): if len(args) != 1 and len(args) != 2: - raise CommandError("delete_course requires one argument: |commit|") + raise CommandError("delete_course requires one or more arguments: |commit|") loc_str = args[0] From b7a6eadca45dd2654da2c7bd19346bc1d43d7dc3 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 1 Mar 2013 16:33:27 -0500 Subject: [PATCH 08/11] new cohorts tested --- .../discussion/discussion_module_view.coffee | 2 +- .../django_comment_client/base/views.py | 18 ++++++++++-------- .../django_comment_client/forum/views.py | 9 +++++++-- lms/templates/discussion/_new_post.html | 2 +- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/common/static/coffee/src/discussion/discussion_module_view.coffee b/common/static/coffee/src/discussion/discussion_module_view.coffee index 4e4c2d1e7a..2e58b2c0b8 100644 --- a/common/static/coffee/src/discussion/discussion_module_view.coffee +++ b/common/static/coffee/src/discussion/discussion_module_view.coffee @@ -79,7 +79,7 @@ if Backbone? #determined in the coffeescript based on whether or not there's a #group id - if response.is_cohorted + if response.is_cohorted and response.is_moderator source = "script#_inline_discussion_cohorted" else source = "script#_inline_discussion" diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index e4e5ce1550..a479fd530f 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -98,22 +98,24 @@ def create_thread(request, course_id, commentable_id): #by the group id in the request this all goes away # Cohort the thread if the commentable is cohorted. - #if is_commentable_cohorted(course_id, commentable_id): - # user_group_id = get_cohort_id(user, course_id) + if is_commentable_cohorted(course_id, commentable_id): + user_group_id = get_cohort_id(user, course_id) + # TODO (vshnayder): once we have more than just cohorts, we'll want to # change this to a single get_group_for_user_and_commentable function # that can do different things depending on the commentable_id - # if cached_has_permission(request.user, "see_all_cohorts", course_id): + if cached_has_permission(request.user, "see_all_cohorts", course_id): # admins can optionally choose what group to post as - # group_id = post.get('group_id', user_group_id) - # else: + group_id = post.get('group_id', user_group_id) + else: # regular users always post with their own id. - # group_id = user_group_id - group_id = post.get('group_id') + group_id = user_group_id +# group_id = post.get('group_id') + if group_id: thread.update_attributes(group_id=group_id) - log.debug("Saving thread %r", thread.attributes) + #log.debug("Saving thread %r", thread.attributes) thread.save() if post.get('auto_subscribe', 'false').lower() == 'true': diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index bb1dc00339..8a3bfa7510 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -133,14 +133,18 @@ def inline_discussion(request, course_id, discussion_id): #if the commentable is cohorted, otherwise everything is not cohorted #and no one has the option of choosing a cohort is_cohorted = is_course_cohorted(course_id) and is_commentable_cohorted(course_id, discussion_id) - + is_moderator = cached_has_permission(request.user, "see_all_cohorts", course_id) + cohorts_list = list() if is_cohorted: cohorts_list.append({'name':'All Groups','id':None}) #if you're a mod, send all cohorts and let you pick - if cached_has_permission(request.user, "see_all_cohorts", course_id): + + + + if is_moderator: cohorts = get_course_cohorts(course_id) for c in cohorts: cohorts_list.append({'name':c.name, 'id':c.id}) @@ -171,6 +175,7 @@ def inline_discussion(request, course_id, discussion_id): 'allow_anonymous_to_peers': allow_anonymous_to_peers, 'allow_anonymous': allow_anonymous, 'cohorts': cohorts_list, + 'is_moderator': is_moderator, 'is_cohorted': is_cohorted }) diff --git a/lms/templates/discussion/_new_post.html b/lms/templates/discussion/_new_post.html index feaabf894c..7848dd1488 100644 --- a/lms/templates/discussion/_new_post.html +++ b/lms/templates/discussion/_new_post.html @@ -53,7 +53,7 @@ %for c in cohorts: From e1ef6a83238f5a22db0b668a9b6b0b0c4271baf7 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 1 Mar 2013 16:43:55 -0500 Subject: [PATCH 09/11] clean up commments --- .../django_comment_client/base/views.py | 3 +-- .../django_comment_client/forum/views.py | 16 +--------------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index a479fd530f..c93ca5f308 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -96,6 +96,7 @@ def create_thread(request, course_id, commentable_id): #kevinchugh because the new requirement is that all groups will be determined #by the group id in the request this all goes away + #not anymore, only for admins # Cohort the thread if the commentable is cohorted. if is_commentable_cohorted(course_id, commentable_id): @@ -110,12 +111,10 @@ def create_thread(request, course_id, commentable_id): else: # regular users always post with their own id. group_id = user_group_id -# group_id = post.get('group_id') if group_id: thread.update_attributes(group_id=group_id) - #log.debug("Saving thread %r", thread.attributes) thread.save() if post.get('auto_subscribe', 'false').lower() == 'true': diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 8a3bfa7510..d4a6d0cc38 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -142,29 +142,15 @@ def inline_discussion(request, course_id, discussion_id): #if you're a mod, send all cohorts and let you pick - - if is_moderator: cohorts = get_course_cohorts(course_id) for c in cohorts: cohorts_list.append({'name':c.name, 'id':c.id}) else: - #otherwise, just make a dictionary of two - #user_cohort = get_cohort(cc_user, course_id) - #if user_cohort: - # user_cohort_name = user_cohort.name - # user_cohort_id = user_cohort.id - #else: - # user_cohort_name = user_cohort_id = None + #students don't get to choose cohorts_list = None - #if user_cohort: - # cohorts_list.append({'name':user_cohort_name, 'id':user_cohort_id}) - #else: - # cohorts_list = None - - return utils.JsonResponse({ 'discussion_data': map(utils.safe_content, threads), 'user_info': user_info, From bcea53d4b7b0bc3fbf6c602bb3575d08694695c9 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Sat, 2 Mar 2013 19:36:11 -0500 Subject: [PATCH 10/11] do the metadata comparison on the parent not the course as we don't actually know which vertical we ended up picking (could be arbitrary). --- cms/djangoapps/contentstore/tests/test_contentstore.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 66e6551019..b4d2afbef0 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -465,7 +465,9 @@ class ContentStoreTest(ModuleStoreTestCase): # check for grace period definition which should be defined at the course level self.assertIn('graceperiod', new_module.metadata) - self.assertEqual(course.metadata['graceperiod'], new_module.metadata['graceperiod']) + self.assertEqual(parent.metadata['graceperiod'], new_module.metadata['graceperiod']) + + self.assertEqual(course.metadata['xqa_key'], new_module.metadata['xqa_key']) # # now let's define an override at the leaf node level From 3f9bc86c9e1b919d51611953819899ebea3c8387 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Sat, 2 Mar 2013 21:26:24 -0500 Subject: [PATCH 11/11] need to pass commit parameter --- cms/djangoapps/contentstore/tests/test_contentstore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index b4d2afbef0..8e4a016a0f 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -171,7 +171,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') - delete_course(ms, cs, location) + delete_course(ms, cs, location, commit=True) items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) self.assertEqual(len(items), 0)