mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 08:01:26 +00:00
fix: git sync missed metadata-only deploys, deploy check missed job link (#10662)
* fix: git sync missed metadata-only deploys, deploy check missed job link Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: skip the deploy hook when the mute toggle matched no row Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: pin ee ref forward of main so the bump only adds this change Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: update ee-repo-ref to a65162b22b127b54c0686095ee1b16b04e3111f7 This commit updates the EE repository reference after PR #724 was merged in windmill-ee-private. Previous ee-repo-ref: ac5f646c3ace7e5841200c6b83b34fb4371340d9 New ee-repo-ref: a65162b22b127b54c0686095ee1b16b04e3111f7 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
@@ -1 +1 @@
|
||||
60c20e686cead73ff075512b15c6e2d6232beca6
|
||||
a65162b22b127b54c0686095ee1b16b04e3111f7
|
||||
|
||||
@@ -381,6 +381,7 @@ async fn toggle_workspace_error_handler(
|
||||
async fn toggle_workspace_error_handler(
|
||||
authed: ApiAuthed,
|
||||
Extension(user_db): Extension<UserDB>,
|
||||
Extension(db): Extension<DB>,
|
||||
Path((w_id, path)): Path<(String, StripPath)>,
|
||||
Json(req): Json<ToggleWorkspaceErrorHandler>,
|
||||
) -> Result<String> {
|
||||
@@ -401,9 +402,10 @@ async fn toggle_workspace_error_handler(
|
||||
.await?
|
||||
.unwrap_or(None);
|
||||
|
||||
let mut updated_rows = 0;
|
||||
let response = match error_handler_maybe {
|
||||
Some(_) => {
|
||||
sqlx::query_scalar!(
|
||||
updated_rows = sqlx::query_scalar!(
|
||||
r#"
|
||||
UPDATE
|
||||
flow
|
||||
@@ -418,7 +420,8 @@ async fn toggle_workspace_error_handler(
|
||||
req.muted,
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
.await?
|
||||
.rows_affected();
|
||||
Ok("".to_string())
|
||||
}
|
||||
None => Err(Error::BadRequest(
|
||||
@@ -428,6 +431,37 @@ async fn toggle_workspace_error_handler(
|
||||
|
||||
tx.commit().await?;
|
||||
|
||||
// `ws_error_handler_muted` is part of the synced flow metadata, so the
|
||||
// toggle is a deploy like any other edit of it. The version is a
|
||||
// placeholder: git sync keys off the path and kind alone. The update runs
|
||||
// under RLS against an unchecked path, so it can match nothing — deploy
|
||||
// only what it actually wrote.
|
||||
if updated_rows > 0 {
|
||||
handle_deployment_metadata(
|
||||
&authed.email,
|
||||
&authed.username,
|
||||
&db,
|
||||
&w_id,
|
||||
DeployedObject::Flow {
|
||||
path: path.to_path().to_string(),
|
||||
parent_path: None,
|
||||
version: 0,
|
||||
},
|
||||
Some(format!(
|
||||
"Flow '{}' {} the workspace error handler",
|
||||
path.to_path(),
|
||||
if req.muted.unwrap_or(false) {
|
||||
"muted"
|
||||
} else {
|
||||
"unmuted"
|
||||
}
|
||||
)),
|
||||
true,
|
||||
None,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
return response;
|
||||
}
|
||||
|
||||
|
||||
@@ -851,6 +851,7 @@ async fn delete_folder(
|
||||
async fn add_owner(
|
||||
authed: ApiAuthed,
|
||||
Extension(user_db): Extension<UserDB>,
|
||||
Extension(db): Extension<DB>,
|
||||
Extension(webhook): Extension<WebhookShared>,
|
||||
Path((w_id, name)): Path<(String, String)>,
|
||||
Json(Owner { owner, .. }): Json<Owner>,
|
||||
@@ -905,6 +906,18 @@ async fn add_owner(
|
||||
|
||||
tx.commit().await?;
|
||||
|
||||
handle_deployment_metadata(
|
||||
&authed.email,
|
||||
&authed.username,
|
||||
&db,
|
||||
&w_id,
|
||||
DeployedObject::Folder { path: format!("f/{}", name) },
|
||||
Some(format!("Folder '{}' changed permissions", name)),
|
||||
true,
|
||||
None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
webhook.send_message(
|
||||
w_id.clone(),
|
||||
WebhookMessage::UpdateFolder { workspace: w_id, name: name.clone() },
|
||||
@@ -916,6 +929,7 @@ async fn add_owner(
|
||||
async fn remove_owner(
|
||||
authed: ApiAuthed,
|
||||
Extension(user_db): Extension<UserDB>,
|
||||
Extension(db): Extension<DB>,
|
||||
Extension(webhook): Extension<WebhookShared>,
|
||||
Path((w_id, name)): Path<(String, String)>,
|
||||
Json(Owner { owner, write }): Json<Owner>,
|
||||
@@ -999,6 +1013,18 @@ async fn remove_owner(
|
||||
|
||||
tx.commit().await?;
|
||||
|
||||
handle_deployment_metadata(
|
||||
&authed.email,
|
||||
&authed.username,
|
||||
&db,
|
||||
&w_id,
|
||||
DeployedObject::Folder { path: format!("f/{}", name) },
|
||||
Some(format!("Folder '{}' changed permissions", name)),
|
||||
true,
|
||||
None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
webhook.send_message(
|
||||
w_id.clone(),
|
||||
WebhookMessage::UpdateFolder { workspace: w_id, name: name.clone() },
|
||||
|
||||
@@ -2827,6 +2827,7 @@ async fn toggle_workspace_error_handler(
|
||||
async fn toggle_workspace_error_handler(
|
||||
authed: ApiAuthed,
|
||||
Extension(user_db): Extension<UserDB>,
|
||||
Extension(db): Extension<DB>,
|
||||
Path((w_id, path)): Path<(String, StripPath)>,
|
||||
Json(req): Json<ToggleWorkspaceErrorHandler>,
|
||||
) -> Result<String> {
|
||||
@@ -2842,7 +2843,7 @@ async fn toggle_workspace_error_handler(
|
||||
|
||||
match error_handler_maybe {
|
||||
Some(_) => {
|
||||
sqlx::query_scalar!(
|
||||
let updated = sqlx::query_scalar!(
|
||||
"UPDATE script
|
||||
SET ws_error_handler_muted = $3
|
||||
WHERE ctid = (
|
||||
@@ -2859,6 +2860,38 @@ async fn toggle_workspace_error_handler(
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
tx.commit().await?;
|
||||
|
||||
// `ws_error_handler_muted` is part of the synced script metadata, so
|
||||
// the toggle is a deploy like any other edit of it. The hash is a
|
||||
// placeholder: git sync keys off the path and kind alone. The update
|
||||
// runs under RLS against an unchecked path, so it can match nothing —
|
||||
// deploy only what it actually wrote.
|
||||
if updated.rows_affected() > 0 {
|
||||
handle_deployment_metadata(
|
||||
&authed.email,
|
||||
&authed.username,
|
||||
&db,
|
||||
&w_id,
|
||||
DeployedObject::Script {
|
||||
hash: ScriptHash(0),
|
||||
path: path.to_path().to_string(),
|
||||
parent_path: None,
|
||||
},
|
||||
Some(format!(
|
||||
"Script '{}' {} the workspace error handler",
|
||||
path.to_path(),
|
||||
if req.muted.unwrap_or(false) {
|
||||
"muted"
|
||||
} else {
|
||||
"unmuted"
|
||||
}
|
||||
)),
|
||||
true,
|
||||
None,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
Ok("".to_string())
|
||||
}
|
||||
None => {
|
||||
|
||||
@@ -884,6 +884,14 @@ fn parse_pr_check_error(result_raw: &str) -> Option<String> {
|
||||
})
|
||||
}
|
||||
|
||||
/// The run page for `job_id`, or `None` when the instance has no `BASE_URL` set
|
||||
/// (it defaults to empty) — a check must not carry a link that goes nowhere.
|
||||
#[cfg(all(feature = "enterprise", feature = "private"))]
|
||||
fn job_run_url(base_url: &str, job_id: &uuid::Uuid, workspace_id: &str) -> Option<String> {
|
||||
let base = base_url.trim_end_matches('/');
|
||||
(!base.is_empty()).then(|| format!("{base}/run/{job_id}?workspace={workspace_id}"))
|
||||
}
|
||||
|
||||
#[cfg(all(feature = "enterprise", feature = "private"))]
|
||||
fn format_change_list(changes: &[(String, String)]) -> Vec<String> {
|
||||
let mut lines = Vec::new();
|
||||
@@ -898,7 +906,19 @@ fn format_change_list(changes: &[(String, String)]) -> Vec<String> {
|
||||
|
||||
#[cfg(all(test, feature = "enterprise", feature = "private"))]
|
||||
mod git_sync_check_tests {
|
||||
use super::{format_change_list, parse_git_sync_changes, parse_pr_check_error};
|
||||
use super::{format_change_list, job_run_url, parse_git_sync_changes, parse_pr_check_error};
|
||||
|
||||
#[test]
|
||||
fn job_run_url_is_none_without_a_base_url() {
|
||||
let id = uuid::Uuid::nil();
|
||||
assert_eq!(
|
||||
job_run_url("https://app.windmill.dev/", &id, "w").as_deref(),
|
||||
Some("https://app.windmill.dev/run/00000000-0000-0000-0000-000000000000?workspace=w")
|
||||
);
|
||||
// BASE_URL defaults to empty; a link built from it would 404 the reader.
|
||||
assert_eq!(job_run_url("", &id, "w"), None);
|
||||
assert_eq!(job_run_url("/", &id, "w"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pr_check_error_is_a_field_not_a_substring() {
|
||||
@@ -1376,6 +1396,10 @@ async fn maybe_post_git_sync_check(
|
||||
} else {
|
||||
None
|
||||
};
|
||||
// The creating call could only link the check to the workspace's run list —
|
||||
// the check predates the job fulfilling it. Now that the job is known, point
|
||||
// both the summary and the check's "Details" link at its logs.
|
||||
let job_url = job_run_url(&windmill_common::BASE_URL.load(), job_id, workspace_id);
|
||||
|
||||
let (conclusion, title, summary): (&str, String, String) = if is_deploy {
|
||||
// Phase 6: real deploy pull -> "Deployed N changes" / "In sync" / failure.
|
||||
@@ -1383,8 +1407,7 @@ async fn maybe_post_git_sync_check(
|
||||
(
|
||||
"failure",
|
||||
format!("Deploy to {} failed", workspace_id),
|
||||
"Deploying the latest commit failed. See the job in Windmill for details."
|
||||
.to_string(),
|
||||
"Deploying the latest commit failed.".to_string(),
|
||||
)
|
||||
} else {
|
||||
match parse_git_sync_changes(result_raw) {
|
||||
@@ -1447,15 +1470,13 @@ async fn maybe_post_git_sync_check(
|
||||
(
|
||||
"failure",
|
||||
"Windmill diff failed".to_string(),
|
||||
"The dry-run pull reported an unrecognized error. See the job in Windmill for details."
|
||||
.to_string(),
|
||||
"The dry-run pull reported an unrecognized error.".to_string(),
|
||||
)
|
||||
} else if !success {
|
||||
(
|
||||
"failure",
|
||||
"Windmill diff failed".to_string(),
|
||||
"The dry-run pull to compute the diff failed. See the job in Windmill for details."
|
||||
.to_string(),
|
||||
"The dry-run pull to compute the diff failed.".to_string(),
|
||||
)
|
||||
} else {
|
||||
match parse_git_sync_changes(result_raw) {
|
||||
@@ -1495,6 +1516,10 @@ async fn maybe_post_git_sync_check(
|
||||
}
|
||||
};
|
||||
|
||||
let check_summary = match job_url.as_deref() {
|
||||
Some(url) => format!("{summary}\n\n[See the job in Windmill]({url})"),
|
||||
None => summary.clone(),
|
||||
};
|
||||
if let Err(e) = windmill_common::git_sync_ee::update_check_run(
|
||||
db,
|
||||
workspace_id,
|
||||
@@ -1502,7 +1527,8 @@ async fn maybe_post_git_sync_check(
|
||||
check.check_run_id,
|
||||
conclusion,
|
||||
&title,
|
||||
&summary,
|
||||
&check_summary,
|
||||
job_url.as_deref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -1520,8 +1546,12 @@ async fn maybe_post_git_sync_check(
|
||||
.as_deref()
|
||||
.map(|s| &s[..s.len().min(7)])
|
||||
.unwrap_or("latest");
|
||||
let job_row = job_url
|
||||
.as_deref()
|
||||
.map(|url| format!("\n| **Job** | [See the logs]({url}) |"))
|
||||
.unwrap_or_default();
|
||||
let body = format!(
|
||||
"{marker}\n### Windmill deploy preview\n\n| | |\n|---|---|\n| **Workspace** | `{workspace_id}` |\n| **Status** | {title} |\n| **Commit** | `{head}` |\n\n<details><summary>Details</summary>\n\n{summary}\n\n</details>"
|
||||
"{marker}\n### Windmill deploy preview\n\n| | |\n|---|---|\n| **Workspace** | `{workspace_id}` |\n| **Status** | {title} |\n| **Commit** | `{head}` |{job_row}\n\n<details><summary>Details</summary>\n\n{summary}\n\n</details>"
|
||||
);
|
||||
if let Err(e) = windmill_common::git_sync_ee::upsert_pr_comment(
|
||||
db,
|
||||
|
||||
Reference in New Issue
Block a user