From 162b2e34ec3fca3bff108292965e891ae8166c1b Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Thu, 25 Feb 2021 17:44:58 +0100
Subject: [PATCH 3/3] api_views: only show filtering by OUs if relevant
 (#49670)

---
 src/authentic2/api_views.py | 16 +++++++-----
 tests/test_api.py           | 52 ++++++++++++++++++++++++++++---------
 2 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/src/authentic2/api_views.py b/src/authentic2/api_views.py
index 131733f2..fa2162d4 100644
--- a/src/authentic2/api_views.py
+++ b/src/authentic2/api_views.py
@@ -1154,7 +1154,9 @@ class StatisticsAPI(ViewSet):
 
     def list(self, request):
         statistics = []
-        ous = [{'id': ou.slug, 'label': ou.name} for ou in get_ou_model().objects.all()]
+        OU = get_ou_model()
+        services_ous = [{'id': ou.slug, 'label': ou.name} for ou in OU.objects.exclude(service__isnull=True)]
+        users_ous = [{'id': ou.slug, 'label': ou.name} for ou in OU.objects.exclude(user__isnull=True)]
         services = [
             {'id': '%s %s' % (service['slug'], service['ou__slug']), 'label': service['name']}
             for service in Service.objects.values('slug', 'name', 'ou__slug')
@@ -1173,16 +1175,16 @@ class StatisticsAPI(ViewSet):
         for action in self.get_extra_actions():
             url = self.reverse_action(action.url_name)
             filters = common_filters.copy()
-            if 'services_ou' in action.filters:
+            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': ous}
+                    {'id': 'services_ou', 'label': _('Services organizational unit'), 'options': services_ous}
                 )
-            if 'users_ou' in action.filters:
+            if 'users_ou' in action.filters and len(users_ous) > 1:
                 filters.append(
-                    {'id': 'users_ou', 'label': _('Users organizational unit'), 'options': ous}
+                    {'id': 'users_ou', 'label': _('Users organizational unit'), 'options': users_ous}
                 )
-            if 'service' in action.filters:
-                filters.append({'id': 'service', 'label': _('Service'), 'options': services})
             data = {
                 'name': action.kwargs['name'],
                 'url': url,
diff --git a/tests/test_api.py b/tests/test_api.py
index 7f1267dd..a58ca9b7 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -2032,6 +2032,7 @@ def test_api_password_change_mark_as_deleted(app, settings, admin, ou1):
 
 @pytest.mark.skipif(drf_version.startswith('3.4'), reason='no support for old django rest framework')
 def test_api_statistics_list(app, admin):
+    OU = get_ou_model()
     headers = basic_authorization_header(admin)
     resp = app.get('/api/statistics/', headers=headers)
     assert len(resp.json['data']) == 6
@@ -2051,16 +2052,6 @@ def test_api_statistics_list(app, admin):
                 "required": True,
                 "default": "month",
             },
-            {
-                'id': 'services_ou',
-                'label': 'Services organizational unit',
-                'options': [{'id': 'default', 'label': 'Default organizational unit'}],
-            },
-            {
-                'id': 'users_ou',
-                'label': 'Users organizational unit',
-                'options': [{'id': 'default', 'label': 'Default organizational unit'}],
-            },
             {'id': 'service', 'label': 'Service', 'options': []},
         ],
     }
@@ -2086,9 +2077,46 @@ def test_api_statistics_list(app, admin):
 
     service = Service.objects.create(name='Service1', slug='service1', ou=get_default_ou())
     service = Service.objects.create(name='Service2', slug='service2', ou=get_default_ou())
-    login_stats['filters'][3]['options'].append({'id': 'service1 default', 'label': 'Service1'})
-    login_stats['filters'][3]['options'].append({'id': 'service2 default', 'label': 'Service2'})
+    login_stats['filters'][1]['options'].append({'id': 'service1 default', 'label': 'Service1'})
+    login_stats['filters'][1]['options'].append({'id': 'service2 default', 'label': 'Service2'})
+
+    resp = app.get('/api/statistics/', headers=headers)
+    assert login_stats in resp.json['data']
 
+    # adding second ou doesn't change anything
+    ou = OU.objects.create(name='Second OU', slug='second')
+    resp = app.get('/api/statistics/', headers=headers)
+    assert login_stats in resp.json['data']
+
+    # if there are services in two differents OUs, filter is shown
+    service.ou = ou
+    service.save()
+    login_stats['filters'][1]['options'][1]['id'] = 'service2 second'
+    login_stats['filters'].append(
+            {
+                'id': 'services_ou',
+                'label': 'Services organizational unit',
+                'options': [
+                    {'id': 'default', 'label': 'Default organizational unit'},
+                    {'id': 'second', 'label': 'Second OU'}
+                ],
+            }
+    )
+    resp = app.get('/api/statistics/', headers=headers)
+    assert login_stats in resp.json['data']
+
+    # same goes with users
+    user = User.objects.create(username='john.doe', email='john.doe@example.com', ou=ou)
+    login_stats['filters'].append(
+            {
+                'id': 'users_ou',
+                'label': 'Users organizational unit',
+                'options': [
+                    {'id': 'default', 'label': 'Default organizational unit'},
+                    {'id': 'second', 'label': 'Second OU'}
+                ],
+            }
+    )
     resp = app.get('/api/statistics/', headers=headers)
     assert login_stats in resp.json['data']
 
-- 
2.20.1