From 9e232c337eb5fbf841b3da9b9ab6a186e0509656 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Mon, 20 Feb 2023 17:52:28 +0100
Subject: [PATCH 1/5] api: split statistics filters code (#63368)

---
 src/authentic2/api_views.py | 64 ++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/src/authentic2/api_views.py b/src/authentic2/api_views.py
index 43edcdeb0..92b2f9e50 100644
--- a/src/authentic2/api_views.py
+++ b/src/authentic2/api_views.py
@@ -30,6 +30,7 @@ from django.db import models, transaction
 from django.shortcuts import get_object_or_404
 from django.utils.dateparse import parse_datetime
 from django.utils.encoding import force_str
+from django.utils.functional import cached_property
 from django.utils.text import slugify
 from django.utils.timezone import now
 from django.utils.translation import gettext_lazy as _
@@ -1620,19 +1621,6 @@ class StatisticsAPI(ViewSet):
 
     def list(self, request):
         statistics = []
-        services_ous = [
-            {'id': ou.slug, 'label': ou.name}
-            for ou in OrganizationalUnit.objects.exclude(service__isnull=True)
-        ]
-        users_ous = [
-            {'id': ou.slug, 'label': ou.name}
-            for ou in OrganizationalUnit.objects.exclude(user__isnull=True).order_by('name')
-        ]
-        services = [
-            {'id': '%s %s' % (service['slug'], service['ou__slug']), 'label': service['name']}
-            for service in Service.objects.values('slug', 'name', 'ou__slug').order_by('ou__name', 'name')
-        ]
-
         time_interval_field = StatisticsSerializer().get_fields()['time_interval']
         common_filters = [
             {
@@ -1648,16 +1636,7 @@ class StatisticsAPI(ViewSet):
         for action in self.get_extra_actions():
             url = self.reverse_action(action.url_name)
             filters = common_filters.copy()
-            if 'service' in action.filters:
-                filters.append({'id': 'service', 'label': _('Service'), 'options': services})
-            if 'services_ou' in action.filters and len(services_ous) > 1:
-                filters.append(
-                    {'id': 'services_ou', 'label': _('Services organizational unit'), 'options': services_ous}
-                )
-            if 'users_ou' in action.filters and len(users_ous) > 1:
-                filters.append(
-                    {'id': 'users_ou', 'label': _('Users organizational unit'), 'options': users_ous}
-                )
+            filters.extend(self.get_additional_filters(action.filters))
             data = {
                 'name': action.kwargs['name'],
                 'url': url,
@@ -1673,6 +1652,45 @@ class StatisticsAPI(ViewSet):
             }
         )
 
+    @cached_property
+    def services_ous(self):
+        return [
+            {'id': ou.slug, 'label': ou.name}
+            for ou in OrganizationalUnit.objects.exclude(service__isnull=True)
+        ]
+
+    @cached_property
+    def users_ous(self):
+        return [
+            {'id': ou.slug, 'label': ou.name}
+            for ou in OrganizationalUnit.objects.exclude(user__isnull=True).order_by('name')
+        ]
+
+    @cached_property
+    def services(self):
+        return [
+            {'id': '%s %s' % (service['slug'], service['ou__slug']), 'label': service['name']}
+            for service in Service.objects.values('slug', 'name', 'ou__slug').order_by('ou__name', 'name')
+        ]
+
+    def get_additional_filters(self, filter_ids):
+        filters = []
+        if 'service' in filter_ids:
+            filters.append({'id': 'service', 'label': _('Service'), 'options': self.services})
+        if 'services_ou' in filter_ids and len(self.services_ous) > 1:
+            filters.append(
+                {
+                    'id': 'services_ou',
+                    'label': _('Services organizational unit'),
+                    'options': self.services_ous,
+                }
+            )
+        if 'users_ou' in filter_ids and len(self.users_ous) > 1:
+            filters.append(
+                {'id': 'users_ou', 'label': _('Users organizational unit'), 'options': self.users_ous}
+            )
+        return filters
+
     def get_statistics(self, request, klass, method):
         serializer = StatisticsSerializer(data=request.query_params)
         if not serializer.is_valid():
-- 
2.39.2


From 2b3cd08e4259a0befadd0ca4a3f42055e7d8ec46 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Tue, 21 Feb 2023 15:31:51 +0100
Subject: [PATCH 2/5] tests: do not check whole statistics json (#63368)

---
 tests/api/test_all.py | 99 ++++++++++++++++++-------------------------
 1 file changed, 41 insertions(+), 58 deletions(-)

diff --git a/tests/api/test_all.py b/tests/api/test_all.py
index c95c938a6..780f79aa5 100644
--- a/tests/api/test_all.py
+++ b/tests/api/test_all.py
@@ -2713,103 +2713,86 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name):
     Event.objects.create(type=event_type, references=[portal], data=dict(method2, service_name=str(portal)))
 
     resp = app.get('/api/statistics/%s/?time_interval=month' % event_name, headers=headers)
-    data = resp.json['data']
-    assert data == {
-        'x_labels': ['2020-02', '2020-03'],
-        'series': [
-            {'label': 'FranceConnect', 'data': [None, 1]},
-            {'label': 'password', 'data': [2, 1]},
-        ],
-    }
+    assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
+    assert resp.json['data']['series'] == [
+        {'label': 'FranceConnect', 'data': [None, 1]},
+        {'label': 'password', 'data': [2, 1]},
+    ]
 
     # default time interval is 'month'
-    month_data = data
+    month_data = resp.json['data']
     resp = app.get('/api/statistics/%s/' % event_name, headers=headers)
-    data = resp.json['data']
-    assert month_data == data
+    assert month_data == resp.json['data']
 
     resp = app.get(
         '/api/statistics/%s/?time_interval=month&services_ou=default' % event_name, headers=headers
     )
-    data = resp.json['data']
-    assert data == {
-        'x_labels': ['2020-02', '2020-03'],
-        'series': [{'label': 'password', 'data': [1, 1]}],
-    }
+    assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
+    assert resp.json['data']['series'] == [{'label': 'password', 'data': [1, 1]}]
 
     # legacy way to filter by service OU
-    services_ou_data = data
+    services_ou_data = resp.json['data']
     resp = app.get('/api/statistics/%s/?time_interval=month&ou=default' % event_name, headers=headers)
-    data = resp.json['data']
-    assert services_ou_data == data
+    assert services_ou_data == resp.json['data']
 
     resp = app.get(
         '/api/statistics/%s/?time_interval=month&users_ou=default&service=agendas default' % event_name,
         headers=headers,
     )
-    data = resp.json['data']
-    assert data == {
-        'x_labels': ['2020-02'],
-        'series': [{'label': 'password', 'data': [1]}],
-    }
+    assert resp.json['data']['x_labels'] == ['2020-02']
+    assert resp.json['data']['series'] == [{'label': 'password', 'data': [1]}]
 
     resp = app.get('/api/statistics/%s/?time_interval=month&users_ou=default' % event_name, headers=headers)
-    data = resp.json['data']
-    assert data == {'x_labels': ['2020-02'], 'series': [{'label': 'password', 'data': [1]}]}
+    assert resp.json['data']['x_labels'] == ['2020-02']
+    assert resp.json['data']['series'] == [{'label': 'password', 'data': [1]}]
 
     resp = app.get(
         '/api/statistics/%s/?time_interval=month&service=agendas default' % event_name, headers=headers
     )
-    data = resp.json['data']
-    assert data == {'x_labels': ['2020-02', '2020-03'], 'series': [{'label': 'password', 'data': [1, 1]}]}
+    assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
+    assert resp.json['data']['series'] == [{'label': 'password', 'data': [1, 1]}]
 
     resp = app.get(
         '/api/statistics/%s/?time_interval=month&start=2020-03-01T01:01' % event_name, headers=headers
     )
-    data = resp.json['data']
-    assert data == {
-        'x_labels': ['2020-03'],
-        'series': [{'label': 'FranceConnect', 'data': [1]}, {'label': 'password', 'data': [1]}],
-    }
+    assert resp.json['data']['x_labels'] == ['2020-03']
+    assert resp.json['data']['series'] == [
+        {'label': 'FranceConnect', 'data': [1]},
+        {'label': 'password', 'data': [1]},
+    ]
 
     resp = app.get(
         '/api/statistics/%s/?time_interval=month&end=2020-03-01T01:01' % event_name, headers=headers
     )
-    data = resp.json['data']
-    assert data == {'x_labels': ['2020-02'], 'series': [{'label': 'password', 'data': [2]}]}
+    assert resp.json['data']['x_labels'] == ['2020-02']
+    assert resp.json['data']['series'] == [{'label': 'password', 'data': [2]}]
 
     resp = app.get('/api/statistics/%s/?time_interval=month&end=2020-03-01' % event_name, headers=headers)
-    data = resp.json['data']
-    assert data == {'x_labels': ['2020-02'], 'series': [{'label': 'password', 'data': [2]}]}
+    assert resp.json['data']['x_labels'] == ['2020-02']
+    assert resp.json['data']['series'] == [{'label': 'password', 'data': [2]}]
 
     resp = app.get(
         '/api/statistics/%s/?time_interval=year&service=portal second' % event_name, headers=headers
     )
-    data = resp.json['data']
-    assert data == {
-        'x_labels': ['2020'],
-        'series': [{'label': 'FranceConnect', 'data': [1]}, {'label': 'password', 'data': [1]}],
-    }
+    assert resp.json['data']['x_labels'] == ['2020']
+    assert resp.json['data']['series'] == [
+        {'label': 'FranceConnect', 'data': [1]},
+        {'label': 'password', 'data': [1]},
+    ]
 
     resp = app.get('/api/statistics/service_%s/?time_interval=month' % event_name, headers=headers)
-    data = resp.json['data']
-    assert data == {
-        'x_labels': ['2020-02', '2020-03'],
-        'series': [
-            {'data': [1, 1], 'label': 'agendas'},
-            {'data': [1, 1], 'label': 'portal'},
-        ],
-    }
+    assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
+    assert resp.json['data']['series'] == [
+        {'data': [1, 1], 'label': 'agendas'},
+        {'data': [1, 1], 'label': 'portal'},
+    ]
 
     resp = app.get('/api/statistics/service_ou_%s/?time_interval=month' % event_name, headers=headers)
-    data = resp.json['data']
-    assert data == {
-        'x_labels': ['2020-02', '2020-03'],
-        'series': [
-            {'data': [1, 1], 'label': 'Default organizational unit'},
-            {'data': [1, 1], 'label': 'Second OU'},
-        ],
-    }
+    assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
+    assert resp.json['data']['series'] == [
+        {'data': [1, 1], 'label': 'Default organizational unit'},
+        {'data': [1, 1], 'label': 'Second OU'},
+    ]
 
 
 def test_api_statistics_no_crash_older_drf(app, admin):
-- 
2.39.2


From 104b33fb88e8638f96324bd24a439c923163cec8 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Tue, 21 Feb 2023 15:32:38 +0100
Subject: [PATCH 3/5] api: drop legacy OU api filter (#63368)

---
 src/authentic2/api_views.py | 3 +--
 tests/api/test_all.py       | 5 -----
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/src/authentic2/api_views.py b/src/authentic2/api_views.py
index 92b2f9e50..fb6ca3c05 100644
--- a/src/authentic2/api_views.py
+++ b/src/authentic2/api_views.py
@@ -1598,7 +1598,6 @@ class StatisticsSerializer(serializers.Serializer):
     service = ServiceOUField(child=serializers.SlugField(max_length=256), required=False)
     services_ou = serializers.SlugField(required=False, allow_blank=False, max_length=256)
     users_ou = serializers.SlugField(required=False, allow_blank=False, max_length=256)
-    ou = serializers.SlugField(required=False, allow_blank=False, max_length=256)  # legacy
     start = serializers.DateTimeField(required=False, input_formats=['iso-8601', '%Y-%m-%d'])
     end = serializers.DateTimeField(required=False, input_formats=['iso-8601', '%Y-%m-%d'])
 
@@ -1706,7 +1705,7 @@ class StatisticsAPI(ViewSet):
 
         allowed_filters = getattr(self, self.action).filters
         service = data.get('service')
-        services_ou = data.get('services_ou') or data.get('ou')  # legacy 'ou' filter
+        services_ou = data.get('services_ou')
         users_ou = data.get('users_ou')
 
         if service and 'service' in allowed_filters:
diff --git a/tests/api/test_all.py b/tests/api/test_all.py
index 780f79aa5..860bc5a87 100644
--- a/tests/api/test_all.py
+++ b/tests/api/test_all.py
@@ -2730,11 +2730,6 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name):
     assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
     assert resp.json['data']['series'] == [{'label': 'password', 'data': [1, 1]}]
 
-    # legacy way to filter by service OU
-    services_ou_data = resp.json['data']
-    resp = app.get('/api/statistics/%s/?time_interval=month&ou=default' % event_name, headers=headers)
-    assert services_ou_data == resp.json['data']
-
     resp = app.get(
         '/api/statistics/%s/?time_interval=month&users_ou=default&service=agendas default' % event_name,
         headers=headers,
-- 
2.39.2


From 30251c80b1ceb54ad23a723a287e3497cdba5659 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Tue, 21 Feb 2023 10:40:19 +0100
Subject: [PATCH 4/5] api: add cleaner endpoints for statistics (#63368)

---
 src/authentic2/api_views.py |  56 +++++++++++++---
 tests/api/test_all.py       | 123 ++++++++++++++++++++++++++++++++++--
 2 files changed, 166 insertions(+), 13 deletions(-)

diff --git a/src/authentic2/api_views.py b/src/authentic2/api_views.py
index fb6ca3c05..67f5238c4 100644
--- a/src/authentic2/api_views.py
+++ b/src/authentic2/api_views.py
@@ -1593,8 +1593,14 @@ class ServiceOUField(serializers.ListField):
 
 class StatisticsSerializer(serializers.Serializer):
     TIME_INTERVAL_CHOICES = [('day', _('Day')), ('month', _('Month')), ('year', _('Year'))]
+    GROUP_BY_CHOICES = [
+        ('authentication_type', _('Authentication type')),
+        ('service', _('Service')),
+        ('service_ou', _('Organizational unit')),
+    ]
 
     time_interval = serializers.ChoiceField(choices=TIME_INTERVAL_CHOICES, default='month')
+    group_by = serializers.ChoiceField(choices=GROUP_BY_CHOICES, default='authentication_type')
     service = ServiceOUField(child=serializers.SlugField(max_length=256), required=False)
     services_ou = serializers.SlugField(required=False, allow_blank=False, max_length=256)
     users_ou = serializers.SlugField(required=False, allow_blank=False, max_length=256)
@@ -1632,15 +1638,32 @@ class StatisticsAPI(ViewSet):
                 'default': time_interval_field.default,
             }
         ]
+        group_by_field = StatisticsSerializer().get_fields()['group_by']
+        group_by_filter = {
+            'id': 'group_by',
+            'label': _('Group by'),
+            'options': [{'id': key, 'label': label} for key, label in group_by_field.choices.items()],
+            'has_subfilters': True,
+            'required': True,
+            'default': group_by_field.default,
+        }
         for action in self.get_extra_actions():
             url = self.reverse_action(action.url_name)
             filters = common_filters.copy()
+
+            if action.url_name in ('login-new', 'registration-new'):
+                filters.append(group_by_filter)
+                deprecated = False
+            else:
+                deprecated = True
+
             filters.extend(self.get_additional_filters(action.filters))
             data = {
                 'name': action.kwargs['name'],
                 'url': url,
                 'id': action.url_name,
                 'filters': filters,
+                'deprecated': deprecated,
             }
             statistics.append(data)
 
@@ -1690,7 +1713,7 @@ class StatisticsAPI(ViewSet):
             )
         return filters
 
-    def get_statistics(self, request, klass, method):
+    def get_statistics(self, request, klass, method=None):
         serializer = StatisticsSerializer(data=request.query_params)
         if not serializer.is_valid():
             response = {'data': [], 'err': 1, 'err_desc': serializer.errors}
@@ -1703,7 +1726,19 @@ class StatisticsAPI(ViewSet):
             'end': data.get('end'),
         }
 
-        allowed_filters = getattr(self, self.action).filters
+        subfilters = []
+        if not method:
+            method = {
+                'authentication_type': 'get_method_statistics',
+                'service': 'get_service_statistics',
+                'service_ou': 'get_service_ou_statistics',
+            }[data['group_by']]
+
+            if data['group_by'] == 'authentication_type':
+                allowed_filters = ('services_ou', 'users_ou', 'service')
+                subfilters = self.get_additional_filters(allowed_filters)
+        else:
+            allowed_filters = getattr(self, self.action).filters
         service = data.get('service')
         services_ou = data.get('services_ou')
         users_ou = data.get('users_ou')
@@ -1722,12 +1757,17 @@ class StatisticsAPI(ViewSet):
         if users_ou and 'users_ou' in allowed_filters:
             kwargs['users_ou'] = get_object_or_404(OrganizationalUnit, slug=users_ou)
 
-        return Response(
-            {
-                'data': getattr(klass, method)(**kwargs),
-                'err': 0,
-            }
-        )
+        data = getattr(klass, method)(**kwargs)
+        data['subfilters'] = subfilters
+        return Response({'data': data, 'err': 0})
+
+    @stat(name=_('Login count'))
+    def login_new(self, request):
+        return self.get_statistics(request, UserLogin)
+
+    @stat(name=_('Registration count'))
+    def registration_new(self, request):
+        return self.get_statistics(request, UserRegistration)
 
     @stat(name=_('Login count by authentication type'), filters=('services_ou', 'users_ou', 'service'))
     def login(self, request):
diff --git a/tests/api/test_all.py b/tests/api/test_all.py
index 860bc5a87..b35159f85 100644
--- a/tests/api/test_all.py
+++ b/tests/api/test_all.py
@@ -2592,7 +2592,7 @@ def test_api_users_delete(settings, app, admin, simple_user):
 def test_api_statistics_list(app, admin):
     headers = basic_authorization_header(admin)
     resp = app.get('/api/statistics/', headers=headers)
-    assert len(resp.json['data']) == 6
+    assert len(resp.json['data']) == 8
     login_stats = {
         'name': 'Login count by authentication type',
         'url': 'https://testserver/api/statistics/login/',
@@ -2611,6 +2611,7 @@ def test_api_statistics_list(app, admin):
             },
             {'id': 'service', 'label': 'Service', 'options': []},
         ],
+        'deprecated': True,
     }
     assert login_stats in resp.json['data']
     assert {
@@ -2630,6 +2631,7 @@ def test_api_statistics_list(app, admin):
                 "default": "month",
             },
         ],
+        'deprecated': True,
     } in resp.json['data']
 
     service = Service.objects.create(name='Service1', slug='service1', ou=get_default_ou())
@@ -2678,14 +2680,117 @@ def test_api_statistics_list(app, admin):
     assert login_stats in resp.json['data']
 
 
+def test_api_statistics_list_new(app, admin):
+    headers = basic_authorization_header(admin)
+    resp = app.get('/api/statistics/', headers=headers)
+    login_stat = [x for x in resp.json['data'] if x['id'] == 'login-new'][0]
+    assert login_stat == {
+        'name': 'Login count',
+        'url': 'https://testserver/api/statistics/login_new/',
+        'id': 'login-new',
+        'filters': [
+            {
+                'id': 'time_interval',
+                'label': 'Time interval',
+                'options': [
+                    {'id': 'day', 'label': 'Day'},
+                    {'id': 'month', 'label': 'Month'},
+                    {'id': 'year', 'label': 'Year'},
+                ],
+                'required': True,
+                'default': 'month',
+            },
+            {
+                'id': 'group_by',
+                'label': 'Group by',
+                'options': [
+                    {'id': 'authentication_type', 'label': 'Authentication type'},
+                    {'id': 'service', 'label': 'Service'},
+                    {'id': 'service_ou', 'label': 'Organizational unit'},
+                ],
+                'has_subfilters': True,
+                'required': True,
+                'default': 'authentication_type',
+            },
+        ],
+        'deprecated': False,
+    }
+
+    registration_stat = [x for x in resp.json['data'] if x['id'] == 'registration-new'][0]
+    assert registration_stat['name'] == 'Registration count'
+    assert registration_stat['filters'] == login_stat['filters']
+
+
+@pytest.mark.parametrize('endpoint', ['login_new', 'registration_new'])
+def test_api_statistics_subfilters(app, admin, endpoint):
+    service = Service.objects.create(name='Service1', slug='service1', ou=get_default_ou())
+    service = Service.objects.create(name='Service2', slug='service2', ou=get_default_ou())
+
+    headers = basic_authorization_header(admin)
+    resp = app.get('/api/statistics/%s/' % endpoint, headers=headers)
+    assert len(resp.json['data']['subfilters']) == 1
+    assert resp.json['data']['subfilters'][0] == {
+        'id': 'service',
+        'label': 'Service',
+        'options': [
+            {'id': 'service1 default', 'label': 'Service1'},
+            {'id': 'service2 default', 'label': 'Service2'},
+        ],
+    }
+
+    # adding second ou doesn't change anything
+    ou = OU.objects.create(name='Second OU', slug='second')
+    new_resp = app.get('/api/statistics/%s/' % endpoint, headers=headers)
+    assert new_resp.json == resp.json
+
+    # if there are services in two differents OUs, filter is shown
+    service.ou = ou
+    service.save()
+    resp = app.get('/api/statistics/%s/' % endpoint, headers=headers)
+    assert len(resp.json['data']['subfilters']) == 2
+    assert resp.json['data']['subfilters'][1] == {
+        'id': 'services_ou',
+        'label': 'Services organizational unit',
+        'options': [
+            {'id': 'default', 'label': 'Default organizational unit'},
+            {'id': 'second', 'label': 'Second OU'},
+        ],
+    }
+
+    # same goes with users
+    User.objects.create(username='john.doe', email='john.doe@example.com', ou=ou)
+    resp = app.get('/api/statistics/%s/' % endpoint, headers=headers)
+    assert len(resp.json['data']['subfilters']) == 3
+    assert resp.json['data']['subfilters'][2] == {
+        'id': 'users_ou',
+        'label': 'Users organizational unit',
+        'options': [
+            {'id': 'default', 'label': 'Default organizational unit'},
+            {'id': 'second', 'label': 'Second OU'},
+        ],
+    }
+
+    resp = app.get('/api/statistics/%s/?group_by=service' % endpoint, headers=headers)
+    assert len(resp.json['data']['subfilters']) == 0
+
+    resp = app.get('/api/statistics/%s/?group_by=service_ou' % endpoint, headers=headers)
+    assert len(resp.json['data']['subfilters']) == 0
+
+
 @pytest.mark.parametrize(
-    'event_type_name,event_name', [('user.login', 'login'), ('user.registration', 'registration')]
+    'event_type_name,event_name',
+    [
+        ('user.login', 'login'),
+        ('user.registration', 'registration'),
+        ('user.login', 'login_new'),
+        ('user.registration', 'registration_new'),
+    ],
 )
 def test_api_statistics(app, admin, freezer, event_type_name, event_name):
     headers = basic_authorization_header(admin)
 
     resp = app.get('/api/statistics/login/?time_interval=month', headers=headers)
-    assert resp.json == {"data": {"series": [], "x_labels": []}, "err": 0}
+    assert resp.json == {"data": {"series": [], "x_labels": [], "subfilters": []}, "err": 0}
 
     user = User.objects.create(username='john.doe', email='john.doe@example.com', ou=get_default_ou())
     ou = OU.objects.create(name='Second OU', slug='second')
@@ -2775,14 +2880,22 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name):
         {'label': 'password', 'data': [1]},
     ]
 
-    resp = app.get('/api/statistics/service_%s/?time_interval=month' % event_name, headers=headers)
+    if 'new' in event_name:
+        url = '/api/statistics/%s/?time_interval=month&group_by=service' % event_name
+    else:
+        url = '/api/statistics/service_%s/?time_interval=month' % event_name
+    resp = app.get(url, headers=headers)
     assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
     assert resp.json['data']['series'] == [
         {'data': [1, 1], 'label': 'agendas'},
         {'data': [1, 1], 'label': 'portal'},
     ]
 
-    resp = app.get('/api/statistics/service_ou_%s/?time_interval=month' % event_name, headers=headers)
+    if 'new' in event_name:
+        url = '/api/statistics/%s/?time_interval=month&group_by=service_ou' % event_name
+    else:
+        url = '/api/statistics/service_ou_%s/?time_interval=month' % event_name
+    resp = app.get(url, headers=headers)
     assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
     assert resp.json['data']['series'] == [
         {'data': [1, 1], 'label': 'Default organizational unit'},
-- 
2.39.2


From e3ba72b7bd3d68bdea8f8c36bd02a3b1d73edcb6 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Thu, 23 Feb 2023 15:12:04 +0100
Subject: [PATCH 5/5] api: add global login and registration counts to
 statistics (#63368)

---
 src/authentic2/api_views.py           |  9 ++--
 src/authentic2/apps/journal/models.py | 11 +++++
 tests/api/test_all.py                 | 69 +++++++++++++++------------
 3 files changed, 56 insertions(+), 33 deletions(-)

diff --git a/src/authentic2/api_views.py b/src/authentic2/api_views.py
index 67f5238c4..1800998b3 100644
--- a/src/authentic2/api_views.py
+++ b/src/authentic2/api_views.py
@@ -1600,7 +1600,7 @@ class StatisticsSerializer(serializers.Serializer):
     ]
 
     time_interval = serializers.ChoiceField(choices=TIME_INTERVAL_CHOICES, default='month')
-    group_by = serializers.ChoiceField(choices=GROUP_BY_CHOICES, default='authentication_type')
+    group_by = serializers.ChoiceField(choices=GROUP_BY_CHOICES, default='global')
     service = ServiceOUField(child=serializers.SlugField(max_length=256), required=False)
     services_ou = serializers.SlugField(required=False, allow_blank=False, max_length=256)
     users_ou = serializers.SlugField(required=False, allow_blank=False, max_length=256)
@@ -1611,11 +1611,13 @@ class StatisticsSerializer(serializers.Serializer):
 def stat(**kwargs):
     '''Extend action decorator to allow passing statistics related info.'''
     filters = kwargs.pop('filters', [])
+    name = kwargs['name']
     kwargs['detail'] = False
     decorator = action(**kwargs)
 
     def wraps(func):
         func.filters = filters
+        func.name = name
         return decorator(func)
 
     return wraps
@@ -1644,8 +1646,6 @@ class StatisticsAPI(ViewSet):
             'label': _('Group by'),
             'options': [{'id': key, 'label': label} for key, label in group_by_field.choices.items()],
             'has_subfilters': True,
-            'required': True,
-            'default': group_by_field.default,
         }
         for action in self.get_extra_actions():
             url = self.reverse_action(action.url_name)
@@ -1729,6 +1729,7 @@ class StatisticsAPI(ViewSet):
         subfilters = []
         if not method:
             method = {
+                'global': 'get_global_statistics',
                 'authentication_type': 'get_method_statistics',
                 'service': 'get_service_statistics',
                 'service_ou': 'get_service_ou_statistics',
@@ -1737,6 +1738,8 @@ class StatisticsAPI(ViewSet):
             if data['group_by'] == 'authentication_type':
                 allowed_filters = ('services_ou', 'users_ou', 'service')
                 subfilters = self.get_additional_filters(allowed_filters)
+            elif data['group_by'] == 'global':
+                kwargs['y_label'] = getattr(self, self.action).name
         else:
             allowed_filters = getattr(self, self.action).filters
         service = data.get('service')
diff --git a/src/authentic2/apps/journal/models.py b/src/authentic2/apps/journal/models.py
index 6b0f95001..77e27d13c 100644
--- a/src/authentic2/apps/journal/models.py
+++ b/src/authentic2/apps/journal/models.py
@@ -38,6 +38,7 @@ from django.utils.translation import gettext_lazy as _
 from authentic2.utils.cache import GlobalCache
 
 from . import sql
+from .utils import Statistics
 
 if django.VERSION < (3, 1):
     from django.contrib.postgres.fields.jsonb import JSONField  # noqa pylint: disable=ungrouped-imports
@@ -163,6 +164,16 @@ class EventTypeDefinition(metaclass=EventTypeDefinitionMeta):
         qs = qs.annotate(count=Count('id'))
         return qs.order_by(group_by_time)
 
+    @classmethod
+    def get_global_statistics(cls, group_by_time, y_label, start=None, end=None):
+        qs = cls.get_statistics(group_by_time=group_by_time, start=start, end=end)
+        stats = Statistics(qs, time_interval=group_by_time)
+
+        for stat in qs:
+            stats.add(x_label=stat[group_by_time], y_label=y_label, value=stat['count'])
+
+        return stats.to_json()
+
     def __repr__(self):
         return '<EventTypeDefinition %r %s>' % (self.name, self.label)
 
diff --git a/tests/api/test_all.py b/tests/api/test_all.py
index b35159f85..bcc2e0992 100644
--- a/tests/api/test_all.py
+++ b/tests/api/test_all.py
@@ -2709,8 +2709,6 @@ def test_api_statistics_list_new(app, admin):
                     {'id': 'service_ou', 'label': 'Organizational unit'},
                 ],
                 'has_subfilters': True,
-                'required': True,
-                'default': 'authentication_type',
             },
         ],
         'deprecated': False,
@@ -2728,6 +2726,9 @@ def test_api_statistics_subfilters(app, admin, endpoint):
 
     headers = basic_authorization_header(admin)
     resp = app.get('/api/statistics/%s/' % endpoint, headers=headers)
+    assert len(resp.json['data']['subfilters']) == 0
+
+    resp = app.get('/api/statistics/%s/?group_by=authentication_type' % endpoint, headers=headers)
     assert len(resp.json['data']['subfilters']) == 1
     assert resp.json['data']['subfilters'][0] == {
         'id': 'service',
@@ -2740,13 +2741,13 @@ def test_api_statistics_subfilters(app, admin, endpoint):
 
     # adding second ou doesn't change anything
     ou = OU.objects.create(name='Second OU', slug='second')
-    new_resp = app.get('/api/statistics/%s/' % endpoint, headers=headers)
+    new_resp = app.get('/api/statistics/%s/?group_by=authentication_type' % endpoint, headers=headers)
     assert new_resp.json == resp.json
 
     # if there are services in two differents OUs, filter is shown
     service.ou = ou
     service.save()
-    resp = app.get('/api/statistics/%s/' % endpoint, headers=headers)
+    resp = app.get('/api/statistics/%s/?group_by=authentication_type' % endpoint, headers=headers)
     assert len(resp.json['data']['subfilters']) == 2
     assert resp.json['data']['subfilters'][1] == {
         'id': 'services_ou',
@@ -2759,7 +2760,7 @@ def test_api_statistics_subfilters(app, admin, endpoint):
 
     # same goes with users
     User.objects.create(username='john.doe', email='john.doe@example.com', ou=ou)
-    resp = app.get('/api/statistics/%s/' % endpoint, headers=headers)
+    resp = app.get('/api/statistics/%s/?group_by=authentication_type' % endpoint, headers=headers)
     assert len(resp.json['data']['subfilters']) == 3
     assert resp.json['data']['subfilters'][2] == {
         'id': 'users_ou',
@@ -2776,6 +2777,9 @@ def test_api_statistics_subfilters(app, admin, endpoint):
     resp = app.get('/api/statistics/%s/?group_by=service_ou' % endpoint, headers=headers)
     assert len(resp.json['data']['subfilters']) == 0
 
+    resp = app.get('/api/statistics/%s/' % endpoint, headers=headers)
+    assert len(resp.json['data']['subfilters']) == 0
+
 
 @pytest.mark.parametrize(
     'event_type_name,event_name',
@@ -2817,7 +2821,12 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name):
     )
     Event.objects.create(type=event_type, references=[portal], data=dict(method2, service_name=str(portal)))
 
-    resp = app.get('/api/statistics/%s/?time_interval=month' % event_name, headers=headers)
+    params = {}
+    if 'new' in event_name:
+        params['group_by'] = 'authentication_type'
+
+    url = '/api/statistics/%s/' % event_name
+    resp = app.get(url, headers=headers, params=params)
     assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
     assert resp.json['data']['series'] == [
         {'label': 'FranceConnect', 'data': [None, 1]},
@@ -2826,53 +2835,44 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name):
 
     # default time interval is 'month'
     month_data = resp.json['data']
-    resp = app.get('/api/statistics/%s/' % event_name, headers=headers)
+    resp = app.get(url, headers=headers, params=params)
     assert month_data == resp.json['data']
 
-    resp = app.get(
-        '/api/statistics/%s/?time_interval=month&services_ou=default' % event_name, headers=headers
-    )
+    resp = app.get(url, headers=headers, params={'services_ou': 'default', **params})
     assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
     assert resp.json['data']['series'] == [{'label': 'password', 'data': [1, 1]}]
 
     resp = app.get(
-        '/api/statistics/%s/?time_interval=month&users_ou=default&service=agendas default' % event_name,
-        headers=headers,
+        url, headers=headers, params={'service': 'agendas default', 'users_ou': 'default', **params}
     )
     assert resp.json['data']['x_labels'] == ['2020-02']
     assert resp.json['data']['series'] == [{'label': 'password', 'data': [1]}]
 
-    resp = app.get('/api/statistics/%s/?time_interval=month&users_ou=default' % event_name, headers=headers)
+    resp = app.get(url, headers=headers, params={'users_ou': 'default', **params})
     assert resp.json['data']['x_labels'] == ['2020-02']
     assert resp.json['data']['series'] == [{'label': 'password', 'data': [1]}]
 
-    resp = app.get(
-        '/api/statistics/%s/?time_interval=month&service=agendas default' % event_name, headers=headers
-    )
+    resp = app.get(url, headers=headers, params={'service': 'agendas default', **params})
     assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
     assert resp.json['data']['series'] == [{'label': 'password', 'data': [1, 1]}]
 
-    resp = app.get(
-        '/api/statistics/%s/?time_interval=month&start=2020-03-01T01:01' % event_name, headers=headers
-    )
+    resp = app.get(url, headers=headers, params={'start': '2020-03-01T01:01', **params})
     assert resp.json['data']['x_labels'] == ['2020-03']
     assert resp.json['data']['series'] == [
         {'label': 'FranceConnect', 'data': [1]},
         {'label': 'password', 'data': [1]},
     ]
 
-    resp = app.get(
-        '/api/statistics/%s/?time_interval=month&end=2020-03-01T01:01' % event_name, headers=headers
-    )
+    resp = app.get(url, headers=headers, params={'end': '2020-03-01T01:01', **params})
     assert resp.json['data']['x_labels'] == ['2020-02']
     assert resp.json['data']['series'] == [{'label': 'password', 'data': [2]}]
 
-    resp = app.get('/api/statistics/%s/?time_interval=month&end=2020-03-01' % event_name, headers=headers)
+    resp = app.get(url, headers=headers, params={'end': '2020-03-01', **params})
     assert resp.json['data']['x_labels'] == ['2020-02']
     assert resp.json['data']['series'] == [{'label': 'password', 'data': [2]}]
 
     resp = app.get(
-        '/api/statistics/%s/?time_interval=year&service=portal second' % event_name, headers=headers
+        url, headers=headers, params={'time_interval': 'year', 'service': 'portal second', **params}
     )
     assert resp.json['data']['x_labels'] == ['2020']
     assert resp.json['data']['series'] == [
@@ -2881,10 +2881,10 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name):
     ]
 
     if 'new' in event_name:
-        url = '/api/statistics/%s/?time_interval=month&group_by=service' % event_name
+        params['group_by'] = 'service'
     else:
-        url = '/api/statistics/service_%s/?time_interval=month' % event_name
-    resp = app.get(url, headers=headers)
+        url = '/api/statistics/service_%s/' % event_name
+    resp = app.get(url, headers=headers, params=params)
     assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
     assert resp.json['data']['series'] == [
         {'data': [1, 1], 'label': 'agendas'},
@@ -2892,16 +2892,25 @@ def test_api_statistics(app, admin, freezer, event_type_name, event_name):
     ]
 
     if 'new' in event_name:
-        url = '/api/statistics/%s/?time_interval=month&group_by=service_ou' % event_name
+        params['group_by'] = 'service_ou'
     else:
-        url = '/api/statistics/service_ou_%s/?time_interval=month' % event_name
-    resp = app.get(url, headers=headers)
+        url = '/api/statistics/service_ou_%s/' % event_name
+    resp = app.get(url, headers=headers, params=params)
     assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
     assert resp.json['data']['series'] == [
         {'data': [1, 1], 'label': 'Default organizational unit'},
         {'data': [1, 1], 'label': 'Second OU'},
     ]
 
+    if 'new' in event_name:
+        del params['group_by']
+
+        resp = app.get(url, headers=headers, params=params)
+        assert resp.json['data']['x_labels'] == ['2020-02', '2020-03']
+        assert len(resp.json['data']['series']) == 1
+        assert resp.json['data']['series'][0]['data'] == [2, 2]
+        assert 'count' in resp.json['data']['series'][0]['label']
+
 
 def test_api_statistics_no_crash_older_drf(app, admin):
     headers = basic_authorization_header(admin)
-- 
2.39.2