-
Notifications
You must be signed in to change notification settings - Fork 0
Add channels:inspect command to open dashboard #136
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import { Args, Flags } from "@oclif/core"; | ||
| import chalk from "chalk"; | ||
|
|
||
| import { AblyBaseCommand } from "../../base-command.js"; | ||
| import openUrl from "../../utils/open-url.js"; | ||
|
|
||
| export default class ChannelsInspect extends AblyBaseCommand { | ||
| static override args = { | ||
| channel: Args.string({ | ||
| description: "The name of the channel to inspect in the Ably dashboard", | ||
| required: true, | ||
| }), | ||
| }; | ||
|
|
||
| static override description = | ||
| "Open the Ably dashboard to inspect a specific channel"; | ||
|
|
||
| static override examples = ["<%= config.bin %> <%= command.id %> my-channel"]; | ||
|
|
||
| static override flags = { | ||
| ...AblyBaseCommand.globalFlags, | ||
| app: Flags.string({ | ||
| description: "App ID to use (uses current app if not specified)", | ||
| env: "ABLY_APP_ID", | ||
| }), | ||
| }; | ||
|
|
||
| async run(): Promise<void> { | ||
| const { args, flags } = await this.parse(ChannelsInspect); | ||
|
|
||
| const currentAccount = this.configManager.getCurrentAccount(); | ||
| const accountId = currentAccount?.accountId; | ||
| if (!accountId) { | ||
| this.error( | ||
| `No account configured. Please log in first with ${chalk.cyan('"ably accounts login"')}.`, | ||
| ); | ||
| } | ||
|
|
||
| const appId = flags.app ?? this.configManager.getCurrentAppId(); | ||
| if (!appId) { | ||
| this.error( | ||
| `No app selected. Please select an app first with ${chalk.cyan('"ably apps switch"')} or specify one with ${chalk.cyan("--app")}.`, | ||
| ); | ||
| } | ||
|
|
||
| const url = `https://ably.com/accounts/${accountId}/apps/${appId}/channels/${encodeURIComponent(args.channel)}`; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto coderabbit, this needs to work on the staging sites so should be configurable. It'll also need to be added to the terminal server, because that's where the web CLI will be running |
||
|
|
||
| if (this.isWebCliMode) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than |
||
| this.log(`${chalk.cyan("Visit")} ${url}`); | ||
| } else { | ||
| await openUrl(url, this); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; | ||
| import { runCommand } from "@oclif/test"; | ||
| import { getMockConfigManager } from "../../../helpers/mock-config-manager.js"; | ||
|
|
||
| describe("channels:inspect command", () => { | ||
| const originalEnv = process.env.ABLY_WEB_CLI_MODE; | ||
|
|
||
| afterEach(() => { | ||
| if (originalEnv === undefined) { | ||
| delete process.env.ABLY_WEB_CLI_MODE; | ||
| } else { | ||
| process.env.ABLY_WEB_CLI_MODE = originalEnv; | ||
| } | ||
|
|
||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| describe("normal CLI mode", () => { | ||
| beforeEach(() => { | ||
| delete process.env.ABLY_WEB_CLI_MODE; | ||
| }); | ||
|
|
||
| it("should open browser with correct dashboard URL", async () => { | ||
| const mockConfig = getMockConfigManager(); | ||
| const accountId = mockConfig.getCurrentAccount()!.accountId!; | ||
| const appId = mockConfig.getCurrentAppId()!; | ||
|
|
||
| const { stdout } = await runCommand( | ||
| ["channels:inspect", "my-channel"], | ||
| import.meta.url, | ||
| ); | ||
|
|
||
| expect(stdout).toContain("Opening"); | ||
| expect(stdout).toContain("in your browser"); | ||
| expect(stdout).toContain( | ||
| `https://ably.com/accounts/${accountId}/apps/${appId}/channels/my-channel`, | ||
| ); | ||
| }); | ||
|
|
||
| it("should URL-encode special characters in channel name", async () => { | ||
| const mockConfig = getMockConfigManager(); | ||
| const accountId = mockConfig.getCurrentAccount()!.accountId!; | ||
| const appId = mockConfig.getCurrentAppId()!; | ||
|
|
||
| const { stdout } = await runCommand( | ||
| ["channels:inspect", "my-channel/foo#bar"], | ||
| import.meta.url, | ||
| ); | ||
|
|
||
| expect(stdout).toContain( | ||
| `https://ably.com/accounts/${accountId}/apps/${appId}/channels/my-channel%2Ffoo%23bar`, | ||
| ); | ||
| }); | ||
|
|
||
| it("should error when no account is configured", async () => { | ||
| const mockConfig = getMockConfigManager(); | ||
| mockConfig.clearAccounts(); | ||
|
|
||
| const { error } = await runCommand( | ||
| ["channels:inspect", "my-channel"], | ||
| import.meta.url, | ||
| ); | ||
|
|
||
| expect(error).toBeDefined(); | ||
| expect(error?.message).toContain("No account configured"); | ||
| expect(error?.message).toContain("ably accounts login"); | ||
| }); | ||
|
|
||
| it("should error when no app is selected", async () => { | ||
| const mockConfig = getMockConfigManager(); | ||
| mockConfig.setCurrentAppIdForAccount(undefined); | ||
|
|
||
| const { error } = await runCommand( | ||
| ["channels:inspect", "my-channel"], | ||
| import.meta.url, | ||
| ); | ||
|
|
||
| expect(error).toBeDefined(); | ||
| expect(error?.message).toContain("No app selected"); | ||
| expect(error?.message).toContain("ably apps switch"); | ||
| expect(error?.message).toContain("--app"); | ||
| }); | ||
|
|
||
| it("should use --app flag over current app", async () => { | ||
| const mockConfig = getMockConfigManager(); | ||
| const accountId = mockConfig.getCurrentAccount()!.accountId!; | ||
|
|
||
| const { stdout } = await runCommand( | ||
| ["channels:inspect", "my-channel", "--app", "custom-app-id"], | ||
| import.meta.url, | ||
| ); | ||
|
|
||
| expect(stdout).toContain( | ||
| `https://ably.com/accounts/${accountId}/apps/custom-app-id/channels/my-channel`, | ||
| ); | ||
| }); | ||
|
|
||
| it("should use --app flag when no current app is set", async () => { | ||
| const mockConfig = getMockConfigManager(); | ||
| const accountId = mockConfig.getCurrentAccount()!.accountId!; | ||
| mockConfig.setCurrentAppIdForAccount(undefined); | ||
|
|
||
| const { stdout } = await runCommand( | ||
| ["channels:inspect", "my-channel", "--app", "override-app"], | ||
| import.meta.url, | ||
| ); | ||
|
|
||
| expect(stdout).toContain( | ||
| `https://ably.com/accounts/${accountId}/apps/override-app/channels/my-channel`, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe("web CLI mode", () => { | ||
| beforeEach(() => { | ||
| process.env.ABLY_WEB_CLI_MODE = "true"; | ||
| }); | ||
|
|
||
| it("should display URL without opening browser", async () => { | ||
| const mockConfig = getMockConfigManager(); | ||
| const accountId = mockConfig.getCurrentAccount()!.accountId!; | ||
| const appId = mockConfig.getCurrentAppId()!; | ||
|
|
||
| const { stdout } = await runCommand( | ||
| ["channels:inspect", "my-channel"], | ||
| import.meta.url, | ||
| ); | ||
|
|
||
| expect(stdout).toContain("Visit"); | ||
| expect(stdout).toContain( | ||
| `https://ably.com/accounts/${accountId}/apps/${appId}/channels/my-channel`, | ||
| ); | ||
| expect(stdout).not.toContain("Opening"); | ||
| expect(stdout).not.toContain("in your browser"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("help", () => { | ||
| it("should display help with --help flag", async () => { | ||
| const { stdout } = await runCommand( | ||
| ["channels:inspect", "--help"], | ||
| import.meta.url, | ||
| ); | ||
|
|
||
| expect(stdout).toContain("Open the Ably dashboard to inspect"); | ||
| expect(stdout).toContain("USAGE"); | ||
| expect(stdout).toContain("ARGUMENTS"); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
Repository: ably/ably-cli
Length of output: 3771
🏁 Script executed:
Repository: ably/ably-cli
Length of output: 378
🏁 Script executed:
Repository: ably/ably-cli
Length of output: 5269
🏁 Script executed:
Repository: ably/ably-cli
Length of output: 2654
Hardcoded dashboard base URL violates the "no hardcoded endpoint URLs" guideline.
Line 46 hardcodes
https://ably.comas the dashboard base. This breaks for staging/enterprise environments and is not configurable. AdashboardHostflag or env-override (e.g.,ABLY_DASHBOARD_URL) should supply the base, consistent with howcontrol-hostis used elsewhere in the codebase.♻️ Suggested refactor
Alternatively, add a
dashboard-hostglobal flag toAblyBaseCommand.globalFlags(similar tocontrol-host) and useflags["dashboard-host"]here.🤖 Prompt for AI Agents