From d3b3ac8e8c206814805d1f7cdd21c944b1cdab5a Mon Sep 17 00:00:00 2001 From: Amir Qayyum Khan Date: Mon, 18 Jul 2016 17:49:48 +0500 Subject: [PATCH 1/2] Hide coaches/staff/instructors from coach's enrollment tab, gradebook, grades csv file --- lms/djangoapps/ccx/tests/test_utils.py | 43 ++++++++++++++++++- lms/djangoapps/ccx/tests/test_views.py | 33 ++++++++++++++ lms/djangoapps/ccx/utils.py | 20 +++++++++ lms/djangoapps/ccx/views.py | 30 +++++++++++-- .../instructor/views/gradebook_api.py | 6 ++- 5 files changed, 127 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/ccx/tests/test_utils.py b/lms/djangoapps/ccx/tests/test_utils.py index 76c1ba996773..a807cedde6ab 100644 --- a/lms/djangoapps/ccx/tests/test_utils.py +++ b/lms/djangoapps/ccx/tests/test_utils.py @@ -12,7 +12,10 @@ CourseInstructorRole, CourseStaffRole, ) -from student.tests.factories import AdminFactory +from student.tests.factories import ( + AdminFactory, + UserFactory, +) from student.models import CourseEnrollment, CourseEnrollmentException @@ -31,6 +34,7 @@ from lms.djangoapps.ccx.utils import ( add_master_course_staff_to_ccx, ccx_course, + list_course_members, remove_master_course_staff_from_ccx ) from lms.djangoapps.ccx.tests.factories import CcxFactory @@ -112,6 +116,43 @@ def test_get_chapters(self): ) +@attr('shard_1') +class TestCCXMembers(CcxTestCase): + """ + Tests for the `list_course_members` util function + """ + def setUp(self): + super(TestCCXMembers, self).setUp() + self.staff = self.make_staff() + self.instructor = self.make_instructor() + self.student = UserFactory.create(is_staff=False) + + self.make_coach() + self.ccx = self.make_ccx() + self.ccx_locator = CCXLocator.from_course_locator(self.course.id, self.ccx.id) + + def create_ccx_and_enroll(self): + """ + Create student and setup staff users. + """ + CourseEnrollment.enroll(self.student, self.ccx_locator) + add_master_course_staff_to_ccx(self.course, self.ccx_locator, self.ccx.display_name) + + def test_members_list(self): + """ + Assert that list of enrolled student do not have staff and coaches. + """ + self.create_ccx_and_enroll() + self.assertTrue(CourseStaffRole(self.course.id).has_user(self.staff)) + self.assertTrue(CourseInstructorRole(self.course.id).has_user(self.instructor)) + + members = [enrollment.user for enrollment in list_course_members(self.ccx_locator)] + self.assertIn(self.student, members) + self.assertNotIn(self.staff, members) + self.assertNotIn(self.instructor, members) + self.assertNotIn(self.coach, members) + + class TestStaffOnCCX(CcxTestCase): """ Tests for staff on ccx courses. diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index e4fce7b45ddf..c04e96e903d1 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -1026,6 +1026,30 @@ def setUp(self): self.client.login(username=coach.username, password="test") self.addCleanup(RequestCache.clear_request_cache) + # create staff and instructor on course. + self.staff = self.make_staff() + self.instructor = self.make_instructor() + + def make_staff(self): + """ + create staff user. + """ + staff = UserFactory.create(password="test") + role = CourseStaffRole(self.course.id) + role.add_users(staff) + + return staff + + def make_instructor(self): + """ + create instructor user. + """ + instructor = UserFactory.create(password="test") + role = CourseInstructorRole(self.course.id) + role.add_users(instructor) + + return instructor + @patch('ccx.views.render_to_response', intercept_renderer) @patch('instructor.views.gradebook_api.MAX_STUDENTS_PER_PAGE_GRADE_BOOK', 1) def test_gradebook(self): @@ -1040,6 +1064,11 @@ def test_gradebook(self): self.assertEqual(response.status_code, 200) # Max number of student per page is one. Patched setting MAX_STUDENTS_PER_PAGE_GRADE_BOOK = 1 self.assertEqual(len(response.mako_context['students']), 1) # pylint: disable=no-member + + self.assertNotIn(self.staff.username, response.mako_context['students']) + self.assertNotIn(self.instructor.username, response.mako_context['students']) + self.assertNotIn(self.coach.username, response.mako_context['students']) + student_info = response.mako_context['students'][0] # pylint: disable=no-member self.assertEqual(student_info['grade_summary']['percent'], 0.5) self.assertEqual( @@ -1066,6 +1095,10 @@ def test_grades_csv(self): rows = response.content.strip().split('\r') headers = rows[0] + self.assertNotIn(self.staff.username, rows) + self.assertNotIn(self.instructor.username, rows) + self.assertNotIn(self.coach.username, rows) + # picking first student records data = dict(zip(headers.strip().split(','), rows[1].strip().split(','))) self.assertNotIn('HW 04', data) diff --git a/lms/djangoapps/ccx/utils.py b/lms/djangoapps/ccx/utils.py index 943c992349aa..39eeb02e69f8 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -463,3 +463,23 @@ def remove_master_course_staff_from_ccx(master_course, ccx_key, display_name, se email_students=send_email, email_params=email_params, ) + + +def list_course_members(course_id): + """ + Returns list of students enroll in course/ccx excluding staff, instructors and coaches on master course. + + Args: + course_id (CourseLocator): the course key + + """ + course_locator = course_id + if getattr(course_id, 'ccx', None): + course_locator = course_id.to_course_locator() + + staff = CourseStaffRole(course_locator).users_with_role() + admins = CourseInstructorRole(course_locator).users_with_role() + coaches = CourseCcxCoachRole(course_locator).users_with_role() + + query_set = CourseEnrollment.objects.filter(course_id=course_id, is_active=True) + return query_set.exclude(user__in=staff).exclude(user__in=admins).exclude(user__in=coaches) diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index 3cee41dc1058..179954136c48 100644 --- a/lms/djangoapps/ccx/views.py +++ b/lms/djangoapps/ccx/views.py @@ -35,7 +35,11 @@ from edxmako.shortcuts import render_to_response from opaque_keys.edx.keys import CourseKey from ccx_keys.locator import CCXLocator -from student.roles import CourseCcxCoachRole +from student.roles import ( + CourseCcxCoachRole, + CourseInstructorRole, + CourseStaffRole +) from student.models import CourseEnrollment from instructor.views.api import _split_input_list @@ -61,6 +65,7 @@ get_ccx_by_ccx_id, get_ccx_creation_dict, get_date, + list_course_members, parse_date, prep_course_for_grading, ) @@ -153,7 +158,7 @@ def dashboard(request, course, ccx=None): context['schedule'] = json.dumps(schedule, indent=4) context['save_url'] = reverse( 'save_ccx', kwargs={'course_id': ccx_locator}) - context['ccx_members'] = CourseEnrollment.objects.filter(course_id=ccx_locator, is_active=True) + context['ccx_members'] = list_course_members(ccx_locator) context['gradebook_url'] = reverse( 'ccx_gradebook', kwargs={'course_id': ccx_locator}) context['grades_csv_url'] = reverse( @@ -499,7 +504,17 @@ def ccx_gradebook(request, course, ccx=None): ccx_key = CCXLocator.from_course_locator(course.id, ccx.id) with ccx_course(ccx_key) as course: prep_course_for_grading(course, request) - student_info, page = get_grade_book_page(request, course, course_key=ccx_key) + + staff = [user.username for user in CourseStaffRole(course.id).users_with_role()] + instructors = [user.username for user in CourseInstructorRole(course.id).users_with_role()] + coaches = [user.username for user in CourseCcxCoachRole(course.id).users_with_role()] + + exclude_users = { + 'staff': staff, + 'instructors': instructors, + 'coaches': coaches + } + student_info, page = get_grade_book_page(request, course, course_key=ccx_key, exclude=exclude_users) return render_to_response('courseware/gradebook.html', { 'page': page, @@ -524,7 +539,11 @@ def ccx_grades_csv(request, course, ccx=None): if not ccx: raise Http404 + staff = [user.username for user in CourseStaffRole(course.id).users_with_role()] + instructors = [user.username for user in CourseInstructorRole(course.id).users_with_role()] + coaches = [user.username for user in CourseCcxCoachRole(course.id).users_with_role()] ccx_key = CCXLocator.from_course_locator(course.id, ccx.id) + with ccx_course(ccx_key) as course: prep_course_for_grading(course, request) @@ -532,6 +551,11 @@ def ccx_grades_csv(request, course, ccx=None): courseenrollment__course_id=ccx_key, courseenrollment__is_active=1 ).order_by('username').select_related("profile") + + enrolled_students = enrolled_students.exclude( + username__in=staff + ).exclude(username__in=instructors).exclude(username__in=coaches) + grades = iterate_grades_for(course, enrolled_students) header = None diff --git a/lms/djangoapps/instructor/views/gradebook_api.py b/lms/djangoapps/instructor/views/gradebook_api.py index 220bfe4cad9a..1cff4afcc7e2 100644 --- a/lms/djangoapps/instructor/views/gradebook_api.py +++ b/lms/djangoapps/instructor/views/gradebook_api.py @@ -64,7 +64,7 @@ def calculate_page_info(offset, total_students): } -def get_grade_book_page(request, course, course_key): +def get_grade_book_page(request, course, course_key, exclude=None): """ Get student records per page along with page information i.e current page, total pages and offset information. @@ -76,6 +76,10 @@ def get_grade_book_page(request, course, course_key): courseenrollment__is_active=1 ).order_by('username').select_related("profile") + if exclude: + for __, list in exclude.items(): + enrolled_students = enrolled_students.exclude(username__in=list) + total_students = enrolled_students.count() page = calculate_page_info(current_offset, total_students) offset = page["offset"] From f2c5b2ee2260f60367939b84ec2704b9a8c74ce6 Mon Sep 17 00:00:00 2001 From: Amir Qayyum Khan Date: Tue, 19 Jul 2016 16:27:23 +0500 Subject: [PATCH 2/2] Added new tests and fix minor issues --- lms/djangoapps/ccx/tests/test_views.py | 57 ++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index c04e96e903d1..9c199325136c 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -66,7 +66,10 @@ CcxTestCase, flatten, ) -from lms.djangoapps.ccx.utils import is_email +from lms.djangoapps.ccx.utils import ( + is_email, + add_master_course_staff_to_ccx +) from lms.djangoapps.ccx.views import get_date from xmodule.modulestore.django import modulestore @@ -1029,6 +1032,7 @@ def setUp(self): # create staff and instructor on course. self.staff = self.make_staff() self.instructor = self.make_instructor() + add_master_course_staff_to_ccx(self.course, self.ccx_key, ccx.display_name) def make_staff(self): """ @@ -1050,6 +1054,15 @@ def make_instructor(self): return instructor + def contains(self, list, filter): + """ + Search value in given list. + """ + for obj in list: + if filter(obj): + return True + return False + @patch('ccx.views.render_to_response', intercept_renderer) @patch('instructor.views.gradebook_api.MAX_STUDENTS_PER_PAGE_GRADE_BOOK', 1) def test_gradebook(self): @@ -1065,9 +1078,30 @@ def test_gradebook(self): # Max number of student per page is one. Patched setting MAX_STUDENTS_PER_PAGE_GRADE_BOOK = 1 self.assertEqual(len(response.mako_context['students']), 1) # pylint: disable=no-member - self.assertNotIn(self.staff.username, response.mako_context['students']) - self.assertNotIn(self.instructor.username, response.mako_context['students']) - self.assertNotIn(self.coach.username, response.mako_context['students']) + self.assertFalse( + self.contains( + response.mako_context['students'], + lambda student: student['username'] == self.staff.username + ) + ) + self.assertFalse( + self.contains( + response.mako_context['students'], + lambda student: student['username'] == self.instructor.username + ) + ) + self.assertFalse( + self.contains( + response.mako_context['students'], + lambda student: student['username'] == self.coach.username + ) + ) + self.assertTrue( + self.contains( + response.mako_context['students'], + lambda student: student['username'] == self.student.username + ) + ) student_info = response.mako_context['students'][0] # pylint: disable=no-member self.assertEqual(student_info['grade_summary']['percent'], 0.5) @@ -1095,9 +1129,18 @@ def test_grades_csv(self): rows = response.content.strip().split('\r') headers = rows[0] - self.assertNotIn(self.staff.username, rows) - self.assertNotIn(self.instructor.username, rows) - self.assertNotIn(self.coach.username, rows) + self.assertFalse( + self.contains(rows, lambda row: self.staff.username in row) + ) + self.assertFalse( + self.contains(rows, lambda row: self.instructor.username in row) + ) + self.assertFalse( + self.contains(rows, lambda row: self.coach.username in row) + ) + self.assertTrue( + self.contains(rows, lambda row: self.student.username in row) + ) # picking first student records data = dict(zip(headers.strip().split(','), rows[1].strip().split(',')))