- Notifications
You must be signed in to change notification settings - Fork1k
feat(site): add webpush notification serviceworker#17123
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
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| {enabled ?( | ||
| subscribed ?( | ||
| <Buttonvariant="outline"disabled={loading}onClick={unsubscribe}> |
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.
Is it just for experimental purposes? Dropping a button on NavBar may look inconsistent. If this is permanent, perhaps ask Bruno for his guidance.
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.
Yep, just for experimental. We'll need to better integrate this later.
81b8c6e to58cc953CompareThere 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
This PR adds webpush notification support by implementing a dedicated service worker and integrating related API and UI changes. Key changes include:
- Bundling and serving the service worker through Vite with proper proxy configuration
- Implementing the webpush notifications logic, including subscription management via a new React hook and API endpoints
- Updating tests to verify proper webpush payload handling and functionality
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| site/vite.config.mts | Configures service worker bundling and proxy endpoints |
| site/src/testHelpers/entities.ts | Updates mock build info to include a webpush public key |
| site/src/serviceWorker.ts | Implements service worker logic for push and notification click events |
| site/src/modules/dashboard/Navbar/NavbarView.tsx | Adds buttons to enable/disable webpush notifications |
| site/src/index.tsx | Registers the service worker for push notifications |
| site/src/contexts/useWebpushNotifications.ts | Implements a hook for managing webpush subscriptions |
| site/src/api/api.ts | Introduces API methods for creating and deleting webpush subscriptions |
| coderd/webpush/webpush_test.go | Updates tests with random notifications and payload assertions |
| coderd/webpush/webpush.go | Modifies the dispatcher to include a VAPID subscriber and logs errors with endpoint details |
| coderd/coderdtest/coderdtest.go | Updates test setup for the webpush dispatcher with the proper subscriber URL |
| cli/server.go | Integrates a check for HTTPS access URLs before initializing webpush notifications |
Comments suppressed due to low confidence (1)
coderd/webpush/webpush_test.go:239
- [nitpick] The parameters for assert.Equal are inverted. Swap the arguments so that the expected value comes first to improve error messages and consistency.
assert.Equal(t, r.Header.Get("content-encoding"), "aes128gcm")Uh oh!
There was an error while loading.Please reload this page.
e1f27a7 intomainUh oh!
There was an error while loading.Please reload this page.
/cherry-pick release/2.21 |
Uh oh!
There was an error while loading.Please reload this page.
Based on#17091
Original changes fromhttps://github.com/coder/coder/commits/kyle/tasks/
Notes: