Unverified Commit 5a90e8a5 authored by Tamika Tannis's avatar Tamika Tannis Committed by GitHub

Notifications Improvements (#301)

* 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

* Updates for RequestMetadataForm

* Switch to generating a  for url + comment required for request column descriptions

* Update some tests + comment out ones that need update before merge

* Update some tests + comment out ones that need update before merge

* Code cleanup

* Update and rename notification_utils python tests

* Modify resource_url check + add docstrings for python tests

* Component cleanup

* Cleanup component tests

* Fix some typos
parent 523826c0
...@@ -48,18 +48,29 @@ def get_mail_client(): # type: ignore ...@@ -48,18 +48,29 @@ def get_mail_client(): # type: ignore
return mail_client return mail_client
def validate_options(*, options: Dict) -> None:
"""
Raises an Exception if the options do not contain resource_path or resource_name
"""
if options.get('resource_path') is None:
raise Exception('resource_path was not provided in the notification options')
if options.get('resource_name')is None:
raise Exception('resource_name was not provided in the notification options')
def get_notification_html(*, notification_type: str, options: Dict, sender: str) -> str: def get_notification_html(*, notification_type: str, options: Dict, sender: str) -> str:
""" """
Returns the formatted html for the notification based on the notification_type Returns the formatted html for the notification based on the notification_type
:return: A string representing the html markup to send in the notification :return: A string representing the html markup to send in the notification
""" """
resource_url = options.get('resource_url') validate_options(options=options)
if resource_url is None:
raise Exception('resource_url was not provided in the notification options')
resource_name = options.get('resource_name') url_base = app.config['FRONTEND_BASE']
if resource_name is None: resource_url = '{url_base}{resource_path}'.format(resource_path=options.get('resource_path'),
raise Exception('resource_name was not provided in the notification options') url_base=url_base)
joined_chars = resource_url[len(url_base) - 1:len(url_base) + 1]
if joined_chars.count('/') != 1:
raise Exception('Configured "FRONTEND_BASE" and "resource_path" do not form a valid url')
notification_strings = NOTIFICATION_STRINGS.get(notification_type) notification_strings = NOTIFICATION_STRINGS.get(notification_type)
if notification_strings is None: if notification_strings is None:
...@@ -67,7 +78,7 @@ def get_notification_html(*, notification_type: str, options: Dict, sender: str) ...@@ -67,7 +78,7 @@ def get_notification_html(*, notification_type: str, options: Dict, sender: str)
greeting = 'Hello,<br/>' greeting = 'Hello,<br/>'
notification = notification_strings.get('notification', '').format(resource_url=resource_url, notification = notification_strings.get('notification', '').format(resource_url=resource_url,
resource_name=resource_name, resource_name=options.get('resource_name'),
sender=sender) sender=sender)
comment = notification_strings.get('comment', '') comment = notification_strings.get('comment', '')
end_note = notification_strings.get('end_note', '') end_note = notification_strings.get('end_note', '')
...@@ -105,11 +116,12 @@ def get_notification_subject(*, notification_type: str, options: Dict) -> str: ...@@ -105,11 +116,12 @@ def get_notification_subject(*, notification_type: str, options: Dict) -> str:
:param options: data necessary to render email template content :param options: data necessary to render email template content
:return: The subject to be used with the notification :return: The subject to be used with the notification
""" """
resource_name = options.get('resource_name')
notification_subject_dict = { notification_subject_dict = {
'added': 'You are now an owner of {}'.format(options['resource_name']), 'added': 'You are now an owner of {}'.format(resource_name),
'removed': 'You have been removed as an owner of {}'.format(options['resource_name']), 'removed': 'You have been removed as an owner of {}'.format(resource_name),
'edited': 'Your dataset {}\'s metadata has been edited'.format(options['resource_name']), 'edited': 'Your dataset {}\'s metadata has been edited'.format(resource_name),
'requested': 'Request for metadata on {}'.format(options['resource_name']), 'requested': 'Request for metadata on {}'.format(resource_name),
} }
return notification_subject_dict.get(notification_type, '') return notification_subject_dict.get(notification_type, '')
......
...@@ -11,15 +11,15 @@ import AppConfig from 'config/config'; ...@@ -11,15 +11,15 @@ import AppConfig from 'config/config';
import ColumnDescEditableText from 'components/TableDetail/ColumnDescEditableText'; import ColumnDescEditableText from 'components/TableDetail/ColumnDescEditableText';
import { GlobalState } from 'ducks/rootReducer'; import { GlobalState } from 'ducks/rootReducer';
import { logClick } from 'ducks/utilMethods'; import { logClick } from 'ducks/utilMethods';
import { ToggleRequestAction } from 'ducks/notification/types'; import { OpenRequestAction } from 'ducks/notification/types';
import { openRequestDescriptionDialog } from 'ducks/notification/reducer'; import { openRequestDescriptionDialog } from 'ducks/notification/reducer';
import { TableColumn } from 'interfaces'; import { RequestMetadataType, TableColumn } from 'interfaces';
// TODO: Use css-modules instead of 'import' // TODO: Use css-modules instead of 'import'
import './styles.scss'; import './styles.scss';
interface DispatchFromProps { interface DispatchFromProps {
openRequestDescriptionDialog: () => ToggleRequestAction; openRequestDescriptionDialog: (requestMetadataType: RequestMetadataType, columnName: string) => OpenRequestAction;
} }
interface OwnProps { interface OwnProps {
...@@ -47,7 +47,7 @@ class DetailListItem extends React.Component<DetailListItemProps, DetailListItem ...@@ -47,7 +47,7 @@ class DetailListItem extends React.Component<DetailListItemProps, DetailListItem
} }
openRequest = () => { openRequest = () => {
this.props.openRequestDescriptionDialog(); this.props.openRequestDescriptionDialog(RequestMetadataType.COLUMN_DESCRIPTION, this.props.data.name);
} }
onClick = (e) => { onClick = (e) => {
...@@ -156,7 +156,7 @@ class DetailListItem extends React.Component<DetailListItemProps, DetailListItem ...@@ -156,7 +156,7 @@ class DetailListItem extends React.Component<DetailListItemProps, DetailListItem
</div> </div>
{ {
notificationsEnabled() && notificationsEnabled() &&
<Dropdown id={`detail-list-item-dropdown:${this.props.index}`} pullRight={true}> <Dropdown id={`detail-list-item-dropdown:${this.props.index}`} pullRight={true} className="column-dropdown">
<Dropdown.Toggle noCaret={true} className="dropdown-icon-more"> <Dropdown.Toggle noCaret={true} className="dropdown-icon-more">
<img className="icon icon-more"/> <img className="icon icon-more"/>
</Dropdown.Toggle> </Dropdown.Toggle>
......
...@@ -96,26 +96,30 @@ ...@@ -96,26 +96,30 @@
} }
} }
.dropdown-icon-more { .column-dropdown {
border-style: none; height: fit-content;
border-radius: 4px;
height: 22px; .dropdown-icon-more {
width: 22px; border-style: none;
padding: 4px; border-radius: 4px;
margin-right: 5px; height: 22px;
.icon { width: 22px;
background-color: $gray-light; padding: 4px;
height: 14px; margin-right: 5px;
-webkit-mask-size: 14px;
mask-size: 14px;
width: 14px;
margin: 0;
}
&:hover,
&:focus {
background-color: $gray-lightest;
.icon { .icon {
background-color: $gray-base; background-color: $gray-light;
height: 14px;
-webkit-mask-size: 14px;
mask-size: 14px;
width: 14px;
margin: 0;
}
&:hover,
&:focus {
background-color: $gray-lightest;
.icon {
background-color: $gray-base;
}
} }
} }
} }
......
...@@ -3,13 +3,15 @@ import './styles.scss'; ...@@ -3,13 +3,15 @@ import './styles.scss';
import { GlobalState } from 'ducks/rootReducer'; import { GlobalState } from 'ducks/rootReducer';
import { connect } from 'react-redux'; import { connect } from 'react-redux';
import { ToggleRequestAction } from 'ducks/notification/types'; import { OpenRequestAction } from 'ducks/notification/types';
import { openRequestDescriptionDialog } from 'ducks/notification/reducer'; import { openRequestDescriptionDialog } from 'ducks/notification/reducer';
import { bindActionCreators } from 'redux'; import { bindActionCreators } from 'redux';
import { REQUEST_DESCRIPTION } from './constants'; import { REQUEST_DESCRIPTION } from './constants';
import { RequestMetadataType } from 'interfaces';
export interface DispatchFromProps { export interface DispatchFromProps {
openRequestDescriptionDialog: () => ToggleRequestAction; openRequestDescriptionDialog: (requestMetadataType: RequestMetadataType) => OpenRequestAction;
} }
export type RequestDescriptionTextProps = DispatchFromProps; export type RequestDescriptionTextProps = DispatchFromProps;
...@@ -24,7 +26,7 @@ export class RequestDescriptionText extends React.Component<RequestDescriptionTe ...@@ -24,7 +26,7 @@ export class RequestDescriptionText extends React.Component<RequestDescriptionTe
} }
openRequest = () => { openRequest = () => {
this.props.openRequestDescriptionDialog(); this.props.openRequestDescriptionDialog(RequestMetadataType.TABLE_DESCRIPTION);
} }
render() { render() {
......
...@@ -5,12 +5,13 @@ import { shallow } from 'enzyme'; ...@@ -5,12 +5,13 @@ import { shallow } from 'enzyme';
import { RequestDescriptionText, mapDispatchToProps, RequestDescriptionTextProps } from '../'; import { RequestDescriptionText, mapDispatchToProps, RequestDescriptionTextProps } from '../';
import globalState from 'fixtures/globalState'; import globalState from 'fixtures/globalState';
import { REQUEST_DESCRIPTION } from '../constants'; import { REQUEST_DESCRIPTION } from '../constants';
import { RequestMetadataType } from 'interfaces';
describe('RequestDescriptionText', () => { describe('RequestDescriptionText', () => {
const setup = (propOverrides?: Partial<RequestDescriptionTextProps>) => { const setup = (propOverrides?: Partial<RequestDescriptionTextProps>) => {
const props: RequestDescriptionTextProps = { const props: RequestDescriptionTextProps = {
openRequestDescriptionDialog: jest.fn(), openRequestDescriptionDialog: jest.fn(),
...propOverrides, ...propOverrides,
}; };
const wrapper = shallow<RequestDescriptionText>(<RequestDescriptionText {...props} />) const wrapper = shallow<RequestDescriptionText>(<RequestDescriptionText {...props} />)
return {props, wrapper} return {props, wrapper}
...@@ -21,10 +22,10 @@ describe('RequestDescriptionText', () => { ...@@ -21,10 +22,10 @@ describe('RequestDescriptionText', () => {
const { props, wrapper } = setup(); const { props, wrapper } = setup();
const openRequestDescriptionDialogSpy = jest.spyOn(props, 'openRequestDescriptionDialog'); const openRequestDescriptionDialogSpy = jest.spyOn(props, 'openRequestDescriptionDialog');
wrapper.instance().openRequest(); wrapper.instance().openRequest();
expect(openRequestDescriptionDialogSpy).toHaveBeenCalled(); expect(openRequestDescriptionDialogSpy).toHaveBeenCalledWith(RequestMetadataType.TABLE_DESCRIPTION);
}); });
}); });
describe('render', () => { describe('render', () => {
it('renders Request Description button with correct text', () => { it('renders Request Description button with correct text', () => {
const { props, wrapper } = setup(); const { props, wrapper } = setup();
...@@ -40,7 +41,7 @@ describe('RequestDescriptionText', () => { ...@@ -40,7 +41,7 @@ describe('RequestDescriptionText', () => {
dispatch = jest.fn(() => Promise.resolve()); dispatch = jest.fn(() => Promise.resolve());
result = mapDispatchToProps(dispatch); result = mapDispatchToProps(dispatch);
}); });
it('sets openRequestDescriptionDialog on the props', () => { it('sets openRequestDescriptionDialog on the props', () => {
expect(result.openRequestDescriptionDialog).toBeInstanceOf(Function); expect(result.openRequestDescriptionDialog).toBeInstanceOf(Function);
}); });
......
...@@ -11,3 +11,10 @@ export const SEND_BUTTON = 'Send Request'; ...@@ -11,3 +11,10 @@ export const SEND_BUTTON = 'Send Request';
export const SEND_INPROGRESS_MESSAGE = 'Your request is being sent...'; export const SEND_INPROGRESS_MESSAGE = 'Your request is being sent...';
export const SEND_FAILURE_MESSAGE = 'Your request was not successfully sent, please try again'; export const SEND_FAILURE_MESSAGE = 'Your request was not successfully sent, please try again';
export const SEND_SUCCESS_MESSAGE = 'Your request has been successfully sent'; export const SEND_SUCCESS_MESSAGE = 'Your request has been successfully sent';
export const RECIPIENT_LIST_DELIMETER = ', ';
export const COMMENT_PLACEHOLDER_DEFAULT = 'Please enter more information about your request';
export const COMMENT_PLACEHOLDER_COLUMN = 'Please enter which column(s) need an improved description';
export const COLUMN_REQUESTED_COMMENT_PREFIX = 'Description requested for column: ';
...@@ -4,7 +4,7 @@ import { bindActionCreators } from 'redux'; ...@@ -4,7 +4,7 @@ import { bindActionCreators } from 'redux';
import './styles.scss'; import './styles.scss';
import { NotificationType, SendNotificationOptions, SendingState } from 'interfaces'; import { NotificationType, RequestMetadataType, SendNotificationOptions, SendingState, TableMetadata } from 'interfaces';
import FlashMessage from 'components/common/FlashMessage' import FlashMessage from 'components/common/FlashMessage'
...@@ -17,31 +17,37 @@ import { ...@@ -17,31 +17,37 @@ import {
REQUEST_TYPE, REQUEST_TYPE,
TABLE_DESCRIPTION, TABLE_DESCRIPTION,
COLUMN_DESCRIPTIONS, COLUMN_DESCRIPTIONS,
COLUMN_REQUESTED_COMMENT_PREFIX,
COMMENT_PLACEHOLDER_COLUMN,
COMMENT_PLACEHOLDER_DEFAULT,
ADDITIONAL_DETAILS, ADDITIONAL_DETAILS,
RECIPIENT_LIST_DELIMETER,
SEND_BUTTON, SEND_BUTTON,
SEND_FAILURE_MESSAGE, SEND_FAILURE_MESSAGE,
SEND_INPROGRESS_MESSAGE, SEND_INPROGRESS_MESSAGE,
SEND_SUCCESS_MESSAGE, SEND_SUCCESS_MESSAGE,
} from './constants' } from './constants'
import { ToggleRequestAction, SubmitNotificationRequest } from 'ducks/notification/types'; import { CloseRequestAction, OpenRequestAction, SubmitNotificationRequest } from 'ducks/notification/types';
import { closeRequestDescriptionDialog, submitNotification } from 'ducks/notification/reducer'; import { closeRequestDescriptionDialog, submitNotification } from 'ducks/notification/reducer';
interface StateFromProps { interface StateFromProps {
columnName?: string;
requestMetadataType?: RequestMetadataType;
userEmail: string; userEmail: string;
displayName: string; tableMetadata: TableMetadata;
tableOwners: Array<string>; tableOwners: string[];
requestIsOpen: boolean; requestIsOpen: boolean;
sendState: SendingState; sendState: SendingState;
} }
export interface DispatchFromProps { export interface DispatchFromProps {
submitNotification: ( submitNotification: (
recipients: Array<string>, recipients: string[],
sender: string, sender: string,
notificationType: NotificationType, notificationType: NotificationType,
options?: SendNotificationOptions options?: SendNotificationOptions
) => SubmitNotificationRequest; ) => SubmitNotificationRequest;
closeRequestDescriptionDialog: () => ToggleRequestAction; closeRequestDescriptionDialog: () => CloseRequestAction;
} }
export type RequestMetadataProps = StateFromProps & DispatchFromProps; export type RequestMetadataProps = StateFromProps & DispatchFromProps;
...@@ -91,19 +97,20 @@ export class RequestMetadataForm extends React.Component<RequestMetadataProps, R ...@@ -91,19 +97,20 @@ export class RequestMetadataForm extends React.Component<RequestMetadataProps, R
const form = document.getElementById("RequestForm") as HTMLFormElement; const form = document.getElementById("RequestForm") as HTMLFormElement;
const formData = new FormData(form); const formData = new FormData(form);
const recipientString = formData.get('recipients') as string const recipientString = formData.get('recipients') as string
const recipients = recipientString.split(",") const recipients = recipientString.split(RECIPIENT_LIST_DELIMETER.trim())
const sender = formData.get('sender') as string; const sender = formData.get('sender') as string;
const descriptionRequested = formData.get('table-description') === "on"; const descriptionRequested = formData.get('table-description') === "on";
const fieldsRequested = formData.get('column-description') === "on"; const fieldsRequested = formData.get('column-description') === "on";
const comment = formData.get('comment') as string; const comment = formData.get('comment') as string;
const { cluster, database, schema, table_name } = this.props.tableMetadata;
this.props.submitNotification( this.props.submitNotification(
recipients, recipients,
sender, sender,
NotificationType.METADATA_REQUESTED, NotificationType.METADATA_REQUESTED,
{ {
comment, comment,
resource_name: this.props.displayName, resource_name: `${schema}.${table_name}`,
resource_url: window.location.href, resource_path: `/table_detail/${cluster}/${database}/${schema}/${table_name}`,
description_requested: descriptionRequested, description_requested: descriptionRequested,
fields_requested: fieldsRequested, fields_requested: fieldsRequested,
} }
...@@ -111,14 +118,19 @@ export class RequestMetadataForm extends React.Component<RequestMetadataProps, R ...@@ -111,14 +118,19 @@ export class RequestMetadataForm extends React.Component<RequestMetadataProps, R
}; };
render() { render() {
if (this.props.sendState !== SendingState.IDLE) { const { columnName, requestIsOpen, requestMetadataType, sendState, tableMetadata, tableOwners, userEmail } = this.props;
const tableDescriptionNeeded = requestMetadataType === RequestMetadataType.TABLE_DESCRIPTION;
const colDescriptionNeeded = requestMetadataType === RequestMetadataType.COLUMN_DESCRIPTION;
const defaultComment = columnName ? `${COLUMN_REQUESTED_COMMENT_PREFIX}${columnName}`: '';
if (sendState !== SendingState.IDLE) {
return ( return (
<div className="request-component"> <div className="request-component">
{this.renderFlashMessage()} {this.renderFlashMessage()}
</div> </div>
); );
} }
if (!this.props.requestIsOpen) { if (!requestIsOpen) {
return (null); return (null);
} }
return ( return (
...@@ -130,20 +142,43 @@ export class RequestMetadataForm extends React.Component<RequestMetadataProps, R ...@@ -130,20 +142,43 @@ export class RequestMetadataForm extends React.Component<RequestMetadataProps, R
<form onSubmit={ this.submitNotification } id="RequestForm"> <form onSubmit={ this.submitNotification } id="RequestForm">
<div id="sender-form-group" className="form-group"> <div id="sender-form-group" className="form-group">
<label>{FROM_LABEL}</label> <label>{FROM_LABEL}</label>
<input type="email" name="sender" className="form-control" required={true} value={this.props.userEmail} readOnly={true}/> <input type="email" name="sender" className="form-control" required={true} value={userEmail} readOnly={true}/>
</div> </div>
<div id="recipients-form-group" className="form-group"> <div id="recipients-form-group" className="form-group">
<label>{TO_LABEL}</label> <label>{TO_LABEL}</label>
<input type="email" name="recipients" className="form-control" required={true} multiple={true} defaultValue={this.props.tableOwners.join(",")}/> <input type="text" name="recipients" className="form-control" required={true} multiple={true} defaultValue={tableOwners.join(RECIPIENT_LIST_DELIMETER)}/>
</div> </div>
<div id="request-type-form-group" className="form-group"> <div id="request-type-form-group" className="form-group">
<label>{REQUEST_TYPE}</label> <label>{REQUEST_TYPE}</label>
<label className="select-label"><input type="checkbox" name="table-description"/>{TABLE_DESCRIPTION}</label> <label className="select-label">
<label className="select-label"><input type="checkbox" name="column-description"/>{COLUMN_DESCRIPTIONS}</label> <input
type="checkbox"
name="table-description"
defaultChecked={tableDescriptionNeeded}
/>
{TABLE_DESCRIPTION}
</label>
<label className="select-label">
<input
type="checkbox"
name="column-description"
defaultChecked={colDescriptionNeeded}
/>
{COLUMN_DESCRIPTIONS}
</label>
</div> </div>
<div id="additional-comments-form-group" className="form-group"> <div id="additional-comments-form-group" className="form-group">
<label>{ADDITIONAL_DETAILS}</label> <label>{ADDITIONAL_DETAILS}</label>
<textarea className="form-control" name="comment" rows={ 8 } maxLength={ 2000 } /> <textarea
className="form-control"
name="comment"
placeholder={ colDescriptionNeeded ? COMMENT_PLACEHOLDER_COLUMN : COMMENT_PLACEHOLDER_DEFAULT }
required={ colDescriptionNeeded }
rows={ 8 }
maxLength={ 2000 }
>
{ defaultComment }
</textarea>
</div> </div>
<button id="submit-request-button" className="btn btn-primary" type="submit"> <button id="submit-request-button" className="btn btn-primary" type="submit">
{SEND_BUTTON} {SEND_BUTTON}
...@@ -156,16 +191,22 @@ export class RequestMetadataForm extends React.Component<RequestMetadataProps, R ...@@ -156,16 +191,22 @@ export class RequestMetadataForm extends React.Component<RequestMetadataProps, R
export const mapStateToProps = (state: GlobalState) => { export const mapStateToProps = (state: GlobalState) => {
const userEmail = state.user.loggedInUser.email; const userEmail = state.user.loggedInUser.email;
const displayName = `${state.tableMetadata.tableData.schema}.${state.tableMetadata.tableData.table_name}`; const { columnName, requestMetadataType, requestIsOpen, sendState } = state.notification;
const ownerObj = state.tableMetadata.tableOwners.owners; const ownerObj = state.tableMetadata.tableOwners.owners;
const { requestIsOpen, sendState } = state.notification; const mappedProps = {
return {
userEmail, userEmail,
displayName,
requestIsOpen, requestIsOpen,
sendState, sendState,
tableMetadata: state.tableMetadata.tableData,
tableOwners: Object.keys(ownerObj), tableOwners: Object.keys(ownerObj),
}; };
if (columnName) {
mappedProps['columnName'] = columnName;
}
if (requestMetadataType) {
mappedProps['requestMetadataType'] = requestMetadataType;
}
return mappedProps;
}; };
export const mapDispatchToProps = (dispatch: any) => { export const mapDispatchToProps = (dispatch: any) => {
......
@import 'variables'; @import 'variables';
input[type="email"] {
color: $text-medium !important;
}
.request-component { .request-component {
box-shadow: 0 0 24px -2px rgba(0, 0, 0, .2); box-shadow: 0 0 24px -2px rgba(0, 0, 0, .2);
border-radius: 6px; border-radius: 6px;
...@@ -33,6 +29,10 @@ input[type="email"] { ...@@ -33,6 +29,10 @@ input[type="email"] {
font-weight: $font-weight-body-regular; font-weight: $font-weight-body-regular;
} }
input,
textarea {
color: $text-medium;
}
input[type="checkbox"] { input[type="checkbox"] {
margin-right: 8px; margin-right: 8px;
} }
......
...@@ -12,7 +12,7 @@ describe('sendNotification', () => { ...@@ -12,7 +12,7 @@ describe('sendNotification', () => {
const testNotificationType = NotificationType.OWNER_ADDED; const testNotificationType = NotificationType.OWNER_ADDED;
const testOptions = { const testOptions = {
resource_name: 'testResource', resource_name: 'testResource',
resource_url: 'https://testResource.com', resource_path: '/testResource',
description_requested: false, description_requested: false,
fields_requested: false, fields_requested: false,
}; };
......
import { NotificationType, SendNotificationOptions, SendingState } from 'interfaces' import { NotificationType, RequestMetadataType, SendNotificationOptions, SendingState } from 'interfaces'
import { import {
SubmitNotification, SubmitNotification,
SubmitNotificationRequest, SubmitNotificationRequest,
SubmitNotificationResponse, SubmitNotificationResponse,
ToggleRequest, ToggleRequest,
ToggleRequestAction, CloseRequestAction,
OpenRequestAction,
} from './types'; } from './types';
/* ACTIONS */ /* ACTIONS */
export function submitNotification(recipients: Array<string>, sender: string, notificationType: NotificationType, options?: SendNotificationOptions): SubmitNotificationRequest { export function submitNotification(recipients: Array<string>, sender: string, notificationType: NotificationType, options?: SendNotificationOptions): SubmitNotificationRequest {
return { return {
payload: { payload: {
recipients, recipients,
sender, sender,
notificationType, notificationType,
options options
}, },
type: SubmitNotification.REQUEST, type: SubmitNotification.REQUEST,
}; };
...@@ -31,20 +32,34 @@ export function submitNotificationSuccess(): SubmitNotificationResponse { ...@@ -31,20 +32,34 @@ export function submitNotificationSuccess(): SubmitNotificationResponse {
}; };
}; };
export function closeRequestDescriptionDialog(): ToggleRequestAction { export function closeRequestDescriptionDialog(): CloseRequestAction {
return { return {
type: ToggleRequest.CLOSE, type: ToggleRequest.CLOSE
}; };
}; };
export function openRequestDescriptionDialog(): ToggleRequestAction { export function openRequestDescriptionDialog(requestMetadataType: RequestMetadataType, columnName?: string): OpenRequestAction {
if (columnName) {
return {
type: ToggleRequest.OPEN,
payload: {
columnName,
requestMetadataType
}
}
}
return { return {
type: ToggleRequest.OPEN, type: ToggleRequest.OPEN,
payload: {
requestMetadataType
}
} }
} }
/* REDUCER */ /* REDUCER */
export interface NotificationReducerState { export interface NotificationReducerState {
columnName?: string,
requestMetadataType?: RequestMetadataType,
requestIsOpen: boolean, requestIsOpen: boolean,
sendState: SendingState, sendState: SendingState,
}; };
...@@ -68,6 +83,7 @@ export default function reducer(state: NotificationReducerState = initialState, ...@@ -68,6 +83,7 @@ export default function reducer(state: NotificationReducerState = initialState,
} }
case SubmitNotification.REQUEST: case SubmitNotification.REQUEST:
return { return {
...state,
requestIsOpen: false, requestIsOpen: false,
sendState: SendingState.WAITING, sendState: SendingState.WAITING,
} }
...@@ -77,10 +93,16 @@ export default function reducer(state: NotificationReducerState = initialState, ...@@ -77,10 +93,16 @@ export default function reducer(state: NotificationReducerState = initialState,
sendState: SendingState.IDLE, sendState: SendingState.IDLE,
} }
case ToggleRequest.OPEN: case ToggleRequest.OPEN:
return { const newState = {
requestMetadataType: (<OpenRequestAction>action).payload.requestMetadataType,
requestIsOpen: true, requestIsOpen: true,
sendState: SendingState.IDLE, sendState: SendingState.IDLE,
} }
const columnName = (<OpenRequestAction>action).payload.columnName;
if (columnName) {
newState['columnName'] = columnName;
}
return newState;
default: default:
return state; return state;
} }
......
import { testSaga } from 'redux-saga-test-plan'; import { testSaga } from 'redux-saga-test-plan';
import { NotificationType, SendingState } from 'interfaces'; import { NotificationType, RequestMetadataType, SendingState } from 'interfaces';
import * as API from '../api/v0'; import * as API from '../api/v0';
import reducer, { import reducer, {
...@@ -23,7 +23,7 @@ const testSender = 'user2@test.com'; ...@@ -23,7 +23,7 @@ const testSender = 'user2@test.com';
const testNotificationType = NotificationType.OWNER_ADDED; const testNotificationType = NotificationType.OWNER_ADDED;
const testOptions = { const testOptions = {
resource_name: 'testResource', resource_name: 'testResource',
resource_url: 'https://testResource.com', resource_path: '/testResource',
description_requested: false, description_requested: false,
fields_requested: false, fields_requested: false,
}; };
...@@ -55,9 +55,23 @@ describe('notifications ducks', () => { ...@@ -55,9 +55,23 @@ describe('notifications ducks', () => {
expect(action.type).toBe(ToggleRequest.CLOSE); expect(action.type).toBe(ToggleRequest.CLOSE);
}); });
it('openRequestDescriptionDialog - returns the action to trigger the request description to opem', () => { it('openRequestDescriptionDialog - returns the action to trigger the request description to open', () => {
const action = openRequestDescriptionDialog(); const testType = RequestMetadataType.TABLE_DESCRIPTION;
expect(action.type).toBe(ToggleRequest.OPEN); const action = openRequestDescriptionDialog(testType);
const { payload, type } = action;
expect(type).toBe(ToggleRequest.OPEN);
expect(payload.requestMetadataType).toBe(testType);
expect(payload.columnName).toBe(undefined);
});
it('openRequestDescriptionDialog w/ columnName - returns the action to trigger the request description to open', () => {
const testType = RequestMetadataType.TABLE_DESCRIPTION;
const testName = 'columnName';
const action = openRequestDescriptionDialog(testType, testName);
const { payload, type } = action;
expect(type).toBe(ToggleRequest.OPEN);
expect(payload.requestMetadataType).toBe(testType);
expect(payload.columnName).toBe(testName);
}); });
}); });
...@@ -73,8 +87,18 @@ describe('notifications ducks', () => { ...@@ -73,8 +87,18 @@ describe('notifications ducks', () => {
expect(reducer(testState, { type: 'INVALID.ACTION' })).toEqual(testState); expect(reducer(testState, { type: 'INVALID.ACTION' })).toEqual(testState);
}); });
it('should handle ToggleRequest.OPEN', () => { it('should handle ToggleRequest.OPEN without columnName', () => {
expect(reducer(testState, openRequestDescriptionDialog())).toEqual({ expect(reducer(testState, openRequestDescriptionDialog(RequestMetadataType.TABLE_DESCRIPTION))).toEqual({
requestMetadataType: RequestMetadataType.TABLE_DESCRIPTION,
requestIsOpen: true,
sendState: SendingState.IDLE,
});
});
it('should handle ToggleRequest.OPEN with columnName', () => {
expect(reducer(testState, openRequestDescriptionDialog(RequestMetadataType.TABLE_DESCRIPTION, 'col'))).toEqual({
columnName: 'col',
requestMetadataType: RequestMetadataType.TABLE_DESCRIPTION,
requestIsOpen: true, requestIsOpen: true,
sendState: SendingState.IDLE, sendState: SendingState.IDLE,
}); });
...@@ -97,6 +121,7 @@ describe('notifications ducks', () => { ...@@ -97,6 +121,7 @@ describe('notifications ducks', () => {
it('should handle SubmitNotification.REQUEST', () => { it('should handle SubmitNotification.REQUEST', () => {
const action = submitNotification(testRecipients, testSender, testNotificationType, testOptions); const action = submitNotification(testRecipients, testSender, testNotificationType, testOptions);
expect(reducer(testState, action)).toEqual({ expect(reducer(testState, action)).toEqual({
...testState,
requestIsOpen: false, requestIsOpen: false,
sendState: SendingState.WAITING, sendState: SendingState.WAITING,
}); });
......
import { NotificationType, SendNotificationOptions } from 'interfaces' import { NotificationType, RequestMetadataType, SendNotificationOptions } from 'interfaces'
export enum SubmitNotification { export enum SubmitNotification {
REQUEST = 'amundsen/notification/SUBMIT_NOTIFICATION_REQUEST', REQUEST = 'amundsen/notification/SUBMIT_NOTIFICATION_REQUEST',
...@@ -9,7 +9,7 @@ export enum SubmitNotification { ...@@ -9,7 +9,7 @@ export enum SubmitNotification {
export interface SubmitNotificationRequest { export interface SubmitNotificationRequest {
type: SubmitNotification.REQUEST; type: SubmitNotification.REQUEST;
payload: { payload: {
recipients: Array<string>, recipients: string[],
sender: string, sender: string,
notificationType: NotificationType, notificationType: NotificationType,
options?: SendNotificationOptions options?: SendNotificationOptions
...@@ -24,6 +24,14 @@ export enum ToggleRequest { ...@@ -24,6 +24,14 @@ export enum ToggleRequest {
CLOSE = 'close', CLOSE = 'close',
}; };
export interface ToggleRequestAction { export interface OpenRequestAction {
type: ToggleRequest.OPEN | ToggleRequest.CLOSE; type: ToggleRequest.OPEN,
payload: {
columnName?: string,
requestMetadataType: RequestMetadataType,
}
};
export interface CloseRequestAction {
type: ToggleRequest.CLOSE
}; };
...@@ -41,12 +41,12 @@ export function getTableTagsFromResponseData(responseData: API.TableDataAPI): Ta ...@@ -41,12 +41,12 @@ export function getTableTagsFromResponseData(responseData: API.TableDataAPI): Ta
/** /**
* Creates post data for sending a notification to owners when they are added/removed * Creates post data for sending a notification to owners when they are added/removed
*/ */
export function createOwnerNotificationData(payload: UpdateOwnerPayload, resourceName: string) { export function createOwnerNotificationData(payload: UpdateOwnerPayload, tableData: TableMetadata) {
return { return {
notificationType: payload.method === UpdateMethod.PUT ? NotificationType.OWNER_ADDED : NotificationType.OWNER_REMOVED, notificationType: payload.method === UpdateMethod.PUT ? NotificationType.OWNER_ADDED : NotificationType.OWNER_REMOVED,
options: { options: {
resource_name: resourceName, resource_name: `${tableData.schema}.${tableData.table_name}`,
resource_url: window.location.href, resource_path: `/table_detail/${tableData.cluster}/${tableData.database}/${tableData.schema}/${tableData.table_name}`
}, },
recipients: [payload.id], recipients: [payload.id],
}; };
......
...@@ -73,29 +73,34 @@ describe('helpers', () => { ...@@ -73,29 +73,34 @@ describe('helpers', () => {
}); });
describe('createOwnerNotificationData', () => { describe('createOwnerNotificationData', () => {
let testData;
let testId;
let expectedName;
let expectedPath;
beforeAll(() => {
testData = globalState.tableMetadata.tableData;
testId = 'testId@test.com';
expectedName = `${testData.schema}.${testData.table_name}`;
expectedPath = `/table_detail/${testData.cluster}/${testData.database}/${testData.schema}/${testData.table_name}`;
});
it('creates correct request data for PUT', () => { it('creates correct request data for PUT', () => {
const testId = 'testId@test.com'; expect(Helpers.createOwnerNotificationData({ method: UpdateMethod.PUT, id: testId }, testData)).toMatchObject({
const testMethod = UpdateMethod.PUT;
const testName = 'schema.tableName';
expect(Helpers.createOwnerNotificationData({ method: testMethod, id: testId }, testName)).toMatchObject({
notificationType: NotificationType.OWNER_ADDED, notificationType: NotificationType.OWNER_ADDED,
options: { options: {
resource_name: testName, resource_name: expectedName,
resource_url: window.location.href, resource_path: expectedPath,
}, },
recipients: [testId], recipients: [testId],
}); });
}); });
it('creates correct request data for DELETE', () => { it('creates correct request data for DELETE', () => {
const testId = 'testId@test.com'; expect(Helpers.createOwnerNotificationData({ method: UpdateMethod.DELETE, id: testId }, testData)).toMatchObject({
const testMethod = UpdateMethod.DELETE;
const testName = 'schema.tableName';
expect(Helpers.createOwnerNotificationData({ method: testMethod, id: testId }, testName)).toMatchObject({
notificationType: NotificationType.OWNER_REMOVED, notificationType: NotificationType.OWNER_REMOVED,
options: { options: {
resource_name: testName, resource_name: expectedName,
resource_url: window.location.href, resource_path: expectedPath,
}, },
recipients: [testId], recipients: [testId],
}); });
......
...@@ -89,13 +89,13 @@ export function getTableOwners(tableKey: string) { ...@@ -89,13 +89,13 @@ export function getTableOwners(tableKey: string) {
} }
/* TODO: Typing return type generates redux-saga related type error that need more dedicated debugging */ /* TODO: Typing return type generates redux-saga related type error that need more dedicated debugging */
export function generateOwnerUpdateRequests(updateArray: UpdateOwnerPayload[], tableKey: string, resourceName: string) { export function generateOwnerUpdateRequests(updateArray: UpdateOwnerPayload[], tableData: TableMetadata) {
const updateRequests = []; const updateRequests = [];
/* Create the request for updating each owner*/ /* Create the request for updating each owner*/
updateArray.forEach((item) => { updateArray.forEach((item) => {
const updatePayload = createOwnerUpdatePayload(item, tableKey); const updatePayload = createOwnerUpdatePayload(item, tableData.key);
const notificationData = createOwnerNotificationData(item, resourceName); const notificationData = createOwnerNotificationData(item, tableData);
/* Chain requests to send notification on success to desired users */ /* Chain requests to send notification on success to desired users */
const request = const request =
......
...@@ -12,7 +12,7 @@ export function* updateTableOwnerWorker(action: UpdateTableOwnerRequest): SagaIt ...@@ -12,7 +12,7 @@ export function* updateTableOwnerWorker(action: UpdateTableOwnerRequest): SagaIt
const state = yield select(); const state = yield select();
const tableData = state.tableMetadata.tableData; const tableData = state.tableMetadata.tableData;
try { try {
const requestList = API.generateOwnerUpdateRequests(payload.updateArray, tableData.key, `${tableData.schema}.${tableData.table_name}`); const requestList = API.generateOwnerUpdateRequests(payload.updateArray, tableData);
yield all(requestList); yield all(requestList);
const newOwners = yield call(API.getTableOwners, tableData.key); const newOwners = yield call(API.getTableOwners, tableData.key);
yield put(updateTableOwnerSuccess(newOwners)); yield put(updateTableOwnerSuccess(newOwners));
......
...@@ -131,7 +131,7 @@ describe('tableMetadata:owners ducks', () => { ...@@ -131,7 +131,7 @@ describe('tableMetadata:owners ducks', () => {
sagaTest = (action) => { sagaTest = (action) => {
return testSaga(updateTableOwnerWorker, action) return testSaga(updateTableOwnerWorker, action)
.next().select() .next().select()
.next(globalState).all(API.generateOwnerUpdateRequests(updatePayload, globalState.tableMetadata.tableData.key, globalState.tableMetadata.tableData.table_name)) .next(globalState).all(API.generateOwnerUpdateRequests(updatePayload, globalState.tableMetadata.tableData))
.next().call(API.getTableOwners, globalState.tableMetadata.tableData.key) .next().call(API.getTableOwners, globalState.tableMetadata.tableData.key)
.next(expectedOwners).put(updateTableOwnerSuccess(expectedOwners)); .next(expectedOwners).put(updateTableOwnerSuccess(expectedOwners));
}; };
......
import { GlobalState } from 'ducks/rootReducer'; import { GlobalState } from 'ducks/rootReducer';
import { ResourceType, SendingState } from 'interfaces'; import { RequestMetadataType, ResourceType, SendingState } from 'interfaces';
const globalState: GlobalState = { const globalState: GlobalState = {
announcements: { announcements: {
......
...@@ -6,9 +6,14 @@ export enum NotificationType { ...@@ -6,9 +6,14 @@ export enum NotificationType {
METADATA_REQUESTED = 'requested', METADATA_REQUESTED = 'requested',
} }
export enum RequestMetadataType {
COLUMN_DESCRIPTION = 'columnDescriptionRequested',
TABLE_DESCRIPTION = 'tableDescriptionRequested',
}
export interface SendNotificationOptions { export interface SendNotificationOptions {
resource_name: string, resource_name: string,
resource_url: string, resource_path: string,
description_requested: boolean, description_requested: boolean,
fields_requested: boolean, fields_requested: boolean,
comment?: string, comment?: string,
......
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