diff --git a/amundsen_application/api/mail/v0.py b/amundsen_application/api/mail/v0.py index bd0601aa2..c6ee9bdb3 100644 --- a/amundsen_application/api/mail/v0.py +++ b/amundsen_application/api/mail/v0.py @@ -16,8 +16,11 @@ @mail_blueprint.route('/feedback', methods=['POST']) def feedback() -> Response: + """ + Uses the configured instance of BaseMailClient client configured on the MAIL_CLIENT + config variable to send an email with data from the Feedback component + """ try: - """ An instance of BaseMailClient client must be configured on MAIL_CLIENT """ mail_client = get_mail_client() data = request.form.to_dict() text_content = '\r\n'.join('{}:\r\n{}\r\n'.format(key, val) for key, val in data.items()) @@ -83,16 +86,19 @@ def _feedback(*, @mail_blueprint.route('/notification', methods=['POST']) def notification() -> Response: - # TODO: Write unit tests once actual logic is implemented + """ + Uses the configured instance of BaseMailClient client configured on the MAIL_CLIENT + config variable to send a notification based on data passed from the request + """ try: data = request.get_json() + return send_notification( + notification_type=data['notificationType'], + options=data['options'], + recipients=data['recipients'], + sender=data['sender'] + ) except Exception as e: message = 'Encountered exception: ' + str(e) logging.exception(message) return make_response(jsonify({'msg': message}), HTTPStatus.INTERNAL_SERVER_ERROR) - return send_notification( - notification_type=data['notificationType'], - options=data['options'], - recipients=data['recipients'], - sender=data['sender'] - ) diff --git a/amundsen_application/api/utils/notification_utils.py b/amundsen_application/api/utils/notification_utils.py index 9e592c064..05debaab9 100644 --- a/amundsen_application/api/utils/notification_utils.py +++ b/amundsen_application/api/utils/notification_utils.py @@ -25,27 +25,26 @@ def _log_send_notification(*, notification_type: str, options: Dict, recipients: """ Logs the content of a sent notification""" pass # pragma: no cover - # TODO: write tests try: - mail_client = get_mail_client() - - notification_content = get_notification_content( - notification_type=notification_type, - sender=sender, - options=options - ) - if sender in recipients: recipients.remove(sender) if len(recipients) == 0: logging.info('No recipients exist for notification') return make_response( jsonify({ - 'msg': 'Sender is excluded from notification recipients, no recipients exist.' + 'msg': 'No valid recipients exist for notification, notification was not sent.' }), HTTPStatus.OK ) + mail_client = get_mail_client() + + notification_content = get_notification_content( + notification_type=notification_type, + sender=sender, + options=options + ) + _log_send_notification( notification_type=notification_type, options=options, @@ -86,7 +85,6 @@ def get_mail_client(): # type: ignore Gets a mail_client object to send emails, raises an exception if mail client isn't implemented """ - # TODO: write tests mail_client = app.config['MAIL_CLIENT'] if not mail_client: @@ -101,37 +99,44 @@ def get_notification_content(*, notification_type: str, sender: str, options: Di the input notification_type and data provided :param notification_type: type of notification :param options: data necessary to render email template content - :return: subject and html Dict + :return: html and subject Dict """ - notification_type_dict = { - 'added': { - 'subject': 'You are now an owner of {}'.format(options['resource_name']), - 'html': 'notifications/notification_added.html' - }, - 'removed': { - 'subject': 'You have been removed as an owner of {}'.format(options['resource_name']), - 'html': 'notifications/notification_removed.html' - }, - 'edited': { - 'subject': 'Your dataset {}\'s metadata has been edited'.format(options['resource_name']), - 'html': 'notifications/notification_edited.html' - }, - 'requested': { - 'subject': 'Request for metadata on {}'.format(options['resource_name']), - 'html': 'notifications/notification_requested.html' - }, + template = get_notification_template(notification_type=notification_type) + return { + 'html': render_template(template, sender=sender, options=options), + 'subject': get_notification_subject(notification_type=notification_type, options=options), } - html = render_template(notification_type_dict.get( - notification_type, {}).get('html'), - sender=sender, - options=options - ) - return { - 'subject': notification_type_dict[notification_type]['subject'], - 'html': html, +def get_notification_template(*, notification_type: str) -> str: + """ + Returns the template to use for the given notification_type + :param notification_type: type of notification + :return: The path to the template + """ + notification_template_dict = { + 'added': 'notifications/notification_added.html', + 'removed': 'notifications/notification_removed.html', + 'edited': 'notifications/notification_edited.html', + 'requested': 'notifications/notification_requested.html', + } + return notification_template_dict.get(notification_type, '') + + +def get_notification_subject(*, notification_type: str, options: Dict) -> str: + """ + Returns the subject to use for the given notification_type + :param notification_type: type of notification + :param options: data necessary to render email template content + :return: The subject to be used with the notification + """ + notification_subject_dict = { + 'added': 'You are now an owner of {}'.format(options['resource_name']), + 'removed': 'You have been removed as an owner of {}'.format(options['resource_name']), + 'edited': 'Your dataset {}\'s metadata has been edited'.format(options['resource_name']), + 'requested': 'Request for metadata on {}'.format(options['resource_name']), } + return notification_subject_dict.get(notification_type, '') def table_key_to_url(*, table_key: str) -> str: diff --git a/amundsen_application/config.py b/amundsen_application/config.py index ba0df98a6..82ddcfcd5 100644 --- a/amundsen_application/config.py +++ b/amundsen_application/config.py @@ -24,6 +24,7 @@ class LocalConfig(Config): TESTING = False LOG_LEVEL = 'DEBUG' + FRONTEND_PORT = '5000' # If installing locally directly from the github source # modify these ports if necessary to point to you local search and metadata services SEARCH_PORT = '5001' @@ -32,7 +33,11 @@ class LocalConfig(Config): # If installing using the Docker bootstrap, this should be modified to the docker host ip. LOCAL_HOST = '0.0.0.0' - FRONTEND_BASE = 'http://localhost:5000/' + FRONTEND_BASE = os.environ.get('FRONTEND_BASE', + 'http://{LOCAL_HOST}:{PORT}'.format( + LOCAL_HOST=LOCAL_HOST, + PORT=FRONTEND_PORT) + ) SEARCHSERVICE_REQUEST_CLIENT = None SEARCHSERVICE_REQUEST_HEADERS = None diff --git a/tests/unit/api/mail/test_v0.py b/tests/unit/api/mail/test_v0.py index 1768ac1fb..2f3ccb60f 100644 --- a/tests/unit/api/mail/test_v0.py +++ b/tests/unit/api/mail/test_v0.py @@ -88,3 +88,42 @@ def test_feedback_client_propagate_status_code(self) -> None: 'rating': '10', 'comment': 'test' }) self.assertEqual(response.status_code, expected_code) + + @unittest.mock.patch('amundsen_application.api.mail.v0.send_notification') + def test_notification_endpoint_calls_send_notification(self, send_notification_mock) -> None: + """ + Test that the endpoint calls send_notification with the correct information + from the request json + :return: + """ + test_recipients = ['test@test.com'] + test_sender = 'test2@test.com' + test_notification_type = 'added' + test_options = {} + + with local_app.test_client() as test: + test.post('/api/mail/v0/notification', json={ + 'recipients': test_recipients, + 'sender': test_sender, + 'notificationType': test_notification_type, + 'options': test_options, + }) + send_notification_mock.assert_called_with( + notification_type=test_notification_type, + options=test_options, + recipients=test_recipients, + sender=test_sender + ) + + @unittest.mock.patch('amundsen_application.api.mail.v0.send_notification') + def test_notification_endpoint_fails_with_exception(self, send_notification_mock) -> None: + """ + Test that the endpoint returns 500 exception when error occurs + :return: + """ + with local_app.test_client() as test: + # generates error + response = test.post('/api/mail/v0/notification', json=None) + + self.assertEquals(response.status_code, HTTPStatus.INTERNAL_SERVER_ERROR) + self.assertFalse(send_notification_mock.called) diff --git a/tests/unit/api/metadata/test_v0.py b/tests/unit/api/metadata/test_v0.py index e4092e46b..9db6dfff5 100644 --- a/tests/unit/api/metadata/test_v0.py +++ b/tests/unit/api/metadata/test_v0.py @@ -328,6 +328,64 @@ def test_update_table_owner_success(self) -> None: ) self.assertEqual(response.status_code, HTTPStatus.OK) + @responses.activate + @unittest.mock.patch('amundsen_application.api.metadata.v0.send_notification') + def test_update_table_owner_send_notification_added(self, send_notification_mock) -> None: + """ + Test update_table_owner with PUT calls send_notification + using the 'added' notification type + :return: + """ + url = local_app.config['METADATASERVICE_BASE'] + TABLE_ENDPOINT + '/db://cluster.schema/table/owner/test' + responses.add(responses.PUT, url, json={}, status=HTTPStatus.OK) + with local_app.test_client() as test: + test.put( + '/api/metadata/v0/update_table_owner', + json={ + 'key': 'db://cluster.schema/table', + 'owner': 'test', + 'name': 'schema.table' + } + ) + send_notification_mock.assert_called_with( + notification_type='added', + options={ + 'resource_name': 'schema.table', + 'resource_url': 'http://0.0.0.0:5000/table_detail/cluster/db/schema/table' + }, + recipients=['test'], + sender=local_app.config['AUTH_USER_METHOD'](local_app).email + ) + + @responses.activate + @unittest.mock.patch('amundsen_application.api.metadata.v0.send_notification') + def test_update_table_owner_send_notification_deleted(self, send_notification_mock) -> None: + """ + Test update_table_owner with DELETE calls send_notification + using the 'removed' notification type + :return: + """ + url = local_app.config['METADATASERVICE_BASE'] + TABLE_ENDPOINT + '/db://cluster.schema/table/owner/test' + responses.add(responses.DELETE, url, json={}, status=HTTPStatus.OK) + with local_app.test_client() as test: + test.delete( + '/api/metadata/v0/update_table_owner', + json={ + 'key': 'db://cluster.schema/table', + 'owner': 'test', + 'name': 'schema.table' + } + ) + send_notification_mock.assert_called_with( + notification_type='removed', + options={ + 'resource_name': 'schema.table', + 'resource_url': 'http://0.0.0.0:5000/table_detail/cluster/db/schema/table' + }, + recipients=['test'], + sender=local_app.config['AUTH_USER_METHOD'](local_app).email + ) + @responses.activate def test_get_last_indexed_success(self) -> None: """ diff --git a/tests/unit/utils/notification/test_v0.py b/tests/unit/utils/notification/test_v0.py index e37a05a2a..5cf92c4da 100644 --- a/tests/unit/utils/notification/test_v0.py +++ b/tests/unit/utils/notification/test_v0.py @@ -1,12 +1,35 @@ import unittest +from http import HTTPStatus +from typing import Dict, List + +from flask import jsonify, make_response, Response + from amundsen_application import create_app -from amundsen_application.api.utils.notification_utils import table_key_to_url +from amundsen_application.api.exceptions import MailClientNotImplemented +from amundsen_application.api.utils.notification_utils import get_notification_content, get_mail_client, \ + get_notification_template, get_notification_subject, send_notification, table_key_to_url + +from amundsen_application.base.base_mail_client import BaseMailClient local_app = create_app('amundsen_application.config.TestConfig', 'tests/templates') -class MetadataTest(unittest.TestCase): +class MockMailClient(BaseMailClient): + def __init__(self, status_code: int, recipients: List = []) -> None: + self.status_code = status_code + + def send_email(self, + sender: str = None, + recipients: List = [], + subject: str = None, + text: str = None, + html: str = None, + options: Dict = {}) -> Response: + return make_response(jsonify({}), self.status_code) + + +class NotificationUtilsTest(unittest.TestCase): def setUp(self) -> None: self.mock_table_key = 'db://cluster.schema/table' @@ -18,3 +41,173 @@ def test_table_key_to_url(self) -> None: with local_app.app_context(): url = table_key_to_url(table_key=self.mock_table_key) self.assertEqual('{}/table_detail/cluster/db/schema/table'.format(local_app.config['FRONTEND_BASE']), url) + + @unittest.mock.patch('amundsen_application.api.utils.notification_utils.render_template') + @unittest.mock.patch('amundsen_application.api.utils.notification_utils.get_notification_template') + @unittest.mock.patch('amundsen_application.api.utils.notification_utils.get_notification_subject') + def test_get_notification_content(self, get_subject_mock, get_template_mock, render_template_mock) -> None: + """ + Test successful executiion of get_notification_content + :return: + """ + with local_app.app_context(): + get_subject_mock.return_value = 'Test Subject' + get_template_mock.return_value = 'test.html' + render_template_mock.return_value = 'testHTML' + + test_notification_type = 'test' + test_sender = 'test@test.com' + test_options = {'resource_name': 'testtable'} + + result_dict = get_notification_content(notification_type=test_notification_type, + sender=test_sender, + options=test_options + ) + expected_dict = { + 'subject': 'Test Subject', + 'html': 'testHTML' + } + self.assertDictEqual(result_dict, expected_dict) + get_subject_mock.assert_called_with(notification_type=test_notification_type, options=test_options) + get_template_mock.assert_called_with(notification_type=test_notification_type) + render_template_mock.assert_called_with('test.html', sender=test_sender, options=test_options) + + def test_get_notification_template(self) -> None: + """ + Test successful executiion of get_notification_template for supported notification types + :return: + """ + for n in ['added', 'removed', 'edited', 'requested']: + result = get_notification_template(notification_type=n) + self.assertEqual(result, 'notifications/notification_{}.html'.format(n)) + + def test_get_notification_subject_added(self) -> None: + """ + Test successful executed of get_notification_subject for the 'added' notification type + :return: + """ + result = get_notification_subject(notification_type='added', options={'resource_name': 'testtable'}) + self.assertEqual(result, 'You are now an owner of testtable') + + def test_get_notification_subject_removed(self) -> None: + """ + Test successful executed of get_notification_subject for the 'removed' notification type + :return: + """ + result = get_notification_subject(notification_type='removed', options={'resource_name': 'testtable'}) + self.assertEqual(result, 'You have been removed as an owner of testtable') + + def test_get_notification_subject_edited(self) -> None: + """ + Test successful executed of get_notification_subject for the 'edited' notification type + :return: + """ + result = get_notification_subject(notification_type='edited', options={'resource_name': 'testtable'}) + self.assertEqual(result, 'Your dataset testtable\'s metadata has been edited') + + def test_get_notification_subject_requested(self) -> None: + """ + Test successful executed of get_notification_subject for the 'requested' notification type + :return: + """ + result = get_notification_subject(notification_type='requested', options={'resource_name': 'testtable'}) + self.assertEqual(result, 'Request for metadata on testtable') + + def test_get_mail_client_success(self) -> None: + """ + Test get_mail_client returns the configured mail client if one is configured + :return: + """ + with local_app.app_context(): + local_app.config['MAIL_CLIENT'] = unittest.mock.Mock() + self.assertEqual(get_mail_client(), local_app.config['MAIL_CLIENT']) + + def test_get_mail_client_error(self) -> None: + """ + Test get_mail_client raised MailClientNotImplemented if no mail client is configured + :return: + """ + with local_app.app_context(): + self.assertRaises(MailClientNotImplemented, get_mail_client) + + @unittest.mock.patch('amundsen_application.api.utils.notification_utils.get_notification_content') + @unittest.mock.patch('amundsen_application.api.utils.notification_utils.get_mail_client') + def test_send_notification_success(self, get_mail_client, get_notification_content) -> None: + """ + Test successful execution of send_notification + :return: + """ + with local_app.app_context(): + get_mail_client.return_value = MockMailClient(status_code=HTTPStatus.OK) + + test_recipients = ['test@test.com'] + test_sender = 'test2@test.com' + test_notification_type = '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_notification_content.assert_called_with( + notification_type=test_notification_type, + sender=test_sender, + options=test_options + ) + self.assertEqual(response.status_code, HTTPStatus.OK) + + @unittest.mock.patch('amundsen_application.api.utils.notification_utils.get_notification_content') + @unittest.mock.patch('amundsen_application.api.utils.notification_utils.get_mail_client') + def test_remove_sender_from_notification(self, get_mail_client, get_notification_content) -> None: + """ + Test sender is removed if they exist in recipients + :return: + """ + with local_app.app_context(): + mock_client = MockMailClient(status_code=HTTPStatus.OK) + mock_client.send_email = unittest.mock.Mock() + get_mail_client.return_value = mock_client + + mock_content = { + 'html': 'testHTML', + 'subject': 'Test Subject' + } + get_notification_content.return_value = mock_content + + test_sender = 'test@test.com' + test_recipients = [test_sender, 'test2@test.com'] + test_notification_type = 'added' + test_options = {} + expected_recipients = ['test2@test.com'] + + send_notification( + notification_type=test_notification_type, + options=test_options, + recipients=test_recipients, + sender=test_sender + ) + mock_client.send_email.assert_called_with( + recipients=expected_recipients, + sender=test_sender, + subject=mock_content['subject'], + html=mock_content['html'], + options={'email_type': 'notification'}, + ) + + def test_no_recipients_for_notification(self) -> None: + """ + Test 200 response with appropriate message if no recipients exist + :return: + """ + with local_app.app_context(): + response = send_notification( + notification_type='added', + options={}, + recipients=[], + sender='test@test.com' + ) + self.assertEqual(response.status_code, HTTPStatus.OK)