Merge pull request #1069 from migonzalvar/squash-notifications

Squash field changes on notification emails

taiga/base/templates/emails/updates-body-html.jinja

@@ -416,7 +416,7 @@
                                                         <tr>
                                                             <th colspan="2"><h2>{{ _("Updates") }}</h2></th>
                                                         </tr>
-                                                        {% for entry in history_entries%}
+                                                        {% for entry in history_entries %}
                                                             {% if entry.comment %}
                                                             <tr>
                                                                 <td colspan="2">

taiga/projects/notifications/services.py

@@ -39,6 +39,7 @@ from taiga.projects.history.services import (make_key_from_model_object,
 from taiga.permissions.services import user_has_perm
 
 from .models import HistoryChangeNotification, Watched
+from .squashing import squash_history_entries
 
 
 def notify_policy_exists(project, user) -> bool:
@@ -250,6 +251,8 @@ def send_sync_notifications(notification_id):
         return
 
     history_entries = tuple(notification.history_entries.all().order_by("created_at"))
+    history_entries = squash_history_entries(history_entries)
+
     obj, _ = get_last_snapshot_for_key(notification.key)
     obj_class = get_model_from_key(obj.key)
 

taiga/projects/notifications/squashing.py

@@ -0,0 +1,84 @@
+from collections import namedtuple, OrderedDict
+
+
+HistoryEntry = namedtuple('HistoryEntry', 'comment values_diff')
+
+
+# These fields are ignored
+
+EXCLUDED_FIELDS = (
+    'description',
+    'description_html',
+    'blocked_note',
+    'blocked_note_html',
+    'content',
+    'content_html',
+    'epics_order',
+    'backlog_order',
+    'kanban_order',
+    'sprint_order',
+    'taskboard_order',
+    'us_order',
+    'custom_attributes',
+    'tribe_gig',
+)
+
+# These fields can't be squashed because we don't have
+# a squashing algorithm yet.
+
+NON_SQUASHABLE_FIELDS = (
+    'points',
+    'attachments',
+    'watchers',
+    'description_diff',
+    'content_diff',
+    'blocked_note_diff',
+    'custom_attributes',
+)
+
+
+def is_squashable(field):
+    return field not in EXCLUDED_FIELDS and field not in NON_SQUASHABLE_FIELDS
+
+
+def summary(field, entries):
+    """
+    Given an iterable of HistoryEntry of the same type return a summarized list.
+    """
+    if len(entries) <= 1:
+        return entries
+
+    # Apply squashing algorithm. In this case, get first `from` and last `to`.
+    initial = entries[0].values_diff[field]
+    final = entries[-1].values_diff[field]
+    from_, to = initial[0], final[1]
+
+    # If the resulting squashed `from` and `to` are equal we can skip
+    # this entry completely
+
+    return [] if from_ == to else [HistoryEntry('', {field: [from_, to]})]
+
+
+def squash_history_entries(history_entries):
+    """
+    Given an iterable of HistoryEntry, squash them summarizing entries that have
+    a squashable algorithm available.
+    """
+    history_entries = (HistoryEntry(entry.comment, entry.values_diff) for entry in history_entries)
+    grouped = OrderedDict()
+    for entry in history_entries:
+        if entry.comment:
+            yield entry
+            continue
+
+        for field, diff in entry.values_diff.items():
+            if is_squashable(field):
+                grouped.setdefault(field, [])
+                grouped[field].append(HistoryEntry('', {field: diff}))
+            else:
+                yield HistoryEntry('', {field: diff})
+
+    for field, entries in grouped.items():
+        squashed = summary(field, entries)
+        for entry in squashed:
+            yield entry

tests/unit/test_notifications_squashing.py

@@ -0,0 +1,99 @@
+from taiga.projects.notifications import squashing
+
+
+def assert_(expected, squashed, *, ordered=True):
+    """
+    Check if expected entries are the same as the squashed.
+
+    Allow to specify if they must maintain the order or conversely they can
+    appear in any order.
+    """
+    squashed = list(squashed)
+    assert len(expected) == len(squashed)
+    if ordered:
+        assert expected == squashed
+    else:
+        # Can't use a set, just check all of the squashed entries
+        # are in the expected ones.
+        for entry in squashed:
+            assert entry in expected
+
+
+def test_squash_omits_comments():
+    history_entries = [
+        squashing.HistoryEntry(comment='A', values_diff={'status': ['A', 'B']}),
+        squashing.HistoryEntry(comment='B', values_diff={'status': ['B', 'C']}),
+        squashing.HistoryEntry(comment='C', values_diff={'status': ['C', 'B']}),
+    ]
+    squashed = squashing.squash_history_entries(history_entries)
+    assert_(history_entries, squashed)
+
+
+def test_squash_allowed_grouped_at_the_end():
+    history_entries = [
+        squashing.HistoryEntry(comment='A', values_diff={}),
+        squashing.HistoryEntry(comment='', values_diff={'status': ['A', 'B']}),
+        squashing.HistoryEntry(comment='', values_diff={'status': ['B', 'C']}),
+        squashing.HistoryEntry(comment='', values_diff={'status': ['C', 'D']}),
+        squashing.HistoryEntry(comment='', values_diff={'status': ['D', 'C']}),
+        squashing.HistoryEntry(comment='Z', values_diff={}),
+    ]
+    expected = [
+        squashing.HistoryEntry(comment='A', values_diff={}),
+        squashing.HistoryEntry(comment='Z', values_diff={}),
+        squashing.HistoryEntry(comment='', values_diff={'status': ['A', 'C']}),
+    ]
+
+    squashed = squashing.squash_history_entries(history_entries)
+    assert_(expected, squashed)
+
+
+def test_squash_remove_noop_changes():
+    history_entries = [
+        squashing.HistoryEntry(comment='', values_diff={'status': ['A', 'B']}),
+        squashing.HistoryEntry(comment='', values_diff={'status': ['B', 'A']}),
+    ]
+    expected = []
+
+    squashed = squashing.squash_history_entries(history_entries)
+    assert_(expected, squashed)
+
+
+def test_squash_remove_noop_changes_but_maintain_others():
+    history_entries = [
+        squashing.HistoryEntry(comment='', values_diff={'status': ['A', 'B'], 'type': ['1', '2']}),
+        squashing.HistoryEntry(comment='', values_diff={'status': ['B', 'A']}),
+    ]
+    expected = [
+        squashing.HistoryEntry(comment='', values_diff={'type': ['1', '2']}),
+    ]
+
+    squashed = squashing.squash_history_entries(history_entries)
+    assert_(expected, squashed)
+
+
+def test_squash_values_diff_with_multiple_fields():
+    history_entries = [
+        squashing.HistoryEntry(comment='', values_diff={'status': ['A', 'B'], 'type': ['1', '2']}),
+        squashing.HistoryEntry(comment='', values_diff={'status': ['B', 'C']}),
+    ]
+    expected = [
+        squashing.HistoryEntry(comment='', values_diff={'type': ['1', '2']}),
+        squashing.HistoryEntry(comment='', values_diff={'status': ['A', 'C']}),
+    ]
+
+    squashed = squashing.squash_history_entries(history_entries)
+    assert_(expected, squashed, ordered=False)
+
+
+def test_squash_arrays():
+    history_entries = [
+        squashing.HistoryEntry(comment='', values_diff={'tags': [['A', 'B'], ['A']]}),
+        squashing.HistoryEntry(comment='', values_diff={'tags': [['A'], ['A', 'C']]}),
+    ]
+    expected = [
+        squashing.HistoryEntry(comment='', values_diff={'tags': [['A', 'B'], ['A', 'C']]}),
+    ]
+
+    squashed = squashing.squash_history_entries(history_entries)
+    assert_(expected, squashed, ordered=False)