From 1fab283cf9c0d61b397654e954365fe6dd4b9f77 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 14 Oct 2026 13:47:21 +0200 Subject: [PATCH] let a legacy migration whose comment reads like a scope get a down (#11765) * fix: let a legacy migration whose comment reads like a scope get a down Co-Authored-By: Claude Opus 5.5 * test: pin adding a down to a migration whose comment predates scopes Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- backend/tests/datatable_migration_add_down.rs | 31 +++++++++++++++++++ .../src/datatable_migrations.rs | 11 ++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/backend/tests/datatable_migration_add_down.rs b/backend/tests/datatable_migration_add_down.rs index 47ea4c253b..1259628068 100644 --- a/backend/tests/datatable_migration_add_down.rs +++ b/backend/tests/datatable_migration_add_down.rs @@ -174,3 +174,34 @@ async fn test_add_down_to_a_migration_whose_comment_predates_role_annotations( assert_eq!(resp.status(), 200, "{}", resp.text().await?); Ok(()) } + +/// Likewise for `-- schemas`: the scope annotation is held only to up SQL being written, so a +/// definition whose stored up opens with prose starting with the word still accepts its down. +#[sqlx::test(fixtures("datatable_migrations_grants"))] +async fn test_add_down_to_a_migration_whose_comment_predates_scope_annotations( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + setup_applied_migration_without_down(&db).await?; + let code_up = format!("-- Schemas: changes for the widgets feature\n{CODE_UP}"); + sqlx::query("UPDATE datatable_migrations SET code_up = $1 WHERE workspace_id = 'dtmig-ws'") + .bind(&code_up) + .execute(&db) + .await?; + + let server = ApiServer::start(db.clone()).await?; + let resp = authed(reqwest::Client::new().post(format!( + "http://localhost:{}/api/w/dtmig-ws/workspaces/upsert_datatable_migration/main", + server.addr.port() + ))) + .json(&json!({ + "timestamp": VERSION, + "name": "create_widgets", + "code_up": code_up, + "code_down": CODE_DOWN, + })) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + Ok(()) +} diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index f3ad81de36..839b449f37 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -1741,7 +1741,16 @@ async fn upsert_datatable_migration( if unchanged { return Ok(upserted_message); } - let scope = parse_migration_schemas(&payload.code_up)?; + // Only up SQL being written is held to the annotation: a stored up that opens with prose + // starting with the word `schemas` predates scopes, and must still accept a down added to it. + let up_changed = existing + .as_ref() + .is_none_or(|existing| existing.code_up != payload.code_up); + let scope = if up_changed { + parse_migration_schemas(&payload.code_up)? + } else { + None + }; ensure_may_edit_migration( &db, &w_id,