From 681b765b9afe62418c8860a47bce920eb7e46a60 Mon Sep 17 00:00:00 2001 From: mabashian Date: Mon, 3 Aug 2020 15:20:17 -0400 Subject: [PATCH 1/2] Adds support for toggling approval notifications on orgs and wfjts --- .../src/api/mixins/Notifications.mixin.js | 14 ++++ awx/ui_next/src/api/models/Organizations.js | 21 ++++++ .../src/api/models/WorkflowJobTemplates.js | 21 ++++++ .../NotificationList/NotificationList.jsx | 37 ++++++++++- .../NotificationList/NotificationListItem.jsx | 50 ++++++++++---- .../NotificationListItem.test.jsx | 65 +++++++++++++++++-- .../NotificationListItem.test.jsx.snap | 10 ++- .../CredentialEdit/CredentialEdit.test.jsx | 5 +- .../src/screens/Organization/Organization.jsx | 1 + .../screens/Template/WorkflowJobTemplate.jsx | 1 + 10 files changed, 200 insertions(+), 25 deletions(-) diff --git a/awx/ui_next/src/api/mixins/Notifications.mixin.js b/awx/ui_next/src/api/mixins/Notifications.mixin.js index 0198f0054f..87a7002ec5 100644 --- a/awx/ui_next/src/api/mixins/Notifications.mixin.js +++ b/awx/ui_next/src/api/mixins/Notifications.mixin.js @@ -87,6 +87,13 @@ const NotificationsMixin = parent => notificationId, notificationType ) { + if (notificationType === 'approvals') { + return this.associateNotificationTemplatesApprovals( + resourceId, + notificationId + ); + } + if (notificationType === 'started') { return this.associateNotificationTemplatesStarted( resourceId, @@ -126,6 +133,13 @@ const NotificationsMixin = parent => notificationId, notificationType ) { + if (notificationType === 'approvals') { + return this.disassociateNotificationTemplatesApprovals( + resourceId, + notificationId + ); + } + if (notificationType === 'started') { return this.disassociateNotificationTemplatesStarted( resourceId, diff --git a/awx/ui_next/src/api/models/Organizations.js b/awx/ui_next/src/api/models/Organizations.js index 267c9aba1e..a76b1e9ab1 100644 --- a/awx/ui_next/src/api/models/Organizations.js +++ b/awx/ui_next/src/api/models/Organizations.js @@ -19,6 +19,27 @@ class Organizations extends InstanceGroupsMixin(NotificationsMixin(Base)) { createUser(id, data) { return this.http.post(`${this.baseUrl}${id}/users/`, data); } + + readNotificationTemplatesApprovals(id, params) { + return this.http.get( + `${this.baseUrl}${id}/notification_templates_approvals/`, + { params } + ); + } + + associateNotificationTemplatesApprovals(resourceId, notificationId) { + return this.http.post( + `${this.baseUrl}${resourceId}/notification_templates_approvals/`, + { id: notificationId } + ); + } + + disassociateNotificationTemplatesApprovals(resourceId, notificationId) { + return this.http.post( + `${this.baseUrl}${resourceId}/notification_templates_approvals/`, + { id: notificationId, disassociate: true } + ); + } } export default Organizations; diff --git a/awx/ui_next/src/api/models/WorkflowJobTemplates.js b/awx/ui_next/src/api/models/WorkflowJobTemplates.js index 3074608796..91739d1082 100644 --- a/awx/ui_next/src/api/models/WorkflowJobTemplates.js +++ b/awx/ui_next/src/api/models/WorkflowJobTemplates.js @@ -65,6 +65,27 @@ class WorkflowJobTemplates extends SchedulesMixin(NotificationsMixin(Base)) { destroySurvey(id) { return this.http.delete(`${this.baseUrl}${id}/survey_spec/`); } + + readNotificationTemplatesApprovals(id, params) { + return this.http.get( + `${this.baseUrl}${id}/notification_templates_approvals/`, + { params } + ); + } + + associateNotificationTemplatesApprovals(resourceId, notificationId) { + return this.http.post( + `${this.baseUrl}${resourceId}/notification_templates_approvals/`, + { id: notificationId } + ); + } + + disassociateNotificationTemplatesApprovals(resourceId, notificationId) { + return this.http.post( + `${this.baseUrl}${resourceId}/notification_templates_approvals/`, + { id: notificationId, disassociate: true } + ); + } } export default WorkflowJobTemplates; diff --git a/awx/ui_next/src/components/NotificationList/NotificationList.jsx b/awx/ui_next/src/components/NotificationList/NotificationList.jsx index b1bca91d37..f274e4cca3 100644 --- a/awx/ui_next/src/components/NotificationList/NotificationList.jsx +++ b/awx/ui_next/src/components/NotificationList/NotificationList.jsx @@ -17,7 +17,13 @@ const QS_CONFIG = getQSConfig('notification', { order_by: 'name', }); -function NotificationList({ apiModel, canToggleNotifications, id, i18n }) { +function NotificationList({ + apiModel, + canToggleNotifications, + id, + i18n, + showApprovalsToggle, +}) { const location = useLocation(); const [isToggleLoading, setIsToggleLoading] = useState(false); const [toggleError, setToggleError] = useState(null); @@ -27,6 +33,7 @@ function NotificationList({ apiModel, canToggleNotifications, id, i18n }) { result: { notifications, itemCount, + approvalsTemplateIds, startedTemplateIds, successTemplateIds, errorTemplateIds, @@ -71,7 +78,7 @@ function NotificationList({ apiModel, canToggleNotifications, id, i18n }) { apiModel.readNotificationTemplatesError(id, idMatchParams), ]); - return { + const rtnObj = { notifications: notificationsResults, itemCount: notificationsCount, startedTemplateIds: startedTemplates.results.map(st => st.id), @@ -79,10 +86,27 @@ function NotificationList({ apiModel, canToggleNotifications, id, i18n }) { errorTemplateIds: errorTemplates.results.map(e => e.id), typeLabels: labels, }; - }, [apiModel, id, location]), + + if (showApprovalsToggle) { + const { + data: approvalsTemplates, + } = await apiModel.readNotificationTemplatesApprovals( + id, + idMatchParams + ); + rtnObj.approvalsTemplateIds = approvalsTemplates.results.map( + st => st.id + ); + } else { + rtnObj.approvalsTemplateIds = []; + } + + return rtnObj; + }, [apiModel, id, location, showApprovalsToggle]), { notifications: [], itemCount: 0, + approvalsTemplateIds: [], startedTemplateIds: [], successTemplateIds: [], errorTemplateIds: [], @@ -186,10 +210,12 @@ function NotificationList({ apiModel, canToggleNotifications, id, i18n }) { detailUrl={`/notifications/${notification.id}`} canToggleNotifications={canToggleNotifications && !isToggleLoading} toggleNotification={handleNotificationToggle} + approvalsTurnedOn={approvalsTemplateIds.includes(notification.id)} errorTurnedOn={errorTemplateIds.includes(notification.id)} startedTurnedOn={startedTemplateIds.includes(notification.id)} successTurnedOn={successTemplateIds.includes(notification.id)} typeLabels={typeLabels} + showApprovalsToggle={showApprovalsToggle} /> )} /> @@ -212,6 +238,11 @@ NotificationList.propTypes = { apiModel: shape({}).isRequired, id: number.isRequired, canToggleNotifications: bool.isRequired, + showApprovalsToggle: bool, +}; + +NotificationList.defaultProps = { + showApprovalsToggle: false, }; export default withI18n()(NotificationList); diff --git a/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx b/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx index 6857114479..cbf048bc7c 100644 --- a/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx +++ b/awx/ui_next/src/components/NotificationList/NotificationListItem.jsx @@ -17,25 +17,25 @@ const DataListAction = styled(_DataListAction)` align-items: center; display: grid; grid-gap: 16px; - grid-template-columns: repeat(3, max-content); + grid-template-columns: ${props => `repeat(${props.columns}, max-content)`}; `; const Label = styled.b` margin-right: 20px; `; -function NotificationListItem(props) { - const { - canToggleNotifications, - notification, - detailUrl, - startedTurnedOn, - successTurnedOn, - errorTurnedOn, - toggleNotification, - i18n, - typeLabels, - } = props; - +function NotificationListItem({ + canToggleNotifications, + notification, + detailUrl, + approvalsTurnedOn, + startedTurnedOn, + successTurnedOn, + errorTurnedOn, + toggleNotification, + i18n, + typeLabels, + showApprovalsToggle, +}) { return ( + {showApprovalsToggle && ( + + toggleNotification( + notification.id, + approvalsTurnedOn, + 'approvals' + ) + } + aria-label={i18n._(t`Toggle notification approvals`)} + /> + )} ', () => { /> ); expect(wrapper.find('NotificationListItem')).toMatchSnapshot(); + expect(wrapper.find('Switch').length).toBe(3); + }); + + test('shows approvals toggle when configured', () => { + wrapper = mountWithContexts( + + ); + expect(wrapper.find('Switch').length).toBe(4); }); test('displays correct label in correct column', () => { @@ -58,7 +73,46 @@ describe('', () => { expect(typeCell.text()).toContain('Slack'); }); - test('handles start click when toggle is on', () => { + test('handles approvals click when toggle is on', () => { + wrapper = mountWithContexts( + + ); + wrapper + .find('Switch[aria-label="Toggle notification approvals"]') + .first() + .find('input') + .simulate('change'); + expect(toggleNotification).toHaveBeenCalledWith(9000, true, 'approvals'); + }); + + test('handles approvals click when toggle is off', () => { + wrapper = mountWithContexts( + + ); + wrapper + .find('Switch[aria-label="Toggle notification approvals"]') + .find('input') + .simulate('change'); + expect(toggleNotification).toHaveBeenCalledWith(9000, false, 'approvals'); + }); + + test('handles started click when toggle is on', () => { wrapper = mountWithContexts( ', () => { /> ); wrapper - .find('Switch') - .first() + .find('Switch[aria-label="Toggle notification start"]') .find('input') .simulate('change'); expect(toggleNotification).toHaveBeenCalledWith(9000, true, 'started'); }); - test('handles start click when toggle is off', () => { + test('handles started click when toggle is off', () => { wrapper = mountWithContexts( ', () => { expect(toggleNotification).toHaveBeenCalledWith(9000, false, 'started'); }); - test('handles error click when toggle is on', () => { + test('handles success click when toggle is on', () => { wrapper = mountWithContexts( ', () => { expect(toggleNotification).toHaveBeenCalledWith(9000, true, 'success'); }); - test('handles error click when toggle is off', () => { + test('handles success click when toggle is off', () => { wrapper = mountWithContexts( initially renders succesfully and displays correct label 1`] = ` initially renders succe "notification_type": "slack", } } + showApprovalsToggle={false} startedTurnedOn={false} successTurnedOn={false} toggleNotification={[MockFunction]} @@ -215,6 +217,7 @@ exports[` initially renders succe initially renders succe initially renders succe align-items: center; display: grid; grid-gap: 16px; - grid-template-columns: repeat(3, max-content); + grid-template-columns: ", + [Function], + "; ", ], }, @@ -257,11 +263,13 @@ exports[` initially renders succe aria-label="actions" aria-labelledby="items-list-item-9000" className="sc-bwzfXH llKtln" + columns={3} id="items-list-item-9000" rowid="items-list-item-9000" >
', () => { test('handleCancel returns the user to credential detail', async () => { await waitForElement(wrapper, 'isLoading', el => el.length === 0); - wrapper.find('Button[aria-label="Cancel"]').simulate('click'); + await act(async () => { + wrapper.find('Button[aria-label="Cancel"]').simulate('click'); + }); + wrapper.update(); expect(history.location.pathname).toEqual('/credentials/3/details'); }); diff --git a/awx/ui_next/src/screens/Organization/Organization.jsx b/awx/ui_next/src/screens/Organization/Organization.jsx index 6e8cf87f1d..d16113d8dc 100644 --- a/awx/ui_next/src/screens/Organization/Organization.jsx +++ b/awx/ui_next/src/screens/Organization/Organization.jsx @@ -200,6 +200,7 @@ class Organization extends Component { id={Number(match.params.id)} canToggleNotifications={canToggleNotifications} apiModel={OrganizationsAPI} + showApprovalsToggle /> )} diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx index 8f19ebb6d3..51abaab73a 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx @@ -233,6 +233,7 @@ class WorkflowJobTemplate extends Component { id={Number(match.params.id)} canToggleNotifications={canToggleNotifications} apiModel={WorkflowJobTemplatesAPI} + showApprovalsToggle /> )} From daeb5a8de8b42d22b23fbe5ea646fdeed495568c Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 20 Aug 2020 14:24:05 -0400 Subject: [PATCH 2/2] Only disable single notification row when toggling, not all rows --- .../NotificationList/NotificationList.jsx | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/awx/ui_next/src/components/NotificationList/NotificationList.jsx b/awx/ui_next/src/components/NotificationList/NotificationList.jsx index f274e4cca3..b11e27f9e5 100644 --- a/awx/ui_next/src/components/NotificationList/NotificationList.jsx +++ b/awx/ui_next/src/components/NotificationList/NotificationList.jsx @@ -25,7 +25,7 @@ function NotificationList({ showApprovalsToggle, }) { const location = useLocation(); - const [isToggleLoading, setIsToggleLoading] = useState(false); + const [loadingToggleIds, setLoadingToggleIds] = useState([]); const [toggleError, setToggleError] = useState(null); const { @@ -123,7 +123,7 @@ function NotificationList({ isCurrentlyOn, status ) => { - setIsToggleLoading(true); + setLoadingToggleIds(loadingToggleIds.concat([notificationId])); try { if (isCurrentlyOn) { await apiModel.disassociateNotificationTemplate( @@ -153,7 +153,9 @@ function NotificationList({ } catch (err) { setToggleError(err); } finally { - setIsToggleLoading(false); + setLoadingToggleIds( + loadingToggleIds.filter(item => item !== notificationId) + ); } }; @@ -208,7 +210,10 @@ function NotificationList({ key={notification.id} notification={notification} detailUrl={`/notifications/${notification.id}`} - canToggleNotifications={canToggleNotifications && !isToggleLoading} + canToggleNotifications={ + canToggleNotifications && + !loadingToggleIds.includes(notification.id) + } toggleNotification={handleNotificationToggle} approvalsTurnedOn={approvalsTemplateIds.includes(notification.id)} errorTurnedOn={errorTemplateIds.includes(notification.id)} @@ -223,7 +228,7 @@ function NotificationList({ setToggleError(null)} > {i18n._(t`Failed to toggle notification.`)}