Page MenuHomePhabricator
Differential D7094

[keyserver] Send updateBadgeCount notifs to macOS
ClosedPublic
Actions

Authored by michal on Mar 17 2023, 10:55 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 17, 6:47 PM
Unknown Object (File)
Sun, Nov 17, 2:33 PM
Unknown Object (File)
Fri, Nov 15, 6:53 AM
Unknown Object (File)
Wed, Nov 13, 4:57 PM
Unknown Object (File)
Wed, Nov 13, 11:19 AM
Unknown Object (File)
Wed, Nov 13, 11:01 AM
Unknown Object (File)
Wed, Nov 13, 12:29 AM
Unknown Object (File)
Thu, Nov 7, 10:46 AM
View All Files
Subscribers
ashoat
atul

Details

Reviewers
tomek
kamil
ashoat
Commits
rCOMM725b8786a3f1: [keyserver] Send updateBadgeCount notifs to macOS
Summary

ENG-3174

We want to send updateBadgeCount notifs to macOS. This works exactly the same as the iOS version, just replaced the ios with macos.

Test Plan
  • Open the desktop app
  • Send a message from another user -> desktop app gets a notification and macOS dock badge updates
  • Read the message on another device -> the dock badge disappears, in the logs we can check that the we got unreadBadgeCount notif

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

michal created this revision.Mar 17 2023, 10:55 AM
Herald added subscribers: atul, ashoat. · View Herald TranscriptMar 17 2023, 10:55 AM
michal added inline comments.Mar 17 2023, 10:56 AM
desktop/src/push-notifications.js
31–33 ↗(On Diff #23824)

This is no longer true when sending updateBadgeCount notifications. This will still show run on older clients, but it's just a console warning and only visible if you run the desktop app through a terminal.

Harbormaster completed remote builds in B17388: Diff 23824.Mar 17 2023, 11:11 AM
michal requested review of this revision.Mar 17 2023, 11:11 AM
ashoat accepted this revision.Mar 17 2023, 11:24 AM
ashoat added inline comments.
keyserver/src/push/send.js
1003 ↗(On Diff #23824)

Can we delete this?

1025 ↗(On Diff #23824)

Can we delete this?

This revision is now accepted and ready to land.Mar 17 2023, 11:24 AM
michal updated this revision to Diff 23838.Mar 20 2023, 5:43 AM

Remove unnecessary lines

Harbormaster completed remote builds in B17396: Diff 23838.Mar 20 2023, 5:58 AM
ashoat added a comment.Mar 20 2023, 2:37 PM

This diff seems to call getAPNsNotificationTopic with two parameters, but the version of that function in master only takes one parameter. Can you please update this diff's dependencies so I can see the rest of your stack? Please always set diff dependencies!

Separately, there are still two only for Flow lines in send.js – would be good to get rid of all of them!

michal updated this revision to Diff 23979.Mar 22 2023, 5:02 AM

Remove the remaining // only for flow lines

michal added a comment.Mar 22 2023, 5:08 AM

This diff seems to call getAPNsNotificationTopic with two parameters, but the version of that function in master only takes one parameter. Can you please update this diff's dependencies so I can see the rest of your stack? Please always set diff dependencies!

There's no stack for this diff, I'm calling getAPNsNotificationTopic with one argument, it's just an object with two fields.

Harbormaster completed remote builds in B17511: Diff 23979.Mar 22 2023, 5:17 AM
ashoat added a comment.Mar 22 2023, 2:08 PM
In D7094#211792, @michal wrote:

This diff seems to call getAPNsNotificationTopic with two parameters, but the version of that function in master only takes one parameter. Can you please update this diff's dependencies so I can see the rest of your stack? Please always set diff dependencies!

There's no stack for this diff, I'm calling getAPNsNotificationTopic with one argument, it's just an object with two fields.

Oops!! Sorry for missing this and getting myself confused

michal updated this revision to Diff 24014.Mar 23 2023, 2:43 AM

Actually remove the remaining "only for flow" lines, not sure why this didn't commit before...

Harbormaster completed remote builds in B17542: Diff 24014.Mar 23 2023, 2:58 AM
Closed by commit rCOMM725b8786a3f1: [keyserver] Send updateBadgeCount notifs to macOS (authored by Michal Gniadek <michal.gniadek@swmansion.com>). · Explain WhyMar 23 2023, 3:01 AM
This revision was automatically updated to reflect the committed changes.
Michal Gniadek <michal.gniadek@swmansion.com> added a commit: rCOMM725b8786a3f1: [keyserver] Send updateBadgeCount notifs to macOS.

Revision Contents
Changeset List

PathSize
desktop/
src/
3 lines
keyserver/
src/
push/
33 lines

Diff 24015

View Options

desktop/src/push-notifications.js

Loading...
View Options

keyserver/src/push/send.js

Loading...