Feedback Component & MailClient Improvements (#131)

* Update feedback forms + add unit test; Update base mail client to take form data

* Complete test coverage for Feedback

* Code cleanup + extract string constants

* Update base mail client

* Correctly propagate MailClient failures + inform user in the UI

* Revert change of a error level

* Lint + Text fixes

* Cleanup Feedback tests, update Feedback close icon + add docstring for send_email

* Renamed incorrect reference to form_data -> optional_data

* Move style for close button to _buttons_default

amundsen_application/api/mail/v0.py

@@ -36,6 +36,7 @@ def feedback() -> Response:
         repro_steps = data.get('repro-steps')
         feature_summary = data.get('feature-summary')
         value_prop = data.get('value-prop')
+        subject = data.get('subject') or data.get('feedback-type')
 
         _feedback(feedback_type=feedback_type,
                   rating=rating,
@@ -43,9 +44,10 @@ def feedback() -> Response:
                   bug_summary=bug_summary,
                   repro_steps=repro_steps,
                   feature_summary=feature_summary,
-                  value_prop=value_prop)
+                  value_prop=value_prop,
+                  subject=subject)
 
-        response = mail_client.send_email(text=text_content, html=html_content)
+        response = mail_client.send_email(subject=subject, text=text_content, html=html_content, optional_data=data)
         status_code = response.status_code
 
         if status_code == HTTPStatus.OK:
@@ -69,6 +71,7 @@ def _feedback(*,
               bug_summary: str,
               repro_steps: str,
               feature_summary: str,
-              value_prop: str) -> None:
+              value_prop: str,
+              subject: str) -> None:
     """ Logs the content of the feedback form """
     pass  # pragma: no cover

amundsen_application/base/base_mail_client.py

 import abc
-from typing import List
+from typing import Dict, List
 
 from flask import Response
 
@@ -10,5 +10,21 @@ class BaseMailClient(abc.ABC):
         pass  # pragma: no cover
 
     @abc.abstractmethod
-    def send_email(self, sender: str, recipients: List[str], subject: str, text: str, html: str) -> Response:
+    def send_email(self,
+                   sender: str,
+                   recipients: List[str],
+                   subject: str,
+                   text: str,
+                   html: str,
+                   optional_data: Dict) -> Response:
+        """
+        Sends an email using the following parameters
+        :param sender: The sending address associated with the email
+        :param recipients: A list of receipients for the email
+        :param subject: The subject of the email
+        :param text: Plain text email content
+        :param html: HTML email content
+        :param optional_data: A dictionary of any values needed for custom implementations
+        :return:
+        """
         raise NotImplementedError  # pragma: no cover

amundsen_application/static/css/_buttons-default.scss

@@ -91,6 +91,29 @@
     }
   }
 
+  &.btn-close {
+    height: 18px;
+    width: 18px;
+    margin: 4px 0 0 0;
+    padding: 0;
+
+    background-color: $gray-light;
+    border: none;
+    mask-image: url('/static/images/icons/Close.svg');
+    -webkit-mask-image: url('/static/images/icons/Close.svg');
+    mask-size: contain;
+    -webkit-mask-size: contain;
+    mask-position: center;
+    -webkit-mask-position: center;
+    mask-size: 110%;
+    -webkit-mask-size: 110%;
+
+    &:focus,
+    &:not(.disabled):hover,
+    &:not([disabled]):hover {
+      background-color: $gray-dark;
+    }
+  }
 
   &.disabled,
   &:disabled {

amundsen_application/static/jest.config.js

@@ -7,10 +7,10 @@ module.exports = {
           statements: 50, // 100
       },
       './js/components': {
-          branches: 30, // 75
-          functions: 30, // 75
-          lines: 35, // 75
-          statements: 35, // 75
+          branches: 35, // 75
+          functions: 40, // 75
+          lines: 45, // 75
+          statements: 45, // 75
       },
       './js/ducks': {
           branches: 0, // 75

amundsen_application/static/js/components/Feedback/FeedbackForm/BugReportFeedbackForm/index.tsx

@@ -7,28 +7,40 @@ import AbstractFeedbackForm, { DispatchFromProps, StateFromProps } from '../../F
 import { GlobalState } from 'ducks/rootReducer';
 import { submitFeedback, resetFeedback } from 'ducks/feedback/reducer';
 
+import {
+  BUG_SUMMARY_LABEL,
+  BUG_SUMMARY_PLACEHOLDER,
+  REPRO_STEPS_LABEL,
+  REPRO_STEPS_PLACEHOLDER,
+  SUBJECT_LABEL,
+  SUBJECT_PLACEHOLDER,
+  SUBMIT_TEXT,
+} from '../../constants';
+
 export class BugReportFeedbackForm extends AbstractFeedbackForm {
   constructor(props) {
-    super(props)
+    super(props);
   }
-
+  
   renderCustom() {
     return (
       <form id={AbstractFeedbackForm.FORM_ID} onSubmit={ this.submitForm }>
         <input type="hidden" name="feedback-type" value="Bug Report"/>
         <div className="form-group">
-          <label>Bug Summary</label>
+          <label>{SUBJECT_LABEL}</label>
+          <input type="text" name="subject" className="form-control" required={ true } placeholder={SUBJECT_PLACEHOLDER} />
+        </div>
+        <div className="form-group">
+          <label>{BUG_SUMMARY_LABEL}</label>
           <textarea name="bug-summary" className="form-control" required={ true }
-                    rows={3} maxLength={ 2000 } placeholder="What went wrong?"/>
+                    rows={3} maxLength={ 2000 } placeholder={BUG_SUMMARY_PLACEHOLDER}/>
         </div>
         <div className="form-group">
-          <label>Reproduction Steps</label>
+          <label>{REPRO_STEPS_LABEL}</label>
           <textarea name="repro-steps" className="form-control" rows={5} required={ true }
-                    maxLength={ 2000 } placeholder="What you did to encounter this bug?"/>
-        </div>
-        <div>
-          <button className="btn btn-default submit" type="submit">Submit</button>
+                    maxLength={ 2000 } placeholder={REPRO_STEPS_PLACEHOLDER}/>
         </div>
+        <button className="btn btn-default submit" type="submit">{SUBMIT_TEXT}</button>
       </form>
     );
   }

amundsen_application/static/js/components/Feedback/FeedbackForm/BugReportFeedbackForm/tests/index.spec.tsx

+import * as React from 'react';
+
+import { shallow } from 'enzyme';
+
+import AbstractFeedbackForm, { FeedbackFormProps } from 'components/Feedback/FeedbackForm';
+import { SendingState } from 'components/Feedback/types';
+import { BugReportFeedbackForm, mapDispatchToProps, mapStateToProps } from '../';
+import {
+  BUG_SUMMARY_LABEL,
+  BUG_SUMMARY_PLACEHOLDER,
+  REPRO_STEPS_LABEL,
+  REPRO_STEPS_PLACEHOLDER,
+  SUBJECT_LABEL,
+  SUBJECT_PLACEHOLDER,
+  SUBMIT_TEXT,
+} from 'components/Feedback/constants';
+
+import globalState from 'fixtures/globalState';
+
+describe('BugReportFeedbackForm', () => {
+  const setup = () => {
+    const props: FeedbackFormProps = {
+      sendState: SendingState.IDLE,
+      submitFeedback: jest.fn(),
+      resetFeedback: jest.fn(),
+    };
+    return shallow<BugReportFeedbackForm>(<BugReportFeedbackForm {...props} />)
+  };
+
+  it('is instance of AbstractFeedbackForm', () => {
+    expect(setup().instance()).toBeInstanceOf(AbstractFeedbackForm);
+  });
+
+  describe('renderCustom', () => {
+    let wrapper;
+    let form;
+    beforeAll(() => {
+      wrapper = setup();
+      form = wrapper.find('form');
+    });
+
+    it('renders form with correct props', () => {
+      expect(form.props()).toMatchObject({
+        id: AbstractFeedbackForm.FORM_ID,
+        onSubmit: wrapper.instance().submitForm,
+      });
+    });
+
+    it('renders feedback-type input as first child with correct props', () => {
+      expect(form.children().at(0).find('input').props()).toMatchObject({
+        type: 'hidden',
+        name: 'feedback-type',
+        value: 'Bug Report'
+      });
+    });
+
+    describe('renders subject input as second child', () => {
+      it('renders correct label', () => {
+        expect(form.children().at(1).find('label').text()).toEqual(SUBJECT_LABEL);
+      });
+
+      it('renders input with correct props', () => {
+        expect(form.children().at(1).find('input').props()).toMatchObject({
+          type: 'text',
+          name: 'subject',
+          className: 'form-control',
+          required: true,
+          placeholder: SUBJECT_PLACEHOLDER,
+        });
+      });
+    });
+
+    describe('renders bug-summary input as third child', () => {
+      it('renders correct label', () => {
+        expect(form.children().at(2).find('label').text()).toEqual(BUG_SUMMARY_LABEL);
+      });
+
+      it('renders textarea with correct props', () => {
+        expect(form.children().at(2).find('textarea').props()).toMatchObject({
+          name: 'bug-summary',
+          className: 'form-control',
+          required: true,
+          rows: 3,
+          maxLength: 2000,
+          placeholder: BUG_SUMMARY_PLACEHOLDER,
+        });
+      });
+    });
+
+    describe('renders repro-steps input as fourth child', () => {
+      it('renders correct label', () => {
+        expect(form.children().at(3).find('label').text()).toEqual(REPRO_STEPS_LABEL);
+      });
+
+      it('renders textarea with correct props', () => {
+        expect(form.children().at(3).find('textarea').props()).toMatchObject({
+          name: 'repro-steps',
+          className: 'form-control',
+          required: true,
+          rows: 5,
+          maxLength: 2000,
+          placeholder: REPRO_STEPS_PLACEHOLDER,
+        });
+      });
+    });
+
+    it('renders submit button with correct props', () => {
+      expect(form.find('button').props()).toMatchObject({
+        className: 'btn btn-default submit',
+        type: 'submit',
+      });
+    });
+
+    it('renders submit button with correct text', () => {
+      expect(form.find('button').text()).toEqual(SUBMIT_TEXT)
+    });
+  });
+});
+
+describe('mapDispatchToProps', () => {
+  let dispatch;
+  let result;
+  beforeAll(() => {
+    dispatch = jest.fn(() => Promise.resolve());
+    result = mapDispatchToProps(dispatch);
+  });
+
+  it('sets submitFeedback on the props', () => {
+    expect(result.submitFeedback).toBeInstanceOf(Function);
+  });
+
+  it('sets resetFeedback on the props', () => {
+    expect(result.resetFeedback).toBeInstanceOf(Function);
+  });
+});
+
+describe('mapStateToProps', () => {
+  let result;
+  beforeAll(() => {
+    result = mapStateToProps(globalState);
+  });
+
+  it('sets sendState on the props', () => {
+    expect(result.sendState).toEqual(globalState.feedback.sendState);
+  });
+});

amundsen_application/static/js/components/Feedback/FeedbackForm/RatingFeedbackForm/index.tsx

@@ -7,11 +7,19 @@ import AbstractFeedbackForm, { DispatchFromProps, StateFromProps } from '../../F
 import { GlobalState } from 'ducks/rootReducer';
 import { submitFeedback, resetFeedback } from 'ducks/feedback/reducer';
 
+import {
+  COMMENTS_PLACEHOLDER,
+  RATING_LABEL,
+  RATING_LOW_TEXT,
+  RATING_HIGH_TEXT,
+  SUBMIT_TEXT,
+} from '../../constants';
+
 export class RatingFeedbackForm extends AbstractFeedbackForm {
   constructor(props) {
-    super(props)
+    super(props);
   }
-
+  
   renderCustom() {
     const ratings = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
     const radioButtonSet = ratings.map(rating => (
@@ -21,23 +29,24 @@ export class RatingFeedbackForm extends AbstractFeedbackForm {
       </div>
     ));
 
-    /* TODO: harcoded strings that should be translatable/customizable */
     return (
       <form id={AbstractFeedbackForm.FORM_ID} onSubmit={ this.submitForm }>
         <input type="hidden" name="feedback-type" value="NPS Rating"/>
-        <div>How likely are you to recommend this tool to a friend or co-worker?</div>
-        <div className="radio-set">
-          { radioButtonSet }
-        </div>
-        <div>
-          <div className="nps-label pull-left text-left">Not Very Likely</div>
-          <div className="nps-label pull-right text-right">Very Likely</div>
-        </div>
-        <textarea name="comment" form={AbstractFeedbackForm.FORM_ID}
-          rows={ 5 } maxLength={ 2000 } placeholder="Additional Comments"/>
-        <div>
-          <button className="btn btn-default submit" type="submit">Submit</button>
+        <div className="form-group">
+          <label>{RATING_LABEL}</label>
+          <div>
+            <div className="radio-set">
+              { radioButtonSet }
+            </div>
+            <div>
+              <div className="nps-label pull-left text-left">{RATING_LOW_TEXT}</div>
+              <div className="nps-label pull-right text-right">{RATING_HIGH_TEXT}</div>
+            </div>
+          </div>
         </div>
+        <textarea className="form-control form-group" name="comment" form={AbstractFeedbackForm.FORM_ID}
+          rows={ 8 } maxLength={ 2000 } placeholder={COMMENTS_PLACEHOLDER}/>
+        <button className="btn btn-default submit" type="submit">{SUBMIT_TEXT}</button>
       </form>
     );
   }

amundsen_application/static/js/components/Feedback/FeedbackForm/RatingFeedbackForm/tests/index.spec.tsx

+import * as React from 'react';
+
+import { shallow } from 'enzyme';
+
+import AbstractFeedbackForm, { FeedbackFormProps } from 'components/Feedback/FeedbackForm';
+import { SendingState } from 'components/Feedback/types';
+import { RatingFeedbackForm, mapDispatchToProps, mapStateToProps } from '../';
+import {
+  COMMENTS_PLACEHOLDER,
+  RATING_LABEL,
+  RATING_LOW_TEXT,
+  RATING_HIGH_TEXT,
+  SUBMIT_TEXT,
+} from 'components/Feedback/constants';
+
+import globalState from 'fixtures/globalState';
+
+describe('RatingFeedbackForm', () => {
+  const setup = () => {
+    const props: FeedbackFormProps = {
+      sendState: SendingState.IDLE,
+      submitFeedback: jest.fn(),
+      resetFeedback: jest.fn(),
+    };
+    return shallow(<RatingFeedbackForm {...props}/>);
+  };
+
+  it('is instance of AbstractFeedbackForm', () => {
+    expect(setup().instance()).toBeInstanceOf(AbstractFeedbackForm);
+  });
+
+  describe('renderCustom', () => {
+    let wrapper;
+    let form;
+    beforeAll(() => {
+      wrapper = setup();
+      form = wrapper.find('form');
+    });
+
+    it('renders form with correct props', () => {
+      expect(form.props()).toMatchObject({
+        id: AbstractFeedbackForm.FORM_ID,
+        onSubmit: wrapper.instance().submitForm,
+      });
+    });
+
+    it('renders feedback-type input as first child with correct props', () => {
+      expect(form.children().at(0).find('input').props()).toMatchObject({
+        type: 'hidden',
+        name: 'feedback-type',
+        value: 'NPS Rating'
+      });
+    });
+
+    describe('renders rating form group as second child', () => {
+      let ratingGroup;
+      let ratingComponent;
+      beforeAll(() => {
+        ratingGroup = form.children().at(1);
+        ratingComponent = ratingGroup.children().at(1);
+      })
+      it('renders correct label', () => {
+        expect(ratingGroup.children().at(0).find('label').text()).toEqual(RATING_LABEL);
+      });
+
+      describe('correctly renders radioButtonSet', () => {
+        let radioSet;
+        let ratings;
+        beforeAll(() => {
+          ratings = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
+          radioSet = ratingComponent.children().at(0);
+        });
+        it('renders radio button input for each rating', () => {
+          ratings.forEach((value, index) => {
+            expect(radioSet.find('input').at(index).props()).toMatchObject({
+              type: 'radio',
+              id: `value${value}:input`,
+              name: 'rating',
+              value: `${value}`,
+            });
+          });
+        });
+
+        it('renders label with value for each rating', () => {
+          ratings.forEach((value, index) => {
+            expect(radioSet.find('label').at(index).text()).toEqual(`${value}`);
+          });
+        });
+      });
+
+      it('renders left nps label', () => {
+        expect(ratingComponent.children().at(1).find('.nps-label.pull-left.text-left').text()).toEqual(RATING_LOW_TEXT);
+      });
+
+      it('renders right nps label', () => {
+        expect(ratingComponent.children().at(1).find('.nps-label.pull-right.text-right').text()).toEqual(RATING_HIGH_TEXT);
+      });
+    });
+
+    it('renders textarea with correct props as third child', () => {
+      expect(form.children().at(2).find('textarea').props()).toMatchObject({
+        name: 'comment',
+        className: 'form-control form-group',
+        form: AbstractFeedbackForm.FORM_ID,
+        rows: 8,
+        maxLength: 2000,
+        placeholder: COMMENTS_PLACEHOLDER,
+      });
+    });
+
+    it('renders submit button with correct props as fourth child', () => {
+      expect(form.children().at(3).find('button').props()).toMatchObject({
+        className: 'btn btn-default submit',
+        type: 'submit',
+      });
+    });
+
+    it('renders submit button with correct text', () => {
+      expect(form.children().at(3).find('button').text()).toEqual(SUBMIT_TEXT)
+    });
+  });
+});
+
+describe('mapDispatchToProps', () => {
+  let dispatch;
+  let result;
+  beforeAll(() => {
+    dispatch = jest.fn(() => Promise.resolve());
+    result = mapDispatchToProps(dispatch);
+  });
+
+  it('sets submitFeedback on the props', () => {
+    expect(result.submitFeedback).toBeInstanceOf(Function);
+  });
+
+  it('sets resetFeedback on the props', () => {
+    expect(result.resetFeedback).toBeInstanceOf(Function);
+  });
+});
+
+describe('mapStateToProps', () => {
+  let result;
+  beforeAll(() => {
+    result = mapStateToProps(globalState);
+  });
+
+  it('sets sendState on the props', () => {
+    expect(result.sendState).toEqual(globalState.feedback.sendState);
+  });
+});

amundsen_application/static/js/components/Feedback/FeedbackForm/RequestFeedbackForm/index.tsx

@@ -7,29 +7,40 @@ import AbstractFeedbackForm, { DispatchFromProps, StateFromProps } from '../../F
 import { GlobalState } from 'ducks/rootReducer';
 import { submitFeedback, resetFeedback } from 'ducks/feedback/reducer';
 
+import {
+  FEATURE_SUMMARY_LABEL,
+  FEATURE_SUMMARY_PLACEHOLDER,
+  PROPOSITION_LABEL,
+  PROPOSITION_PLACEHOLDER,
+  SUBJECT_LABEL,
+  SUBJECT_PLACEHOLDER,
+  SUBMIT_TEXT,
+} from '../../constants';
 
 export class RequestFeedbackForm extends AbstractFeedbackForm {
   constructor(props) {
-    super(props)
+    super(props);
   }
-
+  
   renderCustom() {
     return (
       <form id={AbstractFeedbackForm.FORM_ID} onSubmit={ this.submitForm }>
         <input type="hidden" name="feedback-type" value="Feature Request"/>
         <div className="form-group">
-          <label>Feature Summary</label>
+          <label>{SUBJECT_LABEL}</label>
+          <input type="text" name="subject" className="form-control" required={ true } placeholder={SUBJECT_PLACEHOLDER} />
+        </div>
+        <div className="form-group">
+          <label>{FEATURE_SUMMARY_LABEL}</label>
           <textarea name="feature-summary" className="form-control" rows={3} required={ true }
-                    maxLength={ 2000 } placeholder="What feature are you requesting?"/>
+                    maxLength={ 2000 } placeholder={FEATURE_SUMMARY_PLACEHOLDER} />
         </div>
         <div className="form-group">
-          <label>Value Proposition</label>
+          <label>{PROPOSITION_LABEL}</label>
           <textarea name="value-prop" className="form-control" rows={5} required={ true }
-                    maxLength={ 2000 } placeholder="How does this feature add value?"/>
-        </div>
-        <div>
-          <button className="btn btn-default submit" type="submit">Submit</button>
+                    maxLength={ 2000 } placeholder={PROPOSITION_PLACEHOLDER} />
         </div>
+        <button className="btn btn-default submit" type="submit">{SUBMIT_TEXT}</button>
       </form>
     );
   }

amundsen_application/static/js/components/Feedback/FeedbackForm/RequestFeedbackForm/tests/index.spec.tsx

+import * as React from 'react';
+
+import { shallow } from 'enzyme';
+
+import AbstractFeedbackForm, { FeedbackFormProps } from 'components/Feedback/FeedbackForm';
+import { SendingState } from 'components/Feedback/types';
+import { RequestFeedbackForm, mapDispatchToProps, mapStateToProps } from '../';
+import {
+  FEATURE_SUMMARY_LABEL,
+  FEATURE_SUMMARY_PLACEHOLDER,
+  PROPOSITION_LABEL,
+  PROPOSITION_PLACEHOLDER,
+  SUBJECT_LABEL,
+  SUBJECT_PLACEHOLDER,
+  SUBMIT_TEXT,
+} from 'components/Feedback/constants';
+
+import globalState from 'fixtures/globalState';
+
+describe('RequestFeedbackForm', () => {
+  const setup = () => {
+    const props: FeedbackFormProps = {
+      sendState: SendingState.IDLE,
+      submitFeedback: jest.fn(),
+      resetFeedback: jest.fn(),
+    };
+    return shallow(<RequestFeedbackForm {...props}/>)
+  };
+
+  it('is instance of AbstractFeedbackForm', () => {
+    expect(setup().instance()).toBeInstanceOf(AbstractFeedbackForm);
+  });
+
+  describe('renderCustom', () => {
+    let wrapper;
+    let form;
+    beforeAll(() => {
+      wrapper = setup();
+      form = wrapper.find('form');
+    });
+
+    it('renders form with correct props', () => {
+      expect(form.props()).toMatchObject({
+        id: AbstractFeedbackForm.FORM_ID,
+        onSubmit: wrapper.instance().submitForm,
+      });
+    });
+
+    it('renders feedback-type input as first child with correct props', () => {
+      expect(form.children().at(0).find('input').props()).toMatchObject({
+        type: 'hidden',
+        name: 'feedback-type',
+        value: 'Feature Request'
+      });
+    });
+
+    describe('renders subject input as second child', () => {
+      it('renders correct label', () => {
+        expect(form.children().at(1).find('label').text()).toEqual(SUBJECT_LABEL);
+      });
+
+      it('renders input with correct props', () => {
+        expect(form.children().at(1).find('input').props()).toMatchObject({
+          type: 'text',
+          name: 'subject',
+          className: 'form-control',
+          required: true,
+          placeholder: SUBJECT_PLACEHOLDER,
+        });
+      });
+    });
+
+    describe('renders feature-summary input as third child', () => {
+      it('renders correct label', () => {
+        expect(form.children().at(2).find('label').text()).toEqual(FEATURE_SUMMARY_LABEL);
+      });
+
+      it('renders textarea with correct props', () => {
+        expect(form.children().at(2).find('textarea').props()).toMatchObject({
+          name: 'feature-summary',
+          className: 'form-control',
+          required: true,
+          rows: 3,
+          maxLength: 2000,
+          placeholder: FEATURE_SUMMARY_PLACEHOLDER,
+        });
+      });
+    });
+
+    describe('renders value-prop input as fourth child', () => {
+      it('renders correct label', () => {
+        expect(form.children().at(3).find('label').text()).toEqual(PROPOSITION_LABEL);
+      });
+
+      it('renders textarea with correct props', () => {
+        expect(form.children().at(3).find('textarea').props()).toMatchObject({
+          name: 'value-prop',
+          className: 'form-control',
+          required: true,
+          rows: 5,
+          maxLength: 2000,
+          placeholder: PROPOSITION_PLACEHOLDER,
+        });
+      });
+    });
+
+    it('renders submit button with correct props', () => {
+      expect(form.find('button').props()).toMatchObject({
+        className: 'btn btn-default submit',
+        type: 'submit',
+      });
+    });
+
+    it('renders submit button with correct text', () => {
+      expect(form.find('button').text()).toEqual(SUBMIT_TEXT)
+    });
+  });
+});
+
+describe('mapDispatchToProps', () => {
+  let dispatch;
+  let result;
+  beforeAll(() => {
+    dispatch = jest.fn(() => Promise.resolve());
+    result = mapDispatchToProps(dispatch);
+  });
+
+  it('sets submitFeedback on the props', () => {
+    expect(result.submitFeedback).toBeInstanceOf(Function);
+  });
+
+  it('sets resetFeedback on the props', () => {
+    expect(result.resetFeedback).toBeInstanceOf(Function);
+  });
+});
+
+describe('mapStateToProps', () => {
+  let result;
+  beforeAll(() => {
+    result = mapStateToProps(globalState);
+  });
+
+  it('sets sendState on the props', () => {
+    expect(result.sendState).toEqual(globalState.feedback.sendState);
+  });
+});

amundsen_application/static/js/components/Feedback/FeedbackForm/index.tsx

@@ -7,9 +7,10 @@ import { ResetFeedbackRequest, SubmitFeedbackRequest } from 'ducks/feedback/type
 
 import { SendingState } from '../types';
 
-interface FeedbackFormState {
-  sendState: SendingState;
-}
+import {
+  SUBMIT_FAILURE_MESSAGE,
+  SUBMIT_SUCCESS_MESSAGE,
+} from '../constants';
 
 export interface StateFromProps {
   sendState: SendingState;
@@ -20,28 +21,15 @@ export interface DispatchFromProps {
   resetFeedback: () => ResetFeedbackRequest;
 }
 
-type FeedbackFormProps = StateFromProps & DispatchFromProps;
+export type FeedbackFormProps = StateFromProps & DispatchFromProps;
 
-abstract class AbstractFeedbackForm extends React.Component<FeedbackFormProps, FeedbackFormState> {
-  public static defaultProps: FeedbackFormProps = {
-    sendState: SendingState.IDLE,
-    submitFeedback: () => undefined,
-    resetFeedback: () => undefined,
-  };
+abstract class AbstractFeedbackForm extends React.Component<FeedbackFormProps> {
+  public static defaultProps: Partial<FeedbackFormProps> = {};
 
   static FORM_ID = "feedback-form";
 
   protected constructor(props) {
     super(props);
-
-    this.state = {
-      sendState: this.props.sendState
-    };
-  }
-
-  static getDerivedStateFromProps(nextProps, prevState) {
-    const { sendState } = nextProps;
-    return { sendState };
   }
 
   submitForm = (event) => {
@@ -52,13 +40,20 @@ abstract class AbstractFeedbackForm extends React.Component<FeedbackFormProps, F
   };
 
   render() {
-    if (this.state.sendState === SendingState.WAITING) {
+    if (this.props.sendState === SendingState.WAITING) {
       return <LoadingSpinner/>;
     }
-    if (this.state.sendState === SendingState.COMPLETE) {
+    if (this.props.sendState === SendingState.COMPLETE) {
+      return (
+        <div className="status-message">
+          {SUBMIT_SUCCESS_MESSAGE}
+        </div>
+      );
+    }
+    if (this.props.sendState === SendingState.ERROR) {
       return (
-        <div className="success-message">
-          Your feedback has been successfully submitted
+        <div className="status-message">
+          {SUBMIT_FAILURE_MESSAGE}
         </div>
       );
     }

amundsen_application/static/js/components/Feedback/FeedbackForm/styles.scss

@@ -30,11 +30,11 @@
 .nps-label {
   font-family: $font-family-sans-serif-bold;
   font-weight: $font-weight-sans-serif-bold;
-  margin-bottom: 10px;
+  margin-bottom: 15px;
   width: 60px;
 }
 
-.success-message {
+.status-message {
   font-family: $font-family-sans-serif-bold;
   font-weight: $font-weight-sans-serif-bold;
   text-align: center;
@@ -52,8 +52,13 @@ input[type="radio"] {
   margin: 5px;
 }
 
+input[type="text"] {
+  color: $text-medium !important;
+}
+
 textarea {
   width: 100%;
+  color: $text-medium !important;
   border: 1px solid $gray-lighter;
   border-radius: 5px;
   padding: 10px;

amundsen_application/static/js/components/Feedback/FeedbackForm/tests/index.spec.tsx

+import * as React from 'react';
+
+import { shallow } from 'enzyme';
+
+import LoadingSpinner from 'components/common/LoadingSpinner';
+import { SendingState } from '../../types';
+import FeedbackForm, { FeedbackFormProps } from '../';
+import { RatingFeedbackForm } from '../RatingFeedbackForm';
+
+import {
+  SUBMIT_FAILURE_MESSAGE,
+  SUBMIT_SUCCESS_MESSAGE,
+} from '../../constants';
+
+const mockFormData = { key1: 'val1', key2: 'val2' };
+// @ts-ignore: How to mock FormData without TypeScript error?
+global.FormData = () => (mockFormData);
+
+describe('FeedbackForm', () => {
+  const setup = (propOverrides?: Partial<FeedbackFormProps>) => {
+    const props: FeedbackFormProps = {
+      sendState: SendingState.IDLE,
+      submitFeedback: jest.fn(),
+      resetFeedback: jest.fn(),
+      ...propOverrides
+    };
+    const wrapper = shallow<RatingFeedbackForm>(<RatingFeedbackForm {...props} />);
+    return { props, wrapper };
+  };
+
+  describe('submitForm', () => {
+    it('calls submitFeedback with formData', () => {
+      const { props, wrapper } = setup();
+      // @ts-ignore: mocked events throw type errors
+      wrapper.instance().submitForm({ preventDefault: jest.fn() });
+      expect(props.submitFeedback).toHaveBeenCalledWith(mockFormData);
+    });
+  });
+
+  describe('render', () => {
+    it('calls renderCustom if sendState is not WAITING or COMPLETE', () => {
+      const { props, wrapper } = setup();
+      const renderCustomSpy = jest.spyOn(wrapper.instance(), 'renderCustom');
+      wrapper.instance().render();
+      expect(renderCustomSpy).toHaveBeenCalled();
+    });
+
+    it('renders LoadingSpinner if sendState is WAITING', () => {
+      const { props, wrapper } = setup({sendState: SendingState.WAITING});
+      expect(wrapper.find('LoadingSpinner').exists()).toBeTruthy();
+    });
+
+    it('renders confirmation status message if sendState is COMPLETE', () => {
+      const { props, wrapper } = setup({sendState: SendingState.COMPLETE});
+      expect(wrapper.find('div.status-message').text()).toEqual(SUBMIT_SUCCESS_MESSAGE);
+    });
+
+    it('renders failure status message if sendState is ERROR', () => {
+      const { props, wrapper } = setup({sendState: SendingState.ERROR});
+      expect(wrapper.find('div.status-message').text()).toEqual(SUBMIT_FAILURE_MESSAGE);
+    });
+  });
+});

amundsen_application/static/js/components/Feedback/constants.ts

+/* TODO: harcoded string that should be translatable/customizable */
+/* Form */
+export const BUTTON_CLOSE_TEXT = 'Close';
+export const FEEDBACK_TITLE = 'Product Feedback';
+export const SUBMIT_FAILURE_MESSAGE = 'Your feedback was not submitted, please try again';
+export const SUBMIT_SUCCESS_MESSAGE = 'Your feedback has been successfully submitted';
+
+/* Button Set */
+export const BUG_REPORT_TEXT = 'Bug Report';
+export const FEEDBACK_TYPE_TEXT = 'Feedback Type Selector';
+export const RATING_TEXT = 'Rating';
+export const REQUEST_TEXT = 'Request';
+
+/* Nested Forms */
+export const BUG_SUMMARY_LABEL = 'Bug Summary';
+export const BUG_SUMMARY_PLACEHOLDER = 'What went wrong?';
+
+export const COMMENTS_PLACEHOLDER = 'Additional Comments';
+
+export const FEATURE_SUMMARY_LABEL = 'Feature Summary';
+export const FEATURE_SUMMARY_PLACEHOLDER = 'What feature are you requesting?';
+
+export const PROPOSITION_LABEL = 'Value Proposition';
+export const PROPOSITION_PLACEHOLDER = 'How does this feature add value?';
+
+export const RATING_LABEL = 'How likely are you to recommend this tool to a friend or co-worker?';
+
+export const RATING_LOW_TEXT = 'Not Very Likely';
+export const RATING_HIGH_TEXT = 'Very Likely'
+
+export const REPRO_STEPS_LABEL = 'Reproduction Steps';
+export const REPRO_STEPS_PLACEHOLDER = 'What did you do to encounter this bug?';
+
+export const SUBJECT_LABEL = 'Subject';
+export const SUBJECT_PLACEHOLDER = 'Enter a subject';
+
+export const SUBMIT_TEXT = 'Submit';

amundsen_application/static/js/components/Feedback/index.tsx

@@ -6,47 +6,53 @@ import RequestFeedbackForm from './FeedbackForm/RequestFeedbackForm';
 
 import { Button, Panel } from 'react-bootstrap';
 
+import {
+  BUG_REPORT_TEXT,
+  BUTTON_CLOSE_TEXT,
+  FEEDBACK_TITLE,
+  FEEDBACK_TYPE_TEXT,
+  RATING_TEXT,
+  REQUEST_TEXT,
+} from './constants';
+
 // TODO: Use css-modules instead of 'import'
 import './styles.scss';
 
-interface FeedbackProps {
+export interface FeedbackProps {
   content?: React.SFC<any>,
   title?: string,
 }
 
 interface FeedbackState {
-  open: boolean,
+  content: React.SFC<any>,
   feedbackType: FeedbackType,
-  content: React.SFC<any>
+  isOpen: boolean,
 }
 
-enum FeedbackType {
+export enum FeedbackType {
   Rating,
   Bug,
   Request,
 }
 
 export default class Feedback extends React.Component<FeedbackProps, FeedbackState> {
-  /* TODO: harcoded string that should be translatable/customizable */
   static defaultProps = {
     content: <RatingFeedbackForm />,
-    title: 'Product Feedback'
+    title: FEEDBACK_TITLE,
   };
 
   constructor(props) {
     super(props);
 
-    this.toggle = this.toggle.bind(this);
-
     this.state = {
-      open: false,
+      isOpen: false,
       content: this.props.content,
       feedbackType: FeedbackType.Rating,
     };
   }
 
-  toggle() {
-   this.setState({ open: !this.state.open });
+  toggle = () => {
+    this.setState({ isOpen: !this.state.isOpen });
   }
 
   changeType = (type: FeedbackType) => (e) =>  {
@@ -65,42 +71,42 @@ export default class Feedback extends React.Component<FeedbackProps, FeedbackSta
   };
 
   render() {
-    const expandedClass = this.state.open ? 'expanded' : 'collapsed';
+    const expandedClass = this.state.isOpen ? 'expanded' : 'collapsed';
     return (
       <div className={`feedback-component ${expandedClass}`}>
         {
-          this.state.open &&
+          this.state.isOpen &&
           <div>
             <div className="feedback-header">
-              <button type="button" className="close" aria-label="Close" onClick={this.toggle}>
-                <span aria-hidden="true">&times;</span>
-                <span className="sr-only">Close</span>
-              </button>
               <div className="title">
                 {this.props.title.toUpperCase()}
               </div>
+              <button type="button" className="btn btn-close" aria-label={BUTTON_CLOSE_TEXT} onClick={this.toggle} />
             </div>
             <div className="text-center">
-              <div className="btn-group" role="group" aria-label="Feedback Type Selector">
+              <div className="btn-group" role="group" aria-label={FEEDBACK_TYPE_TEXT}>
                 <button type="button"
                         className={"btn btn-default" + (this.state.feedbackType === FeedbackType.Rating? " active": "")}
                         onClick={this.changeType(FeedbackType.Rating)}>
-                  Rating</button>
+                  {RATING_TEXT}
+                </button>
                 <button type="button"
                         className={"btn btn-default" + (this.state.feedbackType === FeedbackType.Bug? " active": "")}
                         onClick={this.changeType(FeedbackType.Bug)}>
-                  Bug Report</button>
+                  {BUG_REPORT_TEXT}
+                </button>
                 <button type="button"
                         className={"btn btn-default" + (this.state.feedbackType === FeedbackType.Request? " active": "")}
                         onClick={this.changeType(FeedbackType.Request)}>
-                  Request</button>
+                  {REQUEST_TEXT}
+                </button>
               </div>
             </div>
             {this.state.content}
           </div>
         }
         {
-          !(this.state.open) &&
+          !(this.state.isOpen) &&
           <img className='icon-speech' src='/static/images/icons/Speech.svg' onClick={this.toggle}/>
         }
       </div>

amundsen_application/static/js/components/Feedback/styles.scss

@@ -36,6 +36,7 @@
 
   .title {
     color: $text-medium;
+    flex-grow: 1;
     font-size: 12px;
     font-family: $font-family-sans-serif-bold;
     font-weight: $font-weight-sans-serif-bold;
@@ -50,6 +51,7 @@
   }
 
   .feedback-header {
+    display: flex;
     margin-bottom: 8px;
   }
 

amundsen_application/static/js/components/Feedback/tests/index.spec.tsx

+import * as React from 'react';
+
+import { shallow } from 'enzyme';
+
+import BugReportFeedbackForm from '../FeedbackForm/BugReportFeedbackForm';
+import RatingFeedbackForm from '../FeedbackForm/RatingFeedbackForm';
+import RequestFeedbackForm from '../FeedbackForm/RequestFeedbackForm';
+import Feedback, { FeedbackProps, FeedbackType } from '../';
+import {
+  BUG_REPORT_TEXT,
+  BUTTON_CLOSE_TEXT,
+  FEEDBACK_TYPE_TEXT,
+  RATING_TEXT,
+  REQUEST_TEXT,
+} from '../constants';
+
+describe('Feedback', () => {
+  const setStateSpy = jest.spyOn(Feedback.prototype, 'setState');
+
+  const setup = (propOverrides?: Partial<FeedbackProps>) => {
+    const props: FeedbackProps = {
+      ...propOverrides
+    };
+    const wrapper = shallow<Feedback>(<Feedback {...props} />)
+    return { props, wrapper };
+  };
+
+  describe('constructor', () => {
+    let props;
+    let wrapper;
+    beforeAll(() => {
+      const setupResult = setup();
+      props = setupResult.props;
+      wrapper = setupResult.wrapper;
+    });
+    it('sets state.isOpen to false', () => {
+      expect(wrapper.state().isOpen).toEqual(false);
+    });
+
+    it('sets state.content from defaultProps', () => {
+      expect(wrapper.state().content).toEqual(Feedback.defaultProps.content);
+    });
+
+    it('sets state.feedbackType to FeedbackType.Rating', () => {
+      expect(wrapper.state().feedbackType).toEqual(FeedbackType.Rating);
+    });
+  });
+
+  describe('toggle', () => {
+    it('calls setState with negation of state.isOpen', () => {
+      setStateSpy.mockClear();
+      const { props, wrapper } = setup();
+      const previsOpenState = wrapper.state().isOpen;
+      wrapper.instance().toggle();
+      expect(setStateSpy).toHaveBeenCalledWith({ isOpen: !previsOpenState });
+    });
+  });
+
+  describe('changeType', () => {
+    let props;
+    let wrapper;
+    beforeAll(() => {
+      const setupResult = setup();
+      props = setupResult.props;
+      wrapper = setupResult.wrapper;
+      setStateSpy.mockClear();
+    });
+    it('returns method that calls setState with correct values if type === FeedbackType.Bug', () => {
+      wrapper.instance().changeType(FeedbackType.Bug)();
+      expect(setStateSpy).toHaveBeenCalledWith({ content: <BugReportFeedbackForm />, feedbackType: FeedbackType.Bug});
+    });
+
+    it('returns method that calls setState with correct values if type === FeedbackType.Rating', () => {
+      wrapper.instance().changeType(FeedbackType.Rating)();
+      expect(setStateSpy).toHaveBeenCalledWith({ content: <RatingFeedbackForm />, feedbackType: FeedbackType.Rating});
+    });
+
+    it('returns method that calls setState with correct values if type === FeedbackType.Request', () => {
+      wrapper.instance().changeType(FeedbackType.Request)();
+      expect(setStateSpy).toHaveBeenCalledWith({ content: <RequestFeedbackForm />, feedbackType: FeedbackType.Request});
+    });
+  });
+
+  describe('render', () => {
+    describe('if state.isOpen', () => {
+      let element;
+      let props;
+      let wrapper;
+
+      let changeTypeSpy;
+      let changeTypeMockResult;
+      beforeAll(() => {
+        const setupResult = setup({ title: 'I am a title' });
+        props = setupResult.props;
+        wrapper = setupResult.wrapper;
+        wrapper.instance().toggle();
+
+        changeTypeMockResult = jest.fn(() => {});
+        changeTypeSpy = jest.spyOn(wrapper.instance(), 'changeType').mockImplementation(() => changeTypeMockResult);
+        wrapper.update();
+        element = wrapper.children().at(0);
+      });
+      it('renders wrapper with correct className', () => {
+        expect(wrapper.props().className).toEqual('feedback-component expanded');
+      });
+
+      describe('correct feedback-header', () => {
+        let button;
+        let title;
+        beforeAll(() => {
+          const header = element.children().at(0);
+          title = header.children().at(0);
+          button = header.children().at(1);
+        });
+        it('renders correct title', () => {
+          expect(title.text()).toEqual(props.title.toUpperCase());
+        });
+
+        it('renders close button with correct props', () => {
+          expect(button.props()).toMatchObject({
+            type: 'button',
+            className: 'btn btn-close',
+            'aria-label': BUTTON_CLOSE_TEXT,
+            onClick: wrapper.instance().toggle,
+          });
+        });
+      });
+
+      describe('correct feedback button group', () => {
+        let buttonGroupParent;
+        let buttonGroup;
+        beforeAll(() => {
+          buttonGroupParent = element.children().at(1);
+          buttonGroup = buttonGroupParent.children().at(0);
+        });
+        it('renders button group parent with correct className', () => {
+          expect(buttonGroupParent.props().className).toEqual('text-center');
+        });
+
+        it('renders button group with correct props', () => {
+          expect(buttonGroup.props()).toMatchObject({
+            className: 'btn-group',
+            role: 'group',
+            'aria-label': FEEDBACK_TYPE_TEXT,
+          });
+        });
+
+        describe('renders correct rating button', () => {
+          let button;
+          beforeAll(() => {
+            wrapper.setState({ feedbackType: FeedbackType.Rating });
+            button = wrapper.children().at(0).children().at(1).children().at(0).find('button').at(0);
+          });
+          it('has correct props if active', () => {
+            expect(button.props()).toMatchObject({
+              type: 'button',
+              className: 'btn btn-default active',
+              onClick: changeTypeMockResult,
+            });
+          });
+
+          it('has correct text', () => {
+            expect(button.text()).toEqual(RATING_TEXT);
+          });
+
+          it('has correct props if not active', () => {
+            wrapper.setState({ feedbackType: FeedbackType.Bug });
+            button = wrapper.children().at(0).children().at(1).children().at(0).find('button').at(0);
+            expect(button.props()).toMatchObject({
+              type: 'button',
+              className: 'btn btn-default',
+              onClick: changeTypeMockResult,
+            });
+          });
+        });
+
+        describe('renders correct bug report button', () => {
+          let button;
+          beforeAll(() => {
+            wrapper.setState({ feedbackType: FeedbackType.Bug });
+            button = wrapper.children().at(0).children().at(1).children().at(0).find('button').at(1);
+          });
+          it('has correct props if active', () => {
+            expect(button.props()).toMatchObject({
+              type: 'button',
+              className: 'btn btn-default active',
+              onClick: changeTypeMockResult,
+            });
+          });
+
+          it('has correct text', () => {
+            expect(button.text()).toEqual(BUG_REPORT_TEXT);
+          });
+
+          it('has correct props if not active', () => {
+            wrapper.setState({ feedbackType: FeedbackType.Request });
+            button = wrapper.children().at(0).children().at(1).children().at(0).find('button').at(1);
+            expect(button.props()).toMatchObject({
+              type: 'button',
+              className: 'btn btn-default',
+              onClick: changeTypeMockResult,
+            });
+          });
+        });
+
+        describe('renders correct request button', () => {
+          let button;
+          beforeAll(() => {
+            wrapper.setState({ feedbackType: FeedbackType.Request });
+            button = wrapper.children().at(0).children().at(1).children().at(0).find('button').at(2);
+          });
+          it('has correct props if active', () => {
+            expect(button.props()).toMatchObject({
+              type: 'button',
+              className: 'btn btn-default active',
+              onClick: changeTypeMockResult,
+            });
+          });
+
+          it('has correct text', () => {
+            expect(button.text()).toEqual(REQUEST_TEXT);
+          });
+
+          it('has correct props if not active', () => {
+            wrapper.setState({ feedbackType: FeedbackType.Rating });
+            button = wrapper.children().at(0).children().at(1).children().at(0).find('button').at(2);
+            expect(button.props()).toMatchObject({
+              type: 'button',
+              className: 'btn btn-default',
+              onClick: changeTypeMockResult,
+            });
+          });
+        });
+      });
+
+      /* Note: Using .debug() as a temporary workaround -- do not propagate this pattern.
+         Issue: The content are connected components. Enzyme will throw an error if we access that connected component without a Redux store connected.
+         Workaround: Use .debug() to check that the string representation of the rendered content is what we expect.
+         Future Solution: Connect a mock store OR refactor FeedbackForm to be the connected component on the first place, submitting the form data
+                          of its child form and refreshing it.
+      */
+      it('renders state.content', () => {
+        expect(wrapper.children().at(0).children().at(2).debug()).toEqual('<Connect(RatingFeedbackForm) />');
+      });
+
+      afterAll(() => {
+        changeTypeSpy.mockRestore();
+      });
+    });
+
+    describe('if !state.isOpen', () => {
+      let props;
+      let wrapper;
+      beforeAll(() => {
+        const setupResult = setup();
+        props = setupResult.props;
+        wrapper = setupResult.wrapper;
+      });
+      it('renders wrapper with correct className', () => {
+        expect(wrapper.props().className).toEqual('feedback-component collapsed');
+      });
+
+      it('renders img correct props', () => {
+        expect(wrapper.find('img').props()).toMatchObject({
+          className: 'icon-speech',
+          src: '/static/images/icons/Speech.svg',
+          onClick: wrapper.instance().toggle,
+        });
+      });
+    });
+  });
+});

amundsen_application/static/js/components/Feedback/types.ts

 export enum SendingState {
+  ERROR = "error",
   IDLE = "idle",
   WAITING = "waiting",
   COMPLETE = "complete"

amundsen_application/static/js/ducks/feedback/api/v0.ts

@@ -9,11 +9,5 @@ export function feedbackSubmitFeedback(action: SubmitFeedbackRequest) {
     url: '/api/mail/v0/feedback',
     timeout: 5000,
     headers: {'Content-Type': 'multipart/form-data' }
-  })
-  .then((response: AxiosResponse<any>) => {
-    return response;
-  })
-  .catch((error: AxiosError) => {
-    return error;
   });
 }

amundsen_application/static/js/ducks/feedback/reducer.ts

@@ -38,8 +38,7 @@ export default function reducer(state: FeedbackReducerState = initialState, acti
     case SubmitFeedback.SUCCESS:
       return { sendState: SendingState.COMPLETE };
     case SubmitFeedback.FAILURE:
-      alert('Your feedback was not submitted, please try again');
-      return { sendState: SendingState.IDLE };
+      return { sendState: SendingState.ERROR };
     case ResetFeedback.ACTION:
       return { sendState: SendingState.IDLE };
     default:

amundsen_application/static/js/ducks/feedback/sagas.ts

@@ -15,6 +15,10 @@ function* submitFeedbackWorker(action: SubmitFeedbackRequest): SagaIterator {
     yield put({ type: ResetFeedback.ACTION });
   } catch(error) {
     yield put({ type: SubmitFeedback.FAILURE });
+
+    // TODO - yield delay(2000) on redux-saga upgrade
+    yield call(delay, 2000);
+    yield put({ type: ResetFeedback.ACTION });
   }
 }
 

tests/unit/api/mail/test_v0.py

 import unittest
 
 from http import HTTPStatus
-from typing import List
+from typing import Dict, List
 
 from flask import Response, jsonify, make_response
 
@@ -20,7 +20,8 @@ class MockMailClient(BaseMailClient):
                    recipients: List = [],
                    subject: str = None,
                    text: str = None,
-                   html: str = None) -> Response:
+                   html: str = None,
+                   optional_data: Dict = {}) -> Response:
         return make_response(jsonify({}), self.status_code)
 
 
@@ -33,7 +34,8 @@ class MockBadClient(BaseMailClient):
                    recipients: List = [],
                    subject: str = None,
                    text: str = None,
-                   html: str = None) -> Response:
+                   html: str = None,
+                   optional_data: Dict = {}) -> Response:
         raise Exception('Bad client')