From 6ca8a702aec74e1bf6bdf76f54749812d346f5dc Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Tue, 2 May 2017 23:11:18 +0930 Subject: [PATCH] Mask grades on progress page according to "Show Correctness" setting. --- common/lib/xmodule/xmodule/capa_base.py | 27 +- .../xmodule/xmodule/capa_base_constants.py | 9 - common/lib/xmodule/xmodule/graders.py | 38 ++- .../lib/xmodule/xmodule/tests/test_graders.py | 93 +++++- .../blocks/transformers/__init__.py | 1 + .../blocks/transformers/blocks_api.py | 2 +- lms/djangoapps/course_api/blocks/views.py | 3 + lms/djangoapps/courseware/tests/test_views.py | 299 +++++++++++++++++- lms/djangoapps/grades/new/subsection_grade.py | 12 +- lms/djangoapps/grades/transformer.py | 14 +- lms/templates/courseware/progress.html | 30 +- 11 files changed, 481 insertions(+), 47 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index 847eed20fc..94d71e68c6 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -25,8 +25,9 @@ from capa.responsetypes import StudentInputError, ResponseError, LoncapaProblemE from capa.util import convert_files_to_filenames, get_inner_html_from_xpath from xblock.fields import Boolean, Dict, Float, Integer, Scope, String, XMLString from xblock.scorable import ScorableXBlockMixin, Score -from xmodule.capa_base_constants import RANDOMIZATION, SHOWANSWER, SHOW_CORRECTNESS +from xmodule.capa_base_constants import RANDOMIZATION, SHOWANSWER from xmodule.exceptions import NotFoundError +from xmodule.graders import ShowCorrectness from .fields import Date, Timedelta from .progress import Progress @@ -120,11 +121,11 @@ class CapaFields(object): help=_("Defines when to show whether a learner's answer to the problem is correct. " "Configured on the subsection."), scope=Scope.settings, - default=SHOW_CORRECTNESS.ALWAYS, + default=ShowCorrectness.ALWAYS, values=[ - {"display_name": _("Always"), "value": SHOW_CORRECTNESS.ALWAYS}, - {"display_name": _("Never"), "value": SHOW_CORRECTNESS.NEVER}, - {"display_name": _("Past Due"), "value": SHOW_CORRECTNESS.PAST_DUE}, + {"display_name": _("Always"), "value": ShowCorrectness.ALWAYS}, + {"display_name": _("Never"), "value": ShowCorrectness.NEVER}, + {"display_name": _("Past Due"), "value": ShowCorrectness.PAST_DUE}, ], ) showanswer = String( @@ -921,17 +922,11 @@ class CapaMixin(ScorableXBlockMixin, CapaFields): Limits access to the correct/incorrect flags, messages, and problem score. """ - if self.show_correctness == SHOW_CORRECTNESS.NEVER: - return False - elif self.runtime.user_is_staff: - # This is after the 'never' check because admins can see correctness - # unless the problem explicitly prevents it - return True - elif self.show_correctness == SHOW_CORRECTNESS.PAST_DUE: - return self.is_past_due() - - # else: self.show_correctness == SHOW_CORRECTNESS.ALWAYS - return True + return ShowCorrectness.correctness_available( + show_correctness=self.show_correctness, + due_date=self.close_date, + has_staff_access=self.runtime.user_is_staff, + ) def update_score(self, data): """ diff --git a/common/lib/xmodule/xmodule/capa_base_constants.py b/common/lib/xmodule/xmodule/capa_base_constants.py index 2864825906..7739be238e 100644 --- a/common/lib/xmodule/xmodule/capa_base_constants.py +++ b/common/lib/xmodule/xmodule/capa_base_constants.py @@ -4,15 +4,6 @@ Constants for capa_base problems """ -class SHOW_CORRECTNESS(object): # pylint: disable=invalid-name - """ - Constants for when to show correctness - """ - ALWAYS = "always" - PAST_DUE = "past_due" - NEVER = "never" - - class SHOWANSWER(object): """ Constants for when to show answer diff --git a/common/lib/xmodule/xmodule/graders.py b/common/lib/xmodule/xmodule/graders.py index 0bd7ece904..2f9d191083 100644 --- a/common/lib/xmodule/xmodule/graders.py +++ b/common/lib/xmodule/xmodule/graders.py @@ -10,9 +10,10 @@ import logging import random import sys from collections import OrderedDict -from datetime import datetime # Used by pycontracts. pylint: disable=unused-import +from datetime import datetime from contracts import contract +from pytz import UTC log = logging.getLogger("edx.courseware") @@ -462,3 +463,38 @@ def _min_or_none(itr): return min(itr) except ValueError: return None + + +class ShowCorrectness(object): + """ + Helper class for determining whether correctness is currently hidden for a block. + + When correctness is hidden, this limits the user's access to the correct/incorrect flags, messages, problem scores, + and aggregate subsection and course grades. + """ + + """ + Constants used to indicate when to show correctness + """ + ALWAYS = "always" + PAST_DUE = "past_due" + NEVER = "never" + + @classmethod + def correctness_available(cls, show_correctness='', due_date=None, has_staff_access=False): + """ + Returns whether correctness is available now, for the given attributes. + """ + if show_correctness == cls.NEVER: + return False + elif has_staff_access: + # This is after the 'never' check because course staff can see correctness + # unless the sequence/problem explicitly prevents it + return True + elif show_correctness == cls.PAST_DUE: + # Is it now past the due date? + return (due_date is None or + due_date < datetime.now(UTC)) + + # else: show_correctness == cls.ALWAYS + return True diff --git a/common/lib/xmodule/xmodule/tests/test_graders.py b/common/lib/xmodule/xmodule/tests/test_graders.py index 03c334ff2e..d4eed74d9d 100644 --- a/common/lib/xmodule/xmodule/tests/test_graders.py +++ b/common/lib/xmodule/xmodule/tests/test_graders.py @@ -2,13 +2,15 @@ Grading tests """ -from datetime import datetime +import unittest +from datetime import datetime, timedelta import ddt -import unittest - +from pytz import UTC from xmodule import graders -from xmodule.graders import ProblemScore, AggregatedScore, aggregate_scores +from xmodule.graders import ( + AggregatedScore, ProblemScore, ShowCorrectness, aggregate_scores +) class GradesheetTest(unittest.TestCase): @@ -315,3 +317,86 @@ class GraderTest(unittest.TestCase): with self.assertRaises(ValueError) as error: graders.grader_from_conf([invalid_conf]) self.assertIn(expected_error_message, error.exception.message) + + +@ddt.ddt +class ShowCorrectnessTest(unittest.TestCase): + """ + Tests the correctness_available method + """ + def setUp(self): + super(ShowCorrectnessTest, self).setUp() + + now = datetime.now(UTC) + day_delta = timedelta(days=1) + self.yesterday = now - day_delta + self.today = now + self.tomorrow = now + day_delta + + def test_show_correctness_default(self): + """ + Test that correctness is visible by default. + """ + self.assertTrue(ShowCorrectness.correctness_available()) + + @ddt.data( + (ShowCorrectness.ALWAYS, True), + (ShowCorrectness.ALWAYS, False), + # Any non-constant values behave like "always" + ('', True), + ('', False), + ('other-value', True), + ('other-value', False), + ) + @ddt.unpack + def test_show_correctness_always(self, show_correctness, has_staff_access): + """ + Test that correctness is visible when show_correctness is turned on. + """ + self.assertTrue(ShowCorrectness.correctness_available( + show_correctness=show_correctness, + has_staff_access=has_staff_access + )) + + @ddt.data(True, False) + def test_show_correctness_never(self, has_staff_access): + """ + Test that show_correctness="never" hides correctness from learners and course staff. + """ + self.assertFalse(ShowCorrectness.correctness_available( + show_correctness=ShowCorrectness.NEVER, + has_staff_access=has_staff_access + )) + + @ddt.data( + # Correctness not visible to learners if due date in the future + ('tomorrow', False, False), + # Correctness is visible to learners if due date in the past + ('yesterday', False, True), + # Correctness is visible to learners if due date in the past (just) + ('today', False, True), + # Correctness is visible to learners if there is no due date + (None, False, True), + # Correctness is visible to staff if due date in the future + ('tomorrow', True, True), + # Correctness is visible to staff if due date in the past + ('yesterday', True, True), + # Correctness is visible to staff if there is no due date + (None, True, True), + ) + @ddt.unpack + def test_show_correctness_past_due(self, due_date_str, has_staff_access, expected_result): + """ + Test show_correctness="past_due" to ensure: + * correctness is always visible to course staff + * correctness is always visible to everyone if there is no due date + * correctness is visible to learners after the due date, when there is a due date. + """ + if due_date_str is None: + due_date = None + else: + due_date = getattr(self, due_date_str) + self.assertEquals( + ShowCorrectness.correctness_available(ShowCorrectness.PAST_DUE, due_date, has_staff_access), + expected_result + ) diff --git a/lms/djangoapps/course_api/blocks/transformers/__init__.py b/lms/djangoapps/course_api/blocks/transformers/__init__.py index 36ee5e6bc1..aa8f2bdb95 100644 --- a/lms/djangoapps/course_api/blocks/transformers/__init__.py +++ b/lms/djangoapps/course_api/blocks/transformers/__init__.py @@ -40,6 +40,7 @@ SUPPORTED_FIELDS = [ SupportedFieldType('graded'), SupportedFieldType('format'), SupportedFieldType('due'), + SupportedFieldType('show_correctness'), # 'student_view_data' SupportedFieldType(StudentViewTransformer.STUDENT_VIEW_DATA, StudentViewTransformer), # 'student_view_multi_device' diff --git a/lms/djangoapps/course_api/blocks/transformers/blocks_api.py b/lms/djangoapps/course_api/blocks/transformers/blocks_api.py index ce988d39e3..c4356b200d 100644 --- a/lms/djangoapps/course_api/blocks/transformers/blocks_api.py +++ b/lms/djangoapps/course_api/blocks/transformers/blocks_api.py @@ -44,7 +44,7 @@ class BlocksAPITransformer(BlockStructureTransformer): transform method. """ # collect basic xblock fields - block_structure.request_xblock_fields('graded', 'format', 'display_name', 'category', 'due') + block_structure.request_xblock_fields('graded', 'format', 'display_name', 'category', 'due', 'show_correctness') # collect data from containing transformers StudentViewTransformer.collect(block_structure) diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index 60cb122bba..7caefff9d0 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -172,6 +172,9 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): * due: The due date of the block. Returned only if "due" is included in the "requested_fields" parameter. + + * show_correctness: Whether to show scores/correctness to learners for the current sequence or problem. + Returned only if "show_correctness" is included in the "requested_fields" parameter. """ def list(self, request, usage_key_string): # pylint: disable=arguments-differ diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 4457ddaac9..24c35a79a0 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -29,6 +29,9 @@ from xblock.core import XBlock from xblock.fields import String, Scope from xblock.fragment import Fragment +from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory +from courseware.model_data import FieldDataCache +from courseware.module_render import get_module import courseware.views.views as views import shoppingcart from certificates import api as certs_api @@ -56,6 +59,7 @@ from openedx.core.djangoapps.crawlers.models import CrawlersConfig from openedx.core.djangoapps.credit.api import set_credit_requirements from openedx.core.djangoapps.credit.models import CreditCourse, CreditProvider from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration +from openedx.core.djangolib.testing.utils import get_mock_request from openedx.core.lib.gating import api as gating_api from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseTestConsentRequired from student.models import CourseEnrollment @@ -63,6 +67,7 @@ from student.tests.factories import AdminFactory, UserFactory, CourseEnrollmentF from util.tests.test_date_utils import fake_ugettext, fake_pgettext from util.url import reload_django_url_config from util.views import ensure_valid_course_key +from xmodule.graders import ShowCorrectness from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_MODULESTORE @@ -1165,29 +1170,32 @@ class StartDateTests(ModuleStoreTestCase): # pylint: disable=protected-access, no-member @attr(shard=1) @override_settings(ENABLE_ENTERPRISE_INTEGRATION=False) -@ddt.ddt -class ProgressPageTests(ModuleStoreTestCase): +class ProgressPageBaseTests(ModuleStoreTestCase): """ - Tests that verify that the progress page works correctly. + Base class for progress page tests. """ ENABLED_CACHES = ['default', 'mongo_modulestore_inheritance', 'loc_cache'] ENABLED_SIGNALS = ['course_published'] def setUp(self): - super(ProgressPageTests, self).setUp() + super(ProgressPageBaseTests, self).setUp() self.user = UserFactory.create() self.assertTrue(self.client.login(username=self.user.username, password='test')) self.setup_course() - def setup_course(self, **options): + def create_course(self, **options): """Create the test course.""" self.course = CourseFactory.create( start=datetime(2013, 9, 16, 7, 17, 28), grade_cutoffs={u'çü†øƒƒ': 0.75, 'Pass': 0.5}, **options ) + + def setup_course(self, **course_options): + """Create the test course and content, and enroll the user.""" + self.create_course(**course_options) with self.store.bulk_operations(self.course.id): self.chapter = ItemFactory.create(category='chapter', parent_location=self.course.location) self.section = ItemFactory.create(category='sequential', parent_location=self.chapter.location) @@ -1197,7 +1205,7 @@ class ProgressPageTests(ModuleStoreTestCase): def _get_progress_page(self, expected_status_code=200): """ - Gets the progress page for the user in the course. + Gets the progress page for the currently logged-in user. """ resp = self.client.get( reverse('progress', args=[unicode(self.course.id)]) @@ -1215,6 +1223,14 @@ class ProgressPageTests(ModuleStoreTestCase): self.assertEqual(resp.status_code, expected_status_code) return resp + +# pylint: disable=protected-access, no-member +@ddt.ddt +class ProgressPageTests(ProgressPageBaseTests): + """ + Tests that verify that the progress page works correctly. + """ + @ddt.data('">', '', '') def test_progress_page_xss_prevent(self, malicious_code): """ @@ -1649,6 +1665,277 @@ class ProgressPageTests(ModuleStoreTestCase): } +# pylint: disable=protected-access, no-member +@ddt.ddt +class ProgressPageShowCorrectnessTests(ProgressPageBaseTests): + """ + Tests that verify that the progress page works correctly when displaying subsections where correctness is hidden. + """ + # Constants used in the test data + NOW = datetime.now(UTC) + DAY_DELTA = timedelta(days=1) + YESTERDAY = NOW - DAY_DELTA + TODAY = NOW + TOMORROW = NOW + DAY_DELTA + GRADER_TYPE = 'Homework' + + def setUp(self): + super(ProgressPageShowCorrectnessTests, self).setUp() + self.staff_user = UserFactory.create(is_staff=True) + + def setup_course(self, show_correctness='', due_date=None, graded=False, **course_options): + """ + Set up course with a subsection with the given show_correctness, due_date, and graded settings. + """ + # Use a simple grading policy + course_options['grading_policy'] = { + "GRADER": [{ + "type": self.GRADER_TYPE, + "min_count": 2, + "drop_count": 0, + "short_label": "HW", + "weight": 1.0 + }], + "GRADE_CUTOFFS": { + 'A': .9, + 'B': .33 + } + } + self.create_course(**course_options) + + metadata = dict( + show_correctness=show_correctness, + ) + if due_date is not None: + metadata['due'] = due_date + if graded: + metadata['graded'] = True + metadata['format'] = self.GRADER_TYPE + + with self.store.bulk_operations(self.course.id): + self.chapter = ItemFactory.create(category='chapter', parent_location=self.course.location, + display_name="Section 1") + self.section = ItemFactory.create(category='sequential', parent_location=self.chapter.location, + display_name="Subsection 1", metadata=metadata) + self.vertical = ItemFactory.create(category='vertical', parent_location=self.section.location) + + CourseEnrollmentFactory(user=self.user, course_id=self.course.id, mode=CourseMode.HONOR) + + def add_problem(self): + """ + Add a problem to the subsection + """ + problem_xml = MultipleChoiceResponseXMLFactory().build_xml( + question_text='The correct answer is Choice 1', + choices=[True, False], + choice_names=['choice_0', 'choice_1'] + ) + self.problem = ItemFactory.create(category='problem', parent_location=self.vertical.location, + data=problem_xml, display_name='Problem 1') + # Re-fetch the course from the database + self.course = self.store.get_course(self.course.id) + + def answer_problem(self, value=1, max_value=1): + """ + Submit the given score to the problem on behalf of the user + """ + # Get the module for the problem, as viewed by the user + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( + self.course.id, + self.user, + self.course, + depth=2 + ) + # pylint: disable=protected-access + module = get_module( + self.user, + get_mock_request(self.user), + self.problem.scope_ids.usage_id, + field_data_cache, + )._xmodule + + # Submit the given score/max_score to the problem xmodule + grade_dict = {'value': value, 'max_value': max_value, 'user_id': self.user.id} + module.system.publish(self.problem, 'grade', grade_dict) + + def assert_progress_page_show_grades(self, response, show_correctness, due_date, graded, + show_grades, score, max_score, avg): + """ + Ensures that grades and scores are shown or not shown on the progress page as required. + """ + + expected_score = "
{score}/{max_score}
".format(score=score, max_score=max_score) + percent = score / float(max_score) + + if show_grades: + # If grades are shown, we should be able to see the current problem scores. + self.assertIn(expected_score, response.content) + + if graded: + expected_summary_text = "Problem Scores:" + else: + expected_summary_text = "Practice Scores:" + + else: + # If grades are hidden, we should not be able to see the current problem scores. + self.assertNotIn(expected_score, response.content) + + if graded: + expected_summary_text = "Problem scores are hidden" + else: + expected_summary_text = "Practice scores are hidden" + + if show_correctness == ShowCorrectness.PAST_DUE and due_date: + expected_summary_text += ' until the due date.' + else: + expected_summary_text += '.' + + # Ensure that expected text is present + self.assertIn(expected_summary_text, response.content) + + @ddt.data( + ('', None, False), + ('', None, True), + (ShowCorrectness.ALWAYS, None, False), + (ShowCorrectness.ALWAYS, None, True), + (ShowCorrectness.ALWAYS, YESTERDAY, False), + (ShowCorrectness.ALWAYS, YESTERDAY, True), + (ShowCorrectness.ALWAYS, TODAY, False), + (ShowCorrectness.ALWAYS, TODAY, True), + (ShowCorrectness.ALWAYS, TOMORROW, False), + (ShowCorrectness.ALWAYS, TOMORROW, True), + (ShowCorrectness.NEVER, None, False), + (ShowCorrectness.NEVER, None, True), + (ShowCorrectness.NEVER, YESTERDAY, False), + (ShowCorrectness.NEVER, YESTERDAY, True), + (ShowCorrectness.NEVER, TODAY, False), + (ShowCorrectness.NEVER, TODAY, True), + (ShowCorrectness.NEVER, TOMORROW, False), + (ShowCorrectness.NEVER, TOMORROW, True), + (ShowCorrectness.PAST_DUE, None, False), + (ShowCorrectness.PAST_DUE, None, True), + (ShowCorrectness.PAST_DUE, YESTERDAY, False), + (ShowCorrectness.PAST_DUE, YESTERDAY, True), + (ShowCorrectness.PAST_DUE, TODAY, False), + (ShowCorrectness.PAST_DUE, TODAY, True), + (ShowCorrectness.PAST_DUE, TOMORROW, False), + (ShowCorrectness.PAST_DUE, TOMORROW, True), + ) + @ddt.unpack + def test_progress_page_no_problem_scores(self, show_correctness, due_date, graded): + """ + Test that "no problem scores are present" for a course with no problems, + regardless of the various show correctness settings. + """ + self.setup_course(show_correctness=show_correctness, due_date=due_date, graded=graded) + resp = self._get_progress_page() + + # Test that no problem scores are present + self.assertIn('No problem scores in this section', resp.content) + + @ddt.data( + ('', None, False, True), + ('', None, True, True), + (ShowCorrectness.ALWAYS, None, False, True), + (ShowCorrectness.ALWAYS, None, True, True), + (ShowCorrectness.ALWAYS, YESTERDAY, False, True), + (ShowCorrectness.ALWAYS, YESTERDAY, True, True), + (ShowCorrectness.ALWAYS, TODAY, False, True), + (ShowCorrectness.ALWAYS, TODAY, True, True), + (ShowCorrectness.ALWAYS, TOMORROW, False, True), + (ShowCorrectness.ALWAYS, TOMORROW, True, True), + (ShowCorrectness.NEVER, None, False, False), + (ShowCorrectness.NEVER, None, True, False), + (ShowCorrectness.NEVER, YESTERDAY, False, False), + (ShowCorrectness.NEVER, YESTERDAY, True, False), + (ShowCorrectness.NEVER, TODAY, False, False), + (ShowCorrectness.NEVER, TODAY, True, False), + (ShowCorrectness.NEVER, TOMORROW, False, False), + (ShowCorrectness.NEVER, TOMORROW, True, False), + (ShowCorrectness.PAST_DUE, None, False, True), + (ShowCorrectness.PAST_DUE, None, True, True), + (ShowCorrectness.PAST_DUE, YESTERDAY, False, True), + (ShowCorrectness.PAST_DUE, YESTERDAY, True, True), + (ShowCorrectness.PAST_DUE, TODAY, False, True), + (ShowCorrectness.PAST_DUE, TODAY, True, True), + (ShowCorrectness.PAST_DUE, TOMORROW, False, False), + (ShowCorrectness.PAST_DUE, TOMORROW, True, False), + ) + @ddt.unpack + def test_progress_page_hide_scores_from_learner(self, show_correctness, due_date, graded, show_grades): + """ + Test that problem scores are hidden on progress page when correctness is not available to the learner, and that + they are visible when it is. + """ + self.setup_course(show_correctness=show_correctness, due_date=due_date, graded=graded) + self.add_problem() + + self.client.login(username=self.user.username, password='test') + resp = self._get_progress_page() + + # Ensure that expected text is present + self.assert_progress_page_show_grades(resp, show_correctness, due_date, graded, show_grades, 0, 1, 0) + + # Submit answers to the problem, and re-fetch the progress page + self.answer_problem() + + resp = self._get_progress_page() + + # Test that the expected text is still present. + self.assert_progress_page_show_grades(resp, show_correctness, due_date, graded, show_grades, 1, 1, .5) + + @ddt.data( + ('', None, False, True), + ('', None, True, True), + (ShowCorrectness.ALWAYS, None, False, True), + (ShowCorrectness.ALWAYS, None, True, True), + (ShowCorrectness.ALWAYS, YESTERDAY, False, True), + (ShowCorrectness.ALWAYS, YESTERDAY, True, True), + (ShowCorrectness.ALWAYS, TODAY, False, True), + (ShowCorrectness.ALWAYS, TODAY, True, True), + (ShowCorrectness.ALWAYS, TOMORROW, False, True), + (ShowCorrectness.ALWAYS, TOMORROW, True, True), + (ShowCorrectness.NEVER, None, False, False), + (ShowCorrectness.NEVER, None, True, False), + (ShowCorrectness.NEVER, YESTERDAY, False, False), + (ShowCorrectness.NEVER, YESTERDAY, True, False), + (ShowCorrectness.NEVER, TODAY, False, False), + (ShowCorrectness.NEVER, TODAY, True, False), + (ShowCorrectness.NEVER, TOMORROW, False, False), + (ShowCorrectness.NEVER, TOMORROW, True, False), + (ShowCorrectness.PAST_DUE, None, False, True), + (ShowCorrectness.PAST_DUE, None, True, True), + (ShowCorrectness.PAST_DUE, YESTERDAY, False, True), + (ShowCorrectness.PAST_DUE, YESTERDAY, True, True), + (ShowCorrectness.PAST_DUE, TODAY, False, True), + (ShowCorrectness.PAST_DUE, TODAY, True, True), + (ShowCorrectness.PAST_DUE, TOMORROW, False, True), + (ShowCorrectness.PAST_DUE, TOMORROW, True, True), + ) + @ddt.unpack + def test_progress_page_hide_scores_from_staff(self, show_correctness, due_date, graded, show_grades): + """ + Test that problem scores are hidden from staff viewing a learner's progress page only if show_correctness=never. + """ + self.setup_course(show_correctness=show_correctness, due_date=due_date, graded=graded) + self.add_problem() + + # Login as a course staff user to view the student progress page. + self.client.login(username=self.staff_user.username, password='test') + + resp = self._get_student_progress_page() + + # Ensure that expected text is present + self.assert_progress_page_show_grades(resp, show_correctness, due_date, graded, show_grades, 0, 1, 0) + + # Submit answers to the problem, and re-fetch the progress page + self.answer_problem() + resp = self._get_student_progress_page() + + # Test that the expected text is still present. + self.assert_progress_page_show_grades(resp, show_correctness, due_date, graded, show_grades, 1, 1, .5) + + @attr(shard=1) class VerifyCourseKeyDecoratorTests(TestCase): """ diff --git a/lms/djangoapps/grades/new/subsection_grade.py b/lms/djangoapps/grades/new/subsection_grade.py index 1d397d7bac..2595f06295 100644 --- a/lms/djangoapps/grades/new/subsection_grade.py +++ b/lms/djangoapps/grades/new/subsection_grade.py @@ -7,7 +7,7 @@ from logging import getLogger from lms.djangoapps.grades.scores import get_score, possibly_scored from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade from xmodule import block_metadata_utils, graders -from xmodule.graders import AggregatedScore +from xmodule.graders import AggregatedScore, ShowCorrectness from ..config.waffle import waffle, WRITE_ONLY_IF_ENGAGED @@ -27,6 +27,7 @@ class SubsectionGradeBase(object): self.format = getattr(subsection, 'format', '') self.due = getattr(subsection, 'due', None) self.graded = getattr(subsection, 'graded', False) + self.show_correctness = getattr(subsection, 'show_correctness', '') self.course_version = getattr(subsection, 'course_version', None) self.subtree_edited_timestamp = getattr(subsection, 'subtree_edited_on', None) @@ -47,6 +48,12 @@ class SubsectionGradeBase(object): ) return self.all_total.attempted + def show_grades(self, has_staff_access): + """ + Returns whether subsection scores are currently available to users with or without staff access. + """ + return ShowCorrectness.correctness_available(self.show_correctness, self.due, has_staff_access) + class ZeroSubsectionGrade(SubsectionGradeBase): """ @@ -224,7 +231,7 @@ class SubsectionGrade(SubsectionGradeBase): log_func( u"Grades: SG.{}, subsection: {}, course: {}, " u"version: {}, edit: {}, user: {}," - u"total: {}/{}, graded: {}/{}".format( + u"total: {}/{}, graded: {}/{}, show_correctness: {}".format( log_statement, self.location, self.location.course_key, @@ -235,5 +242,6 @@ class SubsectionGrade(SubsectionGradeBase): self.all_total.possible, self.graded_total.earned, self.graded_total.possible, + self.show_correctness, ) ) diff --git a/lms/djangoapps/grades/transformer.py b/lms/djangoapps/grades/transformer.py index 1b9ee6098f..afe298402b 100644 --- a/lms/djangoapps/grades/transformer.py +++ b/lms/djangoapps/grades/transformer.py @@ -1,11 +1,11 @@ """ Grades Transformer """ +import json from base64 import b64encode from functools import reduce as functools_reduce from hashlib import sha1 from logging import getLogger -import json from lms.djangoapps.course_blocks.transformers.utils import collect_unioned_set_field, get_field_on_block from openedx.core.djangoapps.content.block_structure.transformer import BlockStructureTransformer @@ -29,6 +29,7 @@ class GradesTransformer(BlockStructureTransformer): graded: (boolean) has_score: (boolean) weight: (numeric) + show_correctness: (string) when to show grades (one of 'always', 'past_due', 'never') Additionally, the following value is calculated and stored as a transformer_block_field for each block: @@ -37,7 +38,16 @@ class GradesTransformer(BlockStructureTransformer): """ WRITE_VERSION = 4 READ_VERSION = 4 - FIELDS_TO_COLLECT = [u'due', u'format', u'graded', u'has_score', u'weight', u'course_version', u'subtree_edited_on'] + FIELDS_TO_COLLECT = [ + u'due', + u'format', + u'graded', + u'has_score', + u'weight', + u'course_version', + u'subtree_edited_on', + u'show_correctness', + ] EXPLICIT_GRADED_FIELD_NAME = 'explicit_graded' diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index 017463f81f..a566897c00 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -180,12 +180,30 @@ from django.utils.http import urlquote_plus %endif

%if len(section.problem_scores.values()) > 0: -
-
${ _("Problem Scores: ") if section.graded else _("Practice Scores: ")}
- %for score in section.problem_scores.values(): -
${"{0:.3n}/{1:.3n}".format(float(score.earned),float(score.possible))}
- %endfor -
+ %if section.show_grades(staff_access): +
+
${ _("Problem Scores: ") if section.graded else _("Practice Scores: ")}
+ %for score in section.problem_scores.values(): +
${"{0:.3n}/{1:.3n}".format(float(score.earned),float(score.possible))}
+ %endfor +
+ %else: +

+ %if section.show_correctness == 'past_due': + %if section.graded: + ${_("Problem scores are hidden until the due date.")} + %else: + ${_("Practice scores are hidden until the due date.")} + %endif + %else: + %if section.graded: + ${_("Problem scores are hidden.")} + %else: + ${_("Practice scores are hidden.")} + %endif + %endif +

+ %endif %else:

${_("No problem scores in this section")}

%endif