diff --git a/common/src/system_column.rs b/common/src/system_column.rs index 3eb6bd37..045623b7 100644 --- a/common/src/system_column.rs +++ b/common/src/system_column.rs @@ -46,6 +46,12 @@ pub const ACCOUNT_REFERENCE_COLUMN: &str = "account_id"; pub const ACCOUNT_API_COLUMN: &str = "account"; /// Internal version of a managed row. It is never exposed as editable data. pub const ROW_VERSION_COLUMN: &str = "version"; +/// Records the user who closed an accounting journal. It is exposed only by +/// the dedicated accounting API, never by the generic table API. +pub const CLOSED_BY_USER_ID_COLUMN: &str = "closed_by_user_id"; + +/// Fixed server-owned columns that generic table APIs must omit entirely. +pub const INTERNAL_COLUMN_NAMES: [&str; 2] = [ROW_VERSION_COLUMN, CLOSED_BY_USER_ID_COLUMN]; /// The longest name any system column carries physically. /// @@ -71,17 +77,33 @@ const fn longest_system_column_name() -> usize { longest } - let longest = longest_of( - &LEADING_SYSTEM_COLUMNS, - if ACCOUNT_REFERENCE_COLUMN.len() > ROW_VERSION_COLUMN.len() { - ACCOUNT_REFERENCE_COLUMN.len() - } else { - ROW_VERSION_COLUMN.len() - }, - ); + const fn longest_name(names: &[&str], mut longest: usize) -> usize { + let mut index = 0; + while index < names.len() { + let length = names[index].len(); + if length > longest { + longest = length; + } + index += 1; + } + longest + } + + let longest = longest_name(&INTERNAL_COLUMN_NAMES, ACCOUNT_REFERENCE_COLUMN.len()); + let longest = longest_of(&LEADING_SYSTEM_COLUMNS, longest); longest_of(&TRAILING_SYSTEM_COLUMNS, longest) } +/// Fixed internal column names. Link-version columns are also internal, but +/// their generated names are loaded from table metadata at runtime. +pub fn internal_column_names() -> impl Iterator { + INTERNAL_COLUMN_NAMES.into_iter() +} + +pub fn is_internal_column(name: &str) -> bool { + internal_column_names().any(|internal_name| internal_name == name) +} + /// Every system column name, whether or not the column is on a given table. /// /// A name is reserved for all tables even when only some tables carry the @@ -92,12 +114,13 @@ pub fn system_column_names() -> impl Iterator { .chain(TRAILING_SYSTEM_COLUMNS.iter()) .map(|column| column.name) .chain(std::iter::once(ACCOUNT_REFERENCE_COLUMN)) - .chain(std::iter::once(ROW_VERSION_COLUMN)) + .chain(internal_column_names()) } -/// Whether `name` belongs to the system column vocabulary and is safe to show -/// to a client as-is. Whether a user may claim a virtual API name as an alias -/// is decided with the table's capabilities in scope. +/// Whether `name` belongs to the reserved system column vocabulary. Internal +/// columns must still be removed before a row or structure reaches a client. +/// Whether a user may claim a virtual API name as an alias is decided with the +/// table's capabilities in scope. pub fn is_system_column(name: &str) -> bool { name == ACCOUNT_API_COLUMN || system_column_names().any(|system_name| system_name == name) } @@ -114,7 +137,8 @@ pub fn system_column_name_list() -> String { #[cfg(test)] mod tests { use super::{ - ACCOUNT_API_COLUMN, ACCOUNT_REFERENCE_COLUMN, LONGEST_SYSTEM_COLUMN_NAME, is_system_column, + ACCOUNT_API_COLUMN, ACCOUNT_REFERENCE_COLUMN, CLOSED_BY_USER_ID_COLUMN, + LONGEST_SYSTEM_COLUMN_NAME, ROW_VERSION_COLUMN, is_internal_column, is_system_column, system_column_name_list, system_column_names, }; @@ -131,6 +155,15 @@ mod tests { assert!(is_system_column(ACCOUNT_API_COLUMN)); } + #[test] + fn internal_columns_are_reserved_but_not_public() { + for name in [ROW_VERSION_COLUMN, CLOSED_BY_USER_ID_COLUMN] { + assert!(is_system_column(name)); + assert!(is_internal_column(name)); + } + assert!(!is_internal_column(ACCOUNT_REFERENCE_COLUMN)); + } + #[test] fn an_ordinal_is_not_a_system_column() { assert!(!is_system_column("1")); @@ -155,7 +188,7 @@ mod tests { fn the_name_list_reads_as_a_sentence_fragment() { assert_eq!( system_column_name_list(), - "'id', 'deleted', 'row_revision', 'created_at', 'account_id', 'version', 'account'" + "'id', 'deleted', 'row_revision', 'created_at', 'account_id', 'version', 'closed_by_user_id', 'account'" ); } } diff --git a/server b/server index 8b7a88ec..52ca7446 160000 --- a/server +++ b/server @@ -1 +1 @@ -Subproject commit 8b7a88ec3ee3095e5a25f305d68b477696b57939 +Subproject commit 52ca7446cce0c15095d46519995e7478e2b6eca5 diff --git a/web/src/pages/admin/admin/loader.rs b/web/src/pages/admin/admin/loader.rs index f991b446..bb2668e2 100644 --- a/web/src/pages/admin/admin/loader.rs +++ b/web/src/pages/admin/admin/loader.rs @@ -3,7 +3,11 @@ use axum::http::HeaderMap; use crate::{ AppState, auth::GetAuthorizationRequest, - definitions::{common::Empty, table_structure::GetTableStructureRequest}, + definitions::{ + common::Empty, table_definition::GetTableCatalogRequest, + table_structure::GetTableStructureRequest, + }, + pages::GLOBAL_SCOPE, services::{AuthenticationError, authenticated_request}, }; @@ -40,32 +44,52 @@ pub(crate) async fn load_admin_page( .map_err(|error| LoadError::Backend(error.message().to_string()))? .into_inner(); - let profiles = profile_tree - .profiles - .iter() - .map(|profile| ProfileView { - name: profile.name.clone(), - table_count: profile.tables.len(), - }) - .collect::>(); + // Global tables belong to every profile, so the tree repeats them under + // each one. They are browsed under the global scope instead, and a + // profile shows only the tables it owns. + let global_tables = definitions + .get_table_catalog( + authenticated_request(headers, GetTableCatalogRequest { profile_name: None }) + .map_err(authentication_error)?, + ) + .await + .map_err(|error| LoadError::Backend(error.message().to_string()))? + .into_inner() + .tables; + + let mut profiles = vec![ProfileView { + label: "Global".to_string(), + scope: GLOBAL_SCOPE.to_string(), + table_count: global_tables.len(), + global: true, + }]; + profiles.extend(profile_tree.profiles.iter().map(|profile| ProfileView { + label: profile.name.clone(), + scope: profile.name.clone(), + table_count: profile.tables.iter().filter(|table| !table.global).count(), + global: false, + })); let selected_profile = (!selection.profile.is_empty()).then_some(selection.profile); - let profile = match selected_profile.as_deref() { + let selected_tables = match selected_profile.as_deref() { + Some(GLOBAL_SCOPE) => Some(global_tables.as_slice()), Some(name) => Some( profile_tree .profiles .iter() .find(|profile| profile.name == name) - .ok_or_else(|| LoadError::InvalidSelection(format!("Unknown profile '{name}'")))?, + .ok_or_else(|| LoadError::InvalidSelection(format!("Unknown profile '{name}'")))? + .tables + .as_slice(), ), None => None, }; - let tables = profile - .map(|profile| { - profile - .tables + let tables = selected_tables + .map(|tables| { + tables .iter() + .filter(|table| table.global == (selected_profile.as_deref() == Some(GLOBAL_SCOPE))) .map(|table| TableView { name: table.name.clone(), // One entry per link, named by the column carrying it, so a @@ -85,7 +109,7 @@ pub(crate) async fn load_admin_page( let selected_table = (!selection.table.is_empty()).then_some(selection.table); if let Some(table_name) = selected_table.as_deref() { - if profile.is_none() || !tables.iter().any(|table| table.name == table_name) { + if selected_tables.is_none() || !tables.iter().any(|table| table.name == table_name) { return Err(LoadError::InvalidSelection(format!( "Table '{table_name}' is not part of the selected profile" ))); diff --git a/web/src/pages/admin/admin/state.rs b/web/src/pages/admin/admin/state.rs index aa8e9548..30d0018c 100644 --- a/web/src/pages/admin/admin/state.rs +++ b/web/src/pages/admin/admin/state.rs @@ -21,6 +21,11 @@ pub(crate) struct AdminPageState { } impl AdminPageState { + /// Whether the pane is pointed at the global scope rather than a profile. + pub(crate) fn is_global(&self) -> bool { + self.selected_profile.as_deref() == Some(crate::pages::GLOBAL_SCOPE) + } + /// The columns the user defined, in the order the backend reported them. /// The columns pane renders these first. pub(crate) fn user_columns(&self) -> Vec<&ColumnView> { @@ -34,10 +39,16 @@ impl AdminPageState { } } +/// One entry in the Profiles pane: a real profile, or the global scope the +/// shared tables live in. #[derive(Debug)] pub(crate) struct ProfileView { - pub name: String, + /// What the pane shows. + pub label: String, + /// What the pane posts as `?profile=`. + pub scope: String, pub table_count: usize, + pub global: bool, } #[derive(Debug)] diff --git a/web/src/pages/admin/admin/ui.rs b/web/src/pages/admin/admin/ui.rs index 549d6d37..0cd743a8 100644 --- a/web/src/pages/admin/admin/ui.rs +++ b/web/src/pages/admin/admin/ui.rs @@ -81,6 +81,45 @@ mod tests { } } + #[test] + fn the_profiles_pane_offers_the_global_scope() { + use crate::pages::admin::admin::state::ProfileView; + + let page = AdminPageState { + nav: Nav::default(), + profiles: vec![ + ProfileView { + label: "Global".to_string(), + scope: crate::pages::GLOBAL_SCOPE.to_string(), + table_count: 2, + global: true, + }, + ProfileView { + label: "books".to_string(), + scope: "books".to_string(), + table_count: 5, + global: false, + }, + ], + selected_profile: Some(crate::pages::GLOBAL_SCOPE.to_string()), + tables: Vec::new(), + selected_table: None, + columns: Vec::new(), + can_manage_tables: true, + can_manage_scripts: true, + can_manage_validations: true, + can_export: true, + }; + + let html = render_workspace(&page); + assert!(html.contains(r#"name="profile" value="__global""#)); + assert!(html.contains("shared by every profile")); + // Selecting it marks it, and its empty state names the global scope + // rather than a profile that has no tables. + assert!(html.contains("browser-item selected")); + assert!(html.contains("There are no global tables.")); + } + #[test] fn system_columns_render_after_the_user_defined_ones() { let column = |name: &str, system: bool| crate::pages::admin::admin::state::ColumnView { diff --git a/web/src/pages/admin/table_definition/state.rs b/web/src/pages/admin/table_definition/state.rs index 3b8d1400..6c1bf7eb 100644 --- a/web/src/pages/admin/table_definition/state.rs +++ b/web/src/pages/admin/table_definition/state.rs @@ -7,7 +7,7 @@ use crate::schema::{ColumnCatalog, ColumnDraft}; -pub(crate) const GLOBAL_SCOPE: &str = "__global"; +pub(crate) use crate::pages::GLOBAL_SCOPE; /// The profile and table the workspace is pointed at. Arrives as a query /// string on the selector and on the column-panel endpoints, and as hidden diff --git a/web/src/pages/mod.rs b/web/src/pages/mod.rs index 5e1015b4..20afce8c 100644 --- a/web/src/pages/mod.rs +++ b/web/src/pages/mod.rs @@ -1,3 +1,10 @@ +/// The pseudo-profile a page posts when the selection is the global scope. +/// +/// Global tables belong to every profile, so the backend keeps them in a +/// schema of their own rather than in any one profile's. A selector that lists +/// profiles offers this alongside them, and the backend recognises the name. +pub(crate) const GLOBAL_SCOPE: &str = "__global"; + pub(crate) mod add_logic; pub(crate) mod add_table; pub(crate) mod add_validation; diff --git a/web/templates/pages/admin/admin/workspace.html b/web/templates/pages/admin/admin/workspace.html index c564e97d..aa1c37d2 100644 --- a/web/templates/pages/admin/admin/workspace.html +++ b/web/templates/pages/admin/admin/workspace.html @@ -12,10 +12,13 @@ {% else %} {% for profile in page.profiles %}
-
{% endfor %} @@ -29,7 +32,11 @@ {% if page.selected_profile.is_none() %}

Select a profile to see its tables.

{% else if page.tables.is_empty() %} -

This profile has no tables.

+ {% if page.is_global() %} +

There are no global tables.

+ {% else %} +

This profile has no tables of its own.

+ {% endif %} {% else %} {% for table in page.tables %}