Add meta info to stats

binder/route_decorators.py

@@ -0,0 +1,48 @@
+from .exceptions import BinderMethodNotAllowed, BinderNotFound
+
+
+def _route_decorator(is_detail, name=None, methods=None, extra_route='', unauthenticated=False, *, fetch_obj=False):
+	def decorator(func):
+		def wrapper(self, request=None, *args, **kwargs):
+			if methods is not None and request.method not in methods:
+				raise BinderMethodNotAllowed(methods)
+
+			if fetch_obj:
+				if 'pk' in kwargs:
+					pk = kwargs['pk']
+					del kwargs['pk']
+
+					try:
+						kwargs['obj'] = self.get_queryset(request).get(pk=pk)
+					except self.model.DoesNotExist:
+						raise BinderNotFound()
+				else:
+					if len(args) == 0:
+						raise Exception('Can not fetch_obj if there is no pk!')
+
+					args = list(args)
+					pk = args[0]
+					try:
+						args[0] = self.get_queryset(request).get(pk=pk)
+					except self.model.DoesNotExist:
+						raise BinderNotFound()
+
+			return func(self, request, *args, **kwargs)
+		if is_detail:
+			wrapper.detail_route = True
+		else:
+			wrapper.list_route = True
+
+		wrapper.route_name = name
+		wrapper.extra_route = extra_route
+		wrapper.unauthenticated = unauthenticated
+		return wrapper
+	return decorator
+
+
+def list_route(*args, **kwargs):
+	return _route_decorator(False, *args, **kwargs)
+
+
+def detail_route(*args, **kwargs):
+	return _route_decorator(True, *args, **kwargs)

binder/router.py

@@ -4,53 +4,9 @@
 from django.urls import reverse, re_path
 
 from binder.views import ModelView
-from .exceptions import BinderRequestError, BinderCSRFFailure, BinderMethodNotAllowed, BinderNotFound
+from .exceptions import BinderRequestError, BinderCSRFFailure
 
-
-def _route_decorator(is_detail, name=None, methods=None, extra_route='', unauthenticated=False, *, fetch_obj=False):
-	def decorator(func):
-		def wrapper(self, request=None, *args, **kwargs):
-			if methods is not None and request.method not in methods:
-				raise BinderMethodNotAllowed(methods)
-
-			if fetch_obj:
-				if 'pk' in kwargs:
-					pk = kwargs['pk']
-					del kwargs['pk']
-
-					try:
-						kwargs['obj'] = self.get_queryset(request).get(pk=pk)
-					except self.model.DoesNotExist:
-						raise BinderNotFound()
-				else:
-					if len(args) == 0:
-						raise Exception('Can not fetch_obj if there is no pk!')
-
-					args = list(args)
-					pk = args[0]
-					try:
-						args[0] = self.get_queryset(request).get(pk=pk)
-					except self.model.DoesNotExist:
-						raise BinderNotFound()
-
-			return func(self, request, *args, **kwargs)
-		if is_detail:
-			wrapper.detail_route = True
-		else:
-			wrapper.list_route = True
-
-		wrapper.route_name = name
-		wrapper.extra_route = extra_route
-		wrapper.unauthenticated = unauthenticated
-		return wrapper
-	return decorator
-
-
-def list_route(*args, **kwargs):
-	return _route_decorator(False, *args, **kwargs)
-
-def detail_route(*args, **kwargs):
-	return _route_decorator(True, *args, **kwargs)
+from .route_decorators import _route_decorator, list_route, detail_route  # noqa: for backwards compatibility
 
 
 
@@ -184,14 +140,6 @@ def urls(self):
 				urls.append(re_path(r'^{}/(?P<pk>[0-9]+)/{}/$'.format(route.route, ff),
 						view.as_view(), {'file_field': ff, 'router': self}, name='{}.{}'.format(name, ff)))
 
-			# Stats endpoint
-			urls.append(re_path(
-				r'^{}/stats/$'.format(route.route),
-				view.as_view(),
-				{'method': 'stats_view', 'router': self},
-				name='{}.stats'.format(name),
-			))
-
 			# Custom endpoints
 			for m in dir(view):
 				method = getattr(view, m)

binder/views.py

@@ -20,7 +20,7 @@
 from django.http.request import RawPostDataException
 from django.http.multipartparser import MultiPartParser
 from django.db import models, connections
-from django.db.models import Q, F, Count, Sum, Min, Max, Avg
+from django.db.models import Q, F, Count
 from django.db.models.lookups import Transform
 from django.utils import timezone
 from django.db import transaction
@@ -33,6 +33,7 @@
 from .orderable_agg import OrderableArrayAgg, GroupConcat, StringAgg
 from .models import FieldFilter, BinderModel, ContextAnnotation, OptionalAnnotation, BinderFileField, BinderImageField
 from .json import JsonResponse, jsonloads, jsondumps
+from .route_decorators import list_route
 
 
 DEFAULT_STATS = {
@@ -2924,6 +2925,7 @@ def view_history(self, request, pk=None, **kwargs):
 			return history.view_changesets(request, changesets.order_by('-id'))
 
 
+	@list_route('stats', methods=['GET'])
 	def stats_view(self, request):
 		# We only apply annotations when used, so we can just pretend everything is included to simplify stuff
 		try:
@@ -2982,20 +2984,28 @@ def _get_stat(self, request, queryset, stat, annotations, include_annotations):
 			group_by = stat['group_by']
 		except KeyError:
 			# No group by so just return a simple stat
-			return queryset.aggregate(result=stat['expr'])['result']
+			return {
+				'value': queryset.aggregate(result=stat['expr'])['result'],
+				'filters': stat.get('filters', {}),
+			}
 
-		group_by = group_by.replace('.', '__')
-		# The jsonloads/jsondumps is to make sure we can handle different
-		# types as keys, an example is dates.
+		django_group_by = group_by.replace('.', '__')
 		return {
-			jsonloads(jsondumps(key)): value
-			for key, value in (
-				queryset
-				.order_by()
-				.values(group_by)
-				.annotate(_binder_stat=stat['expr'])
-				.values_list(group_by, '_binder_stat')
-			)
+			'value': {
+				# The jsonloads/jsondumps is to make sure we can handle different
+				# types as keys, an example is dates.
+				jsonloads(jsondumps(key)): value
+				for key, value in (
+					queryset
+					.order_by()
+					.exclude(**{django_group_by: None})
+					.values(django_group_by)
+					.annotate(_binder_stat=stat['expr'])
+					.values_list(django_group_by, '_binder_stat')
+				)
+			},
+			'group_by': group_by,
+			'filters': stat.get('filters', {}),
 		}
 
 

tests/test_stats.py

@@ -5,6 +5,8 @@
 
 from .testapp.models import Animal, Caretaker, Zoo
 
+from .compare import assert_json, ANY
+
 
 class StatsTest(TestCase):
 
@@ -24,26 +26,32 @@ def setUp(self):
 
 		self.assertTrue(self.client.login(username='testuser', password='test'))
 
-	def get_stats(self, *stats, params={}):
+	def get_stats(self, *stats, status=200, params={}):
 		res = self.client.get('/animal/stats/', {
 			'stats': ','.join(stats),
 			**params,
 		})
-		if res.status_code != 200:
-			print(res.content.decode())
-		self.assertEqual(res.status_code, 200)
+		self.assertEqual(res.status_code, status)
 		return json.loads(res.content)
 
 	def test_animals_without_caretaker(self):
 		res = self.get_stats('without_caretaker')
-		self.assertEqual(res, {'without_caretaker': 1})
+		self.assertEqual(res, {
+			'without_caretaker': {
+				'value': 1,
+				'filters': {'caretaker:isnull': 'true'},
+			},
+		})
 
 	def test_animals_by_zoo(self):
 		res = self.get_stats('by_zoo')
-		self.assertEqual(res, {'by_zoo': {
-			'Zoo 1': 1,
-			'Zoo 2': 2,
-		}})
+		self.assertEqual(res, {
+			'by_zoo': {
+				'value': {'Zoo 1': 1, 'Zoo 2': 2},
+				'filters': {},
+				'group_by': 'zoo.name',
+			},
+		})
 
 	def test_stats_filtered(self):
 		res = self.get_stats(
@@ -53,7 +61,25 @@ def test_stats_filtered(self):
 			params={'.zoo.name': 'Zoo 1'},
 		)
 		self.assertEqual(res, {
-			'total': 1,
-			'without_caretaker': 0,
-			'by_zoo': {'Zoo 1': 1},
+			'total': {
+				'value': 1,
+				'filters': {},
+			},
+			'without_caretaker': {
+				'value': 0,
+				'filters': {'caretaker:isnull': 'true'},
+			},
+			'by_zoo': {
+				'value': {'Zoo 1': 1},
+				'filters': {},
+				'group_by': 'zoo.name',
+			},
+		})
+
+	def test_stat_not_found(self):
+		res = self.get_stats('does_not_exist', status=418)
+		assert_json(res, {
+			'code': 'RequestError',
+			'message': 'unknown stat: does_not_exist',
+			'debug': ANY(),
 		})