diff --git a/web/locales/cs/main.ftl b/web/locales/cs/main.ftl index 8e12e901..2603077c 100644 --- a/web/locales/cs/main.ftl +++ b/web/locales/cs/main.ftl @@ -712,6 +712,7 @@ draft-err-profile-name = Zadejte název nového profilu. draft-err-table-conflict-global = Tabulka s názvem `{ $name }` už existuje. Název sdílené tabulky musí být volný v každém profilu. draft-err-table-conflict = Tabulka s názvem `{ $name }` už existuje v profilu `{ $profile }`, nebo je sdílená všemi profily. draft-err-no-columns = Před uložením přidejte alespoň jeden sloupec. +draft-err-ragged-columns = Odeslaná data sloupců jsou neúplná: odesláno bylo { $posted } sloupců, ale jen { $kept } má všechna pole. Každý sloupec potřebuje název, typ, indexování, kvantitativní evidenci, povinnost, zaokrouhlení a měnu. draft-err-alias-taken = Alias `{ $alias }` pro generovaný sloupec `{ $source }` už používá jiný sloupec. draft-err-accounting-currency = Účetní měna musí být třímístný kód ISO-4217 diff --git a/web/locales/en/main.ftl b/web/locales/en/main.ftl index ee5544f8..167996df 100644 --- a/web/locales/en/main.ftl +++ b/web/locales/en/main.ftl @@ -695,6 +695,7 @@ draft-err-profile-name = Enter a name for the new profile. draft-err-table-conflict-global = A table named `{ $name }` already exists. A shared table's name has to be free in every profile. draft-err-table-conflict = A table named `{ $name }` already exists in profile `{ $profile }`, or is shared by every profile. draft-err-no-columns = Add at least one column before saving. +draft-err-ragged-columns = The posted column data is incomplete: { $posted } columns were sent, but only { $kept } carry every field. Each column needs a name, type, indexing, quantity-ledger, required, rounding and currency value. draft-err-alias-taken = Alias `{ $alias }` for generated column `{ $source }` is already taken by another column. draft-err-accounting-currency = Accounting currency must be a three-letter ISO-4217 code diff --git a/web/locales/sk/main.ftl b/web/locales/sk/main.ftl index 8a0e224f..dc151772 100644 --- a/web/locales/sk/main.ftl +++ b/web/locales/sk/main.ftl @@ -712,6 +712,7 @@ draft-err-profile-name = Zadajte názov nového profilu. draft-err-table-conflict-global = Tabuľka s názvom `{ $name }` už existuje. Názov zdieľanej tabuľky musí byť voľný v každom profile. draft-err-table-conflict = Tabuľka s názvom `{ $name }` už existuje v profile `{ $profile }`, alebo je zdieľaná všetkými profilmi. draft-err-no-columns = Pred uložením pridajte aspoň jeden stĺpec. +draft-err-ragged-columns = Odoslané údaje o stĺpcoch sú neúplné: odoslaných bolo { $posted } stĺpcov, ale iba { $kept } má všetky polia. Každý stĺpec potrebuje názov, typ, indexovanie, kvantitatívnu evidenciu, povinnosť, zaokrúhľovanie a menu. draft-err-alias-taken = Alias `{ $alias }` pre generovaný stĺpec `{ $source }` už používa iný stĺpec. draft-err-accounting-currency = Účtovná mena musí byť trojpísmenový kód ISO-4217 diff --git a/web/src/pages/add_table/draft.rs b/web/src/pages/add_table/draft.rs index 704f18dd..343ff113 100644 --- a/web/src/pages/add_table/draft.rs +++ b/web/src/pages/add_table/draft.rs @@ -545,6 +545,11 @@ impl TableDraft { ) }); } + // A ragged post can leave no columns at all, and "add at least one + // column" would then be answering a question nobody asked. + if let Some(ragged) = self.columns.ragged { + return Err(ragged.message(locale)); + } if self.columns.is_empty() { return Err(tr!(locale, "draft-err-no-columns")); } diff --git a/web/src/pages/add_table/state.rs b/web/src/pages/add_table/state.rs index 3f658def..291fa328 100644 --- a/web/src/pages/add_table/state.rs +++ b/web/src/pages/add_table/state.rs @@ -107,6 +107,15 @@ impl BuilderForm { pub(crate) fn to_draft(&self) -> TableDraft { let creating_new_profile = self.creating_new_profile(); + let (added, ragged) = columns_from_rows( + &self.column_names, + &self.column_types, + &self.column_indexed, + &self.column_quantity_ledger, + &self.column_required, + &self.column_rounding, + &self.column_currencies, + ); let columns = ColumnDraft { name_input: self.column_name_input.clone(), type_input: self.column_type_input.clone(), @@ -120,15 +129,8 @@ impl BuilderForm { 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( - &self.column_names, - &self.column_types, - &self.column_indexed, - &self.column_quantity_ledger, - &self.column_required, - &self.column_rounding, - &self.column_currencies, - ), + added, + ragged, // Filled in by the loader from `ListColumnTypes`, never by the // form: it is the vocabulary the draft is validated against. catalog: ColumnCatalog::default(), diff --git a/web/src/schema/mod.rs b/web/src/schema/mod.rs index aef41232..8a47a692 100644 --- a/web/src/schema/mod.rs +++ b/web/src/schema/mod.rs @@ -493,6 +493,10 @@ pub(crate) struct ColumnDraft { /// point at. Set by the page: the builder knows the name being typed, and /// the append screen knows the table it is appending to. pub table_name: String, + + /// Set when the post that rebuilt `added` had ragged column vectors, so + /// validation can say that rather than describe the truncated result. + pub ragged: Option, } impl ColumnDraft { @@ -1015,6 +1019,12 @@ impl ColumnDraft { /// from a posted form has not been through that path, so this is what a /// tampered-with or truncated post is held to. pub(crate) fn validate(&self, locale: crate::i18n::Locale) -> Result<(), String> { + // Before anything about the columns themselves: if the post was + // ragged, `added` is not what was sent, and every message below would + // be describing a list the caller never posted. + if let Some(ragged) = self.ragged { + return Err(ragged.message(locale)); + } let claimed = self.claimed_names(); for column in &self.added { if let Some(error) = @@ -1442,6 +1452,15 @@ pub(crate) struct ColumnForm { impl ColumnForm { pub(crate) fn to_draft(&self, catalog: ColumnCatalog, creating_table: bool) -> ColumnDraft { + let (added, ragged) = columns_from_rows( + &self.column_names, + &self.column_types, + &self.column_indexed, + &self.column_quantity_ledger, + &self.column_required, + &self.column_rounding, + &self.column_currencies, + ); ColumnDraft { name_input: self.column_name_input.clone(), type_input: self.column_type_input.clone(), @@ -1455,15 +1474,8 @@ impl ColumnForm { 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( - &self.column_names, - &self.column_types, - &self.column_indexed, - &self.column_quantity_ledger, - &self.column_required, - &self.column_rounding, - &self.column_currencies, - ), + added, + ragged, catalog, creating_table, // Filled in by the page, which is what knows the table these @@ -1478,11 +1490,36 @@ fn is_yes(value: &str) -> bool { value.trim().eq_ignore_ascii_case("yes") } +/// A post whose parallel column vectors were not all the same length: `kept` +/// columns could be rebuilt, while the longest vector carried `posted`. +/// +/// The page itself always writes all seven values per staged column, so this +/// only happens to something posting the form by hand — and it used to be +/// invisible, surfacing as "Add at least one column before saving" when the +/// short vector was empty, or as silently dropped columns when it was not. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct RaggedColumns { + pub kept: usize, + pub posted: usize, +} + +impl RaggedColumns { + pub(crate) fn message(self, locale: crate::i18n::Locale) -> String { + crate::tr!( + locale, + "draft-err-ragged-columns", + "posted" => self.posted as i64, + "kept" => self.kept as i64, + ) + } +} + /// Zips the posted column vectors back into column definitions. /// /// The vectors are parallel, so a short one — a truncated or tampered-with /// post — simply limits how many columns are reconstructed rather than -/// mis-pairing them. +/// mis-pairing them. The mismatch is reported alongside, so the draft can say +/// so instead of quietly holding fewer columns than were sent. pub(crate) fn columns_from_rows( names: &[String], types: &[String], @@ -1491,8 +1528,8 @@ pub(crate) fn columns_from_rows( required: &[String], rounding: &[String], currencies: &[String], -) -> Vec { - let count = [ +) -> (Vec, Option) { + let lengths = [ names.len(), types.len(), indexed.len(), @@ -1500,12 +1537,15 @@ pub(crate) fn columns_from_rows( required.len(), rounding.len(), currencies.len(), - ] - .into_iter() - .min() - .unwrap_or(0); + ]; + let count = lengths.into_iter().min().unwrap_or(0); + let longest = lengths.into_iter().max().unwrap_or(0); + let ragged = (longest > count).then_some(RaggedColumns { + kept: count, + posted: longest, + }); - (0..count) + let columns = (0..count) .map(|index| ColumnDefinition { name: names[index].clone(), data_type: types[index].clone(), @@ -1515,7 +1555,8 @@ pub(crate) fn columns_from_rows( money_mode: MoneyMode::from_input(&rounding[index]), currency: currencies[index].clone(), }) - .collect() + .collect(); + (columns, ragged) } #[cfg(test)] @@ -2016,7 +2057,7 @@ pub(crate) mod tests { #[test] fn mismatched_column_vectors_never_mis_pair() { - let columns = columns_from_rows( + let (columns, ragged) = columns_from_rows( &["number".to_string(), "total".to_string()], &["text".to_string()], &["yes".to_string(), "no".to_string()], @@ -2030,6 +2071,31 @@ pub(crate) mod tests { assert_eq!(columns[0].name, "number"); assert_eq!(columns[0].data_type, "text"); assert!(columns[0].indexed); + // And the truncation is reported rather than passed off as the draft + // the caller posted: two columns were sent, one survived the zip. + assert_eq!(ragged, Some(RaggedColumns { kept: 1, posted: 2 })); + } + + /// The message a ragged post gets. It used to be "Add at least one column + /// before saving", which describes the *result* of dropping the caller's + /// columns instead of the mismatch that dropped them. + #[test] + fn a_ragged_post_says_how_many_columns_went_missing() { + let (columns, ragged) = columns_from_rows( + &["number".to_string(), "total".to_string()], + &[], + &[], + &[], + &[], + &[], + &[], + ); + + assert!(columns.is_empty()); + let ragged = ragged.expect("two names and no other vector is ragged"); + let message = ragged.message(crate::i18n::Locale::default()); + assert!(message.contains('2'), "{message}"); + assert!(!message.contains("at least one"), "{message}"); } /// The declared order is the order the table gets its columns in, so it is