Conversation
When --channel-name is provided without a direct recipient, the push notification is published to the channel with the payload wrapped in extras.push, routing it to push-subscribed devices via the channel. If a direct recipient (--device-id, --client-id, --recipient) is also present, --channel-name is ignored with a warning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe changes add support for publishing push notifications via a channel in addition to device/client recipients. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI Command
participant Validation
participant Publisher
participant PushAdmin as Push Admin API
participant Channels as Channels API
participant Output
User->>CLI: ably push publish --channel-name=my-channel
CLI->>Validation: Validate flags<br/>(has channel-name OR recipient?)
Validation-->>CLI: ✓ Valid
CLI->>Publisher: Route to publisher<br/>(hasDirectRecipient?)
alt Channel-based Publishing
Publisher->>Channels: Get channel instance
Channels->>Channels: publish({<br/>extras: { push: payload }<br/>})
Channels-->>Publisher: ✓ Published
Publisher->>Output: Format response<br/>({ published: true, channel })
else Recipient-based Publishing (if recipient provided)
Publisher->>PushAdmin: publish(recipient, payload)
PushAdmin-->>Publisher: ✓ Published
Publisher->>Output: Format response<br/>(success message)
end
Output-->>User: Display result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
maratal
changed the title
channel-name flag to push publish
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/push/publish.ts`:
- Line 236: The chained call rest.channels.get(channelName).publish({ extras: {
push: payload } }) is too long for Prettier; split the expression across
multiple lines to satisfy line-length rules. Specifically, break after getting
the channel (rest.channels.get(channelName)) and place the .publish( call on the
next line, with the object literal formatted over multiple lines (e.g., open
.publish( on its own line, then place extras and push on separate indented
lines) and close the call on a new line. Keep the same identifiers (rest,
channels, get, channelName, publish, payload) and behavior unchanged.
- Around line 99-105: The warning emitted when hasDirectRecipient and
flags["channel-name"] is true uses this.log(formatWarning(...)) and will corrupt
--json output; wrap that this.log call in a guard that checks
this.shouldOutputJson(flags) (i.e., only call this.log when
!this.shouldOutputJson(flags)) so the formatWarning message is emitted only for
human-readable runs; update the block around the this.log/formatWarning
invocation in publish (where hasDirectRecipient and flags["channel-name"] are
checked) to use the conditional before logging.
In `@test/unit/commands/push/publish.test.ts`:
- Around line 148-157: The test currently JSON.parses stdout directly which can
fail due to Prettier/whitespace formatting; update the test to trim the CLI
output before parsing by calling JSON.parse(stdout.trim()) so it tolerates
trailing newlines/formatting. Locate the test using runCommand in
publish.test.ts (the it block "should output JSON with channel when publishing
via channel") and change JSON.parse(stdout) to JSON.parse(stdout.trim()) to make
the assertion robust.
- Around line 173-183: Formatting in the "should handle channel publish errors"
test is inconsistent with project Prettier rules; reformat the test in
test/unit/commands/push/publish.test.ts (the it(...) block) so it matches the
code style: proper indentation, spacing around arrow functions, consistent
semicolons, and no stray blank/merged lines — specifically fix the block that
uses getMockAblyRest(),
mock.channels._getChannel("err-channel").publish.mockRejectedValue(new
Error("Channel error")), and the await runCommand([...], import.meta.url) call
so Prettier passes; you can run Prettier or your project's formatter to apply
the exact corrections.
- Around line 105-134: The tests "should publish via channel wrapping payload in
extras.push" and "should ignore --channel-name when --device-id is also
provided" have long single-line argument arrays and matcher chains that fail
Prettier; reformat the runCommand calls and expect(...) assertions into
multiline style so each argument or chained matcher is on its own line (e.g.,
break the runCommand argument array across multiple lines and break the
expect.objectContaining/expect.anything matcher chains into multiple indented
lines), keeping the same values and assertions for getMockAblyRest(),
runCommand(...), channel.publish expectations, and mock.push.admin.publish
checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e84d3f9a-95cf-4a66-bd46-ad5042445fcd
📒 Files selected for processing (3)
README.mdsrc/commands/push/publish.tstest/unit/commands/push/publish.test.ts
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
src/commands/push/publish.ts
Outdated
| description: "Raw recipient JSON for advanced targeting", | ||
| exclusive: ["device-id", "client-id"], | ||
| }), | ||
| "channel-name": Flags.string({ |
There was a problem hiding this comment.
On other commands we use --channel so we should have the same here
There was a problem hiding this comment.
Done — renamed to --channel.
| } | ||
|
|
||
| if (hasDirectRecipient && flags["channel-name"]) { | ||
| this.log( |
There was a problem hiding this comment.
Per coderabbit, this would break json mode
…-json - Rename flag to --channel for consistency with other commands (per Andy) - Wrap --channel-ignored warning in !shouldOutputJson() guard to avoid polluting machine-readable output (per CodeRabbit/Andy) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Summary
--channel-nameflag topush publishcommandextras.push, routing it to push-subscribed devices via the channel--device-id,--client-id,--recipient) is also provided,--channel-nameis silently ignored with a warningSummary by CodeRabbit
Release Notes
New Features
--channel-nameflag toably push publishfor publishing push notifications to channels. When both recipient flags (e.g.,--device-id) and--channel-nameare provided, the channel flag is ignored with a warning.Documentation
ably push publishcommand documentation to expand targeting scope from "device or client" to "device, client, or channel".ably channels presence updatewith--pretty-jsonformatting.