- Notifications
You must be signed in to change notification settings - Fork5
feat: send push notifications on VPN failures#196
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
3172ac2 to4a6433aCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Adds user-facing push notifications when the VPN or URI handler fails, and refactors notification setup and handling into the AppDelegate.
- Bumps the
fluid-menu-bar-extradependency revision. - Sends a
.vpnFailurenotification on VPN state failures. - Refactors
UNUserNotificationCenterdelegate logic intoAppDelegate, registers categories, and updatessendNotification.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Coder-Desktop/project.yml | Updatedfluid-menu-bar-extra revision |
| Coder-Desktop/Coder-Desktop/VPN/VPNService.swift | Send push notification on VPN failures |
| Coder-Desktop/Coder-Desktop/Notifications.swift | Moved delegate intoAppDelegate, added categories andNotificationCategory enum, updatedsendNotification signature |
| Coder-Desktop/Coder-Desktop/Coder_DesktopApp.swift | Call new category registration, set delegate toself, update URI failure call |
| @@ -1,8 +1,23 @@ | |||
| import UserNotifications | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The oldNotifDelegate class was removed but I don’t see its full definition cleaned up. Please remove any leftoverNotifDelegate declaration to avoid dead code.
Copilot uses AI. Check for mistakes.
| nonisolatedfunc userNotificationCenter( | ||
| _:UNUserNotificationCenter, | ||
| willPresent _:UNNotification | ||
| )async->UNNotificationPresentationOptions{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ensure theuserNotificationCenter(_:willPresent:withCompletionHandler:) method is inside theAppDelegate extension so banners still appear while the app is active.
Copilot uses AI. Check for mistakes.
Might come back to this later. |
Uh oh!
There was an error while loading.Please reload this page.
Relates to#195.
When the macOS VPN unexpectedly disconnects, Coder Connect currently just displays a red error message:
Now, when this happens, we'll additionally send a push notification with the same message:
The above error message was produced by force quitting the system extension.
Clicking these VPN failure notifications also opens the tray:
notifdemo.mov
Oncecoder/coder#18602 is closed, we can also send push notifications when Coder Connect disconnects / attempts to dial the Coder deployment. We currently do not support being notified of these events.