diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 1fe7df91c8..6d01c3424c 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -306,18 +306,25 @@ def course_search_index_handler(request, course_key_string): The restful handler for course indexing. GET html: return status of indexing task + json: return status of indexing task """ # Only global staff (PMs) are able to index courses if not GlobalStaff().has_user(request.user): raise PermissionDenied() course_key = CourseKey.from_string(course_key_string) + content_type = request.META.get('CONTENT_TYPE', None) + if content_type is None: + content_type = "application/json; charset=utf-8" with modulestore().bulk_operations(course_key): try: reindex_course_and_check_access(course_key, request.user) except SearchIndexingError as search_err: - return HttpResponse(search_err.error_list, status=500) - - return HttpResponse({}, status=200) + return HttpResponse(json.dumps({ + "user_message": search_err.error_list + }), content_type=content_type, status=500) + return HttpResponse(json.dumps({ + "user_message": _("Course has been successfully reindexed.") + }), content_type=content_type, status=200) def _course_outline_json(request, course_module): @@ -487,7 +494,7 @@ def course_index(request, course_key): lms_link = get_lms_link_for_item(course_module.location) reindex_link = None if settings.FEATURES.get('ENABLE_COURSEWARE_INDEX', False): - reindex_link = "/course_search_index/{course_id}".format(course_id=unicode(course_key)) + reindex_link = "/course/{course_id}/search_reindex".format(course_id=unicode(course_key)) sections = course_module.get_children() course_structure = _course_outline_json(request, course_module) locator_to_show = request.REQUEST.get('show', None) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index ed226a926e..7304ab73d9 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -25,6 +25,7 @@ from student.tests.factories import UserFactory from course_action_state.managers import CourseRerunUIStateManager from django.conf import settings from django.core.exceptions import PermissionDenied +from django.utils.translation import ugettext as _ from search.api import perform_search import pytz @@ -347,6 +348,8 @@ class TestCourseReIndex(CourseTestCase): TEST_INDEX_FILENAME = "test_root/index_file.dat" + SUCCESSFUL_RESPONSE = _("Course has been successfully reindexed.") + def setUp(self): """ Set up the for the course outline tests. @@ -388,7 +391,7 @@ class TestCourseReIndex(CourseTestCase): response = self.client.get(index_url, {}, HTTP_ACCEPT='application/json') # A course with the default release date should display as "Unscheduled" - self.assertEqual(response.content, '') + self.assertIn(self.SUCCESSFUL_RESPONSE, response.content) self.assertEqual(response.status_code, 200) response = self.client.post(index_url, {}, HTTP_ACCEPT='application/json') @@ -409,6 +412,33 @@ class TestCourseReIndex(CourseTestCase): response = non_staff_client.get(index_url, {}, HTTP_ACCEPT='application/json') self.assertEqual(response.status_code, 403) + def test_content_type_none(self): + """ + Test json content type is set if none is selected + """ + index_url = reverse_course_url('course_search_index_handler', self.course.id) + response = self.client.get(index_url, {}, CONTENT_TYPE=None) + + # A course with the default release date should display as "Unscheduled" + self.assertIn(self.SUCCESSFUL_RESPONSE, response.content) + self.assertEqual(response.status_code, 200) + + @mock.patch('xmodule.html_module.HtmlDescriptor.index_dictionary') + def test_reindex_course_search_index_error(self, mock_index_dictionary): + """ + Test json response with mocked error data for html + """ + + # set mocked exception response + err = SearchIndexingError + mock_index_dictionary.return_value = err + + index_url = reverse_course_url('course_search_index_handler', self.course.id) + + # Start manual reindex and check error in response + response = self.client.get(index_url, {}, HTTP_ACCEPT='application/json') + self.assertEqual(response.status_code, 500) + def test_reindex_json_responses(self): """ Test json response with real data @@ -455,7 +485,7 @@ class TestCourseReIndex(CourseTestCase): self.assertEqual(response['results'], []) # set mocked exception response - err = Exception + err = SearchIndexingError mock_index_dictionary.return_value = err # Start manual reindex and check error in response @@ -465,7 +495,7 @@ class TestCourseReIndex(CourseTestCase): @mock.patch('xmodule.html_module.HtmlDescriptor.index_dictionary') def test_reindex_html_error_json_responses(self, mock_index_dictionary): """ - Test json response with rmocked error data for html + Test json response with mocked error data for html """ # Check results not indexed response = perform_search( @@ -477,7 +507,7 @@ class TestCourseReIndex(CourseTestCase): self.assertEqual(response['results'], []) # set mocked exception response - err = Exception + err = SearchIndexingError mock_index_dictionary.return_value = err # Start manual reindex and check error in response @@ -487,7 +517,7 @@ class TestCourseReIndex(CourseTestCase): @mock.patch('xmodule.seq_module.SequenceDescriptor.index_dictionary') def test_reindex_seq_error_json_responses(self, mock_index_dictionary): """ - Test json response with rmocked error data for sequence + Test json response with mocked error data for sequence """ # Check results not indexed response = perform_search( diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index 1e216c6794..4e0efc2d4d 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -1,7 +1,7 @@ -define(["jquery", "js/common_helpers/ajax_helpers", "js/views/utils/view_utils", "js/views/pages/course_outline", +define(["jquery", "sinon", "js/common_helpers/ajax_helpers", "js/views/utils/view_utils", "js/views/pages/course_outline", "js/models/xblock_outline_info", "js/utils/date_utils", "js/spec_helpers/edit_helpers", "js/common_helpers/template_helpers"], - function($, AjaxHelpers, ViewUtils, CourseOutlinePage, XBlockOutlineInfo, DateUtils, EditHelpers, TemplateHelpers) { + function($, Sinon, AjaxHelpers, ViewUtils, CourseOutlinePage, XBlockOutlineInfo, DateUtils, EditHelpers, TemplateHelpers) { describe("CourseOutlinePage", function() { var createCourseOutlinePage, displayNameInput, model, outlinePage, requests, @@ -90,16 +90,16 @@ define(["jquery", "js/common_helpers/ajax_helpers", "js/views/utils/view_utils", createMockIndexJSON = function(option) { if(option){ - return { - status: 200, - responseText: '' - }; + return JSON.stringify({ + "developer_message" : "Course has been successfully reindexed.", + "user_message": "Course has been successfully reindexed." + }); } else { - return { - status: 500, - responseText: JSON.stringify('Could not index item: course/slashes:mock+item') - }; + return JSON.stringify({ + "developer_message" : "Could not reindex course.", + "user_message": "Could not reindex course." + }); } }; @@ -324,12 +324,12 @@ define(["jquery", "js/common_helpers/ajax_helpers", "js/views/utils/view_utils", verifyItemsExpanded('section', true); }); - it('can start reindex of a course - respond success', function() { + it('can start reindex of a course', function() { createCourseOutlinePage(this, mockSingleSectionCourseJSON); var reindexSpy = spyOn(outlinePage, 'startReIndex').andCallThrough(); var successSpy = spyOn(outlinePage, 'onIndexSuccess').andCallThrough(); var reindexButton = outlinePage.$('.button.button-reindex'); - var test_url = '/course_search_index/5'; + var test_url = '/course/5/search_reindex'; reindexButton.attr('href', test_url) reindexButton.trigger('click'); AjaxHelpers.expectJsonRequest(requests, 'GET', test_url); @@ -338,16 +338,18 @@ define(["jquery", "js/common_helpers/ajax_helpers", "js/views/utils/view_utils", expect(successSpy).toHaveBeenCalled(); }); - it('can start reindex of a course - respond fail', function() { + it('shows an error message when reindexing fails', function() { createCourseOutlinePage(this, mockSingleSectionCourseJSON); var reindexSpy = spyOn(outlinePage, 'startReIndex').andCallThrough(); + var errorSpy = spyOn(outlinePage, 'onIndexError').andCallThrough(); var reindexButton = outlinePage.$('.button.button-reindex'); - var test_url = '/course_search_index/5'; + var test_url = '/course/5/search_reindex'; reindexButton.attr('href', test_url) reindexButton.trigger('click'); AjaxHelpers.expectJsonRequest(requests, 'GET', test_url); - AjaxHelpers.respondWithJson(requests, createMockIndexJSON(false)); + AjaxHelpers.respondWithError(requests, 500, createMockIndexJSON(false)); expect(reindexSpy).toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalled(); }); }); diff --git a/cms/static/js/views/pages/course_outline.js b/cms/static/js/views/pages/course_outline.js index 34779b7486..c2e1ecb2d2 100644 --- a/cms/static/js/views/pages/course_outline.js +++ b/cms/static/js/views/pages/course_outline.js @@ -2,8 +2,9 @@ * This page is used to show the user an outline of the course. */ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views/utils/xblock_utils", - "js/views/course_outline", "js/views/utils/view_utils", "js/views/feedback_alert"], - function ($, _, gettext, BasePage, XBlockViewUtils, CourseOutlineView, ViewUtils, AlertView) { + "js/views/course_outline", "js/views/utils/view_utils", "js/views/feedback_alert", + "js/views/feedback_notification"], + function ($, _, gettext, BasePage, XBlockViewUtils, CourseOutlineView, ViewUtils, AlertView, NoteView) { var expandedLocators, CourseOutlinePage; CourseOutlinePage = BasePage.extend({ @@ -111,21 +112,33 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views var target = $(event.currentTarget); target.css('cursor', 'wait'); this.startReIndex(target.attr('href')) - .done(function() {self.onIndexSuccess();}) + .done(function(data) {self.onIndexSuccess(data);}) + .fail(function(data) {self.onIndexError(data);}) .always(function() {target.css('cursor', 'pointer');}); }, startReIndex: function(reindex_url) { return $.ajax({ - url: reindex_url, - method: 'GET' + url: reindex_url, + method: 'GET', + global: false, + contentType: "application/json; charset=utf-8", + dataType: "json" }); }, - onIndexSuccess: function() { + onIndexSuccess: function(data) { var msg = new AlertView.Announcement({ title: gettext('Course Index'), - message: gettext('Course has been successfully reindexed.') + message: data.user_message + }); + msg.show(); + }, + + onIndexError: function(data) { + var msg = new NoteView.Error({ + title: gettext('There were errors reindexing course.'), + message: data.user_message }); msg.show(); } diff --git a/cms/urls.py b/cms/urls.py index 89ef86040c..38cfef37d7 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -81,7 +81,7 @@ urlpatterns += patterns( ), url(r'^home/$', 'course_listing', name='home'), url( - r'^course_search_index/{}?$'.format(settings.COURSE_KEY_PATTERN), + r'^course/{}/search_reindex?$'.format(settings.COURSE_KEY_PATTERN), 'course_search_index_handler', name='course_search_index_handler' ),