mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: app history lists in deployed order, so the picker numbers it right
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
dbb63cfb36
commit
a96cf4b912
+2
-2
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT a.id as app_id, av.id as version_id, dm.deployment_msg as deployment_msg,\n av.created_by as created_by, av.created_at as created_at\n FROM app a LEFT JOIN app_version av ON a.id = av.app_id LEFT JOIN deployment_metadata dm ON av.id = dm.app_version\n WHERE a.workspace_id = $1 AND a.path = $2\n ORDER BY av.created_at DESC",
|
||||
"query": "SELECT a.id as app_id, av.id as version_id, dm.deployment_msg as deployment_msg,\n av.created_by as created_by, av.created_at as created_at\n FROM app a LEFT JOIN app_version av ON a.id = av.app_id LEFT JOIN deployment_metadata dm ON av.id = dm.app_version\n WHERE a.workspace_id = $1 AND a.path = $2\n ORDER BY array_position(a.versions, av.id) DESC NULLS LAST, av.id DESC",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
@@ -43,5 +43,5 @@
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "1b1607e9933f8f843d05e6a1601712d042bb3fd6454802bc686c4fd1dd47a4e6"
|
||||
"hash": "c8b369881add5574c1762431e2fb58eb04892babb4684be314f369227424e640"
|
||||
}
|
||||
@@ -0,0 +1,102 @@
|
||||
//! The deployed order of an app's versions is the order they were appended to
|
||||
//! `app.versions`, not the order of their `created_at`.
|
||||
//!
|
||||
//! `app_version.created_at` defaults to `now()`, which in Postgres is the
|
||||
//! transaction's start time, while the append happens under the app row's lock.
|
||||
//! Two deploys that overlap therefore land in one order and carry timestamps in
|
||||
//! the other. The head the editor guards against, and the sequence the diff
|
||||
//! picker numbers, both have to follow the array.
|
||||
//!
|
||||
//! Users from the `base` fixture: test-user (admin, token SECRET_TOKEN).
|
||||
|
||||
use serde_json::json;
|
||||
use sqlx::{Pool, Postgres};
|
||||
use windmill_test_utils::*;
|
||||
|
||||
const TOKEN: &str = "SECRET_TOKEN";
|
||||
|
||||
#[sqlx::test(fixtures("base"))]
|
||||
async fn test_app_head_follows_the_append_order_not_the_timestamps(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let ws = format!(
|
||||
"http://localhost:{}/api/w/test-workspace",
|
||||
server.addr.port()
|
||||
);
|
||||
let client = reqwest::Client::new();
|
||||
|
||||
let res = client
|
||||
.post(format!("{ws}/apps/create"))
|
||||
.header("Authorization", format!("Bearer {TOKEN}"))
|
||||
.json(&json!({
|
||||
"path": "u/test-user/order_app",
|
||||
"summary": "ordered",
|
||||
"value": {},
|
||||
"policy": { "execution_mode": "publisher", "triggerables": {} }
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
assert!(res.status().is_success(), "{}", res.text().await?);
|
||||
|
||||
let first: i64 = sqlx::query_scalar!(
|
||||
"SELECT versions[array_upper(versions, 1)] FROM app
|
||||
WHERE workspace_id = 'test-workspace' AND path = 'u/test-user/order_app'"
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?
|
||||
.expect("the created app has a version");
|
||||
|
||||
// The overlapping deploy: appended after `first`, so it is the version that
|
||||
// landed, but stamped before it, so a timestamp sort puts it underneath.
|
||||
let second: i64 = sqlx::query_scalar!(
|
||||
"INSERT INTO app_version (app_id, value, created_by, created_at, raw_app)
|
||||
SELECT app_id, value, 'racer', created_at - interval '1 hour', raw_app
|
||||
FROM app_version WHERE id = $1
|
||||
RETURNING id",
|
||||
first
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
sqlx::query!(
|
||||
"UPDATE app SET versions = array_append(versions, $1::bigint)
|
||||
WHERE workspace_id = 'test-workspace' AND path = 'u/test-user/order_app'",
|
||||
second
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
|
||||
let head: serde_json::Value = client
|
||||
.get(format!(
|
||||
"{ws}/apps/get_latest_version/u/test-user/order_app"
|
||||
))
|
||||
.header("Authorization", format!("Bearer {TOKEN}"))
|
||||
.send()
|
||||
.await?
|
||||
.json()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
head["version"], second,
|
||||
"the head is the version appended last, not the newest timestamp: {head}"
|
||||
);
|
||||
|
||||
let history: Vec<serde_json::Value> = client
|
||||
.get(format!("{ws}/apps/history/p/u/test-user/order_app"))
|
||||
.header("Authorization", format!("Bearer {TOKEN}"))
|
||||
.send()
|
||||
.await?
|
||||
.json()
|
||||
.await?;
|
||||
let listed: Vec<i64> = history
|
||||
.iter()
|
||||
.map(|v| v["version"].as_i64().unwrap())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
listed,
|
||||
vec![second, first],
|
||||
"the picker numbers the history by deployed order, so it leads with the head"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -1241,12 +1241,18 @@ async fn get_app_history(
|
||||
let path = path.to_path();
|
||||
check_scopes(&authed, || format!("apps:read:{}", &path))?;
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
// Newest first in the order the versions were deployed, which the picker numbers
|
||||
// (`v1`, `v2`, …) and reads the head off. That is the position in `app.versions`,
|
||||
// not `created_at`: the latter is the deploying transaction's start time, so two
|
||||
// that overlap can carry it in the opposite order from the one they landed in. A
|
||||
// version absent from the array (restored from trash, copied by a fork) never sat
|
||||
// in that sequence, so it trails the ones that did.
|
||||
let query_result = sqlx::query!(
|
||||
"SELECT a.id as app_id, av.id as version_id, dm.deployment_msg as deployment_msg,
|
||||
av.created_by as created_by, av.created_at as created_at
|
||||
FROM app a LEFT JOIN app_version av ON a.id = av.app_id LEFT JOIN deployment_metadata dm ON av.id = dm.app_version
|
||||
WHERE a.workspace_id = $1 AND a.path = $2
|
||||
ORDER BY av.created_at DESC",
|
||||
ORDER BY array_position(a.versions, av.id) DESC NULLS LAST, av.id DESC",
|
||||
w_id,
|
||||
path,
|
||||
).fetch_all(&mut *tx).await?;
|
||||
|
||||
Reference in New Issue
Block a user