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

Create generalized API for requesting periodic push notifications #2527

Open
taoeffect opened this issue Jan 20, 2025 · 0 comments · May be fixed by #2546
Open

Create generalized API for requesting periodic push notifications #2527

taoeffect opened this issue Jan 20, 2025 · 0 comments · May be fixed by #2546
Assignees
Labels
App:Backend App:Frontend Kind:Core Anything that changes or affects the fundamental core data structures & design of the application. Kind:Enhancement Improvements, new features, performance upgrades, etc. Priority:High

Comments

@taoeffect
Copy link
Member

taoeffect commented Jan 20, 2025

Problem

This code needs to only run for requested push subscriptions:

// Recurring task to send messages to push clients (for periodic notifications)
setInterval(() => {
  const pubsub = sbp('okTurtles.data/get', PUBSUB_INSTANCE)
  // Notification text
  const notification = JSON.stringify({ type: 'recurring' })
  // Find push subscriptions that do _not_ have a WS open. This means clients
  // that are 'asleep' and that might be woken up by the push event
  Object.values(pubsub.pushSubscriptions || {})
    .filter((pushSubscription: Object) => pushSubscription.sockets.size === 0)
    .forEach((pushSubscription: Object) => {
      postEvent(pushSubscription, notification).catch((e) => {
        console.warn(e, 'Error sending recurring message to web push client', pushSubscription.id)
      })
    })
// Repeat every 12 hours
}, 12 * 60 * 60 * 1000)

Solution

Implement it in a generic way so that apps can specify when registering push notifications that the server periodically send these to them.

Note: there should be a minimum request period that you can't go below, e.g. no app should be able to request periodic notifications more frequently than say every 5 minutes. In other words, the server should enforce this limit, and return an error to clients that want more frequent notifications. The error message should include the server's configured minimum time, so that if some server say wants it to be every 30 minutes, apps can figure out and adjust to this new minimum time as needed. There could also be a new RESTful GET API (e.g. /config) that returns the server's configuration values such as minimum notification time. Alternatively, the server can do Math.max(requestedTime, SERVER_MIN_PERIODIC_TIME), with some sort of warning returned (this way is probably simpler and better..).

@taoeffect taoeffect added App:Backend App:Frontend Kind:Core Anything that changes or affects the fundamental core data structures & design of the application. Kind:Enhancement Improvements, new features, performance upgrades, etc. Priority:High labels Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App:Backend App:Frontend Kind:Core Anything that changes or affects the fundamental core data structures & design of the application. Kind:Enhancement Improvements, new features, performance upgrades, etc. Priority:High
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants