From 1d2c21249b5df1b30431a074ca90a46124e37e6b Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 27 Jun 2019 10:15:11 -0400 Subject: [PATCH 1/2] Show notification type in its own column --- .../src/api/mixins/Notifications.mixin.js | 4 + .../NotificationListItem.jsx | 88 ++++--- .../NotificationListItem.test.jsx | 61 +++-- .../NotificationListItem.test.jsx.snap | 105 ++++---- .../OrganizationNotifications.jsx | 34 ++- .../OrganizationNotifications.test.jsx | 46 ++-- .../OrganizationNotifications.test.jsx.snap | 236 ++++++++++-------- 7 files changed, 331 insertions(+), 243 deletions(-) diff --git a/awx/ui_next/src/api/mixins/Notifications.mixin.js b/awx/ui_next/src/api/mixins/Notifications.mixin.js index 9072e30874..eccadc5dc2 100644 --- a/awx/ui_next/src/api/mixins/Notifications.mixin.js +++ b/awx/ui_next/src/api/mixins/Notifications.mixin.js @@ -1,5 +1,9 @@ const NotificationsMixin = parent => class extends parent { + readOptionsNotificationTemplates(id) { + return this.http.options(`${this.baseUrl}${id}/notification_templates/`); + } + readNotificationTemplates(id, params = {}) { return this.http.get(`${this.baseUrl}${id}/notification_templates/`, { params, diff --git a/awx/ui_next/src/components/NotificationsList/NotificationListItem.jsx b/awx/ui_next/src/components/NotificationsList/NotificationListItem.jsx index f8e243ad39..38294bcb12 100644 --- a/awx/ui_next/src/components/NotificationsList/NotificationListItem.jsx +++ b/awx/ui_next/src/components/NotificationsList/NotificationListItem.jsx @@ -4,7 +4,6 @@ import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Link } from 'react-router-dom'; import { - Badge, Switch as PFSwitch, DataListItem, DataListItemRow, @@ -39,6 +38,7 @@ function NotificationListItem(props) { errorTurnedOn, toggleNotification, i18n, + typeLabels, } = props; return ( @@ -47,50 +47,47 @@ function NotificationListItem(props) { key={notification.id} > - - - - {notification.name} - - - - {notification.notification_type} - - , - - - toggleNotification( - notification.id, - successTurnedOn, - 'success' - ) - } - aria-label={i18n._(t`Toggle notification success`)} - /> - - toggleNotification(notification.id, errorTurnedOn, 'error') - } - aria-label={i18n._(t`Toggle notification failure`)} - /> - , - ]} + + + {notification.name} + + , + + {typeLabels[notification.notification_type]} + , + + toggleNotification( + notification.id, + successTurnedOn, + 'success' + )} + aria-label={i18n._(t`Toggle notification success`)} + /> + toggleNotification( + notification.id, + errorTurnedOn, + 'error' + )} + aria-label={i18n._(t`Toggle notification failure`)} + /> + + ]} /> @@ -108,6 +105,7 @@ NotificationListItem.propTypes = { errorTurnedOn: bool, successTurnedOn: bool, toggleNotification: func.isRequired, + typeLabels: shape().isRequired }; NotificationListItem.defaultProps = { diff --git a/awx/ui_next/src/components/NotificationsList/NotificationListItem.test.jsx b/awx/ui_next/src/components/NotificationsList/NotificationListItem.test.jsx index 8c2d5afd47..66c0ad5a01 100644 --- a/awx/ui_next/src/components/NotificationsList/NotificationListItem.test.jsx +++ b/awx/ui_next/src/components/NotificationsList/NotificationListItem.test.jsx @@ -6,6 +6,16 @@ describe('', () => { let wrapper; let toggleNotification; + const mockNotif = { + id: 9000, + name: 'Foo', + notification_type: 'slack' + }; + + const typeLabels = { + slack: 'Slack' + }; + beforeEach(() => { toggleNotification = jest.fn(); }); @@ -18,34 +28,42 @@ describe('', () => { jest.clearAllMocks(); }); - test('initially renders succesfully', () => { + test('initially renders succesfully and displays correct label', () => { wrapper = mountWithContexts( ); expect(wrapper.find('NotificationListItem')).toMatchSnapshot(); }); + test('displays correct label in correct column', () => { + wrapper = mountWithContexts( + + ); + const typeCell = wrapper.find('DataListCell').at(1).find('div'); + expect(typeCell.text()).toBe('Slack'); + }); + test('handles success click when toggle is on', () => { wrapper = mountWithContexts( ); wrapper @@ -59,15 +77,12 @@ describe('', () => { test('handles success click when toggle is off', () => { wrapper = mountWithContexts( ); wrapper @@ -81,15 +96,12 @@ describe('', () => { test('handles error click when toggle is on', () => { wrapper = mountWithContexts( ); wrapper @@ -103,15 +115,12 @@ describe('', () => { test('handles error click when toggle is off', () => { wrapper = mountWithContexts( ); wrapper diff --git a/awx/ui_next/src/components/NotificationsList/__snapshots__/NotificationListItem.test.jsx.snap b/awx/ui_next/src/components/NotificationsList/__snapshots__/NotificationListItem.test.jsx.snap index 8f17dfe0c1..8a147d78c8 100644 --- a/awx/ui_next/src/components/NotificationsList/__snapshots__/NotificationListItem.test.jsx.snap +++ b/awx/ui_next/src/components/NotificationsList/__snapshots__/NotificationListItem.test.jsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[` initially renders succesfully 1`] = ` +exports[` initially renders succesfully and displays correct label 1`] = ` initially renders succe } successTurnedOn={false} toggleNotification={[MockFunction]} + typeLabels={ + Object { + "slack": "Slack", + } + } > initially renders succe Foo - - slack - + , + + Slack , initially renders succe - - - - - slack - - - - + + + + + + + +
+ Slack
diff --git a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx index 0e9b24e353..206cbafd81 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx @@ -35,6 +35,7 @@ class OrganizationNotifications extends Component { notifications: [], successTemplateIds: [], errorTemplateIds: [], + typeLabels: null }; this.handleNotificationToggle = this.handleNotificationToggle.bind(this); this.handleNotificationErrorClose = this.handleNotificationErrorClose.bind( @@ -56,13 +57,25 @@ class OrganizationNotifications extends Component { async loadNotifications() { const { id, location } = this.props; + const { typeLabels } = this.state; const params = parseNamespacedQueryString(QS_CONFIG, location.search); + const promises = [ + OrganizationsAPI.readNotificationTemplates(id, params) + ]; + + if (!typeLabels) { + promises.push(OrganizationsAPI.readOptionsNotificationTemplates(id)); + } + this.setState({ contentError: null, hasContentLoading: true }); try { - const { - data: { count: itemCount = 0, results: notifications = [] }, - } = await OrganizationsAPI.readNotificationTemplates(id, params); + const [{ + data: { + count: itemCount = 0, + results: notifications = [], + } + }, optionsResponse] = await Promise.all(promises); let idMatchParams; if (notifications.length > 0) { @@ -79,12 +92,21 @@ class OrganizationNotifications extends Component { OrganizationsAPI.readNotificationTemplatesError(id, idMatchParams), ]); - this.setState({ + const stateToUpdate = { itemCount, notifications, successTemplateIds: successTemplates.results.map(s => s.id), errorTemplateIds: errorTemplates.results.map(e => e.id), - }); + }; + + if (!typeLabels) { + const { data: { actions: { GET: { notification_type: {choices} } } } } = optionsResponse; + // The structure of choices looks like [['slack', 'Slack'], ['email', 'Email'], ...] + stateToUpdate.typeLabels = + choices.reduce((map, notifType) => ({ ...map, [notifType[0]]: notifType[1]}), {}); + } + + this.setState(stateToUpdate); } catch (err) { this.setState({ contentError: err }); } finally { @@ -148,6 +170,7 @@ class OrganizationNotifications extends Component { notifications, successTemplateIds, errorTemplateIds, + typeLabels, } = this.state; return ( @@ -169,6 +192,7 @@ class OrganizationNotifications extends Component { toggleNotification={this.handleNotificationToggle} errorTurnedOn={errorTemplateIds.includes(notification.id)} successTurnedOn={successTemplateIds.includes(notification.id)} + typeLabels={typeLabels} /> )} /> diff --git a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.test.jsx b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.test.jsx index a023a157a7..06e2e4f41d 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.test.jsx @@ -8,26 +8,36 @@ import OrganizationNotifications from './OrganizationNotifications'; jest.mock('@api'); describe('', () => { - let data; + const data = { + count: 2, + results: [{ + id: 1, + name: 'Notification one', + url: '/api/v2/notification_templates/1/', + notification_type: 'email', + }, { + id: 2, + name: 'Notification two', + url: '/api/v2/notification_templates/2/', + notification_type: 'email', + }] + }; + + OrganizationsAPI.readOptionsNotificationTemplates.mockReturnValue({ + data: { + actions: { + GET: { + notification_type: { + choices: [ + ['email', 'Email'] + ] + } + } + } + } + }); beforeEach(() => { - data = { - count: 2, - results: [ - { - id: 1, - name: 'Notification one', - url: '/api/v2/notification_templates/1/', - notification_type: 'email', - }, - { - id: 2, - name: 'Notification two', - url: '/api/v2/notification_templates/2/', - notification_type: 'email', - }, - ], - }; OrganizationsAPI.readNotificationTemplates.mockReturnValue({ data }); OrganizationsAPI.readNotificationTemplatesSuccess.mockReturnValue({ data: { results: [{ id: 1 }] }, diff --git a/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap b/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap index 1e0c33d06d..03ce55e487 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap +++ b/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap @@ -410,9 +410,9 @@ exports[` initially renders succesfully 1`] = ` "$$typeof": Symbol(react.forward_ref), "attrs": Array [], "componentStyle": ComponentStyle { - "componentId": "sc-bxivhb", + "componentId": "sc-htpNat", "isStatic": true, - "lastClassName": "gYEJOJ", + "lastClassName": "dqEVhr", "rules": Array [ "flex-grow: 1;", ], @@ -420,7 +420,7 @@ exports[` initially renders succesfully 1`] = ` "displayName": "Styled(ToolbarItem)", "foldedComponentIds": Array [], "render": [Function], - "styledComponentId": "sc-bxivhb", + "styledComponentId": "sc-htpNat", "target": [Function], "toString": [Function], "warnTooManyClasses": [Function], @@ -430,10 +430,10 @@ exports[` initially renders succesfully 1`] = ` forwardedRef={null} >
initially renders succesfully 1`] = ` "$$typeof": Symbol(react.forward_ref), "attrs": Array [], "componentStyle": ComponentStyle { - "componentId": "sc-htpNat", + "componentId": "sc-bwzfXH", "isStatic": true, - "lastClassName": "jWbbwS", + "lastClassName": "iNPUwu", "rules": Array [ "padding: 0;", ], @@ -1451,7 +1451,7 @@ exports[` initially renders succesfully 1`] = ` "displayName": "Styled(Button)", "foldedComponentIds": Array [], "render": [Function], - "styledComponentId": "sc-htpNat", + "styledComponentId": "sc-bwzfXH", "target": [Function], "toString": [Function], "warnTooManyClasses": [Function], @@ -1464,7 +1464,7 @@ exports[` initially renders succesfully 1`] = ` >
+ + +
+ + + +
+ Email
@@ -2094,6 +2110,11 @@ exports[` initially renders succesfully 1`] = ` } successTurnedOn={false} toggleNotification={[Function]} + typeLabels={ + Object { + "email": "Email", + } + } > initially renders succesfully 1`] = ` } successTurnedOn={false} toggleNotification={[Function]} + typeLabels={ + Object { + "email": "Email", + } + } > initially renders succesfully 1`] = ` Notification two
- - email - + , + + Email , initially renders succesfully 1`] = ` - - - - - email - - - - + + + + + + + +
+ Email
From 12c0b80102d696c903b5fa56452e2572c400c4e9 Mon Sep 17 00:00:00 2001 From: mabashian Date: Mon, 1 Jul 2019 13:51:13 -0400 Subject: [PATCH 2/2] Prettify files --- .../NotificationListItem.jsx | 87 ++++++++++--------- .../NotificationListItem.test.jsx | 9 +- .../OrganizationNotifications.jsx | 34 +++++--- .../OrganizationNotifications.test.jsx | 37 ++++---- 4 files changed, 91 insertions(+), 76 deletions(-) diff --git a/awx/ui_next/src/components/NotificationsList/NotificationListItem.jsx b/awx/ui_next/src/components/NotificationsList/NotificationListItem.jsx index 38294bcb12..9578aec49e 100644 --- a/awx/ui_next/src/components/NotificationsList/NotificationListItem.jsx +++ b/awx/ui_next/src/components/NotificationsList/NotificationListItem.jsx @@ -47,47 +47,50 @@ function NotificationListItem(props) { key={notification.id} > - - - {notification.name} - - , - - {typeLabels[notification.notification_type]} - , - - toggleNotification( - notification.id, - successTurnedOn, - 'success' - )} - aria-label={i18n._(t`Toggle notification success`)} - /> - toggleNotification( - notification.id, - errorTurnedOn, - 'error' - )} - aria-label={i18n._(t`Toggle notification failure`)} - /> - - ]} + + + + {notification.name} + + + , + + {typeLabels[notification.notification_type]} + , + + + toggleNotification( + notification.id, + successTurnedOn, + 'success' + ) + } + aria-label={i18n._(t`Toggle notification success`)} + /> + + toggleNotification(notification.id, errorTurnedOn, 'error') + } + aria-label={i18n._(t`Toggle notification failure`)} + /> + , + ]} />
@@ -105,7 +108,7 @@ NotificationListItem.propTypes = { errorTurnedOn: bool, successTurnedOn: bool, toggleNotification: func.isRequired, - typeLabels: shape().isRequired + typeLabels: shape().isRequired, }; NotificationListItem.defaultProps = { diff --git a/awx/ui_next/src/components/NotificationsList/NotificationListItem.test.jsx b/awx/ui_next/src/components/NotificationsList/NotificationListItem.test.jsx index 66c0ad5a01..74d1f6a070 100644 --- a/awx/ui_next/src/components/NotificationsList/NotificationListItem.test.jsx +++ b/awx/ui_next/src/components/NotificationsList/NotificationListItem.test.jsx @@ -9,11 +9,11 @@ describe('', () => { const mockNotif = { id: 9000, name: 'Foo', - notification_type: 'slack' + notification_type: 'slack', }; const typeLabels = { - slack: 'Slack' + slack: 'Slack', }; beforeEach(() => { @@ -51,7 +51,10 @@ describe('', () => { typeLabels={typeLabels} /> ); - const typeCell = wrapper.find('DataListCell').at(1).find('div'); + const typeCell = wrapper + .find('DataListCell') + .at(1) + .find('div'); expect(typeCell.text()).toBe('Slack'); }); diff --git a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx index 206cbafd81..ec0334d3a9 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx @@ -35,7 +35,7 @@ class OrganizationNotifications extends Component { notifications: [], successTemplateIds: [], errorTemplateIds: [], - typeLabels: null + typeLabels: null, }; this.handleNotificationToggle = this.handleNotificationToggle.bind(this); this.handleNotificationErrorClose = this.handleNotificationErrorClose.bind( @@ -60,9 +60,7 @@ class OrganizationNotifications extends Component { const { typeLabels } = this.state; const params = parseNamespacedQueryString(QS_CONFIG, location.search); - const promises = [ - OrganizationsAPI.readNotificationTemplates(id, params) - ]; + const promises = [OrganizationsAPI.readNotificationTemplates(id, params)]; if (!typeLabels) { promises.push(OrganizationsAPI.readOptionsNotificationTemplates(id)); @@ -70,12 +68,12 @@ class OrganizationNotifications extends Component { this.setState({ contentError: null, hasContentLoading: true }); try { - const [{ - data: { - count: itemCount = 0, - results: notifications = [], - } - }, optionsResponse] = await Promise.all(promises); + const [ + { + data: { count: itemCount = 0, results: notifications = [] }, + }, + optionsResponse, + ] = await Promise.all(promises); let idMatchParams; if (notifications.length > 0) { @@ -100,10 +98,20 @@ class OrganizationNotifications extends Component { }; if (!typeLabels) { - const { data: { actions: { GET: { notification_type: {choices} } } } } = optionsResponse; + const { + data: { + actions: { + GET: { + notification_type: { choices }, + }, + }, + }, + } = optionsResponse; // The structure of choices looks like [['slack', 'Slack'], ['email', 'Email'], ...] - stateToUpdate.typeLabels = - choices.reduce((map, notifType) => ({ ...map, [notifType[0]]: notifType[1]}), {}); + stateToUpdate.typeLabels = choices.reduce( + (map, notifType) => ({ ...map, [notifType[0]]: notifType[1] }), + {} + ); } this.setState(stateToUpdate); diff --git a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.test.jsx b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.test.jsx index 06e2e4f41d..6aa6ee12d4 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.test.jsx @@ -10,17 +10,20 @@ jest.mock('@api'); describe('', () => { const data = { count: 2, - results: [{ - id: 1, - name: 'Notification one', - url: '/api/v2/notification_templates/1/', - notification_type: 'email', - }, { - id: 2, - name: 'Notification two', - url: '/api/v2/notification_templates/2/', - notification_type: 'email', - }] + results: [ + { + id: 1, + name: 'Notification one', + url: '/api/v2/notification_templates/1/', + notification_type: 'email', + }, + { + id: 2, + name: 'Notification two', + url: '/api/v2/notification_templates/2/', + notification_type: 'email', + }, + ], }; OrganizationsAPI.readOptionsNotificationTemplates.mockReturnValue({ @@ -28,13 +31,11 @@ describe('', () => { actions: { GET: { notification_type: { - choices: [ - ['email', 'Email'] - ] - } - } - } - } + choices: [['email', 'Email']], + }, + }, + }, + }, }); beforeEach(() => {