diff --git a/common/proto/table_definition.proto b/common/proto/table_definition.proto index d67bedfc..c682a75e 100644 --- a/common/proto/table_definition.proto +++ b/common/proto/table_definition.proto @@ -108,14 +108,18 @@ message PostTableDefinitionRequest { // column, though, so it is free to be anything: this is where that choice is // made, instead of a RenameColumnAlias call afterwards. // -// The ACCOUNTING_TRANSFER connectors are the exception. They are refused here -// exactly as RenameColumnAlias refuses them. +// Only ACCOUNTING's columns may be renamed. Renaming a generated column +// requires its relationship to whatever generated it to be recorded, so the +// rest of the system can find it without knowing its name; ACCOUNTING has that +// in table_accounting_definitions, and nothing else does. The ACCOUNTING_ +// TRANSFER connectors and the PHONE and IBAN companions are refused, the +// latter because a row write finds them by rebuilding their names from their +// parent column's name. message GeneratedColumnAlias { // The name the backend would otherwise give the column: one of ACCOUNTING's - // "name", "tax_point_date", "debit", "credit" or "account", or a companion - // such as "work_phone_extension". Must name a column the request really - // generates -- an alias for anything else is rejected rather than ignored, - // so a typo cannot pass silently. + // "name", "tax_point_date", "debit", "credit" or "account". Must name a + // column the request really generates -- an alias for anything else is + // rejected rather than ignored, so a typo cannot pass silently. string generated_name = 1; // What the column should be called instead. Same rules as any column name. diff --git a/common/src/proto/descriptor.bin b/common/src/proto/descriptor.bin index 37ac7e13..5d420cab 100644 Binary files a/common/src/proto/descriptor.bin and b/common/src/proto/descriptor.bin differ diff --git a/common/src/proto/komp_ac.table_definition.rs b/common/src/proto/komp_ac.table_definition.rs index 7c25fc4f..75c11b64 100644 --- a/common/src/proto/komp_ac.table_definition.rs +++ b/common/src/proto/komp_ac.table_definition.rs @@ -56,16 +56,20 @@ pub struct PostTableDefinitionRequest { /// column, though, so it is free to be anything: this is where that choice is /// made, instead of a RenameColumnAlias call afterwards. /// -/// The ACCOUNTING_TRANSFER connectors are the exception. They are refused here -/// exactly as RenameColumnAlias refuses them. +/// Only ACCOUNTING's columns may be renamed. Renaming a generated column +/// requires its relationship to whatever generated it to be recorded, so the +/// rest of the system can find it without knowing its name; ACCOUNTING has that +/// in table_accounting_definitions, and nothing else does. The ACCOUNTING\_ +/// TRANSFER connectors and the PHONE and IBAN companions are refused, the +/// latter because a row write finds them by rebuilding their names from their +/// parent column's name. #[derive(serde::Serialize, serde::Deserialize)] #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] pub struct GeneratedColumnAlias { /// The name the backend would otherwise give the column: one of ACCOUNTING's - /// "name", "tax_point_date", "debit", "credit" or "account", or a companion - /// such as "work_phone_extension". Must name a column the request really - /// generates -- an alias for anything else is rejected rather than ignored, - /// so a typo cannot pass silently. + /// "name", "tax_point_date", "debit", "credit" or "account". Must name a + /// column the request really generates -- an alias for anything else is + /// rejected rather than ignored, so a typo cannot pass silently. #[prost(string, tag = "1")] pub generated_name: ::prost::alloc::string::String, /// What the column should be called instead. Same rules as any column name. diff --git a/server b/server index de424f24..9984e3b2 160000 --- a/server +++ b/server @@ -1 +1 @@ -Subproject commit de424f245d0eced5419913bfd704d1a8d6b65b4d +Subproject commit 9984e3b272514f51092e049465d040afb0630279 diff --git a/web/src/pages/add_table/draft.rs b/web/src/pages/add_table/draft.rs index b459ab5d..e7024140 100644 --- a/web/src/pages/add_table/draft.rs +++ b/web/src/pages/add_table/draft.rs @@ -21,10 +21,6 @@ use crate::{ /// them, and [`TableDraft::preview_rows`] is where that is explained. pub(crate) const ACCOUNTING_FIELD_TYPE: &str = "accounting"; -/// The compound type whose companions the backend refuses to rename: they are -/// the connectors a transfer is posted through, and are looked up by name. -pub(crate) const ACCOUNTING_TRANSFER_FIELD_TYPE: &str = "accounting_transfer"; - /// 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. pub(crate) const ACCOUNT_API_COLUMN: &str = "account"; @@ -229,13 +225,18 @@ impl TableDraft { /// The generated columns this draft would let the user rename, in the order /// they appear in the column list. /// - /// ACCOUNTING_TRANSFER's companions are left out: the backend refuses to - /// name them anything else. Everything else a definition row or a companion - /// type generates is a display name over a physical column, and is free. + /// ACCOUNTING's, and only ACCOUNTING's. A generated column can carry a name + /// of its own once the rest of the system can find it without that name -- + /// which for ACCOUNTING is `table_accounting_definitions`, recording its + /// columns by physical name. The ACCOUNTING_TRANSFER connectors and the + /// PHONE and IBAN companions have no such record: a companion is found on + /// write by rebuilding its name from its parent's, so renaming one would + /// hide it from the write path. The backend refuses those, and this does + /// not offer what the backend refuses. pub(crate) fn aliasable_generated_columns(&self) -> Vec { let mut names = Vec::new(); for (index, column) in self.columns.added.iter().enumerate() { - if column.data_type == ACCOUNTING_TRANSFER_FIELD_TYPE { + if column.data_type != ACCOUNTING_FIELD_TYPE { continue; } names.extend( @@ -244,9 +245,7 @@ impl TableDraft { .iter() .map(|generated| generated.name.clone()), ); - if column.data_type == ACCOUNTING_FIELD_TYPE { - names.push(ACCOUNT_API_COLUMN.to_string()); - } + names.push(ACCOUNT_API_COLUMN.to_string()); } names } @@ -556,13 +555,24 @@ mod tests { ); } - /// The transfer connectors are resolved by name by the posting engine, and - /// the backend refuses to rename them, so they are never offered. + /// Only ACCOUNTING's columns are offered. The transfer connectors are + /// resolved by name by the posting engine, and a phone or IBAN companion is + /// found on write by rebuilding its name from its parent's -- the backend + /// refuses both, so neither is offered here. #[test] - fn accounting_transfer_connectors_are_not_aliasable() { - let draft = draft_with_column("accounting_transfer", "accounting_transfer"); + fn only_accounting_columns_are_aliasable() { + for (name, data_type) in [ + ("accounting_transfer", "accounting_transfer"), + ("work_phone", "phone"), + ("bank_account", "iban"), + ] { + let draft = draft_with_column(name, data_type); - assert!(draft.aliasable_generated_columns().is_empty()); + assert!( + draft.aliasable_generated_columns().is_empty(), + "`{data_type}` should offer no aliases" + ); + } } #[test] diff --git a/web/src/pages/add_table/state.rs b/web/src/pages/add_table/state.rs index 90f852ff..37caad79 100644 --- a/web/src/pages/add_table/state.rs +++ b/web/src/pages/add_table/state.rs @@ -13,10 +13,7 @@ use crate::schema::{ColumnCatalog, ColumnDraft, columns_from_rows}; -use super::draft::{ - ACCOUNT_API_COLUMN, ACCOUNTING_FIELD_TYPE, ACCOUNTING_TRANSFER_FIELD_TYPE, GeneratedAlias, - TableDraft, -}; +use super::draft::{ACCOUNT_API_COLUMN, ACCOUNTING_FIELD_TYPE, GeneratedAlias, TableDraft}; /// The `profile_name` option meaning "create a new profile too". pub(crate) const NEW_PROFILE: &str = "__new__"; @@ -227,10 +224,11 @@ impl AddTablePageState { }); // A generated column is named by the backend, but the name is a - // display name: it can be aliased, and the alias is applied as a - // rename the moment the table exists. The connectors of an - // ACCOUNTING_TRANSFER are the exception the backend protects. - let aliasable = column.data_type != ACCOUNTING_TRANSFER_FIELD_TYPE; + // display name, and ACCOUNTING's columns can be given one of the + // user's own in the request that creates them. Only ACCOUNTING's: + // see `TableDraft::aliasable_generated_columns` for why the other + // generated columns cannot be renamed at all. + let aliasable = column.data_type == ACCOUNTING_FIELD_TYPE; for generated in columns.generated_columns_of(index) { let mut tags = vec![format!("generated by {}", column.data_type)];