From aa1333c363b1383cfa53620bdcdd34107e4681fc Mon Sep 17 00:00:00 2001 From: Chris Rossi Date: Tue, 16 Dec 2014 11:01:00 -0500 Subject: [PATCH] MIT: CCX. Hide course blocks not in the CCX from view for coaches and students --- common/lib/xmodule/xmodule/course_module.py | 23 ++- lms/djangoapps/courseware/grades.py | 15 +- .../instructor/offline_gradecalc.py | 1 - lms/djangoapps/pocs/overrides.py | 34 +++++ lms/djangoapps/pocs/tests/test_views.py | 139 ++++++++++++++---- lms/djangoapps/pocs/views.py | 127 +++++++++------- 6 files changed, 247 insertions(+), 92 deletions(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 48a6807510..08844018af 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -11,7 +11,7 @@ from datetime import datetime import dateutil.parser from lazy import lazy - +from xmodule.exceptions import UndefinedContext from xmodule.seq_module import SequenceDescriptor, SequenceModule from xmodule.graders import grader_from_conf from xmodule.tabs import CourseTabList @@ -1213,21 +1213,34 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): """ - + try: + module = getattr(self, '_xmodule', None) + if not module: + module = self + except UndefinedContext: + module = self all_descriptors = [] graded_sections = {} - def yield_descriptor_descendents(module_descriptor): - for child in module_descriptor.get_children(): + def yield_descendents(module): + for child in module.get_children(): yield child - for module_descriptor in yield_descriptor_descendents(child): + for module_descriptor in yield_descendents(child): yield module_descriptor +<<<<<<< HEAD for chapter in self.get_children(): for section in chapter.get_children(): if section.graded: xmoduledescriptors = list(yield_descriptor_descendents(section)) xmoduledescriptors.append(section) +======= + for c in module.get_children(): + for s in c.get_children(): + if s.graded: + xmoduledescriptors = list(yield_descendents(s)) + xmoduledescriptors.append(s) +>>>>>>> Hide course blocks not in the CCX from view for coaches and students # The xmoduledescriptors included here are only the ones that have scores. section_description = { diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 1abfe2c092..199cb7da05 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -246,6 +246,8 @@ def _grade(student, request, course, keep_raw_scores): totaled_scores[section_format] = format_scores + # Grading policy might be overriden by a POC, need to reset it + course.set_grading_policy(course.grading_policy) grade_summary = course.grader.grade(totaled_scores, generate_random_scores=settings.GENERATE_PROFILE_SCORES) # We round the grade here, to make sure that the grade is an whole percentage and @@ -329,6 +331,8 @@ def _progress_summary(student, request, course): # This student must not have access to the course. return None + course_module = getattr(course_module, '_x_module', course_module) + submissions_scores = sub_api.get_scores(course.id.to_deprecated_string(), anonymous_id_for_user(student, course.id)) chapters = [] @@ -479,7 +483,7 @@ def manual_transaction(): transaction.commit() -def iterate_grades_for(course_id, students): +def iterate_grades_for(course_or_id, students): """Given a course_id and an iterable of students (User), yield a tuple of: (student, gradeset, err_msg) for every student enrolled in the course. @@ -497,7 +501,10 @@ def iterate_grades_for(course_id, students): make up the final grade. (For display) - raw_scores: contains scores for every graded module """ - course = courses.get_course_by_id(course_id) + if isinstance(course_or_id, basestring): + course = courses.get_course_by_id(course_or_id) + else: + course = course_or_id # We make a fake request because grading code expects to be able to look at # the request. We have to attach the correct user to the request before @@ -505,7 +512,7 @@ def iterate_grades_for(course_id, students): request = RequestFactory().get('/') for student in students: - with dog_stats_api.timer('lms.grades.iterate_grades_for', tags=[u'action:{}'.format(course_id)]): + with dog_stats_api.timer('lms.grades.iterate_grades_for', tags=[u'action:{}'.format(course.id)]): try: request.user = student # Grading calls problem rendering, which calls masquerading, @@ -522,7 +529,7 @@ def iterate_grades_for(course_id, students): 'Cannot grade student %s (%s) in course %s because of exception: %s', student.username, student.id, - course_id, + course.id, exc.message ) yield student, {}, exc.message diff --git a/lms/djangoapps/instructor/offline_gradecalc.py b/lms/djangoapps/instructor/offline_gradecalc.py index 83bb5f72d8..ad0b727acc 100644 --- a/lms/djangoapps/instructor/offline_gradecalc.py +++ b/lms/djangoapps/instructor/offline_gradecalc.py @@ -81,7 +81,6 @@ def student_grades(student, request, course, keep_raw_scores=False, use_offline= This is the main interface to get grades. It has the same parameters as grades.grade, as well as use_offline. If use_offline is True then this will look for an offline computed gradeset in the DB. ''' - if not use_offline: return grades.grade(student, request, course, keep_raw_scores=keep_raw_scores) diff --git a/lms/djangoapps/pocs/overrides.py b/lms/djangoapps/pocs/overrides.py index 5af4d799c9..0c9b61f456 100644 --- a/lms/djangoapps/pocs/overrides.py +++ b/lms/djangoapps/pocs/overrides.py @@ -3,6 +3,9 @@ API related to providing field overrides for individual students. This is used by the individual due dates feature. """ import json +import threading + +from contextlib import contextmanager from courseware.field_overrides import FieldOverrideProvider @@ -22,10 +25,41 @@ class PersonalOnlineCoursesOverrideProvider(FieldOverrideProvider): return default +class _PocContext(threading.local): + """ + A threading local used to implement the `with_poc` context manager, that + keeps track of the POC currently set as the context. + """ + poc = None + + +_POC_CONTEXT = _PocContext() + + +@contextmanager +def poc_context(poc): + """ + A context manager which can be used to explicitly set the POC that is in + play for field overrides. This mechanism overrides the standard mechanism + of looking in the user's session to see if they are enrolled in a POC and + viewing that POC. + """ + prev = _POC_CONTEXT.poc + _POC_CONTEXT.poc = poc + yield + _POC_CONTEXT.poc = prev + + def get_current_poc(user): """ TODO Needs to look in user's session """ + # If poc context is explicitly set, that takes precedence over the user's + # session. + poc = _POC_CONTEXT.poc + if poc: + return poc + # Temporary implementation. Final implementation will need to look in # user's session so user can switch between (potentially multiple) POC and # MOOC views. See courseware.courses.get_request_for_thread for idea to diff --git a/lms/djangoapps/pocs/tests/test_views.py b/lms/djangoapps/pocs/tests/test_views.py index 50045e7967..edf4c6c040 100644 --- a/lms/djangoapps/pocs/tests/test_views.py +++ b/lms/djangoapps/pocs/tests/test_views.py @@ -5,9 +5,11 @@ import pytz from mock import patch from capa.tests.response_xml_factory import StringResponseXMLFactory +from courseware.field_overrides import OverrideFieldData from courseware.tests.factories import StudentModuleFactory from courseware.tests.helpers import LoginEnrollmentTestCase from django.core.urlresolvers import reverse +from django.test.utils import override_settings from edxmako.shortcuts import render_to_response from student.roles import CoursePocCoachRole from student.tests.factories import ( @@ -26,7 +28,7 @@ from ..models import ( PocMembership, PocFutureMembership, ) -from ..overrides import get_override_for_poc +from ..overrides import get_override_for_poc, override_field_for_poc from .factories import ( PocFactory, PocMembershipFactory, @@ -369,9 +371,8 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): ) -USER_COUNT = 2 - - +@override_settings(FIELD_OVERRIDE_PROVIDERS=( + 'pocs.overrides.PersonalOnlineCoursesOverrideProvider',)) class TestPocGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): """ Tests for Personal Online Courses views. @@ -391,39 +392,65 @@ class TestPocGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): 2010, 5, 12, 2, 42, tzinfo=pytz.UTC) chapter = ItemFactory.create( start=start, parent=course, category='sequential') - section = ItemFactory.create( - parent=chapter, - category="sequential", - metadata={'graded': True, 'format': 'Homework'} - ) + sections = [ + ItemFactory.create( + parent=chapter, + category="sequential", + metadata={'graded': True, 'format': 'Homework'}) + for _ in xrange(4)] role = CoursePocCoachRole(self.course.id) role.add_users(coach) self.poc = poc = PocFactory(course_id=self.course.id, coach=self.coach) - self.users = [UserFactory.create() for _ in xrange(USER_COUNT)] - for user in self.users: - CourseEnrollmentFactory.create(user=user, course_id=self.course.id) - PocMembershipFactory(poc=poc, student=user, active=True) + self.student = student = UserFactory.create() + CourseEnrollmentFactory.create(user=student, course_id=self.course.id) + PocMembershipFactory(poc=poc, student=student, active=True) - for i in xrange(USER_COUNT - 1): - category = "problem" - item = ItemFactory.create( - parent_location=section.location, - category=category, - data=StringResponseXMLFactory().build_xml(answer='foo'), - metadata={'rerandomize': 'always'} - ) + for i, section in enumerate(sections): + for j in xrange(4): + item = ItemFactory.create( + parent=section, + category="problem", + data=StringResponseXMLFactory().build_xml(answer='foo'), + metadata={'rerandomize': 'always'} + ) - for j, user in enumerate(self.users): StudentModuleFactory.create( grade=1 if i < j else 0, max_grade=1, - student=user, + student=student, course_id=self.course.id, module_state_key=item.location ) + # Apparently the test harness doesn't use LmsFieldStorage, and I'm not + # sure if there's a way to poke the test harness to do so. So, we'll + # just inject the override field storage in this brute force manner. + OverrideFieldData.provider_classes = None + for block in iter_blocks(course): + block._field_data = OverrideFieldData.wrap( # pylint: disable=protected-access + coach, block._field_data) # pylint: disable=protected-access + block._field_data_cache = {} + visible_children(block) + + patch_context = patch('pocs.views.get_course_by_id') + get_course = patch_context.start() + get_course.return_value = course + self.addCleanup(patch_context.stop) + + override_field_for_poc(poc, course, 'grading_policy', { + 'GRADER': [ + {'drop_count': 0, + 'min_count': 2, + 'short_label': 'HW', + 'type': 'Homework', + 'weight': 1} + ], + 'GRADE_CUTOFFS': {'Pass': 0.75}, + }) + override_field_for_poc( + poc, sections[-1], 'visible_to_staff_only', True) @patch('pocs.views.render_to_response', intercept_renderer) def test_gradebook(self): @@ -433,13 +460,13 @@ class TestPocGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): ) response = self.client.get(url) self.assertEqual(response.status_code, 200) - student_info = response.mako_context['students'] - self.assertEqual(len(student_info), USER_COUNT) - self.assertEqual(student_info[0]['grade_summary']['percent'], 0.0) - self.assertEqual(student_info[1]['grade_summary']['percent'], 0.02) + student_info = response.mako_context['students'][0] + self.assertEqual(student_info['grade_summary']['percent'], 0.5) self.assertEqual( - student_info[1]['grade_summary']['grade_breakdown'][0]['percent'], - 0.015) + student_info['grade_summary']['grade_breakdown'][0]['percent'], + 0.5) + self.assertEqual( + len(student_info['grade_summary']['section_breakdown']), 4) def test_grades_csv(self): url = reverse( @@ -448,8 +475,35 @@ class TestPocGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): ) response = self.client.get(url) self.assertEqual(response.status_code, 200) - self.assertEqual( - len(response.content.strip().split('\n')), USER_COUNT + 1) + headers, row = ( + row.strip().split(',') for row in + response.content.strip().split('\n') + ) + data = dict(zip(headers, row)) + self.assertEqual(data['HW 01'], '0.75') + self.assertEqual(data['HW 02'], '0.5') + self.assertEqual(data['HW 03'], '0.25') + self.assertEqual(data['HW Avg'], '0.5') + self.assertTrue('HW 04' not in data) + + @patch('courseware.views.render_to_response', intercept_renderer) + def test_student_progress(self): + patch_context = patch('courseware.views.get_course_with_access') + get_course = patch_context.start() + get_course.return_value = self.course + self.addCleanup(patch_context.stop) + + self.client.login(username=self.student.username, password="test") + url = reverse( + 'progress', + kwargs={'course_id': self.course.id.to_deprecated_string()} + ) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + grades = response.mako_context['grade_summary'] + self.assertEqual(grades['percent'], 0.5) + self.assertEqual( grades['grade_breakdown'][0]['percent'], 0.5) + self.assertEqual(len(grades['section_breakdown']), 4) def flatten(seq): @@ -457,3 +511,26 @@ def flatten(seq): For [[1, 2], [3, 4]] returns [1, 2, 3, 4]. Does not recurse. """ return [x for sub in seq for x in sub] + + +def iter_blocks(course): + """ + Returns an iterator over all of the blocks in a course. + """ + def visit(block): + yield block + for child in block.get_children(): + for descendant in visit(child): # wish they'd backport yield from + yield descendant + return visit(course) + +def visible_children(block): + block_get_children = block.get_children + def get_children(): + def iter_children(): + for child in block_get_children(): + child._field_data_cache = {} + if not child.visible_to_staff_only: + yield child + return list(iter_children()) + block.get_children = get_children diff --git a/lms/djangoapps/pocs/views.py b/lms/djangoapps/pocs/views.py index 43127b7b87..86317ca6ad 100644 --- a/lms/djangoapps/pocs/views.py +++ b/lms/djangoapps/pocs/views.py @@ -20,6 +20,8 @@ from django.contrib.auth.models import User from courseware.courses import get_course_by_id from courseware.field_overrides import disable_overrides from courseware.grades import iterate_grades_for +from courseware.model_data import FieldDataCache +from courseware.module_render import get_module_for_descriptor from edxmako.shortcuts import render_to_response from opaque_keys.edx.locations import SlashSeparatedCourseKey from student.roles import CoursePocCoachRole @@ -33,6 +35,7 @@ from .overrides import ( clear_override_for_poc, get_override_for_poc, override_field_for_poc, + poc_context, ) from .utils import enroll_email, unenroll_email @@ -290,68 +293,90 @@ def poc_gradebook(request, course): """ Show the gradebook for this POC. """ + # Need course module for overrides to function properly + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( + course.id, request.user, course, depth=2) + course = get_module_for_descriptor( + request.user, request, course, field_data_cache, course.id) + poc = get_poc_for_coach(course, request.user) - enrolled_students = User.objects.filter( - pocmembership__poc=poc, - pocmembership__active=1 - ).order_by('username').select_related("profile") + with poc_context(poc): + course._field_data_cache = {} + course.set_grading_policy(course.grading_policy) # this is so awful + enrolled_students = User.objects.filter( + pocmembership__poc=poc, + pocmembership__active=1 + ).order_by('username').select_related("profile") - student_info = [ - { - 'username': student.username, - 'id': student.id, - 'email': student.email, - 'grade_summary': student_grades(student, request, course), - 'realname': student.profile.name, - } - for student in enrolled_students - ] + student_info = [ + { + 'username': student.username, + 'id': student.id, + 'email': student.email, + 'grade_summary': student_grades(student, request, course), + 'realname': student.profile.name, + } + for student in enrolled_students + ] - return render_to_response('courseware/gradebook.html', { - 'students': student_info, - 'course': course, - 'course_id': course.id, - 'staff_access': request.user.is_staff, - 'ordered_grades': sorted( - course.grade_cutoffs.items(), key=lambda i: i[1], reverse=True), - }) + return render_to_response('courseware/gradebook.html', { + 'students': student_info, + 'course': course, + 'course_id': course.id, + 'staff_access': request.user.is_staff, + 'ordered_grades': sorted( + course.grade_cutoffs.items(), key=lambda i: i[1], reverse=True), + }) @cache_control(no_cache=True, no_store=True, must_revalidate=True) @coach_dashboard def poc_grades_csv(request, course): + """ + Download grades as CSV. + """ + # Need course module for overrides to function properly + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( + course.id, request.user, course, depth=2) + course = get_module_for_descriptor( + request.user, request, course, field_data_cache, course.id) + poc = get_poc_for_coach(course, request.user) - enrolled_students = User.objects.filter( - pocmembership__poc=poc, - pocmembership__active=1 - ).order_by('username').select_related("profile") - grades = iterate_grades_for(course.id, enrolled_students) + with poc_context(poc): + course._field_data_cache = {} + course.set_grading_policy(course.grading_policy) # this is so awful + enrolled_students = User.objects.filter( + pocmembership__poc=poc, + pocmembership__active=1 + ).order_by('username').select_related("profile") + grades = iterate_grades_for(course, enrolled_students) - header = None - rows = [] - for student, gradeset, err_msg in grades: - if gradeset: - # We were able to successfully grade this student for this course. - if not header: - # Encode the header row in utf-8 encoding in case there are - # unicode characters - header = [section['label'].encode('utf-8') - for section in gradeset[u'section_breakdown']] - rows.append(["id", "email", "username", "grade"] + header) + header = None + rows = [] + for student, gradeset, err_msg in grades: + if gradeset: + # We were able to successfully grade this student for this + # course. + if not header: + # Encode the header row in utf-8 encoding in case there are + # unicode characters + header = [section['label'].encode('utf-8') + for section in gradeset[u'section_breakdown']] + rows.append(["id", "email", "username", "grade"] + header) - percents = { - section['label']: section.get('percent', 0.0) - for section in gradeset[u'section_breakdown'] - if 'label' in section - } + percents = { + section['label']: section.get('percent', 0.0) + for section in gradeset[u'section_breakdown'] + if 'label' in section + } - row_percents = [percents.get(label, 0.0) for label in header] - rows.append([student.id, student.email, student.username, - gradeset['percent']] + row_percents) + row_percents = [percents.get(label, 0.0) for label in header] + rows.append([student.id, student.email, student.username, + gradeset['percent']] + row_percents) - buffer = StringIO() - writer = csv.writer(buffer) - for row in rows: - writer.writerow(row) + buffer = StringIO() + writer = csv.writer(buffer) + for row in rows: + writer.writerow(row) - return HttpResponse(buffer.getvalue(), content_type='text/csv') + return HttpResponse(buffer.getvalue(), content_type='text/plain')