cis-oss/notify
2
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects 1 Releases 1 Wiki Activity

Feature/message sending #2

Merged
B00tLoad merged 28 commits from feature/message-sending into develop 2025-08-31 13:59:38 +02:00
Conversation 6 Commits 28 Files Changed 9 +3437 -112
B00tLoad commented 2025-08-30 21:38:40 +02:00 (Migrated from github.com)
Copy Link
No description provided.
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-08-30 21:39:17 +02:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment
Copy Link

Pull Request Overview

This PR introduces a complete message sending feature for the Pushover library, transitioning from type definitions to a fully functional implementation. The implementation provides a robust TypeScript client for the Pushover API with comprehensive message validation, error handling, and support for all Pushover features including emergency priorities and receipt management.

Key changes:

  • Complete rewrite of the core implementation with a new Pushover class that handles API communication
  • Comprehensive message validation using Zod schemas with support for all Pushover message types
  • Added basic test coverage for the exported functionality

Reviewed Changes

Copilot reviewed 6 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/index.ts Simplified to export the new Pushover class and types from the main implementation
src/Pushover.ts New comprehensive implementation with full API support, validation, and error handling
tests/index.spec.ts Basic test suite verifying the main export and class structure
package.json Added testing dependencies, build configurations, and package metadata
eslint.config.mjs Added eslint-plugin-only-warn for better development experience
.husky/pre-commit Updated to use pnpm instead of npx

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

## Pull Request Overview This PR introduces a complete message sending feature for the Pushover library, transitioning from type definitions to a fully functional implementation. The implementation provides a robust TypeScript client for the Pushover API with comprehensive message validation, error handling, and support for all Pushover features including emergency priorities and receipt management. Key changes: - Complete rewrite of the core implementation with a new `Pushover` class that handles API communication - Comprehensive message validation using Zod schemas with support for all Pushover message types - Added basic test coverage for the exported functionality ### Reviewed Changes Copilot reviewed 6 out of 9 changed files in this pull request and generated 3 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | src/index.ts | Simplified to export the new Pushover class and types from the main implementation | | src/Pushover.ts | New comprehensive implementation with full API support, validation, and error handling | | tests/index.spec.ts | Basic test suite verifying the main export and class structure | | package.json | Added testing dependencies, build configurations, and package metadata | | eslint.config.mjs | Added eslint-plugin-only-warn for better development experience | | .husky/pre-commit | Updated to use pnpm instead of npx | </details> --- <sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/cis-oss/pushover/new/main/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">learn how to get started</a>.</sub>
src/Pushover.ts
@@ -0,0 +1,718 @@
import https from "node:https";
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 21:39:16 +02:00
Copy Link

Missing closing parenthesis in the template literal. Should be ${data.length}): ${data}.

              `Received response body (length: ${data.length}): ${data}`,
Missing closing parenthesis in the template literal. Should be `${data.length}): ${data}`. ```suggestion `Received response body (length: ${data.length}): ${data}`, ```
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 21:39:17 +02:00
Copy Link

The nullish coalescing operator is unnecessary here since options.user is required in the ValidateOptions interface and cannot be undefined.

    params.append("user", options.user);
The nullish coalescing operator is unnecessary here since `options.user` is required in the `ValidateOptions` interface and cannot be undefined. ```suggestion params.append("user", options.user); ```
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 21:39:17 +02:00
Copy Link

This incorrectly resolves with a Promise instead of the resolved value. Should be Promise.all(promises).then(resolve).catch(reject) or use async/await pattern.

    if (options.recipients.length === 0) {
      throw new Error("No recipients specified.");
    }

    const {
      success,
      error,
      data: parsedMessage,
    } = MessageSchema.safeParse(message);

    if (!success) {
      throw new Error(`Message validation failed: ${error}`);
    }

    if (options.verbose) {
      console.log("Verbose mode enabled. Logging message and options:");
      console.log(parsedMessage);
      console.log(options);
      console.log("----------------------");
      console.log("Sending message...");
    }

    const promises = options.recipients.map((recipient) =>
      this.sendToSingleRecipient(
        parsedMessage,
        recipient,
        options.verbose ?? false,
      ),
    );

    return Promise.all(promises);
This incorrectly resolves with a Promise instead of the resolved value. Should be `Promise.all(promises).then(resolve).catch(reject)` or use `async/await` pattern. ```suggestion if (options.recipients.length === 0) { throw new Error("No recipients specified."); } const { success, error, data: parsedMessage, } = MessageSchema.safeParse(message); if (!success) { throw new Error(`Message validation failed: ${error}`); } if (options.verbose) { console.log("Verbose mode enabled. Logging message and options:"); console.log(parsedMessage); console.log(options); console.log("----------------------"); console.log("Sending message..."); } const promises = options.recipients.map((recipient) => this.sendToSingleRecipient( parsedMessage, recipient, options.verbose ?? false, ), ); return Promise.all(promises); ```
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-08-30 22:14:54 +02:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment
Copy Link

Pull Request Overview

This PR implements the core message sending functionality for a Pushover notification library. It replaces the existing type definitions with a complete implementation that includes message validation, API communication, and comprehensive TypeScript types.

  • Implements a full Pushover class with methods for sending messages, validating users, and managing emergency notifications
  • Adds comprehensive Zod schema validation for message payloads with proper error handling
  • Sets up Jest testing framework and initial test coverage for the exported functionality

Reviewed Changes

Copilot reviewed 6 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/index.ts Replaces type definitions with imports from the new Pushover implementation and exports the main class
src/Pushover.ts Implements the complete Pushover client class with full API functionality and TypeScript interfaces
tests/index.spec.ts Adds basic tests to verify the Pushover class is properly exported and has expected methods
package.json Adds Jest testing dependencies and configures build/test scripts
eslint.config.mjs Adds eslint-plugin-only-warn to convert errors to warnings
.husky/pre-commit Updates pre-commit hook to use pnpm instead of npx

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

## Pull Request Overview This PR implements the core message sending functionality for a Pushover notification library. It replaces the existing type definitions with a complete implementation that includes message validation, API communication, and comprehensive TypeScript types. - Implements a full Pushover class with methods for sending messages, validating users, and managing emergency notifications - Adds comprehensive Zod schema validation for message payloads with proper error handling - Sets up Jest testing framework and initial test coverage for the exported functionality ### Reviewed Changes Copilot reviewed 6 out of 9 changed files in this pull request and generated 4 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | src/index.ts | Replaces type definitions with imports from the new Pushover implementation and exports the main class | | src/Pushover.ts | Implements the complete Pushover client class with full API functionality and TypeScript interfaces | | tests/index.spec.ts | Adds basic tests to verify the Pushover class is properly exported and has expected methods | | package.json | Adds Jest testing dependencies and configures build/test scripts | | eslint.config.mjs | Adds eslint-plugin-only-warn to convert errors to warnings | | .husky/pre-commit | Updates pre-commit hook to use pnpm instead of npx | </details> --- <sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/cis-oss/pushover/new/main/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">learn how to get started</a>.</sub>
src/Pushover.ts
@@ -0,0 +1,718 @@
import https from "node:https";
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 22:14:53 +02:00
Copy Link

Calling resolve with a Promise will cause the method to return Promise<Promise<PushoverMessageResponse[]>> instead of the expected Promise<PushoverMessageResponse[]>. Use await Promise.all(promises) or restructure to avoid wrapping a Promise in another Promise.

    if (options.recipients.length === 0) {
      throw new Error("No recipients specified.");
    }

    const {
      success,
      error,
      data: parsedMessage,
    } = MessageSchema.safeParse(message);

    if (!success) {
      throw new Error(`Message validation failed: ${error}`);
    }

    if (options.verbose) {
      console.log("Verbose mode enabled. Logging message and options:");
      console.log(parsedMessage);
      console.log(options);
      console.log("----------------------");
      console.log("Sending message...");
    }

    const promises = options.recipients.map((recipient) =>
      this.sendToSingleRecipient(
        parsedMessage,
        recipient,
        options.verbose ?? false,
      ),
    );

    return Promise.all(promises);
Calling `resolve` with a Promise will cause the method to return `Promise<Promise<PushoverMessageResponse[]>>` instead of the expected `Promise<PushoverMessageResponse[]>`. Use `await Promise.all(promises)` or restructure to avoid wrapping a Promise in another Promise. ```suggestion if (options.recipients.length === 0) { throw new Error("No recipients specified."); } const { success, error, data: parsedMessage, } = MessageSchema.safeParse(message); if (!success) { throw new Error(`Message validation failed: ${error}`); } if (options.verbose) { console.log("Verbose mode enabled. Logging message and options:"); console.log(parsedMessage); console.log(options); console.log("----------------------"); console.log("Sending message..."); } const promises = options.recipients.map((recipient) => this.sendToSingleRecipient( parsedMessage, recipient, options.verbose ?? false, ), ); return Promise.all(promises); ```
src/Pushover.ts
@@ -0,0 +169,4 @@
* emergencyOpts: {
* retry: 60, // Retry every 60 seconds
* expire: 3600, // Expire after 1 hour
* tags: ["critical", "infra"]
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 22:14:54 +02:00
Copy Link

Missing comma after the tags array in the JSDoc example. Should be tags: [\"critical\", \"infra\"],

 *     tags: ["critical", "infra"],
Missing comma after the `tags` array in the JSDoc example. Should be `tags: [\"critical\", \"infra\"],` ```suggestion * tags: ["critical", "infra"], ```
src/Pushover.ts
@@ -0,0 +372,4 @@
): Promise<PushoverMessageResponse[]> {
return new Promise((resolve, reject) => {
if (options.recipients.length === 0) {
reject(new Error("No recipients specified."));
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 22:14:53 +02:00
Copy Link

The function continues executing after rejecting the promise when no recipients are specified. Add a return statement after the reject call to prevent further execution.

        reject(new Error("No recipients specified."));
        return;
The function continues executing after rejecting the promise when no recipients are specified. Add a `return` statement after the reject call to prevent further execution. ```suggestion reject(new Error("No recipients specified.")); return; ```
src/Pushover.ts
@@ -0,0 +438,4 @@
if (message.emergencyOpts.callback)
params.append("callback", message.emergencyOpts.callback);
if (message.emergencyOpts.tags)
params.append("tags", message.emergencyOpts.tags.join());
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 22:14:54 +02:00
Copy Link

The join() method is called without arguments, which will join array elements with commas. However, this may not be the intended format for the Pushover API. Consider specifying an explicit separator like join(',') or verify the expected format from the API documentation.

        params.append("tags", message.emergencyOpts.tags.join(','));
The `join()` method is called without arguments, which will join array elements with commas. However, this may not be the intended format for the Pushover API. Consider specifying an explicit separator like `join(',')` or verify the expected format from the API documentation. ```suggestion params.append("tags", message.emergencyOpts.tags.join(',')); ```
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-08-30 22:15:59 +02:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment
Copy Link

Pull Request Overview

This PR introduces a complete message sending feature for the Pushover notification service, replacing the previous type-only implementation with a fully functional API client. The main changes include implementing the core Pushover class with HTTP request handling, comprehensive message validation using Zod schemas, and support for all Pushover API features including emergency notifications, receipt management, and user validation.

Key changes:

  • Complete rewrite of the main entry point to export a functional Pushover class
  • Implementation of comprehensive Pushover API client with full feature support
  • Addition of basic test coverage for the exported functionality

Reviewed Changes

Copilot reviewed 6 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/index.ts Replaces type definitions with imports from new Pushover class implementation
src/Pushover.ts Implements complete Pushover API client with validation, sending, and management features
tests/index.spec.ts Adds basic test coverage for exported Pushover class
package.json Updates build configuration and adds testing dependencies
eslint.config.mjs Adds eslint-plugin-only-warn for development
.husky/pre-commit Updates pre-commit hook to use pnpm

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

## Pull Request Overview This PR introduces a complete message sending feature for the Pushover notification service, replacing the previous type-only implementation with a fully functional API client. The main changes include implementing the core Pushover class with HTTP request handling, comprehensive message validation using Zod schemas, and support for all Pushover API features including emergency notifications, receipt management, and user validation. Key changes: - Complete rewrite of the main entry point to export a functional Pushover class - Implementation of comprehensive Pushover API client with full feature support - Addition of basic test coverage for the exported functionality ### Reviewed Changes Copilot reviewed 6 out of 9 changed files in this pull request and generated 4 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | src/index.ts | Replaces type definitions with imports from new Pushover class implementation | | src/Pushover.ts | Implements complete Pushover API client with validation, sending, and management features | | tests/index.spec.ts | Adds basic test coverage for exported Pushover class | | package.json | Updates build configuration and adds testing dependencies | | eslint.config.mjs | Adds eslint-plugin-only-warn for development | | .husky/pre-commit | Updates pre-commit hook to use pnpm | </details> --- <sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/cis-oss/pushover/new/main/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">learn how to get started</a>.</sub>
src/Pushover.ts
@@ -0,0 +1,718 @@
import https from "node:https";
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 22:15:59 +02:00
Copy Link

This resolve call should use await or return the Promise.all directly. Currently, it's resolving with a Promise object instead of the actual results. Change to resolve(await Promise.all(promises)) or restructure to return the Promise.all directly.

    if (options.recipients.length === 0) {
      throw new Error("No recipients specified.");
    }

    const {
      success,
      error,
      data: parsedMessage,
    } = MessageSchema.safeParse(message);

    if (!success) {
      throw new Error(`Message validation failed: ${error}`);
    }

    if (options.verbose) {
      console.log("Verbose mode enabled. Logging message and options:");
      console.log(parsedMessage);
      console.log(options);
      console.log("----------------------");
      console.log("Sending message...");
    }

    const promises = options.recipients.map((recipient) =>
      this.sendToSingleRecipient(
        parsedMessage,
        recipient,
        options.verbose ?? false,
      ),
    );

    return Promise.all(promises);
This resolve call should use `await` or return the Promise.all directly. Currently, it's resolving with a Promise object instead of the actual results. Change to `resolve(await Promise.all(promises))` or restructure to return the Promise.all directly. ```suggestion if (options.recipients.length === 0) { throw new Error("No recipients specified."); } const { success, error, data: parsedMessage, } = MessageSchema.safeParse(message); if (!success) { throw new Error(`Message validation failed: ${error}`); } if (options.verbose) { console.log("Verbose mode enabled. Logging message and options:"); console.log(parsedMessage); console.log(options); console.log("----------------------"); console.log("Sending message..."); } const promises = options.recipients.map((recipient) => this.sendToSingleRecipient( parsedMessage, recipient, options.verbose ?? false, ), ); return Promise.all(promises); ```
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 22:15:59 +02:00
Copy Link

Missing comma after the array in the JSDoc example code block.

Missing comma after the array in the JSDoc example code block.
src/Pushover.ts
@@ -0,0 +372,4 @@
): Promise<PushoverMessageResponse[]> {
return new Promise((resolve, reject) => {
if (options.recipients.length === 0) {
reject(new Error("No recipients specified."));
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 22:15:59 +02:00
Copy Link

Missing return statement after the reject call. This allows code execution to continue, which could lead to unexpected behavior. Add return; after line 356.

        reject(new Error("No recipients specified."));
        return;
Missing `return` statement after the reject call. This allows code execution to continue, which could lead to unexpected behavior. Add `return;` after line 356. ```suggestion reject(new Error("No recipients specified.")); return; ```
src/Pushover.ts
@@ -0,0 +438,4 @@
if (message.emergencyOpts.callback)
params.append("callback", message.emergencyOpts.callback);
if (message.emergencyOpts.tags)
params.append("tags", message.emergencyOpts.tags.join());
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-08-30 22:15:58 +02:00
Copy Link

The join() method is called without a separator argument. This will use the default comma separator, but it's unclear if this is the intended format for the Pushover API. Consider explicitly specifying the separator as join(',') for clarity.

        params.append("tags", message.emergencyOpts.tags.join(","));
The `join()` method is called without a separator argument. This will use the default comma separator, but it's unclear if this is the intended format for the Pushover API. Consider explicitly specifying the separator as `join(',')` for clarity. ```suggestion params.append("tags", message.emergencyOpts.tags.join(",")); ```
jateute (Migrated from github.com) approved these changes 2025-08-30 22:22:56 +02:00
jateute (Migrated from github.com) left a comment
Copy Link

lgtm

lgtm
B00tLoad (Migrated from github.com) reviewed 2025-08-30 22:24:42 +02:00
src/Pushover.ts
@@ -0,0 +372,4 @@
): Promise<PushoverMessageResponse[]> {
return new Promise((resolve, reject) => {
if (options.recipients.length === 0) {
reject(new Error("No recipients specified."));
B00tLoad (Migrated from github.com) commented 2025-08-30 22:24:42 +02:00
Copy Link

fixed

fixed
jateute (Migrated from github.com) approved these changes 2025-08-31 10:19:30 +02:00
jateute (Migrated from github.com) left a comment
Copy Link

lgtm

lgtm
Sign in to join this conversation.
Reviewers
No Reviewers
copilot-pull-request-reviewer[bot]
B00tLoad
jateute
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No Label enhancement
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
B00tLoad jateute
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: cis-oss/notify#2
Delete Branch "feature/message-sending"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?