From 77aa87af069da05b267b7f26518a907ca3e79b15 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Thu, 8 Dec 2022 15:23:22 +0100
Subject: [PATCH] statistics: improve label sorting on group by (#71656)

---
 tests/api/test_statistics.py | 17 +++++++++++++++--
 wcs/statistics/views.py      | 28 +++++++++++++++++++++++-----
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/tests/api/test_statistics.py b/tests/api/test_statistics.py
index 0b71201e4..bcaff4255 100644
--- a/tests/api/test_statistics.py
+++ b/tests/api/test_statistics.py
@@ -738,8 +738,9 @@ def test_statistics_forms_count_group_by(pub, formdef, anonymise):
 
     # group by item field without time interval
     resp = get_app(pub).get(sign_uri(url + '&group-by=test-item&time_interval=none'))
-    assert resp.json['data']['x_labels'] == ['baz', 'Foo', 'None']
-    assert resp.json['data']['series'] == [{'data': [3, 13, 4], 'label': 'Forms Count'}]
+    # Foo is first because it has a display value, baz is second because it has not, None is always last
+    assert resp.json['data']['x_labels'] == ['Foo', 'baz', 'None']
+    assert resp.json['data']['series'] == [{'data': [13, 3, 4], 'label': 'Forms Count'}]
 
     # group by items field without time interval
     resp = get_app(pub).get(sign_uri(url + '&group-by=test-items&time_interval=none'))
@@ -751,6 +752,18 @@ def test_statistics_forms_count_group_by(pub, formdef, anonymise):
     assert resp.json['data']['x_labels'] == ['Mail', 'Web']
     assert resp.json['data']['series'] == [{'data': [3, 17], 'label': 'Forms Count'}]
 
+    # group by status without time interval
+    resp = get_app(pub).get(sign_uri(url + '&group-by=status&time_interval=none'))
+    assert resp.json['data']['x_labels'] == ['New status', 'End status']
+    assert resp.json['data']['series'] == [{'data': [17, 3], 'label': 'Forms Count'}]
+
+    # check statuses order
+    formdef.workflow.possible_status = list(reversed(formdef.workflow.possible_status))
+    formdef.workflow.store()
+    resp = get_app(pub).get(sign_uri(url + '&group-by=status&time_interval=none'))
+    assert resp.json['data']['x_labels'] == ['End status', 'New status']
+    assert resp.json['data']['series'] == [{'data': [3, 17], 'label': 'Forms Count'}]
+
     # group by on block field is not supported
     resp = get_app(pub).get(sign_uri(url + '&group-by=blockdata_bool'))
     assert resp.json['data']['series'] == [{'data': [16, 0, 4], 'label': 'Forms Count'}]
diff --git a/wcs/statistics/views.py b/wcs/statistics/views.py
index 4610691cb..3c86b1e5a 100644
--- a/wcs/statistics/views.py
+++ b/wcs/statistics/views.py
@@ -457,10 +457,10 @@ class FormsCountView(RestrictedView):
         for group in seen_group_values:
             totals_by_group[group] = [totals.get(group) for totals in totals_by_time.values()]
 
+        totals_by_label = self.get_totals_by_label(totals_by_group, group_labels)
+
         x_labels = list(totals_by_time)
-        series = [
-            {'label': group_labels.get(group, group), 'data': data} for group, data in totals_by_group.items()
-        ]
+        series = [{'label': label, 'data': data} for label, data in totals_by_label.items()]
         series.sort(key=lambda x: x['label'].lower())
         return x_labels, series
 
@@ -472,10 +472,28 @@ class FormsCountView(RestrictedView):
             for group in groups:
                 totals_by_group[group] += total
 
-        x_labels = [group_labels.get(group, group) for group in totals_by_group]
-        series = [{'label': self.label, 'data': [total for total in totals_by_group.values()]}]
+        totals_by_label = self.get_totals_by_label(totals_by_group, group_labels)
+
+        x_labels = list(totals_by_label)
+        series = [{'label': self.label, 'data': [total for total in totals_by_label.values()]}]
         return x_labels, series
 
+    def get_totals_by_label(self, totals_by_group, group_labels):
+        groups = list(totals_by_group)
+        group_label_indexes = {group: i for i, group in enumerate(group_labels)}
+
+        def get_group_order(group):
+            if group is None:
+                # None choice should always be last
+                return len(group_label_indexes) + 1
+            if group not in group_label_indexes:
+                # unknown group should be last but before none
+                return len(group_label_indexes)
+            return group_label_indexes[group]
+
+        groups.sort(key=get_group_order)
+        return {group_labels.get(group, group): totals_by_group[group] for group in groups}
+
 
 class CardsCountView(FormsCountView):
     formdef_class = CardDef
-- 
2.35.1