Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push notifications #1418

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft

Push notifications #1418

wants to merge 15 commits into from

Conversation

jellybob
Copy link
Contributor

@jellybob jellybob commented Apr 1, 2024

Work in progress PR, this handles the absolute basics of registering for push notifications, and being able to send a notification to an endpoint.

Things still to do:

  • Provide a reasonable UI
  • Test on multiple platforms (so far only Safari on macOS has been tested)
  • Job to send notifications on upcoming favourited content
  • Job to notify of upcoming volunteer shifts
  • Better handling of service worker updates
  • Being able to open a specific page, and customisable title/body/actions
  • Write some tests for send_queued_notifications and deliver_notification
  • Handle replacement of notification endpoints

@wlcx
Copy link
Member

wlcx commented Apr 3, 2024

Haven't looked through super closely, but looks great and could also fit really well alongside APNS push notifications for apple wallet tickets...

@jellybob jellybob force-pushed the push-notifications branch from e582fb2 to 10df98f Compare April 15, 2024 19:51
@jellybob
Copy link
Contributor Author

This is probably in a reasonable state for testing now. No notifications are being queued other than those requested with the test button, but I think there's some value in wider testing of that basic functionality given how finicky WebPush has been for me so far.

You'll need to set WEBPUSH_PUBLIC_KEY and WEBPUSH_PRIVATE_KEY. You can generate a key pair with npx web-push generate-vapid-keys.

@jellybob jellybob force-pushed the push-notifications branch from 4d67261 to dfdc6c1 Compare April 21, 2024 20:52


@scheduled_task(minutes=15)
def queue_content_notifications(time=None) -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deeply dislike the approach being taken here, if anyone with sufficient SQLAlchemy knowledge wants to turn it into a single query with some joins I'd be very happy.

@jellybob jellybob force-pushed the push-notifications branch from c9c21d5 to 77d4d55 Compare April 29, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants