feat: add push notifications #2369

iamacook · 2023-08-08T14:03:54Z

What it solves

Implements push notifications for transaction events.

How this PR fixes it

New "Notifications" settings sections have been added per-Safes/all added Safes, as well as a feature banner and associated tracking has been added. Subscribed categories dispatch push notifccations when the Safe UI is either out of focus or closed (but the browser is open).

The per-Safe "Notifications" section allows for notification subscription to all notification types, requiring a confirmatory signature as "Confirmation requests" are included. The global section is similar, subscribing to user selected (added) Safes with n number of signatures per selected chain.

Notifications are grouped into three categories: "Incoming transactions", "Outgoing transactions" and "Confirmation requests" according to our Transaction Service webhooks.

Incoming transactions
- INCOMING_ETHER
- INCOMING_TOKEN
Outgoing transactions
- MODULE_TRANSACTION
- EXECUTED_MULTISIG_TRANSACTION
Confirmation requests
- CONFIRMATION_REQUEST

A banner is shown once per chain that has added Safes, subscribing to notifications for all added Safes on that chain.

User interaction within the UI is tracked directly. ~~Due to technical limitations, direct notification interaction events (opening/closing) are cached and tracked when the UI is again opened.~~ Moved to a separate PR #2500.

How to test it

⚠️ Due to the complexity of this feature, I would advise explaining the feature on a call and maybe even testing it together. ⚠️

Registering for notifications should succeed, showing a notification and then displayind them accordingly when the Safe UI is not in focus or is closed (but the browser is open).
Unregistering a Safe should only unregister itself if there are others subscribed to on that chain or the entire device if it is the last Safe subscribed to on that chain.
Tracking of user interaction should happen immediately.
~~Tacking of notification iteraction should happen when the UI is reopened.~~ (Moved to a separate PR feat: add push notification tracking #2500).
The MacOS info banner should only show on Mac devices.

If you want to reset your current registration, you need to:

Reset browser notification permissions
Unregister the sw.js service worker
Delete all firebase-*/notification-* IndexedDB databases

If you want to show the subscription banner again, you need to delete the SAFE_v2__dismissPushNotifications localStorage key.

Screenshots

Please note that these are slightly dated but just demonstrate the notification. See the designs.

Checklist

I've tested the branch on mobile 📱
I've documented how it affects the analytics (if at all) 📊
I've written a unit/e2e test for it (if applicable) 🧑‍💻

github-actions · 2023-08-08T14:04:11Z

Branch preview

✅ Deploy successful!

https://firebase--walletweb.review-wallet-web.5afe.dev

github-actions · 2023-08-08T14:06:45Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

This reverts commit 2654d76.

github-actions · 2023-08-17T07:23:05Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

package.json

src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx

usame-algan · 2023-09-08T12:48:11Z

src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx

+    })
+  }, [dismissedBannerPerChain, safe.chainId, setDismissedBannerPerChain])
+
+  // Click outside to dismiss banner


nit: I would be in favour of having an explicit "x" Button since pressing outside the banner can happen by accident.

Good point. We're using the same style as the batch banner atm. @liliiaorlenko, what are your thoughts?

As per discussion, I've added a close button to the banner which also negates the above comment regarding the tracking.

src/components/settings/PushNotifications/index.tsx

src/components/settings/PushNotifications/hooks/useNotificationPreferences.ts

src/components/settings/PushNotifications/PushNotificationsBanner/styles.module.css

src/components/settings/PushNotifications/logic.ts

src/hooks/useDecodeTx.ts

usame-algan

Works great and thanks for implementing the changes 👍

Will discuss the notification dismissal tomorrow but it is non-blocking imo. LGTM 🚀

* feat: add push notification tracking * fix: address review comments * fix: error message

liliya-soroka · 2023-09-18T10:29:29Z

Feedback: A notification banner is displayed for the safe with enabled notifications through Global settings. It's confusing, as I just enabled and signed notifications enabling for this safe

Steps: 1. make sure that you have a few safes in the app on different networks 2. run app 3. Select Enable Notifications using Global settings 4. enable notifications for all your safes 5. come back to the safes list and switch to the safe on another network Current result: Notifications banner is displayed

github-actions · 2023-09-25T07:37:00Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

francovenica · 2023-09-25T15:14:40Z

Looks good.
The ticket was checked by several people, in windows and mac for the different type of notifications.
Checked the notification for incomming tokens, outgoing tx (which is not just tokens, is the execution of any tx) and the confirmation requests.
All the issues reported in the QA document were fixed or discussed: https://www.notion.so/safe-global/QA-Feedback-for-notifications-epic-2391-557125a8a91240a59903655e0cfc3bd3

A few notes for future iterations that we might want to revisit:
You get a notification for every tx executed in a "bulk transaction execution".
The "Confirm request" only works for safes with at least 3 owners and a policy of at least 2. I assume this is cuz for a threshold of 1 you don't need to confirm anymore, and if the safe has 2 owners, the tx 2nd confirmation needed is a "sign and execute" which I believe is different from a "just confirm" type of signing.

feat: implement Firebase Cloud Messaging

92106a8

iamacook requested review from usame-algan and kirkkonen August 8, 2023 14:03

iamacook self-assigned this Aug 8, 2023

iamacook marked this pull request as draft August 8, 2023 14:32

iamacook added 10 commits August 8, 2023 18:41

fix: move to custom next-pwa worker

2654d76

Revert "fix: move to custom next-pwa worker"

d3b8624

This reverts commit 2654d76.

fix: remove next-pwa

6fded4a

fix: convert SW to TS + add notification text

ae952c5

fix: improve notifications

2168cdf

refactor: separate types + add test

70ee4ca

fix: improve UI + add partial tests

771ba43

fix: add more test coverage

bba709a

fix: list Safes

94918a1

Merge branch 'dev' into firebase

d363062

iamacook linked an issue Aug 17, 2023 that may be closed by this pull request

[Epic] Push notification #2391

Closed

iamacook added 3 commits August 22, 2023 12:41

Merge branch 'dev' into firebase

77f90df

fix: migrate to SDK

1a48177

feat: basic notification preferences

819e652

iamacook changed the title ~~[PoC] feat: implement Firebase Cloud Messaging~~ feat: implement Firebase Cloud Messaging Aug 22, 2023

iamacook added 7 commits August 22, 2023 13:44

fix: tests

7a4e788

fix: implement designs

ef23f7c

feat: per-Safe preferences

1ded885

refactor: move registrations to IndexedDB

ea9a72a

fix: adjust style, add banner + registration list

b71acd5

fix: split databases + finish design

12d9c1d

fix: database reactivity with ExternalStore

c770a39

fix: address review comments

a115c14

katspaugh approved these changes Sep 8, 2023

View reviewed changes

usame-algan reviewed Sep 8, 2023

View reviewed changes

iamacook added 3 commits September 14, 2023 12:28

fix: address comments

12116b7

fix: regenerate lockfile

93b614e

fix: address comments

cc5fa66

usame-algan approved these changes Sep 14, 2023

View reviewed changes

iamacook added 2 commits September 18, 2023 09:14

feat: add push notification tracking (#2500)

a8ed67a

* feat: add push notification tracking * fix: address review comments * fix: error message

feat: add close button to banner

f7836a2

iamacook added 8 commits September 18, 2023 18:16

fix: remove unnecessary types + qa findings

28517d7

fix: texts, confirmation registration + enable all

8cea4ed

fix: mock

944bad8

fix: spacing

124827d

fix: spacing

e0bd7b8

Merge branch 'firebase' of github.com:safe-global/web-core into firebase

482a015

fix: use EIP-191

e9e94c0

Merge remote-tracking branch 'origin/dev' into firebase

19f4219

iamacook added 6 commits September 25, 2023 09:42

fix: test

75b6177

feat: add feature flag

21bdb3d

fix: test

8c2ae58

Merge branch 'dev' into firebase

8c381b2

fix: add space + only show banner on added Safes

e18c16b

fix: remove space

1cb4116

fix: e2e

7b85e74

iamacook merged commit 301facc into dev Sep 25, 2023
9 checks passed

iamacook deleted the firebase branch September 25, 2023 16:28

github-actions bot locked and limited conversation to collaborators Sep 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add push notifications #2369

feat: add push notifications #2369

iamacook commented Aug 8, 2023 •

edited

Loading

github-actions bot commented Aug 8, 2023 •

edited

Loading

github-actions bot commented Aug 8, 2023 •

edited

Loading

github-actions bot commented Aug 17, 2023 •

edited

Loading

usame-algan Sep 8, 2023

iamacook Sep 14, 2023

iamacook Sep 18, 2023

usame-algan left a comment

liliya-soroka commented Sep 18, 2023 •

edited by iamacook

Loading

github-actions bot commented Sep 25, 2023 •

edited

Loading

francovenica commented Sep 25, 2023 •

edited

Loading

feat: add push notifications #2369

feat: add push notifications #2369

Conversation

iamacook commented Aug 8, 2023 • edited Loading

What it solves

How this PR fixes it

How to test it

Screenshots

Checklist

github-actions bot commented Aug 8, 2023 • edited Loading

Branch preview

github-actions bot commented Aug 8, 2023 • edited Loading

ESLint Summary View Full Report

github-actions bot commented Aug 17, 2023 • edited Loading

ESLint Summary View Full Report

usame-algan Sep 8, 2023

Choose a reason for hiding this comment

iamacook Sep 14, 2023

Choose a reason for hiding this comment

iamacook Sep 18, 2023

Choose a reason for hiding this comment

usame-algan left a comment

Choose a reason for hiding this comment

liliya-soroka commented Sep 18, 2023 • edited by iamacook Loading

github-actions bot commented Sep 25, 2023 • edited Loading

ESLint Summary View Full Report

francovenica commented Sep 25, 2023 • edited Loading

iamacook commented Aug 8, 2023 •

edited

Loading

github-actions bot commented Aug 8, 2023 •

edited

Loading

github-actions bot commented Aug 8, 2023 •

edited

Loading

github-actions bot commented Aug 17, 2023 •

edited

Loading

liliya-soroka commented Sep 18, 2023 •

edited by iamacook

Loading

github-actions bot commented Sep 25, 2023 •

edited

Loading

francovenica commented Sep 25, 2023 •

edited

Loading