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

feat: add push notifications #1704

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

aarushtools
Copy link
Member

Proposed changes

  • add the ability to send push notifications via webpush
  • tests for notifications and pages to view notification logs and send a custom one
  • adds user push notification preferences and implements sending push notifications on announcements, absences, glance, etc. (eighth waitlist push notification option is implemented in the preferences model but unused)

Brief description of rationale

Many of Ion's current notification features rely on sending emails but people check push notifications more often. This makes the PWA experience more intuitive especially on mobile

Notes

  • In production, the VAPID keys (generated via create_vapid_keys.py) should be generated once and never changed, otherwise all push subscriptions will be invalidated and users will have to manually subscribe to push notifications again

Video of it working on windows

Video of it working on iOS

@aarushtools aarushtools force-pushed the webpush branch 6 times, most recently from 986c2f5 to 19100ed Compare July 30, 2024 21:08
@alanzhu0
Copy link
Member

This is great!!! Are you all done or still working on stuff?
One useful notification option to add is when your bus arrives you get sent a notif, and also maybe some text description of where it's parked (e.g. "in front of the curb," "right front row", "left back row"). Maybe those notifs have to be delayed so they aren't sent until the end of the day (4 pm most days, but dynamically calculated for early release). And also maybe a notif that sends at 4 pm "your bus isn't here yet" if it hasn't arrived.

great work!!

@aarushtools
Copy link
Member Author

aarushtools commented Jul 30, 2024

Everything is done, I'll try adding that feature 👍

@aarushtools aarushtools force-pushed the webpush branch 2 times, most recently from 51736f7 to 27ee277 Compare July 31, 2024 01:43
@aarushtools
Copy link
Member Author

I added it, here's what it looks like:
image
and the bus spot id -> string translations:
image

@coveralls
Copy link

coveralls commented Jul 31, 2024

Coverage Status

coverage: 79.119% (-0.2%) from 79.361%
when pulling 9ca0850 on aarushtools:webpush
into e5e51f2 on tjcsl:dev.

@aarushtools aarushtools marked this pull request as ready for review July 31, 2024 02:41
@aarushtools aarushtools requested a review from a team as a code owner July 31, 2024 02:41
@aarushtools aarushtools force-pushed the webpush branch 3 times, most recently from 73f2fd1 to fa19346 Compare August 8, 2024 00:12
@aarushtools aarushtools force-pushed the webpush branch 9 times, most recently from cc9af14 to 6129d74 Compare August 19, 2024 21:52
@aarushtools aarushtools force-pushed the webpush branch 2 times, most recently from 7df9991 to f212a96 Compare August 26, 2024 21:29
@aarushtools aarushtools force-pushed the webpush branch 6 times, most recently from f219e26 to 9ca0850 Compare August 27, 2024 21:45
@alanzhu0
Copy link
Member

alanzhu0 commented Aug 27, 2024

  • On mobile, on preferences the subscribe page looks too transparent, can you decrease the opacity there?
  • It doesn't work on Edge, right? I can subscribe now actually, but I don't get notified. If that's right, please include a note to the user about that, and also a decent number of people use Edge so getting support for it would be good.
  • There's a notification sound on mobile - can you turn off the sound? I don't think people will like that.
  • Can you make Django admin interfaces for the webpush-related models?
  • Does the bus notification only check at 4 pm right now? Can you do something where if the bus hasn't arrived at 4 pm, send the bus delayed notification you're already doing, then when the bus does arrive send another notification saying so?

@aarushtools aarushtools force-pushed the webpush branch 2 times, most recently from 848cc66 to c134c08 Compare August 28, 2024 23:03
@aarushtools
Copy link
Member Author

@alanzhu0 everythings done 👍

@aarushtools
Copy link
Member Author

Idk why it's failing I ran build_docs like 20 times

@aarushtools
Copy link
Member Author

aarushtools commented Aug 30, 2024

to test:

  • APP: Announcements
    • announcement posted sends a push notification when notify option is enabled
      • test if content is truncated when its too long
  • APP: Bus
    • push notification sent at dismissal time indicating where your bus is
      • works with different day end times
      • test all possible bus location messages
    • push notification sent at dismissal time indicating if your bus is delayed or not
      • works with different day end times
    • push notification sent when delayed bus arrives
      • works with different day end times
    • push notification sent to users with bus mentioned in bus announcement
      • test different counties
  • APP: Eighth
    • push notification sent to student when an activity they signed up for is cancelled
    • push notification sent to student when they get an absence
      • cron
      • admin command
    • push reminder notification to sign up 40 mins prior to blocks locking.
      • works with different signup lock times
    • test glance notification
      • works with multiple blocks (2+)
  • APP: Polls
    • test notification sent when a poll is created (and the notify option is selected)
  • APP: Notifications
    • test post notification
      • content is truncated?
      • works on individual user(s)
      • works on group(s)
      • works on everyone
    • test notification logs
    • test rescheduling of notifications
  • Other:
    • Docker:
      • test vapid keys generated on containers creation
    • JS:
      • test silent notifications option, per device
      • test with different browsers to see if it can detect supported/unsupported accurately
        • ios 16.4+ (safari)
        • chrome
        • opera
        • macos 16+ (safari)
        • firefox
        • edge (confirm that it says the browser is unsupported)

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.

3 participants