This commit is contained in:
Priec
2026-08-15 18:31:51 +02:00
parent 56baa35cf2
commit 10d9784b9b
14 changed files with 404 additions and 111 deletions

View File

@@ -33,8 +33,8 @@ use crate::{
use super::{
loader::{self, load_page},
state::{
CopyForm, DeleteForm, GeneratedTableView, InvoiceTemplateForm, LoadError, PageInputs,
PresentationForm, Selection, TableDefinitionPageState,
AliasForm, CopyForm, DeleteForm, DetailColumn, GeneratedTableView, InvoiceTemplateForm,
LoadError, OrderForm, PageInputs, Selection, TableDefinitionPageState,
},
ui,
};
@@ -316,67 +316,164 @@ pub(crate) async fn add_columns(
}
}
/// POST /admin/tables/presentation — SetColumnPresentation.
pub(crate) async fn set_column_presentation(
/// POST /admin/tables/presentation/alias — SetColumnPresentation, renaming one
/// column.
///
/// The request the backend wants is the whole table, so the columns this write
/// is not about are filled in from a fresh read rather than from the browser.
/// That is the point of the split: the only thing the form contributes is the
/// alias and the id of the column it was typed into.
pub(crate) async fn set_column_alias(
State(state): State<AppState>,
headers: HeaderMap,
Form(form): Form<PresentationForm>,
Form(form): Form<AliasForm>,
) -> Response {
if let Some(rejection) = reject_cross_site(&headers) {
return rejection;
}
let mut inputs = PageInputs::for_selection(Selection {
let inputs = PageInputs::for_selection(Selection {
profile: form.profile.clone(),
table: form.table.clone(),
});
inputs.presentation = form.clone();
if form.column_ids.is_empty()
|| form.column_ids.len() != form.aliases.len()
|| form.aliases.iter().any(|alias| alias.trim().is_empty())
{
let message = tr!(
Locale::from_headers(&headers),
"td-err-choose-rename"
);
return refuse(
state,
headers,
inputs,
Page::Presentation,
message,
)
.await;
let alias = form.alias.trim().to_string();
if alias.is_empty() {
let message = tr!(Locale::from_headers(&headers), "td-err-choose-rename");
return refuse(state, headers, inputs, Page::Presentation, message).await;
}
let mut columns = form
.column_ids
let columns = match current_columns(&state, &headers, &inputs).await {
Ok(columns) => columns,
Err(response) => return response,
};
if !columns.iter().any(|column| column.column_id == form.column_id) {
let message = tr!(Locale::from_headers(&headers), "td-err-unknown-column");
return refuse(state, headers, inputs, Page::Presentation, message).await;
}
let presentation = columns
.iter()
.copied()
.zip(form.aliases.iter())
.map(|(column_id, alias)| ColumnPresentation {
column_id,
alias: alias.trim().to_string(),
.map(|column| ColumnPresentation {
column_id: column.column_id,
alias: if column.column_id == form.column_id {
alias.clone()
} else {
column.name.clone()
},
})
.collect();
apply_presentation(
state,
headers,
inputs,
form.profile,
form.table,
form.expected_row_version,
presentation,
)
.await
}
/// POST /admin/tables/presentation/order — SetColumnPresentation, moving one
/// column past its neighbour.
///
/// 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.
pub(crate) async fn set_column_order(
State(state): State<AppState>,
headers: HeaderMap,
Form(form): Form<OrderForm>,
) -> Response {
if let Some(rejection) = reject_cross_site(&headers) {
return rejection;
}
let inputs = PageInputs::for_selection(Selection {
profile: form.profile.clone(),
table: form.table.clone(),
});
let columns = match current_columns(&state, &headers, &inputs).await {
Ok(columns) => columns,
Err(response) => return response,
};
let Some(index) = columns
.iter()
.position(|column| column.column_id == form.column_id)
else {
let message = tr!(Locale::from_headers(&headers), "td-err-unknown-column");
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
.iter()
.map(|column| ColumnPresentation {
column_id: column.column_id,
alias: column.name.clone(),
})
.collect::<Vec<_>>();
if let Some((direction, index)) = form.action.split_once(':') {
if let Ok(index) = index.parse::<usize>() {
let other = match direction {
"up" => index.checked_sub(1),
"down" if index + 1 < columns.len() => Some(index + 1),
_ => None,
};
if let Some(other) = other {
columns.swap(index, other);
}
}
presentation.swap(index, swap_with);
apply_presentation(
state,
headers,
inputs,
form.profile,
form.table,
form.expected_row_version,
presentation,
)
.await
}
/// The table's columns as the backend has them now, in their current order.
///
/// Both presentation writes have to send every column, and the ones they are
/// not about must carry the name the server holds this moment -- not the name
/// the browser was showing when the page was drawn.
async fn current_columns(
state: &AppState,
headers: &HeaderMap,
inputs: &PageInputs,
) -> Result<Vec<DetailColumn>, Response> {
match load_page(state.clone(), headers, inputs.clone()).await {
Ok(page) => Ok(page.detail.map(|detail| detail.columns).unwrap_or_default()),
Err(error) => Err(load_error_response(headers, error)),
}
}
/// The half both writes share: send the presentation, answer with the page.
///
/// `expected_row_version` is the browser's, not the one the read above saw, so
/// a definition that changed under the user is still refused by the backend
/// rather than silently written over.
async fn apply_presentation(
state: AppState,
headers: HeaderMap,
inputs: PageInputs,
profile: String,
table: String,
expected_row_version: i64,
columns: Vec<ColumnPresentation>,
) -> Response {
let request = SetColumnPresentationRequest {
profile_name: form.profile.clone(),
table_name: form.table.clone(),
profile_name: profile,
table_name: table,
columns,
expected_row_version: form.expected_row_version,
expected_row_version,
};
let Ok(request) = authenticated_request(&headers, request) else {
return Redirect::to("/login").into_response();
@@ -385,28 +482,28 @@ pub(crate) async fn set_column_presentation(
let mut definitions = state.definitions.clone();
match definitions.set_column_presentation(request).await {
Ok(response) if response.get_ref().success => {
let mut inputs = inputs;
inputs.status = Some(response.into_inner().message);
inputs.presentation = PresentationForm {
profile: form.profile,
table: form.table,
..Default::default()
};
respond(state, headers, inputs, Page::Presentation, StatusCode::OK).await
}
Ok(response) => {
let message = response.into_inner().message;
let message = if message.is_empty() {
tr!(
Locale::from_headers(&headers),
"td-err-backend-no-rename"
)
tr!(Locale::from_headers(&headers), "td-err-backend-no-rename")
} else {
message
};
refuse(state, headers, inputs, Page::Presentation, message).await
}
Err(error) => {
refuse(state, headers, inputs, Page::Presentation, error.message().to_string()).await
refuse(
state,
headers,
inputs,
Page::Presentation,
error.message().to_string(),
)
.await
}
}
}

View File

@@ -38,9 +38,18 @@ 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`.
.route("/admin/tables/presentation", get(logic::presentation_page))
.route(
"/admin/tables/presentation",
get(logic::presentation_page).post(logic::set_column_presentation),
"/admin/tables/presentation/alias",
post(logic::set_column_alias),
)
.route(
"/admin/tables/presentation/order",
post(logic::set_column_order),
)
.route("/admin/tables/delete", get(logic::delete_page))
.route("/admin/tables/delete", post(logic::delete_table))

View File

@@ -173,10 +173,18 @@ pub(crate) struct GeneratedTableView {
pub parent_table_name: String,
}
/// The rename panel's inputs, kept across a failed submit so the user does not
/// retype them.
/// Renaming one column.
///
/// `SetColumnPresentation` sets names and order in one call and insists on
/// being given every column, so a browser that posts the whole table posts a
/// list it read at some earlier moment. When the ids and the aliases came from
/// two different moments, they still zipped into a valid request -- one that
/// renamed `amount` to `note` and `note` to `amount`, and took every validation
/// and type along with the names. This form carries one alias and the id of the
/// column it was typed into, so there is no list to misalign; the handler fills
/// the rest of the table in from the backend.
#[derive(Clone, Debug, Default, serde::Deserialize)]
pub(crate) struct PresentationForm {
pub(crate) struct AliasForm {
#[serde(default)]
pub profile: String,
#[serde(default)]
@@ -184,11 +192,29 @@ pub(crate) struct PresentationForm {
#[serde(default)]
pub expected_row_version: i64,
#[serde(default)]
pub column_ids: Vec<i64>,
pub column_id: i64,
#[serde(default)]
pub aliases: Vec<String>,
pub alias: String,
}
/// Moving one column past its neighbour.
///
/// 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.
#[derive(Clone, Debug, Default, serde::Deserialize)]
pub(crate) struct OrderForm {
#[serde(default)]
pub action: String,
pub profile: String,
#[serde(default)]
pub table: String,
#[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,
}
/// The copy-profile panel. An empty `table_names` copies the whole profile,
@@ -248,7 +274,6 @@ pub(crate) struct PageInputs {
pub selection: Selection,
/// The columns staged for `AddTableColumns`.
pub columns: ColumnDraft,
pub presentation: PresentationForm,
pub copy: CopyForm,
pub invoice: InvoiceTemplateForm,
pub status: Option<String>,

View File

@@ -340,7 +340,7 @@ mod tests {
assert!(html.contains("/admin/tables/delete"));
assert!(html.contains("Type <code>invoice</code> to confirm"));
// The other writes are links in the switcher, not forms on the page.
assert!(!html.contains(r#"hx-post="/admin/tables/presentation""#));
assert!(!html.contains(r#"hx-post="/admin/tables/presentation/alias""#));
assert!(!html.contains("/admin/profiles/copy?profile=billing\" method"));
}
@@ -404,28 +404,54 @@ mod tests {
assert!(!html.contains(r#"id="column-form""#));
let html = render_presentation_page(&state);
assert!(!html.contains(r#"name="aliases""#));
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.
#[test]
fn column_presentation_posts_stable_ids_aliases_and_order_controls() {
fn renaming_and_reordering_are_separate_forms() {
let html = render_presentation_page(&page());
assert!(html.contains(r#"hx-post="/admin/tables/presentation""#), "{html}");
assert!(html.contains(r#"name="column_ids" value="1""#), "{html}");
assert!(
html.contains(r#"hx-post="/admin/tables/presentation/alias""#),
"{html}"
);
assert!(
html.contains(r#"hx-post="/admin/tables/presentation/order""#),
"{html}"
);
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="aliases" value="number""#), "{html}");
assert!(html.contains(r#"name="action" value="save""#), "{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}");
// 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}");
assert!(!html.contains(r#"name="aliases""#), "{html}");
assert!(!html.contains(r#"name="action""#), "{html}");
// And the order form's own markup names no alias.
let order_form = html
.split(r#"hx-post="/admin/tables/presentation/order""#)
.nth(1)
.and_then(|rest| rest.split("</form>").next())
.expect("the order form is rendered");
assert!(!order_form.contains(r#"name="alias""#), "{order_form}");
}
#[test]
fn adding_columns_and_presentation_are_separate_pages() {
let add_html = render_add_columns_page(&page());
assert!(add_html.contains(r#"id="column-form""#), "{add_html}");
assert!(!add_html.contains(r#"name="aliases""#), "{add_html}");
assert!(!add_html.contains(r#"name="alias""#), "{add_html}");
let presentation_html = render_presentation_page(&page());
assert!(presentation_html.contains(r#"name="aliases""#), "{presentation_html}");
assert!(presentation_html.contains(r#"name="alias""#), "{presentation_html}");
assert!(!presentation_html.contains(r#"id="column-form""#), "{presentation_html}");
}

View File

@@ -6,11 +6,10 @@ use crate::{i18n::Locale, tr};
use crate::definitions::table_structure::TableStructureResponse;
/// The columns a CSV carries, for an export and for the import that reads one
/// back.
/// The columns an import can write, and the export's default header.
///
/// Both ends use this list, so a file the export writes is a file the import
/// accepts. That only holds if every system column is left out: a row is
/// Both ends use this list, so a file the default export writes is a file the
/// import accepts. That only holds if every system column is left out: a row is
/// inserted with `post_table_data`, which takes user columns and nothing else,
/// so exporting `row_revision` or `created_at` produced a file whose own
/// re-import the server answered with `Invalid column`. The names come from
@@ -25,16 +24,48 @@ pub(crate) fn exportable_columns(schema: &TableStructureResponse) -> Vec<String>
.collect()
}
/// Every column of a row the server will actually hand over: the user's
/// columns, plus `id`, `deleted` and `row_revision`. This is what the export
/// writes when asked for the whole row.
///
/// "Everything" is bounded twice, and both bounds are the server's. Internal
/// columns -- the link version columns, `version`, `closed_by_user_id` -- are
/// dropped inside `get_table_structure` and never reach this crate. And
/// `created_at` is in the structure but not in the row: a read selects the
/// leading system columns and the user's, so asking for it would write a header
/// over a column of blanks. Filtering by what a read returns keeps the file
/// honest about that.
///
/// Such a file is a record of the table, not a file to load back: the system
/// columns in it are the server's to write, and an import ignores them.
pub(crate) fn all_columns(schema: &TableStructureResponse) -> Vec<String> {
schema
.columns
.iter()
.filter(|column| !is_read_omitted_column(&column.name))
.map(|column| column.name.clone())
.collect()
}
/// Whether `name` is one of the columns the server puts on every managed
/// table. The virtual `account` name is deliberately not checked: it is an
/// alias a user may write to, not a column the server fills in.
fn is_system_column(name: &str) -> bool {
pub(crate) fn is_system_column(name: &str) -> bool {
crate::system_column::LEADING_SYSTEM_COLUMNS
.iter()
.chain(crate::system_column::TRAILING_SYSTEM_COLUMNS.iter())
.any(|column| column.name == name)
}
/// Whether the structure lists `name` but a row read leaves it out. The
/// trailing system columns are declared after the user's and are not part of
/// what `get_table_data` selects.
fn is_read_omitted_column(name: &str) -> bool {
crate::system_column::TRAILING_SYSTEM_COLUMNS
.iter()
.any(|column| column.name == name)
}
pub(crate) fn column_types(schema: &TableStructureResponse) -> HashMap<String, String> {
schema
.columns
@@ -115,4 +146,30 @@ mod tests {
assert_eq!(exportable_columns(&schema), vec!["number".to_string()]);
}
/// The whole-row export keeps the system columns the server hands over,
/// and drops `created_at`, which the structure lists but a read does not
/// return.
#[test]
fn the_whole_row_is_what_a_read_returns() {
let schema = TableStructureResponse {
columns: vec![
column("id", true),
column("deleted", false),
column("row_revision", false),
column("number", false),
column("created_at", false),
],
};
assert_eq!(
all_columns(&schema),
vec![
"id".to_string(),
"deleted".to_string(),
"row_revision".to_string(),
"number".to_string(),
]
);
}
}

View File

@@ -19,7 +19,7 @@ use super::{
super::common::{
csv::write_record,
loader::LoadError,
schema::exportable_columns,
schema::{all_columns, exportable_columns},
},
loader::load_page,
state::ExportForm,
@@ -55,6 +55,14 @@ pub(crate) async fn export_csv(
Ok(targets) => targets,
Err(message) => return reject(&headers, message),
};
// Everything the structure exposes, or only what an import could write
// back. The first is a record of the table, the second a file that round
// trips.
let columns_of = if form.include_system_columns() {
all_columns
} else {
exportable_columns
};
let Some(profile) = catalog.profiles.iter().find(|profile| profile.name == profile_name) else {
return reject(
&headers,
@@ -118,7 +126,7 @@ pub(crate) async fn export_csv(
&tr!(Locale::from_headers(&headers), "export-err-negative-count"),
);
};
tables.push(ExportTable { name: table_name.clone(), columns: exportable_columns(&structure), count });
tables.push(ExportTable { name: table_name.clone(), columns: columns_of(&structure), count });
}
let mut csv = String::new();

View File

@@ -6,6 +6,10 @@ pub(crate) struct ExportForm {
pub profile_name: String,
#[serde(default)]
pub table_names: String,
/// An unchecked checkbox is not posted at all, so its absence is the
/// `false` and any value it does carry is the `true`.
#[serde(default)]
pub include_system_columns: Option<String>,
}
pub(crate) struct ExportPageState {
@@ -31,4 +35,9 @@ impl ExportForm {
}
Ok((profile.to_string(), tables))
}
/// Whether the download carries the system columns as well.
pub(crate) fn include_system_columns(&self) -> bool {
self.include_system_columns.is_some()
}
}

View File

@@ -21,7 +21,7 @@ use super::{
super::common::{
csv::parse_csv,
loader::LoadError,
schema::{column_types, csv_value, exportable_columns},
schema::{column_types, csv_value, exportable_columns, is_system_column},
},
loader::load_page,
state::ImportForm,
@@ -134,6 +134,12 @@ pub(crate) async fn import_csv(
}
for (index, column) in columns.iter().enumerate() {
let belongs = table_headers.as_ref().map_or(table_names.len() == 1, |headers| headers.get(index).is_some_and(|name| name == &table.name));
// A system column is the server's to write, so a file that carries
// one -- an export taken with the system columns included -- loads
// with that column left where it is, not refused.
if belongs && is_system_column(column) {
continue;
}
if belongs && !table.columns.contains(column) {
return reject(
&headers,