Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear notifications after opening a report #31762

Merged
merged 55 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
3c5c0fa
cache notifications so they can be cleared later
arosiclair Nov 22, 2023
f67a53f
replace unnecessary object params with regular params
arosiclair Nov 22, 2023
43667cb
remove unused delay functionality
arosiclair Nov 22, 2023
dfd1246
replace object params with regular params
arosiclair Nov 22, 2023
6f432cf
remove unused promise
arosiclair Nov 22, 2023
3156398
add reportID to notification data
arosiclair Nov 22, 2023
78cadff
clear local notifications when opening a report
arosiclair Nov 22, 2023
9d8d1f7
fix data for modified expense notification
arosiclair Nov 22, 2023
3ef347c
use onclose to clear the cache
arosiclair Nov 27, 2023
e4ca268
use empty object by default for safety
arosiclair Nov 27, 2023
1f0b1a0
improve docs for LocalNotification lib
arosiclair Nov 27, 2023
ed5e3a1
clear local notifications when the window is focused
arosiclair Nov 27, 2023
041354f
prevent background ReportScreens from clearing notifications
arosiclair Nov 28, 2023
8d442eb
undo name change to avoid new JS lib error
arosiclair Nov 28, 2023
99a6df1
prettier
arosiclair Nov 28, 2023
e41144f
clarifying comments
arosiclair Nov 28, 2023
0f5bd2e
rename prevNotificationID to notificationID
arosiclair Nov 29, 2023
0bd4077
Merge branch 'arosiclair-clear-read-notifications' into arosiclair-cl…
arosiclair Nov 29, 2023
ea5f257
Merge branch 'main' of github.com:Expensify/App into arosiclair-clear…
arosiclair Nov 29, 2023
6edd1d9
add android native module for clearing notifications
arosiclair Nov 29, 2023
8586346
Merge branch 'main' of github.com:Expensify/App into arosiclair-clear…
arosiclair Nov 30, 2023
e5923f2
add clearReportNotifications lib and use airship APIs on native
arosiclair Nov 30, 2023
03e0b52
remove unnecessary native lib
arosiclair Nov 30, 2023
88d388a
Revert "rename prevNotificationID to notificationID"
arosiclair Nov 30, 2023
81737f1
Merge branch 'main' of github.com:Expensify/App into arosiclair-clear…
arosiclair Dec 5, 2023
872c430
param fixes
arosiclair Dec 5, 2023
d8b3479
convert to typescript
arosiclair Dec 5, 2023
a25b7a7
use common types module and default to web implementation
arosiclair Dec 5, 2023
4657139
correctly parse the notification and report IDs
arosiclair Dec 5, 2023
8de5ed2
delete accidental jsbundle
arosiclair Dec 5, 2023
d917b93
Merge branch 'main' of github.com:Expensify/App into arosiclair-clear…
arosiclair Dec 6, 2023
84e563e
use new NotificationData type
arosiclair Dec 6, 2023
302c394
mock getActiveNotifications to fix tests
arosiclair Dec 6, 2023
af158c5
remove useless jsdoc
arosiclair Dec 6, 2023
8e27363
prettier
arosiclair Dec 6, 2023
b420620
simplify clearNotifications
arosiclair Dec 6, 2023
4e8f316
use AppState on native
arosiclair Dec 7, 2023
e4e342e
rename to useAppFocusEvent
arosiclair Dec 7, 2023
05a19de
use airship v15.3.1 patch
arosiclair Dec 7, 2023
afa8e4a
Merge branch 'main' of github.com:Expensify/App into arosiclair-clear…
arosiclair Dec 7, 2023
8776f36
use release airship 15.3.1 version
arosiclair Dec 7, 2023
9b5f2dc
actually use the release version of 15.3.1
arosiclair Dec 7, 2023
6dce2d2
update podfile and gemfile
arosiclair Dec 7, 2023
9ebe227
catch and log unexpected errors
arosiclair Dec 8, 2023
01e7905
Merge branch 'main' of github.com:Expensify/App into arosiclair-clear…
arosiclair Dec 11, 2023
2cbaaef
Merge branch 'main' of github.com:Expensify/App into arosiclair-clear…
arosiclair Dec 12, 2023
dc7bac6
Merge branch 'main' of github.com:Expensify/App into arosiclair-clear…
arosiclair Dec 13, 2023
c6c3e98
remove inconsistent param validation
arosiclair Dec 14, 2023
2f7707f
reduce number of array iterations
arosiclair Dec 14, 2023
a318f0f
move native implementation to PushNotification lib
arosiclair Dec 14, 2023
7a6581c
ensure the app is visible before marking the report read
arosiclair Dec 14, 2023
be23d55
use unified type for ClearReportNotifications
arosiclair Dec 14, 2023
84acbc0
Merge branch 'main' of github.com:Expensify/App into arosiclair-clear…
arosiclair Dec 14, 2023
dc3cc66
undo accidental change
arosiclair Dec 14, 2023
ede3435
set readActionSkipped only when report is unread
arosiclair Dec 15, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ GEM
declarative (0.0.20)
digest-crc (0.6.5)
rake (>= 12.0.0, < 14.0.0)
domain_name (0.6.20231109)
domain_name (0.5.20190701)
unf (>= 0.0.5, < 1.0.0)
dotenv (2.8.1)
emoji_regex (3.2.3)
escape (0.0.4)
Expand Down Expand Up @@ -261,6 +262,9 @@ GEM
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
uber (0.1.0)
unf (0.1.4)
unf_ext
unf_ext (0.0.9.1)
unicode-display_width (2.5.0)
webrick (1.8.1)
word_wrap (1.0.0)
Expand Down Expand Up @@ -294,4 +298,4 @@ RUBY VERSION
ruby 2.6.10p210

BUNDLED WITH
2.1.4
2.4.7
1 change: 1 addition & 0 deletions __mocks__/@ua/react-native-airship.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const Airship = {
enableUserNotifications: () => Promise.resolve(false),
clearNotifications: jest.fn(),
getNotificationStatus: () => Promise.resolve({airshipOptIn: false, systemEnabled: false}),
getActiveNotifications: () => Promise.resolve([]),
},
contact: {
identify: jest.fn(),
Expand Down
42 changes: 21 additions & 21 deletions ios/Podfile.lock
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
PODS:
- Airship (16.11.3):
- Airship/Automation (= 16.11.3)
- Airship/Basement (= 16.11.3)
- Airship/Core (= 16.11.3)
- Airship/ExtendedActions (= 16.11.3)
- Airship/MessageCenter (= 16.11.3)
- Airship/Automation (16.11.3):
- Airship (16.12.1):
- Airship/Automation (= 16.12.1)
- Airship/Basement (= 16.12.1)
- Airship/Core (= 16.12.1)
- Airship/ExtendedActions (= 16.12.1)
- Airship/MessageCenter (= 16.12.1)
- Airship/Automation (16.12.1):
- Airship/Core
- Airship/Basement (16.11.3)
- Airship/Core (16.11.3):
- Airship/Basement (16.12.1)
- Airship/Core (16.12.1):
- Airship/Basement
- Airship/ExtendedActions (16.11.3):
- Airship/ExtendedActions (16.12.1):
- Airship/Core
- Airship/MessageCenter (16.11.3):
- Airship/MessageCenter (16.12.1):
- Airship/Core
- Airship/PreferenceCenter (16.11.3):
- Airship/PreferenceCenter (16.12.1):
- Airship/Core
- AirshipFrameworkProxy (2.0.8):
- Airship (= 16.11.3)
- Airship/MessageCenter (= 16.11.3)
- Airship/PreferenceCenter (= 16.11.3)
- AirshipFrameworkProxy (2.1.1):
- Airship (= 16.12.1)
- Airship/MessageCenter (= 16.12.1)
- Airship/PreferenceCenter (= 16.12.1)
- AppAuth (1.6.2):
- AppAuth/Core (= 1.6.2)
- AppAuth/ExternalUserAgent (= 1.6.2)
Expand Down Expand Up @@ -558,8 +558,8 @@ PODS:
- React-jsinspector (0.72.4)
- React-logger (0.72.4):
- glog
- react-native-airship (15.2.6):
- AirshipFrameworkProxy (= 2.0.8)
- react-native-airship (15.3.1):
- AirshipFrameworkProxy (= 2.1.1)
- React-Core
- react-native-blob-util (0.17.3):
- React-Core
Expand Down Expand Up @@ -1160,8 +1160,8 @@ EXTERNAL SOURCES:
:path: "../node_modules/react-native/ReactCommon/yoga"

SPEC CHECKSUMS:
Airship: c70eed50e429f97f5adb285423c7291fb7a032ae
AirshipFrameworkProxy: 7bc4130c668c6c98e2d4c60fe4c9eb61a999be99
Airship: 2f4510b497a8200780752a5e0304a9072bfffb6d
AirshipFrameworkProxy: ea1b6c665c798637b93c465b5e505be3011f1d9d
AppAuth: 3bb1d1cd9340bd09f5ed189fb00b1cc28e1e8570
boost: 57d2868c099736d80fcd648bf211b4431e51a558
BVLinearGradient: 421743791a59d259aec53f4c58793aad031da2ca
Expand Down Expand Up @@ -1224,7 +1224,7 @@ SPEC CHECKSUMS:
React-jsiexecutor: c7f826e40fa9cab5d37cab6130b1af237332b594
React-jsinspector: aaed4cf551c4a1c98092436518c2d267b13a673f
React-logger: da1ebe05ae06eb6db4b162202faeafac4b435e77
react-native-airship: 5d19f4ba303481cf4101ff9dee9249ef6a8a6b64
react-native-airship: 6ded22e4ca54f2f80db80b7b911c2b9b696d9335
react-native-blob-util: 99f4d79189252f597fe0d810c57a3733b1b1dea6
react-native-cameraroll: 8ffb0af7a5e5de225fd667610e2979fc1f0c2151
react-native-config: 7cd105e71d903104e8919261480858940a6b9c0e
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
"@rnmapbox/maps": "^10.0.11",
"@shopify/flash-list": "^1.6.1",
"@types/node": "^18.14.0",
"@ua/react-native-airship": "^15.2.6",
"@ua/react-native-airship": "^15.3.1",
"awesome-phonenumber": "^5.4.0",
"babel-polyfill": "^6.26.0",
"canvas-size": "^1.2.6",
Expand Down
20 changes: 20 additions & 0 deletions src/hooks/useAppFocusEvent/index.native.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import {useEffect} from 'react';
import {AppState} from 'react-native';
import {UseAppFocusEvent, UseAppFocusEventCallback} from './types';

const useAppFocusEvent: UseAppFocusEvent = (callback: UseAppFocusEventCallback) => {
useEffect(() => {
const subscription = AppState.addEventListener('change', (appState) => {
if (appState !== 'active') {
return;
}
callback();
});

return () => {
subscription.remove();
};
}, [callback]);
};

export default useAppFocusEvent;
19 changes: 19 additions & 0 deletions src/hooks/useAppFocusEvent/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import {useEffect} from 'react';
import {UseAppFocusEvent, UseAppFocusEventCallback} from './types';

/**
* Runs the given callback when the app is focused (eg: after re-opening the app, switching tabs, or focusing the window)
*
* @param callback the function to run when the app is focused. This should be memoized with `useCallback`.
*/
const useAppFocusEvent: UseAppFocusEvent = (callback: UseAppFocusEventCallback) => {
useEffect(() => {
window.addEventListener('focus', callback);

return () => {
window.removeEventListener('focus', callback);
};
}, [callback]);
};

export default useAppFocusEvent;
5 changes: 5 additions & 0 deletions src/hooks/useAppFocusEvent/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type UseAppFocusEventCallback = () => void;

type UseAppFocusEvent = (callback: UseAppFocusEventCallback) => void;

export type {UseAppFocusEventCallback, UseAppFocusEvent};
122 changes: 61 additions & 61 deletions src/libs/Notification/LocalNotification/BrowserNotifications.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// Web and desktop implementation only. Do not import for direct use. Use LocalNotification.
import Str from 'expensify-common/lib/str';
import {ImageSourcePropType} from 'react-native';
import EXPENSIFY_ICON_URL from '@assets/images/expensify-logo-round-clearspace.png';
import * as ReportUtils from '@libs/ReportUtils';
import * as AppUpdate from '@userActions/AppUpdate';
import {Report, ReportAction} from '@src/types/onyx';
import focusApp from './focusApp';
import {PushParams, ReportCommentParams} from './types';
import {LocalNotificationClickHandler, LocalNotificationData} from './types';

const DEFAULT_DELAY = 4000;
const notificationCache: Record<string, Notification> = {};

/**
* Checks if the user has granted permission to show browser notifications
Expand Down Expand Up @@ -34,42 +37,33 @@ function canUseBrowserNotifications(): Promise<boolean> {
/**
* Light abstraction around browser push notifications.
* Checks for permission before determining whether to send.
* @return resolves with Notification object or undefined
*
* @param icon Path to icon
* @param data extra data to attach to the notification
*/
function push({title, body, delay = DEFAULT_DELAY, onClick = () => {}, tag = '', icon}: PushParams): Promise<void | Notification> {
return new Promise((resolve) => {
if (!title || !body) {
throw new Error('BrowserNotification must include title and body parameter.');
function push(title: string, body = '', icon: string | ImageSourcePropType = '', data: LocalNotificationData = {}, onClick: LocalNotificationClickHandler = () => {}) {
canUseBrowserNotifications().then((canUseNotifications) => {
if (!canUseNotifications) {
return;
}

canUseBrowserNotifications().then((canUseNotifications) => {
if (!canUseNotifications) {
resolve();
return;
}

const notification = new Notification(title, {
body,
tag,
icon: String(icon),
});

// If we pass in a delay param greater than 0 the notification
// will auto-close after the specified time.
if (delay > 0) {
setTimeout(notification.close.bind(notification), delay);
}

notification.onclick = () => {
onClick();
window.parent.focus();
window.focus();
focusApp();
notification.close();
};

resolve(notification);
// We cache these notifications so that we can clear them later
const notificationID = Str.guid();
notificationCache[notificationID] = new Notification(title, {
body,
icon: String(icon),
data,
});
notificationCache[notificationID].onclick = () => {
onClick();
window.parent.focus();
window.focus();
focusApp();
notificationCache[notificationID].close();
};
notificationCache[notificationID].onclose = () => {
delete notificationCache[notificationID];
};
});
}

Expand All @@ -80,19 +74,21 @@ function push({title, body, delay = DEFAULT_DELAY, onClick = () => {}, tag = '',
export default {
/**
* Create a report comment notification
*
* @param usesIcon true if notification uses right circular icon
*/
pushReportCommentNotification({report, reportAction, onClick}: ReportCommentParams, usesIcon = false) {
let title: string | undefined;
let body: string | undefined;
pushReportCommentNotification(report: Report, reportAction: ReportAction, onClick: LocalNotificationClickHandler, usesIcon = false) {
let title;
let body;
const icon = usesIcon ? EXPENSIFY_ICON_URL : '';

const isChatRoom = ReportUtils.isChatRoom(report);

const {person, message} = reportAction;
const plainTextPerson = person?.map((f) => f.text).join();
const plainTextPerson = person?.map((f) => f.text).join() ?? '';

// Specifically target the comment part of the message
const plainTextMessage = message?.find((f) => f.type === 'COMMENT')?.text;
const plainTextMessage = message?.find((f) => f.type === 'COMMENT')?.text ?? '';

if (isChatRoom) {
const roomName = ReportUtils.getReportName(report);
Expand All @@ -103,36 +99,40 @@ export default {
body = plainTextMessage;
}

push({
title: title ?? '',
body,
delay: 0,
onClick,
icon: usesIcon ? EXPENSIFY_ICON_URL : '',
});
const data = {
reportID: report.reportID,
};

push(title, body, icon, data, onClick);
},

pushModifiedExpenseNotification({reportAction, onClick}: ReportCommentParams, usesIcon = false) {
push({
title: reportAction.person?.map((f) => f.text).join(', ') ?? '',
body: ReportUtils.getModifiedExpenseMessage(reportAction),
delay: 0,
onClick,
icon: usesIcon ? EXPENSIFY_ICON_URL : '',
});
pushModifiedExpenseNotification(report: Report, reportAction: ReportAction, onClick: LocalNotificationClickHandler, usesIcon = false) {
const title = reportAction.person?.map((f) => f.text).join(', ') ?? '';
const body = ReportUtils.getModifiedExpenseMessage(reportAction);
const icon = usesIcon ? EXPENSIFY_ICON_URL : '';
const data = {
reportID: report.reportID,
};
push(title, body, icon, data, onClick);
},

/**
* Create a notification to indicate that an update is available.
*/
pushUpdateAvailableNotification() {
push({
title: 'Update available',
body: 'A new version of this app is available!',
delay: 0,
onClick: () => {
AppUpdate.triggerUpdateAvailable();
},
push('Update available', 'A new version of this app is available!', '', {}, () => {
AppUpdate.triggerUpdateAvailable();
});
},

/**
* Clears all open notifications where shouldClearNotification returns true
*
* @param shouldClearNotification a function that receives notification.data and returns true/false if the notification should be cleared
*/
clearNotifications(shouldClearNotification: (notificationData: LocalNotificationData) => boolean) {
Object.values(notificationCache)
.filter((notification) => shouldClearNotification(notification.data))
.forEach((notification) => notification.close());
},
Comment on lines +133 to +137
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB. We don't need to supply a function as param. There is only one function, we can just keep passing the reportID

    clearNotifications(reportID: string) {
        Object.values(notificationCache)
            .filter((notification) => notification.data?.reportID === reportID)
            .forEach((notification) => notification.close());
    },

};
Loading