From 9494db583dfb8b8f631a7d576fbb4e9c2d855ee5 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Fri, 24 Jun 2016 16:50:27 -0400 Subject: [PATCH 1/2] Better handle missing notification_type in patches to notification template Addresses #2628 --- awx/api/serializers.py | 2 ++ awx/main/tests/functional/test_notifications.py | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 2e3b2d41f0..8c7d8de450 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2435,6 +2435,8 @@ class NotificationTemplateSerializer(BaseSerializer): def validate(self, attrs): from awx.api.views import NotificationTemplateDetail + if 'notification_type' not in attrs: + raise serializers.ValidationError('Missing required fields for Notification Configuration: notification_type') notification_class = NotificationTemplate.CLASS_FOR_NOTIFICATION_TYPE[attrs['notification_type']] missing_fields = [] incorrect_type_fields = [] diff --git a/awx/main/tests/functional/test_notifications.py b/awx/main/tests/functional/test_notifications.py index d6db3e608c..1b251333e2 100644 --- a/awx/main/tests/functional/test_notifications.py +++ b/awx/main/tests/functional/test_notifications.py @@ -104,3 +104,7 @@ def test_notification_template_merging(get, post, user, organization, project, n organization.notification_templates_any.add(notification_template) project.notification_templates_any.add(notification_template) assert len(project.notification_templates['any']) == 1 + +@pytest.mark.django_db +def test_notification_template_invalid_patch(patch, notification_template, organization, admin): + patch(reverse('api:notification_template_detail', args=(notification_template.id,)), { 'organization': organization.id}, admin, expect=400) From 2b323fc24e5e5e5cf247c2a5bd2f49fbc0064b8b Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Mon, 27 Jun 2016 09:40:39 -0400 Subject: [PATCH 2/2] Better handle notification template patches for notification_type This makes it so patches don't require the notification_type to be present --- awx/api/serializers.py | 11 +++++++++-- awx/main/tests/functional/test_notifications.py | 8 ++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 8c7d8de450..a9a1f1e29c 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2435,9 +2435,16 @@ class NotificationTemplateSerializer(BaseSerializer): def validate(self, attrs): from awx.api.views import NotificationTemplateDetail - if 'notification_type' not in attrs: + + notification_type = None + if 'notification_type' in attrs: + notification_type = attrs['notification_type'] + elif self.instance: + notification_type = self.instance.notification_type + if not notification_type: raise serializers.ValidationError('Missing required fields for Notification Configuration: notification_type') - notification_class = NotificationTemplate.CLASS_FOR_NOTIFICATION_TYPE[attrs['notification_type']] + + notification_class = NotificationTemplate.CLASS_FOR_NOTIFICATION_TYPE[notification_type] missing_fields = [] incorrect_type_fields = [] error_list = [] diff --git a/awx/main/tests/functional/test_notifications.py b/awx/main/tests/functional/test_notifications.py index 1b251333e2..c54e9fb3f2 100644 --- a/awx/main/tests/functional/test_notifications.py +++ b/awx/main/tests/functional/test_notifications.py @@ -106,5 +106,9 @@ def test_notification_template_merging(get, post, user, organization, project, n assert len(project.notification_templates['any']) == 1 @pytest.mark.django_db -def test_notification_template_invalid_patch(patch, notification_template, organization, admin): - patch(reverse('api:notification_template_detail', args=(notification_template.id,)), { 'organization': organization.id}, admin, expect=400) +def test_notification_template_simple_patch(patch, notification_template, admin): + patch(reverse('api:notification_template_detail', args=(notification_template.id,)), { 'name': 'foo'}, admin, expect=200) + +@pytest.mark.django_db +def test_notification_template_invalid_notification_type(patch, notification_template, admin): + patch(reverse('api:notification_template_detail', args=(notification_template.id,)), { 'notification_type': 'invalid'}, admin, expect=400)