Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3581,9 +3581,9 @@ Publish a push notification to a device, client, or channel
```
USAGE
$ ably push publish [-v] [--json | --pretty-json] [--device-id <value> | --client-id <value> | --recipient
<value>] [--channel <value>] [--title <value>] [--body <value>] [--sound <value>] [--icon <value>] [--badge <value>]
[--data <value>] [--collapse-key <value>] [--ttl <value>] [--payload <value>] [--apns <value>] [--fcm <value>]
[--web <value>] [-f]
<value>] [--channel <value>] [--message <value>] [--title <value>] [--body <value>] [--sound <value>] [--icon
<value>] [--badge <value>] [--data <value>] [--collapse-key <value>] [--ttl <value>] [--payload <value>] [--apns
<value>] [--fcm <value>] [--web <value>] [-f]

FLAGS
-f, --force Skip confirmation prompt (required with --json)
Expand All @@ -3600,6 +3600,8 @@ FLAGS
--fcm=<value> FCM-specific override as JSON
--icon=<value> Notification icon
--json Output in JSON format
--message=<value> Realtime message data to include alongside the push notification (only applies when
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not against this approach, but tbh it's a bit confusing. --channel command is a shortcut and should work as such. I think the proper way would be to put push --body as realtime message. WDYT?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that works well for a few reasons...

  1. --channel isn't a shortcut... it's config option, same as client-id or device-id
  2. The push body can be different from the realtime message e.g. a user may wish to send some long form data for the app to consume, but wishes to show a much shorter push body e.g. push body = "your friend just reached a high score" and the realtime message could be "{message: "Jamie just beat your high score", score: 77, yourhighScore: 55}" or whatever else

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there was the option of using a positional arg so it would be

ably push publish --channel="test" --title="new notification" --body="notification body" "this is the realtime message"

but i think that is more confusing e.g. a user might mistake the positional arg for the notification body (and then there isn't an argument description like we have now iirc)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, what about fallback to body if message omitted?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think no realtime message but a notification to an entire channel as recipients is a valid use case?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand your response, I mean if --message is omitted, then --body is being put to the realtime data field. Use case - when those two are the same, why have both?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my point is that a user could want to publish a push notification to a realtime channel but have no realtime message associated with it.

If we send the --body as the realtime message, then that implies a user always has to send something to the realtime channel when doing push, which isn't true?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's true, then this behavior should be supported server-side (or within SDK), because right now you've got an empty message fired. Look at this screenshot.

publishing via --channel)
--payload=<value> Full notification payload as JSON (overrides convenience flags)
--pretty-json Output in colorized JSON format
--recipient=<value> Raw recipient JSON for advanced targeting
Expand Down Expand Up @@ -3632,6 +3634,10 @@ EXAMPLES

$ ably push publish --channel my-channel --payload ./notification.json

$ ably push publish --channel my-channel --title Hello --body World --message 'Hello from push'

$ ably push publish --channel my-channel --title Hello --body World --message '{"event":"push","text":"Hello"}'

$ ably push publish --recipient '{"transportType":"apns","deviceToken":"token123"}' --title Hello --body World

$ ably push publish --device-id device-123 --title Hello --body World --json
Expand Down
43 changes: 39 additions & 4 deletions src/commands/push/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export default class PushPublish extends AblyBaseCommand {
'<%= config.bin %> <%= command.id %> --channel my-channel --title Hello --body World --data \'{"key":"value"}\'',
'<%= config.bin %> <%= command.id %> --channel my-channel --payload \'{"notification":{"title":"Hello","body":"World"},"data":{"key":"value"}}\'',
"<%= config.bin %> <%= command.id %> --channel my-channel --payload ./notification.json",
"<%= config.bin %> <%= command.id %> --channel my-channel --title Hello --body World --message 'Hello from push'",
'<%= config.bin %> <%= command.id %> --channel my-channel --title Hello --body World --message \'{"event":"push","text":"Hello"}\'',
'<%= config.bin %> <%= command.id %> --recipient \'{"transportType":"apns","deviceToken":"token123"}\' --title Hello --body World',
"<%= config.bin %> <%= command.id %> --device-id device-123 --title Hello --body World --json",
];
Expand All @@ -45,6 +47,10 @@ export default class PushPublish extends AblyBaseCommand {
description:
"Target channel name (publishes push notification via the channel using extras.push; ignored if --device-id, --client-id, or --recipient is also provided)",
}),
message: Flags.string({
description:
"Realtime message data to include alongside the push notification (only applies when publishing via --channel)",
}),
title: Flags.string({
description: "Notification title",
}),
Expand Down Expand Up @@ -91,6 +97,14 @@ export default class PushPublish extends AblyBaseCommand {
const hasDirectRecipient =
flags["device-id"] || flags["client-id"] || flags.recipient;

if (flags.message && !flags.channel) {
this.fail(
"--message can only be used with --channel (realtime message data is not applicable when publishing directly to a device or client)",
flags as BaseFlags,
"pushPublish",
);
}

if (!hasDirectRecipient && !flags.channel) {
this.fail(
"A target is required: --device-id, --client-id, --recipient, or --channel",
Expand All @@ -104,6 +118,12 @@ export default class PushPublish extends AblyBaseCommand {
"--channel is ignored when --device-id, --client-id, or --recipient is provided.",
flags as BaseFlags,
);
if (flags.message) {
this.logWarning(
"--message is ignored when --device-id, --client-id, or --recipient is provided.",
flags as BaseFlags,
);
}
}

try {
Expand Down Expand Up @@ -257,13 +277,28 @@ export default class PushPublish extends AblyBaseCommand {
}
}

await rest.channels
.get(channelName)
.publish({ extras: { push: payload } });
const publishMessage: Record<string, unknown> = {
extras: { push: payload },
};
if (flags.message) {
try {
publishMessage.data = JSON.parse(flags.message);
} catch {
publishMessage.data = flags.message;
}
}

await rest.channels.get(channelName).publish(publishMessage);

if (this.shouldOutputJson(flags)) {
this.logJsonResult(
{ notification: { published: true, channel: channelName } },
{
notification: {
published: true,
channel: channelName,
...(flags.message ? { messageData: publishMessage.data } : {}),
},
},
flags,
);
}
Expand Down
121 changes: 121 additions & 0 deletions test/unit/commands/push/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe("push:publish command", () => {
"--title",
"--body",
"--payload",
"--message",
]);

describe("functionality", () => {
Expand Down Expand Up @@ -195,9 +196,129 @@ describe("push:publish command", () => {
expect(result.notification).toHaveProperty("published", true);
expect(result.notification).toHaveProperty("channel", "my-channel");
});

it("should include string message data when publishing via channel", async () => {
const mock = getMockAblyRest();
const channel = mock.channels._getChannel("my-channel");

await runCommand(
[
"push:publish",
"--channel",
"my-channel",
"--title",
"Hello",
"--message",
"hello-world",
"--force",
],
import.meta.url,
);

expect(channel.publish).toHaveBeenCalledWith(
expect.objectContaining({
data: "hello-world",
extras: {
push: expect.objectContaining({
notification: expect.objectContaining({ title: "Hello" }),
}),
},
}),
);
});

it("should parse JSON message data when publishing via channel", async () => {
const mock = getMockAblyRest();
const channel = mock.channels._getChannel("my-channel");

await runCommand(
[
"push:publish",
"--channel",
"my-channel",
"--title",
"Hello",
"--message",
'{"key":"val"}',
"--force",
],
import.meta.url,
);

expect(channel.publish).toHaveBeenCalledWith(
expect.objectContaining({
data: { key: "val" },
extras: {
push: expect.objectContaining({
notification: expect.objectContaining({ title: "Hello" }),
}),
},
}),
);
});

it("should ignore --message when direct recipient overrides --channel", async () => {
const mock = getMockAblyRest();

const { stdout, stderr } = await runCommand(
[
"push:publish",
"--device-id",
"dev-1",
"--channel",
"my-channel",
"--message",
"hello",
"--title",
"Hi",
],
import.meta.url,
);

expect(stdout + stderr).toContain("--message is ignored");
expect(mock.push.admin.publish).toHaveBeenCalledWith(
{ deviceId: "dev-1" },
expect.anything(),
);
});

it("should include messageData in JSON output when --message is used", async () => {
const { stdout } = await runCommand(
[
"push:publish",
"--channel",
"my-channel",
"--title",
"Hi",
"--message",
"hello",
"--json",
"--force",
],
import.meta.url,
);

const result = parseJsonOutput(stdout);
expect(result).toHaveProperty("notification");
expect(result.notification).toHaveProperty("published", true);
expect(result.notification).toHaveProperty("channel", "my-channel");
expect(result.notification).toHaveProperty("messageData", "hello");
});
});

describe("error handling", () => {
it("should fail when --message is used without --channel", async () => {
const { error } = await runCommand(
["push:publish", "--message", "hello", "--title", "Hi"],
import.meta.url,
);

expect(error).toBeDefined();
expect(error?.message).toContain(
"--message can only be used with --channel",
);
});

it("should handle API errors", async () => {
const mock = getMockAblyRest();
mock.push.admin.publish.mockRejectedValue(new Error("Publish failed"));
Expand Down
Loading