From 4568c0e5abf26dbac89053c9f91ef02e6e5937f1 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 12 Apr 2016 11:42:45 +0100
Subject: [PATCH] Sort template stats by usage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Template stats should show the most-used template first.

This commit:
- re-writes the `aggregate_usage` function to use `itertools.groupby`,
  which can do aggregation, and can return data in a structure that’s
  easy to sort on
- uses generators so that we’re not keeping lots of rows of template
  stats in memory

https://www.pivotaltracker.com/story/show/117348893
---
 app/main/views/dashboard.py            | 38 ++++++++++++++++++++------
 tests/app/main/views/test_dashboard.py |  8 +++---
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py
index 0130d7dc5..8e59e61aa 100644
--- a/app/main/views/dashboard.py
+++ b/app/main/views/dashboard.py
@@ -1,4 +1,6 @@
 from datetime import date
+from collections import namedtuple
+from itertools import groupby
 
 from flask import (
     render_template,
@@ -90,12 +92,30 @@ def add_rates_to(delivery_statistics):
 
 
 def aggregate_usage(template_statistics):
-    import collections
-    stats = collections.OrderedDict()
-    for item in template_statistics:
-        stat = stats.get(item['template']['id'])
-        if stat:
-            stat['usage_count'] = stat['usage_count'] + item['usage_count']
-        else:
-            stats[item['template']['id']] = item
-    return stats.values()
+
+    immutable_template = namedtuple('Template', ['template_type', 'name', 'id'])
+
+    # grouby requires the list to be sorted by template first
+    statistics_sorted_by_template = sorted(
+        (
+            (
+                immutable_template(**row['template']),
+                row['usage_count']
+            )
+            for row in template_statistics
+        ),
+        key=lambda items: items[0]
+    )
+
+    # then group and sort the result by usage
+    return sorted(
+        (
+            {
+                'usage_count': sum(usage[1] for usage in usages),
+                'template': template
+            }
+            for template, usages in groupby(statistics_sorted_by_template, lambda items: items[0])
+        ),
+        key=lambda row: row['usage_count'],
+        reverse=True
+    )
diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py
index d2d066298..0c4acc625 100644
--- a/tests/app/main/views/test_dashboard.py
+++ b/tests/app/main/views/test_dashboard.py
@@ -93,12 +93,12 @@ def test_should_show_recent_templates_on_dashboard(app_,
         first_row = page.tbody.find_all('tr')[0]
         table_data = first_row.find_all('td')
         assert len(table_data) == 3
-        assert table_data[2].text.strip() == '13'
+        assert table_data[2].text.strip() == '206'
 
         second_row = page.tbody.find_all('tr')[1]
         table_data = second_row.find_all('td')
         assert len(table_data) == 3
-        assert table_data[2].text.strip() == '206'
+        assert table_data[2].text.strip() == '13'
 
 
 def _test_dashboard_menu(mocker, app_, usr, service, permissions):
@@ -270,7 +270,7 @@ def test_aggregate_template_stats():
 
     assert len(expected) == 2
     for item in expected:
-        if item['template']['id'] == 1:
+        if item['template'].id == 1:
             assert item['usage_count'] == 13
-        elif item['template']['id'] == 2:
+        elif item['template'].id == 2:
             assert item['usage_count'] == 206