Page MenuHomePhabricator

[keyserver] Send updateBadgeCount notifs to macOS
ClosedPublic

Authored by michal on Mar 17 2023, 10:55 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 1, 12:51 AM
Unknown Object (File)
Fri, Sep 27, 8:09 AM
Unknown Object (File)
Sat, Sep 7, 9:54 PM
Unknown Object (File)
Sat, Sep 7, 9:54 PM
Unknown Object (File)
Sat, Sep 7, 9:54 PM
Unknown Object (File)
Sat, Sep 7, 9:54 PM
Unknown Object (File)
Sat, Sep 7, 9:54 PM
Unknown Object (File)
Sat, Sep 7, 9:54 PM
Subscribers

Details

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

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.

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

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!

Remove the remaining // only for flow lines

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.

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

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