fix: notification_utils.send_notification accept all 2xx status codes (#860)

* Accept all 2xx status codes from mail clients
Signed-off-by: JacobSMoller <scherffenberg91@gmail.com>

* Updates test_send_notification_success to include an ACCEPTED status
Signed-off-by: JacobSMoller <scherffenberg91@gmail.com>

* Also check for all 2xx codes for feedback emails
Signed-off-by: JacobSMoller <scherffenberg91@gmail.com>

amundsen_application/api/mail/v0.py

@@ -56,7 +56,7 @@ def feedback() -> Response:
         response = mail_client.send_email(html=html_content, subject=subject, optional_data=options)
         status_code = response.status_code
 
-        if status_code == HTTPStatus.OK:
+        if 200 <= status_code < 300:
             message = 'Success'
         else:
             message = 'Mail client failed with status code ' + str(status_code)

amundsen_application/api/utils/notification_utils.py

@@ -218,7 +218,7 @@ def send_notification(*, notification_type: str, options: Dict, recipients: List
         )
         status_code = response.status_code
 
-        if status_code == HTTPStatus.OK:
+        if 200 <= status_code < 300:
             message = 'Success'
         else:
             message = 'Mail client failed with status code ' + str(status_code)

tests/unit/api/mail/test_v0.py

@@ -59,12 +59,15 @@ class MailTest(unittest.TestCase):
         Test mail client success
         :return:
         """
-        local_app.config['MAIL_CLIENT'] = MockMailClient(status_code=200)
-        with local_app.test_client() as test:
-            response = test.post('/api/mail/v0/feedback', json={
-                'rating': '10', 'comment': 'test'
-            })
-            self.assertEqual(response.status_code, HTTPStatus.OK)
+        status_codes = [HTTPStatus.OK, HTTPStatus.ACCEPTED]
+        for status_code in status_codes:
+            local_app.config['MAIL_CLIENT'] = MockMailClient(status_code=status_code)
+            with self.subTest():
+                with local_app.test_client() as test:
+                    response = test.post('/api/mail/v0/feedback', json={
+                        'rating': '10', 'comment': 'test'
+                    })
+                    self.assertTrue(200 <= response.status_code <= 300)
 
     def test_feedback_client_raise_exception(self) -> None:
         """

tests/unit/utils/test_notification_utils.py

@@ -319,29 +319,34 @@ class NotificationUtilsTest(unittest.TestCase):
         Test successful execution of send_notification
         :return:
         """
-        with local_app.app_context():
-            get_mail_client.return_value = MockMailClient(status_code=HTTPStatus.OK)
-            get_notif_subject.return_value = 'Test Subject'
-            get_notif_html.return_value = '<div>test html</div>'
-
-            test_recipients = ['test@test.com']
-            test_sender = 'test2@test.com'
-            test_notification_type = NotificationType.OWNER_ADDED
-            test_options = {}
-
-            response = send_notification(
-                notification_type=test_notification_type,
-                options=test_options,
-                recipients=test_recipients,
-                sender=test_sender
-            )
-
-            get_mail_client.assert_called
-            get_notif_subject.assert_called_with(notification_type=test_notification_type, options=test_options)
-            get_notif_html.assert_called_with(notification_type=test_notification_type,
-                                              options=test_options,
-                                              sender=test_sender)
-            self.assertEqual(response.status_code, HTTPStatus.OK)
+        status_codes = [HTTPStatus.OK, HTTPStatus.ACCEPTED]
+
+        for status_code in status_codes:
+            with self.subTest():
+                with local_app.app_context():
+                    get_mail_client.return_value = MockMailClient(status_code=status_code)
+                    get_notif_subject.return_value = 'Test Subject'
+                    get_notif_html.return_value = '<div>test html</div>'
+
+                    test_recipients = ['test@test.com']
+                    test_sender = 'test2@test.com'
+                    test_notification_type = NotificationType.OWNER_ADDED
+                    test_options = {}
+
+                    response = send_notification(
+                        notification_type=test_notification_type,
+                        options=test_options,
+                        recipients=test_recipients,
+                        sender=test_sender
+                    )
+
+                    get_mail_client.assert_called
+                    get_notif_subject.assert_called_with(notification_type=test_notification_type,
+                                                         options=test_options)
+                    get_notif_html.assert_called_with(notification_type=test_notification_type,
+                                                      options=test_options,
+                                                      sender=test_sender)
+                    self.assertTrue(200 <= response.status_code <= 300)
 
     @unittest.mock.patch('amundsen_application.api.utils.notification_utils.get_notification_html')
     @unittest.mock.patch('amundsen_application.api.utils.notification_utils.get_notification_subject')