web fixes

This commit is contained in:
Priec
2026-08-14 18:46:31 +02:00
parent 0e6207fb44
commit 1c3d292479
13 changed files with 1458 additions and 105 deletions

View File

@@ -13,16 +13,15 @@
use crate::{
definitions::table_definition::{GeneratedColumnAlias, PostTableDefinitionRequest},
schema::{ColumnCatalog, ColumnDraft, proto_columns, validate_identifier},
schema::{
ColumnCatalog, ColumnDraft, proto_columns, validate_identifier, validate_table_name,
},
};
/// The compound type that also brings a system column with it. Which columns
/// it generates is the catalog's answer; the physical `account_id` is not one
/// of them, and [`TableDraft::preview_rows`] is where that is explained.
pub(crate) const ACCOUNTING_FIELD_TYPE: &str = "accounting";
/// The compound type whose connectors remain tied to their backend names.
pub(crate) const ACCOUNTING_TRANSFER_FIELD_TYPE: &str = "accounting_transfer";
pub(crate) use crate::schema::{ACCOUNTING_FIELD_TYPE, ACCOUNTING_TRANSFER_FIELD_TYPE};
/// The virtual field the ACCOUNTING foreign key is written as. It is a column
/// like any other once created, so it can be aliased too.
@@ -143,17 +142,9 @@ impl TableDraft {
self.row_display_columns.clear();
return;
}
// A compound column leaves no column of its own name behind, so it can
// never identify a row; `row_display_candidates` does not offer one.
if !self.columns.is_indexable(index - 1) {
return;
}
let Some(column) = self
.columns
.added
.get(index - 1)
.map(|column| column.name.clone())
else {
// Whatever is not on the candidate list is not the user's to choose,
// so a crafted post naming a definition row changes nothing.
let Some(column) = self.row_display_column_names().get(index - 1).cloned() else {
return;
};
match self
@@ -168,6 +159,38 @@ impl TableDraft {
}
}
/// The columns a row may be identified by, in the order the table declares
/// them.
///
/// Every column the table will really hold: the declared ones, and what the
/// definition rows among them expand into — under the names they were
/// aliased to, which is what the table will carry. A definition row itself
/// is not one, having no column of its own name.
pub(crate) fn row_display_column_names(&self) -> Vec<String> {
let mut names = Vec::new();
for (index, column) in self.columns.added.iter().enumerate() {
if self.columns.can_identify_row(index) {
names.push(column.name.clone());
}
let generated = self.columns.generated_columns_of(index);
let account_reported = generated
.iter()
.any(|generated| generated.name == ACCOUNT_API_COLUMN);
names.extend(
generated
.iter()
.map(|generated| self.generated_display_name(&generated.name)),
);
// The account link is a column of the table like the rest of
// ACCOUNTING's companions, whether or not the catalog reports it
// among them.
if column.data_type == ACCOUNTING_FIELD_TYPE && !account_reported {
names.push(self.generated_display_name(ACCOUNT_API_COLUMN));
}
}
names
}
/// Records the tables the target profile offers as link targets. A table
/// cannot link to itself, so its own name is never among them.
#[cfg(test)]
@@ -316,6 +339,7 @@ impl TableDraft {
let mut taken: Vec<String> = vec![
"id".to_string(),
"deleted".to_string(),
"row_revision".to_string(),
"created_at".to_string(),
];
taken.extend(self.columns.added.iter().map(|column| column.name.clone()));
@@ -388,6 +412,15 @@ impl TableDraft {
option: "default false".to_string(),
source: "system".to_string(),
},
// Every managed table carries one, and the preview claims to be the
// schema as it will exist.
PreviewRow {
mark: String::new(),
column: "row_revision".to_string(),
data_type: "BIGINT".to_string(),
option: "not null, default 1".to_string(),
source: "system".to_string(),
},
];
for (index, column) in self.columns.added.iter().enumerate() {
@@ -462,14 +495,24 @@ impl TableDraft {
if let Some(error) = validate_accounting_currency(self) {
return Err(error);
}
if let Some(error) = validate_identifier(self.table_name.trim(), "Table name", true) {
if let Some(error) = validate_table_name(self.table_name.trim()) {
return Err(error);
}
if self.table_name_conflicts() {
return Err(format!(
"A table named `{}` already exists in profile `{}`.",
self.table_name, profile_name
));
// A shared table lands in every profile at once, so its name has to
// be free in all of them — and naming a profile in that message
// would name the wrong thing, there being none.
return Err(if self.global {
format!(
"A table named `{}` already exists. A shared table's name has to be free in every profile.",
self.table_name
)
} else {
format!(
"A table named `{}` already exists in profile `{profile_name}`, or is shared by every profile.",
self.table_name
)
});
}
if self.columns.is_empty() {
return Err("Add at least one column before saving.".to_string());
@@ -530,6 +573,7 @@ mod tests {
data_type: data_type.to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: if matches!(data_type, "money" | "accounting") {
"EUR".to_string()
@@ -648,6 +692,7 @@ mod tests {
data_type: "text".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: String::new(),
});
@@ -685,7 +730,15 @@ mod tests {
let mut draft = draft_with_column("total", "int");
draft.existing_profile_tables = vec!["invoice".to_string()];
assert!(draft.validate().is_err());
let error = draft.validate().unwrap_err();
assert!(error.contains("profile `billing`"), "{error}");
// A shared table has no profile to name, and its name has to be free
// everywhere rather than in one place.
draft.global = true;
let error = draft.validate().unwrap_err();
assert!(!error.contains("profile ``"), "{error}");
assert!(error.contains("every profile"), "{error}");
}
#[test]
@@ -721,6 +774,7 @@ mod tests {
data_type: "date".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: String::new(),
});
@@ -735,24 +789,49 @@ mod tests {
assert!(draft.row_display_columns.is_empty());
}
/// A compound column expands into schema-managed companions, so there is
/// no column of that name for a row to be identified by — the builder does
/// not offer it, and a crafted post cannot choose it either.
/// A definition row expands into schema-managed companions, so there is no
/// column of *its* name for a row to be identified by — but the columns it
/// expands into are columns like any other, and the server takes them here.
/// Offering only the declared ones is what left a table whose only readable
/// column was generated with nothing to be shown by.
#[test]
fn a_compound_column_never_identifies_a_row() {
fn a_row_is_identified_by_the_columns_the_table_really_holds() {
let mut draft = draft_with_column("number", "text");
draft.columns.added.push(ColumnDefinition {
name: "accounting".to_string(),
data_type: "accounting".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: "EUR".to_string(),
});
draft.toggle_row_display_candidate(2);
// The declared column, then what the definition row generates — and
// never the definition row itself.
assert_eq!(
draft.row_display_column_names(),
["number", "name", "tax_point_date", "debit", "credit", "account"]
);
assert!(!draft.row_display_column_names().contains(&"accounting".to_string()));
assert!(draft.row_display_columns.is_empty());
// `name` is the second candidate, and choosing it names the generated
// column rather than the row it came from.
draft.toggle_row_display_candidate(2);
assert_eq!(draft.row_display_columns, vec!["name"]);
// Under the name it was aliased to, which is what the table will carry.
draft.toggle_row_display_candidate(2);
draft.generated_aliases = vec![GeneratedAlias {
source: "name".to_string(),
alias: "popis".to_string(),
}];
draft.toggle_row_display_candidate(2);
assert_eq!(draft.row_display_columns, vec!["popis"]);
// Past the end of the candidate list nothing is chosen.
draft.toggle_row_display_candidate(99);
assert_eq!(draft.row_display_columns, vec!["popis"]);
}
#[test]
@@ -775,7 +854,10 @@ mod tests {
.map(|row| row.column.as_str())
.collect::<Vec<_>>();
assert_eq!(columns, vec!["id", "deleted", "number", "created_at"]);
assert_eq!(
columns,
vec!["id", "deleted", "row_revision", "number", "created_at"]
);
// No display column chosen, so `id` identifies the row.
assert_eq!(rows[0].mark, "[x]");
}
@@ -791,6 +873,7 @@ mod tests {
data_type: "date".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: String::new(),
});
@@ -821,6 +904,7 @@ mod tests {
data_type: "accounting".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Rounded,
currency: "CZK".to_string(),
});
@@ -836,6 +920,7 @@ mod tests {
vec![
"id",
"deleted",
"row_revision",
"number",
"accounting",
"name",
@@ -871,4 +956,98 @@ mod tests {
assert_eq!(draft.into_request().unwrap().indexes, vec!["number"]);
}
/// The request the builder sends is one the server will take: a link
/// carries no index of the request's own — the server makes that one — and
/// `required` travels with the column it belongs to.
#[test]
fn the_request_names_no_link_among_its_indexes_and_carries_required() {
let mut draft = draft_with_column("number", "text");
draft.columns.added[0].required = true;
draft.columns.added.push(ColumnDefinition {
name: "billing_customer".to_string(),
data_type: "link(customer)".to_string(),
// As a tampered post would have it.
indexed: true,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: String::new(),
});
// Refused rather than quietly repaired, because the server refuses it.
let error = draft.clone().into_request().unwrap_err();
assert!(error.contains("indexed automatically"), "{error}");
draft.columns.added[1].indexed = false;
draft.columns.toggle_indexed(0);
let request = draft.into_request().unwrap();
assert_eq!(request.indexes, vec!["number"]);
assert!(request.columns[0].required);
assert!(!request.columns[1].required);
}
/// A table name has less room than a column name, and the names every
/// profile's own tables carry are not free.
#[test]
fn a_table_name_is_held_to_the_rules_a_table_name_has() {
let mut draft = draft_with_column("total", "int");
draft.table_name = "t".repeat(39);
let error = draft.validate().unwrap_err();
assert!(error.contains("38 characters"), "{error}");
draft.table_name = "accounts".to_string();
assert!(draft.validate().is_err());
draft.table_name = "general_ledger".to_string();
assert!(draft.validate().is_err());
draft.table_name = "invoice".to_string();
assert!(draft.validate().is_ok());
}
/// A shared table has no books of its own, so the two definition rows that
/// post to a profile's books are refused on one.
#[test]
fn a_shared_table_refuses_the_columns_that_belong_to_one_profile() {
let mut draft = draft_with_column("accounting", "accounting");
draft.global = true;
draft.columns.global = true;
assert!(draft.validate().is_err());
draft.columns.added.clear();
draft.columns.added.push(ColumnDefinition {
name: "quantity".to_string(),
data_type: "int".to_string(),
indexed: false,
quantity_ledger: true,
required: false,
money_mode: MoneyMode::Exact,
currency: String::new(),
});
assert!(draft.validate().is_err());
}
/// The preview is the schema as it will exist, so it carries every system
/// column the server puts on a managed table — including the one an alias
/// may therefore not take.
#[test]
fn the_preview_and_the_aliases_know_every_system_column() {
let draft = draft_with_column("number", "text");
assert!(
draft
.preview_rows()
.iter()
.any(|row| row.column == "row_revision" && row.source == "system")
);
let mut draft = draft_with_column("accounting", "accounting");
draft.generated_aliases = vec![GeneratedAlias {
source: "name".to_string(),
alias: "row_revision".to_string(),
}];
assert!(draft.validate_generated_aliases().is_err());
}
}

View File

@@ -54,6 +54,12 @@ pub(crate) async fn load_page(
.column_types,
);
// The panel holds a column to the rules of the table it is being described
// for, so it is told what that table is on every render rather than left to
// read a copy that a `refresh` could have moved on from.
draft.columns.global = draft.global;
draft.columns.table_name = draft.table_name.trim().to_ascii_lowercase();
let tree = definitions
.get_profile_tree(
authenticated_request(headers, Empty {}).map_err(|_| LoadError::Unauthenticated)?,
@@ -88,7 +94,7 @@ pub(crate) async fn load_page(
.into_iter()
// The profile's ledger accounts are not a link target of their own: an
// ACCOUNTING column is how a row is posted to one.
.filter(|table| table.name != "accounts")
.filter(|table| table.name != crate::schema::ACCOUNTS_TABLE)
.map(|table| RelationTableOption {
name: table.name,
global: table.global,

View File

@@ -176,6 +176,16 @@ fn apply_action(page: &mut AddTablePageState, form: &BuilderForm) {
},
"toggle-index" => page.draft.columns.toggle_indexed(index),
"toggle-display" => page.draft.toggle_row_display_candidate(index),
// Picking a scope or a type only changes which fields apply, so there
// is nothing to do — except when the change has just made a column
// already in the list impossible. Switching to the shared scope is the
// one that does: the picker stops offering the columns that post to a
// profile's books, and one added before the switch is still there.
"refresh" => {
if let Err(message) = page.draft.columns.validate() {
page.error = Some(message);
}
}
_ => {}
}
}
@@ -197,3 +207,57 @@ fn load_error_response(error: LoadError) -> Response {
.into_response(),
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::schema::{ColumnDefinition, MoneyMode};
fn page_with_accounting() -> AddTablePageState {
let mut draft = TableDraft::new();
draft.columns.catalog = crate::schema::tests::catalog();
draft.profile_name = "billing".to_string();
draft.table_name = "invoice".to_string();
draft.columns.added.push(ColumnDefinition {
name: "accounting".to_string(),
data_type: "accounting".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: "EUR".to_string(),
});
AddTablePageState {
nav: crate::ui::Nav::default(),
profiles: vec!["billing".to_string()],
draft,
status: None,
error: None,
}
}
fn refresh() -> BuilderForm {
BuilderForm {
action: "refresh".to_string(),
..BuilderForm::default()
}
}
/// Switching an ACCOUNTING draft to the shared scope says so at once. Left
/// to the save, the column stays in a list whose picker no longer offers it
/// and the reason only arrives after the Create button.
#[test]
fn switching_to_the_shared_scope_reports_a_column_that_cannot_come_along() {
let mut page = page_with_accounting();
apply_action(&mut page, &refresh());
assert!(page.error.is_none(), "nothing is wrong yet");
// What the loader does with the posted scope before the action runs.
page.draft.global = true;
page.draft.columns.global = true;
apply_action(&mut page, &refresh());
let error = page.error.expect("the switch is refused out loud");
assert!(error.contains("shared table"), "{error}");
}
}

View File

@@ -61,6 +61,8 @@ pub(crate) struct BuilderForm {
#[serde(default)]
pub column_quantity_ledger_input: String,
#[serde(default)]
pub column_required_input: String,
#[serde(default)]
pub column_rounding_input: String,
#[serde(default)]
pub column_currency_input: String,
@@ -75,6 +77,8 @@ pub(crate) struct BuilderForm {
#[serde(default)]
pub column_quantity_ledger: Vec<String>,
#[serde(default)]
pub column_required: Vec<String>,
#[serde(default)]
pub column_rounding: Vec<String>,
#[serde(default)]
pub column_currencies: Vec<String>,
@@ -113,6 +117,7 @@ impl BuilderForm {
decimal_scale_input: self.decimal_scale_input.clone(),
indexing_input: self.column_indexing_input.clone(),
quantity_ledger_input: self.column_quantity_ledger_input.clone(),
required_input: self.column_required_input.clone(),
rounding_input: self.column_rounding_input.clone(),
currency_input: self.column_currency_input.clone(),
added: columns_from_rows(
@@ -120,6 +125,7 @@ impl BuilderForm {
&self.column_types,
&self.column_indexed,
&self.column_quantity_ledger,
&self.column_required,
&self.column_rounding,
&self.column_currencies,
),
@@ -129,6 +135,12 @@ impl BuilderForm {
// The table is being created here, so the creation-only types are
// on the table.
creating_table: true,
// What the table will be, so the panel can hold a column to the
// rules of the table it is being described for: a shared table
// takes no column that posts to one profile's books, and no link
// may point at the table being created.
global: self.global,
table_name: self.table_name.trim().to_ascii_lowercase(),
};
// Drop display columns whose column is gone, so a stale post cannot
@@ -206,12 +218,17 @@ impl AddTablePageState {
let mut rows = Vec::new();
for (index, column) in columns.added.iter().enumerate() {
// Whether it is indexed has a column of its own, so it is not
// repeated here.
let mut tags = Vec::new();
if column.required {
tags.push("required".to_string());
}
if column.quantity_ledger {
tags.push("quantity ledger".to_string());
}
if !column.option_label().is_empty() {
tags.push(column.option_label());
if !column.currency.is_empty() {
tags.push(format!("{}, {}", column.currency, column.money_mode.label()));
}
rows.push(ColumnRow {
index: Some(index),
@@ -220,7 +237,9 @@ impl AddTablePageState {
name: column.name.clone(),
data_type: column.data_type.clone(),
indexable: columns.is_indexable(index),
indexed: column.indexed,
// A link is indexed whether or not anyone asked: the server
// builds an index for every foreign key.
indexed: column.is_indexed(),
tags,
alias_source: None,
alias: String::new(),
@@ -313,8 +332,17 @@ impl AddTablePageState {
rows
}
/// Row-display candidates: `id` first, then every column, matching the
/// client's candidate list.
/// Row-display candidates: `id` first, then every column the table will
/// really hold.
///
/// That includes the columns a definition row generates — they are columns
/// like any other once the table exists, and the server accepts them here.
/// The definition row itself is not among them: it leaves no column of its
/// own name behind for a row to be shown by.
///
/// The index is the candidate's place in this list, so the button that
/// toggles one names the same column the label does however the list is
/// filtered.
pub(crate) fn row_display_candidates(&self) -> Vec<RowDisplayCandidate> {
let mut candidates = vec![RowDisplayCandidate {
index: 0,
@@ -325,21 +353,13 @@ impl AddTablePageState {
None
},
}];
candidates.extend(
self.draft
.columns
.added
.iter()
.enumerate()
// A compound column expands into schema-managed companions, so
// there is no column of that name for a row to be shown by.
.filter(|(index, _)| self.draft.columns.is_indexable(*index))
.map(|(index, column)| RowDisplayCandidate {
index: index + 1,
name: column.name.clone(),
position: self.draft.row_display_position(&column.name),
}),
);
for name in self.draft.row_display_column_names() {
candidates.push(RowDisplayCandidate {
index: candidates.len(),
position: self.draft.row_display_position(&name),
name,
});
}
candidates
}
}
@@ -409,6 +429,7 @@ mod tests {
column_types: vec!["text".into(), "money".into()],
column_indexed: vec!["yes".into(), "no".into()],
column_quantity_ledger: vec!["no".into(), "no".into()],
column_required: vec!["yes".into(), "no".into()],
column_rounding: vec!["exact".into(), "half-up".into()],
column_currencies: vec![String::new(), "EUR".into()],
relation_tables: vec!["customer".into(), "project".into()],
@@ -480,6 +501,7 @@ mod tests {
data_type: "accounting".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: "EUR".to_string(),
});
@@ -523,6 +545,7 @@ mod tests {
form.column_types.push("accounting".into());
form.column_indexed.push("no".into());
form.column_quantity_ledger.push("no".into());
form.column_required.push("no".into());
form.column_rounding.push("exact".into());
form.column_currencies.push("EUR".into());
form.generated_alias_sources = vec!["debit".into(), "account".into()];

View File

@@ -77,6 +77,7 @@ mod tests {
data_type: "text".to_string(),
indexed: true,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: String::new(),
});
@@ -136,6 +137,7 @@ mod tests {
data_type: "accounting".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: "EUR".to_string(),
});
@@ -172,6 +174,7 @@ mod tests {
data_type: "accounting".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: "EUR".to_string(),
});
@@ -199,6 +202,7 @@ mod tests {
data_type: "accounting".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: "EUR".to_string(),
});
@@ -314,6 +318,7 @@ mod tests {
data_type: "money".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: "EUR".to_string(),
});
@@ -337,6 +342,7 @@ mod tests {
data_type: "accounting".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: "EUR".to_string(),
});
@@ -387,6 +393,115 @@ mod tests {
assert!(html.contains("htmx:beforeSwap"));
}
/// Choosing FKlink takes the Indexing choice away and says who makes the
/// index instead. Offering the choice is what sent a definition the server
/// refuses outright — a link is indexed automatically.
#[test]
fn choosing_a_link_replaces_the_index_choice_with_who_makes_it() {
let mut state = page();
assert!(render_builder(&state).contains(r#"name="column_indexing_input""#));
state.draft.columns.type_input = "link".to_string();
let html = render_builder(&state);
assert!(!html.contains(r#"name="column_indexing_input""#), "{html}");
assert!(html.contains("Indexed automatically"));
// The rest of the link's own fields are still there.
assert!(html.contains(r#"name="link_table_input""#));
assert!(html.contains("Link alias"));
}
/// And a link already in the list is reported as indexed, without a toggle
/// that would do nothing.
#[test]
fn a_link_in_the_column_list_is_shown_as_indexed_by_the_server() {
let mut state = page();
state.draft.columns.added.push(ColumnDefinition {
name: "billing_customer".to_string(),
data_type: "link(customer)".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: String::new(),
});
let html = render_builder(&state);
assert!(html.contains("indexed automatically"), "{html}");
// `number` keeps its toggle; the link is offered none.
assert!(html.contains(r#""action": "toggle-index", "index": "0""#));
assert!(!html.contains(r#""action": "toggle-index", "index": "1""#));
}
/// A column can be made required, and the answer travels with the draft.
#[test]
fn a_column_can_be_required_and_says_so_in_the_list() {
let mut state = page();
assert!(render_builder(&state).contains(r#"name="column_required_input""#));
state.draft.columns.added[0].required = true;
let html = render_builder(&state);
assert!(html.contains(r#"name="column_required" value="yes""#));
assert!(html.contains(r#"<span class="tag">required</span>"#));
}
/// A shared table keeps no books, so the columns that post to a profile's
/// books are not on offer and the page says why.
#[test]
fn the_shared_scope_offers_no_column_that_belongs_to_one_profile() {
let mut state = page();
state.draft.global = true;
state.draft.columns.global = true;
let html = render_builder(&state);
assert!(!html.contains(r#"<option value="accounting""#), "{html}");
assert!(!html.contains(r#"<option value="accounting_transfer""#));
assert!(!html.contains(r#"name="column_quantity_ledger_input""#));
assert!(html.contains("A shared table keeps no books of its own"));
// Everything else is still offered.
assert!(html.contains(r#"<option value="money""#));
}
/// A row is identified by the columns the table will really hold, which
/// includes what a definition row generates.
#[test]
fn generated_columns_can_identify_a_row() {
let mut state = page();
state.draft.columns.added.push(ColumnDefinition {
name: "accounting".to_string(),
data_type: "accounting".to_string(),
indexed: false,
quantity_ledger: false,
required: false,
money_mode: MoneyMode::Exact,
currency: "EUR".to_string(),
});
let candidates = state
.row_display_candidates()
.into_iter()
.map(|candidate| candidate.name)
.collect::<Vec<_>>();
assert_eq!(
candidates,
[
"id",
"number",
"name",
"tax_point_date",
"debit",
"credit",
"account"
]
);
// The definition row is not one of them, having no column of its name.
assert!(!candidates.contains(&"accounting".to_string()));
}
/// A load failure has no draft to render, so the dialog is the response.
#[test]
fn a_load_failure_answers_with_the_dialog() {