mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-25 00:03:52 +00:00
fix(flows): default update_flow body path from URL via EditFlow
Harmonizes updateFlow with the EditVariable/EditResource/EditApp convention: the flow to update is identified by the URL, so the body path is optional and only needed to rename. Fixes the 422 at the API layer for every client (MCP, the in-app AI chat, raw HTTP), not just the MCP tool schema. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
ff49507c4d
commit
d1c0d4b9e0
@@ -47,7 +47,7 @@ use windmill_common::HUB_BASE_URL;
|
||||
use windmill_common::{
|
||||
db::UserDB,
|
||||
error::{self, to_anyhow, Error, JsonResult, Result},
|
||||
flows::{Flow, FlowWithStarred, ListFlowQuery, ListableFlow, NewFlow},
|
||||
flows::{EditFlow, Flow, FlowWithStarred, ListFlowQuery, ListableFlow, NewFlow},
|
||||
jobs::JobPayload,
|
||||
schedule::Schedule,
|
||||
utils::{http_get_from_hub, not_found_if_none, paginate, Pagination, RunnableKind, StripPath},
|
||||
@@ -1007,7 +1007,7 @@ async fn update_flow(
|
||||
Extension(db): Extension<DB>,
|
||||
Extension(webhook): Extension<WebhookShared>,
|
||||
Path((w_id, flow_path)): Path<(String, StripPath)>,
|
||||
Json(nf): Json<NewFlow>,
|
||||
Json(ef): Json<EditFlow>,
|
||||
) -> Result<String> {
|
||||
if authed.is_operator {
|
||||
return Err(Error::NotAuthorized(
|
||||
@@ -1015,6 +1015,8 @@ async fn update_flow(
|
||||
));
|
||||
}
|
||||
let flow_path = flow_path.to_path();
|
||||
// The URL identifies the flow being updated; the body path is only needed to rename.
|
||||
let nf = ef.into_new_flow(flow_path);
|
||||
check_scopes(&authed, || format!("flows:write:{}", flow_path))?;
|
||||
|
||||
if let RuleCheckResult::Blocked(msg) = check_deploy_rules(
|
||||
|
||||
@@ -145,6 +145,57 @@ impl NewFlow {
|
||||
}
|
||||
}
|
||||
|
||||
/// Body for updating an existing flow. Mirrors `NewFlow`, but `path` is optional: the
|
||||
/// flow to update is identified by the URL, so the body only needs `path` to rename it.
|
||||
/// This matches the `EditVariable` / `EditResource` / `EditApp` convention and lets a
|
||||
/// caller update in place without restating the path.
|
||||
#[derive(Debug, Deserialize)]
|
||||
pub struct EditFlow {
|
||||
#[serde(default)]
|
||||
pub path: Option<String>,
|
||||
pub summary: String,
|
||||
pub description: Option<String>,
|
||||
#[serde(deserialize_with = "validate_flow_value")]
|
||||
pub value: Box<RawValue>,
|
||||
pub schema: Option<Schema>,
|
||||
pub tag: Option<String>,
|
||||
pub dedicated_worker: Option<bool>,
|
||||
pub timeout: Option<i32>,
|
||||
pub deployment_message: Option<String>,
|
||||
pub visible_to_runner_only: Option<bool>,
|
||||
pub on_behalf_of_email: Option<String>,
|
||||
pub preserve_on_behalf_of: Option<bool>,
|
||||
pub ws_error_handler_muted: Option<bool>,
|
||||
#[serde(default)]
|
||||
pub labels: Option<Vec<String>>,
|
||||
#[serde(default)]
|
||||
pub skip_draft_deletion: Option<bool>,
|
||||
}
|
||||
|
||||
impl EditFlow {
|
||||
/// Resolve into a `NewFlow`, defaulting the target path to `current_path` (the flow's
|
||||
/// URL path) when the body omits it. A body `path` that differs renames the flow.
|
||||
pub fn into_new_flow(self, current_path: &str) -> NewFlow {
|
||||
NewFlow {
|
||||
path: self.path.unwrap_or_else(|| current_path.to_string()),
|
||||
summary: self.summary,
|
||||
description: self.description,
|
||||
value: self.value,
|
||||
schema: self.schema,
|
||||
tag: self.tag,
|
||||
dedicated_worker: self.dedicated_worker,
|
||||
timeout: self.timeout,
|
||||
deployment_message: self.deployment_message,
|
||||
visible_to_runner_only: self.visible_to_runner_only,
|
||||
on_behalf_of_email: self.on_behalf_of_email,
|
||||
preserve_on_behalf_of: self.preserve_on_behalf_of,
|
||||
ws_error_handler_muted: self.ws_error_handler_muted,
|
||||
labels: self.labels,
|
||||
skip_draft_deletion: self.skip_draft_deletion,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_retry(retry: &Retry, module_id: &str) -> anyhow::Result<()> {
|
||||
if retry.exponential.attempts > 0 && retry.exponential.seconds == 0 {
|
||||
return Err(anyhow::anyhow!(
|
||||
@@ -1233,6 +1284,22 @@ mod tests {
|
||||
use super::*;
|
||||
use serde_json::json;
|
||||
|
||||
#[test]
|
||||
fn edit_flow_defaults_path_from_url_and_renames_when_given() {
|
||||
// Omitted body path: update in place at the URL path (GIT-925 — a body
|
||||
// without `path` used to fail deserialization on the required NewFlow.path).
|
||||
let ef: EditFlow =
|
||||
serde_json::from_value(json!({ "summary": "s", "value": { "modules": [] } })).unwrap();
|
||||
assert_eq!(ef.into_new_flow("f/team/my_flow").path, "f/team/my_flow");
|
||||
|
||||
// Explicit body path: rename to it.
|
||||
let ef: EditFlow = serde_json::from_value(
|
||||
json!({ "path": "f/team/renamed", "summary": "s", "value": { "modules": [] } }),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(ef.into_new_flow("f/team/my_flow").path, "f/team/renamed");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn flow_value_ignores_notes_and_groups() {
|
||||
// FlowValue should parse successfully even when notes/groups are present —
|
||||
|
||||
Reference in New Issue
Block a user