Merge pull request #15018 from open-craft/jill/mit-capa-progress-page

Mask grades on progress page according to show_correctness setting

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):
         """

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

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

common/lib/xmodule/xmodule/tests/test_graders.py

@@ -2,13 +2,15 @@
 Grading tests
 """
 
-from datetime import datetime
-
-import ddt
 import unittest
+from datetime import datetime, timedelta
 
+import ddt
+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
+        )

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'

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)

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

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,
             )
         )

lms/djangoapps/grades/transformer.py

 """
 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'
 

lms/templates/courseware/progress.html

@@ -180,12 +180,30 @@ from django.utils.http import urlquote_plus
                                             %endif
                                         </p>
                                         %if len(section.problem_scores.values()) > 0:
-                                        <dl class="scores">
-                                            <dt class="hd hd-6">${ _("Problem Scores: ") if section.graded else _("Practice Scores: ")}</dt>
-                                            %for score in section.problem_scores.values():
-                                            <dd>${"{0:.3n}/{1:.3n}".format(float(score.earned),float(score.possible))}</dd>
-                                            %endfor
-                                        </dl>
+                                          %if section.show_grades(staff_access):
+                                          <dl class="scores">
+                                              <dt class="hd hd-6">${ _("Problem Scores: ") if section.graded else _("Practice Scores: ")}</dt>
+                                              %for score in section.problem_scores.values():
+                                              <dd>${"{0:.3n}/{1:.3n}".format(float(score.earned),float(score.possible))}</dd>
+                                              %endfor
+                                          </dl>
+                                          %else:
+                                            <p class="hide-scores">
+                                            %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
+                                            </p>
+                                          %endif
                                         %else:
                                         <p class="no-scores">${_("No problem scores in this section")}</p>
                                         %endif