From 28a2dd9a18e1aafe14d26995ad0cfcacb83f6d6e Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 28 Jan 2013 16:33:28 -0500 Subject: [PATCH 1/5] support reordering of static tabs in studio --- cms/djangoapps/contentstore/views.py | 41 +++++++++++++++++++++++-- cms/static/coffee/src/views/tabs.coffee | 16 +++++++++- cms/urls.py | 1 + 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 816ccab091..326b47ee64 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -903,6 +903,36 @@ def static_pages(request, org, course, coursename): def edit_static(request, org, course, coursename): return render_to_response('edit-static-page.html', {}) + +@login_required +@expect_json +def reorder_tabs(request): + tabs = request.POST['tabs'] + logging.debug('tabs = {0} {1}'.format(tabs.__class__, tabs)) + + if len(tabs) > 0: + course = get_course_for_item(tabs[0]) + + if not has_access(request.user, course.location): + raise PermissionDenied() + + # first filter out all non-static tabs from the tabs list + course_tabs = [t for t in course.tabs if t['type'] != 'static_tab'] + + # OK, re-assemble the static tabs in the new order + for tab in tabs: + item = modulestore('direct').get_item(Location(tab)) + + course_tabs.append({ 'type':'static_tab', + 'name' : item.metadata.get('display_name'), + 'url_slug' : item.location.name} + ) + + course.tabs = course_tabs + modulestore('direct').update_metadata(course.location, course.metadata) + + return HttpResponse() + @login_required @ensure_csrf_cookie def edit_tabs(request, org, course, coursename): @@ -914,12 +944,19 @@ def edit_tabs(request, org, course, coursename): if not has_access(request.user, location): raise PermissionDenied() - static_tabs = modulestore('direct').get_items(static_tabs_loc) - # see tabs have been uninitialized (e.g. supporing courses created before tab support in studio) if course_item.tabs is None or len(course_item.tabs) == 0: initialize_course_tabs(course_item) + # first get all static tabs from the tabs list + # we do this because this is also the order in which items are displayed in the LMS + static_tabs_refs = [t for t in course_item.tabs if t['type'] == 'static_tab'] + + static_tabs = [] + for static_tab_ref in static_tabs_refs: + static_tab_loc = Location(location)._replace(category='static_tab', name=static_tab_ref['url_slug']) + static_tabs.append(modulestore('direct').get_item(static_tab_loc)) + components = [ static_tab.location.url() for static_tab diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index 1fbc6ffa7f..3799eb25ee 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/cms/static/coffee/src/views/tabs.coffee @@ -15,7 +15,7 @@ class CMS.Views.TabsEdit extends Backbone.View @$('.components').sortable( handle: '.drag-handle' - update: (event, ui) => alert 'not yet implemented!' + update: @tabMoved helper: 'clone' opacity: '0.5' placeholder: 'component-placeholder' @@ -24,6 +24,20 @@ class CMS.Views.TabsEdit extends Backbone.View items: '> .component' ) + tabMoved: (event, ui) => + tabs = [] + @$('.component').each((idx, element) => + tabs.push($(element).data('id')) + ) + $.ajax({ + type:'POST', + url: '/reorder_tabs', + data: JSON.stringify({ + tabs : tabs + }), + contentType: 'application/json' + }) + addNewTab: (event) => event.preventDefault() diff --git a/cms/urls.py b/cms/urls.py index 6f8736551b..2b329dc16b 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -17,6 +17,7 @@ urlpatterns = ('', url(r'^publish_draft$', 'contentstore.views.publish_draft', name='publish_draft'), url(r'^unpublish_unit$', 'contentstore.views.unpublish_unit', name='unpublish_unit'), url(r'^create_new_course', 'contentstore.views.create_new_course', name='create_new_course'), + url(r'^reorder_tabs', 'contentstore.views.reorder_tabs', name='reorder_tabs'), url(r'^(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', 'contentstore.views.course_index', name='course_index'), From 855c8bb7e7c7f9a6f1258d3e13e0f7a1cc6d8037 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 29 Jan 2013 13:44:42 -0500 Subject: [PATCH 2/5] add unit test for tab reordering --- cms/djangoapps/contentstore/tests/tests.py | 28 +++++++++++++++++++ cms/djangoapps/contentstore/views.py | 1 - .../data/full/policies/6.002_Spring_2012.json | 1 + 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 3025ee78a4..bfdcfe1438 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -350,6 +350,34 @@ class ContentStoreTest(TestCase): def test_edit_unit_full(self): self.check_edit_unit('full') + def test_static_tab_reordering(self): + import_from_xml(modulestore(), 'common/test/data/', ['full']) + + # reverse the ordering + tabs = { 'tabs' : ['i4x://edX/full/static_tab/resources', 'i4x://edX/full/static_tab/syllabus'] } + resp = self.client.post(reverse('reorder_tabs'), json.dumps(tabs), "application/json") + + ms = modulestore('direct') + course = ms.get_item(Location(['i4x','edX','full','course','6.002_Spring_2012', None])) + # compare to make sure that the tabs information is in the expected order after the server call + + resource_idx = 0 + syllabus_idx = 0 + idx = 0 + for tab in course.tabs: + if tab['type'] == 'static_tab': + if tab['url_slug'] == 'resources': + resource_idx = idx + elif tab['url_slug'] == 'syllabus': + syllabus_idx = idx + idx+=1 + + self.assertLess(resource_idx, syllabus_idx) + + + + + def test_about_overrides(self): ''' This test case verifies that a course can use specialized override for about data, e.g. /about/Fall_2012/effort.html diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 326b47ee64..205bf787fd 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -908,7 +908,6 @@ def edit_static(request, org, course, coursename): @expect_json def reorder_tabs(request): tabs = request.POST['tabs'] - logging.debug('tabs = {0} {1}'.format(tabs.__class__, tabs)) if len(tabs) > 0: course = get_course_for_item(tabs[0]) diff --git a/common/test/data/full/policies/6.002_Spring_2012.json b/common/test/data/full/policies/6.002_Spring_2012.json index 345309ff5c..2f55528b7b 100644 --- a/common/test/data/full/policies/6.002_Spring_2012.json +++ b/common/test/data/full/policies/6.002_Spring_2012.json @@ -8,6 +8,7 @@ {"type": "courseware"}, {"type": "course_info", "name": "Course Info"}, {"type": "static_tab", "url_slug": "syllabus", "name": "Syllabus"}, + {"type": "static_tab", "url_slug": "resources", "name": "Resources"}, {"type": "discussion", "name": "Discussion"}, {"type": "wiki", "name": "Wiki"}, {"type": "progress", "name": "Progress"} From 74fcf9661159b4d600d58b0281ff4bed9a076b75 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 30 Jan 2013 09:43:57 -0500 Subject: [PATCH 3/5] don't hard code the ordering of the tabs in the test. Take the current ordering defintion (in the test data) and reverse it --- cms/djangoapps/contentstore/tests/tests.py | 27 +++++++++++----------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index bfdcfe1438..fd811fb101 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -353,27 +353,26 @@ class ContentStoreTest(TestCase): def test_static_tab_reordering(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - # reverse the ordering - tabs = { 'tabs' : ['i4x://edX/full/static_tab/resources', 'i4x://edX/full/static_tab/syllabus'] } - resp = self.client.post(reverse('reorder_tabs'), json.dumps(tabs), "application/json") - ms = modulestore('direct') + course = ms.get_item(Location(['i4x','edX','full','course','6.002_Spring_2012', None])) + + # reverse the ordering + reverse_tabs = [] + for tab in course.tabs: + if tab['type'] == 'static_tab': + reverse_tabs.insert(0, 'i4x://edX/full/static_tab/{0}'.format(tab['url_slug'])) + + resp = self.client.post(reverse('reorder_tabs'), json.dumps({'tabs':reverse_tabs}), "application/json") + course = ms.get_item(Location(['i4x','edX','full','course','6.002_Spring_2012', None])) # compare to make sure that the tabs information is in the expected order after the server call - resource_idx = 0 - syllabus_idx = 0 - idx = 0 + course_tabs = [] for tab in course.tabs: if tab['type'] == 'static_tab': - if tab['url_slug'] == 'resources': - resource_idx = idx - elif tab['url_slug'] == 'syllabus': - syllabus_idx = idx - idx+=1 - - self.assertLess(resource_idx, syllabus_idx) + course_tabs.append('i4x://edX/full/static_tab/{0}'.format(tab['url_slug'])) + self.assertEqual(reverse_tabs, course_tabs) From e5115953ad6badda720c83601a439227f4b49d52 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 30 Jan 2013 11:36:28 -0500 Subject: [PATCH 4/5] rework redorder tabs so that we account for the fact that static tabs can be in any arbitrary order (with respect to non-static tabs) when working with an imported course --- cms/djangoapps/contentstore/views.py | 49 +++++++++++++++++++--------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 205bf787fd..cf7c3984fb 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -908,30 +908,47 @@ def edit_static(request, org, course, coursename): @expect_json def reorder_tabs(request): tabs = request.POST['tabs'] + course = get_course_for_item(tabs[0]) - if len(tabs) > 0: - course = get_course_for_item(tabs[0]) + if not has_access(request.user, course.location): + raise PermissionDenied() - if not has_access(request.user, course.location): - raise PermissionDenied() + # get list of course_tabs + course_tabs = [t for t in course.tabs if t['type'] == 'static_tab'] - # first filter out all non-static tabs from the tabs list - course_tabs = [t for t in course.tabs if t['type'] != 'static_tab'] + # make sure they are the same lengths (i.e. the number of passed in tabs equals the number + # that we know about) otherwise we can drop some! + if len(course_tabs) != len(tabs): + return HttpResponseBadRequest() - # OK, re-assemble the static tabs in the new order - for tab in tabs: - item = modulestore('direct').get_item(Location(tab)) + # load all reference tabs, return BadRequest if we can't find any of them + tab_items =[] + for tab in tabs: + item = modulestore('direct').get_item(Location(tab)) + if item is None: + return HttpResponseBadRequest() - course_tabs.append({ 'type':'static_tab', - 'name' : item.metadata.get('display_name'), - 'url_slug' : item.location.name} - ) - - course.tabs = course_tabs - modulestore('direct').update_metadata(course.location, course.metadata) + tab_items.append(item) + # now just go through the existing course_tabs and re-order the static tabs + reordered_tabs = [] + static_tab_idx = 0 + for tab in course.tabs: + if tab['type'] == 'static_tab': + reordered_tabs.append({'type': 'static_tab', + 'name' : tab_items[static_tab_idx].metadata.get('display_name'), + 'url_slug' : tab_items[static_tab_idx].location.name}) + static_tab_idx += 1 + else: + reordered_tabs.append(tab) + + + # OK, re-assemble the static tabs in the new order + course.tabs = reordered_tabs + modulestore('direct').update_metadata(course.location, course.metadata) return HttpResponse() + @login_required @ensure_csrf_cookie def edit_tabs(request, org, course, coursename): From 7f32aae47d1510067c6cb8365f6667b3e8a60fb4 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 30 Jan 2013 11:50:11 -0500 Subject: [PATCH 5/5] rename method to something that actually better reflects what it is doing. Also rename a variable to help with readability --- cms/djangoapps/contentstore/tests/tests.py | 4 ++-- cms/djangoapps/contentstore/views.py | 10 +++++----- cms/static/coffee/src/views/tabs.coffee | 2 +- cms/urls.py | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index fd811fb101..8eddf000fe 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -362,11 +362,11 @@ class ContentStoreTest(TestCase): if tab['type'] == 'static_tab': reverse_tabs.insert(0, 'i4x://edX/full/static_tab/{0}'.format(tab['url_slug'])) - resp = self.client.post(reverse('reorder_tabs'), json.dumps({'tabs':reverse_tabs}), "application/json") + resp = self.client.post(reverse('reorder_static_tabs'), json.dumps({'tabs':reverse_tabs}), "application/json") course = ms.get_item(Location(['i4x','edX','full','course','6.002_Spring_2012', None])) + # compare to make sure that the tabs information is in the expected order after the server call - course_tabs = [] for tab in course.tabs: if tab['type'] == 'static_tab': diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index cf7c3984fb..b8981ecaa0 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -906,19 +906,19 @@ def edit_static(request, org, course, coursename): @login_required @expect_json -def reorder_tabs(request): +def reorder_static_tabs(request): tabs = request.POST['tabs'] course = get_course_for_item(tabs[0]) if not has_access(request.user, course.location): raise PermissionDenied() - # get list of course_tabs - course_tabs = [t for t in course.tabs if t['type'] == 'static_tab'] - + # get list of existing static tabs in course # make sure they are the same lengths (i.e. the number of passed in tabs equals the number # that we know about) otherwise we can drop some! - if len(course_tabs) != len(tabs): + + existing_static_tabs = [t for t in course.tabs if t['type'] == 'static_tab'] + if len(existing_static_tabs) != len(tabs): return HttpResponseBadRequest() # load all reference tabs, return BadRequest if we can't find any of them diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index 3799eb25ee..5a826c1794 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/cms/static/coffee/src/views/tabs.coffee @@ -31,7 +31,7 @@ class CMS.Views.TabsEdit extends Backbone.View ) $.ajax({ type:'POST', - url: '/reorder_tabs', + url: '/reorder_static_tabs', data: JSON.stringify({ tabs : tabs }), diff --git a/cms/urls.py b/cms/urls.py index 2b329dc16b..406d3966bb 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -17,7 +17,7 @@ urlpatterns = ('', url(r'^publish_draft$', 'contentstore.views.publish_draft', name='publish_draft'), url(r'^unpublish_unit$', 'contentstore.views.unpublish_unit', name='unpublish_unit'), url(r'^create_new_course', 'contentstore.views.create_new_course', name='create_new_course'), - url(r'^reorder_tabs', 'contentstore.views.reorder_tabs', name='reorder_tabs'), + url(r'^reorder_static_tabs', 'contentstore.views.reorder_static_tabs', name='reorder_static_tabs'), url(r'^(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', 'contentstore.views.course_index', name='course_index'),