diff --git a/client b/client index c48deeb2..c923d0de 160000 --- a/client +++ b/client @@ -1 +1 @@ -Subproject commit c48deeb2caee4b4e19fbf1b50a2f654b7c8c68e4 +Subproject commit c923d0de22b3ed20cd9d0fbd322456c18e7aa338 diff --git a/common/build.rs b/common/build.rs index efb54993..618d93f5 100644 --- a/common/build.rs +++ b/common/build.rs @@ -161,6 +161,12 @@ fn main() -> Result<(), Box> { ".komp_ac.table_definition.PostTableDefinitionRequest", "#[derive(serde::Serialize, serde::Deserialize)]", ) + // Carried by the two requests that create columns, both of which are + // serialised, so it has to be serialisable with them. + .type_attribute( + ".komp_ac.table_definition.GeneratedColumnAlias", + "#[derive(serde::Serialize, serde::Deserialize)]", + ) .type_attribute( ".komp_ac.table_definition.CreateInvoiceTemplateTableRequest", "#[derive(serde::Serialize, serde::Deserialize)]", diff --git a/common/proto/table_definition.proto b/common/proto/table_definition.proto index c73de16a..d67bedfc 100644 --- a/common/proto/table_definition.proto +++ b/common/proto/table_definition.proto @@ -94,6 +94,32 @@ message PostTableDefinitionRequest { // visible from every profile. profile_name and accounting_currency are ignored. bool global = 9; + // Names for the columns a definition row generates, in place of the ones the + // backend would give them. See GeneratedColumnAlias. + repeated GeneratedColumnAlias generated_aliases = 10; + +} + +// Renames one column a definition row generates, at the moment it is created. +// +// A generated column cannot be named in the column list -- the definition row +// is required to be named after its own type, and what it expands into is the +// backend's to decide. The name is only ever a display name over a physical +// 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. +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. + string generated_name = 1; + + // What the column should be called instead. Same rules as any column name. + string alias = 2; } // Defines the input for explicitly creating tables backed by one invoice @@ -147,6 +173,10 @@ message AddTableColumnsRequest { // Optional indexes for the new columns only. repeated string indexes = 4; + // Names for the columns the appended definition rows generate, exactly as on + // PostTableDefinitionRequest. + repeated GeneratedColumnAlias generated_aliases = 5; + } enum MoneyRounding { diff --git a/common/src/proto/descriptor.bin b/common/src/proto/descriptor.bin index 20521138..37ac7e13 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 ffb8f478..7c25fc4f 100644 --- a/common/src/proto/komp_ac.table_definition.rs +++ b/common/src/proto/komp_ac.table_definition.rs @@ -43,6 +43,34 @@ pub struct PostTableDefinitionRequest { /// visible from every profile. profile_name and accounting_currency are ignored. #[prost(bool, tag = "9")] pub global: bool, + /// Names for the columns a definition row generates, in place of the ones the + /// backend would give them. See GeneratedColumnAlias. + #[prost(message, repeated, tag = "10")] + pub generated_aliases: ::prost::alloc::vec::Vec, +} +/// Renames one column a definition row generates, at the moment it is created. +/// +/// A generated column cannot be named in the column list -- the definition row +/// is required to be named after its own type, and what it expands into is the +/// backend's to decide. The name is only ever a display name over a physical +/// 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. +#[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. + #[prost(string, tag = "1")] + pub generated_name: ::prost::alloc::string::String, + /// What the column should be called instead. Same rules as any column name. + #[prost(string, tag = "2")] + pub alias: ::prost::alloc::string::String, } /// Defines the input for explicitly creating tables backed by one invoice /// template. typst_source must contain exactly one field declaration: @@ -110,6 +138,10 @@ pub struct AddTableColumnsRequest { /// Optional indexes for the new columns only. #[prost(string, repeated, tag = "4")] pub indexes: ::prost::alloc::vec::Vec<::prost::alloc::string::String>, + /// Names for the columns the appended definition rows generate, exactly as on + /// PostTableDefinitionRequest. + #[prost(message, repeated, tag = "5")] + pub generated_aliases: ::prost::alloc::vec::Vec, } /// Describes one user-defined column for a table. #[derive(serde::Serialize, serde::Deserialize)] diff --git a/server b/server index 2e88284d..de424f24 160000 --- a/server +++ b/server @@ -1 +1 @@ -Subproject commit 2e88284dd3a721ebe6e187f17b5c6c3ccadcc505 +Subproject commit de424f245d0eced5419913bfd704d1a8d6b65b4d diff --git a/web/src/pages/add_table/draft.rs b/web/src/pages/add_table/draft.rs index a2f386d7..b459ab5d 100644 --- a/web/src/pages/add_table/draft.rs +++ b/web/src/pages/add_table/draft.rs @@ -12,9 +12,7 @@ //! what identifies one of its rows. use crate::{ - definitions::table_definition::{ - PostTableDefinitionRequest, - }, + definitions::table_definition::{GeneratedColumnAlias, PostTableDefinitionRequest}, schema::{ColumnCatalog, ColumnDraft, proto_columns, validate_identifier}, }; @@ -23,6 +21,28 @@ 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"; + +/// An alias asked for one of the columns a definition row generates. +/// +/// A generated column cannot be named in the column list — the definition row +/// is named after its own type, and what it expands into is the backend's to +/// decide. The name is only ever a display name over a physical column, though, +/// so the request carries the name to use and the column is created under it. +#[derive(Clone, Debug, Default)] +pub(crate) struct GeneratedAlias { + /// The generated column's own name, which is what the rename asks for. + pub source: String, + /// What the user wants to see instead. Empty means "leave it alone". + pub alias: String, +} + /// One row of the "Table definition preview" — the schema as it will exist. pub(crate) struct PreviewRow { pub mark: String, @@ -66,6 +86,10 @@ pub(crate) struct TableDraft { /// Tables already defined in the target profile — a new table may not /// reuse one of these names. pub existing_profile_tables: Vec, + + /// Names asked for the generated columns, sent with the request that + /// creates them. + pub generated_aliases: Vec, } impl TableDraft { @@ -200,6 +224,110 @@ impl TableDraft { .collect() } + // ---- generated-column aliases ---------------------------------------- + + /// 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. + 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 { + continue; + } + names.extend( + self.columns + .generated_columns_of(index) + .iter() + .map(|generated| generated.name.clone()), + ); + if column.data_type == ACCOUNTING_FIELD_TYPE { + names.push(ACCOUNT_API_COLUMN.to_string()); + } + } + names + } + + /// The alias typed for one generated column, empty when none was. + pub(crate) fn alias_for(&self, source: &str) -> &str { + self.generated_aliases + .iter() + .find(|entry| entry.source == source) + .map(|entry| entry.alias.trim()) + .unwrap_or_default() + } + + /// The name a generated column will be known by: its alias when one was + /// asked for, otherwise the name the backend gives it. + pub(crate) fn generated_display_name(&self, source: &str) -> String { + match self.alias_for(source) { + "" => source.to_string(), + alias => alias.to_string(), + } + } + + /// The aliases the request carries: only the generated columns still in the + /// draft, and only where an alias was actually asked for. + /// + /// Sending an alias for a column the request does not generate is an error + /// on the backend rather than a no-op, so a stale entry -- one left behind + /// by a column since removed -- is dropped here. + pub(crate) fn generated_alias_requests(&self) -> Vec { + self.aliased_generated_columns() + .into_iter() + .map(|(generated_name, alias)| GeneratedColumnAlias { + generated_name, + alias, + }) + .collect() + } + + pub(crate) fn aliased_generated_columns(&self) -> Vec<(String, String)> { + self.aliasable_generated_columns() + .into_iter() + .filter_map(|source| { + let alias = self.alias_for(&source); + (!alias.is_empty() && alias != source).then(|| (source, alias.to_string())) + }) + .collect() + } + + /// Every alias has to be a legal column name, and has to be free: the table + /// is about to hold the declared columns, the generated ones and the system + /// ones, and two columns cannot share a name. + fn validate_generated_aliases(&self) -> Result<(), String> { + let generated = self.aliasable_generated_columns(); + let renames = self.aliased_generated_columns(); + + // What the table would hold with every alias applied. + let mut taken: Vec = vec![ + "id".to_string(), + "deleted".to_string(), + "created_at".to_string(), + ]; + taken.extend(self.columns.added.iter().map(|column| column.name.clone())); + taken.extend( + generated + .iter() + .map(|source| self.generated_display_name(source)), + ); + + for (source, alias) in &renames { + if let Some(error) = validate_identifier(alias, "Column alias", true) { + return Err(error); + } + if taken.iter().filter(|name| *name == alias).count() > 1 { + return Err(format!( + "Alias `{alias}` for generated column `{source}` is already taken by another column." + )); + } + } + Ok(()) + } + // ---- derived state --------------------------------------------------- /// The profile name to validate and persist: the typed input while creating @@ -271,7 +399,7 @@ impl TableDraft { for generated in self.columns.generated_columns_of(index) { rows.push(PreviewRow { mark: String::new(), - column: generated.name.clone(), + column: self.generated_display_name(&generated.name), data_type: generated.data_type.clone(), option: if generated.inherits_currency { format!("{}, {}", column.currency, column.money_mode.label()) @@ -291,7 +419,10 @@ impl TableDraft { mark: String::new(), column: "account_id".to_string(), data_type: "BIGINT".to_string(), - option: "not null, → accounts, written as account".to_string(), + option: format!( + "not null, → accounts, written as {}", + self.generated_display_name(ACCOUNT_API_COLUMN) + ), source: "system".to_string(), }); } @@ -333,6 +464,7 @@ impl TableDraft { if self.columns.is_empty() { return Err("Add at least one column before saving.".to_string()); } + self.validate_generated_aliases()?; self.columns.validate() } @@ -352,6 +484,10 @@ impl TableDraft { }, row_display_columns: self.row_display_columns.clone(), global: self.global, + // The generated columns are named in the same request that creates + // them, so the table is never briefly live under names the user did + // not ask for. + generated_aliases: self.generated_alias_requests(), }) } } @@ -408,6 +544,102 @@ mod tests { assert_eq!(draft.into_request().unwrap().accounting_currency, "EUR"); } + /// Every column an ACCOUNTING row generates may be aliased, including the + /// account foreign key the catalog does not report. + #[test] + fn accounting_generates_columns_that_can_all_be_aliased() { + let draft = draft_with_column("accounting", "accounting"); + + assert_eq!( + draft.aliasable_generated_columns(), + ["name", "tax_point_date", "debit", "credit", "account"] + ); + } + + /// The transfer connectors are resolved by name by the posting engine, and + /// the backend refuses to rename them, so they are never offered. + #[test] + fn accounting_transfer_connectors_are_not_aliasable() { + let draft = draft_with_column("accounting_transfer", "accounting_transfer"); + + assert!(draft.aliasable_generated_columns().is_empty()); + } + + #[test] + fn an_alias_is_sent_only_when_it_changes_the_name() { + let mut draft = draft_with_column("accounting", "accounting"); + draft.generated_aliases = vec![ + GeneratedAlias { + source: "debit".to_string(), + alias: "md".to_string(), + }, + GeneratedAlias { + source: "credit".to_string(), + alias: "credit".to_string(), + }, + GeneratedAlias { + source: "name".to_string(), + alias: String::new(), + }, + // A stale pair for a column that is no longer in the draft. + GeneratedAlias { + source: "source_period_id".to_string(), + alias: "start".to_string(), + }, + ]; + + assert_eq!( + draft.aliased_generated_columns(), + [("debit".to_string(), "md".to_string())] + ); + assert_eq!(draft.generated_display_name("debit"), "md"); + assert_eq!(draft.generated_display_name("credit"), "credit"); + assert!(draft.validate().is_ok()); + assert!( + draft + .preview_rows() + .iter() + .any(|row| row.column == "md" && row.source == "generated") + ); + + // The request that creates the columns is the request that names them. + let request = draft.into_request().unwrap(); + assert_eq!(request.generated_aliases.len(), 1); + assert_eq!(request.generated_aliases[0].generated_name, "debit"); + assert_eq!(request.generated_aliases[0].alias, "md"); + } + + #[test] + fn an_alias_must_be_a_legal_and_free_column_name() { + let mut draft = draft_with_column("accounting", "accounting"); + draft.columns.added.push(ColumnDefinition { + name: "note".to_string(), + data_type: "text".to_string(), + indexed: false, + quantity_ledger: false, + money_mode: MoneyMode::Exact, + currency: String::new(), + }); + + draft.generated_aliases = vec![GeneratedAlias { + source: "debit".to_string(), + alias: "Md".to_string(), + }]; + assert!(draft.validate().is_err(), "an alias is a column name"); + + draft.generated_aliases[0].alias = "note".to_string(); + assert!(draft.validate().is_err(), "a declared column holds the name"); + + draft.generated_aliases[0].alias = "credit".to_string(); + assert!(draft.validate().is_err(), "another generated column does"); + + draft.generated_aliases[0].alias = "id".to_string(); + assert!(draft.validate().is_err(), "a system column does"); + + draft.generated_aliases[0].alias = "md".to_string(); + assert!(draft.validate().is_ok()); + } + #[test] fn existing_profile_sends_no_accounting_currency() { let draft = draft_with_column("total", "int"); diff --git a/web/src/pages/add_table/state.rs b/web/src/pages/add_table/state.rs index cedd9dc8..90f852ff 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::{ACCOUNTING_FIELD_TYPE, 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__"; @@ -82,6 +85,13 @@ pub(crate) struct BuilderForm { #[serde(default)] pub row_display_columns: Vec, + + // One pair per generated column offered an alias, in list order: the + // generated column's own name, and what the user typed for it. + #[serde(default)] + pub generated_alias_sources: Vec, + #[serde(default)] + pub generated_alias_names: Vec, } impl BuilderForm { @@ -148,6 +158,18 @@ impl BuilderForm { // Filled in by the loader from the live profile tree, never by the // client: it is what duplicate table names are checked against. existing_profile_tables: Vec::new(), + // Which of these still name a generated column is the catalog's + // answer, and the catalog is not filled in yet, so a pair is kept + // here and matched up when the aliases are read. + generated_aliases: self + .generated_alias_sources + .iter() + .zip(self.generated_alias_names.iter()) + .map(|(source, alias)| GeneratedAlias { + source: source.trim().to_string(), + alias: alias.trim().to_string(), + }) + .collect(), } } } @@ -200,8 +222,16 @@ impl AddTablePageState { indexable: columns.is_indexable(index), indexed: column.indexed, tags, + alias_source: None, + alias: String::new(), }); + // 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; + for generated in columns.generated_columns_of(index) { let mut tags = vec![format!("generated by {}", column.data_type)]; if generated.inherits_currency { @@ -216,6 +246,8 @@ impl AddTablePageState { indexable: false, indexed: false, tags, + alias_source: aliasable.then(|| generated.name.clone()), + alias: self.draft.alias_for(&generated.name).to_string(), }); } @@ -233,8 +265,13 @@ impl AddTablePageState { indexed: true, tags: vec![ "system column".to_string(), - "written as account".to_string(), + format!( + "written as {}", + self.draft.generated_display_name(ACCOUNT_API_COLUMN) + ), ], + alias_source: Some(ACCOUNT_API_COLUMN.to_string()), + alias: self.draft.alias_for(ACCOUNT_API_COLUMN).to_string(), }); } } @@ -287,6 +324,12 @@ pub(crate) struct ColumnRow { pub indexable: bool, pub indexed: bool, pub tags: Vec, + /// The generated column an alias would rename, when this row is one that + /// may be aliased. `None` for a declared column — it is named where it is + /// described — and for the columns the backend refuses to rename. + pub alias_source: Option, + /// The alias typed for it so far. + pub alias: String, } pub(crate) struct RowDisplayCandidate { @@ -425,6 +468,51 @@ mod tests { assert!(row("name").tags.contains(&"generated by accounting".to_string())); } + /// The generated columns of an ACCOUNTING row are the user's to alias, and + /// the alias travels back with the rest of the draft. + #[test] + fn the_generated_columns_of_an_accounting_row_offer_an_alias() { + let mut form = posted_form(); + form.column_names.push("accounting".into()); + form.column_types.push("accounting".into()); + form.column_indexed.push("no".into()); + form.column_quantity_ledger.push("no".into()); + form.column_rounding.push("exact".into()); + form.column_currencies.push("EUR".into()); + form.generated_alias_sources = vec!["debit".into(), "account".into()]; + form.generated_alias_names = vec![" md ".into(), "ucet".into()]; + + let mut draft = form.to_draft(); + draft.columns.catalog = crate::schema::tests::catalog(); + + assert_eq!(draft.alias_for("debit"), "md"); + assert_eq!( + draft.aliased_generated_columns(), + vec![ + ("debit".to_string(), "md".to_string()), + ("account".to_string(), "ucet".to_string()), + ] + ); + + let page = AddTablePageState { + nav: crate::ui::Nav::default(), + profiles: Vec::new(), + draft, + status: None, + error: None, + }; + let rows = page.column_rows(); + let row = |name: &str| rows.iter().find(|row| row.name == name).unwrap(); + assert_eq!(row("debit").alias_source.as_deref(), Some("debit")); + assert_eq!(row("debit").alias, "md"); + assert_eq!(row("account_id").alias_source.as_deref(), Some("account")); + assert!(row("account_id").tags.contains(&"written as ucet".to_string())); + assert!( + row("accounting").alias_source.is_none(), + "a declared column is named where it is described" + ); + } + #[test] fn the_draft_never_trusts_the_posted_table_list() { // `existing_profile_tables` is what duplicate-name checks read, so it diff --git a/web/src/pages/add_table/ui.rs b/web/src/pages/add_table/ui.rs index 976e605c..722ace2d 100644 --- a/web/src/pages/add_table/ui.rs +++ b/web/src/pages/add_table/ui.rs @@ -125,6 +125,34 @@ mod tests { assert!(html.contains("TIMESTAMPTZ")); } + /// The columns an ACCOUNTING row generates get a name field of their own, + /// which is what makes them aliasable at creation time. + #[test] + fn generated_accounting_columns_get_an_alias_field() { + let mut page = page(); + page.draft.columns.added.push(ColumnDefinition { + name: "accounting".to_string(), + data_type: "accounting".to_string(), + indexed: false, + quantity_ledger: false, + money_mode: MoneyMode::Exact, + currency: "EUR".to_string(), + }); + + let html = render_builder(&page); + + assert!(!html.contains("Template error"), "{html}"); + for generated in ["name", "tax_point_date", "debit", "credit", "account"] { + assert!( + html.contains(&format!( + r#""# + )), + "`{generated}` should be aliasable" + ); + } + assert!(html.contains(r#"name="generated_alias_names""#)); + } + #[test] fn the_page_offers_the_new_profile_option_and_the_currency_list() { let html = render_page(&page()); diff --git a/web/src/pages/admin/table_definition/logic.rs b/web/src/pages/admin/table_definition/logic.rs index 61942712..5706ac85 100644 --- a/web/src/pages/admin/table_definition/logic.rs +++ b/web/src/pages/admin/table_definition/logic.rs @@ -246,6 +246,10 @@ pub(crate) async fn add_columns( table_name: inputs.selection.table.clone(), columns: proto_columns(&inputs.columns.added), indexes: inputs.columns.selected_index_names(), + // The append panel names its columns itself; the only generated ones it + // can produce are the phone and IBAN companions, which it does not + // offer to rename. The table's own rename form is where that is done. + generated_aliases: Vec::new(), }; let Ok(request) = authenticated_request(&headers, request) else { return Redirect::to("/login").into_response(); diff --git a/web/templates/pages/add_table/builder.html b/web/templates/pages/add_table/builder.html index 3d8b6cf0..9ef48686 100644 --- a/web/templates/pages/add_table/builder.html +++ b/web/templates/pages/add_table/builder.html @@ -238,6 +238,7 @@
  • account_id system column — the account each row is posted to, written as account
    • {% endif %} +

    Each of these can be given a name of your own in the column list once the row is added; leave a field empty to keep the name shown here.

    These names are reserved: no column of your own may use them, and the table may hold only one {{ page.draft.columns.pending_compound_name() }}. They land where this row sits in the column list, and can be moved with it.

    {% endif %} @@ -276,7 +277,20 @@ ↳ {% endif %} - {{ row.name }} + + {{ row.name }} + {# + A generated column is named by the backend, so it cannot be + named in the request that creates the table — but the name is a + display name, and an alias here is applied as a rename as soon + as the table exists. Leaving it empty keeps the backend's name. + #} + {% if let Some(source) = row.alias_source %} + + + {% endif %} + {{ row.data_type }} {# A compound column expands into schema-managed companions, so