wip/63368-Clarifier-les-statistiques (#63368) #1

Merged
vdeniaud merged 5 commits from wip/63368-Clarifier-les-statistiques into main 2023-02-27 09:44:51 +01:00
3 changed files with 292 additions and 121 deletions

View File

@ -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 _
@ -1592,12 +1593,17 @@ 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='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)
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'])
@ -1605,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
@ -1620,19 +1628,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 = [
{
@ -1645,24 +1640,30 @@ 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,
}
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}
)
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)
@ -1673,7 +1674,46 @@ class StatisticsAPI(ViewSet):
}
)
def get_statistics(self, request, klass, method):
@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=None):
serializer = StatisticsSerializer(data=request.query_params)
if not serializer.is_valid():
response = {'data': [], 'err': 1, 'err_desc': serializer.errors}
@ -1686,9 +1726,24 @@ class StatisticsAPI(ViewSet):
'end': data.get('end'),
}
allowed_filters = getattr(self, self.action).filters
subfilters = []
if not method:
vdeniaud marked this conversation as resolved Outdated

Ici il y a du code posé dans 0004 et qui est modifié dans 0005 sans que ce soit lié à l’objet du commit, il me semble. Peut-être tout déplacer directement dans 0004 ?

Ici il y a du code posé dans 0004 et qui est modifié dans 0005 sans que ce soit lié à l’objet du commit, il me semble. Peut-être tout déplacer directement dans 0004 ?
method = {
'global': 'get_global_statistics',
'authentication_type': 'get_method_statistics',

Ici question de lisibilité du code, j’ai pas capté tout de suite à la relecture qu’il y deux method, la méthode d’authentification qui est l’une des options possibles pour la méthode de regroupement. Peut-être mettre authn_method pour nommer la première.

Ici question de lisibilité du code, j’ai pas capté tout de suite à la relecture qu’il y deux `method`, la méthode d’authentification qui est l’une des options possibles pour la méthode de regroupement. Peut-être mettre `authn_method` pour nommer la première.

Renommé en « authentication_type » ce qui correspond à la valeur verbeuse du choix en question.

Renommé en « authentication_type » ce qui correspond à la valeur verbeuse du choix en question.
'service': 'get_service_statistics',
'service_ou': 'get_service_ou_statistics',
}[data['group_by']]
if data['group_by'] == 'authentication_type':

Il me semble qu’il n’y a rien dans le ChoiceField DRF qui impose que la valeur par défaut fasse partie des choix possibles. Alors peut-être qu’on pourrait directement inclure ce 'get_global_statistics' comme valeur par défaut à la définition du champ StatisticsSerializer.group_by (et du coup le required=False est implicite, pas besoin de le préciser) ?

Il me semble qu’il n’y a rien dans le ChoiceField DRF qui impose que la valeur par défaut fasse partie des choix possibles. Alors peut-être qu’on pourrait directement inclure ce `'get_global_statistics'` comme valeur par défaut à la définition du champ `StatisticsSerializer.group_by` (et du coup le required=False est implicite, pas besoin de le préciser) ?

Bonne idée, merci pour le niveau d'indentation en moins !

Bonne idée, merci pour le niveau d'indentation en moins !
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')
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:
@ -1705,12 +1760,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):

View File

@ -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)

View File

@ -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,121 @@ 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,
},
],
'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']) == 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',
'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/?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/?group_by=authentication_type' % 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/?group_by=authentication_type' % 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
resp = app.get('/api/statistics/%s/' % 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')
@ -2712,104 +2821,95 @@ 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]},
],
}
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]},
{'label': 'password', 'data': [2, 1]},
]
# default time interval is 'month'
month_data = data
resp = app.get('/api/statistics/%s/' % event_name, headers=headers)
data = resp.json['data']
assert month_data == data
month_data = resp.json['data']
resp = app.get(url, headers=headers, params=params)
assert month_data == resp.json['data']
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&services_ou=default' % event_name, headers=headers
url, headers=headers, params={'service': 'agendas default', 'users_ou': 'default', **params}
)
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']
assert resp.json['data']['series'] == [{'label': 'password', 'data': [1]}]
# legacy way to filter by service OU
services_ou_data = 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
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(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(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(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(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=month&users_ou=default&service=agendas default' % event_name,
headers=headers,
url, headers=headers, params={'time_interval': 'year', 'service': 'portal second', **params}
)
data = resp.json['data']
assert data == {
'x_labels': ['2020-02'],
'series': [{'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/%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]}]}
if 'new' in event_name:
params['group_by'] = 'service'
else:
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'},
{'data': [1, 1], 'label': 'portal'},
]
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]}]}
if 'new' in event_name:
params['group_by'] = 'service_ou'
else:
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'},
]
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]}],
}
if 'new' in event_name:
del params['group_by']
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]}]}
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]}]}
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]}],
}
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'},
],
}
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'},
],
}
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):