Merge pull request #844 from edx/christina/bad_user

Be robust to username not being available.

openassessment/templates/openassessmentblock/response/oa_response_cancelled.html

@@ -25,9 +25,15 @@
 
                 <div class="message__content">
                     <p>
-                        {% blocktrans with removed_datetime=workflow_cancellation.cancelled_at|utc|date:"N j, Y H:i e" removed_by_username=workflow_cancellation.cancelled_by %}
-                            Your submission has been cancelled by {{ removed_by_username }} on {{ removed_datetime }}
-                        {% endblocktrans %}
+                        {% if workflow_cancellation.cancelled_by %}
+                            {% blocktrans with removed_datetime=workflow_cancellation.cancelled_at|utc|date:"N j, Y H:i e" removed_by_username=workflow_cancellation.cancelled_by %}
+                                Your submission has been cancelled by {{ removed_by_username }} on {{ removed_datetime }}
+                            {% endblocktrans %}
+                        {% else %}
+                            {% blocktrans with removed_datetime=workflow_cancellation.cancelled_at|utc|date:"N j, Y H:i e" %}
+                                Your submission was cancelled on {{ removed_datetime }}
+                            {% endblocktrans %}
+                        {% endif %}
                     </p>
                     <p>
                         <!-- Comments: Reason for Cancellation-->

openassessment/templates/openassessmentblock/staff_area/oa_staff_grade_learners_assessment.html

@@ -12,9 +12,13 @@
                     <div class="staff-assessment__display">
                         <header class="staff-assessment__display__header">
                             <h4 class="staff-assessment__display__title">
-                                {% blocktrans %}
-                                    Response for: {{ student_username }}
-                                {% endblocktrans %}
+                                {% if student_username %}
+                                    {% blocktrans %}
+                                        Response for: {{ student_username }}
+                                    {% endblocktrans %}
+                                {% else %}
+                                    {% trans "Learner Response" %}
+                                {% endif %}
                             </h4>
                         </header>
 

openassessment/templates/openassessmentblock/staff_area/oa_staff_override_assessment.html

@@ -11,9 +11,13 @@
                 <div class="staff-assessment__display">
                     <header class="staff-assessment__display__header">
                         <h3 class="staff-assessment__display__title">
-                            {% blocktrans %}
-                                Response for: {{ student_username }}
-                            {% endblocktrans %}
+                            {% if student_username %}
+                                {% blocktrans %}
+                                    Response for: {{ student_username }}
+                                {% endblocktrans %}
+                            {% else %}
+                                {% trans "Learner Response" %}
+                            {% endif %}
                         </h3>
                     </header>
 

openassessment/templates/openassessmentblock/staff_area/oa_student_info.html

@@ -21,9 +21,15 @@
             <div class="ui-toggle-visibility__content">
                 {% if workflow_cancellation %}
                     <p>
-                        {% blocktrans with removed_by_username=workflow_cancellation.cancelled_by removed_datetime=workflow_cancellation.cancelled_at|utc|date:"F j, Y H:i e" %}
-                            Learner submission removed by {{ removed_by_username }} on {{ removed_datetime }}
-                        {% endblocktrans %}
+                        {% if workflow_cancellation.cancelled_by %}
+                            {% blocktrans with removed_by_username=workflow_cancellation.cancelled_by removed_datetime=workflow_cancellation.cancelled_at|utc|date:"F j, Y H:i e" %}
+                                Learner submission removed by {{ removed_by_username }} on {{ removed_datetime }}
+                            {% endblocktrans %}
+                        {% else %}
+                            {% blocktrans with removed_datetime=workflow_cancellation.cancelled_at|utc|date:"F j, Y H:i e" %}
+                                Learner submission removed on {{ removed_datetime }}
+                            {% endblocktrans %}
+                        {% endif %}
                     </p>
                     <!-- Comments: Reason for Cancellation-->
                     <p>

openassessment/xblock/openassessmentblock.py

@@ -953,8 +953,24 @@ class OpenAssessmentBlock(
             return unicode(key)
 
     def get_username(self, anonymous_user_id):
+        """
+        Return the username of the user associated with anonymous_user_id
+        Args:
+            anonymous_user_id (str): the anonymous user id of the user
+
+        Returns: the username if it can be identified. If the xblock service to converts to a real user
+            fails, returns None and logs the error.
+
+        """
         if hasattr(self, "xmodule_runtime"):
-            return self.xmodule_runtime.get_real_user(anonymous_user_id).username
+            user = self.xmodule_runtime.get_real_user(anonymous_user_id)
+            if user:
+                return user.username
+            else:
+                logger.exception(
+                    "XBlock service could not find user for anonymous_user_id '{}'".format(anonymous_user_id)
+                )
+                return None
 
     def _adjust_start_date_for_beta_testers(self, start):
         if hasattr(self, "xmodule_runtime"):

openassessment/xblock/test/test_openassessment.py

@@ -8,7 +8,6 @@ from mock import Mock, patch, MagicMock, PropertyMock
 
 from openassessment.xblock import openassessmentblock
 from openassessment.xblock.resolve_dates import DISTANT_PAST, DISTANT_FUTURE
-from openassessment.workflow import api as workflow_api
 from openassessment.workflow.errors import AssessmentWorkflowError
 from .base import XBlockHandlerTestCase, scenario
 
@@ -659,3 +658,20 @@ class TestDates(XBlockHandlerTestCase):
 
         if released is not None:
             self.assertEqual(xblock.is_released(step=step), released)
+
+    @scenario('data/basic_scenario.xml')
+    def test_get_username(self, xblock):
+        user = MagicMock()
+        user.username = "Bob"
+
+        xblock.xmodule_runtime = MagicMock()
+        xblock.xmodule_runtime.get_real_user.return_value = user
+
+        self.assertEqual('Bob', xblock.get_username('anon_id'))
+
+    @scenario('data/basic_scenario.xml')
+    def test_get_username_unknown_id(self, xblock):
+        xblock.xmodule_runtime = MagicMock()
+        xblock.xmodule_runtime.get_real_user.return_value = None
+
+        self.assertIsNone(xblock.get_username('unknown_id'))