From 9651f8c709f791f11510b79066e2b592751cba49 Mon Sep 17 00:00:00 2001 From: Parikshit Singh Date: Sat, 30 May 2026 01:40:37 +0530 Subject: [PATCH 1/2] feat: question update --db; fix Slack alerts/notifications for v0.59+ Two related gaps in the CLI that make it impossible to do an in-place database swap on a saved card, or to set up a Slack alert that the Metabase v0.59+ notification API actually accepts. ## question update --db The CLI exposed --name, --sql, --display, etc. on `question update`, but not a way to change the underlying database. Setting only the top-level database_id via PUT /api/card/{id} leaves dataset_query.database pointing at the original DB, so the card keeps running against the old source even after the update succeeds. New flag updates both fields so they stay consistent. Factored out applyDatabaseToDatasetQuery as a pure helper with regression tests covering v0.59+ stages and legacy native shapes. ## Slack alerts / notifications Three coupled bugs that together made `--channel-type slack` unsupported on Metabase v0.59+: 1. channel_type was sent as the bare "slack"/"email" string. v0.59+ requires the "channel/" prefix; the bare form fails with `{"specific-errors":{"handlers":[{"channel_type": ["unknown error, received: :slack"]}]}}`. The new canonicalizeChannelType helper accepts both forms and emits the prefixed one. 2. recipients only supported user-id targets (always emitting `notification-recipient/user`). Slack channel-name targets need a `notification-recipient/raw-value` recipient with `details.value: "#channel"`. Added --slack-channel on `alert create`, `alert update`, and `notification create`, and surfaced helpers (slackChannelRecipient, userRecipient, cronSubscription) on the API module so library users can compose handlers correctly too. 3. Schedule was attached to handlers[].schedule. v0.59+ moved scheduling to top-level subscriptions[] as `notification-subscription/cron` with a cron_schedule string. translateChannelsToSubscriptions converts both the legacy schedule_type/schedule_hour cadence and a new --schedule flag to the new shape; the legacy form is translated to Quartz cron internally for back-compat. Also renamed the payload fields the API now expects: alert_condition + alert_above_goal -> send_condition (has_result / goal_above / goal_below), alert_first_only -> send_once. Both legacy field names still work as CLI input; the translation happens inside AlertApi. ## Verified end-to-end - `metabase-cli question update 24355 --db 35 --unsafe` now updates database_id AND dataset_query.database in one PUT; the card is immediately runnable against the new DB. - `metabase-cli alert create --card 24355 --condition rows --channel-type slack --slack-channel "#nsdc-assessment-alert" --schedule "0 0 * * * ?"` creates alert #58 against v0.59.4 (d4fb593), with the v0.59+ canonical body: handlers: [{ channel_type: "channel/slack", recipients: [{ type: "notification-recipient/raw-value", details: { value: "#nsdc-assessment-alert" } }] }], subscriptions: [{ type: "notification-subscription/cron", cron_schedule: "0 0 * * * ?" }], payload: { send_condition: "has_result", send_once: false } ## Tests Full suite stays green: 142 passing (138 baseline + 4 new). New coverage: applyDatabaseToDatasetQuery on v0.59+ stages and legacy native shapes, and an AlertApi.create regression that asserts channel/slack + raw-value recipient + top-level cron subscription on the outgoing body. --- CHANGELOG.md | 15 ++++ README.md | 23 ++++++ src/api/alert.ts | 152 +++++++++++++++++++++++++++-------- src/api/notification.ts | 119 +++++++++++++++++++++++---- src/commands/alert.ts | 116 ++++++++++++++++++++------ src/commands/notification.ts | 94 +++++++++++++++++----- src/commands/question.ts | 28 +++++-- test/api-modules.test.ts | 52 ++++++++++-- test/question-update.test.ts | 46 ++++++++++- 9 files changed, 537 insertions(+), 108 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb74b89..7b32d34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,21 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added + +- `question update --db ` flag — moves a saved card to a different database. Updates both the top-level `database_id` and `dataset_query.database` so the two never drift. Without this, `PUT /api/card/{id}` with a new `database_id` alone leaves `dataset_query.database` pointing at the old DB and the card keeps running against the original source. +- `alert create` / `notification create`: `--slack-channel ` flag for posting to a Slack channel by name. Emits a `notification-recipient/raw-value` recipient with `details.value: "#channel"`, which is the only shape v0.59+ Metabase accepts for channel-name targets. +- `alert create` / `alert update`: `--schedule ` (Quartz/Spring), `--schedule-type hourly|daily|weekly|monthly`, and `--schedule-hour` flags. Schedules are now attached to the top-level `subscriptions[]` as `notification-subscription/cron` (the location v0.59+ expects). +- `notification create`: `--condition rows|has_result|goal_above|goal_below`, `--send-once`, and `--disable-links` flags so the full v0.59+ payload is reachable from the CLI. + +### Fixed + +- `alert create` / `notification create` with `--channel-type slack` no longer returns `400 Bad Request: {"specific-errors":{"handlers":[{"channel_type":["unknown error, received: :slack"]}]}}`. The CLI now canonicalizes the channel type to the `channel/slack` / `channel/email` form Metabase v0.59+ requires. Both the bare (`slack`) and prefixed (`channel/slack`) forms are accepted. +- `notification create --schedule` no longer attaches a raw cron string to `handlers[].schedule` (which the v0.59+ API silently ignored). The cron is now mounted at the notification level as `subscriptions[{type: "notification-subscription/cron", cron_schedule: "..."}]`, matching the server-side payload model. +- `AlertApi.create` / `AlertApi.update` translate `alert_condition` + `alert_above_goal` → `send_condition` (`has_result` / `goal_above` / `goal_below`) and `alert_first_only` → `send_once`, matching the renamed payload fields in v0.59+. Older callers can keep using the legacy field names; the translation is internal. + ## [0.6.1] - 2026-04-20 ### Added diff --git a/README.md b/README.md index 98bbd79..10bfa1b 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,7 @@ metabase-cli question create --name "Revenue Trend" --sql-file trend.sql --db 1 metabase-cli question update 42 --name "New Name" metabase-cli question update 42 --sql-file updated-query.sql --unsafe metabase-cli question update 42 --display line --viz-file viz.json +metabase-cli question update 42 --db 35 --sql-file ch-query.sql --unsafe # Move card to a different DB # Delete a question metabase-cli question delete 42 @@ -274,10 +275,21 @@ metabase-cli alert list metabase-cli alert show 3 metabase-cli alert create --card 42 --condition rows --first-only metabase-cli alert create --card 42 --condition goal --above-goal --recipients 1,2,3 + +# Slack alert: post to a channel on every cron tick while the question has rows. +metabase-cli alert create \ + --card 42 \ + --condition rows \ + --channel-type slack \ + --slack-channel "#alerts" \ + --schedule "0 0 * * * ?" # Quartz cron — hourly on the hour + metabase-cli alert update 3 --condition goal --above-goal metabase-cli alert delete 3 ``` +`--channel-type` accepts the bare values `slack` / `email` or the prefixed `channel/slack` / `channel/email` — the CLI canonicalizes to the prefixed form Metabase v0.59+ requires. Slack handlers need `--slack-channel <#name>` (a `notification-recipient/raw-value` is emitted under the hood) — Slack user IDs alone aren't enough. `--schedule` accepts a Quartz/Spring cron string and is attached as a top-level `notification-subscription/cron`; for legacy callers the older `--schedule-type hourly|daily|weekly|monthly` + `--schedule-hour` still works and is translated to cron. + ### Revisions ```bash @@ -331,10 +343,21 @@ metabase-cli segment delete 1 metabase-cli notification list metabase-cli notification show 1 metabase-cli notification create --card 42 --channel-type email --recipients "1,2,3" + +# Slack channel post — raw-value recipient + top-level cron subscription. +metabase-cli notification create \ + --card 42 \ + --channel-type slack \ + --slack-channel "#alerts" \ + --schedule "0 0 * * * ?" \ + --condition has_result + metabase-cli notification update 1 --active false metabase-cli notification send 1 ``` +The `--condition` flag accepts `rows` / `has_result` / `goal_above` / `goal_below`. `--send-once` corresponds to the v0.59+ payload's `send_once` (the old `alert_first_only`). + ## File Input Commands that accept inline SQL or JSON also support reading from files. This is useful for complex multi-line queries or JSON configurations. Each `--flag` has a corresponding `--flag-file` alternative: diff --git a/src/api/alert.ts b/src/api/alert.ts index 735ea7f..42fa9b0 100644 --- a/src/api/alert.ts +++ b/src/api/alert.ts @@ -1,13 +1,32 @@ import type { MetabaseClient } from "../client.js"; +import { + canonicalizeChannelType, + cronSubscription, + type NotificationHandler, + type NotificationRecipient, + type NotificationSendCondition, + type NotificationSubscription, + slackChannelRecipient, + userRecipient, +} from "./notification.js"; export interface AlertChannel { channel_type: string; enabled: boolean; + /** User-id recipients (email handlers, occasionally Slack DMs). */ recipients?: { id: number }[]; - details?: Record; - schedule_type?: string; + /** + * Slack channel name (e.g. "#alerts"). The legacy API put this on + * `details.channel`; the v0.59+ API expects a raw-value recipient with + * `details.value`. The CLI normalizes either form into the new shape. + */ + details?: { channel?: string; [k: string]: unknown }; + /** Legacy cadence fields, kept for back-compat with pre-v0.59 callers. */ + schedule_type?: "hourly" | "daily" | "weekly" | "monthly" | string; schedule_hour?: number; schedule_day?: string; + /** Cron schedule (Quartz/Spring). Preferred over schedule_type on v0.59+. */ + cron_schedule?: string; } export interface CreateAlertParams { @@ -26,27 +45,17 @@ export interface UpdateAlertParams { channels?: AlertChannel[]; } -interface NotificationHandler { - channel_type: string; - recipients?: { type: string; user_id: number }[]; - schedule?: { - schedule_type?: string; - schedule_hour?: number; - schedule_day?: string; - }; -} - interface NotificationPayload { card_id: number; - alert_condition?: "rows" | "goal"; - alert_first_only?: boolean; - alert_above_goal?: boolean; + send_once?: boolean; + send_condition?: NotificationSendCondition; } interface NotificationCreateBody { payload_type: "notification/card"; payload: NotificationPayload; handlers: NotificationHandler[]; + subscriptions?: NotificationSubscription[]; active: boolean; } @@ -54,30 +63,98 @@ interface NotificationUpdateBody { payload_type?: "notification/card"; payload?: Partial; handlers?: NotificationHandler[]; + subscriptions?: NotificationSubscription[]; active?: boolean; } -function translateChannelsToHandlers(channels: AlertChannel[]): NotificationHandler[] { +function mapSendCondition( + condition: "rows" | "goal", + aboveGoal?: boolean, +): NotificationSendCondition { + if (condition === "rows") return "has_result"; + return aboveGoal ? "goal_above" : "goal_below"; +} + +// Translate the legacy schedule_type/schedule_hour shape into a cron expression +// the new subscriptions[] API understands. Returns undefined if the channel +// carries no schedule info. +function channelToCron(ch: AlertChannel): string | undefined { + if (ch.cron_schedule) return ch.cron_schedule; + const t = ch.schedule_type; + if (!t) return undefined; + // Quartz/Spring cron: seconds minutes hours day-of-month month day-of-week + const hour = ch.schedule_hour ?? 0; + switch (t) { + case "hourly": + return "0 0 * * * ?"; + case "daily": + return `0 0 ${hour} * * ?`; + case "weekly": { + // Map day name to Quartz DOW (1=Sun..7=Sat) + const dow: Record = { + sun: 1, + mon: 2, + tue: 3, + wed: 4, + thu: 5, + fri: 6, + sat: 7, + }; + const d = ch.schedule_day ? dow[ch.schedule_day.toLowerCase().slice(0, 3)] : 2; + return `0 0 ${hour} ? * ${d ?? 2}`; + } + case "monthly": + return `0 0 ${hour} 1 * ?`; + default: + return undefined; + } +} + +export function translateChannelsToHandlers(channels: AlertChannel[]): NotificationHandler[] { return channels.map((ch) => { - const handler: NotificationHandler = { - channel_type: ch.channel_type, - }; + const channelType = canonicalizeChannelType(ch.channel_type); + const recipients: NotificationRecipient[] = []; + if (ch.recipients) { - handler.recipients = ch.recipients.map((r) => ({ - type: "notification-recipient/user", - user_id: r.id, - })); + for (const r of ch.recipients) { + recipients.push(userRecipient(r.id)); + } } - if (ch.schedule_type || ch.schedule_hour !== undefined || ch.schedule_day) { - handler.schedule = {}; - if (ch.schedule_type) handler.schedule.schedule_type = ch.schedule_type; - if (ch.schedule_hour !== undefined) handler.schedule.schedule_hour = ch.schedule_hour; - if (ch.schedule_day) handler.schedule.schedule_day = ch.schedule_day; + + // Slack channel name can arrive as legacy details.channel or details.value; + // accept either and emit the raw-value recipient the v0.59+ API requires. + const slackChannel = + typeof ch.details?.channel === "string" + ? ch.details.channel + : typeof (ch.details as { value?: string } | undefined)?.value === "string" + ? (ch.details as { value?: string }).value + : undefined; + if (channelType === "channel/slack" && slackChannel) { + recipients.push(slackChannelRecipient(slackChannel)); } - return handler; + + return { channel_type: channelType, recipients }; }); } +export function translateChannelsToSubscriptions( + channels: AlertChannel[], +): NotificationSubscription[] { + const subs: NotificationSubscription[] = []; + // The new API attaches a single schedule at the notification level. If + // multiple channels carry conflicting schedules we take the first one + // found, since that matches how the legacy API behaved (the schedule was + // notification-wide even when stored per-channel). + for (const ch of channels) { + const cron = channelToCron(ch); + if (cron) { + subs.push(cronSubscription(cron)); + break; + } + } + return subs; +} + export class AlertApi { constructor(private client: MetabaseClient) {} @@ -93,15 +170,16 @@ export class AlertApi { } async create(params: CreateAlertParams): Promise { + const subscriptions = translateChannelsToSubscriptions(params.channels); const body: NotificationCreateBody = { payload_type: "notification/card", payload: { card_id: params.card.id, - alert_condition: params.alert_condition, - alert_first_only: params.alert_first_only, - alert_above_goal: params.alert_above_goal, + send_once: params.alert_first_only, + send_condition: mapSendCondition(params.alert_condition, params.alert_above_goal), }, handlers: translateChannelsToHandlers(params.channels), + ...(subscriptions.length > 0 ? { subscriptions } : {}), active: true, }; return this.client.post("/api/notification", body); @@ -114,13 +192,17 @@ export class AlertApi { const payload: Partial = {}; if (params.card) payload.card_id = params.card.id; - if (params.alert_condition !== undefined) payload.alert_condition = params.alert_condition; - if (params.alert_first_only !== undefined) payload.alert_first_only = params.alert_first_only; - if (params.alert_above_goal !== undefined) payload.alert_above_goal = params.alert_above_goal; + if (params.alert_condition !== undefined || params.alert_above_goal !== undefined) { + const condition = params.alert_condition ?? "rows"; + payload.send_condition = mapSendCondition(condition, params.alert_above_goal); + } + if (params.alert_first_only !== undefined) payload.send_once = params.alert_first_only; if (Object.keys(payload).length > 0) body.payload = payload; if (params.channels) { body.handlers = translateChannelsToHandlers(params.channels); + const subs = translateChannelsToSubscriptions(params.channels); + if (subs.length > 0) body.subscriptions = subs; } return this.client.put(`/api/notification/${id}`, body); diff --git a/src/api/notification.ts b/src/api/notification.ts index df6dceb..c27e249 100644 --- a/src/api/notification.ts +++ b/src/api/notification.ts @@ -1,29 +1,116 @@ import type { MetabaseClient } from "../client.js"; +// Metabase v0.59+ notification API +// (replaces the legacy /api/alert + /api/pulse endpoints). +// +// The shape mirrors the wire format the server actually accepts -- not the +// pre-v0.59 alert shape with `channels[]` and `schedule_type` at the top +// level. Three things that have moved: +// +// 1. channel_type now requires the `channel/` prefix +// ("channel/email", "channel/slack"). The bare values "email" / "slack" +// are rejected with 400 -- the canonicalize helper accepts both. +// 2. Schedule lives on top-level `subscriptions[]`, not on handlers. +// Use `notification-subscription/cron` with a cron_schedule string. +// 3. Slack channel names go through a `notification-recipient/raw-value` +// recipient with `details.value: "#channel"`. The legacy user-id +// recipient (`notification-recipient/user`) is still used for email. + +export type NotificationChannelType = "channel/email" | "channel/slack" | (string & {}); +export type NotificationRecipientType = + | "notification-recipient/user" + | "notification-recipient/raw-value"; + +export type NotificationSendCondition = "has_result" | "goal_above" | "goal_below"; + +export interface NotificationRecipient { + type: NotificationRecipientType; + /** Present when type === "notification-recipient/user" */ + user_id?: number; + /** Present when type === "notification-recipient/raw-value" (e.g. Slack channel name). */ + details?: { value: string }; +} + +export interface NotificationHandler { + channel_type: NotificationChannelType; + channel_id?: number | null; + recipients?: NotificationRecipient[]; + template_id?: number | null; +} + +export interface NotificationCronSubscription { + type: "notification-subscription/cron"; + cron_schedule: string; + ui_display_type?: string; +} + +export type NotificationSubscription = NotificationCronSubscription; + +export interface NotificationCardPayload { + card_id: number; + /** When true, send once and stop. Equivalent of the old `alert_first_only`. */ + send_once?: boolean; + /** + * has_result -- alert when the question returns any rows + * goal_above -- alert when the metric crosses above a numeric goal + * goal_below -- alert when the metric crosses below a numeric goal + * Equivalent of the old `alert_condition` + `alert_above_goal`. + */ + send_condition?: NotificationSendCondition; + disable_links?: boolean; +} + export interface CreateNotificationParams { - payload_type: string; - payload: { card_id: number }; - handlers: { - channel_type: string; - channel_id?: number; - recipients?: { type: string; user_id: number }[]; - schedule?: string; - }[]; + payload_type: "notification/card"; + payload: NotificationCardPayload; + handlers: NotificationHandler[]; + subscriptions?: NotificationSubscription[]; active: boolean; } export interface UpdateNotificationParams { - payload_type?: string; - payload?: { card_id: number }; - handlers?: { - channel_type: string; - channel_id?: number; - recipients?: { type: string; user_id: number }[]; - schedule?: string; - }[]; + payload_type?: "notification/card"; + payload?: Partial; + handlers?: NotificationHandler[]; + subscriptions?: NotificationSubscription[]; active?: boolean; } +/** + * Accepts both the bare ("slack", "email") and prefixed + * ("channel/slack", "channel/email") forms, and always returns the prefixed + * form the v0.59+ API expects. + */ +export function canonicalizeChannelType(raw: string): NotificationChannelType { + if (raw.startsWith("channel/")) return raw as NotificationChannelType; + return `channel/${raw}` as NotificationChannelType; +} + +/** Build a Slack-channel-name recipient (raw-value, with details.value). */ +export function slackChannelRecipient(channelName: string): NotificationRecipient { + const value = channelName.startsWith("#") ? channelName : `#${channelName}`; + return { + type: "notification-recipient/raw-value", + details: { value }, + }; +} + +/** Build a user-id recipient (used by email handlers, occasionally Slack DMs). */ +export function userRecipient(userId: number): NotificationRecipient { + return { + type: "notification-recipient/user", + user_id: userId, + }; +} + +/** Build a top-level cron subscription. */ +export function cronSubscription(cron: string): NotificationCronSubscription { + return { + type: "notification-subscription/cron", + cron_schedule: cron, + }; +} + export class NotificationApi { constructor(private client: MetabaseClient) {} diff --git a/src/commands/alert.ts b/src/commands/alert.ts index bd92a92..05a2e2e 100644 --- a/src/commands/alert.ts +++ b/src/commands/alert.ts @@ -1,12 +1,42 @@ import { Command } from "commander"; -import { AlertApi } from "../api/alert.js"; +import { AlertApi, type AlertChannel } from "../api/alert.js"; import { formatEntityTable, formatJson } from "../utils/output.js"; import { resolveClient } from "./helpers.js"; +function buildChannel(opts: { + channelType: string; + recipients?: string; + slackChannel?: string; + schedule?: string; + scheduleType?: string; + scheduleHour?: string; +}): AlertChannel { + const ch: AlertChannel = { + channel_type: opts.channelType, + enabled: true, + }; + if (opts.recipients) { + ch.recipients = opts.recipients + .split(",") + .map((id) => ({ id: parseInt(id.trim()) })) + .filter((r) => !Number.isNaN(r.id)); + } + if (opts.slackChannel) { + ch.details = { channel: opts.slackChannel }; + } + if (opts.schedule) { + ch.cron_schedule = opts.schedule; + } else if (opts.scheduleType) { + ch.schedule_type = opts.scheduleType; + if (opts.scheduleHour !== undefined) ch.schedule_hour = parseInt(opts.scheduleHour); + } + return ch; +} + export function alertCommand(): Command { const cmd = new Command("alert") .description( - "Manage alerts. Note: Alerts use the Notification API internally. For full notification control, use 'metabase-cli notification'.", + "Manage alerts. Alerts use the Notification API internally (changed in v0.6.0). For full notification control, use 'metabase-cli notification'.", ) .addHelpText( "after", @@ -15,6 +45,7 @@ Examples: $ metabase-cli alert list $ metabase-cli alert show 3 $ metabase-cli alert create --card 42 --condition rows --first-only + $ metabase-cli alert create --card 42 --condition rows --channel-type slack --slack-channel "#alerts" --schedule "0 0 * * * ?" $ metabase-cli alert update 3 --condition goal --above-goal $ metabase-cli alert delete 3`, ); @@ -43,8 +74,8 @@ Examples: const rows = (alerts as any[]).map((a) => ({ id: a.id, card_name: a.payload?.card?.name ?? a.payload?.card_id ?? "", - alert_condition: a.payload?.alert_condition ?? "", - alert_first_only: a.payload?.alert_first_only ?? "", + alert_condition: a.payload?.send_condition ?? a.payload?.alert_condition ?? "", + alert_first_only: a.payload?.send_once ?? a.payload?.alert_first_only ?? "", creator: a.creator ? `${a.creator.first_name} ${a.creator.last_name}` : (a.creator_id ?? ""), @@ -83,22 +114,42 @@ Examples: .option("--condition ", "Alert condition: rows, goal", "rows") .option("--first-only", "Only alert the first time", false) .option("--above-goal", "Alert when above goal (for goal condition)", false) - .option("--channel-type ", "Channel type: email, slack", "email") - .option("--recipients ", "Comma-separated recipient user IDs") + .option( + "--channel-type ", + "Channel type: email, slack (accepts bare or channel/-prefixed values)", + "email", + ) + .option("--recipients ", "Comma-separated recipient user IDs (email handlers)") + .option( + "--slack-channel ", + "Slack channel name (e.g. #alerts) -- required for Slack handlers", + ) + .option( + "--schedule ", + "Quartz/Spring cron schedule (e.g. '0 0 * * * ?' for hourly on the hour)", + ) + .option( + "--schedule-type ", + "Legacy schedule cadence: hourly | daily | weekly | monthly", + ) + .option("--schedule-hour ", "Hour for daily/weekly schedules (0-23)") .addHelpText( "after", ` Examples: $ metabase-cli alert create --card 42 --condition rows --first-only - $ metabase-cli alert create --card 42 --condition goal --above-goal --channel-type email --recipients 1,2,3`, + $ metabase-cli alert create --card 42 --condition goal --above-goal --channel-type email --recipients 1,2,3 + $ metabase-cli alert create --card 42 --condition rows --channel-type slack --slack-channel "#alerts" --schedule "0 0 * * * ?"`, ) .action(async (opts) => { const client = await resolveClient(); const api = new AlertApi(client); - const recipients = opts.recipients - ? opts.recipients.split(",").map((id: string) => ({ id: parseInt(id.trim()) })) - : []; + if (opts.channelType === "slack" && !opts.slackChannel && !opts.recipients) { + throw new Error( + "Slack handler requires --slack-channel <#name> or --recipients .", + ); + } const alert = await api.create({ card: { id: opts.card }, @@ -106,11 +157,14 @@ Examples: alert_first_only: opts.firstOnly, alert_above_goal: opts.aboveGoal || undefined, channels: [ - { - channel_type: opts.channelType, - enabled: true, - recipients, - }, + buildChannel({ + channelType: opts.channelType, + recipients: opts.recipients, + slackChannel: opts.slackChannel, + schedule: opts.schedule, + scheduleType: opts.scheduleType, + scheduleHour: opts.scheduleHour, + }), ], }); console.log(`Alert #${(alert as any).id} created.`); @@ -125,12 +179,17 @@ Examples: .option("--above-goal", "Alert when above goal") .option("--channel-type ", "Channel type: email, slack") .option("--recipients ", "Comma-separated recipient user IDs") + .option("--slack-channel ", "Slack channel name (e.g. #alerts)") + .option("--schedule ", "Quartz/Spring cron schedule") + .option("--schedule-type ", "Legacy cadence: hourly | daily | weekly | monthly") + .option("--schedule-hour ", "Hour for daily/weekly schedules (0-23)") .addHelpText( "after", ` Examples: $ metabase-cli alert update 3 --condition goal --above-goal - $ metabase-cli alert update 3 --recipients 1,2,3`, + $ metabase-cli alert update 3 --recipients 1,2,3 + $ metabase-cli alert update 3 --channel-type slack --slack-channel "#alerts" --schedule "0 0 * * * ?"`, ) .action(async (id: string, opts) => { const client = await resolveClient(); @@ -142,16 +201,23 @@ Examples: if (opts.condition) updates.alert_condition = opts.condition; if (opts.firstOnly !== undefined) updates.alert_first_only = opts.firstOnly; if (opts.aboveGoal !== undefined) updates.alert_above_goal = opts.aboveGoal; - if (opts.channelType || opts.recipients) { - const recipients = opts.recipients - ? opts.recipients.split(",").map((rid: string) => ({ id: parseInt(rid.trim()) })) - : []; + const channelTouched = + opts.channelType || + opts.recipients || + opts.slackChannel || + opts.schedule || + opts.scheduleType || + opts.scheduleHour !== undefined; + if (channelTouched) { updates.channels = [ - { - channel_type: opts.channelType || "email", - enabled: true, - recipients, - }, + buildChannel({ + channelType: opts.channelType || "email", + recipients: opts.recipients, + slackChannel: opts.slackChannel, + schedule: opts.schedule, + scheduleType: opts.scheduleType, + scheduleHour: opts.scheduleHour, + }), ]; } diff --git a/src/commands/notification.ts b/src/commands/notification.ts index 46df9d2..49c3988 100644 --- a/src/commands/notification.ts +++ b/src/commands/notification.ts @@ -1,8 +1,28 @@ import { Command } from "commander"; -import { NotificationApi } from "../api/notification.js"; +import { + canonicalizeChannelType, + cronSubscription, + NotificationApi, + type NotificationHandler, + type NotificationRecipient, + type NotificationSendCondition, + type NotificationSubscription, + slackChannelRecipient, + userRecipient, +} from "../api/notification.js"; import { formatEntityTable, formatJson } from "../utils/output.js"; import { resolveClient } from "./helpers.js"; +function parseSendCondition(raw: string): NotificationSendCondition { + const v = raw.toLowerCase(); + if (v === "rows" || v === "has_result") return "has_result"; + if (v === "goal_above" || v === "above_goal") return "goal_above"; + if (v === "goal_below" || v === "below_goal") return "goal_below"; + throw new Error( + `Invalid --condition "${raw}". Use one of: rows | has_result | goal_above | goal_below`, + ); +} + export function notificationCommand(): Command { const cmd = new Command("notification").description("Manage notifications").addHelpText( "after", @@ -11,6 +31,7 @@ Examples: $ metabase-cli notification list $ metabase-cli notification show 1 $ metabase-cli notification create --card 42 --channel-type email --recipients "1,2,3" + $ metabase-cli notification create --card 42 --channel-type slack --slack-channel "#alerts" --schedule "0 0 * * * ?" $ metabase-cli notification send 1`, ); @@ -65,39 +86,74 @@ Examples: .command("create") .description("Create a new notification") .requiredOption("--card ", "Card ID to notify on", parseInt) - .option("--channel-type ", "Channel type: email, slack", "email") - .option("--recipients ", "Comma-separated recipient user IDs") - .option("--schedule ", "Cron expression for the schedule") + .option( + "--channel-type ", + "Channel type: email, slack (accepts bare or channel/-prefixed values)", + "email", + ) + .option("--recipients ", "Comma-separated recipient user IDs (email handlers)") + .option( + "--slack-channel ", + "Slack channel name (e.g. #alerts) -- emits a raw-value recipient. Required for Slack channels; user IDs alone are not enough.", + ) + .option( + "--schedule ", + "Quartz/Spring cron schedule (e.g. '0 0 * * * ?' for hourly on the hour). Mounted as a top-level subscription, NOT on the handler.", + ) + .option( + "--condition ", + "Send condition: rows | has_result | goal_above | goal_below", + "has_result", + ) + .option("--send-once", "Send only the first time the condition is met", false) + .option("--disable-links", "Disable links in the notification message", false) .addHelpText( "after", ` Examples: $ metabase-cli notification create --card 42 --channel-type email --recipients "1,2,3" - $ metabase-cli notification create --card 42 --channel-type slack --schedule "0 9 * * *"`, + $ metabase-cli notification create --card 42 --channel-type slack --slack-channel "#alerts" --schedule "0 0 * * * ?" + $ metabase-cli notification create --card 42 --channel-type slack --slack-channel "#alerts" --condition goal_above --schedule "0 */15 * * * ?"`, ) .action(async (opts) => { const client = await resolveClient(); const api = new NotificationApi(client); - const recipientIds = opts.recipients - ? opts.recipients.split(",").map((id: string) => parseInt(id.trim())) - : []; + const channelType = canonicalizeChannelType(opts.channelType); + + const recipients: NotificationRecipient[] = []; + if (opts.recipients) { + for (const id of opts.recipients.split(",")) { + const parsed = parseInt(id.trim()); + if (!Number.isNaN(parsed)) recipients.push(userRecipient(parsed)); + } + } + if (opts.slackChannel) { + recipients.push(slackChannelRecipient(opts.slackChannel)); + } - const handler: Record = { - channel_type: opts.channelType, - recipients: recipientIds.map((id: number) => ({ - type: "notification-recipient/user", - user_id: id, - })), - }; - if (opts.schedule) { - handler.schedule = opts.schedule; + if (channelType === "channel/slack" && recipients.length === 0) { + throw new Error( + "Slack handler requires either --slack-channel <#name> or --recipients .", + ); } + const handler: NotificationHandler = { channel_type: channelType, recipients }; + + const subscriptions: NotificationSubscription[] = opts.schedule + ? [cronSubscription(opts.schedule)] + : []; + const notification = await api.create({ payload_type: "notification/card", - payload: { card_id: opts.card }, - handlers: [handler as any], + payload: { + card_id: opts.card, + send_condition: parseSendCondition(opts.condition), + send_once: !!opts.sendOnce, + disable_links: !!opts.disableLinks, + }, + handlers: [handler], + subscriptions, active: true, }); console.log(`Notification #${(notification as any).id} created.`); diff --git a/src/commands/question.ts b/src/commands/question.ts index 00ad92c..7ac9a79 100644 --- a/src/commands/question.ts +++ b/src/commands/question.ts @@ -23,6 +23,17 @@ const TAG_TYPE_TO_PARAM_TYPE: Record = { // object (legacy `{query, template-tags}` shape) under stages corrupts the // card: Metabase saves the extra object but the query processor fails to // substitute parameters, leaving the card unrunnable. +// Returns a new DatasetQuery with the top-level `database` field set to the +// given id. Card updates that change the underlying database must also flow +// through dataset_query.database -- if only `database_id` is sent at the top +// level, Metabase keeps running the card against the original database. +export function applyDatabaseToDatasetQuery( + existing: DatasetQuery, + databaseId: number, +): DatasetQuery { + return { ...existing, database: databaseId }; +} + export function buildDatasetQueryUpdate( existing: DatasetQuery, sql: string | undefined, @@ -374,6 +385,7 @@ Examples: .option("--collection ", "Move to collection", parseInt) .option("--sql ", "New SQL query") .option("--sql-file ", "Read new SQL query from a file") + .option("--db ", "Move the question to a different database", parseInt) .option("--display ", "Change display type (table, line, bar, pie, scalar, etc.)") .option("--viz ", "Visualization settings as JSON (merged with existing)") .option("--viz-file ", "Read visualization settings from a JSON file") @@ -388,6 +400,7 @@ Safe mode blocks updates to questions you didn't create. Use --unsafe to bypass. Examples: $ metabase-cli question update 42 --name "New Name" $ metabase-cli question update 42 --sql-file updated-query.sql --unsafe + $ metabase-cli question update 42 --db 35 --sql-file ch-query.sql --unsafe $ metabase-cli question update 42 --display line $ metabase-cli question update 42 --viz-file viz.json $ metabase-cli question update 42 --sql-file query.sql --template-tags-file tags.json`, @@ -441,17 +454,20 @@ Examples: } } - if (opts.sql || opts.sqlFile || templateTags) { + const needsDatasetQueryUpdate = + opts.sql || opts.sqlFile || templateTags || opts.db !== undefined; + if (needsDatasetQueryUpdate) { const sql = opts.sql || opts.sqlFile ? resolveInput(opts.sql, opts.sqlFile, "sql", "sql-file") : undefined; const existing = await api.get(cardId); - updates.dataset_query = buildDatasetQueryUpdate( - existing.dataset_query, - sql, - templateTags, - ); + let datasetQuery = buildDatasetQueryUpdate(existing.dataset_query, sql, templateTags); + if (opts.db !== undefined) { + datasetQuery = applyDatabaseToDatasetQuery(datasetQuery, opts.db); + updates.database_id = opts.db; + } + updates.dataset_query = datasetQuery; if (templateTags) { updates.parameters = buildParametersFromTags(templateTags); diff --git a/test/api-modules.test.ts b/test/api-modules.test.ts index 2b64972..498255f 100644 --- a/test/api-modules.test.ts +++ b/test/api-modules.test.ts @@ -75,7 +75,7 @@ describe("AlertApi", () => { expect(opts.method).toBe("GET"); }); - it("create(params) → POST /api/notification with translated body", async () => { + it("create(params) → POST /api/notification with v0.59+ translated body", async () => { const client = new MetabaseClient(makeProfile()); const api = new AlertApi(client); globalThis.fetch = mockFetch({ id: 1 }); @@ -95,15 +95,55 @@ describe("AlertApi", () => { payload_type: "notification/card", payload: { card_id: 10, - alert_condition: "rows", - alert_first_only: false, + send_condition: "has_result", + send_once: false, }, - handlers: [{ channel_type: "email" }], + // Channel type is canonicalized to the v0.59+ "channel/" form. + handlers: [{ channel_type: "channel/email", recipients: [] }], active: true, }); }); - it("update(1, params) → PUT /api/notification/1 with translated body", async () => { + it("create(slack) → emits raw-value recipient + top-level cron subscription", async () => { + const client = new MetabaseClient(makeProfile()); + const api = new AlertApi(client); + globalThis.fetch = mockFetch({ id: 2 }); + + const params = { + card: { id: 10 }, + alert_condition: "rows" as const, + alert_first_only: false, + channels: [ + { + channel_type: "slack", + enabled: true, + details: { channel: "#alerts" }, + schedule_type: "hourly" as const, + }, + ], + }; + await api.create(params); + + const [, opts] = (globalThis.fetch as any).mock.calls[0]; + const body = JSON.parse(opts.body); + expect(body.handlers).toEqual([ + { + channel_type: "channel/slack", + recipients: [ + { + type: "notification-recipient/raw-value", + details: { value: "#alerts" }, + }, + ], + }, + ]); + // Schedule lives on top-level subscriptions[], not on the handler. + expect(body.subscriptions).toEqual([ + { type: "notification-subscription/cron", cron_schedule: "0 0 * * * ?" }, + ]); + }); + + it("update(1, params) → PUT /api/notification/1 with translated send_once", async () => { const client = new MetabaseClient(makeProfile()); const api = new AlertApi(client); globalThis.fetch = mockFetch({ id: 1 }); @@ -116,7 +156,7 @@ describe("AlertApi", () => { expect(opts.method).toBe("PUT"); expect(JSON.parse(opts.body)).toEqual({ payload_type: "notification/card", - payload: { alert_first_only: true }, + payload: { send_once: true }, }); }); diff --git a/test/question-update.test.ts b/test/question-update.test.ts index ce2a496..a452e11 100644 --- a/test/question-update.test.ts +++ b/test/question-update.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { buildDatasetQueryUpdate } from "../src/commands/question.js"; +import { applyDatabaseToDatasetQuery, buildDatasetQueryUpdate } from "../src/commands/question.js"; import type { DatasetQuery } from "../src/types.js"; // Regression coverage for the bug where updating a v0.59+ card with template @@ -147,3 +147,47 @@ describe("buildDatasetQueryUpdate — legacy (pre-v0.59) shape", () => { expect(out.native).toEqual({ query: "SELECT {{id}} AS x", "template-tags": tags }); }); }); + +// Regression coverage for the `question update --db ` flag. Without +// updating dataset_query.database alongside the top-level database_id, the +// card runs against the original database even after the PUT succeeds. +describe("applyDatabaseToDatasetQuery", () => { + it("overrides the top-level database field on a v0.59+ stages dataset_query", () => { + const existing: DatasetQuery = { + type: "native", + database: 16, + stages: [{ "lib/type": "mbql.stage/native", native: "SELECT 1" }], + }; + + const out = applyDatabaseToDatasetQuery(existing, 35); + + expect(out.database).toBe(35); + expect(out.stages).toEqual(existing.stages); + expect(out.type).toBe("native"); + }); + + it("overrides the top-level database field on a legacy native dataset_query", () => { + const existing: DatasetQuery = { + type: "native", + database: 2, + native: { query: "SELECT 1", "template-tags": {} }, + }; + + const out = applyDatabaseToDatasetQuery(existing, 14); + + expect(out.database).toBe(14); + expect(out.native).toEqual(existing.native); + }); + + it("does not mutate the input dataset_query", () => { + const existing: DatasetQuery = { + type: "native", + database: 16, + stages: [{ "lib/type": "mbql.stage/native", native: "SELECT 1" }], + }; + + applyDatabaseToDatasetQuery(existing, 35); + + expect(existing.database).toBe(16); + }); +}); From 8ed3443ae8dd296aafb3227ab6c91b291e73cbf4 Mon Sep 17 00:00:00 2001 From: rohanraarora Date: Tue, 2 Jun 2026 16:22:40 +0530 Subject: [PATCH 2/2] Validate prefixed channel/slack; fix lone --above-goal on alert update Two review follow-ups on the v0.59+ alert work: - alert create: canonicalize --channel-type before the Slack-recipient guard so the prefixed form (channel/slack) is validated the same as the bare form (slack). Previously the guard was skipped for the prefixed form and an empty-recipient Slack handler was sent. - AlertApi.update: when only --above-goal is supplied (no --condition), imply a goal condition instead of falling back to rows/has_result, which silently downgraded a goal alert. Adds regression tests for both, plus canonicalizeChannelType idempotency. --- src/api/alert.ts | 5 ++++- src/commands/alert.ts | 15 ++++++++++----- test/api-modules.test.ts | 32 +++++++++++++++++++++++++++++++- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/api/alert.ts b/src/api/alert.ts index 42fa9b0..0a57a3b 100644 --- a/src/api/alert.ts +++ b/src/api/alert.ts @@ -193,7 +193,10 @@ export class AlertApi { const payload: Partial = {}; if (params.card) payload.card_id = params.card.id; if (params.alert_condition !== undefined || params.alert_above_goal !== undefined) { - const condition = params.alert_condition ?? "rows"; + // If only --above-goal is supplied, it implies a goal condition; falling + // back to "rows" here would silently map a goal alert to has_result. + const condition = + params.alert_condition ?? (params.alert_above_goal !== undefined ? "goal" : "rows"); payload.send_condition = mapSendCondition(condition, params.alert_above_goal); } if (params.alert_first_only !== undefined) payload.send_once = params.alert_first_only; diff --git a/src/commands/alert.ts b/src/commands/alert.ts index 05a2e2e..7ef92bc 100644 --- a/src/commands/alert.ts +++ b/src/commands/alert.ts @@ -1,5 +1,6 @@ import { Command } from "commander"; import { AlertApi, type AlertChannel } from "../api/alert.js"; +import { canonicalizeChannelType } from "../api/notification.js"; import { formatEntityTable, formatJson } from "../utils/output.js"; import { resolveClient } from "./helpers.js"; @@ -128,10 +129,7 @@ Examples: "--schedule ", "Quartz/Spring cron schedule (e.g. '0 0 * * * ?' for hourly on the hour)", ) - .option( - "--schedule-type ", - "Legacy schedule cadence: hourly | daily | weekly | monthly", - ) + .option("--schedule-type ", "Legacy schedule cadence: hourly | daily | weekly | monthly") .option("--schedule-hour ", "Hour for daily/weekly schedules (0-23)") .addHelpText( "after", @@ -145,7 +143,14 @@ Examples: const client = await resolveClient(); const api = new AlertApi(client); - if (opts.channelType === "slack" && !opts.slackChannel && !opts.recipients) { + // Canonicalize first so the prefixed form (channel/slack) is validated + // the same as the bare form (slack); otherwise the guard is silently + // skipped and an empty-recipient Slack handler goes out. + if ( + canonicalizeChannelType(opts.channelType) === "channel/slack" && + !opts.slackChannel && + !opts.recipients + ) { throw new Error( "Slack handler requires --slack-channel <#name> or --recipients .", ); diff --git a/test/api-modules.test.ts b/test/api-modules.test.ts index 498255f..d22ebee 100644 --- a/test/api-modules.test.ts +++ b/test/api-modules.test.ts @@ -15,7 +15,7 @@ import { RevisionApi } from "../src/api/revision.js"; import { ActivityApi } from "../src/api/activity.js"; import { TimelineApi } from "../src/api/timeline.js"; import { SegmentApi } from "../src/api/segment.js"; -import { NotificationApi } from "../src/api/notification.js"; +import { NotificationApi, canonicalizeChannelType } from "../src/api/notification.js"; import { DashboardApi } from "../src/api/dashboard.js"; function makeProfile(): Profile { @@ -160,6 +160,22 @@ describe("AlertApi", () => { }); }); + it("update with only alert_above_goal → send_condition goal_above (not has_result)", async () => { + const client = new MetabaseClient(makeProfile()); + const api = new AlertApi(client); + globalThis.fetch = mockFetch({ id: 1 }); + + // Passing --above-goal without --condition must imply a goal condition; + // falling back to "rows" would silently map a goal alert to has_result. + await api.update(1, { alert_above_goal: true }); + + const [, opts] = (globalThis.fetch as any).mock.calls[0]; + expect(JSON.parse(opts.body)).toEqual({ + payload_type: "notification/card", + payload: { send_condition: "goal_above" }, + }); + }); + it("delete(1) → PUT /api/notification/1 with { active: false }", async () => { const client = new MetabaseClient(makeProfile()); const api = new AlertApi(client); @@ -494,6 +510,20 @@ describe("NotificationApi", () => { }); }); +// ─── canonicalizeChannelType ───────────────────────────────────────────────── + +describe("canonicalizeChannelType", () => { + it("prefixes bare channel types", () => { + expect(canonicalizeChannelType("slack")).toBe("channel/slack"); + expect(canonicalizeChannelType("email")).toBe("channel/email"); + }); + + it("is idempotent on already-prefixed types (so validation matches either form)", () => { + expect(canonicalizeChannelType("channel/slack")).toBe("channel/slack"); + expect(canonicalizeChannelType("channel/email")).toBe("channel/email"); + }); +}); + // ─── DashboardApi (dashcards update) ───────────────────────────────────────── describe("DashboardApi", () => {