Amundsen Notifications Without Preferences (#273)

* Initial start to notifications API (#215) * initial start to notifications API * fixing some styling * fixed lint errors * update types * added tests * linter, moved notification types * addressed comments regarding imports/enum naming * fixed alphabetical order * Notifs post email functionality (#222) * initial start to notifications API * fixing some styling * fixed lint errors * update types * added tests * linter, moved notification types * added template support * made changes to reflect private changes * added helper function * fixed lint issue * addressed comments, added some type checking and cleaned up comments * testing removing test * fixed linter * fixed lint * fixed linting issues * skip type checking * fixed lint * fixed typing on get request args * removed typing for get request to fix lint issues * fixed linter again * re added test * raise exception inside of getmailclient * added exceptions * addressed comments * whitespace issue * removed calls to get_query_param * fixed syntax error * Send notification when adding/removing owner from table (#237) * basic e2e functionality for adding/removing * send_notification refactor * fix lint errors * blank line lint error * fixed syntax issue * arg typing * addressed comments, fixed code style * Prevent Self-Notifications (#243) * Prevent user from notifying themselves * removed exception * added owner check to send_notification * Fixed return for no recipients (#244) * fixed return for no recipients * fixed linter issue * Request notifications component (#238) * init of request form * basic request component * getting basic functionality in * clearing out css * removed z-index fixes and add constants * fixed string casting * added redux-saga calls * removed reset request notification * fixed tests * addressed comments, added basic test, added redux state management for opening/closing component * added tests, just need to add render test * cleaned up component tests: * addressed html/css comments * removed unecessary styling * removed collapsed class * cleaned up render method * fixed test * Open request component (#254) * added button to open up request component * removed tabledetail changes * className styling * fixed text-decoration * added tests, changed naming for OpenRequest * styling formatting * Add, Request, and Remove Email Copy (#257) * init for fixing email copy for request, add, and remove * removed print statement * fixed python unit test * fixed linter issues * addressed comments, fixed linter issues * added notification unit test * fixed test positional arg * fix test * Add notification action logging (#258) * init of adding action logging * changed location of action logging * fixed linter errors * fixed comment * addressed comments * remove request test call (#259) * hide request if description already exists (#269) * fixed open request button, request form styling (#267) * Added request dropdown component (#262) * init * made fixes * cleaned up code * fixed color issues * fixed import order * fixed styling, changed ducks/sagas * User dropdown (#263) * init * fixed sty;es * fixed test issue * fixed test * added tests, addressed comments * Request Metadata Component Tests (#270) * added tests + readonly field to stop errors * fixed tslint * addressed comments, added header tests * Request form navigation fix, dropdown fix (#272) * Request form navigation fix, dropdown fix * added test * added unique id to dropdown * Creates User Preferences page with no functionality (#266) * init * added event handlers * removed test file * added constants * addressed comments * fixed test, removed all links to page * updated test * fixed call to onclick * removed preferences page * Python cleanup + tests (#277) * Python cleanup + tests * More tests + revert some unecessary changes * Bring dropdown UI closer to design (#278) * Rename OpenRequestDescription for clarity + code cleanup + test additions (#279) * Notifications ducks cleanup + tests (#280) * Notifications ducks cleanup + tests * Fix issues * Fix template for edge case of empty form (#281) * Temporary debugging code, will revert * Temporary debugging code, will revert * Implement notification form confirmation (#289) * Preserve compatibility in base_mail_client (#290) * Notifications Configs + Doc (#291) * Add notification config * Code cleanup * More cleanup + add a test * Add some doc for how to enable features * Add config utils test + fix type error * Relative URLs to child configuration docs (#294) * Relative URLs to child configuration docs Relative URLs to docs in the same folder should do. They work for any branch, local copies of the docs - and should work better if we ever (or whenever :-) we get to having e.g a Sphinx generated site. * Update application_config.md Relative doc link * Update flask_config.md Relative doc link * Update flask_config.md Relative doc link * Remove temporary debugging code * Improve behavior of notification sending for owner editing (#296) * Initial Implementation: Notification only on success * Cleanup + tests: Notification only on success * Cleanup: Remove test code to trigger failure * Cleanup: Lint fix * Workaround for not notifying teams or alumni * Cleanup: Remove import mistake * Utilize NotificationType enums instead of hardcoded string * Remove use of render_template * More minor cleanups * Address some feedback * Cleanup * More cleanup
amundsen-io · Oct 2, 2019 · 50c19e8 · 50c19e8
1 parent 261f9ba
commit 50c19e8
Show file tree

Hide file tree

Showing 66 changed files with 2,398 additions and 175 deletions.
diff --git a/frontend/amundsen_application/api/exceptions.py b/frontend/amundsen_application/api/exceptions.py
@@ -0,0 +1,7 @@
+
+
+class MailClientNotImplemented(Exception):
+    """
+    An exception when Mail Client is not implemented
+    """
+    pass
diff --git a/frontend/amundsen_application/api/mail/v0.py b/frontend/amundsen_application/api/mail/v0.py
@@ -6,6 +6,8 @@
 from flask import current_app as app
 from flask.blueprints import Blueprint
 
+from amundsen_application.api.exceptions import MailClientNotImplemented
+from amundsen_application.api.utils.notification_utils import get_mail_client, send_notification
 from amundsen_application.log.action_log import action_logging
 
 LOGGER = logging.getLogger(__name__)
@@ -15,15 +17,12 @@
 
 @mail_blueprint.route('/feedback', methods=['POST'])
 def feedback() -> Response:
-    """ An instance of BaseMailClient client must be configured on MAIL_CLIENT """
-    mail_client = app.config['MAIL_CLIENT']
-
-    if not mail_client:
-        message = 'An instance of BaseMailClient client must be configured on MAIL_CLIENT'
-        logging.exception(message)
-        return make_response(jsonify({'msg': message}), HTTPStatus.NOT_IMPLEMENTED)
-
+    """
+    Uses the instance of BaseMailClient client configured on the MAIL_CLIENT
+    config variable to send an email with feedback data
+    """
     try:
+        mail_client = get_mail_client()
         data = request.form.to_dict()
         text_content = '\r\n'.join('{}:\r\n{}\r\n'.format(k, v) for k, v in data.items())
         html_content = ''.join('<div><strong>{}:</strong><br/>{}</div><br/>'.format(k, v) for k, v in data.items())
@@ -47,7 +46,12 @@ def feedback() -> Response:
                   value_prop=value_prop,
                   subject=subject)
 
-        response = mail_client.send_email(subject=subject, text=text_content, html=html_content, optional_data=data)
+        options = {
+            'email_type': 'feedback',
+            'form_data': data
+        }
+
+        response = mail_client.send_email(subject=subject, text=text_content, html=html_content, optional_data=options)
         status_code = response.status_code
 
         if status_code == HTTPStatus.OK:
@@ -57,9 +61,13 @@ def feedback() -> Response:
             logging.error(message)
 
         return make_response(jsonify({'msg': message}), status_code)
-    except Exception as e:
+    except MailClientNotImplemented as e:
         message = 'Encountered exception: ' + str(e)
         logging.exception(message)
+        return make_response(jsonify({'msg': message}), HTTPStatus.NOT_IMPLEMENTED)
+    except Exception as e1:
+        message = 'Encountered exception: ' + str(e1)
+        logging.exception(message)
         return make_response(jsonify({'msg': message}), HTTPStatus.INTERNAL_SERVER_ERROR)
 
 
@@ -75,3 +83,37 @@ def _feedback(*,
               subject: str) -> None:
     """ Logs the content of the feedback form """
     pass  # pragma: no cover
+
+
+@mail_blueprint.route('/notification', methods=['POST'])
+def notification() -> Response:
+    """
+    Uses the instance of BaseMailClient client configured on the MAIL_CLIENT
+    config variable to send a notification email based on data passed from the request
+    """
+    try:
+        data = request.get_json()
+
+        notification_type = data.get('notificationType')
+        if notification_type is None:
+            message = 'Encountered exception: notificationType must be provided in the request payload'
+            logging.exception(message)
+            return make_response(jsonify({'msg': message}), HTTPStatus.BAD_REQUEST)
+
+        sender = data.get('sender')
+        if sender is None:
+            sender = app.config['AUTH_USER_METHOD'](app).email
+
+        options = data.get('options', {})
+        recipients = data.get('recipients', [])
+
+        return send_notification(
+            notification_type=notification_type,
+            options=options,
+            recipients=recipients,
+            sender=sender
+        )
+    except Exception as e:
+        message = 'Encountered exception: ' + str(e)
+        logging.exception(message)
+        return make_response(jsonify({'msg': message}), HTTPStatus.INTERNAL_SERVER_ERROR)
diff --git a/frontend/amundsen_application/api/metadata/v0.py b/frontend/amundsen_application/api/metadata/v0.py
@@ -138,28 +138,33 @@ def _get_table_metadata(*, table_key: str, index: int, source: str) -> Dict[str,
         return results_dict
 
 
-@action_logging
-def _update_table_owner(*, table_key: str, method: str, owner: str) -> Dict[str, str]:
-    try:
-        table_endpoint = _get_table_endpoint()
-        url = '{0}/{1}/owner/{2}'.format(table_endpoint, table_key, owner)
-        request_metadata(url=url, method=method)
-
-        # TODO: Figure out a way to get this payload from flask.jsonify which wraps with app's response_class
-        return {'msg': 'Updated owner'}
-    except Exception as e:
-        return {'msg': 'Encountered exception: ' + str(e)}
-
-
 @metadata_blueprint.route('/update_table_owner', methods=['PUT', 'DELETE'])
 def update_table_owner() -> Response:
+
+    @action_logging
+    def _log_update_table_owner(*, table_key: str, method: str, owner: str) -> None:
+        pass  # pragma: no cover
+
     try:
         args = request.get_json()
         table_key = get_query_param(args, 'key')
         owner = get_query_param(args, 'owner')
 
-        payload = jsonify(_update_table_owner(table_key=table_key, method=request.method, owner=owner))
-        return make_response(payload, HTTPStatus.OK)
+        table_endpoint = _get_table_endpoint()
+        url = '{0}/{1}/owner/{2}'.format(table_endpoint, table_key, owner)
+        method = request.method
+        _log_update_table_owner(table_key=table_key, method=method, owner=owner)
+
+        response = request_metadata(url=url, method=method)
+        status_code = response.status_code
+
+        if status_code == HTTPStatus.OK:
+            message = 'Updated owner'
+        else:
+            message = 'There was a problem updating owner {0}'.format(owner)
+
+        payload = jsonify({'msg': message})
+        return make_response(payload, status_code)
     except Exception as e:
         payload = jsonify({'msg': 'Encountered exception: ' + str(e)})
         return make_response(payload, HTTPStatus.INTERNAL_SERVER_ERROR)

diff --git a/frontend/amundsen_application/api/utils/notification_utils.py b/frontend/amundsen_application/api/utils/notification_utils.py
@@ -0,0 +1,184 @@
+import logging
+
+from http import HTTPStatus
+
+from flask import current_app as app
+from flask import jsonify, make_response, Response
+from typing import Dict, List
+
+from amundsen_application.api.exceptions import MailClientNotImplemented
+from amundsen_application.log.action_log import action_logging
+
+NOTIFICATION_STRINGS = {
+    'added': {
+        'comment': ('<br/>What is expected of you?<br/>As an owner, you take an important part in making '
+                    'sure that the datasets you own can be used as swiftly as possible across the company.<br/>'
+                    'Make sure the metadata is correct and up to date.<br/>'),
+        'end_note': ('<br/>If you think you are not the best person to own this dataset and know someone who might '
+                     'be, please contact this person and ask them if they want to replace you. It is important that we '
+                     'keep multiple owners for each dataset to ensure continuity.<br/>'),
+        'notification': ('<br/>You have been added to the owners list of the <a href="{resource_url}">'
+                         '{resource_name}</a> dataset by {sender}.<br/>'),
+    },
+    'removed': {
+        'comment': '',
+        'end_note': ('<br/>If you think you have been incorrectly removed as an owner, '
+                     'add yourself back to the owners list.<br/>'),
+        'notification': ('<br/>You have been removed from the owners list of the <a href="{resource_url}">'
+                         '{resource_name}</a> dataset by {sender}.<br/>'),
+    },
+    'requested': {
+        'comment': '',
+        'end_note': '<br/>Please visit the provided link and improve descriptions on that resource.<br/>',
+        'notification': '<br/>{sender} is trying to use <a href="{resource_url}">{resource_name}</a>, ',
+    }
+}
+
+
+def get_mail_client():  # type: ignore
+    """
+    Gets a mail_client object to send emails, raises an exception
+    if mail client isn't implemented
+    """
+    mail_client = app.config['MAIL_CLIENT']
+
+    if not mail_client:
+        raise MailClientNotImplemented('An instance of BaseMailClient client must be configured on MAIL_CLIENT')
+
+    return mail_client
+
+
+def get_notification_html(*, notification_type: str, options: Dict, sender: str) -> str:
+    """
+    Returns the formatted html for the notification based on the notification_type
+    :return: A string representing the html markup to send in the notification
+    """
+    resource_url = options.get('resource_url')
+    if resource_url is None:
+        raise Exception('resource_url was not provided in the notification options')
+
+    resource_name = options.get('resource_name')
+    if resource_name is None:
+        raise Exception('resource_name was not provided in the notification options')
+
+    notification_strings = NOTIFICATION_STRINGS.get(notification_type)
+    if notification_strings is None:
+        raise Exception('Unsupported notification_type')
+
+    greeting = 'Hello,<br/>'
+    notification = notification_strings.get('notification', '').format(resource_url=resource_url,
+                                                                       resource_name=resource_name,
+                                                                       sender=sender)
+    comment = notification_strings.get('comment', '')
+    end_note = notification_strings.get('end_note', '')
+    salutation = '<br/>Thanks,<br/>Amundsen Team'
+
+    if notification_type == 'requested':
+        options_comment = options.get('comment')
+        need_resource_description = options.get('description_requested')
+        need_fields_descriptions = options.get('fields_requested')
+
+        if need_resource_description and need_fields_descriptions:
+            notification = notification + 'and requests improved table and column descriptions.<br/>'
+        elif need_resource_description:
+            notification = notification + 'and requests an improved table description.<br/>'
+        elif need_fields_descriptions:
+            notification = notification + 'and requests improved column descriptions.<br/>'
+        else:
+            notification = notification + 'and requests more information about that resource.<br/>'
+
+        if options_comment:
+            comment = ('<br/>{sender} has included the following information with their request:'
+                       '<br/>{comment}<br/>').format(sender=sender, comment=options_comment)
+
+    return '{greeting}{notification}{comment}{end_note}{salutation}'.format(greeting=greeting,
+                                                                            notification=notification,
+                                                                            comment=comment,
+                                                                            end_note=end_note,
+                                                                            salutation=salutation)
+
+
+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 send_notification(*, notification_type: str, options: Dict, recipients: List, sender: str) -> Response:
+    """
+    Sends a notification via email to a given list of recipients
+    :param notification_type: type of notification
+    :param options: data necessary to render email template content
+    :param recipients: list of recipients who should receive notification
+    :param sender: email of notification sender
+    :return: Response
+    """
+    @action_logging
+    def _log_send_notification(*, notification_type: str, options: Dict, recipients: List, sender: str) -> None:
+        """ Logs the content of a sent notification"""
+        pass  # pragma: no cover
+
+    try:
+        if not app.config['NOTIFICATIONS_ENABLED']:
+            message = 'Notifications are not enabled. Request was accepted but no notification will be sent.'
+            logging.exception(message)
+            return make_response(jsonify({'msg': message}), HTTPStatus.ACCEPTED)
+        if sender in recipients:
+            recipients.remove(sender)
+        if len(recipients) == 0:
+            logging.info('No recipients exist for notification')
+            return make_response(
+                jsonify({
+                    'msg': 'No valid recipients exist for notification, notification was not sent.'
+                }),
+                HTTPStatus.OK
+            )
+
+        mail_client = get_mail_client()
+
+        html = get_notification_html(notification_type=notification_type, options=options, sender=sender)
+        subject = get_notification_subject(notification_type=notification_type, options=options)
+
+        _log_send_notification(
+            notification_type=notification_type,
+            options=options,
+            recipients=recipients,
+            sender=sender
+        )
+
+        response = mail_client.send_email(
+            recipients=recipients,
+            sender=sender,
+            subject=subject,
+            html=html,
+            optional_data={
+                'email_type': 'notification'
+            },
+        )
+        status_code = response.status_code
+
+        if status_code == HTTPStatus.OK:
+            message = 'Success'
+        else:
+            message = 'Mail client failed with status code ' + str(status_code)
+            logging.error(message)
+
+        return make_response(jsonify({'msg': message}), status_code)
+    except MailClientNotImplemented as e:
+        message = 'Encountered exception: ' + str(e)
+        logging.exception(message)
+        return make_response(jsonify({'msg': message}), HTTPStatus.NOT_IMPLEMENTED)
+    except Exception as e1:
+        message = 'Encountered exception: ' + str(e1)
+        logging.exception(message)
+        return make_response(jsonify({'msg': message}), HTTPStatus.INTERNAL_SERVER_ERROR)
diff --git a/frontend/amundsen_application/config.py b/frontend/amundsen_application/config.py
@@ -18,12 +18,17 @@ class Config:
     # Request Timeout Configurations in Seconds
     REQUEST_SESSION_TIMEOUT_SEC = 3
 
+    # Mail Client Features
+    MAIL_CLIENT = None
+    NOTIFICATIONS_ENABLED = False
+
 
 class LocalConfig(Config):
     DEBUG = False
     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,6 +37,12 @@ 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 = 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
     SEARCHSERVICE_BASE = os.environ.get('SEARCHSERVICE_BASE',
@@ -57,8 +68,12 @@ class LocalConfig(Config):
     AUTH_USER_METHOD = None  # type: Optional[function]
     GET_PROFILE_URL = None
 
-    MAIL_CLIENT = None
-
 
 class TestConfig(LocalConfig):
     AUTH_USER_METHOD = get_test_user
+    NOTIFICATIONS_ENABLED = True
+
+
+class TestNotificationsDisabledConfig(LocalConfig):
+    AUTH_USER_METHOD = get_test_user
+    NOTIFICATIONS_ENABLED = False