From 75545eb2561bfd60bddadb0d60eb6bc488dd6b27 Mon Sep 17 00:00:00 2001 From: Miguel Gonzalez Date: Thu, 1 Feb 2018 20:34:48 +0100 Subject: [PATCH 1/5] add space --- taiga/base/templates/emails/updates-body-html.jinja | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taiga/base/templates/emails/updates-body-html.jinja b/taiga/base/templates/emails/updates-body-html.jinja index f07dbe74..04881791 100644 --- a/taiga/base/templates/emails/updates-body-html.jinja +++ b/taiga/base/templates/emails/updates-body-html.jinja @@ -416,7 +416,7 @@

{{ _("Updates") }}

- {% for entry in history_entries%} + {% for entry in history_entries %} {% if entry.comment %} From dbafa5264d24c5ab661b58b47b1654574165d5d4 Mon Sep 17 00:00:00 2001 From: Miguel Gonzalez Date: Mon, 5 Feb 2018 18:44:42 +0100 Subject: [PATCH 2/5] add initial implementation notifications squashing --- taiga/projects/notifications/squashing.py | 81 ++++++++++++++++++++ tests/unit/test_notifications_squashing.py | 86 ++++++++++++++++++++++ 2 files changed, 167 insertions(+) create mode 100644 taiga/projects/notifications/squashing.py create mode 100644 tests/unit/test_notifications_squashing.py diff --git a/taiga/projects/notifications/squashing.py b/taiga/projects/notifications/squashing.py new file mode 100644 index 00000000..b3a9437b --- /dev/null +++ b/taiga/projects/notifications/squashing.py @@ -0,0 +1,81 @@ +from collections import namedtuple + + +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', + 'tags', + '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 + + initial = entries[0].values_diff[field] + final = entries[-1].values_diff[field] + from_, to = initial[0], final[1] + 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) + from collections import OrderedDict + 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 diff --git a/tests/unit/test_notifications_squashing.py b/tests/unit/test_notifications_squashing.py new file mode 100644 index 00000000..a481d6f8 --- /dev/null +++ b/tests/unit/test_notifications_squashing.py @@ -0,0 +1,86 @@ +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) From 1a3b6bc3cac6ffef22564c8248b82e217aaf6bce Mon Sep 17 00:00:00 2001 From: Miguel Gonzalez Date: Mon, 5 Feb 2018 21:19:09 +0100 Subject: [PATCH 3/5] apply squash_history_entries --- taiga/projects/notifications/services.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/taiga/projects/notifications/services.py b/taiga/projects/notifications/services.py index b6c0fbdd..93f42484 100644 --- a/taiga/projects/notifications/services.py +++ b/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) From 3180dfeda0858d326fcb2968ef2434f122c48840 Mon Sep 17 00:00:00 2001 From: Miguel Gonzalez Date: Tue, 6 Feb 2018 13:45:34 +0100 Subject: [PATCH 4/5] move import to the top --- taiga/projects/notifications/squashing.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/taiga/projects/notifications/squashing.py b/taiga/projects/notifications/squashing.py index b3a9437b..bd7ca66f 100644 --- a/taiga/projects/notifications/squashing.py +++ b/taiga/projects/notifications/squashing.py @@ -1,4 +1,4 @@ -from collections import namedtuple +from collections import namedtuple, OrderedDict HistoryEntry = namedtuple('HistoryEntry', 'comment values_diff') @@ -61,7 +61,6 @@ def squash_history_entries(history_entries): a squashable algorithm available. """ history_entries = (HistoryEntry(entry.comment, entry.values_diff) for entry in history_entries) - from collections import OrderedDict grouped = OrderedDict() for entry in history_entries: if entry.comment: From 9337f0462106b000e7daf7d4734a4b6d12d45636 Mon Sep 17 00:00:00 2001 From: Miguel Gonzalez Date: Tue, 6 Feb 2018 14:17:31 +0100 Subject: [PATCH 5/5] activate tags field --- taiga/projects/notifications/squashing.py | 6 +++++- tests/unit/test_notifications_squashing.py | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/taiga/projects/notifications/squashing.py b/taiga/projects/notifications/squashing.py index bd7ca66f..13ff9b3b 100644 --- a/taiga/projects/notifications/squashing.py +++ b/taiga/projects/notifications/squashing.py @@ -29,7 +29,6 @@ EXCLUDED_FIELDS = ( NON_SQUASHABLE_FIELDS = ( 'points', 'attachments', - 'tags', 'watchers', 'description_diff', 'content_diff', @@ -49,9 +48,14 @@ def summary(field, entries): 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]})] diff --git a/tests/unit/test_notifications_squashing.py b/tests/unit/test_notifications_squashing.py index a481d6f8..bb6b6228 100644 --- a/tests/unit/test_notifications_squashing.py +++ b/tests/unit/test_notifications_squashing.py @@ -84,3 +84,16 @@ def test_squash_values_diff_with_multiple_fields(): 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)