Python cleanup + tests

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']
-    )

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:

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

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)

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:
         """