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.

Sign up for GitHub

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

Jump to bottom

feat: Add the ability to manage channels and send broadcasts #164

Open
wants to merge 60 commits into
base: master
Choose a base branch
from
Open

feat: Add the ability to manage channels and send broadcasts #164

wants to merge 60 commits into from

Conversation

cbaker6
Copy link

@cbaker6 cbaker6 commented Dec 30, 2024 •
edited
Loading

Issue

Continuation of #163 which allowed the additional properties needed for broadcast notifications, but doesn't modify the Client API to GET, POST, DELETE to the respective Apple paths.

Closes: #162
Closes: #146
Closes: #111

Approach

Follow the documentation related to channel management and broadcasting. This enables the functionality described in Apple's WWDC 2024 videos: Broadcast Updates to Your Live Activities.

The newly added API methods to node-apn will have the following footprint (TypeScript):

export class Provider extends EventEmitter {
  /**
   * Manage channels using a specific action.
   *
   * @param notifications - A Notification or an Array of Notifications to send. Each notification should specify the respective channelId it's directed to.
   * @param bundleId - The bundleId for your application.
   * @param action - Specifies the action to perform on the channel(s).
   */
  manageChannels(notifications: Notification|Notification[], bundleId: string, action: ChannelAction): Promise<Responses<BroadcastResponse,BroadcastResponseFailure>>;

  /**
   * Broadcast notificaitons to channel(s).
   *
   * @param notifications - A Notification or an Array of Notifications to send. Each notification should specify the respective channelId it's directed to.
   * @param bundleId: The bundleId for your application.
   */
  broadcast(notifications: Notification|Notification[], bundleId: string): Promise<Responses<BroadcastResponse,BroadcastResponseFailure>>;
}

export type ChannelAction = 'create' | 'read' | 'readAll' | 'delete';

export interface ResponseSent {
  device: string;
}

export interface BroadcastResponse {
  bundleId: string;
  "apns-request-id"?: string;
  "apns-channel-id"?: string;
  "message-storage-policy"?: number;
  "push-type"?: string;
  "channels"?: string[];
}

export interface LoggerResponse extends Partial<ResponseSent>, Partial<BroadcastResponse> {}

export interface ResponseFailure {
  device: string;
  error?: Error;
  status?: number;
  response?: {
    reason: string;
    timestamp?: string;
  };
}

export interface BroadcastResponseFailure extends Omit<ResponseFailure, "device"> {
  bundleId: string;
}

export interface LoggerResponseFailure extends Partial<ResponseFailure>, Partial<BroadcastResponseFailure> {}

export interface Responses<R,F> {
  sent: R[];
  failed: F[];
}

Non-Breaking Changes

Changes are currently non-breaking as they are made to the client API, which isn't exposed to developers, adding new methods that weren't available. The typescript changes are improvements but are non-breaking as well.

I'll update this if a breaking change is required.

@parse-github-assistant Parse GitHub Assistant
Copy link

parse-github-assistant bot commented Dec 30, 2024 •
edited
Loading

Thanks for opening this pull request!

@cbaker6 cbaker6 marked this pull request as draft December 30, 2024 01:53
@cbaker6 cbaker6 changed the title feat: Add the ability to use different http methods feat: Add the ability to setup and send broadcast notifications Dec 31, 2024
@cbaker6 cbaker6 changed the title feat: Add the ability to setup and send broadcast notifications feat: Add the ability to manage and send broadcast notifications Dec 31, 2024
@cbaker6 cbaker6 closed this Dec 31, 2024
@cbaker6 cbaker6 reopened this Dec 31, 2024
@codecov Codecov
Copy link

codecov bot commented Jan 1, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 92.78351% with 21 lines in your changes missing coverage. Please review.

Project coverage is 95.24%. Comparing base (4eb3686) to head (3c05075).
Report is 16 commits behind head on master.

Files with missing lines Patch % Lines
lib/client.js 89.06% 21 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #164      +/-   ##
==========================================
+ Coverage   93.53%   95.24%   +1.70%     
==========================================
  Files          23       23              
  Lines         588      820     +232     
==========================================
+ Hits          550      781     +231     
- Misses         38       39       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cbaker6
Copy link
Author

cbaker6 commented Jan 2, 2025

@mtrezza and @dplewis when you have a chance, can you do an initial review of this PR? To enable broadcasting, a larger amount of code changes are needed than normal, in particular because client.js hasn't been updated in 3+ years, was designed for original style push notifications (can only POST to a single path), and was based purely on promise style as opposed to async/await.

I have slowly integrated the changes needed to the client to support broadcasting and hopefully future proof it so changes can be added more easily with less code. The test cases have been updated with the async/await code, but all of the original tests still ensure their expected results.

I still have test cases to add, but I don't expect much changes to client.js and provider.js with the exception of the new test cases catching bugs I introduced. I'll also add a self review to explain some of my thinking with certain changes.

To help guide you through the code changes (again, my apologies for the large PR), I recommend the following:

  1. Go through the description of the PR update here: feat: Add the ability to manage channels and send broadcasts #164 (comment)
  2. Review changes in lib/config.js
  3. Review tests added to test/config.js
  4. Review changes in index.d.ts
  5. Review changes in lib/provider.js
  6. Review changes in test/provider.js (I need to add more test methods here)
  7. Review changes in lib/proxy.js
  8. Review changes in lib/client.js (largest changes, needed restricting, but I'm not planning to make more changes here unless you have suggestions)
  9. Review changes in test/client.js (also a large amount of updates, but original tests to report original results. I still have to add more tests here)
  10. The multiclient stuff, but those behave much like the original client

Open to any feedback/suggestions...

@cbaker6 cbaker6 changed the title feat: Add the ability to manage and send broadcast notifications feat: Add the ability to manage channels and send broadcasts Jan 4, 2025
cbaker6 added 28 commits January 4, 2025 16:21
@cbaker6
Copy link
Author

cbaker6 commented Jan 14, 2025

@mtrezza the code for this is finished and ready for a full review.

I still need to update some Markdown docs for usage of the new methods before merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtrezza mtrezza mtrezza left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@cbaker6 @mtrezza