From b75d69c7be5502dac13dc28944356708017d8bb8 Mon Sep 17 00:00:00 2001 From: Gabriel Krell Date: Sun, 22 Oct 2017 19:42:14 -0500 Subject: [PATCH 1/5] Make ASM group_id mandatory If the optional ASM is specified, the API says the group_id is mandatory. --- sendgrid/helpers/mail/mail.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/sendgrid/helpers/mail/mail.py b/sendgrid/helpers/mail/mail.py index de41bad70..c211fbb0b 100644 --- a/sendgrid/helpers/mail/mail.py +++ b/sendgrid/helpers/mail/mail.py @@ -703,15 +703,9 @@ def get(self): class ASM(object): - def __init__(self, group_id=None, groups_to_display=None): - self._group_id = None - self._groups_to_display = None - - if group_id is not None: - self._group_id = group_id - - if groups_to_display is not None: - self._groups_to_display = groups_to_display + def __init__(self, group_id, groups_to_display=None): + self._group_id = group_id + self._groups_to_display = groups_to_display @property def group_id(self): From 9a9ae6bfc79cfac4355e922e26b3f47246de2176 Mon Sep 17 00:00:00 2001 From: Gabriel Krell Date: Sun, 22 Oct 2017 20:56:54 -0500 Subject: [PATCH 2/5] Limit ASM's groups_to_display API says "You can specify up to 25 groups to display." If param is too long, raise ValueError. --- sendgrid/helpers/mail/mail.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sendgrid/helpers/mail/mail.py b/sendgrid/helpers/mail/mail.py index c211fbb0b..b59be6f84 100644 --- a/sendgrid/helpers/mail/mail.py +++ b/sendgrid/helpers/mail/mail.py @@ -704,6 +704,8 @@ def get(self): class ASM(object): def __init__(self, group_id, groups_to_display=None): + if groups_to_display is not None and len(groups_to_display) > 25: + raise ValueError("groups_to_display exceeds max length of 25") self._group_id = group_id self._groups_to_display = groups_to_display From 4bc2e9cce3152db87a61d071cf311bfe71ee1cbb Mon Sep 17 00:00:00 2001 From: Gabriel Krell Date: Sun, 22 Oct 2017 21:36:13 -0500 Subject: [PATCH 3/5] Add a Mail/ASM unit test Do we really need this? It makes Codecov happy though. --- test/test_mail.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test_mail.py b/test/test_mail.py index 3356eef52..8b88f5b14 100644 --- a/test/test_mail.py +++ b/test/test_mail.py @@ -468,6 +468,9 @@ def test_unicode_values_in_substitutions_helper(self): json.dumps(expected_result, sort_keys=True) ) + def test_asm_display_group_limit(self): + self.assertRaises(ValueError, ASM, 1, list(range(26))) + def test_disable_tracking(self): tracking_settings = TrackingSettings() tracking_settings.click_tracking = ClickTracking(False, False) From 54e4ec7cdf9db5e89ba06ab148076caf4809d917 Mon Sep 17 00:00:00 2001 From: Gabriel Krell Date: Fri, 27 Oct 2017 22:56:32 -0500 Subject: [PATCH 4/5] Move ASM check to setter Oops. Move groups_to_display validation to setter, not init. --- sendgrid/helpers/mail/mail.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sendgrid/helpers/mail/mail.py b/sendgrid/helpers/mail/mail.py index cf573750f..cb787dd4a 100644 --- a/sendgrid/helpers/mail/mail.py +++ b/sendgrid/helpers/mail/mail.py @@ -730,8 +730,6 @@ def get(self): class ASM(object): def __init__(self, group_id, groups_to_display=None): - if groups_to_display is not None and len(groups_to_display) > 25: - raise ValueError("groups_to_display exceeds max length of 25") self._group_id = group_id self._groups_to_display = groups_to_display @@ -749,6 +747,8 @@ def groups_to_display(self): @groups_to_display.setter def groups_to_display(self, value): + if value is not None and len(value) > 25: + raise ValueError("New groups_to_display exceeds max length of 25.") self._groups_to_display = value def get(self): From 0f2ce750de8d1a8ac174259f4d00a9b49db7d1c6 Mon Sep 17 00:00:00 2001 From: Gabriel Krell Date: Sat, 28 Oct 2017 23:57:18 -0500 Subject: [PATCH 5/5] Fix ASM tests --- sendgrid/helpers/mail/mail.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sendgrid/helpers/mail/mail.py b/sendgrid/helpers/mail/mail.py index cb787dd4a..6d2b3b4d0 100644 --- a/sendgrid/helpers/mail/mail.py +++ b/sendgrid/helpers/mail/mail.py @@ -730,8 +730,8 @@ def get(self): class ASM(object): def __init__(self, group_id, groups_to_display=None): - self._group_id = group_id - self._groups_to_display = groups_to_display + self.group_id = group_id + self.groups_to_display = groups_to_display @property def group_id(self):