From b1044ad5b69f3d0ce01d1834943c7576f96b9151 Mon Sep 17 00:00:00 2001 From: Jules Rosser Date: Tue, 31 Jan 2023 18:25:23 +0000 Subject: [PATCH 1/6] don't bundle notification type in PushNotification export --- src/libs/Notification/PushNotification/index.js | 3 --- src/libs/Notification/PushNotification/index.native.js | 2 -- 2 files changed, 5 deletions(-) diff --git a/src/libs/Notification/PushNotification/index.js b/src/libs/Notification/PushNotification/index.js index c4100583442d..f12dbe05a105 100644 --- a/src/libs/Notification/PushNotification/index.js +++ b/src/libs/Notification/PushNotification/index.js @@ -1,11 +1,8 @@ -import NotificationType from './NotificationType'; - // Push notifications are only supported on mobile, so we'll just noop here export default { register: () => {}, deregister: () => {}, onReceived: () => {}, onSelected: () => {}, - TYPE: NotificationType, clearNotifications: () => {}, }; diff --git a/src/libs/Notification/PushNotification/index.native.js b/src/libs/Notification/PushNotification/index.native.js index 4dc3ad1c3a8e..88862717a41a 100644 --- a/src/libs/Notification/PushNotification/index.native.js +++ b/src/libs/Notification/PushNotification/index.native.js @@ -3,7 +3,6 @@ import {AppState} from 'react-native'; import {UrbanAirship, EventType, iOS} from 'urbanairship-react-native'; import lodashGet from 'lodash/get'; import Log from '../../Log'; -import NotificationType from './NotificationType'; import PushNotification from '.'; import * as Report from '../../actions/Report'; @@ -180,6 +179,5 @@ export default { deregister, onReceived, onSelected, - TYPE: NotificationType, clearNotifications, }; From 66f0d8f01f8bcd30791e0b29be1703fc9967526f Mon Sep 17 00:00:00 2001 From: Jules Rosser Date: Tue, 31 Jan 2023 18:25:56 +0000 Subject: [PATCH 2/6] use separate import for NotificationType, resolving dependency loop --- src/libs/actions/Report.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 7abb23497604..9c652e82f8b4 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -7,6 +7,7 @@ import ONYXKEYS from '../../ONYXKEYS'; import * as Pusher from '../Pusher/pusher'; import LocalNotification from '../Notification/LocalNotification'; import PushNotification from '../Notification/PushNotification'; +import NotificationType from '../Notification/PushNotification/NotificationType'; import Navigation from '../Navigation/Navigation'; import * as ActiveClientManager from '../ActiveClientManager'; import Visibility from '../Visibility'; @@ -53,13 +54,13 @@ function getReportChannelName(reportID) { * Setup reportComment push notification callbacks. */ function subscribeToReportCommentPushNotifications() { - PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, onyxData}) => { + PushNotification.onReceived(NotificationType.REPORT_COMMENT, ({reportID, onyxData}) => { Log.info('[Report] Handled event sent by Airship', false, {reportID}); Onyx.update(onyxData); }); // Open correct report when push notification is clicked - PushNotification.onSelected(PushNotification.TYPE.REPORT_COMMENT, ({reportID}) => { + PushNotification.onSelected(NotificationType.REPORT_COMMENT, ({reportID}) => { if (Navigation.canNavigate('navigate')) { // If a chat is visible other than the one we are trying to navigate to, then we need to navigate back if (Navigation.getActiveRoute().slice(1, 2) === ROUTES.REPORT && !Navigation.isActiveRoute(`r/${reportID}`)) { From 1e621401077ca6700db893389bcac5a170a58bfb Mon Sep 17 00:00:00 2001 From: Jules Rosser Date: Tue, 31 Jan 2023 18:43:39 +0000 Subject: [PATCH 3/6] further separate the PushNotification import loop --- src/libs/Notification/PushNotification/index.native.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libs/Notification/PushNotification/index.native.js b/src/libs/Notification/PushNotification/index.native.js index 88862717a41a..8fa1c1447788 100644 --- a/src/libs/Notification/PushNotification/index.native.js +++ b/src/libs/Notification/PushNotification/index.native.js @@ -3,8 +3,7 @@ import {AppState} from 'react-native'; import {UrbanAirship, EventType, iOS} from 'urbanairship-react-native'; import lodashGet from 'lodash/get'; import Log from '../../Log'; -import PushNotification from '.'; -import * as Report from '../../actions/Report'; +import {subscribeToReportCommentPushNotifications} from '../../actions/Report'; const notificationEventActionMap = {}; @@ -113,7 +112,7 @@ function register(accountID) { // while the app is still in background, we must resubscribe to the report // push notification in order to render the report click behaviour correctly PushNotification.init(); - Report.subscribeToReportCommentPushNotifications(); + subscribeToReportCommentPushNotifications(); } /** From 227de27bc7acfafd6f92f91774599dda5ce22c76 Mon Sep 17 00:00:00 2001 From: Jules Rosser Date: Wed, 1 Feb 2023 17:07:56 +0000 Subject: [PATCH 4/6] remove Push notification dependency loop --- src/libs/Notification/PushNotification/index.native.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/Notification/PushNotification/index.native.js b/src/libs/Notification/PushNotification/index.native.js index 8fa1c1447788..33a04eb046c0 100644 --- a/src/libs/Notification/PushNotification/index.native.js +++ b/src/libs/Notification/PushNotification/index.native.js @@ -111,7 +111,7 @@ function register(accountID) { // When the user logged out and then logged in with a different account // while the app is still in background, we must resubscribe to the report // push notification in order to render the report click behaviour correctly - PushNotification.init(); + init(); subscribeToReportCommentPushNotifications(); } From 007f60cac93fd13c365d675ae8e6ceddc5a655f1 Mon Sep 17 00:00:00 2001 From: Jules Rosser Date: Thu, 2 Feb 2023 15:41:23 +0000 Subject: [PATCH 5/6] invoke PushNotification.init from outside module --- src/libs/Notification/PushNotification/index.js | 1 + src/libs/Notification/PushNotification/index.native.js | 7 ------- src/libs/actions/Session/index.js | 5 +++++ 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/libs/Notification/PushNotification/index.js b/src/libs/Notification/PushNotification/index.js index f12dbe05a105..399ab4edbc1c 100644 --- a/src/libs/Notification/PushNotification/index.js +++ b/src/libs/Notification/PushNotification/index.js @@ -1,5 +1,6 @@ // Push notifications are only supported on mobile, so we'll just noop here export default { + init: () => {}, register: () => {}, deregister: () => {}, onReceived: () => {}, diff --git a/src/libs/Notification/PushNotification/index.native.js b/src/libs/Notification/PushNotification/index.native.js index 33a04eb046c0..22c569b810d3 100644 --- a/src/libs/Notification/PushNotification/index.native.js +++ b/src/libs/Notification/PushNotification/index.native.js @@ -3,7 +3,6 @@ import {AppState} from 'react-native'; import {UrbanAirship, EventType, iOS} from 'urbanairship-react-native'; import lodashGet from 'lodash/get'; import Log from '../../Log'; -import {subscribeToReportCommentPushNotifications} from '../../actions/Report'; const notificationEventActionMap = {}; @@ -107,12 +106,6 @@ function register(accountID) { // Regardless of the user's opt-in status, we still want to receive silent push notifications. Log.info(`[PUSH_NOTIFICATIONS] Subscribing to notifications for account ID ${accountID}`); UrbanAirship.setNamedUser(accountID.toString()); - - // When the user logged out and then logged in with a different account - // while the app is still in background, we must resubscribe to the report - // push notification in order to render the report click behaviour correctly - init(); - subscribeToReportCommentPushNotifications(); } /** diff --git a/src/libs/actions/Session/index.js b/src/libs/actions/Session/index.js index bcf3f0919de2..16c65f21b874 100644 --- a/src/libs/actions/Session/index.js +++ b/src/libs/actions/Session/index.js @@ -15,6 +15,7 @@ import * as Authentication from '../../Authentication'; import * as Welcome from '../Welcome'; import * as API from '../../API'; import * as NetworkStore from '../../Network/NetworkStore'; +import * as Report from '../../actions/Report'; import DateUtils from '../../DateUtils'; let credentials = {}; @@ -40,6 +41,10 @@ Onyx.connect({ if (accountID) { PushNotification.register(accountID); + + // Resubscribing is necessary in cases where the app was backgrounded and a different login was used + PushNotification.init(); + Report.subscribeToReportCommentPushNotifications(); } else { PushNotification.deregister(); PushNotification.clearNotifications(); From 465282a953490059e7ce92f0e28357d8908fab64 Mon Sep 17 00:00:00 2001 From: Jules Rosser Date: Thu, 2 Feb 2023 16:19:20 +0000 Subject: [PATCH 6/6] revert notification type refactor --- src/libs/Notification/PushNotification/index.js | 3 +++ src/libs/Notification/PushNotification/index.native.js | 2 ++ src/libs/actions/Report.js | 5 ++--- src/libs/actions/Session/index.js | 6 +++--- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libs/Notification/PushNotification/index.js b/src/libs/Notification/PushNotification/index.js index 399ab4edbc1c..88136ff5dc72 100644 --- a/src/libs/Notification/PushNotification/index.js +++ b/src/libs/Notification/PushNotification/index.js @@ -1,3 +1,5 @@ +import NotificationType from './NotificationType'; + // Push notifications are only supported on mobile, so we'll just noop here export default { init: () => {}, @@ -5,5 +7,6 @@ export default { deregister: () => {}, onReceived: () => {}, onSelected: () => {}, + TYPE: NotificationType, clearNotifications: () => {}, }; diff --git a/src/libs/Notification/PushNotification/index.native.js b/src/libs/Notification/PushNotification/index.native.js index 22c569b810d3..b6787858191d 100644 --- a/src/libs/Notification/PushNotification/index.native.js +++ b/src/libs/Notification/PushNotification/index.native.js @@ -3,6 +3,7 @@ import {AppState} from 'react-native'; import {UrbanAirship, EventType, iOS} from 'urbanairship-react-native'; import lodashGet from 'lodash/get'; import Log from '../../Log'; +import NotificationType from './NotificationType'; const notificationEventActionMap = {}; @@ -171,5 +172,6 @@ export default { deregister, onReceived, onSelected, + TYPE: NotificationType, clearNotifications, }; diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 9c652e82f8b4..7abb23497604 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -7,7 +7,6 @@ import ONYXKEYS from '../../ONYXKEYS'; import * as Pusher from '../Pusher/pusher'; import LocalNotification from '../Notification/LocalNotification'; import PushNotification from '../Notification/PushNotification'; -import NotificationType from '../Notification/PushNotification/NotificationType'; import Navigation from '../Navigation/Navigation'; import * as ActiveClientManager from '../ActiveClientManager'; import Visibility from '../Visibility'; @@ -54,13 +53,13 @@ function getReportChannelName(reportID) { * Setup reportComment push notification callbacks. */ function subscribeToReportCommentPushNotifications() { - PushNotification.onReceived(NotificationType.REPORT_COMMENT, ({reportID, onyxData}) => { + PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, onyxData}) => { Log.info('[Report] Handled event sent by Airship', false, {reportID}); Onyx.update(onyxData); }); // Open correct report when push notification is clicked - PushNotification.onSelected(NotificationType.REPORT_COMMENT, ({reportID}) => { + PushNotification.onSelected(PushNotification.TYPE.REPORT_COMMENT, ({reportID}) => { if (Navigation.canNavigate('navigate')) { // If a chat is visible other than the one we are trying to navigate to, then we need to navigate back if (Navigation.getActiveRoute().slice(1, 2) === ROUTES.REPORT && !Navigation.isActiveRoute(`r/${reportID}`)) { diff --git a/src/libs/actions/Session/index.js b/src/libs/actions/Session/index.js index 16c65f21b874..0ca656a95d42 100644 --- a/src/libs/actions/Session/index.js +++ b/src/libs/actions/Session/index.js @@ -15,7 +15,7 @@ import * as Authentication from '../../Authentication'; import * as Welcome from '../Welcome'; import * as API from '../../API'; import * as NetworkStore from '../../Network/NetworkStore'; -import * as Report from '../../actions/Report'; +import * as Report from '../Report'; import DateUtils from '../../DateUtils'; let credentials = {}; @@ -41,8 +41,8 @@ Onyx.connect({ if (accountID) { PushNotification.register(accountID); - - // Resubscribing is necessary in cases where the app was backgrounded and a different login was used + + // Prevent issue where report linking fails after users switch accounts without closing the app PushNotification.init(); Report.subscribeToReportCommentPushNotifications(); } else {