From d14f1ab5b94a404a03ef0f32f4abdf78c7faefe0 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sun, 3 May 2020 13:15:57 -0700 Subject: [PATCH 1/4] feat: enable unsubscribing notifications --- src/js/actions/index.ts | 34 ++++++++++++++++++++++++++++++ src/js/components/notification.tsx | 13 +++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/js/actions/index.ts b/src/js/actions/index.ts index 66af262a7..5420883e9 100644 --- a/src/js/actions/index.ts +++ b/src/js/actions/index.ts @@ -156,6 +156,40 @@ export function markNotification(id, hostname) { }; } +export const UNSUBSCRIBE_NOTIFICATION = makeAsyncActionSet( + 'UNSUBSCRIBE_NOTIFICATION' +); +export function unsubscribeNotification(id, hostname) { + return (dispatch, getState: () => AppState) => { + const url = `${generateGitHubAPIUrl( + hostname + )}notifications/threads/${id}/subscription`; + const method = 'DELETE'; + + const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname; + const entAccounts = getState().auth.enterpriseAccounts; + const token = isEnterprise + ? getEnterpriseAccountToken(hostname, entAccounts) + : getState().auth.token; + + dispatch({ type: UNSUBSCRIBE_NOTIFICATION.REQUEST }); + + return apiRequestAuth(url, method, token, {}) + .then(function (response) { + dispatch({ + type: UNSUBSCRIBE_NOTIFICATION.SUCCESS, + meta: { id, hostname }, + }); + }) + .catch(function (error) { + dispatch({ + type: UNSUBSCRIBE_NOTIFICATION.FAILURE, + payload: error.response.data, + }); + }); + }; +} + // Repo's Notification export const MARK_REPO_NOTIFICATION = makeAsyncActionSet( diff --git a/src/js/components/notification.tsx b/src/js/components/notification.tsx index ad15cd7bf..40395cf6d 100644 --- a/src/js/components/notification.tsx +++ b/src/js/components/notification.tsx @@ -3,7 +3,7 @@ const { shell } = require('electron'); import * as React from 'react'; import { connect } from 'react-redux'; import { formatDistanceToNow, parseISO } from 'date-fns'; -import Octicon, { Check, getIconByName } from '@primer/octicons-react'; +import Octicon, { Check, Mute, getIconByName } from '@primer/octicons-react'; import styled from 'styled-components'; import { AppState } from '../../types/reducers'; @@ -67,6 +67,7 @@ interface IProps { notification: Notification; markOnClick: boolean; markNotification: (id: string, hostname: string) => void; + unsubscribeNotification?: (id: string, hostname: string) => void; } export const NotificationItem: React.FC = (props) => { @@ -91,6 +92,11 @@ export const NotificationItem: React.FC = (props) => { props.markNotification(notification.id, hostname); }; + const unsubscribe = () => { + const { hostname, notification } = props; + props.unsubscribeNotification(notification.id, hostname); + }; + const { notification } = props; const reason = formatReason(notification.reason); const typeIcon = getNotificationTypeIcon(notification.subject.type); @@ -115,6 +121,11 @@ export const NotificationItem: React.FC = (props) => { {updatedAt} + + + +
diff --git a/src/js/components/notification.test.tsx b/src/js/components/notification.test.tsx index 75c003364..164945823 100644 --- a/src/js/components/notification.test.tsx +++ b/src/js/components/notification.test.tsx @@ -32,6 +32,7 @@ describe('components/notification.js', () => { const props = { markNotification: jest.fn(), + unsubscribeNotification: jest.fn(), markOnClick: false, notification: notification, hostname: 'github.com', @@ -44,6 +45,7 @@ describe('components/notification.js', () => { it('should open a notification in the browser', () => { const props = { markNotification: jest.fn(), + unsubscribeNotification: jest.fn(), markOnClick: false, notification: notification, hostname: 'github.com', @@ -54,30 +56,46 @@ describe('components/notification.js', () => { expect(shell.openExternal).toHaveBeenCalledTimes(1); }); - it('should mark a notification as read', () => { + it('should open a notification in browser & mark it as read', () => { const props = { markNotification: jest.fn(), - markOnClick: false, + unsubscribeNotification: jest.fn(), + markOnClick: true, notification: notification, hostname: 'github.com', }; const { getByRole } = render(); - fireEvent.click(getByRole('button')); + fireEvent.click(getByRole('main')); + expect(shell.openExternal).toHaveBeenCalledTimes(1); expect(props.markNotification).toHaveBeenCalledTimes(1); }); - it('should open a notification in browser & mark it as read', () => { + it('should mark a notification as read', () => { const props = { markNotification: jest.fn(), - markOnClick: true, + unsubscribeNotification: jest.fn(), + markOnClick: false, notification: notification, hostname: 'github.com', }; - const { getByRole } = render(); - fireEvent.click(getByRole('main')); - expect(shell.openExternal).toHaveBeenCalledTimes(1); + const { getByLabelText } = render(); + fireEvent.click(getByLabelText('Mark as Read')); expect(props.markNotification).toHaveBeenCalledTimes(1); }); + + it('should unsubscribe from a notification thread', () => { + const props = { + markNotification: jest.fn(), + unsubscribeNotification: jest.fn(), + markOnClick: false, + notification: notification, + hostname: 'github.com', + }; + + const { getByLabelText } = render(); + fireEvent.click(getByLabelText('Unsubscribe')); + expect(props.unsubscribeNotification).toHaveBeenCalledTimes(1); + }); }); From db84e7e1fc9ed0c0916699a5793ae9921529e9a2 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sun, 3 May 2020 17:14:55 -0700 Subject: [PATCH 3/4] fix: make icon smaller and secondary --- src/js/components/notification.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/js/components/notification.tsx b/src/js/components/notification.tsx index 40395cf6d..ab0d4ed4b 100644 --- a/src/js/components/notification.tsx +++ b/src/js/components/notification.tsx @@ -119,13 +119,11 @@ export const NotificationItem: React.FC = (props) => {
{reason.type} - Updated{' '} {updatedAt} +
- - -
- -
- + + + + +
props.theme.danger}; + cursor: pointer; + } +`; + interface IProps { hostname: string; notification: Notification; @@ -92,7 +103,10 @@ export const NotificationItem: React.FC = (props) => { props.markNotification(notification.id, hostname); }; - const unsubscribe = () => { + const unsubscribe = (event: React.MouseEvent) => { + // Don't trigger onClick of parent element. + event.stopPropagation(); + const { hostname, notification } = props; props.unsubscribeNotification(notification.id, hostname); }; @@ -119,15 +133,15 @@ export const NotificationItem: React.FC = (props) => {
{reason.type} - Updated{' '} {updatedAt} - +
- + ); @@ -139,4 +153,7 @@ export function mapStateToProps(state: AppState) { }; } -export default connect(mapStateToProps, { markNotification })(NotificationItem); +export default connect(mapStateToProps, { + markNotification, + unsubscribeNotification, +})(NotificationItem); diff --git a/src/js/middleware/notifications.ts b/src/js/middleware/notifications.ts index 0c5872aee..835f4e438 100644 --- a/src/js/middleware/notifications.ts +++ b/src/js/middleware/notifications.ts @@ -3,6 +3,7 @@ import { NOTIFICATIONS, MARK_NOTIFICATION, MARK_REPO_NOTIFICATION, + UNSUBSCRIBE_NOTIFICATION, } from '../actions'; import NativeNotifications from '../utils/notifications'; import { updateTrayIcon } from '../utils/comms'; @@ -56,6 +57,7 @@ export default (store) => (next) => (action) => { break; case MARK_NOTIFICATION.SUCCESS: + case UNSUBSCRIBE_NOTIFICATION.SUCCESS: const prevNotificationsCount = accountNotifications.reduce( (memo, acc) => memo + acc.notifications.length, 0 diff --git a/src/js/reducers/notifications.ts b/src/js/reducers/notifications.ts index 19a400b30..8c566411b 100644 --- a/src/js/reducers/notifications.ts +++ b/src/js/reducers/notifications.ts @@ -3,6 +3,7 @@ import { NOTIFICATIONS, MARK_NOTIFICATION, MARK_REPO_NOTIFICATION, + UNSUBSCRIBE_NOTIFICATION, } from '../actions'; import { LOGOUT } from '../../types/actions'; import { Notification } from '../../types/github'; @@ -26,6 +27,7 @@ export default function reducer( case NOTIFICATIONS.FAILURE: return { ...state, isFetching: false, failed: true, response: [] }; case MARK_NOTIFICATION.SUCCESS: + case UNSUBSCRIBE_NOTIFICATION.SUCCESS: const accountIndex = state.response.findIndex( (obj) => obj.hostname === action.meta.hostname );