better UX/UI

web/src/pages/admin/table_definition/logic.rs

@@ -375,12 +375,12 @@ pub(crate) async fn set_column_alias(
     .await
 }
 
-/// POST /admin/tables/presentation/order — SetColumnPresentation, moving one
-/// column past its neighbour.
+/// POST /admin/tables/presentation/order — SetColumnPresentation, saving the
+/// order staged by the browser.
 ///
 /// Every alias in the request is the name the backend just reported, so this
-/// write cannot rename a column even when the browser's copy of the table is
-/// stale. Only the order it sends comes from the form.
+/// write cannot rename a column. The ids from the browser must be an exact
+/// permutation of the current columns before their order is accepted.
 pub(crate) async fn set_column_order(
     State(state): State<AppState>,
     headers: HeaderMap,
@@ -399,33 +399,38 @@ pub(crate) async fn set_column_order(
         Ok(columns) => columns,
         Err(response) => return response,
     };
-    let Some(index) = columns
+    let current_ids = columns
         .iter()
-        .position(|column| column.column_id == form.column_id)
-    else {
-        let message = tr!(Locale::from_headers(&headers), "td-err-unknown-column");
+        .map(|column| column.column_id)
+        .collect::<std::collections::HashSet<_>>();
+    let submitted_ids = form
+        .column_ids
+        .iter()
+        .copied()
+        .collect::<std::collections::HashSet<_>>();
+    if form.column_ids.len() != columns.len()
+        || submitted_ids.len() != form.column_ids.len()
+        || submitted_ids != current_ids
+    {
+        let message = tr!(Locale::from_headers(&headers), "td-err-invalid-order");
         return refuse(state, headers, inputs, Page::Presentation, message).await;
-    };
-    // A column at the end of the table has nowhere further to go, and the
-    // button that says so is disabled; a request that asks anyway is answered
-    // with the table as it is.
-    let swap_with = match form.direction.as_str() {
-        "up" => index.checked_sub(1),
-        "down" if index + 1 < columns.len() => Some(index + 1),
-        _ => None,
-    };
-    let Some(swap_with) = swap_with else {
-        return respond(state, headers, inputs, Page::Presentation, StatusCode::OK).await;
-    };
+    }
 
-    let mut presentation = columns
+    let columns_by_id = columns
         .iter()
-        .map(|column| ColumnPresentation {
-            column_id: column.column_id,
-            alias: column.name.clone(),
+        .map(|column| (column.column_id, column))
+        .collect::<std::collections::HashMap<_, _>>();
+    let presentation = form
+        .column_ids
+        .iter()
+        .map(|column_id| {
+            let column = columns_by_id[column_id];
+            ColumnPresentation {
+                column_id: *column_id,
+                alias: column.name.clone(),
+            }
         })
-        .collect::<Vec<_>>();
-    presentation.swap(index, swap_with);
+        .collect();
 
     apply_presentation(
         state,

web/src/pages/admin/table_definition/mod.rs

@@ -38,10 +38,9 @@ pub(crate) fn router() -> Router<AppState> {
             "/admin/tables/columns/add/builder",
             post(logic::update_columns),
         )
-        // Naming a column and ordering the columns are one backend call but two
-        // forms, because a request that carries both is a request in which a
-        // stale alias can ride along with an unrelated edit. See
-        // `state::AliasForm`.
+        // Naming a column and ordering the columns are one backend call but
+        // separate forms, because a request that carries both lets a stale
+        // alias ride along with an unrelated edit. See `state::AliasForm`.
         .route("/admin/tables/presentation", get(logic::presentation_page))
         .route(
             "/admin/tables/presentation/alias",

web/src/pages/admin/table_definition/state.rs

@@ -201,11 +201,12 @@ pub(crate) struct AliasForm {
     pub alias: String,
 }
 
-/// Moving one column past its neighbour.
+/// Saving a complete, staged column order.
 ///
-/// It carries no alias at all -- not even the one it is moving -- so a reorder
-/// cannot rename anything, whatever the browser still had on screen. See
-/// [`AliasForm`] for why that separation is worth two forms.
+/// It carries ids only and no aliases, so a reorder cannot rename anything,
+/// whatever the browser still had on screen. The handler validates that this
+/// is an exact permutation of the table's current ids and supplies fresh
+/// aliases from the backend. See [`AliasForm`] for why that separation matters.
 #[derive(Clone, Debug, Default, serde::Deserialize)]
 pub(crate) struct OrderForm {
     #[serde(default)]
@@ -215,10 +216,7 @@ pub(crate) struct OrderForm {
     #[serde(default)]
     pub expected_row_version: i64,
     #[serde(default)]
-    pub column_id: i64,
-    /// `up` or `down`. Anything else moves nothing.
-    #[serde(default)]
-    pub direction: String,
+    pub column_ids: Vec<i64>,
 }
 
 /// The copy-profile panel. An empty `table_names` copies the whole profile,

web/src/pages/admin/table_definition/ui.rs

@@ -412,10 +412,9 @@ mod tests {
         assert!(!html.contains(r#"name="alias""#));
     }
 
-    /// Renaming and reordering are two forms, and the split is what the page
-    /// has to keep: an alias is posted with the id of the column it was typed
-    /// into, and the order form posts no alias at all, so moving a column
-    /// cannot carry a stale name along with it.
+    /// Renaming and reordering are separate forms. An alias is posted with its
+    /// column id, while the staged order posts ids only, so saving one cannot
+    /// carry stale values from the other.
     #[test]
     fn renaming_and_reordering_are_separate_forms() {
         let html = render_presentation_page(&page());
@@ -430,13 +429,14 @@ mod tests {
         );
         assert!(html.contains(r#"name="column_id" value="1""#), "{html}");
         assert!(html.contains(r#"name="expected_row_version" value="1""#), "{html}");
-        assert!(html.contains(r#"name="alias" value="number""#), "{html}");
-        assert!(html.contains(r#"name="direction" value="up""#), "{html}");
-        assert!(html.contains(r#"name="direction" value="down""#), "{html}");
+        assert!(html.contains(r#"name="alias" aria-label="New name: number""#), "{html}");
+        assert!(!html.contains(r#"name="alias" value="number""#), "{html}");
+        assert!(html.contains(r#"name="column_ids" value="1""#), "{html}");
+        assert!(html.contains(r#"data-order-reset disabled"#), "{html}");
+        assert!(html.contains(r#"data-order-save disabled"#), "{html}");
 
-        // The old fused form is gone: no list of ids paired positionally with a
-        // list of aliases, and no single save that posts both at once.
-        assert!(!html.contains(r#"name="column_ids""#), "{html}");
+        // The old fused form is gone: the order form carries no positionally
+        // paired alias list and no action that can mix the two operations.
         assert!(!html.contains(r#"name="aliases""#), "{html}");
         assert!(!html.contains(r#"name="action""#), "{html}");