From 640d00c5bef3a31aa331f4cd7acce1a99e41bda1 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Mon, 26 Aug 2013 17:25:34 -0400 Subject: [PATCH 1/3] strip processed fields in instructor dash --- lms/djangoapps/instructor/views/api.py | 23 +++++++++++++-------- lms/djangoapps/instructor/views/legacy.py | 25 +++++++++++++++++------ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 2fcc953e52..1684ba760c 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -233,7 +233,7 @@ def modify_access(request, course_id): request.user, course_id, 'instructor', depth=None ) - email = request.GET.get('email') + email = _clean_field(request.GET.get('email')) rolename = request.GET.get('rolename') action = request.GET.get('action') @@ -433,7 +433,7 @@ def get_student_progress_url(request, course_id): 'progress_url': '/../...' } """ - student_email = request.GET.get('student_email') + student_email = _clean_field(request.GET.get('student_email')) user = User.objects.get(email=student_email) progress_url = reverse('student_progress', kwargs={'course_id': course_id, 'student_id': user.id}) @@ -474,8 +474,8 @@ def reset_student_attempts(request, course_id): request.user, course_id, 'staff', depth=None ) - problem_to_reset = request.GET.get('problem_to_reset') - student_email = request.GET.get('student_email') + problem_to_reset = _clean_field(request.GET.get('problem_to_reset')) + student_email = _clean_field(request.GET.get('student_email')) all_students = request.GET.get('all_students', False) in ['true', 'True', True] delete_module = request.GET.get('delete_module', False) in ['true', 'True', True] @@ -531,8 +531,8 @@ def rescore_problem(request, course_id): all_students and student_email cannot both be present. """ - problem_to_reset = request.GET.get('problem_to_reset') - student_email = request.GET.get('student_email', False) + problem_to_reset = _clean_field(request.GET.get('problem_to_reset')) + student_email = _clean_field(request.GET.get('student_email', False)) all_students = request.GET.get('all_students') in ['true', 'True', True] if not (problem_to_reset and (all_students or student_email)): @@ -576,8 +576,8 @@ def list_instructor_tasks(request, course_id): - `problem_urlname` and `student_email` lists task history for problem AND student (intersection) """ - problem_urlname = request.GET.get('problem_urlname', False) - student_email = request.GET.get('student_email', False) + problem_urlname = _clean_field(request.GET.get('problem_urlname', False)) + student_email = _clean_field(request.GET.get('student_email', False)) if student_email and not problem_urlname: return HttpResponseBadRequest( @@ -693,7 +693,7 @@ def update_forum_role_membership(request, course_id): request.user, course_id, FORUM_ROLE_ADMINISTRATOR ) - email = request.GET.get('email') + email = _clean_field(request.GET.get('email')) rolename = request.GET.get('rolename') action = request.GET.get('action') @@ -815,3 +815,8 @@ def _msk_from_problem_urlname(course_id, urlname): (org, course_name, __) = course_id.split("/") module_state_key = "i4x://" + org + "/" + course_name + "/" + urlname return module_state_key + +def _clean_field(field): + if field: + return field.strip() + return field diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 3e9ff96f1c..df17d15e2e 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -171,6 +171,9 @@ def instructor_dashboard(request, course_id): Form is either urlname or modulename/urlname. If no modulename is provided, "problem" is assumed. """ + # remove whitespace + urlname = _clean_field(urlname) + # tolerate an XML suffix in the urlname if urlname[-4:] == ".xml": urlname = urlname[:-4] @@ -185,6 +188,7 @@ def instructor_dashboard(request, course_id): def get_student_from_identifier(unique_student_identifier): """Gets a student object using either an email address or username""" + unique_student_identifier = _clean_field(unique_student_identifier) msg = "" try: if "@" in unique_student_identifier: @@ -706,12 +710,14 @@ def instructor_dashboard(request, course_id): html_message = request.POST.get("message") text_message = html_to_text(html_message) - email = CourseEmail(course_id=course_id, - sender=request.user, - to_option=email_to_option, - subject=email_subject, - html_message=html_message, - text_message=text_message) + email = CourseEmail( + course_id=course_id, + sender=request.user, + to_option=email_to_option, + subject=email_subject, + html_message=html_message, + text_message=text_message + ) email.save() @@ -994,6 +1000,7 @@ def _add_or_remove_user_group(request, username_or_email, group, group_title, ev to do. """ user = None + username_or_email = _clean_field(username_or_email) try: if '@' in username_or_email: user = User.objects.get(email=username_or_email) @@ -1561,3 +1568,9 @@ def get_background_task_table(course_id, problem_url, student=None): datatable['title'] = "{course_id} > {location}".format(course_id=course_id, location=problem_url) return msg, datatable + +def _clean_field(field): + if field: + return field.strip() + return field + From bc424173ad4684f7d11bd410ce21a31e9e0db517 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Tue, 10 Sep 2013 13:51:42 -0400 Subject: [PATCH 2/3] move _clean_fields to tools.py --- lms/djangoapps/instructor/views/api.py | 6 +----- lms/djangoapps/instructor/views/legacy.py | 7 +------ lms/djangoapps/instructor/views/tools.py | 7 +++++++ 3 files changed, 9 insertions(+), 11 deletions(-) create mode 100644 lms/djangoapps/instructor/views/tools.py diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 1684ba760c..9756b42ee6 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -32,6 +32,7 @@ import instructor_task.api from instructor_task.api_helper import AlreadyRunningError import instructor.enrollment as enrollment from instructor.enrollment import enroll_email, unenroll_email +from instructor.views.tools import _clean_field import instructor.access as access import analytics.basic import analytics.distributions @@ -815,8 +816,3 @@ def _msk_from_problem_urlname(course_id, urlname): (org, course_name, __) = course_id.split("/") module_state_key = "i4x://" + org + "/" + course_name + "/" + urlname return module_state_key - -def _clean_field(field): - if field: - return field.strip() - return field diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index df17d15e2e..a6eb70a608 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -41,6 +41,7 @@ from django_comment_common.models import (Role, FORUM_ROLE_COMMUNITY_TA) from django_comment_client.utils import has_forum_access from instructor.offline_gradecalc import student_grades, offline_grades_available +from instructor.views.tools import _clean_field from instructor_task.api import (get_running_instructor_tasks, get_instructor_task_history, submit_rescore_problem_for_all_students, @@ -1568,9 +1569,3 @@ def get_background_task_table(course_id, problem_url, student=None): datatable['title'] = "{course_id} > {location}".format(course_id=course_id, location=problem_url) return msg, datatable - -def _clean_field(field): - if field: - return field.strip() - return field - diff --git a/lms/djangoapps/instructor/views/tools.py b/lms/djangoapps/instructor/views/tools.py new file mode 100644 index 0000000000..8ab98216e3 --- /dev/null +++ b/lms/djangoapps/instructor/views/tools.py @@ -0,0 +1,7 @@ +""" +Tools for the instructor dashboard +""" +def _clean_field(field): + if field: + return field.strip() + return field From 8bbc11cb2b847c3f956eca54039c73d090651417 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Tue, 10 Sep 2013 18:01:30 -0400 Subject: [PATCH 3/3] rename stripping function, make it more explicit --- lms/djangoapps/instructor/views/api.py | 20 ++++++++++---------- lms/djangoapps/instructor/views/legacy.py | 8 ++++---- lms/djangoapps/instructor/views/tools.py | 8 ++++---- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 9756b42ee6..6c68b7fed6 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -32,7 +32,7 @@ import instructor_task.api from instructor_task.api_helper import AlreadyRunningError import instructor.enrollment as enrollment from instructor.enrollment import enroll_email, unenroll_email -from instructor.views.tools import _clean_field +from instructor.views.tools import strip_if_string import instructor.access as access import analytics.basic import analytics.distributions @@ -234,7 +234,7 @@ def modify_access(request, course_id): request.user, course_id, 'instructor', depth=None ) - email = _clean_field(request.GET.get('email')) + email = strip_if_string(request.GET.get('email')) rolename = request.GET.get('rolename') action = request.GET.get('action') @@ -434,7 +434,7 @@ def get_student_progress_url(request, course_id): 'progress_url': '/../...' } """ - student_email = _clean_field(request.GET.get('student_email')) + student_email = strip_if_string(request.GET.get('student_email')) user = User.objects.get(email=student_email) progress_url = reverse('student_progress', kwargs={'course_id': course_id, 'student_id': user.id}) @@ -475,8 +475,8 @@ def reset_student_attempts(request, course_id): request.user, course_id, 'staff', depth=None ) - problem_to_reset = _clean_field(request.GET.get('problem_to_reset')) - student_email = _clean_field(request.GET.get('student_email')) + problem_to_reset = strip_if_string(request.GET.get('problem_to_reset')) + student_email = strip_if_string(request.GET.get('student_email')) all_students = request.GET.get('all_students', False) in ['true', 'True', True] delete_module = request.GET.get('delete_module', False) in ['true', 'True', True] @@ -532,8 +532,8 @@ def rescore_problem(request, course_id): all_students and student_email cannot both be present. """ - problem_to_reset = _clean_field(request.GET.get('problem_to_reset')) - student_email = _clean_field(request.GET.get('student_email', False)) + problem_to_reset = strip_if_string(request.GET.get('problem_to_reset')) + student_email = strip_if_string(request.GET.get('student_email', False)) all_students = request.GET.get('all_students') in ['true', 'True', True] if not (problem_to_reset and (all_students or student_email)): @@ -577,8 +577,8 @@ def list_instructor_tasks(request, course_id): - `problem_urlname` and `student_email` lists task history for problem AND student (intersection) """ - problem_urlname = _clean_field(request.GET.get('problem_urlname', False)) - student_email = _clean_field(request.GET.get('student_email', False)) + problem_urlname = strip_if_string(request.GET.get('problem_urlname', False)) + student_email = strip_if_string(request.GET.get('student_email', False)) if student_email and not problem_urlname: return HttpResponseBadRequest( @@ -694,7 +694,7 @@ def update_forum_role_membership(request, course_id): request.user, course_id, FORUM_ROLE_ADMINISTRATOR ) - email = _clean_field(request.GET.get('email')) + email = strip_if_string(request.GET.get('email')) rolename = request.GET.get('rolename') action = request.GET.get('action') diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index a6eb70a608..9e77f55a29 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -41,7 +41,7 @@ from django_comment_common.models import (Role, FORUM_ROLE_COMMUNITY_TA) from django_comment_client.utils import has_forum_access from instructor.offline_gradecalc import student_grades, offline_grades_available -from instructor.views.tools import _clean_field +from instructor.views.tools import strip_if_string from instructor_task.api import (get_running_instructor_tasks, get_instructor_task_history, submit_rescore_problem_for_all_students, @@ -173,7 +173,7 @@ def instructor_dashboard(request, course_id): is provided, "problem" is assumed. """ # remove whitespace - urlname = _clean_field(urlname) + urlname = strip_if_string(urlname) # tolerate an XML suffix in the urlname if urlname[-4:] == ".xml": @@ -189,7 +189,7 @@ def instructor_dashboard(request, course_id): def get_student_from_identifier(unique_student_identifier): """Gets a student object using either an email address or username""" - unique_student_identifier = _clean_field(unique_student_identifier) + unique_student_identifier = strip_if_string(unique_student_identifier) msg = "" try: if "@" in unique_student_identifier: @@ -1001,7 +1001,7 @@ def _add_or_remove_user_group(request, username_or_email, group, group_title, ev to do. """ user = None - username_or_email = _clean_field(username_or_email) + username_or_email = strip_if_string(username_or_email) try: if '@' in username_or_email: user = User.objects.get(email=username_or_email) diff --git a/lms/djangoapps/instructor/views/tools.py b/lms/djangoapps/instructor/views/tools.py index 8ab98216e3..11fc135976 100644 --- a/lms/djangoapps/instructor/views/tools.py +++ b/lms/djangoapps/instructor/views/tools.py @@ -1,7 +1,7 @@ """ Tools for the instructor dashboard """ -def _clean_field(field): - if field: - return field.strip() - return field +def strip_if_string(value): + if isinstance(value, basestring): + return value.strip() + return value