diff --git a/packages/cli/src/commands/automations/create/command.ts b/packages/cli/src/commands/automations/create/command.ts index ce8ee5da084..dc516741fa6 100644 --- a/packages/cli/src/commands/automations/create/command.ts +++ b/packages/cli/src/commands/automations/create/command.ts @@ -3,6 +3,7 @@ import { boolean, CLIError, string } from "@superset/cli-framework"; import { command } from "../../../lib/command"; import { resolveHostFilter } from "../../../lib/host-target"; import { formatAutomationDate } from "../format"; +import { assertTriggersConnected } from "../missingConnectionError"; import { resolveAutomationTarget } from "../resolveAutomationTarget"; import { refuseHostFlagsForCloud, @@ -115,6 +116,8 @@ export default command({ v2WorkspaceId: options.workspace ?? undefined, }; + await assertTriggersConnected(ctx.api, organizationId, triggers); + const result = await ctx.api.automation.create.mutate({ name: options.name, prompt, diff --git a/packages/cli/src/commands/automations/missingConnectionError.test.ts b/packages/cli/src/commands/automations/missingConnectionError.test.ts new file mode 100644 index 00000000000..5ac0d91e366 --- /dev/null +++ b/packages/cli/src/commands/automations/missingConnectionError.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from "bun:test"; +import type { ApiClient } from "../../lib/api-client"; +import { + assertTriggersConnected, + missingConnectionError, +} from "./missingConnectionError"; + +const linear = { config: { kind: "linear" } }; +const slack = { config: { kind: "slack" } }; +const live = { connected: true, needsReauth: false }; + +describe("missingConnectionError", () => { + it("refuses a trigger whose integration the caller has not connected", () => { + const error = missingConnectionError([linear], {}); + expect(error?.message).toContain("you have not connected Linear"); + expect(error?.suggestion).toContain("Connect Linear in Settings"); + }); + + it("asks for a reconnect when the connection expired", () => { + const error = missingConnectionError([linear], { + linear: { connected: false, needsReauth: true }, + }); + expect(error?.message).toContain("your Linear connection expired"); + expect(error?.suggestion).toContain("Reconnect Linear"); + }); + + it("refuses a pin to an expired account while another account is live", () => { + const error = missingConnectionError([{ ...linear, connectionId: "old" }], { + linear: { + ...live, + accounts: [ + { id: "old", needsReauth: true }, + { id: "new", needsReauth: false }, + ], + }, + }); + expect(error?.message).toContain("your Linear connection expired"); + }); + + it("refuses a pin to an account the caller does not own", () => { + const error = missingConnectionError( + [{ ...linear, connectionId: "theirs" }], + { linear: { ...live, accounts: [{ id: "mine", needsReauth: false }] } }, + ); + expect(error?.message).toContain("pinned to an account"); + }); + + it("allows a pin to a live account", () => { + expect( + missingConnectionError([{ ...linear, connectionId: "mine" }], { + linear: { ...live, accounts: [{ id: "mine", needsReauth: false }] }, + }), + ).toBeNull(); + }); + + it("allows a trigger whose integration is connected", () => { + expect(missingConnectionError([linear], { linear: live })).toBeNull(); + }); + + it("names every missing integration once", () => { + const error = missingConnectionError([linear, linear, slack], {}); + expect(error?.message).toBe( + "These triggers would never fire: you have not connected Linear; you have not connected Slack", + ); + }); + + it("ignores triggers that need no connection", () => { + expect( + missingConnectionError( + [{ config: { kind: "schedule" } }, { config: { kind: "github" } }], + {}, + ), + ).toBeNull(); + }); +}); + +describe("assertTriggersConnected", () => { + const api = (query: () => Promise) => + ({ integration: { connectionStatus: { query } } }) as unknown as ApiClient; + + it("throws when the status names a missing connection", async () => { + await expect( + assertTriggersConnected( + api(async () => ({})), + "org", + [linear], + ), + ).rejects.toThrow("you have not connected Linear"); + }); + + it("lets the save go ahead when the status query fails", async () => { + await expect( + assertTriggersConnected( + api(async () => { + throw new Error("offline"); + }), + "org", + [linear], + ), + ).resolves.toBeUndefined(); + }); +}); diff --git a/packages/cli/src/commands/automations/missingConnectionError.ts b/packages/cli/src/commands/automations/missingConnectionError.ts new file mode 100644 index 00000000000..6411dcacd00 --- /dev/null +++ b/packages/cli/src/commands/automations/missingConnectionError.ts @@ -0,0 +1,86 @@ +import { CLIError } from "@superset/cli-framework"; +import { TRIGGER_KIND_CONNECTOR } from "@superset/shared/automation-triggers"; +import { getConnector } from "@superset/shared/connectors"; +import type { ApiClient } from "../../lib/api-client"; + +export interface ConnectionState { + connected: boolean; + needsReauth: boolean; + accounts?: { id: string; needsReauth: boolean }[]; +} + +type Problem = "missing" | "expired" | "foreignPin"; + +interface TriggerInput { + config: { kind: string }; + connectionId?: string | null; +} + +function problemFor( + trigger: TriggerInput, + state: ConnectionState | undefined, +): Problem | null { + if (trigger.connectionId) { + const account = state?.accounts?.find( + (candidate) => candidate.id === trigger.connectionId, + ); + if (!account) return "foreignPin"; + return account.needsReauth ? "expired" : null; + } + if (state?.connected) return null; + return state?.needsReauth ? "expired" : "missing"; +} + +export function missingConnectionError( + triggers: TriggerInput[], + status: Record, +): CLIError | null { + const problems = new Map(); + for (const trigger of triggers) { + const connector = TRIGGER_KIND_CONNECTOR[trigger.config.kind]; + if (!connector || problems.has(connector)) continue; + const problem = problemFor(trigger, status[connector]); + if (problem) problems.set(connector, problem); + } + if (problems.size === 0) return null; + + const name = (connector: string) => + getConnector(connector)?.displayName ?? connector; + const reasons: string[] = []; + const steps: string[] = []; + for (const [connector, problem] of problems) { + const n = name(connector); + if (problem === "expired") { + reasons.push(`your ${n} connection expired`); + steps.push(`Reconnect ${n} in Settings → Integrations.`); + } else if (problem === "foreignPin") { + reasons.push( + `a ${n} trigger is pinned to an account you have not connected`, + ); + steps.push( + `Drop its connectionId, or set it to one of your ${n} accounts.`, + ); + } else { + reasons.push(`you have not connected ${n}`); + steps.push(`Connect ${n} in Settings → Integrations.`); + } + } + return new CLIError( + `These triggers would never fire: ${reasons.join("; ")}`, + [...steps, "Then run this command again."].join(" "), + ); +} + +export async function assertTriggersConnected( + api: ApiClient, + organizationId: string | undefined, + triggers: TriggerInput[] | undefined, +): Promise { + if (!organizationId || !triggers?.length) return; + const status = await api.integration.connectionStatus + .query({ organizationId }) + .catch(() => null); + if (!status) return; + const error = missingConnectionError(triggers, status); + if (error) throw error; +} diff --git a/packages/cli/src/commands/automations/update/command.ts b/packages/cli/src/commands/automations/update/command.ts index e9e1d91ba9b..28622dc48b6 100644 --- a/packages/cli/src/commands/automations/update/command.ts +++ b/packages/cli/src/commands/automations/update/command.ts @@ -2,6 +2,7 @@ import { boolean, CLIError, positional, string } from "@superset/cli-framework"; import { CLOUD_HOST_ID } from "@superset/shared/host-routing"; import { command } from "../../../lib/command"; import { resolveHostFilter } from "../../../lib/host-target"; +import { assertTriggersConnected } from "../missingConnectionError"; import { resolveAutomationTarget } from "../resolveAutomationTarget"; import { refuseHostFlagsForCloud, @@ -86,6 +87,12 @@ export default command({ const triggers = resolveTriggers(options); + await assertTriggersConnected( + ctx.api, + ctx.config.organizationId ?? undefined, + triggers, + ); + const targetHostId = resolveHostFilter({ host: options.host ?? undefined, local: options.local ?? undefined,