From 0a5af2a38560aa9a5616ff176520e6ea0ea7149d Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Sun, 10 Aug 2014 23:20:31 -0400 Subject: [PATCH 01/13] Rename legacy dash template --- lms/djangoapps/instructor/views/legacy.py | 2 +- ...structor_dashboard.html => legacy_instructor_dashboard.html} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename lms/templates/courseware/{instructor_dashboard.html => legacy_instructor_dashboard.html} (100%) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 8e8a48513a..ea3b6663c1 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -1045,7 +1045,7 @@ def instructor_dashboard(request, course_id): context['standard_dashboard_url'] = reverse('instructor_dashboard', kwargs={'course_id': course_key.to_deprecated_string()}) - return render_to_response('courseware/instructor_dashboard.html', context) + return render_to_response('courseware/legacy_instructor_dashboard.html', context) def _do_remote_gradebook(user, course, action, args=None, files=None): diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html similarity index 100% rename from lms/templates/courseware/instructor_dashboard.html rename to lms/templates/courseware/legacy_instructor_dashboard.html From a15110f01c1d28ca7593efb4e4b35bc4ee02e0ea Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Sun, 10 Aug 2014 23:36:33 -0400 Subject: [PATCH 02/13] Remove Grade Adjustment code from legacy dash --- .../instructor/tests/test_legacy_reset.py | 70 ----- lms/djangoapps/instructor/views/legacy.py | 268 ------------------ .../legacy_instructor_dashboard.html | 58 +--- 3 files changed, 2 insertions(+), 394 deletions(-) delete mode 100644 lms/djangoapps/instructor/tests/test_legacy_reset.py diff --git a/lms/djangoapps/instructor/tests/test_legacy_reset.py b/lms/djangoapps/instructor/tests/test_legacy_reset.py deleted file mode 100644 index de2d51851a..0000000000 --- a/lms/djangoapps/instructor/tests/test_legacy_reset.py +++ /dev/null @@ -1,70 +0,0 @@ -""" -View-level tests for resetting student state in legacy instructor dash. -""" - -import json -from django.core.urlresolvers import reverse -from django.test.utils import override_settings - -from courseware.tests.helpers import LoginEnrollmentTestCase -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE -from xmodule.modulestore.tests.factories import CourseFactory -from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentFactory - -from courseware.models import StudentModule - -from submissions import api as sub_api -from student.models import anonymous_id_for_user - - -@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) -class InstructorResetStudentStateTest(ModuleStoreTestCase, LoginEnrollmentTestCase): - """ - Reset student state from the legacy instructor dash. - """ - - def setUp(self): - """ - Log in as an instructor, and create a course/student to reset. - """ - instructor = AdminFactory.create() - self.client.login(username=instructor.username, password='test') - self.student = UserFactory.create(username='test', email='test@example.com') - self.course = CourseFactory.create() - CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) - - def test_delete_student_state_resets_scores(self): - problem_location = self.course.id.make_usage_key('dummy', 'module') - - # Create a student module for the user - StudentModule.objects.create( - student=self.student, - course_id=self.course.id, - module_state_key=problem_location, - state=json.dumps({}) - ) - - # Create a submission and score for the student using the submissions API - student_item = { - 'student_id': anonymous_id_for_user(self.student, self.course.id), - 'course_id': self.course.id.to_deprecated_string(), - 'item_id': problem_location.to_deprecated_string(), - 'item_type': 'openassessment' - } - submission = sub_api.create_submission(student_item, 'test answer') - sub_api.set_score(submission['uuid'], 1, 2) - - # Delete student state using the instructor dash - url = reverse('instructor_dashboard_legacy', kwargs={'course_id': self.course.id.to_deprecated_string()}) - response = self.client.post(url, { - 'action': 'Delete student state for module', - 'unique_student_identifier': self.student.email, - 'problem_for_student': problem_location.to_deprecated_string(), - }) - - self.assertEqual(response.status_code, 200) - - # Verify that the student's scores have been reset in the submissions API - score = sub_api.get_score(student_item) - self.assertIs(score, None) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index ea3b6663c1..fac33108e6 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -259,274 +259,6 @@ def instructor_dashboard(request, course_id): track.views.server_track(request, "dump-graded-assignments-config", {}, page="idashboard") msg += dump_grading_context(course) - elif "Rescore ALL students' problem submissions" in action: - problem_location_str = strip_if_string(request.POST.get('problem_for_all_students', '')) - try: - problem_location = course_key.make_usage_key_from_deprecated_string(problem_location_str) - instructor_task = submit_rescore_problem_for_all_students(request, problem_location) - if instructor_task is None: - msg += '{text}'.format( - text=_('Failed to create a background task for rescoring "{problem_url}".').format( - problem_url=problem_location_str - ) - ) - else: - track.views.server_track( - request, - "rescore-all-submissions", - { - "problem": problem_location_str, - "course": course_key.to_deprecated_string() - }, - page="idashboard" - ) - - except (InvalidKeyError, ItemNotFoundError) as err: - msg += '{text}'.format( - text=_('Failed to create a background task for rescoring "{problem_url}": problem not found.').format( - problem_url=problem_location_str - ) - ) - except Exception as err: # pylint: disable=broad-except - log.error("Encountered exception from rescore: {0}".format(err)) - msg += '{text}'.format( - text=_('Failed to create a background task for rescoring "{url}": {message}.').format( - url=problem_location_str, message=err.message - ) - ) - - elif "Reset ALL students' attempts" in action: - problem_location_str = strip_if_string(request.POST.get('problem_for_all_students', '')) - try: - problem_location = course_key.make_usage_key_from_deprecated_string(problem_location_str) - instructor_task = submit_reset_problem_attempts_for_all_students(request, problem_location) - if instructor_task is None: - msg += '{text}'.format( - text=_('Failed to create a background task for resetting "{problem_url}".').format(problem_url=problem_location_str) - ) - else: - track.views.server_track( - request, - "reset-all-attempts", - { - "problem": problem_location_str, - "course": course_key.to_deprecated_string() - }, - page="idashboard" - ) - except (InvalidKeyError, ItemNotFoundError) as err: - log.error('Failure to reset: unknown problem "{0}"'.format(err)) - msg += '{text}'.format( - text=_('Failed to create a background task for resetting "{problem_url}": problem not found.').format( - problem_url=problem_location_str - ) - ) - except Exception as err: # pylint: disable=broad-except - log.error("Encountered exception from reset: {0}".format(err)) - msg += '{text}'.format( - text=_('Failed to create a background task for resetting "{url}": {message}.').format( - url=problem_location_str, message=err.message - ) - ) - - elif "Show Background Task History for Student" in action: - # put this before the non-student case, since the use of "in" will cause this to be missed - unique_student_identifier = request.POST.get('unique_student_identifier', '') - message, student = get_student_from_identifier(unique_student_identifier) - if student is None: - msg += message - else: - problem_location_str = strip_if_string(request.POST.get('problem_for_student', '')) - try: - problem_location = course_key.make_usage_key_from_deprecated_string(problem_location_str) - except InvalidKeyError: - msg += '{text}'.format( - text=_('Could not find problem location "{url}".').format( - url=problem_location_str - ) - ) - else: - message, datatable = get_background_task_table(course_key, problem_location, student) - msg += message - - elif "Show Background Task History" in action: - problem_location_str = strip_if_string(request.POST.get('problem_for_all_students', '')) - try: - problem_location = course_key.make_usage_key_from_deprecated_string(problem_location_str) - except InvalidKeyError: - msg += '{text}'.format( - text=_('Could not find problem location "{url}".').format( - url=problem_location_str - ) - ) - else: - message, datatable = get_background_task_table(course_key, problem_location) - msg += message - - elif ("Reset student's attempts" in action or - "Delete student state for module" in action or - "Rescore student's problem submission" in action): - # get the form data - unique_student_identifier = request.POST.get( - 'unique_student_identifier', '' - ) - problem_location_str = strip_if_string(request.POST.get('problem_for_student', '')) - try: - module_state_key = course_key.make_usage_key_from_deprecated_string(problem_location_str) - except InvalidKeyError: - msg += '{text}'.format( - text=_('Could not find problem location "{url}".').format( - url=problem_location_str - ) - ) - else: - # try to uniquely id student by email address or username - message, student = get_student_from_identifier(unique_student_identifier) - msg += message - student_module = None - if student is not None: - # Reset the student's score in the submissions API - # Currently this is used only by open assessment (ORA 2) - # We need to do this *before* retrieving the `StudentModule` model, - # because it's possible for a score to exist even if no student module exists. - if "Delete student state for module" in action: - try: - sub_api.reset_score( - anonymous_id_for_user(student, course_key), - course_key.to_deprecated_string(), - module_state_key.to_deprecated_string(), - ) - except sub_api.SubmissionError: - # Trust the submissions API to log the error - error_msg = _("An error occurred while deleting the score.") - msg += "{err} ".format(err=error_msg) - - # find the module in question - try: - student_module = StudentModule.objects.get( - student_id=student.id, - course_id=course_key, - module_state_key=module_state_key - ) - msg += _("Found module. ") - - except StudentModule.DoesNotExist as err: - error_msg = _("Couldn't find module with that urlname: {url}. ").format(url=problem_location_str) - msg += "{err_msg} ({err})".format(err_msg=error_msg, err=err) - log.debug(error_msg) - - if student_module is not None: - if "Delete student state for module" in action: - # delete the state - try: - student_module.delete() - - msg += "{text}".format( - text=_("Deleted student module state for {state}!").format(state=module_state_key) - ) - event = { - "problem": problem_location_str, - "student": unique_student_identifier, - "course": course_key.to_deprecated_string() - } - track.views.server_track( - request, - "delete-student-module-state", - event, - page="idashboard" - ) - except Exception as err: # pylint: disable=broad-except - error_msg = _("Failed to delete module state for {id}/{url}. ").format( - id=unique_student_identifier, url=problem_location_str - ) - msg += "{err_msg} ({err})".format(err_msg=error_msg, err=err) - log.exception(error_msg) - elif "Reset student's attempts" in action: - # modify the problem's state - try: - # load the state json - problem_state = json.loads(student_module.state) - old_number_of_attempts = problem_state["attempts"] - problem_state["attempts"] = 0 - # save - student_module.state = json.dumps(problem_state) - student_module.save() - event = { - "old_attempts": old_number_of_attempts, - "student": unicode(student), - "problem": student_module.module_state_key, - "instructor": unicode(request.user), - "course": course_key.to_deprecated_string() - } - track.views.server_track(request, "reset-student-attempts", event, page="idashboard") - msg += "{text}".format( - text=_("Module state successfully reset!") - ) - except Exception as err: # pylint: disable=broad-except - error_msg = _("Couldn't reset module state for {id}/{url}. ").format( - id=unique_student_identifier, url=problem_location_str - ) - msg += "{err_msg} ({err})".format(err_msg=error_msg, err=err) - log.exception(error_msg) - else: - # "Rescore student's problem submission" case - try: - instructor_task = submit_rescore_problem_for_student(request, module_state_key, student) - if instructor_task is None: - msg += '{text}'.format( - text=_('Failed to create a background task for rescoring "{key}" for student {id}.').format( - key=module_state_key, id=unique_student_identifier - ) - ) - else: - track.views.server_track( - request, - "rescore-student-submission", - { - "problem": module_state_key, - "student": unique_student_identifier, - "course": course_key.to_deprecated_string() - }, - page="idashboard" - ) - except Exception as err: # pylint: disable=broad-except - msg += '{text}'.format( - text=_('Failed to create a background task for rescoring "{key}": {id}.').format( - key=module_state_key, id=err.message - ) - ) - log.exception("Encountered exception from rescore: student '{0}' problem '{1}'".format( - unique_student_identifier, module_state_key - ) - ) - - elif "Get link to student's progress page" in action: - unique_student_identifier = request.POST.get('unique_student_identifier', '') - # try to uniquely id student by email address or username - message, student = get_student_from_identifier(unique_student_identifier) - msg += message - if student is not None: - progress_url = reverse('student_progress', kwargs={ - 'course_id': course_key.to_deprecated_string(), - 'student_id': student.id - }) - track.views.server_track( - request, - "get-student-progress-page", - { - "student": unicode(student), - "instructor": unicode(request.user), - "course": course_key.to_deprecated_string() - }, - page="idashboard" - ) - msg += "{text}.".format( - url=progress_url, - text=_("Progress page for username: {username} with email address: {email}").format( - username=student.username, email=student.email - ) - ) - #---------------------------------------- # export grades to remote gradebook diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index c3e5a34b48..00a792a07d 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -263,67 +263,13 @@ function goto( mode) %if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'):

${_("Course-specific grade adjustment")}

-

- ${_("Specify a problem in the course here with its complete location:")} - -

- ## Translators: A location (string of text) follows this sentence. -

${_("You must provide the complete location of the problem. In the Staff Debug viewer, the location looks like this:")}
- i4x://edX/Open_DemoX/problem/78c98390884243b89f6023745231c525

-

- ${_("Then select an action:")} - - -

-

-

${_("These actions run in the background, and status for active tasks will appear in a table below. To see status for all tasks submitted for this problem, click on this button:")} -

-

- -

+

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

-
%endif

${_("Student-specific grade inspection and adjustment")}

-

- ${_("Specify the {platform_name} email address or username of a student here:").format(platform_name=settings.PLATFORM_NAME)} - -

-

- ${_("Click this, and a link to student's progress page will appear below:")} - -

-

- ${_("Specify a problem in the course here with its complete location:")} - -

- ## Translators: A location (string of text) follows this sentence. -

${_("You must provide the complete location of the problem. In the Staff Debug viewer, the location looks like this:")}
- i4x://edX/Open_DemoX/problem/78c98390884243b89f6023745231c525

-

- ${_("Then select an action:")} - - %if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'): - - %endif -

- - %if instructor_access: -

- ${_("You may also delete the entire state of a student for the specified module:")} - -

- %endif - %if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'): -

${_("Rescoring runs in the background, and status for active tasks will appear in a table below. " - "To see status for all tasks submitted for this problem and student, click on this button:")} -

-

- -

- %endif +

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

%endif From d8904d9091ead6c5479652cb0da34f79d70627c6 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Mon, 11 Aug 2014 19:21:04 -0400 Subject: [PATCH 03/13] Clean up Grade Downloads section of legacy dash --- .../tests/test_legacy_download_csv.py | 72 ------------------- .../instructor/tests/test_legacy_xss.py | 3 - lms/djangoapps/instructor/views/legacy.py | 16 ----- .../legacy_instructor_dashboard.html | 12 ++-- 4 files changed, 6 insertions(+), 97 deletions(-) delete mode 100644 lms/djangoapps/instructor/tests/test_legacy_download_csv.py diff --git a/lms/djangoapps/instructor/tests/test_legacy_download_csv.py b/lms/djangoapps/instructor/tests/test_legacy_download_csv.py deleted file mode 100644 index 94f1648e76..0000000000 --- a/lms/djangoapps/instructor/tests/test_legacy_download_csv.py +++ /dev/null @@ -1,72 +0,0 @@ -""" -Unit tests for instructor dashboard - -Based on (and depends on) unit tests for courseware. - -Notes for running by hand: - -./manage.py lms --settings test test lms/djangoapps/instructor -""" - -from django.test.utils import override_settings - -# Need access to internal func to put users in the right group -from django.contrib.auth.models import User - -from django.core.urlresolvers import reverse - -from courseware.tests.helpers import LoginEnrollmentTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE -from student.roles import CourseStaffRole -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory - - -@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) -class TestInstructorDashboardGradeDownloadCSV(ModuleStoreTestCase, LoginEnrollmentTestCase): - ''' - Check for download of csv - ''' - - def setUp(self): - # clear_existing_modulestores() - self.toy = CourseFactory.create(org='edX', course='toy', display_name='2012_Fall') - - # Create two accounts - self.student = 'view@test.com' - self.instructor = 'view2@test.com' - self.password = 'foo' - self.create_account('u1', self.student, self.password) - self.create_account('u2', self.instructor, self.password) - self.activate_user(self.student) - self.activate_user(self.instructor) - - CourseStaffRole(self.toy.id).add_users(User.objects.get(email=self.instructor)) - - self.logout() - self.login(self.instructor, self.password) - self.enroll(self.toy) - - def test_download_grades_csv(self): - course = self.toy - url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) - msg = "url = {0}\n".format(url) - response = self.client.post(url, {'action': 'Download CSV of all student grades for this course'}) - msg += "instructor dashboard download csv grades: response = '{0}'\n".format(response) - - self.assertEqual(response['Content-Type'], 'text/csv', msg) - - cdisp = response['Content-Disposition'] - msg += "Content-Disposition = '%s'\n" % cdisp - self.assertEqual(cdisp, 'attachment; filename=grades_{0}.csv'.format(course.id.to_deprecated_string()), msg) - - body = response.content.replace('\r', '') - msg += "body = '{0}'\n".format(body) - - # All the not-actually-in-the-course hw and labs come from the - # default grading policy string in graders.py - expected_body = '''"ID","Username","Full Name","edX email","External email","HW 01","HW 02","HW 03","HW 04","HW 05","HW 06","HW 07","HW 08","HW 09","HW 10","HW 11","HW 12","HW Avg","Lab 01","Lab 02","Lab 03","Lab 04","Lab 05","Lab 06","Lab 07","Lab 08","Lab 09","Lab 10","Lab 11","Lab 12","Lab Avg","Midterm","Final" -"2","u2","username","view2@test.com","","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0","0" -''' - - self.assertEqual(body, expected_body, msg) diff --git a/lms/djangoapps/instructor/tests/test_legacy_xss.py b/lms/djangoapps/instructor/tests/test_legacy_xss.py index fb5302edf6..e8218dcea4 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_xss.py +++ b/lms/djangoapps/instructor/tests/test_legacy_xss.py @@ -63,6 +63,3 @@ class TestXss(ModuleStoreTestCase): def test_dump_list_of_enrolled(self): self._test_action("Dump list of enrolled students") - - def test_dump_grades(self): - self._test_action("Dump Grades for all students in this course") diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index fac33108e6..c5a1122d39 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -227,12 +227,6 @@ def instructor_dashboard(request, course_id): datatable['title'] = _('List of students enrolled in {course_key}').format(course_key=course_key.to_deprecated_string()) track.views.server_track(request, "list-students", {}, page="idashboard") - elif 'Dump Grades' in action: - log.debug(action) - datatable = get_student_grade_summary_data(request, course, get_grades=True, use_offline=use_offline) - datatable['title'] = _('Summary Grades of students enrolled in {course_key}').format(course_key=course_key.to_deprecated_string()) - track.views.server_track(request, "dump-grades", {}, page="idashboard") - elif 'Dump all RAW grades' in action: log.debug(action) datatable = get_student_grade_summary_data(request, course, get_grades=True, @@ -240,11 +234,6 @@ def instructor_dashboard(request, course_id): datatable['title'] = _('Raw Grades of students enrolled in {course_key}').format(course_key=course_key) track.views.server_track(request, "dump-grades-raw", {}, page="idashboard") - elif 'Download CSV of all student grades' in action: - track.views.server_track(request, "dump-grades-csv", {}, page="idashboard") - return return_csv('grades_{0}.csv'.format(course_key.to_deprecated_string()), - get_student_grade_summary_data(request, course, use_offline=use_offline)) - elif 'Download CSV of all RAW grades' in action: track.views.server_track(request, "dump-grades-csv-raw", {}, page="idashboard") return return_csv('grades_{0}_raw.csv'.format(course_key.to_deprecated_string()), @@ -254,11 +243,6 @@ def instructor_dashboard(request, course_id): track.views.server_track(request, "dump-answer-dist-csv", {}, page="idashboard") return return_csv('answer_dist_{0}.csv'.format(course_key.to_deprecated_string()), get_answers_distribution(request, course_key)) - elif 'Dump description of graded assignments configuration' in action: - # what is "graded assignments configuration"? - track.views.server_track(request, "dump-graded-assignments-config", {}, page="idashboard") - msg += dump_grading_context(course) - #---------------------------------------- # export grades to remote gradebook diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index 00a792a07d..80f17164fe 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -202,16 +202,14 @@ function goto( mode) % endif

- ${_("Gradebook")} + ${_("To view the Gradebook (small courses only), please visit the 'Student Admin' section of the instructor dashboard.")}

-

-

- - +

+ ${_("To download student grades, please visit the 'Data Download' section of the instructor dashboard.")}

@@ -223,7 +221,9 @@ function goto( mode) %if not settings.FEATURES.get('ENABLE_ASYNC_ANSWER_DISTRIBUTION'): %endif - +

+ ${_("To view the graded assignments configuration, please visit the 'Data Download' section of the instructor dashboard.")} +

From 5d7f820d93d88b53da514709d3b6b212a96511fe Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Wed, 13 Aug 2014 15:58:27 -0400 Subject: [PATCH 04/13] Clean up Admin section of legacy dashboard --- lms/djangoapps/instructor/views/legacy.py | 33 ------------------- .../legacy_instructor_dashboard.html | 17 ++-------- 2 files changed, 2 insertions(+), 48 deletions(-) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index c5a1122d39..42dc3ef6f3 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -313,39 +313,6 @@ def instructor_dashboard(request, course_id): msg2, __ = _do_remote_gradebook(request.user, course, 'post-grades', files=files) msg += msg2 - #---------------------------------------- - # Admin - - elif 'List course staff' in action: - role = CourseStaffRole(course.id) - datatable = _role_members_table(role, _("List of Staff"), course_key) - track.views.server_track(request, "list-staff", {}, page="idashboard") - - elif 'List course instructors' in action and GlobalStaff().has_user(request.user): - role = CourseInstructorRole(course.id) - datatable = _role_members_table(role, _("List of Instructors"), course_key) - track.views.server_track(request, "list-instructors", {}, page="idashboard") - - elif action == 'Add course staff': - uname = request.POST['staffuser'] - role = CourseStaffRole(course.id) - msg += add_user_to_role(request, uname, role, 'staff', 'staff') - - elif action == 'Add instructor' and request.user.is_staff: - uname = request.POST['instructor'] - role = CourseInstructorRole(course.id) - msg += add_user_to_role(request, uname, role, 'instructor', 'instructor') - - elif action == 'Remove course staff': - uname = request.POST['staffuser'] - role = CourseStaffRole(course.id) - msg += remove_user_from_role(request, uname, role, 'staff', 'staff') - - elif action == 'Remove instructor' and request.user.is_staff: - uname = request.POST['instructor'] - role = CourseInstructorRole(course.id) - msg += remove_user_from_role(request, uname, role, 'instructor', 'instructor') - #---------------------------------------- # DataDump diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index 80f17164fe..325bab5dee 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -298,24 +298,11 @@ function goto( mode) %if modeflag.get('Admin'): %if instructor_access: -
-

- -

- - - -

+

${_("To add or remove course staff, please visit the 'Membership' section of the instructor dashboard.")}

%endif %if admin_access: -
-

- -

- - -

+

${_("To add or remove course instructors, please visit the 'Membership' section of the instructor dashboard.")}

%endif %if settings.FEATURES['ENABLE_MANUAL_GIT_RELOAD'] and admin_access: From d4a06794d02df7372a001e070e0bc8ca94f1b1a1 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Wed, 13 Aug 2014 16:00:51 -0400 Subject: [PATCH 05/13] Clean up Forum Admin section of legacy dashboard --- .../tests/test_legacy_forum_admin.py | 144 ---------------- lms/djangoapps/instructor/views/legacy.py | 155 ------------------ .../legacy_instructor_dashboard.html | 33 +--- 3 files changed, 1 insertion(+), 331 deletions(-) delete mode 100644 lms/djangoapps/instructor/tests/test_legacy_forum_admin.py diff --git a/lms/djangoapps/instructor/tests/test_legacy_forum_admin.py b/lms/djangoapps/instructor/tests/test_legacy_forum_admin.py deleted file mode 100644 index be323c9eac..0000000000 --- a/lms/djangoapps/instructor/tests/test_legacy_forum_admin.py +++ /dev/null @@ -1,144 +0,0 @@ -""" -Unit tests for instructor dashboard forum administration -""" - - -from django.test.utils import override_settings - -# Need access to internal func to put users in the right group -from django.contrib.auth.models import User - -from django.core.urlresolvers import reverse -from django_comment_common.models import Role, FORUM_ROLE_ADMINISTRATOR, \ - FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_STUDENT -from django_comment_client.utils import has_forum_access - -from courseware.tests.helpers import LoginEnrollmentTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE -from student.roles import CourseStaffRole -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory - - -FORUM_ROLES = [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA] -FORUM_ADMIN_ACTION_SUFFIX = {FORUM_ROLE_ADMINISTRATOR: 'admin', FORUM_ROLE_MODERATOR: 'moderator', FORUM_ROLE_COMMUNITY_TA: 'community TA'} -FORUM_ADMIN_USER = {FORUM_ROLE_ADMINISTRATOR: 'forumadmin', FORUM_ROLE_MODERATOR: 'forummoderator', FORUM_ROLE_COMMUNITY_TA: 'forummoderator'} - - -def action_name(operation, rolename): - if operation == 'List': - return '{0} course forum {1}s'.format(operation, FORUM_ADMIN_ACTION_SUFFIX[rolename]) - else: - return '{0} forum {1}'.format(operation, FORUM_ADMIN_ACTION_SUFFIX[rolename]) - - -@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) -class TestInstructorDashboardForumAdmin(ModuleStoreTestCase, LoginEnrollmentTestCase): - ''' - Check for change in forum admin role memberships - ''' - - def setUp(self): - self.toy = CourseFactory.create(org='edX', course='toy', display_name='2012_Fall') - - # Create two accounts - self.student = 'view@test.com' - self.instructor = 'view2@test.com' - self.password = 'foo' - self.create_account('u1', self.student, self.password) - self.create_account('u2', self.instructor, self.password) - self.activate_user(self.student) - self.activate_user(self.instructor) - - CourseStaffRole(self.toy.id).add_users(User.objects.get(email=self.instructor)) - - self.logout() - self.login(self.instructor, self.password) - self.enroll(self.toy) - - def initialize_roles(self, course_id): - self.admin_role = Role.objects.get_or_create(name=FORUM_ROLE_ADMINISTRATOR, course_id=course_id)[0] - self.moderator_role = Role.objects.get_or_create(name=FORUM_ROLE_MODERATOR, course_id=course_id)[0] - self.community_ta_role = Role.objects.get_or_create(name=FORUM_ROLE_COMMUNITY_TA, course_id=course_id)[0] - - def test_add_forum_admin_users_for_unknown_user(self): - course = self.toy - url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) - username = 'unknown' - for action in ['Add', 'Remove']: - for rolename in FORUM_ROLES: - response = self.client.post(url, {'action': action_name(action, rolename), FORUM_ADMIN_USER[rolename]: username}) - self.assertTrue(response.content.find('Error: unknown username "{0}"'.format(username)) >= 0) - - def test_add_forum_admin_users_for_missing_roles(self): - course = self.toy - url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) - username = 'u1' - for action in ['Add', 'Remove']: - for rolename in FORUM_ROLES: - response = self.client.post(url, {'action': action_name(action, rolename), FORUM_ADMIN_USER[rolename]: username}) - self.assertTrue(response.content.find('Error: unknown rolename "{0}"'.format(rolename)) >= 0) - - def test_remove_forum_admin_users_for_missing_users(self): - course = self.toy - self.initialize_roles(course.id) - url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) - username = 'u1' - action = 'Remove' - for rolename in FORUM_ROLES: - response = self.client.post(url, {'action': action_name(action, rolename), FORUM_ADMIN_USER[rolename]: username}) - self.assertTrue(response.content.find('Error: user "{0}" does not have rolename "{1}"'.format(username, rolename)) >= 0) - - def test_add_and_remove_forum_admin_users(self): - course = self.toy - self.initialize_roles(course.id) - url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) - username = 'u2' - for rolename in FORUM_ROLES: - response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) - self.assertContains(response, 'Added "{0}" to "{1}" forum role = "{2}"'.format(username, course.id.to_deprecated_string(), rolename)) - self.assertTrue(has_forum_access(username, course.id, rolename)) - response = self.client.post(url, {'action': action_name('Remove', rolename), FORUM_ADMIN_USER[rolename]: username}) - self.assertContains(response, 'Removed "{0}" from "{1}" forum role = "{2}"'.format(username, course.id.to_deprecated_string(), rolename)) - self.assertFalse(has_forum_access(username, course.id, rolename)) - - def test_add_and_read_forum_admin_users(self): - course = self.toy - self.initialize_roles(course.id) - url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) - username = 'u2' - for rolename in FORUM_ROLES: - # perform an add, and follow with a second identical add: - self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) - response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) - self.assertTrue(response.content.find('Error: user "{0}" already has rolename "{1}", cannot add'.format(username, rolename)) >= 0) - self.assertTrue(has_forum_access(username, course.id, rolename)) - - def test_add_nonstaff_forum_admin_users(self): - course = self.toy - self.initialize_roles(course.id) - url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) - username = 'u1' - rolename = FORUM_ROLE_ADMINISTRATOR - response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) - self.assertTrue(response.content.find('Error: user "{0}" should first be added as staff'.format(username)) >= 0) - - def test_list_forum_admin_users(self): - course = self.toy - self.initialize_roles(course.id) - url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) - username = 'u2' - added_roles = [FORUM_ROLE_STUDENT] # u2 is already added as a student to the discussion forums - self.assertTrue(has_forum_access(username, course.id, 'Student')) - for rolename in FORUM_ROLES: - response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) - self.assertTrue(has_forum_access(username, course.id, rolename)) - response = self.client.post(url, {'action': action_name('List', rolename), FORUM_ADMIN_USER[rolename]: username}) - for header in ['Username', 'Full name', 'Roles']: - self.assertTrue(response.content.find('{0}'.format(header)) > 0) - self.assertTrue(response.content.find('{0}'.format(username)) >= 0) - # concatenate all roles for user, in sorted order: - added_roles.append(rolename) - added_roles.sort() - roles = ', '.join(added_roles) - self.assertTrue(response.content.find('{0}'.format(roles)) >= 0, 'not finding roles "{0}"'.format(roles)) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 42dc3ef6f3..4276a8c067 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -399,86 +399,6 @@ def instructor_dashboard(request, course_id): msg += "

{0}

".format( remove_user_from_role(request, username_or_email, role, 'beta testers', 'beta-tester')) - #---------------------------------------- - # forum administration - - elif action == 'List course forum admins': - rolename = FORUM_ROLE_ADMINISTRATOR - datatable = {} - msg += _list_course_forum_members(course_key, rolename, datatable) - track.views.server_track( - request, "list-forum-admins", {"course": course_key.to_deprecated_string()}, page="idashboard" - ) - - elif action == 'Remove forum admin': - uname = request.POST['forumadmin'] - msg += _update_forum_role_membership(uname, course, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_REMOVE) - track.views.server_track( - request, "remove-forum-admin", {"username": uname, "course": course_key.to_deprecated_string()}, - page="idashboard" - ) - - elif action == 'Add forum admin': - uname = request.POST['forumadmin'] - msg += _update_forum_role_membership(uname, course, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_ADD) - track.views.server_track( - request, "add-forum-admin", {"username": uname, "course": course_key.to_deprecated_string()}, - page="idashboard" - ) - - elif action == 'List course forum moderators': - rolename = FORUM_ROLE_MODERATOR - datatable = {} - msg += _list_course_forum_members(course_key, rolename, datatable) - track.views.server_track( - request, "list-forum-mods", {"course": course_key.to_deprecated_string()}, page="idashboard" - ) - - elif action == 'Remove forum moderator': - uname = request.POST['forummoderator'] - msg += _update_forum_role_membership(uname, course, FORUM_ROLE_MODERATOR, FORUM_ROLE_REMOVE) - track.views.server_track( - request, "remove-forum-mod", {"username": uname, "course": course_key.to_deprecated_string()}, - page="idashboard" - ) - - elif action == 'Add forum moderator': - uname = request.POST['forummoderator'] - msg += _update_forum_role_membership(uname, course, FORUM_ROLE_MODERATOR, FORUM_ROLE_ADD) - track.views.server_track( - request, "add-forum-mod", {"username": uname, "course": course_key.to_deprecated_string()}, - page="idashboard" - ) - - elif action == 'List course forum community TAs': - rolename = FORUM_ROLE_COMMUNITY_TA - datatable = {} - msg += _list_course_forum_members(course_key, rolename, datatable) - track.views.server_track( - request, "list-forum-community-TAs", {"course": course_key.to_deprecated_string()}, - page="idashboard" - ) - - elif action == 'Remove forum community TA': - uname = request.POST['forummoderator'] - msg += _update_forum_role_membership(uname, course, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_REMOVE) - track.views.server_track( - request, "remove-forum-community-TA", { - "username": uname, "course": course_key.to_deprecated_string() - }, - page="idashboard" - ) - - elif action == 'Add forum community TA': - uname = request.POST['forummoderator'] - msg += _update_forum_role_membership(uname, course, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_ADD) - track.views.server_track( - request, "add-forum-community-TA", { - "username": uname, "course": course_key.to_deprecated_string() - }, - page="idashboard" - ) - #---------------------------------------- # enrollment @@ -778,81 +698,6 @@ def _do_remote_gradebook(user, course, action, args=None, files=None): return msg, datatable - -def _list_course_forum_members(course_key, rolename, datatable): - """ - Fills in datatable with forum membership information, for a given role, - so that it will be displayed on instructor dashboard. - - course_ID = the CourseKey for a course - rolename = one of "Administrator", "Moderator", "Community TA" - - Returns message status string to append to displayed message, if role is unknown. - """ - # make sure datatable is set up properly for display first, before checking for errors - datatable['header'] = [_('Username'), _('Full name'), _('Roles')] - datatable['title'] = _('List of Forum {name}s in course {id}').format( - name=rolename, id=course_key.to_deprecated_string() - ) - datatable['data'] = [] - try: - role = Role.objects.get(name=rolename, course_id=course_key) - except Role.DoesNotExist: - return '' + _('Error: unknown rolename "{rolename}"').format(rolename=rolename) + '' - uset = role.users.all().order_by('username') - msg = 'Role = {0}'.format(rolename) - log.debug('role={0}'.format(rolename)) - datatable['data'] = [[x.username, x.profile.name, ', '.join([ - r.name for r in x.roles.filter(course_id=course_key).order_by('name') - ])] for x in uset] - return msg - - -def _update_forum_role_membership(uname, course, rolename, add_or_remove): - ''' - Supports adding a user to a course's forum role - - uname = username string for user - course = course object - rolename = one of "Administrator", "Moderator", "Community TA" - add_or_remove = one of "add" or "remove" - - Returns message status string to append to displayed message, Status is returned if user - or role is unknown, or if entry already exists when adding, or if entry doesn't exist when removing. - ''' - # check that username and rolename are valid: - try: - user = User.objects.get(username=uname) - except User.DoesNotExist: - return '' + _('Error: unknown username "{username}"').format(username=uname) + '' - try: - role = Role.objects.get(name=rolename, course_id=course.id) - except Role.DoesNotExist: - return '' + _('Error: unknown rolename "{rolename}"').format(rolename=rolename) + '' - - # check whether role already has the specified user: - alreadyexists = role.users.filter(username=uname).exists() - msg = '' - log.debug('rolename={0}'.format(rolename)) - if add_or_remove == FORUM_ROLE_REMOVE: - if not alreadyexists: - msg = '' + _('Error: user "{username}" does not have rolename "{rolename}", cannot remove').format(username=uname, rolename=rolename) + '' - else: - user.roles.remove(role) - msg = '' + _('Removed "{username}" from "{course_id}" forum role = "{rolename}"').format(username=user, course_id=course.id.to_deprecated_string(), rolename=rolename) + '' - else: - if alreadyexists: - msg = '' + _('Error: user "{username}" already has rolename "{rolename}", cannot add').format(username=uname, rolename=rolename) + '' - else: - if (rolename == FORUM_ROLE_ADMINISTRATOR and not has_access(user, 'staff', course)): - msg = '' + _('Error: user "{username}" should first be added as staff before adding as a forum administrator, cannot add').format(username=uname) + '' - else: - user.roles.add(role) - msg = '' + _('Added "{username}" to "{course_id}" forum role = "{rolename}"').format(username=user, course_id=course.id.to_deprecated_string(), rolename=rolename) + '' - - return msg - - def _role_members_table(role, title, course_key): """ Return a data table of usernames and names of users in group_name. diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index 325bab5dee..b8edc06560 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -314,38 +314,7 @@ function goto( mode) ##----------------------------------------------------------------------------- %if modeflag.get('Forum Admin'): - %if instructor_access: -
-

- -

- - -

- %endif - - %if instructor_access or forum_admin_access: -

- - -

- - - - - -

- %else: -

${_("User requires forum administrator privileges to perform administration tasks. See instructor.")}

- %endif - -
-

${_("Explanation of Roles:")}

-

${_("Forum Moderators: can edit or delete any post, remove misuse flags, close and re-open threads, endorse " - "responses, and see posts from all cohorts (if the course is cohorted). Moderators' posts are marked as 'staff'.")}

-

${_("Forum Admins: have moderator privileges, as well as the ability to edit the list of forum moderators " - "(e.g. to appoint a new moderator). Admins' posts are marked as 'staff'.")}

-

${_("Community TAs: have forum moderator privileges, and their posts are labelled 'Community TA'.")}

+

${_("To manage forum roles, please visit the 'Membership' section of the instructor dashboard.")}

%endif ##----------------------------------------------------------------------------- From e3179c37cd1e43e0a89f5aede5e7dec5b0fa44da Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Wed, 13 Aug 2014 16:06:03 -0400 Subject: [PATCH 06/13] Clean up Email section of legacy dashboard --- .../instructor/tests/test_legacy_email.py | 144 ------------------ lms/djangoapps/instructor/views/legacy.py | 77 +--------- .../legacy_instructor_dashboard.html | 73 +-------- 3 files changed, 2 insertions(+), 292 deletions(-) delete mode 100644 lms/djangoapps/instructor/tests/test_legacy_email.py diff --git a/lms/djangoapps/instructor/tests/test_legacy_email.py b/lms/djangoapps/instructor/tests/test_legacy_email.py deleted file mode 100644 index a740a2ef15..0000000000 --- a/lms/djangoapps/instructor/tests/test_legacy_email.py +++ /dev/null @@ -1,144 +0,0 @@ -""" -Unit tests for email feature flag in legacy instructor dashboard. -Additionally tests that bulk email is always disabled for non-Mongo -backed courses, regardless of email feature flag, and that the -view is conditionally available when Course Auth is turned on. -""" -from django.test.utils import override_settings -from django.conf import settings -from django.core.urlresolvers import reverse -from mock import patch - -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE -from student.tests.factories import AdminFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore import ModuleStoreEnum - -from bulk_email.models import CourseAuthorization - - -@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) -class TestInstructorDashboardEmailView(ModuleStoreTestCase): - """ - Check for email view displayed with flag - """ - def setUp(self): - self.course = CourseFactory.create() - - # Create instructor account - instructor = AdminFactory.create() - self.client.login(username=instructor.username, password="test") - - # URL for instructor dash - self.url = reverse('instructor_dashboard_legacy', kwargs={'course_id': self.course.id.to_deprecated_string()}) - # URL for email view - self.email_link = 'Email' - - def tearDown(self): - """ - Undo all patches. - """ - patch.stopall() - - @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) - def test_email_flag_true(self): - # Assert that the URL for the email view is in the response - response = self.client.get(self.url) - self.assertTrue(self.email_link in response.content) - - # Select the Email view of the instructor dash - session = self.client.session - session[u'idash_mode:{0}'.format(self.course.location.course_key.to_deprecated_string())] = 'Email' - session.save() - response = self.client.get(self.url) - - # Ensure we've selected the view properly and that the send_to field is present. - selected_email_link = 'Email' - self.assertTrue(selected_email_link in response.content) - send_to_label = '' - self.assertTrue(send_to_label in response.content) - - @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) - def test_email_flag_unauthorized(self): - # Assert that the URL for the email view is not in the response - # email is enabled, but this course is not authorized to send email - response = self.client.get(self.url) - self.assertFalse(self.email_link in response.content) - - @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) - def test_email_flag_authorized(self): - # Assert that the URL for the email view is in the response - # email is enabled, and this course is authorized to send email - - # Assert that instructor email is not enabled for this course - self.assertFalse(CourseAuthorization.instructor_email_enabled(self.course.id)) - response = self.client.get(self.url) - self.assertFalse(self.email_link in response.content) - - # Authorize the course to use email - cauth = CourseAuthorization(course_id=self.course.id, email_enabled=True) - cauth.save() - - # Assert that instructor email is enabled for this course - self.assertTrue(CourseAuthorization.instructor_email_enabled(self.course.id)) - response = self.client.get(self.url) - self.assertTrue(self.email_link in response.content) - - @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': False}) - def test_email_flag_false(self): - # Assert that the URL for the email view is not in the response - response = self.client.get(self.url) - self.assertFalse(self.email_link in response.content) - - @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) - def test_email_flag_true_xml_store(self): - # If the enable email setting is enabled, but this is an XML backed course, - # the email view shouldn't be available on the instructor dashboard. - - # The course factory uses a MongoModuleStore backing, so patch the - # `get_modulestore_type` method to pretend to be XML-backed. - # This is OK; we're simply testing that the `is_mongo_modulestore_type` flag - # in `instructor/views/legacy.py` is doing the correct thing. - - with patch('xmodule.modulestore.mongo.base.MongoModuleStore.get_modulestore_type') as mock_modulestore: - mock_modulestore.return_value = ModuleStoreEnum.Type.xml - - # Assert that the URL for the email view is not in the response - response = self.client.get(self.url) - self.assertFalse(self.email_link in response.content) - - @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) - def test_send_mail_unauthorized(self): - """ Test 'Send email' action returns an error if course is not authorized to send email. """ - - response = self.client.post( - self.url, { - 'action': 'Send email', - 'to_option': 'all', - 'subject': "Welcome to the course!", - 'message': "Lets start with an introduction!" - } - ) - self.assertContains(response, "Email is not enabled for this course.") - - @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True}) - def test_send_mail_authorized(self): - """ Test 'Send email' action when course is authorized to send email. """ - - course_authorization = CourseAuthorization(course_id=self.course.id, email_enabled=True) - course_authorization.save() - - session = self.client.session - session[u'idash_mode:{0}'.format(self.course.location.course_key.to_deprecated_string())] = 'Email' - session.save() - - response = self.client.post( - self.url, { - 'action': 'Send email', - 'to_option': 'all', - 'subject': 'Welcome to the course!', - 'message': 'Lets start with an introduction!', - } - ) - self.assertContains(response, "Your email was successfully queued for sending.") diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 4276a8c067..e5b7da7da8 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -58,8 +58,7 @@ from instructor_task.api import ( get_instructor_task_history, submit_rescore_problem_for_all_students, submit_rescore_problem_for_student, - submit_reset_problem_attempts_for_all_students, - submit_bulk_course_email + submit_reset_problem_attempts_for_all_students ) from instructor_task.views import get_task_completion_info from edxmako.shortcuts import render_to_response, render_to_string @@ -445,55 +444,6 @@ def instructor_dashboard(request, course_id): ret = _do_enroll_students(course, course_key, students, secure=secure, overload=overload) datatable = ret['datatable'] - #---------------------------------------- - # email - - elif action == 'Send email': - email_to_option = request.POST.get("to_option") - email_subject = request.POST.get("subject") - html_message = request.POST.get("message") - - if bulk_email_is_enabled_for_course(course_key): - try: - # Create the CourseEmail object. This is saved immediately, so that - # any transaction that has been pending up to this point will also be - # committed. - email = CourseEmail.create( - course_key.to_deprecated_string(), request.user, email_to_option, email_subject, html_message - ) - - # Submit the task, so that the correct InstructorTask object gets created (for monitoring purposes) - submit_bulk_course_email(request, course_key, email.id) # pylint: disable=no-member - - except Exception as err: # pylint: disable=broad-except - # Catch any errors and deliver a message to the user - error_msg = "Failed to send email! ({0})".format(err) - msg += "" + error_msg + "" - log.exception(error_msg) - - else: - # If sending the task succeeds, deliver a success message to the user. - if email_to_option == "all": - text = _( - "Your email was successfully queued for sending. " - "Please note that for large classes, it may take up to an hour " - "(or more, if other courses are simultaneously sending email) " - "to send all emails." - ) - else: - text = _('Your email was successfully queued for sending.') - email_msg = '

{text}

'.format(text=text) - else: - msg += "Email is not enabled for this course." - - elif "Show Background Email Task History" in action: - message, datatable = get_background_task_table(course_key, task_type='bulk_course_email') - msg += message - - elif "Show Background Email Task History" in action: - message, datatable = get_background_task_table(course_key, task_type='bulk_course_email') - msg += message - #---------------------------------------- # psychometrics @@ -578,27 +528,6 @@ def instructor_dashboard(request, course_id): if is_studio_course: studio_url = get_cms_course_link(course) - email_editor = None - # HTML editor for email - if idash_mode == 'Email' and is_studio_course: - html_module = HtmlDescriptor( - course.system, - DictFieldData({'data': html_message}), - ScopeIds(None, None, None, course_key.make_usage_key('html', 'dummy')) - ) - fragment = html_module.render('studio_view') - fragment = wrap_xblock( - 'LmsRuntime', html_module, 'studio_view', fragment, None, - extra_data={"course-id": course_key.to_deprecated_string()}, - usage_id_serializer=lambda usage_id: quote_slashes(usage_id.to_deprecated_string()), - request_token=request_token(request), - ) - email_editor = fragment.content - - # Enable instructor email only if the following conditions are met: - # 1. Feature flag is on - # 2. We have explicitly enabled email for the given course via django-admin - # 3. It is NOT an XML course if bulk_email_is_enabled_for_course(course_key): show_email_tab = True @@ -628,10 +557,6 @@ def instructor_dashboard(request, course_id): 'modeflag': {idash_mode: 'selectedmode'}, 'studio_url': studio_url, - 'to_option': email_to_option, # email - 'subject': email_subject, # email - 'editor': email_editor, # email - 'email_msg': email_msg, # email 'show_email_tab': show_email_tab, # email 'problems': problems, # psychometrics diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index b8edc06560..fad8fed6c5 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -407,78 +407,7 @@ function goto( mode) ##----------------------------------------------------------------------------- %if modeflag.get('Email'): - %if email_msg: -

${email_msg}

- %endif - -
    -
  • - - -
    • - -
  • - - %if subject: - - %else: - - %endif - ${_("(Max 128 characters)")} -
    • - -
  • - - - -
    • -
- -
-

${_("Please try not to email students more than once per week. Important things to consider before sending:")}

-
    -
  • ${_("Have you read over the email to make sure it says everything you want to say?")}
    • -
  • ${_("Have you sent the email to yourself first to make sure you're happy with how it's displayed, and that embedded links and images work properly?")}
    • -
-
-

${_("CAUTION!")} - ${_("Once the 'Send Email' button is clicked, your email will be queued for sending.")} - ${_("A queued email CANNOT be cancelled.")}

-
-
- -
- - -
-

These email actions run in the background, and status for active email tasks will appear in a table below. - To see status for all bulk email tasks submitted for this course, click on this button: -

-

- -

+

${_("To send email, please visit the 'Email' section of the instructor dashboard.")}

%endif From 34746013215f043f1681197607de190db935d988 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Wed, 13 Aug 2014 16:29:51 -0400 Subject: [PATCH 07/13] Add CSS rules for legacy dash deprecation warnings --- lms/static/sass/course/instructor/_instructor.scss | 5 +++++ .../courseware/legacy_instructor_dashboard.html | 10 +++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lms/static/sass/course/instructor/_instructor.scss b/lms/static/sass/course/instructor/_instructor.scss index 278c36bd13..d15fbbb125 100644 --- a/lms/static/sass/course/instructor/_instructor.scss +++ b/lms/static/sass/course/instructor/_instructor.scss @@ -2,6 +2,11 @@ display: table; position: relative; + // deprecated content + .deprecated { + font-style: italic; + } + .beta-button-wrapper { position: absolute; top: 2em; diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index fad8fed6c5..05d01c3171 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -201,13 +201,17 @@ function goto( mode) % endif -

+

${_("To view the Gradebook (small courses only), please visit the 'Student Admin' section of the instructor dashboard.")}

+

+ ${_("To perform grade downloads, please visit the 'Data Download' section of the instructor dashboard.")} +

+

${_("To download student grades, please visit the 'Data Download' section of the instructor dashboard.")}

@@ -263,13 +267,13 @@ function goto( mode) %if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'):

${_("Course-specific grade adjustment")}

-

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

+

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

%endif

${_("Student-specific grade inspection and adjustment")}

-

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

+

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

%endif From 762b0d9b6b60b5cea21a649f0e22b3cfe97d211e Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Thu, 20 Nov 2014 22:06:49 -0500 Subject: [PATCH 08/13] Clean up Manage Groups section on legacy dashboard --- lms/djangoapps/instructor/views/legacy.py | 23 ------------------- .../legacy_instructor_dashboard.html | 15 +----------- 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index e5b7da7da8..171d910742 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -375,29 +375,6 @@ def instructor_dashboard(request, course_id): datatable['data'] = [[s.id, unique_id_for_user(s, save=False), anonymous_id_for_user(s, course_key, save=False)] for s in students] return return_csv(course_key.to_deprecated_string().replace('/', '-') + '-anon-ids.csv', datatable) - #---------------------------------------- - # Group management - - elif 'List beta testers' in action: - role = CourseBetaTesterRole(course.id) - datatable = _role_members_table(role, _("List of Beta Testers"), course_key) - track.views.server_track(request, "list-beta-testers", {}, page="idashboard") - - elif action == 'Add beta testers': - users = request.POST['betausers'] - log.debug("users: {0!r}".format(users)) - role = CourseBetaTesterRole(course.id) - for username_or_email in split_by_comma_and_whitespace(users): - msg += "

{0}

".format( - add_user_to_role(request, username_or_email, role, 'beta testers', 'beta-tester')) - - elif action == 'Remove beta testers': - users = request.POST['betausers'] - role = CourseBetaTesterRole(course.id) - for username_or_email in split_by_comma_and_whitespace(users): - msg += "

{0}

".format( - remove_user_from_role(request, username_or_email, role, 'beta testers', 'beta-tester')) - #---------------------------------------- # enrollment diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index 05d01c3171..848c3a9e22 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -386,20 +386,7 @@ function goto( mode) %if modeflag.get('Manage Groups'): %if instructor_access: -
-

- -

- ## Translators: days_early_for_beta should not be translated - ${_("Enter usernames or emails for students who should be beta-testers, one per line, or separated by commas. They will get to " - "see course materials early, as configured via the days_early_for_beta option in the course policy.")} -

-

- - - -

-
+

${_("To manage beta tester roles, please visit the 'Membership' section of the instructor dashboard.")}

%if course.is_cohorted: <%include file="/course_groups/cohort_management.html" /> From 5aeb412bbefd409dc7131139250f038e7b48f359 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Thu, 20 Nov 2014 22:08:53 -0500 Subject: [PATCH 09/13] Remove deprecated cohorts management on legacy dashboard --- lms/djangoapps/instructor/views/legacy.py | 1 - .../course_groups/cohort_management.html | 42 ------------------- lms/templates/course_groups/debug.html | 18 -------- .../legacy_instructor_dashboard.html | 2 +- 4 files changed, 1 insertion(+), 62 deletions(-) delete mode 100644 lms/templates/course_groups/cohort_management.html delete mode 100644 lms/templates/course_groups/debug.html diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 171d910742..71ab0108f2 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -541,7 +541,6 @@ def instructor_dashboard(request, course_id): 'course_errors': modulestore().get_course_errors(course.id), 'instructor_tasks': instructor_tasks, 'offline_grade_log': offline_grades_available(course_key), - 'cohorts_ajax_url': reverse('cohorts', kwargs={'course_key_string': course_key.to_deprecated_string()}), 'analytics_results': analytics_results, 'disable_buttons': disable_buttons, diff --git a/lms/templates/course_groups/cohort_management.html b/lms/templates/course_groups/cohort_management.html deleted file mode 100644 index 4c9ca9e5a2..0000000000 --- a/lms/templates/course_groups/cohort_management.html +++ /dev/null @@ -1,42 +0,0 @@ -<%! from django.utils.translation import ugettext as _ %> -
-

${_("Cohort groups")}

- -
- ${_("Show cohorts")} -
- -
    -
- - - - - - -
diff --git a/lms/templates/course_groups/debug.html b/lms/templates/course_groups/debug.html deleted file mode 100644 index a70d4151ee..0000000000 --- a/lms/templates/course_groups/debug.html +++ /dev/null @@ -1,18 +0,0 @@ - -<%! from django.utils.translation import ugettext as _ %> - - - ## "edX" should not be translated - <%block name="pagetitle"> - - - - - - - -<%include file="/course_groups/cohort_management.html" /> - - - - diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index 848c3a9e22..87758293a1 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -389,7 +389,7 @@ function goto( mode)

${_("To manage beta tester roles, please visit the 'Membership' section of the instructor dashboard.")}

%if course.is_cohorted: - <%include file="/course_groups/cohort_management.html" /> +

${_("To manage course cohorts, please visit the 'Membership' section of the instructor dashboard.")}

%endif %endif From bfa8349ec423de60468e5391d60d43deee436163 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Fri, 21 Nov 2014 09:15:40 -0500 Subject: [PATCH 10/13] Clean up Datadump section on legacy dashboard --- .../instructor/tests/test_legacy_anon_csv.py | 67 ------------------- lms/djangoapps/instructor/views/legacy.py | 34 ---------- .../legacy_instructor_dashboard.html | 10 +-- 3 files changed, 6 insertions(+), 105 deletions(-) delete mode 100644 lms/djangoapps/instructor/tests/test_legacy_anon_csv.py diff --git a/lms/djangoapps/instructor/tests/test_legacy_anon_csv.py b/lms/djangoapps/instructor/tests/test_legacy_anon_csv.py deleted file mode 100644 index 2f298f76dd..0000000000 --- a/lms/djangoapps/instructor/tests/test_legacy_anon_csv.py +++ /dev/null @@ -1,67 +0,0 @@ -""" -Unit tests for instructor dashboard - -Based on (and depends on) unit tests for courseware. - -Notes for running by hand: - -./manage.py lms --settings test test lms/djangoapps/instructor -""" - -from django.test.utils import override_settings - -# Need access to internal func to put users in the right group -from django.contrib.auth.models import User - -from django.core.urlresolvers import reverse - -from courseware.tests.helpers import LoginEnrollmentTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE -import instructor.views.legacy -from student.roles import CourseStaffRole -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory - -from mock import Mock, patch - - -@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) -class TestInstructorDashboardAnonCSV(ModuleStoreTestCase, LoginEnrollmentTestCase): - ''' - Check for download of csv - ''' - - # Note -- I copied this setUp from a similar test - def setUp(self): - # clear_existing_modulestores() - self.toy = CourseFactory.create(org='edX', course='toy', display_name='2012_Fall') - - # Create two accounts - self.student = 'view@test.com' - self.instructor = 'view2@test.com' - self.password = 'foo' - self.create_account('u1', self.student, self.password) - self.create_account('u2', self.instructor, self.password) - self.activate_user(self.student) - self.activate_user(self.instructor) - - CourseStaffRole(self.toy.id).add_users(User.objects.get(email=self.instructor)) - - self.logout() - self.login(self.instructor, self.password) - self.enroll(self.toy) - - @patch.object(instructor.views.legacy, 'anonymous_id_for_user', Mock(return_value='42')) - @patch.object(instructor.views.legacy, 'unique_id_for_user', Mock(return_value='41')) - def test_download_anon_csv(self): - course = self.toy - url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) - response = self.client.post(url, {'action': 'Download CSV of all student anonymized IDs'}) - - self.assertEqual(response['Content-Type'], 'text/csv') - body = response.content.replace('\r', '') - self.assertEqual( - body, - ('"User ID","Anonymized User ID","Course Specific Anonymized User ID"' - '\n"2","41","42"\n') - ) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 71ab0108f2..5b27aa0c0a 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -315,31 +315,6 @@ def instructor_dashboard(request, course_id): #---------------------------------------- # DataDump - elif 'Download CSV of all student profile data' in action: - enrolled_students = User.objects.filter( - courseenrollment__course_id=course_key, - courseenrollment__is_active=1, - ).order_by('username').select_related("profile") - profkeys = ['name', 'language', 'location', 'year_of_birth', 'gender', 'level_of_education', - 'mailing_address', 'goals'] - datatable = {'header': ['username', 'email'] + profkeys} - - def getdat(user): - """ - Return a list of profile data for the given user. - """ - profile = user.profile - return [user.username, user.email] + [getattr(profile, xkey, '') for xkey in profkeys] - - datatable['data'] = [getdat(u) for u in enrolled_students] - datatable['title'] = _('Student profile data for course {course_id}').format( - course_id=course_key.to_deprecated_string() - ) - return return_csv( - 'profiledata_{course_id}.csv'.format(course_id=course_key.to_deprecated_string()), - datatable - ) - elif 'Download CSV of all responses to problem' in action: problem_to_dump = request.POST.get('problem_to_dump', '') @@ -366,15 +341,6 @@ def instructor_dashboard(request, course_id): datatable['title'] = _('Student state for problem {problem}').format(problem=problem_to_dump) return return_csv('student_state_from_{problem}.csv'.format(problem=problem_to_dump), datatable) - elif 'Download CSV of all student anonymized IDs' in action: - students = User.objects.filter( - courseenrollment__course_id=course_key, - ).order_by('id') - - datatable = {'header': ['User ID', 'Anonymized User ID', 'Course Specific Anonymized User ID']} - datatable['data'] = [[s.id, unique_id_for_user(s, save=False), anonymous_id_for_user(s, course_key, save=False)] for s in students] - return return_csv(course_key.to_deprecated_string().replace('/', '-') + '-anon-ids.csv', datatable) - #---------------------------------------- # enrollment diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index 87758293a1..d67514b2fa 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -369,15 +369,17 @@ function goto( mode) %if modeflag.get('Data'):
-

- +

+ ${_("To download student profile data, please visit the 'Data Download' section of the instructor dashboard.")}

+

${_("Problem urlname:")}

-

- + +

+ ${_("To download student anonymized IDs, please visit the 'Data Download' section of the instructor dashboard.")}

%endif From 919e411fd9f99ceb7d10f8e1f602344b83d0d0fa Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Fri, 21 Nov 2014 10:13:53 -0500 Subject: [PATCH 11/13] Only show "Course Statistics at a Glance" in the Admin tab --- .../courseware/legacy_instructor_dashboard.html | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index d67514b2fa..9696d1ec56 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -672,8 +672,7 @@ function goto( mode) ##----------------------------------------------------------------------------- -%if course_stats and modeflag.get('Psychometrics') is None: - +%if modeflag.get('Admin') and course_stats:

@@ -694,6 +693,13 @@ function goto( mode) %endfor

+%else: +
+
+

${_("Course Statistics At A Glance")}

+

+ ${_("These statistics can be viewed under the 'Admin' tab of the legacy instructor dashboard.")} +

%endif ##----------------------------------------------------------------------------- From a5913a4836204af7ab1ee45221898caa7cd0b788 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Fri, 21 Nov 2014 11:49:34 -0500 Subject: [PATCH 12/13] LMS: adding in basic is-deprecated visual styling --- lms/static/sass/base/_base.scss | 5 ++++ lms/static/sass/base/_extends.scss | 7 +++++ .../sass/course/instructor/_instructor.scss | 5 ---- .../legacy_instructor_dashboard.html | 26 +++++++++---------- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/lms/static/sass/base/_base.scss b/lms/static/sass/base/_base.scss index 4a9fbcc1fc..f5437d302e 100644 --- a/lms/static/sass/base/_base.scss +++ b/lms/static/sass/base/_base.scss @@ -315,6 +315,11 @@ mark { display: none; } +// UI - is deprecated +.is-deprecated { + @extend %ui-deprecated; +} + // UI - semantically hide text .sr { @extend %text-sr; diff --git a/lms/static/sass/base/_extends.scss b/lms/static/sass/base/_extends.scss index abfdaa0d5c..7048196f98 100644 --- a/lms/static/sass/base/_extends.scss +++ b/lms/static/sass/base/_extends.scss @@ -113,3 +113,10 @@ padding: 0; } } + +%ui-deprecated { + @extend %t-weight4; + background: tint($warning-color, 85%); + padding: ($baseline/5) ($baseline/2); + color: shade($warning-color, 45%); +} diff --git a/lms/static/sass/course/instructor/_instructor.scss b/lms/static/sass/course/instructor/_instructor.scss index d15fbbb125..278c36bd13 100644 --- a/lms/static/sass/course/instructor/_instructor.scss +++ b/lms/static/sass/course/instructor/_instructor.scss @@ -2,11 +2,6 @@ display: table; position: relative; - // deprecated content - .deprecated { - font-style: italic; - } - .beta-button-wrapper { position: absolute; top: 2em; diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index 9696d1ec56..f6003e1885 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -201,7 +201,7 @@ function goto( mode) % endif -

+

${_("To view the Gradebook (small courses only), please visit the 'Student Admin' section of the instructor dashboard.")}

@@ -225,7 +225,7 @@ function goto( mode) %if not settings.FEATURES.get('ENABLE_ASYNC_ANSWER_DISTRIBUTION'): %endif -

+

${_("To view the graded assignments configuration, please visit the 'Data Download' section of the instructor dashboard.")}

@@ -267,13 +267,13 @@ function goto( mode) %if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'):

${_("Course-specific grade adjustment")}

-

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

+

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

%endif

${_("Student-specific grade inspection and adjustment")}

-

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

+

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

%endif @@ -302,11 +302,11 @@ function goto( mode) %if modeflag.get('Admin'): %if instructor_access: -

${_("To add or remove course staff, please visit the 'Membership' section of the instructor dashboard.")}

+

${_("To add or remove course staff, please visit the 'Membership' section of the instructor dashboard.")}

%endif %if admin_access: -

${_("To add or remove course instructors, please visit the 'Membership' section of the instructor dashboard.")}

+

${_("To add or remove course instructors, please visit the 'Membership' section of the instructor dashboard.")}

%endif %if settings.FEATURES['ENABLE_MANUAL_GIT_RELOAD'] and admin_access: @@ -318,7 +318,7 @@ function goto( mode) ##----------------------------------------------------------------------------- %if modeflag.get('Forum Admin'): -

${_("To manage forum roles, please visit the 'Membership' section of the instructor dashboard.")}

+

${_("To manage forum roles, please visit the 'Membership' section of the instructor dashboard.")}

%endif ##----------------------------------------------------------------------------- @@ -369,7 +369,7 @@ function goto( mode) %if modeflag.get('Data'):
-

+

${_("To download student profile data, please visit the 'Data Download' section of the instructor dashboard.")}

@@ -378,7 +378,7 @@ function goto( mode)

-

+

${_("To download student anonymized IDs, please visit the 'Data Download' section of the instructor dashboard.")}

@@ -388,10 +388,10 @@ function goto( mode) %if modeflag.get('Manage Groups'): %if instructor_access: -

${_("To manage beta tester roles, please visit the 'Membership' section of the instructor dashboard.")}

+

${_("To manage beta tester roles, please visit the 'Membership' section of the instructor dashboard.")}

%if course.is_cohorted: -

${_("To manage course cohorts, please visit the 'Membership' section of the instructor dashboard.")}

+

${_("To manage course cohorts, please visit the 'Membership' section of the instructor dashboard.")}

%endif %endif @@ -400,7 +400,7 @@ function goto( mode) ##----------------------------------------------------------------------------- %if modeflag.get('Email'): -

${_("To send email, please visit the 'Email' section of the instructor dashboard.")}

+

${_("To send email, please visit the 'Email' section of the instructor dashboard.")}

%endif @@ -697,7 +697,7 @@ function goto( mode)

${_("Course Statistics At A Glance")}

-

+

${_("These statistics can be viewed under the 'Admin' tab of the legacy instructor dashboard.")}

%endif From a1c8f8c4686c3b9513e98ac6eff1279fa0ae2077 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Mon, 8 Dec 2014 21:35:49 -0500 Subject: [PATCH 13/13] Style cleanups: legacy instructor dashboard --- lms/djangoapps/instructor/views/legacy.py | 155 +++++++----------- .../legacy_instructor_dashboard.html | 49 ++---- 2 files changed, 74 insertions(+), 130 deletions(-) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 5b27aa0c0a..afcd5c8c4d 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -3,7 +3,6 @@ Instructor Views """ ## NOTE: This is the code for the legacy instructor dashboard ## We are no longer supporting this file or accepting changes into it. -# pylint: skip-file from contextlib import contextmanager import csv import json @@ -27,38 +26,22 @@ from django.core.urlresolvers import reverse from django.core.mail import send_mail from django.utils import timezone -from xmodule_modifiers import wrap_xblock, request_token import xmodule.graders as xmgraders from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from opaque_keys.edx.locations import SlashSeparatedCourseKey -from xmodule.modulestore.exceptions import ItemNotFoundError -from xmodule.html_module import HtmlDescriptor -from opaque_keys import InvalidKeyError -from lms.lib.xblock.runtime import quote_slashes -from submissions import api as sub_api # installed from the edx-submissions repository - -from bulk_email.models import CourseEmail, CourseAuthorization from courseware import grades from courseware.access import has_access from courseware.courses import get_course_with_access, get_cms_course_link -from student.roles import ( - CourseStaffRole, CourseInstructorRole, CourseBetaTesterRole, GlobalStaff -) from courseware.models import StudentModule -from django_comment_common.models import ( - Role, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA -) +from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR from django_comment_client.utils import has_forum_access from instructor.offline_gradecalc import student_grades, offline_grades_available from instructor.views.tools import strip_if_string, bulk_email_is_enabled_for_course, add_block_ids from instructor_task.api import ( get_running_instructor_tasks, get_instructor_task_history, - submit_rescore_problem_for_all_students, - submit_rescore_problem_for_student, - submit_reset_problem_attempts_for_all_students ) from instructor_task.views import get_task_completion_info from edxmako.shortcuts import render_to_response, render_to_string @@ -67,12 +50,8 @@ from psychometrics import psychoanalyze from student.models import ( CourseEnrollment, CourseEnrollmentAllowed, - unique_id_for_user, - anonymous_id_for_user ) import track.views -from xblock.field_data import DictFieldData -from xblock.fields import ScopeIds from django.utils.translation import ugettext as _ from microsite_configuration import microsite @@ -107,10 +86,6 @@ def instructor_dashboard(request, course_id): forum_admin_access = has_forum_access(request.user, course_key, FORUM_ROLE_ADMINISTRATOR) msg = '' - email_msg = '' - email_to_option = None - email_subject = None - html_message = '' show_email_tab = False problems = [] plots = [] @@ -171,23 +146,6 @@ def instructor_dashboard(request, course_id): writer.writerow(encoded_row) return response - def get_student_from_identifier(unique_student_identifier): - """Gets a student object using either an email address or username""" - unique_student_identifier = strip_if_string(unique_student_identifier) - msg = "" - try: - if "@" in unique_student_identifier: - student = User.objects.get(email=unique_student_identifier) - else: - student = User.objects.get(username=unique_student_identifier) - msg += _("Found a single student. ") - except User.DoesNotExist: - student = None - msg += "{text}".format( - text=_("Couldn't find student with that email or username.") - ) - return msg, student - # process actions from form POST action = request.POST.get('action', '') use_offline = request.POST.get('use_offline_grades', False) @@ -266,8 +224,9 @@ def instructor_dashboard(request, course_id): datatable = {'header': ['Student email', 'Match?']} rg_students = [x['email'] for x in rg_stud_data['retdata']] - def domatch(x): - return 'yes' if x.email in rg_students else 'No' + def domatch(student): + """Returns 'yes' if student is pressent in the remote gradebook student list, else returns 'No'""" + return 'yes' if student.email in rg_students else 'No' datatable['data'] = [[x.email, domatch(x)] for x in stud_data['students']] datatable['title'] = action @@ -429,7 +388,7 @@ def instructor_dashboard(request, course_id): analytics_results = {} if idash_mode == 'Analytics': - DASHBOARD_ANALYTICS = [ + dashboard_analytics = [ # "StudentsAttemptedProblems", # num students who tried given problem "StudentsDailyActivity", # active students by day "StudentsDropoffPerDay", # active students dropoff by day @@ -438,7 +397,7 @@ def instructor_dashboard(request, course_id): "ProblemGradeDistribution", # foreach problem, grade distribution ] - for analytic_name in DASHBOARD_ANALYTICS: + for analytic_name in dashboard_analytics: analytics_results[analytic_name] = get_analytics_result(analytic_name) #---------------------------------------- @@ -522,31 +481,31 @@ def _do_remote_gradebook(user, course, action, args=None, files=None): ''' Perform remote gradebook action. Returns msg, datatable. ''' - rg = course.remote_gradebook - if not rg: + rgb = course.remote_gradebook + if not rgb: msg = _("No remote gradebook defined in course metadata") return msg, {} - rgurl = settings.FEATURES.get('REMOTE_GRADEBOOK_URL', '') - if not rgurl: + rgburl = settings.FEATURES.get('REMOTE_GRADEBOOK_URL', '') + if not rgburl: msg = _("No remote gradebook url defined in settings.FEATURES") return msg, {} - rgname = rg.get('name', '') - if not rgname: + rgbname = rgb.get('name', '') + if not rgbname: msg = _("No gradebook name defined in course remote_gradebook metadata") return msg, {} if args is None: args = {} - data = dict(submit=action, gradebook=rgname, user=user.email) + data = dict(submit=action, gradebook=rgbname, user=user.email) data.update(args) try: - resp = requests.post(rgurl, data=data, verify=False, files=files) + resp = requests.post(rgburl, data=data, verify=False, files=files) retdict = json.loads(resp.content) except Exception as err: # pylint: disable=broad-except - msg = _("Failed to communicate with gradebook server at {url}").format(url=rgurl) + "
" + msg = _("Failed to communicate with gradebook server at {url}").format(url=rgburl) + "
" msg += _("Error: {err}").format(err=err) msg += "
resp={resp}".format(resp=resp.content) msg += "
data={data}".format(data=data) @@ -565,6 +524,7 @@ def _do_remote_gradebook(user, course, action, args=None, files=None): return msg, datatable + def _role_members_table(role, title, course_key): """ Return a data table of usernames and names of users in group_name. @@ -784,7 +744,7 @@ def get_student_grade_summary_data(request, course, get_grades=True, get_raw_sco datarow = [student.id, student.username, student.profile.name, student.email] try: datarow.append(student.externalauthmap.external_email) - except: # ExternalAuthMap.DoesNotExist + except Exception: # pylint: disable=broad-except datarow.append('') if get_grades: @@ -843,12 +803,12 @@ def _do_enroll_students(course, course_key, students, secure=False, overload=Fal if overload: # delete all but staff todelete = CourseEnrollment.objects.filter(course_id=course_key) - for ce in todelete: - if not has_access(ce.user, 'staff', course) and ce.user.email.lower() not in new_students_lc: - status[ce.user.email] = 'deleted' - ce.deactivate() + for enrollee in todelete: + if not has_access(enrollee.user, 'staff', course) and enrollee.user.email.lower() not in new_students_lc: + status[enrollee.user.email] = 'deleted' + enrollee.deactivate() else: - status[ce.user.email] = 'is staff' + status[enrollee.user.email] = 'is staff' ceaset = CourseEnrollmentAllowed.objects.filter(course_id=course_key) for cea in ceaset: status[cea.email] = 'removed from pending enrollment list' @@ -882,7 +842,7 @@ def _do_enroll_students(course, course_key, students, secure=False, overload=Fal ) # Composition of email - d = { + email_data = { 'site_name': stripped_site_name, 'registration_url': registration_url, 'course': course, @@ -897,11 +857,11 @@ def _do_enroll_students(course, course_key, students, secure=False, overload=Fal user = User.objects.get(email=student) except User.DoesNotExist: - #Student not signed up yet, put in pending enrollment allowed table + # Student not signed up yet, put in pending enrollment allowed table cea = CourseEnrollmentAllowed.objects.filter(email=student, course_id=course_key) - #If enrollmentallowed already exists, update auto_enroll flag to however it was set in UI - #Will be 0 or 1 records as there is a unique key on email + course_id + # If enrollmentallowed already exists, update auto_enroll flag to however it was set in UI + # Will be 0 or 1 records as there is a unique key on email + course_id if cea: cea[0].auto_enroll = auto_enroll cea[0].save() @@ -909,7 +869,7 @@ def _do_enroll_students(course, course_key, students, secure=False, overload=Fal + ('on' if auto_enroll else 'off') continue - #EnrollmentAllowed doesn't exist so create it + # EnrollmentAllowed doesn't exist so create it cea = CourseEnrollmentAllowed(email=student, course_id=course_key, auto_enroll=auto_enroll) cea.save() @@ -918,9 +878,9 @@ def _do_enroll_students(course, course_key, students, secure=False, overload=Fal if email_students: # User is allowed to enroll but has not signed up yet - d['email_address'] = student - d['message'] = 'allowed_enroll' - send_mail_ret = send_mail_to_student(student, d) + email_data['email_address'] = student + email_data['message'] = 'allowed_enroll' + send_mail_ret = send_mail_to_student(student, email_data) status[student] += (', email sent' if send_mail_ret else '') continue @@ -936,13 +896,13 @@ def _do_enroll_students(course, course_key, students, secure=False, overload=Fal if email_students: # User enrolled for first time, populate dict with user specific info - d['email_address'] = student - d['full_name'] = user.profile.name - d['message'] = 'enrolled_enroll' - send_mail_ret = send_mail_to_student(student, d) + email_data['email_address'] = student + email_data['full_name'] = user.profile.name + email_data['message'] = 'enrolled_enroll' + send_mail_ret = send_mail_to_student(student, email_data) status[student] += (', email sent' if send_mail_ret else '') - except: + except Exception: # pylint: disable=broad-except status[student] = 'rejected' datatable = {'header': ['StudentEmail', 'action']} @@ -977,15 +937,17 @@ def _do_unenroll_students(course_key, students, email_students=False): ) if email_students: course = modulestore().get_course(course_key) - #Composition of email - d = {'site_name': stripped_site_name, - 'course': course} + # Composition of email + data = { + 'site_name': stripped_site_name, + 'course': course + } for student in old_students: isok = False cea = CourseEnrollmentAllowed.objects.filter(course_id=course_key, email=student) - #Will be 0 or 1 records as there is a unique key on email + course_id + # Will be 0 or 1 records as there is a unique key on email + course_id if cea: cea[0].delete() status[student] = "un-enrolled" @@ -996,25 +958,25 @@ def _do_unenroll_students(course_key, students, email_students=False): except User.DoesNotExist: if isok and email_students: - #User was allowed to join but had not signed up yet - d['email_address'] = student - d['message'] = 'allowed_unenroll' - send_mail_ret = send_mail_to_student(student, d) + # User was allowed to join but had not signed up yet + data['email_address'] = student + data['message'] = 'allowed_unenroll' + send_mail_ret = send_mail_to_student(student, data) status[student] += (', email sent' if send_mail_ret else '') continue - #Will be 0 or 1 records as there is a unique key on user + course_id + # Will be 0 or 1 records as there is a unique key on user + course_id if CourseEnrollment.is_enrolled(user, course_key): try: CourseEnrollment.unenroll(user, course_key) status[student] = "un-enrolled" if email_students: - #User was enrolled - d['email_address'] = student - d['full_name'] = user.profile.name - d['message'] = 'enrolled_unenroll' - send_mail_ret = send_mail_to_student(student, d) + # User was enrolled + data['email_address'] = student + data['full_name'] = user.profile.name + data['message'] = 'enrolled_unenroll' + send_mail_ret = send_mail_to_student(student, data) status[student] += (', email sent' if send_mail_ret else '') except Exception: # pylint: disable=broad-except @@ -1025,8 +987,7 @@ def _do_unenroll_students(course_key, students, email_students=False): datatable['data'] = [[x, status[x]] for x in sorted(status)] datatable['title'] = _('Un-enrollment of students') - data = dict(datatable=datatable) - return data + return dict(datatable=datatable) def send_mail_to_student(student, param_dict): @@ -1123,15 +1084,15 @@ def get_answers_distribution(request, course_key): dist = grades.answer_distributions(course.id) - d = {} - d['header'] = ['url_name', 'display name', 'answer id', 'answer', 'count'] + dist = {} + dist['header'] = ['url_name', 'display name', 'answer id', 'answer', 'count'] - d['data'] = [ + dist['data'] = [ [url_name, display_name, answer_id, a, answers[a]] for (url_name, display_name, answer_id), answers in sorted(dist.items()) for a in answers ] - return d + return dist #----------------------------------------------------------------------------- @@ -1153,8 +1114,8 @@ def compute_course_stats(course): children = module.get_children() category = module.__class__.__name__ # HtmlDescriptor, CapaDescriptor, ... counts[category] += 1 - for c in children: - walk(c) + for child in children: + walk(child) walk(course) stats = dict(counts) # number of each kind of module diff --git a/lms/templates/courseware/legacy_instructor_dashboard.html b/lms/templates/courseware/legacy_instructor_dashboard.html index f6003e1885..cf0733c433 100644 --- a/lms/templates/courseware/legacy_instructor_dashboard.html +++ b/lms/templates/courseware/legacy_instructor_dashboard.html @@ -136,7 +136,7 @@ function goto( mode) ${_("Back To Instructor Dashboard")} -

${_("Instructor Dashboard")}

+

${_("Legacy Instructor Dashboard")}

%if settings.FEATURES.get('IS_EDX_DOMAIN', False): ## Only show this banner on the edx.org website (other sites may choose to show this if they wish) @@ -201,21 +201,10 @@ function goto( mode) % endif -

- ${_("To view the Gradebook (small courses only), please visit the 'Student Admin' section of the instructor dashboard.")} -

-

- ${_("To perform grade downloads, please visit the 'Data Download' section of the instructor dashboard.")} -

- -

- ${_("To download student grades, please visit the 'Data Download' section of the instructor dashboard.")} -

-

@@ -226,7 +215,10 @@ function goto( mode) %endif

- ${_("To view the graded assignments configuration, please visit the 'Data Download' section of the instructor dashboard.")} + ${_("To download student grades and view the grading configuration for your course, visit the Data Download section of the Instructor Dashboard.")} +

+

+ ${_("To view the Gradebook (only available for courses with a small number of enrolled students), visit the Student Admin section of the Instructor Dashboard.")}

@@ -267,13 +259,13 @@ function goto( mode) %if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'):

${_("Course-specific grade adjustment")}

-

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

+

${_("To perform these actions, visit the Student Admin section of the Instructor Dashboard.")}

%endif

${_("Student-specific grade inspection and adjustment")}

-

${_("To perform these actions, please visit the 'Student Admin' section of the instructor dashboard.")}

+

${_("To perform these actions, visit the Student Admin section of the Instructor Dashboard.")}

%endif @@ -301,12 +293,8 @@ function goto( mode) ##----------------------------------------------------------------------------- %if modeflag.get('Admin'): - %if instructor_access: -

${_("To add or remove course staff, please visit the 'Membership' section of the instructor dashboard.")}

- %endif - - %if admin_access: -

${_("To add or remove course instructors, please visit the 'Membership' section of the instructor dashboard.")}

+ %if instructor_access or admin_access: +

${_("To add or remove course staff or instructors, visit the Membership section of the Instructor Dashboard.")}

%endif %if settings.FEATURES['ENABLE_MANUAL_GIT_RELOAD'] and admin_access: @@ -318,7 +306,7 @@ function goto( mode) ##----------------------------------------------------------------------------- %if modeflag.get('Forum Admin'): -

${_("To manage forum roles, please visit the 'Membership' section of the instructor dashboard.")}

+

${_("To manage forum roles, visit the Membership section of the Instructor Dashboard.")}

%endif ##----------------------------------------------------------------------------- @@ -369,17 +357,13 @@ function goto( mode) %if modeflag.get('Data'):
-

- ${_("To download student profile data, please visit the 'Data Download' section of the instructor dashboard.")} -

-

${_("Problem urlname:")}

- ${_("To download student anonymized IDs, please visit the 'Data Download' section of the instructor dashboard.")} + ${_("To download student profile data and anonymized IDs, visit the Data Download section of the Instructor Dashboard.")}

%endif @@ -388,19 +372,18 @@ function goto( mode) %if modeflag.get('Manage Groups'): %if instructor_access: -

${_("To manage beta tester roles, please visit the 'Membership' section of the instructor dashboard.")}

- %if course.is_cohorted: -

${_("To manage course cohorts, please visit the 'Membership' section of the instructor dashboard.")}

+

${_("To manage beta tester roles and cohort groups, visit the Membership section of the Instructor Dashboard.")}

+ %else: +

${_("To manage beta tester roles, visit the Membership section of the Instructor Dashboard.")}

%endif - %endif %endif ##----------------------------------------------------------------------------- %if modeflag.get('Email'): -

${_("To send email, please visit the 'Email' section of the instructor dashboard.")}

+

${_("To send email, visit the Email section of the Instructor Dashboard.")}

%endif @@ -698,7 +681,7 @@ function goto( mode)

${_("Course Statistics At A Glance")}

- ${_("These statistics can be viewed under the 'Admin' tab of the legacy instructor dashboard.")} + ${_("View course statistics in the Admin section of this legacy instructor dashboard.")}

%endif