Added parse_boolean function

This commit adds the ability to use true/false on boolean fields and also validates integers properly

wagtail/api/v2/filters.py

@@ -1,13 +1,14 @@
 from __future__ import absolute_import, unicode_literals
 
 from django.conf import settings
+from django.db import models
 from rest_framework.filters import BaseFilterBackend
-from taggit.managers import _TaggableManager
+from taggit.managers import TaggableManager
 
 from wagtail.wagtailcore.models import Page
 from wagtail.wagtailsearch.backends import get_search_backend
 
-from .utils import BadRequestError, pages_for_site
+from .utils import BadRequestError, pages_for_site, parse_boolean
 
 
 class FieldsFilter(BaseFilterBackend):
@@ -20,9 +21,25 @@ class FieldsFilter(BaseFilterBackend):
 
         for field_name, value in request.GET.items():
             if field_name in fields:
-                field = getattr(queryset.model, field_name, None)
+                try:
+                    field = queryset.model._meta.get_field(field_name)
+                except LookupError:
+                    field = None
 
-                if isinstance(field, _TaggableManager):
+                # Convert value into python
+                try:
+                    if isinstance(field, (models.BooleanField, models.NullBooleanField)):
+                        value = parse_boolean(value)
+                    elif isinstance(field, (models.IntegerField, models.AutoField)):
+                        value = int(value)
+                except ValueError as e:
+                    raise BadRequestError("field filter error. '%s' is not a valid value for %s (%s)" % (
+                        value,
+                        field_name,
+                        str(e)
+                    ))
+
+                if isinstance(field, TaggableManager):
                     for tag in value.split(','):
                         queryset = queryset.filter(**{field_name + '__name': tag})
 

wagtail/api/v2/tests/test_pages.py

@@ -376,6 +376,13 @@ class TestPageListing(TestCase):
         page_id_list = self.get_page_id_list(content)
         self.assertEqual(page_id_list, [16])
 
+    def test_filtering_on_boolean(self):
+        response = self.get_response(show_in_menus='false')
+        content = json.loads(response.content.decode('UTF-8'))
+
+        page_id_list = self.get_page_id_list(content)
+        self.assertEqual(page_id_list, [8, 9, 16, 18, 19, 17])
+
     def test_filtering_doesnt_work_on_specific_fields_without_type(self):
         response = self.get_response(date='2013-12-02')
         content = json.loads(response.content.decode('UTF-8'))
@@ -404,6 +411,20 @@ class TestPageListing(TestCase):
         self.assertEqual(response.status_code, 400)
         self.assertEqual(content, {'message': "query parameter is not an operation or a recognised field: not_a_field"})
 
+    def test_filtering_int_validation(self):
+        response = self.get_response(id='abc')
+        content = json.loads(response.content.decode('UTF-8'))
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(content, {'message': "field filter error. 'abc' is not a valid value for id (invalid literal for int() with base 10: 'abc')"})
+
+    def test_filtering_boolean_validation(self):
+        response = self.get_response(show_in_menus='abc')
+        content = json.loads(response.content.decode('UTF-8'))
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(content, {'message': "field filter error. 'abc' is not a valid value for show_in_menus (expected 'true' or 'false', got 'abc')"})
+
 
     # CHILD OF FILTER
 

wagtail/api/v2/tests/tests.py

@@ -2,7 +2,7 @@ from __future__ import absolute_import, unicode_literals
 
 from unittest import TestCase
 
-from ..utils import FieldsParameterParseError, parse_fields_parameter
+from ..utils import FieldsParameterParseError, parse_boolean, parse_fields_parameter
 
 
 class TestParseFieldsParameter(TestCase):
@@ -235,3 +235,42 @@ class TestParseFieldsParameter(TestCase):
             parse_fields_parameter('*,_')
 
         self.assertEqual(str(e.exception), "'_' must be in the first position")
+
+
+class TestParseBoolean(TestCase):
+    # GOOD STUFF
+
+    def test_valid_true(self):
+        parsed = parse_boolean('true')
+
+        self.assertEqual(parsed, True)
+
+    def test_valid_false(self):
+        parsed = parse_boolean('false')
+
+        self.assertEqual(parsed, False)
+
+    def test_valid_1(self):
+        parsed = parse_boolean('1')
+
+        self.assertEqual(parsed, True)
+
+    def test_valid_0(self):
+        parsed = parse_boolean('0')
+
+        self.assertEqual(parsed, False)
+
+
+    # BAD STUFF
+
+    def test_invalid(self):
+        with self.assertRaises(ValueError) as e:
+            parse_boolean('foo')
+
+        self.assertEqual(str(e.exception), "expected 'true' or 'false', got 'foo'")
+
+    def test_invalid_integer(self):
+        with self.assertRaises(ValueError) as e:
+            parse_boolean('2')
+
+        self.assertEqual(str(e.exception), "expected 'true' or 'false', got '2'")

wagtail/api/v2/utils.py

@@ -216,3 +216,20 @@ def parse_fields_parameter(fields_str):
     fields, _ = parse_fields(fields_str)
 
     return fields
+
+
+def parse_boolean(value):
+    """
+    Parses strings into booleans using the following mapping (case-sensitive):
+
+    'true'   => True
+    'false'  => False
+    '1'      => True
+    '0'      => False
+    """
+    if value in ['true', '1']:
+        return True
+    elif value in ['false', '0']:
+        return False
+    else:
+        raise ValueError("expected 'true' or 'false', got '%s'" % value)

wagtail/wagtailadmin/api/filters.py

@@ -2,7 +2,7 @@ from __future__ import absolute_import, unicode_literals
 
 from rest_framework.filters import BaseFilterBackend
 
-from wagtail.api.v2.utils import BadRequestError
+from wagtail.api.v2.utils import BadRequestError, parse_boolean
 
 
 class HasChildrenFilter(BaseFilterBackend):
@@ -13,12 +13,11 @@ class HasChildrenFilter(BaseFilterBackend):
     def filter_queryset(self, request, queryset, view):
         if 'has_children' in request.GET:
             try:
-                has_children_filter = int(request.GET['has_children'])
-                assert has_children_filter is 1 or has_children_filter is 0
-            except (ValueError, AssertionError):
-                raise BadRequestError("has_children must be 1 or 0")
+                has_children_filter = parse_boolean(request.GET['has_children'])
+            except ValueError:
+                raise BadRequestError("has_children must be 'true' or 'false'")
 
-            if has_children_filter == 1:
+            if has_children_filter is True:
                 return queryset.filter(numchild__gt=0)
             else:
                 return queryset.filter(numchild=0)

wagtail/wagtailadmin/tests/api/test_pages.py

@@ -248,32 +248,47 @@ class TestAdminPageListing(AdminAPITestCase, TestPageListing):
     # HAS CHILDREN FILTER
 
     def test_has_children_filter(self):
-        response = self.get_response(has_children=1)
+        response = self.get_response(has_children='true')
         content = json.loads(response.content.decode('UTF-8'))
 
         page_id_list = self.get_page_id_list(content)
         self.assertEqual(page_id_list, [2, 4, 5, 6, 21, 20])
 
     def test_has_children_filter_off(self):
+        response = self.get_response(has_children='false')
+        content = json.loads(response.content.decode('UTF-8'))
+
+        page_id_list = self.get_page_id_list(content)
+        self.assertEqual(page_id_list, [8, 9, 16, 18, 19, 10, 15, 17, 22, 23, 13, 14, 12])
+
+    def test_has_children_filter_int(self):
+        response = self.get_response(has_children=1)
+        content = json.loads(response.content.decode('UTF-8'))
+
+        page_id_list = self.get_page_id_list(content)
+        self.assertEqual(page_id_list, [2, 4, 5, 6, 21, 20])
+
+    def test_has_children_filter_int_off(self):
         response = self.get_response(has_children=0)
         content = json.loads(response.content.decode('UTF-8'))
 
         page_id_list = self.get_page_id_list(content)
         self.assertEqual(page_id_list, [8, 9, 16, 18, 19, 10, 15, 17, 22, 23, 13, 14, 12])
 
+
     def test_has_children_filter_invalid_integer(self):
         response = self.get_response(has_children=3)
         content = json.loads(response.content.decode('UTF-8'))
 
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(content, {'message': "has_children must be 1 or 0"})
+        self.assertEqual(content, {'message': "has_children must be 'true' or 'false'"})
 
     def test_has_children_filter_invalid_value(self):
         response = self.get_response(has_children='yes')
         content = json.loads(response.content.decode('UTF-8'))
 
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(content, {'message': "has_children must be 1 or 0"})
+        self.assertEqual(content, {'message': "has_children must be 'true' or 'false'"})
 
 
 class TestAdminPageDetail(AdminAPITestCase, TestPageDetail):