Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide coaches/staff/instructors from coach's enrollment tab, gradebook, grades csv file #255

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion lms/djangoapps/ccx/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
78 changes: 77 additions & 1 deletion lms/djangoapps/ccx/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1026,6 +1029,40 @@ 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()
add_master_course_staff_to_ccx(self.course, self.ccx_key, ccx.display_name)

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

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):
Expand All @@ -1040,6 +1077,32 @@ 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.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)
self.assertEqual(
Expand All @@ -1066,6 +1129,19 @@ def test_grades_csv(self):
rows = response.content.strip().split('\r')
headers = rows[0]

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(',')))
self.assertNotIn('HW 04', data)
Expand Down
20 changes: 20 additions & 0 deletions lms/djangoapps/ccx/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
30 changes: 27 additions & 3 deletions lms/djangoapps/ccx/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -61,6 +65,7 @@
get_ccx_by_ccx_id,
get_ccx_creation_dict,
get_date,
list_course_members,
parse_date,
prep_course_for_grading,
)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand All @@ -524,14 +539,23 @@ 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)

enrolled_students = User.objects.filter(
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
Expand Down
6 changes: 5 additions & 1 deletion lms/djangoapps/instructor/views/gradebook_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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"]
Expand Down