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 = "
${_("No problem scores in this section")}
%endif