MA-1211; Discussion API - make boolean params case insensitive

lms/djangoapps/discussion_api/forms.py

@@ -9,11 +9,11 @@ from django.forms import (
     Form,
     IntegerField,
     NullBooleanField,
-)
+    Select)
 
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locator import CourseLocator
-from openedx.core.djangoapps.util.forms import MultiValueField
+from openedx.core.djangoapps.util.forms import MultiValueField, ExtendedNullBooleanField
 
 
 class _PaginationForm(Form):
@@ -39,7 +39,7 @@ class ThreadListGetForm(_PaginationForm):
     course_id = CharField()
     topic_id = MultiValueField(required=False)
     text_search = CharField(required=False)
-    following = NullBooleanField(required=False)
+    following = ExtendedNullBooleanField(required=False)
     view = ChoiceField(
         choices=[(choice, choice) for choice in ["unread", "unanswered"]],
         required=False,
@@ -106,9 +106,7 @@ class CommentListGetForm(_PaginationForm):
     A form to validate query parameters in the comment list retrieval endpoint
     """
     thread_id = CharField()
-    # TODO: should we use something better here? This only accepts "True",
-    # "False", "1", and "0"
-    endorsed = NullBooleanField(required=False)
+    endorsed = ExtendedNullBooleanField(required=False)
 
 
 class CommentActionsForm(Form):

lms/djangoapps/discussion_api/tests/test_views.py

@@ -306,7 +306,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "has_endorsed": False,
         }]
         self.register_get_threads_response(source_threads, page=1, num_pages=2)
-        response = self.client.get(self.url, {"course_id": unicode(self.course.id)})
+        response = self.client.get(self.url, {"course_id": unicode(self.course.id), "following": ""})
         self.assert_response_correct(
             response,
             200,
@@ -395,16 +395,15 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "text": ["test search string"],
         })
 
-    def test_following(self):
+    @ddt.data(True, "true", "1")
+    def test_following_true(self, following):
         self.register_get_user_response(self.user)
         self.register_subscribed_threads_response(self.user, [], page=1, num_pages=1)
         response = self.client.get(
             self.url,
             {
                 "course_id": unicode(self.course.id),
-                "page": "1",
-                "page_size": "4",
-                "following": "True",
+                "following": following,
             }
         )
         self.assert_response_correct(
@@ -417,6 +416,39 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             "/api/v1/users/{}/subscribed_threads".format(self.user.id)
         )
 
+    @ddt.data(False, "false", "0")
+    def test_following_false(self, following):
+        response = self.client.get(
+            self.url,
+            {
+                "course_id": unicode(self.course.id),
+                "following": following,
+            }
+        )
+        self.assert_response_correct(
+            response,
+            400,
+            {"field_errors": {
+                "following": {"developer_message": "The value of the 'following' parameter must be true."}
+            }}
+        )
+
+    def test_following_error(self):
+        response = self.client.get(
+            self.url,
+            {
+                "course_id": unicode(self.course.id),
+                "following": "invalid-boolean",
+            }
+        )
+        self.assert_response_correct(
+            response,
+            400,
+            {"field_errors": {
+                "following": {"developer_message": "Invalid Boolean Value."}
+            }}
+        )
+
     @ddt.data(
         ("last_activity_at", "activity"),
         ("comment_count", "comments"),
@@ -740,6 +772,7 @@ class ThreadViewSetDeleteTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
         self.assertEqual(response.status_code, 404)
 
 
+@ddt.ddt
 @httpretty.activate
 @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
 class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
@@ -750,6 +783,15 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
         self.url = reverse("comment-list")
         self.thread_id = "test_thread"
 
+    def make_minimal_cs_thread(self, overrides=None):
+        """
+        Create a thread with the given overrides, plus the course_id if not
+        already in overrides.
+        """
+        overrides = overrides.copy() if overrides else {}
+        overrides.setdefault("course_id", unicode(self.course.id))
+        return make_minimal_cs_thread(overrides)
+
     def test_thread_id_missing(self):
         response = self.client.get(self.url)
         self.assert_response_correct(
@@ -869,6 +911,31 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
             }
         )
 
+    @ddt.data(
+        (True, "endorsed_comment"),
+        ("true", "endorsed_comment"),
+        ("1", "endorsed_comment"),
+        (False, "non_endorsed_comment"),
+        ("false", "non_endorsed_comment"),
+        ("0", "non_endorsed_comment"),
+    )
+    @ddt.unpack
+    def test_question_content(self, endorsed, comment_id):
+        self.register_get_user_response(self.user)
+        thread = self.make_minimal_cs_thread({
+            "thread_type": "question",
+            "endorsed_responses": [make_minimal_cs_comment({"id": "endorsed_comment"})],
+            "non_endorsed_responses": [make_minimal_cs_comment({"id": "non_endorsed_comment"})],
+            "non_endorsed_resp_total": 1,
+        })
+        self.register_get_thread_response(thread)
+        response = self.client.get(self.url, {
+            "thread_id": thread["id"],
+            "endorsed": endorsed,
+        })
+        parsed_content = json.loads(response.content)
+        self.assertEqual(parsed_content["results"][0]["id"], comment_id)
+
 
 @httpretty.activate
 @disable_signal(api, 'comment_deleted')

openedx/core/djangoapps/util/forms.py

@@ -3,7 +3,7 @@ Custom forms-related types
 """
 
 from django.core.exceptions import ValidationError
-from django.forms import Field, MultipleHiddenInput
+from django.forms import Field, MultipleHiddenInput, NullBooleanField, Select
 
 
 class MultiValueField(Field):
@@ -42,3 +42,40 @@ class MultiValueField(Field):
         """
         if values and "" in values:
             raise ValidationError("This field cannot be empty.")
+
+
+class ExtendedNullBooleanField(NullBooleanField):
+    """
+    A field whose valid values are None, True, 'True', 'true', '1',
+    False, 'False', 'false' and '0'.
+    """
+
+    NULL_BOOLEAN_CHOICES = (
+        (None, ""),
+        (True, True),
+        (True, "True"),
+        (True, "true"),
+        (True, "1"),
+        (False, False),
+        (False, "False"),
+        (False, "false"),
+        (False, "0"),
+    )
+
+    widget = Select(choices=NULL_BOOLEAN_CHOICES)
+
+    def to_python(self, value):
+        """
+        Explicitly checks for the string 'True', 'False', 'true',
+        'false', '1' and '0' and returns boolean True or False.
+        Returns None if value is not passed at all and raises an
+        exception for any other value.
+        """
+        if value in (True, 'True', 'true', '1'):
+            return True
+        elif value in (False, 'False', 'false', '0'):
+            return False
+        elif not value:
+            return None
+        else:
+            raise ValidationError("Invalid Boolean Value.")