Merge pull request #224 from edx/will/date-bug-fix

Fix 500 error when self-assessment has default dates

apps/openassessment/xblock/openassessmentblock.py

@@ -433,17 +433,18 @@ class OpenAssessmentBlock(
                 "self-assessment": check whether the self-assessment section is open.
 
         Returns:
-            (tuple): True if the question is closed, False if not. If True,
-                specifies if the "start" date or "due" date is the closing
-                factor.
+            tuple of the form (is_closed, reason, date), where
+                is_closed (bool): indicates whether the step is closed.
+                reason (str or None): specifies the reason the step is closed ("start" or "due")
+                date (datetime or None): is the start/due date.
 
         Examples:
             >>> is_closed()
-            False, None
+            False, None, None
             >>> is_closed(step="submission")
-            True, "due"
+            True, "due", datetime.datetime(2014, 3, 27, 22, 7, 38, 788861)
             >>> is_closed(step="self-assessment")
-            True, "start"
+            True, "start", datetime.datetime(2014, 3, 27, 22, 7, 38, 788861)
 
         """
         submission_range = (self.start, self.submission_due)
@@ -470,11 +471,11 @@ class OpenAssessmentBlock(
         now = dt.datetime.now().replace(tzinfo=pytz.utc)
 
         if now < open_range[0]:
-            return True, "start"
+            return True, "start", open_range[0]
         elif now >= open_range[1]:
-            return True, "due"
+            return True, "due", open_range[1]
         else:
-            return False, None
+            return False, None, None
 
     def is_released(self, step=None):
         """
@@ -492,7 +493,7 @@ class OpenAssessmentBlock(
         """
         # By default, assume that we're published, in case the runtime doesn't support publish date.
         is_published = getattr(self, 'published_date', True) is not None
-        is_closed, reason = self.is_closed(step=step)
+        is_closed, reason, __ = self.is_closed(step=step)
         return is_published and (not is_closed or reason == 'due')
 
     def get_assessment_module(self, mixin_name):

apps/openassessment/xblock/peer_assessment_mixin.py

@@ -111,14 +111,13 @@ class PeerAssessmentMixin(object):
         """
         path = 'openassessmentblock/peer/oa_peer_unavailable.html'
         finished = False
-
-        problem_closed, date = self.is_closed(step="peer-assessment")
+        problem_closed, reason, date = self.is_closed(step="peer-assessment")
         context_dict = {
             "rubric_criteria": self.rubric_criteria,
             "estimated_time": "20 minutes"  # TODO: Need to configure this.
         }
 
-        submissions_closed, __ = self.is_closed(step="submission")
+        submissions_closed, __, __ = self.is_closed(step="submission")
 
         workflow = self.get_workflow_info()
         if workflow is None:
@@ -153,15 +152,11 @@ class PeerAssessmentMixin(object):
                     "Submit your assessment & move to response #{}"
                 ).format(count + 2)
 
-            if assessment.get('due'):
-                context_dict["peer_due"] = self.format_datetime_string(assessment["due"])
-
-
-        if date == "due" and problem_closed:
+        if reason == 'due' and problem_closed:
+            context_dict["peer_due"] = self.format_datetime_string(date)
             path = 'openassessmentblock/peer/oa_peer_closed.html'
-        elif date == 'start' and problem_closed:
-            if assessment.get('start'):
-                context_dict["peer_start"] = self.format_datetime_string(assessment["start"])
+        elif reason == 'start' and problem_closed:
+            context_dict["peer_start"] = self.format_datetime_string(date)
             path = 'openassessmentblock/peer/oa_peer_unavailable.html'
         elif workflow.get("status") == "peer":
             peer_sub = self.get_peer_submission(student_item, assessment, submissions_closed)
@@ -188,7 +183,7 @@ class PeerAssessmentMixin(object):
             assessment,
             over_grading
     ):
-        submissions_closed, __ = self.is_closed(step="submission")
+        submissions_closed, __, __ = self.is_closed(step="submission")
         peer_submission = False
         try:
             peer_submission = peer_api.get_submission_to_assess(

apps/openassessment/xblock/self_assessment_mixin.py

@@ -28,13 +28,13 @@ class SelfAssessmentMixin(object):
         assessment_module = self.get_assessment_module('self-assessment')
 
         path = 'openassessmentblock/self/oa_self_unavailable.html'
-        problem_closed, date = self.is_closed(step="self-assessment")
-        
-        due_date = assessment_module.get('due')
-        if date == 'start' and problem_closed:
-            context["self_start"] = self.format_datetime_string(assessment_module["start"])
-        elif due_date:
-            context["self_due"] = self.format_datetime_string(assessment_module["due"])
+        problem_closed, reason, date = self.is_closed(step="self-assessment")
+
+        if problem_closed:
+            if date == 'start':
+                context["self_start"] = self.format_datetime_string(date)
+            elif date == 'due':
+                context["self_due"] = self.format_datetime_string(date)
 
         workflow = self.get_workflow_info()
         if not workflow:

apps/openassessment/xblock/submission_mixin.py

@@ -175,11 +175,8 @@ class SubmissionMixin(object):
 
         """
         workflow = self.get_workflow_info()
-        problem_closed, date = self.is_closed('submission')
-        sub_due = None
-        if self.submission_due is not None:
-            submission_deadline = dateutil.parser.parse(self.submission_due)
-            sub_due = submission_deadline.strftime("%A, %B %d, %Y %X")
+        problem_closed, __, date = self.is_closed('submission')
+        sub_due = date.strftime("%A, %B %d, %Y %X") if date is not None else None
         context = {
             "saved_response": self.saved_response,
             "save_status": self.save_status,

apps/openassessment/xblock/test/data/self_assessment_default_dates.xml

+<openassessment>
+    <title>Open Assessment Test</title>
+    <prompt>
+        Given the state of the world today, what do you think should be done to
+        combat poverty? Please answer in a short essay of 200-300 words.
+    </prompt>
+    <rubric>
+        <prompt>Read for conciseness, clarity of thought, and form.</prompt>
+        <criterion>
+            <name>𝓒𝓸𝓷𝓬𝓲𝓼𝓮</name>
+            <prompt>How concise is it?</prompt>
+            <option points="3">
+                <name>ﻉซƈﻉɭɭﻉกՇ</name>
+                <explanation>Extremely concise</explanation>
+            </option>
+            <option points="2">
+                <name>Ġööḋ</name>
+                <explanation>Concise</explanation>
+            </option>
+            <option points="1">
+                <name>ק๏๏г</name>
+                <explanation>Wordy</explanation>
+            </option>
+        </criterion>
+        <criterion>
+            <name>Form</name>
+            <prompt>How well-formed is it?</prompt>
+            <option points="3">
+                <name>Good</name>
+                <explanation>Good</explanation>
+            </option>
+            <option points="2">
+                <name>Fair</name>
+                <explanation>Fair</explanation>
+            </option>
+            <option points="1">
+                <name>Poor</name>
+                <explanation>Poor</explanation>
+            </option>
+        </criterion>
+    </rubric>
+    <assessments>
+        <assessment name="peer-assessment"
+            must_grade="5" must_be_graded_by="3"
+            start="2014-12-20T19:00-7:00"
+            due="2014-12-21T22:22-7:00" />
+        <assessment name="self-assessment" />
+    </assessments>
+</openassessment>

apps/openassessment/xblock/test/test_openassessment.py

@@ -143,34 +143,34 @@ class TestDates(XBlockHandlerTestCase):
 
     @scenario('data/basic_scenario.xml')
     def test_start_end_date_checks(self, xblock):
-        xblock.start = dt.datetime(2014, 3, 1).replace(tzinfo=pytz.utc).isoformat()
-        xblock.due = dt.datetime(2014, 3, 5).replace(tzinfo=pytz.utc).isoformat()
+        xblock.start = dt.datetime(2014, 3, 1).replace(tzinfo=pytz.utc)
+        xblock.due = dt.datetime(2014, 3, 5).replace(tzinfo=pytz.utc)
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2014, 2, 28, 23, 59, 59),
-            None, True, "start",
+            None, True, "start", xblock.start,
             released=False
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2014, 3, 1, 1, 1, 1),
-            None, False, None,
+            None, False, None, None,
             released=True
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2014, 3, 4, 23, 59, 59),
-            None, False, None,
+            None, False, None, None,
             released=True
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2014, 3, 5, 1, 1, 1),
-            None, True, "due",
+            None, True, "due", xblock.due,
             released=True
         )
 
@@ -184,20 +184,21 @@ class TestDates(XBlockHandlerTestCase):
             xblock,
             dt.datetime(2014, 2, 28, 23, 59, 59).replace(tzinfo=pytz.utc),
             "submission", True, "start",
+            dt.datetime(2014, 3, 1).replace(tzinfo=pytz.utc),
             released=False
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2014, 3, 1, 1, 1, 1).replace(tzinfo=pytz.utc),
-            "submission", False, None,
+            "submission", False, None, None,
             released=True
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2014, 3, 31, 23, 59, 59).replace(tzinfo=pytz.utc),
-            "submission", False, None,
+            "submission", False, None, None,
             released=True
         )
 
@@ -205,6 +206,7 @@ class TestDates(XBlockHandlerTestCase):
             xblock,
             dt.datetime(2014, 4, 1, 1, 1, 1, 1).replace(tzinfo=pytz.utc),
             "submission", True, "due",
+            dt.datetime(2014, 4, 1).replace(tzinfo=pytz.utc),
             released=True
         )
 
@@ -218,20 +220,21 @@ class TestDates(XBlockHandlerTestCase):
             xblock,
             dt.datetime(2015, 1, 1, 23, 59, 59).replace(tzinfo=pytz.utc),
             "peer-assessment", True, "start",
+            dt.datetime(2015, 1, 2).replace(tzinfo=pytz.utc),
             released=False
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2015, 1, 2, 1, 1, 1).replace(tzinfo=pytz.utc),
-            "peer-assessment", False, None,
+            "peer-assessment", False, None, None,
             released=True
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2015, 3, 31, 23, 59, 59).replace(tzinfo=pytz.utc),
-            "peer-assessment", False, None,
+            "peer-assessment", False, None, None,
             released=True
         )
 
@@ -239,6 +242,7 @@ class TestDates(XBlockHandlerTestCase):
             xblock,
             dt.datetime(2015, 4, 1, 1, 1, 1, 1).replace(tzinfo=pytz.utc),
             "peer-assessment", True, "due",
+            dt.datetime(2015, 4, 1).replace(tzinfo=pytz.utc),
             released=True
         )
 
@@ -252,20 +256,21 @@ class TestDates(XBlockHandlerTestCase):
             xblock,
             dt.datetime(2016, 1, 1, 23, 59, 59).replace(tzinfo=pytz.utc),
             "self-assessment", True, "start",
+            dt.datetime(2016, 1, 2).replace(tzinfo=pytz.utc),
             released=False
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2016, 1, 2, 1, 1, 1).replace(tzinfo=pytz.utc),
-            "self-assessment", False, None,
+            "self-assessment", False, None, None,
             released=True
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2016, 3, 31, 23, 59, 59).replace(tzinfo=pytz.utc),
-            "self-assessment", False, None,
+            "self-assessment", False, None, None,
             released=True
         )
 
@@ -273,6 +278,7 @@ class TestDates(XBlockHandlerTestCase):
             xblock,
             dt.datetime(2016, 4, 1, 1, 1, 1, 1).replace(tzinfo=pytz.utc),
             "self-assessment", True, "due",
+            dt.datetime(2016, 4, 1).replace(tzinfo=pytz.utc),
             released=True
         )
 
@@ -288,20 +294,21 @@ class TestDates(XBlockHandlerTestCase):
             xblock,
             dt.datetime(2014, 2, 28, 23, 59, 59).replace(tzinfo=pytz.utc),
             "peer-assessment", True, "start",
+            dt.datetime(2014, 3, 1).replace(tzinfo=pytz.utc),
             released=False
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2014, 3, 1, 1, 1, 1).replace(tzinfo=pytz.utc),
-            "peer-assessment", False, None,
+            "peer-assessment", False, None, None,
             released=True
         )
 
         self.assert_is_closed(
             xblock,
             dt.datetime(2016, 5, 1, 23, 59, 59).replace(tzinfo=pytz.utc),
-            "peer-assessment", False, None,
+            "peer-assessment", False, None, None,
             released=True
         )
 
@@ -309,6 +316,7 @@ class TestDates(XBlockHandlerTestCase):
             xblock,
             dt.datetime(2016, 5, 2, 1, 1, 1).replace(tzinfo=pytz.utc),
             "peer-assessment", True, "due",
+            dt.datetime(2016, 5, 2).replace(tzinfo=pytz.utc),
             released=True
         )
 
@@ -333,7 +341,10 @@ class TestDates(XBlockHandlerTestCase):
         # If the runtime doesn't provide a published_date field, assume we've been published
         self.assertTrue(xblock.is_released())
 
-    def assert_is_closed(self, xblock, now, step, expected_is_closed, expected_reason, released=None):
+    def assert_is_closed(
+        self, xblock, now, step, expected_is_closed, expected_reason,
+        expected_date=None, released=None
+    ):
         """
         Assert whether the XBlock step is open/closed.
 
@@ -341,8 +352,9 @@ class TestDates(XBlockHandlerTestCase):
             xblock (OpenAssessmentBlock): The xblock under test.
             now (datetime): Time to patch for the xblock's call to datetime.now()
             step (str): The step in the workflow (e.g. "submission", "self-assessment")
-            expected_is_open (bool): Do we expect the step to be open or closed?
-            expecetd_reason (str): Either "start", "due", or None.
+            expected_is_closed (bool): Do we expect the step to be open or closed?
+            expected_reason (str): Either "start", "due", or None.
+            expected_date (datetime): Expected start/due date, or None
 
         Kwargs:
             released (bool): If set, check whether the XBlock has been released.
@@ -358,9 +370,10 @@ class TestDates(XBlockHandlerTestCase):
         self.addCleanup(datetime_patcher.stop)
         mocked_datetime.datetime.now.return_value = now
 
-        is_closed, reason = xblock.is_closed(step=step)
+        is_closed, reason, date = xblock.is_closed(step=step)
         self.assertEqual(is_closed, expected_is_closed)
         self.assertEqual(reason, expected_reason)
+        self.assertEqual(date, expected_date)
 
         if released is not None:
             self.assertEqual(xblock.is_released(step=step), released)

apps/openassessment/xblock/test/test_self.py

@@ -4,6 +4,7 @@ Tests for self assessment handlers in Open Assessment XBlock.
 """
 import copy
 import json
+import datetime
 import mock
 from openassessment.assessment import self_api
 from openassessment.workflow import api as workflow_api
@@ -193,5 +194,15 @@ class TestSelfAssessment(XBlockHandlerTestCase):
         self.assertIsNotNone(self_response)
         self.assertNotIn(submission["answer"]["text"].encode('utf-8'), self_response.body)
 
-        #Validate Self Rendering.
+        # Validate Self Rendering.
+        self.assertIn("available".encode('utf-8'), self_response.body)
+
+    @scenario('data/self_assessment_default_dates.xml', user_id='Bob')
+    def test_no_dates(self, xblock):
+        # In this scenario, the self-assessment has no dates specified,
+        # but the module before it specifies a start date, and the
+        # problem itself specifies a due date.
+        xblock.due = datetime.datetime(4000, 1, 1, 1)
+        self_response = xblock.render_self_assessment({})
+        self.assertIsNotNone(self_response)
         self.assertIn("available".encode('utf-8'), self_response.body)