Skip to content

A
amundsen_dev
Unverified Commit 97678d63 authored by Tamika Tannis's avatar Tamika Tannis Committed by GitHub
Browse files
Options

Custom routes + Further notification cleanup (#309) 

* Further notification cleanup

* Catch any errors getting the user for metrics + add doc for INIT_CUSTOM_ROUTES

* Revert change to _build_metrics
parent 9239d1cf
Hide whitespace changes
Inline Side-by-side
Showing with 110 additions and 77 deletions
amundsen_application/__init__.py
View file @ 97678d63
......@@ -56,4 +56,8 @@ def create_app(config_module_class: str, template_folder: str = None) -> Flask:
app.register_blueprint(search_blueprint)
init_routes(app)
init_custom_routes = app.config.get('INIT_CUSTOM_ROUTES')
if init_custom_routes:
init_custom_routes(app)
return app
amundsen_application/api/utils/notification_utils.py
View file @ 97678d63
import logging
from http import HTTPStatus
from enum import Enum
from flask import current_app as app
from flask import jsonify, make_response, Response
......@@ -9,8 +10,27 @@ from typing import Dict, List
from amundsen_application.api.exceptions import MailClientNotImplemented
from amundsen_application.log.action_log import action_logging
class NotificationType(str, Enum):
"""
Enum to describe supported notification types. Must match NotificationType interface defined in:
https://github.com/lyft/amundsenfrontendlibrary/blob/master/amundsen_application/static/js/interfaces/Notifications.ts
"""
OWNER_ADDED = 'owner_added'
OWNER_REMOVED = 'owner_removed'
METADATA_EDITED = 'metadata_edited'
METADATA_REQUESTED = 'metadata_requested'
@classmethod
def has_value(cls, value: str) -> bool:
for key in cls:
if key.value == value:
return True
return False
NOTIFICATION_STRINGS = {
'added': {
NotificationType.OWNER_ADDED.value: {
'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/>'),
......@@ -20,14 +40,14 @@ NOTIFICATION_STRINGS = {
'notification': ('<br/>You have been added to the owners list of the <a href="{resource_url}">'
'{resource_name}</a> dataset by {sender}.<br/>'),
},
'removed': {
NotificationType.OWNER_REMOVED.value: {
'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': {
NotificationType.METADATA_REQUESTED.value: {
'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>, ',
......@@ -84,7 +104,7 @@ def get_notification_html(*, notification_type: str, options: Dict, sender: str)
end_note = notification_strings.get('end_note', '')
salutation = '<br/>Thanks,<br/>Amundsen Team'
if notification_type == 'requested':
if notification_type == NotificationType.METADATA_REQUESTED:
options_comment = options.get('comment')
need_resource_description = options.get('description_requested')
need_fields_descriptions = options.get('fields_requested')
......@@ -118,12 +138,15 @@ def get_notification_subject(*, notification_type: str, options: Dict) -> str:
"""
resource_name = options.get('resource_name')
notification_subject_dict = {
'added': 'You are now an owner of {}'.format(resource_name),
'removed': 'You have been removed as an owner of {}'.format(resource_name),
'edited': 'Your dataset {}\'s metadata has been edited'.format(resource_name),
'requested': 'Request for metadata on {}'.format(resource_name),
NotificationType.OWNER_ADDED.value: 'You are now an owner of {}'.format(resource_name),
NotificationType.OWNER_REMOVED.value: 'You have been removed as an owner of {}'.format(resource_name),
NotificationType.METADATA_EDITED.value: 'Your dataset {}\'s metadata has been edited'.format(resource_name),
NotificationType.METADATA_REQUESTED.value: 'Request for metadata on {}'.format(resource_name),
}
return notification_subject_dict.get(notification_type, '')
subject = notification_subject_dict.get(notification_type)
if subject is None:
raise Exception('Unsupported notification_type')
return subject
def send_notification(*, notification_type: str, options: Dict, recipients: List, sender: str) -> Response:
......@@ -174,7 +197,7 @@ def send_notification(*, notification_type: str, options: Dict, recipients: List
subject=subject,
html=html,
optional_data={
'email_type': 'notification'
'email_type': notification_type,
},
)
status_code = response.status_code
......
amundsen_application/config.py
View file @ 97678d63
import os
from typing import Dict, Optional, Set # noqa: F401
from typing import Callable, Dict, Optional, Set # noqa: F401
from flask import Flask # noqa: F401
from amundsen_application.tests.test_utils import get_test_user
......@@ -22,6 +25,9 @@ class Config:
MAIL_CLIENT = None
NOTIFICATIONS_ENABLED = False
# Initialize custom routes
INIT_CUSTOM_ROUTES = None # type: Callable[[Flask], None]
class LocalConfig(Config):
DEBUG = False
......
amundsen_application/static/js/interfaces/Notifications.ts
View file @ 97678d63
// TODO: Remove notification types that can be triggered in flask layer if necessary
export enum NotificationType {
OWNER_ADDED = 'added',
OWNER_REMOVED = 'removed',
METADATA_EDITED = 'edited',
METADATA_REQUESTED = 'requested',
OWNER_ADDED = 'owner_added',
OWNER_REMOVED = 'owner_removed',
METADATA_EDITED = 'metadata_edited',
METADATA_REQUESTED = 'metadata_requested',
}
export enum RequestMetadataType {
......
docs/flask_config.md
View file @ 97678d63
......@@ -4,6 +4,18 @@ After modifying any variable in [config.py](https://github.com/lyft/amundsenfron
**NOTE: This document is a work in progress and does not include 100% of features. We welcome PRs to complete this document**
## Custom Routes
In order to add any custom Flask endpoints to Amundsen's frontend application, configure a function on the `INIT_CUSTOM_ROUTES` variable. This function takes the created Flask application and can leverage Flask's [add_url_rule](https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask.add_url_rule) method to add custom routes.
Example: Setting `INIT_CUSTOM_ROUTES` to the `init_custom_routes` method below will expose a `/custom_route` endpoint on the frontend application.
```bash
def init_custom_routes(app: Flask) -> None:
app.add_url_rule('/custom_route', 'custom_route', custom_route)
def custom_route():
pass
```
## Mail Client Features
Amundsen has two features that leverage the custom mail client -- the feedback tool and notifications. For these features a custom implementation of [base_mail_client](https://github.com/lyft/amundsenfrontendlibrary/blob/master/amundsen_application/base/base_mail_client.py) must be mapped to the `MAIL_CLIENT` configuration variable.
......
tests/unit/utils/notification/test_utils.py
View file @ 97678d63
......@@ -9,7 +9,7 @@ from flask import jsonify, make_response, Response
from amundsen_application import create_app
from amundsen_application.api.exceptions import MailClientNotImplemented
from amundsen_application.api.utils.notification_utils import get_mail_client, \
get_notification_html, get_notification_subject, send_notification
get_notification_html, get_notification_subject, send_notification, NotificationType
from amundsen_application.base.base_mail_client import BaseMailClient
local_app = create_app('amundsen_application.config.TestConfig', 'tests/templates')
......@@ -39,50 +39,38 @@ class NotificationUtilsTest(unittest.TestCase):
Test Exception is raised if resource_path is None
:return:
"""
test_notification_type = 'removed'
test_sender = 'test@test.com'
test_options = {'resource_name': 'testtable'}
with local_app.app_context():
self.assertRaises(Exception,
get_notification_html,
notification_type=test_notification_type,
options=test_options,
sender=test_sender)
notification_type=NotificationType.OWNER_REMOVED,
options={'resource_name': 'testtable'},
sender='test@test.com')
def test_validate_resource_path_bad_syntax(self) -> None:
"""
Test Exception is raised if resource_path violates leading '/' syntax
:return:
"""
test_notification_type = 'removed'
test_sender = 'test@test.com'
test_options = {'resource_name': 'testtable', 'resource_path': 'testpath'}
with local_app.app_context():
self.assertRaises(Exception,
get_notification_html,
notification_type=test_notification_type,
options=test_options,
sender=test_sender)
notification_type=NotificationType.OWNER_REMOVED,
options={'resource_name': 'testtable', 'resource_path': 'testpath'},
sender='test@test.com')
def test_get_notification_html_bad_base_url(self) -> None:
"""
Test Exception is raised if configured FRONTEND_BASE violates no trailing '/' syntax
:return:
"""
test_notification_type = 'added'
test_sender = 'test@test.com'
test_options = {'resource_name': 'testtable', 'resource_path': '/testpath'}
with local_app.app_context():
temp = local_app.config['FRONTEND_BASE']
local_app.config['FRONTEND_BASE'] = 'garbagetest_rewrite_file_to_setup_teardown_each_case/'
self.assertRaises(Exception,
get_notification_html,
notification_type=test_notification_type,
options=test_options,
sender=test_sender)
notification_type=NotificationType.OWNER_ADDED,
options={'resource_name': 'testtable', 'resource_path': '/testpath'},
sender='test@test.com')
local_app.config['FRONTEND_BASE'] = temp
def test_get_notification_html_no_resource_name(self) -> None:
......@@ -90,44 +78,35 @@ class NotificationUtilsTest(unittest.TestCase):
Test Exception is raised if resource_name is not provided
:return:
"""
test_notification_type = 'added'
test_sender = 'test@test.com'
test_options = {'resource_path': '/testpath'}
with local_app.app_context():
self.assertRaises(Exception,
get_notification_html,
notification_type=test_notification_type,
options=test_options,
sender=test_sender)
notification_type=NotificationType.OWNER_ADDED,
options={'resource_path': '/testpath'},
sender='test@test.com')
def test_get_notification_html_unsupported_type(self) -> None:
"""
Test Exception is raised if notification_type is not supported
:return:
"""
test_notification_type = 'invalid_type'
test_sender = 'test@test.com'
test_options = {'resource_name': 'testtable', 'resource_path': '/testpath'}
with local_app.app_context():
self.assertRaises(Exception,
get_notification_html,
notification_type=test_notification_type,
options=test_options,
sender=test_sender)
notification_type='invalid_type',
options={'resource_name': 'testtable', 'resource_path': '/testpath'},
sender='test@test.com')
def test_get_notification_html_added_success(self) -> None:
"""
Test successful generation of html for 'added' notification email
:return:
"""
test_notification_type = 'added'
test_sender = 'test@test.com'
test_options = {'resource_name': 'testtable', 'resource_path': '/testpath'}
with local_app.app_context():
html = get_notification_html(notification_type=test_notification_type,
html = get_notification_html(notification_type=NotificationType.OWNER_ADDED,
options=test_options,
sender=test_sender)
expectedHTML = ('Hello,<br/><br/>You have been added to the owners list of the '
......@@ -145,12 +124,11 @@ class NotificationUtilsTest(unittest.TestCase):
Test successful generation of html for 'removed' notification email
:return:
"""
test_notification_type = 'removed'
test_sender = 'test@test.com'
test_options = {'resource_name': 'testtable', 'resource_path': '/testpath'}
with local_app.app_context():
html = get_notification_html(notification_type=test_notification_type,
html = get_notification_html(notification_type=NotificationType.OWNER_REMOVED,
options=test_options,
sender=test_sender)
expectedHTML = ('Hello,<br/><br/>You have been removed from the owners list of the '
......@@ -165,7 +143,6 @@ class NotificationUtilsTest(unittest.TestCase):
all required and optional fields
:return:
"""
test_notification_type = 'requested'
test_sender = 'test@test.com'
test_options = {
'resource_name': 'testtable',
......@@ -175,7 +152,7 @@ class NotificationUtilsTest(unittest.TestCase):
'comment': 'Test Comment'
}
with local_app.app_context():
html = get_notification_html(notification_type=test_notification_type,
html = get_notification_html(notification_type=NotificationType.METADATA_REQUESTED,
options=test_options,
sender=test_sender)
expectedHTML = ('Hello,<br/><br/>test@test.com is trying to use '
......@@ -191,7 +168,6 @@ class NotificationUtilsTest(unittest.TestCase):
all required fields and 'description_requested' optional field
:return:
"""
test_notification_type = 'requested'
test_sender = 'test@test.com'
test_options = {
'resource_name': 'testtable',
......@@ -200,7 +176,7 @@ class NotificationUtilsTest(unittest.TestCase):
}
with local_app.app_context():
html = get_notification_html(notification_type=test_notification_type,
html = get_notification_html(notification_type=NotificationType.METADATA_REQUESTED,
options=test_options,
sender=test_sender)
expectedHTML = ('Hello,<br/><br/>test@test.com is trying to use '
......@@ -215,7 +191,6 @@ class NotificationUtilsTest(unittest.TestCase):
all required fields and 'fields_requested' optional field
:return:
"""
test_notification_type = 'requested'
test_sender = 'test@test.com'
test_options = {
'resource_name': 'testtable',
......@@ -224,7 +199,7 @@ class NotificationUtilsTest(unittest.TestCase):
}
with local_app.app_context():
html = get_notification_html(notification_type=test_notification_type,
html = get_notification_html(notification_type=NotificationType.METADATA_REQUESTED,
options=test_options,
sender=test_sender)
expectedHTML = ('Hello,<br/><br/>test@test.com is trying to use '
......@@ -239,7 +214,6 @@ class NotificationUtilsTest(unittest.TestCase):
all required fields and no optional fields
:return:
"""
test_notification_type = 'requested'
test_sender = 'test@test.com'
test_options = {
'resource_name': 'testtable',
......@@ -247,7 +221,7 @@ class NotificationUtilsTest(unittest.TestCase):
}
with local_app.app_context():
html = get_notification_html(notification_type=test_notification_type,
html = get_notification_html(notification_type=NotificationType.METADATA_REQUESTED,
options=test_options,
sender=test_sender)
expectedHTML = ('Hello,<br/><br/>test@test.com is trying to use '
......@@ -258,36 +232,51 @@ class NotificationUtilsTest(unittest.TestCase):
def test_get_notification_subject_added(self) -> None:
"""
Test successful executed of get_notification_subject for the 'added' notification type
Test successful executed of get_notification_subject for the OWNER_ADDED notification type
:return:
"""
result = get_notification_subject(notification_type='added', options={'resource_name': 'testtable'})
result = get_notification_subject(notification_type=NotificationType.OWNER_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
Test successful executed of get_notification_subject for the OWNER_REMOVED notification type
:return:
"""
result = get_notification_subject(notification_type='removed', options={'resource_name': 'testtable'})
result = get_notification_subject(notification_type=NotificationType.OWNER_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
Test successful executed of get_notification_subject for the METADATA_EDITED notification type
:return:
"""
result = get_notification_subject(notification_type='edited', options={'resource_name': 'testtable'})
result = get_notification_subject(notification_type=NotificationType.METADATA_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
Test successful executed of get_notification_subject for the METADATA_REQUESTED notification type
:return:
"""
result = get_notification_subject(notification_type='requested', options={'resource_name': 'testtable'})
result = get_notification_subject(notification_type=NotificationType.METADATA_REQUESTED,
options={'resource_name': 'testtable'})
self.assertEqual(result, 'Request for metadata on testtable')
def test_get_notification_subject_unsupported_type(self) -> None:
"""
Test Exception is raised if notification_type is not supported
:return:
"""
with local_app.app_context():
self.assertRaises(Exception,
get_notification_subject,
notification_type='invalid_type',
options={'resource_name': 'testtable'})
def test_get_mail_client_success(self) -> None:
"""
Test get_mail_client returns the configured mail client if one is configured
......@@ -312,7 +301,7 @@ class NotificationUtilsTest(unittest.TestCase):
"""
with local_no_notification_app.app_context():
response = send_notification(
notification_type='added',
notification_type=NotificationType.OWNER_ADDED,
options={},
recipients=['test@test.com'],
sender='test2@test.com'
......@@ -334,7 +323,7 @@ class NotificationUtilsTest(unittest.TestCase):
test_recipients = ['test@test.com']
test_sender = 'test2@test.com'
test_notification_type = 'added'
test_notification_type = NotificationType.OWNER_ADDED
test_options = {}
response = send_notification(
......@@ -372,7 +361,7 @@ class NotificationUtilsTest(unittest.TestCase):
test_sender = 'test@test.com'
test_recipients = [test_sender, 'test2@test.com']
test_notification_type = 'added'
test_notification_type = NotificationType.OWNER_ADDED
test_options = {}
expected_recipients = ['test2@test.com']
......@@ -387,7 +376,7 @@ class NotificationUtilsTest(unittest.TestCase):
sender=test_sender,
subject=mock_subject,
html=mock_html,
optional_data={'email_type': 'notification'},
optional_data={'email_type': test_notification_type},
)
def test_no_recipients_for_notification(self) -> None:
......@@ -397,7 +386,7 @@ class NotificationUtilsTest(unittest.TestCase):
"""
with local_app.app_context():
response = send_notification(
notification_type='added',
notification_type=NotificationType.OWNER_ADDED,
options={},
recipients=[],
sender='test@test.com'
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment