diff --git a/client b/client index c923d0de..c15c62e8 160000 --- a/client +++ b/client @@ -1 +1 @@ -Subproject commit c923d0de22b3ed20cd9d0fbd322456c18e7aa338 +Subproject commit c15c62e88b4df273095d9a17ad7655b9d30c92c7 diff --git a/common/proto/table_definition.proto b/common/proto/table_definition.proto index c682a75e..9357a7be 100644 --- a/common/proto/table_definition.proto +++ b/common/proto/table_definition.proto @@ -108,18 +108,17 @@ message PostTableDefinitionRequest { // column, though, so it is free to be anything: this is where that choice is // made, instead of a RenameColumnAlias call afterwards. // -// 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. +// ACCOUNTING, PHONE and IBAN generated columns may be renamed: their +// relationships are recorded by physical column, so the rest of the system can +// find them without knowing their display names. ACCOUNTING_TRANSFER connectors +// are the exception and are refused here exactly as RenameColumnAlias refuses +// them. message GeneratedColumnAlias { // The name the backend would otherwise give the column: one of ACCOUNTING's - // "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. + // "name", "tax_point_date", "debit", "credit" or "account", or a PHONE or + // IBAN companion such as "work_phone_ext". 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 5d420cab..47496694 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 75c11b64..5f7a9cce 100644 --- a/common/src/proto/komp_ac.table_definition.rs +++ b/common/src/proto/komp_ac.table_definition.rs @@ -56,20 +56,19 @@ 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. /// -/// 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. +/// ACCOUNTING, PHONE and IBAN generated columns may be renamed: their +/// relationships are recorded by physical column, so the rest of the system can +/// find them without knowing their display names. ACCOUNTING_TRANSFER connectors +/// are the exception and are refused here exactly as RenameColumnAlias refuses +/// them. #[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". 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", or a PHONE or + /// IBAN companion such as "work_phone_ext". 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 9984e3b2..6e8ac323 160000 --- a/server +++ b/server @@ -1 +1 @@ -Subproject commit 9984e3b272514f51092e049465d040afb0630279 +Subproject commit 6e8ac32389ac7e69556ed0f1afe6fef8c0d68a68 diff --git a/web/src/pages/add_table/draft.rs b/web/src/pages/add_table/draft.rs index e7024140..ccf5ec76 100644 --- a/web/src/pages/add_table/draft.rs +++ b/web/src/pages/add_table/draft.rs @@ -21,6 +21,9 @@ use crate::{ /// 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"; + /// 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"; @@ -225,18 +228,13 @@ impl TableDraft { /// The generated columns this draft would let the user rename, in the order /// they appear in the column list. /// - /// 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. + /// Every generated column whose relationship is recorded independently of + /// its display name. ACCOUNTING_TRANSFER's connectors are the exception: + /// the backend still resolves those by their fixed names. 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_FIELD_TYPE { + if column.data_type == ACCOUNTING_TRANSFER_FIELD_TYPE { continue; } names.extend( @@ -245,7 +243,9 @@ impl TableDraft { .iter() .map(|generated| generated.name.clone()), ); - names.push(ACCOUNT_API_COLUMN.to_string()); + if column.data_type == ACCOUNTING_FIELD_TYPE { + names.push(ACCOUNT_API_COLUMN.to_string()); + } } names } @@ -555,24 +555,34 @@ mod tests { ); } - /// 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. + /// ACCOUNTING, PHONE and IBAN companions may be aliased. Transfer + /// connectors remain fixed because the posting engine resolves their names. #[test] - 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); + fn every_recorded_generated_column_is_aliasable() { + let phone = draft_with_column("work_phone", "phone"); + assert_eq!( + phone.aliasable_generated_columns(), + [ + "work_phone_ext", + "work_phone_type", + "work_phone_country", + "work_phone_calling_code", + ] + ); - assert!( - draft.aliasable_generated_columns().is_empty(), - "`{data_type}` should offer no aliases" - ); - } + let iban = draft_with_column("bank_account", "iban"); + assert_eq!( + iban.aliasable_generated_columns(), + [ + "bank_account_country", + "bank_account_bban", + "bank_account_bank_identifier", + "bank_account_branch_identifier", + ] + ); + + let transfer = draft_with_column("accounting_transfer", "accounting_transfer"); + assert!(transfer.aliasable_generated_columns().is_empty()); } #[test] diff --git a/web/src/pages/add_table/state.rs b/web/src/pages/add_table/state.rs index 37caad79..9f7783b7 100644 --- a/web/src/pages/add_table/state.rs +++ b/web/src/pages/add_table/state.rs @@ -13,7 +13,10 @@ use crate::schema::{ColumnCatalog, ColumnDraft, columns_from_rows}; -use super::draft::{ACCOUNT_API_COLUMN, ACCOUNTING_FIELD_TYPE, GeneratedAlias, TableDraft}; +use super::draft::{ + ACCOUNT_API_COLUMN, ACCOUNTING_FIELD_TYPE, ACCOUNTING_TRANSFER_FIELD_TYPE, GeneratedAlias, + TableDraft, +}; /// The `profile_name` option meaning "create a new profile too". pub(crate) const NEW_PROFILE: &str = "__new__"; @@ -223,12 +226,10 @@ impl AddTablePageState { alias: String::new(), }); - // A generated column is named by the backend, but the name is a - // 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; + // Every generated column with persisted provenance may be named by + // the request. Transfer connectors are still resolved by fixed + // backend names and remain the exception. + let aliasable = column.data_type != ACCOUNTING_TRANSFER_FIELD_TYPE; for generated in columns.generated_columns_of(index) { let mut tags = vec![format!("generated by {}", column.data_type)]; diff --git a/web/src/schema/mod.rs b/web/src/schema/mod.rs index 7bf36dcf..5298cee3 100644 --- a/web/src/schema/mod.rs +++ b/web/src/schema/mod.rs @@ -664,11 +664,22 @@ impl ColumnDraft { /// compound one. This is what the column list shows underneath it, so the /// columns a definition row brings are visible while the table is still /// being described. - pub(crate) fn generated_columns_of(&self, index: usize) -> &[GeneratedColumn] { - self.added - .get(index) - .map(|column| self.catalog.generated_columns(&column.data_type)) - .unwrap_or_default() + pub(crate) fn generated_columns_of(&self, index: usize) -> Vec { + let Some(column) = self.added.get(index) else { + return Vec::new(); + }; + let generated = self.catalog.generated_columns(&column.data_type); + let default_prefix = format!("{}_", column.data_type); + generated + .iter() + .cloned() + .map(|mut companion| { + if let Some(suffix) = companion.name.strip_prefix(&default_prefix) { + companion.name = format!("{}_{}", column.name, suffix); + } + companion + }) + .collect() } /// Whether a column can be indexed or identify a row. A compound column @@ -1115,7 +1126,23 @@ pub(crate) mod tests { grouped("gtin_12", "gtin"), grouped("gtin_13", "gtin"), grouped("gtin_14", "gtin"), - declarable("iban"), + ColumnType { + generated_columns: vec![ + generated_column("iban_country", "iban_country", false), + generated_column("iban_bban", "iban_bban", false), + generated_column( + "iban_bank_identifier", + "iban_bank_identifier", + false, + ), + generated_column( + "iban_branch_identifier", + "iban_branch_identifier", + false, + ), + ], + ..declarable("iban") + }, ColumnType { declarable: false, ..declarable("iban_bban") @@ -1135,7 +1162,19 @@ pub(crate) mod tests { ..declarable("numeric") }, declarable("period"), - declarable("phone"), + ColumnType { + generated_columns: vec![ + generated_column("phone_ext", "phone_extension", false), + generated_column("phone_type", "phone_type", false), + generated_column("phone_country", "phone_country", false), + generated_column( + "phone_calling_code", + "phone_calling_code", + false, + ), + ], + ..declarable("phone") + }, ColumnType { declarable: false, sql_type: "INTEGER".to_string(),