From f8e483efa89a20dbe3718111703edb31340f5ac5 Mon Sep 17 00:00:00 2001 From: Priec Date: Wed, 12 Aug 2026 23:40:14 +0200 Subject: [PATCH] web routing is POG now --- server | 2 +- web/src/lib.rs | 27 +- web/src/pages/add_table/logic.rs | 5 +- web/src/pages/admin/admin/loader.rs | 1 + web/src/pages/admin/admin/state.rs | 9 + web/src/pages/admin/admin/ui.rs | 37 +- .../pages/admin/table_definition/loader.rs | 109 +----- web/src/pages/admin/table_definition/logic.rs | 228 +++++++++--- web/src/pages/admin/table_definition/mod.rs | 48 ++- web/src/pages/admin/table_definition/state.rs | 58 +-- web/src/pages/admin/table_definition/ui.rs | 333 +++++++++++------ web/static/app.css | 19 +- web/templates/pages/admin/admin/admin.html | 3 +- .../pages/admin/admin/workspace.html | 29 ++ .../admin/table_definition/column_panel.html | 8 +- .../pages/admin/table_definition/columns.html | 23 ++ .../admin/table_definition/columns_panel.html | 93 +++++ .../pages/admin/table_definition/context.html | 76 ++++ .../pages/admin/table_definition/copy.html | 14 + .../admin/table_definition/copy_panel.html | 33 ++ .../pages/admin/table_definition/delete.html | 14 + .../admin/table_definition/delete_panel.html | 39 ++ .../admin/table_definition/feedback.html | 24 ++ .../admin/table_definition/from_template.html | 14 + .../table_definition/from_template_panel.html | 53 +++ .../pages/admin/table_definition/history.html | 39 ++ .../table_definition/table_definition.html | 32 -- .../admin/table_definition/workspace.html | 342 ------------------ 28 files changed, 1015 insertions(+), 697 deletions(-) create mode 100644 web/templates/pages/admin/table_definition/columns.html create mode 100644 web/templates/pages/admin/table_definition/columns_panel.html create mode 100644 web/templates/pages/admin/table_definition/context.html create mode 100644 web/templates/pages/admin/table_definition/copy.html create mode 100644 web/templates/pages/admin/table_definition/copy_panel.html create mode 100644 web/templates/pages/admin/table_definition/delete.html create mode 100644 web/templates/pages/admin/table_definition/delete_panel.html create mode 100644 web/templates/pages/admin/table_definition/feedback.html create mode 100644 web/templates/pages/admin/table_definition/from_template.html create mode 100644 web/templates/pages/admin/table_definition/from_template_panel.html create mode 100644 web/templates/pages/admin/table_definition/history.html delete mode 100644 web/templates/pages/admin/table_definition/table_definition.html delete mode 100644 web/templates/pages/admin/table_definition/workspace.html diff --git a/server b/server index 1a3c0493..0b0cb105 160000 --- a/server +++ b/server @@ -1 +1 @@ -Subproject commit 1a3c049329d7f215a433ddc78e9721f59c924a47 +Subproject commit 0b0cb105beb11171f541d612a8a5f5950b1a43bc diff --git a/web/src/lib.rs b/web/src/lib.rs index de3401ac..f7382226 100644 --- a/web/src/lib.rs +++ b/web/src/lib.rs @@ -286,8 +286,14 @@ mod tests { /// answers with the redirect to the login page rather than a 404 or a call /// to the backend. #[tokio::test] - async fn the_table_definition_workspace_is_mounted_and_needs_a_session() { - for path in ["/admin/table-definition", "/admin/table-definition/workspace"] { + async fn the_table_definition_pages_are_mounted_and_need_a_session() { + for path in [ + "/admin/tables/columns", + "/admin/tables/delete", + "/admin/profiles/copy", + "/admin/profiles/history", + "/admin/tables/from-template", + ] { let (status, _) = get(path).await; assert_eq!( status, @@ -296,13 +302,18 @@ mod tests { ); } + // The workspace these came out of is gone, and anything still pointing + // at it lands on the browser rather than on a 404. + let (status, _) = get("/admin/table-definition").await; + assert_eq!(status, axum::http::StatusCode::PERMANENT_REDIRECT); + for path in [ - "/admin/table-definition/columns", - "/admin/table-definition/columns/builder", - "/admin/table-definition/rename", - "/admin/table-definition/delete", - "/admin/table-definition/copy", - "/admin/table-definition/invoice-template", + "/admin/tables/columns", + "/admin/tables/columns/builder", + "/admin/tables/rename", + "/admin/tables/delete", + "/admin/profiles/copy", + "/admin/tables/from-template", ] { let response = test_router() .oneshot( diff --git a/web/src/pages/add_table/logic.rs b/web/src/pages/add_table/logic.rs index 241525b8..90be2be4 100644 --- a/web/src/pages/add_table/logic.rs +++ b/web/src/pages/add_table/logic.rs @@ -108,8 +108,11 @@ pub(crate) async fn create_table( let result = definitions.post_table_definition(request).await; match result { Ok(response) if response.get_ref().success => { + // The admin panel, with the new table selected: it is the browser + // the actions on a table are reached from, so landing there is + // landing next to everything that can be done to it. let location = format!( - "/admin/table-definition?profile={profile_name}&table={}", + "/admin?profile={profile_name}&table={}", page.draft.table_name ); let Ok(location) = HeaderValue::try_from(location) else { diff --git a/web/src/pages/admin/admin/loader.rs b/web/src/pages/admin/admin/loader.rs index bb2668e2..32255067 100644 --- a/web/src/pages/admin/admin/loader.rs +++ b/web/src/pages/admin/admin/loader.rs @@ -102,6 +102,7 @@ pub(crate) async fn load_admin_page( }) .collect(), row_display_columns: table.row_display_columns.clone(), + table_kind: table.table_kind.clone(), }) .collect::>() }) diff --git a/web/src/pages/admin/admin/state.rs b/web/src/pages/admin/admin/state.rs index 30d0018c..0f0d81f9 100644 --- a/web/src/pages/admin/admin/state.rs +++ b/web/src/pages/admin/admin/state.rs @@ -56,6 +56,15 @@ pub(crate) struct TableView { pub name: String, pub depends_on: Vec, pub row_display_columns: Vec, + pub table_kind: String, +} + +impl TableView { + /// System tables are backend-managed: the server refuses every structural + /// write on them, so the pane offers none of the action links for one. + pub(crate) fn is_system(&self) -> bool { + self.table_kind == "system" + } } #[derive(Debug)] diff --git a/web/src/pages/admin/admin/ui.rs b/web/src/pages/admin/admin/ui.rs index 0cd743a8..89cbb95a 100644 --- a/web/src/pages/admin/admin/ui.rs +++ b/web/src/pages/admin/admin/ui.rs @@ -69,7 +69,6 @@ mod tests { }; let html = render_page(&page); for route in [ - "/admin/table-definition", "/admin/tables/new", "/admin/logic/new", "/admin/validation/new", @@ -120,6 +119,42 @@ mod tests { assert!(html.contains("There are no global tables.")); } + /// Selecting a table has to put what can be done to it on the screen. It + /// is the only way in: these pages take their selection from the URL, and + /// nobody is going to type one. + #[test] + fn selecting_a_table_shows_what_can_be_done_to_it() { + use crate::pages::admin::admin::state::TableView; + + let page = AdminPageState { + nav: Nav::default(), + profiles: Vec::new(), + selected_profile: Some("books".to_string()), + tables: vec![TableView { + name: "invoice".to_string(), + depends_on: Vec::new(), + row_display_columns: vec!["number".to_string()], + table_kind: "dynamic".to_string(), + }], + selected_table: Some("invoice".to_string()), + columns: Vec::new(), + can_manage_tables: true, + can_manage_scripts: true, + can_manage_validations: true, + can_export: true, + }; + + let html = render_workspace(&page); + for route in [ + "/admin/tables/columns?profile=books&table=invoice", + "/admin/tables/delete?profile=books&table=invoice", + "/admin/tables/new?profile=books", + "/admin/profiles/copy?profile=books", + ] { + assert!(html.contains(route), "missing {route}\n{html}"); + } + } + #[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/loader.rs b/web/src/pages/admin/table_definition/loader.rs index ebe3ba37..ddeb3255 100644 --- a/web/src/pages/admin/table_definition/loader.rs +++ b/web/src/pages/admin/table_definition/loader.rs @@ -1,9 +1,12 @@ -//! Reads everything the workspace shows. +//! Reads the context every one of the five pages shows. +//! +//! One loader serves them all, because they all need the same thing: which +//! table is being worked on, and what it currently is. Four calls, in this +//! order — the column-type catalog is the vocabulary the append panel offers, +//! the profile tree names the tables, the profile details describe the +//! selected table's columns and scripts, and the rename history explains how +//! those columns got their names. //! -//! Four calls, in this order: the column-type catalog is the vocabulary the -//! append panel offers, the profile tree names the profiles and their tables, -//! the profile details describe the selected table's columns and scripts, and -//! the rename history explains how those columns got their names. //! Nothing here trusts the posted selection — a profile or table that is gone //! is dropped from the selection rather than reported as an error, because the //! commonest way to get here with a stale one is having just deleted it. @@ -13,7 +16,7 @@ use tonic::transport::Channel; use crate::{ AppState, - auth::{GetAuthorizationRequest, ListGrantableObjectsRequest, ListRolePermissionsRequest, ListRolesRequest}, + auth::GetAuthorizationRequest, definitions::{ common::Empty, table_definition::{ @@ -26,8 +29,8 @@ use crate::{ }; use super::state::{ - DetailColumn, GLOBAL_SCOPE, LoadError, PageInputs, RenameEntry, ScriptView, TableDefinitionPageState, - TableDetailView, TablePermissionAction, TableRolePermissions, TableSummary, + DetailColumn, GLOBAL_SCOPE, LoadError, PageInputs, RenameEntry, ScriptView, + TableDefinitionPageState, TableDetailView, TableSummary, }; /// Reads the column-type vocabulary on its own. @@ -92,11 +95,13 @@ pub(crate) async fn load_page( let profiles = tree .profiles .iter() - .map(|profile| profile.name.clone()) + .map(|profile| profile.name.as_str()) .collect::>(); // A profile that no longer exists takes the table selection with it. - if inputs.selection.profile != GLOBAL_SCOPE && !profiles.contains(&inputs.selection.profile) { + if inputs.selection.profile != GLOBAL_SCOPE + && !profiles.contains(&inputs.selection.profile.as_str()) + { inputs.selection.profile.clear(); inputs.selection.table.clear(); } @@ -127,7 +132,6 @@ pub(crate) async fn load_page( format!("{} ({})", dependency.table_name, dependency.column_name) }) .collect(), - row_display_columns: table.row_display_columns.clone(), }) .collect::>() }) @@ -195,8 +199,6 @@ pub(crate) async fn load_page( }) .collect(), row_display_columns: table.row_display_columns, - table_kind: table.table_kind, - name: table.name, }) } false => None, @@ -230,84 +232,8 @@ pub(crate) async fn load_page( false => Vec::new(), }; - let mut permission_object = String::new(); - let mut role_permissions = Vec::new(); - if inputs.selection.has_table() - && crate::authz::can_manage(&authorization, crate::authz::STRUCT_ROLE) - { - let expected_object = crate::authz::table_object( - &inputs.selection.profile, - &inputs.selection.table, - ); - let roles = auth - .list_roles( - authenticated_request(headers, ListRolesRequest {}) - .map_err(|_| LoadError::Unauthenticated)?, - ) - .await - .map_err(|error| LoadError::Backend(error.message().to_string()))? - .into_inner() - .roles; - for role in roles.into_iter().filter(|role| role.kind == "data") { - let grantable = auth - .list_grantable_objects( - authenticated_request( - headers, - ListGrantableObjectsRequest { - target_role: role.name.clone(), - }, - ) - .map_err(|_| LoadError::Unauthenticated)?, - ) - .await - .map_err(|error| LoadError::Backend(error.message().to_string()))? - .into_inner() - .objects - .into_iter() - .find(|object| object.object == expected_object); - let Some(grantable) = grantable else { - continue; - }; - permission_object = expected_object.clone(); - let permissions = auth - .list_role_permissions( - authenticated_request( - headers, - ListRolePermissionsRequest { - role: role.name.clone(), - }, - ) - .map_err(|_| LoadError::Unauthenticated)?, - ) - .await - .map_err(|error| LoadError::Backend(error.message().to_string()))? - .into_inner(); - role_permissions.push(TableRolePermissions { - role: role.name, - actions: grantable - .allowed_actions - .into_iter() - .map(|action| TablePermissionAction { - direct: crate::authz::is_direct_permission( - &permissions.permissions, - &expected_object, - &action, - ), - effective: crate::authz::permissions_permit( - &permissions.effective_permissions, - &expected_object, - &action, - ), - action, - }) - .collect(), - }); - } - } - Ok(TableDefinitionPageState { nav: crate::ui::Nav::new(headers, "admin").with_authorization(&authorization), - profiles, tables, detail, history, @@ -320,7 +246,8 @@ pub(crate) async fn load_page( error: inputs.error, sql: inputs.sql, generated: inputs.generated, - permission_object, - role_permissions, + // Overwritten by the handler, which is the only thing that knows + // which of the pages it is answering for. + active: "", }) } diff --git a/web/src/pages/admin/table_definition/logic.rs b/web/src/pages/admin/table_definition/logic.rs index c2c2974a..61942712 100644 --- a/web/src/pages/admin/table_definition/logic.rs +++ b/web/src/pages/admin/table_definition/logic.rs @@ -1,17 +1,20 @@ -//! The workspace's request handlers — one per `TableDefinition` write. +//! The request handlers behind the five table-definition pages. //! -//! Every write answers with the whole workspace, re-read from the backend, so -//! what the user sees after a change is the definition as it now is rather -//! than the form they submitted. A refused write answers the same way but with -//! 422 and the backend's own message, which `ui/base.html` swaps in because a -//! 4xx still carries the explanation. +//! Each page has a GET that renders it and, except for the read-only history, +//! a POST per write it offers. A write answers with its own page's body, +//! re-read from the backend, so what the user sees after a change is the +//! definition as it now is rather than the form they submitted. A refused +//! write answers the same way but with 422 and the backend's own message, +//! which `ui/base.html` swaps in because a 4xx still carries the explanation. //! -//! The column panel is the exception: staging a column changes nothing on the -//! server, so those interactions swap only the panel. +//! Two exceptions. Staging a column changes nothing on the server, so those +//! interactions swap only the column panel. And a successful delete leaves no +//! page to return to — the table it was about is gone — so it redirects to the +//! admin panel instead. use axum::{ extract::{Query, State}, - http::{HeaderMap, StatusCode}, + http::{HeaderMap, HeaderValue, StatusCode}, response::{Html, IntoResponse, Redirect, Response}, }; use axum_extra::extract::Form; @@ -30,36 +33,124 @@ use super::{ loader::{self, load_page}, state::{ CopyForm, DeleteForm, GeneratedTableView, InvoiceTemplateForm, LoadError, PageInputs, - RenameForm, Selection, + RenameForm, Selection, TableDefinitionPageState, }, ui, }; -/// GET /admin/table-definition -pub(crate) async fn page( +/// Which page a response belongs to: what the switcher marks as current, and +/// which fragment a write on it swaps back. +#[derive(Clone, Copy)] +enum Page { + Columns, + Delete, + Copy, + Template, + History, +} + +impl Page { + fn name(self) -> &'static str { + match self { + Self::Columns => "columns", + Self::Delete => "delete", + Self::Copy => "copy", + Self::Template => "template", + Self::History => "history", + } + } + + fn render_page(self, page: &TableDefinitionPageState) -> String { + match self { + Self::Columns => ui::render_columns_page(page), + Self::Delete => ui::render_delete_page(page), + Self::Copy => ui::render_copy_page(page), + Self::Template => ui::render_template_page(page), + Self::History => ui::render_history_page(page), + } + } + + /// The `#table-panel` swap. History has no writes, so it never asks. + fn render_fragment(self, page: &TableDefinitionPageState) -> String { + match self { + Self::Columns => ui::render_columns_fragment(page), + Self::Delete => ui::render_delete_fragment(page), + Self::Copy => ui::render_copy_fragment(page), + Self::Template => ui::render_template_fragment(page), + Self::History => ui::render_history_page(page), + } + } +} + +// ── The pages ─────────────────────────────────────────────────────────────── + +/// GET /admin/tables/columns +pub(crate) async fn columns_page( State(state): State, headers: HeaderMap, Query(selection): Query, ) -> Response { - match load_page(state, &headers, PageInputs::for_selection(selection)).await { - Ok(page) => Html(ui::render_page(&page)).into_response(), - Err(error) => load_error_response(error), - } + show(state, headers, PageInputs::for_selection(selection), Page::Columns).await } -/// GET /admin/table-definition/workspace — the swap when the selection changes. -pub(crate) async fn workspace( +/// GET /admin/tables/delete +pub(crate) async fn delete_page( State(state): State, headers: HeaderMap, Query(selection): Query, ) -> Response { - match load_page(state, &headers, PageInputs::for_selection(selection)).await { - Ok(page) => Html(ui::render_workspace(&page)).into_response(), + show(state, headers, PageInputs::for_selection(selection), Page::Delete).await +} + +/// GET /admin/profiles/copy +pub(crate) async fn copy_page( + State(state): State, + headers: HeaderMap, + Query(selection): Query, +) -> Response { + show(state, headers, profile_inputs(selection), Page::Copy).await +} + +/// GET /admin/tables/from-template +pub(crate) async fn template_page( + State(state): State, + headers: HeaderMap, + Query(selection): Query, +) -> Response { + show(state, headers, profile_inputs(selection), Page::Template).await +} + +/// GET /admin/profiles/history +pub(crate) async fn history_page( + State(state): State, + headers: HeaderMap, + Query(selection): Query, +) -> Response { + show(state, headers, PageInputs::for_selection(selection), Page::History).await +} + +/// The profile-wide pages act on a profile, so a table in the query is +/// dropped rather than carried into a form that has no use for it. +fn profile_inputs(selection: Selection) -> PageInputs { + PageInputs::for_selection(Selection { + profile: selection.profile, + table: String::new(), + }) +} + +async fn show(state: AppState, headers: HeaderMap, inputs: PageInputs, page: Page) -> Response { + match load_page(state, &headers, inputs).await { + Ok(mut loaded) => { + loaded.active = page.name(); + Html(page.render_page(&loaded)).into_response() + } Err(error) => load_error_response(error), } } -/// POST /admin/table-definition/columns/builder — staging a column to append. +// ── The writes ────────────────────────────────────────────────────────────── + +/// POST /admin/tables/columns/builder — staging a column to append. /// /// Nothing is written here; the panel is swapped back with the column added, /// removed, or its index toggled, exactly as the Add-table builder works. @@ -100,12 +191,15 @@ pub(crate) async fn update_columns( } match load_page(state, &headers, inputs).await { - Ok(page) => Html(ui::render_column_panel(&page)).into_response(), + Ok(mut loaded) => { + loaded.active = Page::Columns.name(); + Html(ui::render_column_panel(&loaded)).into_response() + } Err(error) => load_error_response(error), } } -/// POST /admin/table-definition/columns — AddTableColumns. +/// POST /admin/tables/columns — AddTableColumns. pub(crate) async fn add_columns( State(state): State, headers: HeaderMap, @@ -128,13 +222,15 @@ pub(crate) async fn add_columns( inputs.columns = form.to_draft(catalog.clone(), false); if !inputs.selection.has_table() { - return refuse(state, headers, inputs, "Select a table first.".to_string()).await; + return refuse(state, headers, inputs, Page::Columns, "Select a table first.".to_string()) + .await; } if inputs.columns.is_empty() { return refuse( state, headers, inputs, + Page::Columns, "Describe at least one column before adding.".to_string(), ) .await; @@ -142,7 +238,7 @@ pub(crate) async fn add_columns( // The same checks the server runs, applied to a draft that may have been // rebuilt from a posted form rather than through the panel. if let Err(message) = inputs.columns.validate() { - return refuse(state, headers, inputs, message).await; + return refuse(state, headers, inputs, Page::Columns, message).await; } let request = AddTableColumnsRequest { @@ -166,7 +262,7 @@ pub(crate) async fn add_columns( )); // The columns are the table's now, so the panel starts empty. inputs.columns = crate::schema::ColumnDraft::for_append(catalog); - respond(state, headers, inputs, StatusCode::OK).await + respond(state, headers, inputs, Page::Columns, StatusCode::OK).await } Ok(response) => { let message = response.into_inner().sql; @@ -175,13 +271,15 @@ pub(crate) async fn add_columns( } else { message }; - refuse(state, headers, inputs, message).await + refuse(state, headers, inputs, Page::Columns, message).await + } + Err(error) => { + refuse(state, headers, inputs, Page::Columns, error.message().to_string()).await } - Err(error) => refuse(state, headers, inputs, error.message().to_string()).await, } } -/// POST /admin/table-definition/rename — RenameColumnAlias. +/// POST /admin/tables/rename — RenameColumnAlias. pub(crate) async fn rename_column( State(state): State, headers: HeaderMap, @@ -202,6 +300,7 @@ pub(crate) async fn rename_column( state, headers, inputs, + Page::Columns, "Choose a column and type its new name.".to_string(), ) .await; @@ -226,7 +325,7 @@ pub(crate) async fn rename_column( table: form.table, ..Default::default() }; - respond(state, headers, inputs, StatusCode::OK).await + respond(state, headers, inputs, Page::Columns, StatusCode::OK).await } Ok(response) => { let message = response.into_inner().message; @@ -235,13 +334,18 @@ pub(crate) async fn rename_column( } else { message }; - refuse(state, headers, inputs, message).await + refuse(state, headers, inputs, Page::Columns, message).await + } + Err(error) => { + refuse(state, headers, inputs, Page::Columns, error.message().to_string()).await } - Err(error) => refuse(state, headers, inputs, error.message().to_string()).await, } } -/// POST /admin/table-definition/delete — DeleteTable. +/// POST /admin/tables/delete — DeleteTable. +/// +/// On success there is nothing left for this page to show, so it hands the +/// browser to the admin panel with the profile still selected. pub(crate) async fn delete_table( State(state): State, headers: HeaderMap, @@ -251,7 +355,7 @@ pub(crate) async fn delete_table( return rejection; } - let mut inputs = PageInputs::for_selection(Selection { + let inputs = PageInputs::for_selection(Selection { profile: form.profile.clone(), table: form.table.clone(), }); @@ -263,6 +367,7 @@ pub(crate) async fn delete_table( state, headers, inputs, + Page::Delete, "Type the table's name exactly to confirm the deletion.".to_string(), ) .await; @@ -279,11 +384,18 @@ pub(crate) async fn delete_table( let mut definitions = state.definitions.clone(); match definitions.delete_table(request).await { Ok(response) if response.get_ref().success => { - inputs.status = Some(response.into_inner().message); - // Whatever was selected is gone; the loader drops it, and this - // keeps the workspace from asking for it again. - inputs.selection.table.clear(); - respond(state, headers, inputs, StatusCode::OK).await + // The profile may have gone with it, in which case the admin panel + // refuses the selection; landing on the bare panel is what the + // caller wants then anyway. + let location = format!("/admin?profile={}", form.profile); + match HeaderValue::try_from(location) { + Ok(location) => { + let mut response = Html(String::new()).into_response(); + response.headers_mut().insert("hx-redirect", location); + response + } + Err(_) => (StatusCode::INTERNAL_SERVER_ERROR, "Invalid redirect").into_response(), + } } Ok(response) => { let message = response.into_inner().message; @@ -292,13 +404,15 @@ pub(crate) async fn delete_table( } else { message }; - refuse(state, headers, inputs, message).await + refuse(state, headers, inputs, Page::Delete, message).await + } + Err(error) => { + refuse(state, headers, inputs, Page::Delete, error.message().to_string()).await } - Err(error) => refuse(state, headers, inputs, error.message().to_string()).await, } } -/// POST /admin/table-definition/copy — CopyProfile. +/// POST /admin/profiles/copy — CopyProfile. pub(crate) async fn copy_profile( State(state): State, headers: HeaderMap, @@ -319,6 +433,7 @@ pub(crate) async fn copy_profile( state, headers, inputs, + Page::Copy, "Name the profile to copy into.".to_string(), ) .await; @@ -345,7 +460,7 @@ pub(crate) async fn copy_profile( profile: form.profile, ..Default::default() }; - respond(state, headers, inputs, StatusCode::OK).await + respond(state, headers, inputs, Page::Copy, StatusCode::OK).await } Ok(response) => { let message = response.into_inner().message; @@ -354,13 +469,13 @@ pub(crate) async fn copy_profile( } else { message }; - refuse(state, headers, inputs, message).await + refuse(state, headers, inputs, Page::Copy, message).await } - Err(error) => refuse(state, headers, inputs, error.message().to_string()).await, + Err(error) => refuse(state, headers, inputs, Page::Copy, error.message().to_string()).await, } } -/// POST /admin/table-definition/invoice-template — CreateInvoiceTemplateTable. +/// POST /admin/tables/from-template — CreateInvoiceTemplateTable. pub(crate) async fn create_from_invoice_template( State(state): State, headers: HeaderMap, @@ -381,6 +496,7 @@ pub(crate) async fn create_from_invoice_template( state, headers, inputs, + Page::Template, "A table name and the template's source are both required.".to_string(), ) .await; @@ -417,43 +533,51 @@ pub(crate) async fn create_from_invoice_template( profile: form.profile, ..Default::default() }; - respond(state, headers, inputs, StatusCode::OK).await + respond(state, headers, inputs, Page::Template, StatusCode::OK).await } Ok(_) => { refuse( state, headers, inputs, + Page::Template, "The backend did not create the template's tables.".to_string(), ) .await } - Err(error) => refuse(state, headers, inputs, error.message().to_string()).await, + Err(error) => { + refuse(state, headers, inputs, Page::Template, error.message().to_string()).await + } } } -/// Re-reads the workspace and answers with it. +/// Re-reads the page's body and answers with it. async fn respond( state: AppState, headers: HeaderMap, inputs: PageInputs, + page: Page, status: StatusCode, ) -> Response { match load_page(state, &headers, inputs).await { - Ok(page) => (status, Html(ui::render_workspace(&page))).into_response(), + Ok(mut loaded) => { + loaded.active = page.name(); + (status, Html(page.render_fragment(&loaded))).into_response() + } Err(error) => load_error_response(error), } } -/// Answers a refused write: the workspace as it still is, plus the reason. +/// Answers a refused write: the page as it still is, plus the reason. async fn refuse( state: AppState, headers: HeaderMap, mut inputs: PageInputs, + page: Page, message: String, ) -> Response { inputs.error = Some(message); - respond(state, headers, inputs, StatusCode::UNPROCESSABLE_ENTITY).await + respond(state, headers, inputs, page, StatusCode::UNPROCESSABLE_ENTITY).await } fn load_error_response(error: LoadError) -> Response { diff --git a/web/src/pages/admin/table_definition/mod.rs b/web/src/pages/admin/table_definition/mod.rs index 954f5571..e6f5e82c 100644 --- a/web/src/pages/admin/table_definition/mod.rs +++ b/web/src/pages/admin/table_definition/mod.rs @@ -1,9 +1,18 @@ -//! The table-definition workspace: pick a profile and a table, then do -//! anything the `TableDefinition` service offers to it. +//! What can be done to a table once it exists, as one page per decision. //! -//! Creating a table is the one operation that lives elsewhere — it is a form -//! long enough to want its own page, `pages/add_table` — and the workspace -//! links to it with the chosen profile already filled in. +//! Adding columns, dropping the table, copying its profile, generating tables +//! from a template and reading the rename history are five different jobs, and +//! they used to be five panels stacked on one `/admin/table-definition` +//! workspace — which meant that after creating a table you landed on a screen +//! where finding the delete form meant scrolling past five other forms. +//! +//! They are now five routes, and the browsing they all needed — which profile, +//! which table — is done in the admin panel, which was already a three-pane +//! browser for exactly that. Every page here is entered with its selection in +//! the query string, and links back to the panel it came from. +//! +//! Creating a table is still elsewhere: it is a form long enough to want its +//! own page, `pages/add_table`. mod loader; mod logic; @@ -12,6 +21,7 @@ mod ui; use axum::{ Router, + response::Redirect, routing::{get, post}, }; @@ -19,18 +29,28 @@ use crate::AppState; pub(crate) fn router() -> Router { Router::new() - .route("/admin/table-definition", get(logic::page)) - .route("/admin/table-definition/workspace", get(logic::workspace)) + // Table-scoped. + .route("/admin/tables/columns", get(logic::columns_page)) + .route("/admin/tables/columns", post(logic::add_columns)) .route( - "/admin/table-definition/columns/builder", + "/admin/tables/columns/builder", post(logic::update_columns), ) - .route("/admin/table-definition/columns", post(logic::add_columns)) - .route("/admin/table-definition/rename", post(logic::rename_column)) - .route("/admin/table-definition/delete", post(logic::delete_table)) - .route("/admin/table-definition/copy", post(logic::copy_profile)) + .route("/admin/tables/rename", post(logic::rename_column)) + .route("/admin/tables/delete", get(logic::delete_page)) + .route("/admin/tables/delete", post(logic::delete_table)) + // Profile-scoped. + .route("/admin/profiles/copy", get(logic::copy_page)) + .route("/admin/profiles/copy", post(logic::copy_profile)) + .route("/admin/profiles/history", get(logic::history_page)) .route( - "/admin/table-definition/invoice-template", - post(logic::create_from_invoice_template), + "/admin/tables/from-template", + get(logic::template_page).post(logic::create_from_invoice_template), + ) + // The workspace these came out of. Anything still pointing at it — + // a bookmark, an old link — lands on the browser instead. + .route( + "/admin/table-definition", + get(|| async { Redirect::permanent("/admin") }), ) } diff --git a/web/src/pages/admin/table_definition/state.rs b/web/src/pages/admin/table_definition/state.rs index a61ee357..98d85935 100644 --- a/web/src/pages/admin/table_definition/state.rs +++ b/web/src/pages/admin/table_definition/state.rs @@ -1,8 +1,12 @@ -//! What the workspace renders, and the wire formats its panels post. +//! What the table-definition pages render, and the wire formats they post. //! -//! Every panel is a form of its own, and each one carries the selection it -//! acts on in hidden fields, because the workspace is swapped whole on every -//! write: the response is rebuilt from the live profile tree rather than from +//! One state type serves all five pages, because each of them needs the same +//! context: which profile and table is being worked on, and what that table +//! currently is. What differs is which panel the page renders, and `active` +//! is what says so. +//! +//! Every panel carries the selection it acts on in hidden fields, because a +//! write answers with its page re-read from the backend rather than with //! whatever the browser still had on screen. use crate::schema::{ColumnCatalog, ColumnDraft}; @@ -38,6 +42,21 @@ impl Selection { pub(crate) fn query(&self) -> String { format!("?profile={}&table={}", self.profile, self.table) } + + /// The same, for the profile-wide pages, which have no table to name. + pub(crate) fn profile_query(&self) -> String { + format!("?profile={}", self.profile) + } + + /// What the heading calls the scope, so the global one is not shown by its + /// sentinel name. + pub(crate) fn scope_label(&self) -> String { + if self.is_global() { + "Global — all profiles".to_string() + } else { + self.profile.clone() + } + } } /// One table in the selected profile, from the profile tree. @@ -47,7 +66,6 @@ pub(crate) struct TableSummary { pub table_kind: String, pub global: bool, pub depends_on: Vec, - pub row_display_columns: Vec, } impl TableSummary { @@ -62,18 +80,12 @@ impl TableSummary { #[derive(Clone, Debug)] pub(crate) struct TableDetailView { pub id: i64, - pub name: String, - pub table_kind: String, pub row_display_columns: Vec, pub columns: Vec, pub scripts: Vec, } impl TableDetailView { - pub(crate) fn is_system(&self) -> bool { - self.table_kind == "system" - } - /// Columns a rename may target. Provenance and renameability are separate: /// accounting companions remain renameable while protected generated /// columns do not. @@ -248,7 +260,6 @@ impl PageInputs { /// What the templates read. pub(crate) struct TableDefinitionPageState { pub nav: crate::ui::Nav, - pub profiles: Vec, pub selection: Selection, pub tables: Vec, pub detail: Option, @@ -261,22 +272,17 @@ pub(crate) struct TableDefinitionPageState { pub error: Option, pub sql: Option, pub generated: Vec, - pub permission_object: String, - pub role_permissions: Vec, -} - -pub(crate) struct TableRolePermissions { - pub role: String, - pub actions: Vec, -} - -pub(crate) struct TablePermissionAction { - pub action: String, - pub direct: bool, - pub effective: bool, + /// Which of the pages this is, for the action switcher they share. Set by + /// the handler, because it is the one thing the loader cannot know. + pub active: &'static str, } impl TableDefinitionPageState { + /// Whether `name` is the page being rendered. Read by context.html. + pub(crate) fn is(&self, name: &str) -> bool { + self.active == name + } + fn eligible_link_target(&self, table: &TableSummary) -> bool { table.name != self.selection.table && table.name != "accounts" } @@ -363,8 +369,6 @@ mod tests { fn provenance_and_renameability_are_independent() { let detail = TableDetailView { id: 1, - name: "contact".to_string(), - table_kind: "dynamic".to_string(), row_display_columns: Vec::new(), scripts: Vec::new(), columns: vec![ diff --git a/web/src/pages/admin/table_definition/ui.rs b/web/src/pages/admin/table_definition/ui.rs index ba7d6bfe..3e7f6570 100644 --- a/web/src/pages/admin/table_definition/ui.rs +++ b/web/src/pages/admin/table_definition/ui.rs @@ -1,3 +1,10 @@ +//! One template struct per page, and one per swap target inside it. +//! +//! The pages share a state type and an action switcher, so what differs +//! between them here is only which panel they render. The column vocabulary +//! is threaded through the two that need it, because askama resolves those +//! fields on the struct rather than on `page`. + use askama::Template; use crate::{ @@ -7,32 +14,31 @@ use crate::{ use super::state::TableDefinitionPageState; -/// GET /admin/table-definition — the shell around the workspace. +/// GET /admin/tables/columns #[derive(Template)] -#[template(path = "pages/admin/table_definition/table_definition.html")] -struct TableDefinitionPage<'a> { +#[template(path = "pages/admin/table_definition/columns.html")] +struct ColumnsPage<'a> { nav: Nav, page: &'a TableDefinitionPageState, column_types: Vec, temporal_types: Vec, gtin_types: Vec, currency_codes: &'static [&'static str], - /// False, as on the workspace fragment: the page embeds both, and the - /// outcome is reported once, at the top. + /// False, as on the fragment: the page embeds both, and the outcome is + /// reported once, at the top. standalone_column_panel: bool, } -/// The `#table-definition-workspace` swap, which is the same markup the page -/// embeds, so one template serves both. +/// The `#table-panel` swap on the columns page. #[derive(Template)] -#[template(path = "pages/admin/table_definition/workspace.html")] -struct WorkspaceFragment<'a> { +#[template(path = "pages/admin/table_definition/columns_panel.html")] +struct ColumnsFragment<'a> { page: &'a TableDefinitionPageState, column_types: Vec, temporal_types: Vec, gtin_types: Vec, - /// False: the workspace shows the outcome of the last action itself, at - /// the top, so the panel it embeds must not repeat it. + /// False: the fragment shows the outcome of the last action itself, at the + /// top, so the panel it embeds must not repeat it. standalone_column_panel: bool, } @@ -49,8 +55,58 @@ struct ColumnPanelFragment<'a> { standalone_column_panel: bool, } -pub(crate) fn render_page(page: &TableDefinitionPageState) -> String { - render(&TableDefinitionPage { +/// GET /admin/tables/delete +#[derive(Template)] +#[template(path = "pages/admin/table_definition/delete.html")] +struct DeletePage<'a> { + nav: Nav, + page: &'a TableDefinitionPageState, +} + +#[derive(Template)] +#[template(path = "pages/admin/table_definition/delete_panel.html")] +struct DeleteFragment<'a> { + page: &'a TableDefinitionPageState, +} + +/// GET /admin/profiles/copy +#[derive(Template)] +#[template(path = "pages/admin/table_definition/copy.html")] +struct CopyPage<'a> { + nav: Nav, + page: &'a TableDefinitionPageState, +} + +#[derive(Template)] +#[template(path = "pages/admin/table_definition/copy_panel.html")] +struct CopyFragment<'a> { + page: &'a TableDefinitionPageState, +} + +/// GET /admin/tables/from-template +#[derive(Template)] +#[template(path = "pages/admin/table_definition/from_template.html")] +struct TemplatePage<'a> { + nav: Nav, + page: &'a TableDefinitionPageState, +} + +#[derive(Template)] +#[template(path = "pages/admin/table_definition/from_template_panel.html")] +struct TemplateFragment<'a> { + page: &'a TableDefinitionPageState, +} + +/// GET /admin/profiles/history +#[derive(Template)] +#[template(path = "pages/admin/table_definition/history.html")] +struct HistoryPage<'a> { + nav: Nav, + page: &'a TableDefinitionPageState, +} + +pub(crate) fn render_columns_page(page: &TableDefinitionPageState) -> String { + render(&ColumnsPage { nav: page.nav.clone(), page, // Asking the draft, so the picker can only ever offer what the draft @@ -63,11 +119,9 @@ pub(crate) fn render_page(page: &TableDefinitionPageState) -> String { }) } -pub(crate) fn render_workspace(page: &TableDefinitionPageState) -> String { - render(&WorkspaceFragment { +pub(crate) fn render_columns_fragment(page: &TableDefinitionPageState) -> String { + render(&ColumnsFragment { page, - // Asking the draft, so the picker can only ever offer what the draft - // would accept — the accounting rule is stated once. column_types: page.columns.offered_types(), temporal_types: page.columns.temporal_types(), gtin_types: page.columns.gtin_types(), @@ -78,8 +132,6 @@ pub(crate) fn render_workspace(page: &TableDefinitionPageState) -> String { pub(crate) fn render_column_panel(page: &TableDefinitionPageState) -> String { render(&ColumnPanelFragment { page, - // Asking the draft, so the picker can only ever offer what the draft - // would accept — the accounting rule is stated once. column_types: page.columns.offered_types(), temporal_types: page.columns.temporal_types(), gtin_types: page.columns.gtin_types(), @@ -87,8 +139,48 @@ pub(crate) fn render_column_panel(page: &TableDefinitionPageState) -> String { }) } -/// Used when the workspace itself cannot be loaded. There is nothing left to -/// render, so the dialog is what tells the user why. +pub(crate) fn render_delete_page(page: &TableDefinitionPageState) -> String { + render(&DeletePage { + nav: page.nav.clone(), + page, + }) +} + +pub(crate) fn render_delete_fragment(page: &TableDefinitionPageState) -> String { + render(&DeleteFragment { page }) +} + +pub(crate) fn render_copy_page(page: &TableDefinitionPageState) -> String { + render(&CopyPage { + nav: page.nav.clone(), + page, + }) +} + +pub(crate) fn render_copy_fragment(page: &TableDefinitionPageState) -> String { + render(&CopyFragment { page }) +} + +pub(crate) fn render_template_page(page: &TableDefinitionPageState) -> String { + render(&TemplatePage { + nav: page.nav.clone(), + page, + }) +} + +pub(crate) fn render_template_fragment(page: &TableDefinitionPageState) -> String { + render(&TemplateFragment { page }) +} + +pub(crate) fn render_history_page(page: &TableDefinitionPageState) -> String { + render(&HistoryPage { + nav: page.nav.clone(), + page, + }) +} + +/// Used when a page itself cannot be loaded. There is nothing left to render, +/// so the dialog is what tells the user why. pub(crate) fn render_load_error(message: &str) -> String { render(&Alert::error("Table definition unavailable", message)) } @@ -99,7 +191,7 @@ mod tests { use crate::{ pages::admin::table_definition::state::{ CopyForm, DetailColumn, InvoiceTemplateForm, RenameForm, Selection, TableDetailView, - TablePermissionAction, TableRolePermissions, TableSummary, + TableSummary, }, schema::ColumnDraft, }; @@ -110,28 +202,12 @@ mod tests { table_kind: kind.to_string(), global: false, depends_on: Vec::new(), - row_display_columns: vec!["number".to_string()], } } - #[test] - fn global_tables_have_their_own_scope() { - let mut state = page(); - state.selection.profile = "__global".to_string(); - - let html = render_workspace(&state); - - assert!(html.contains(r#"value="__global" selected"#)); - assert!(html.contains("Global — all profiles")); - assert!(html.contains("/admin/tables/new?global=true")); - assert!(!html.contains("Copy __global")); - assert!(!html.contains("Create tables from an invoice template")); - } - fn page() -> TableDefinitionPageState { TableDefinitionPageState { nav: Nav::default(), - profiles: vec!["billing".to_string(), "payroll".to_string()], selection: Selection { profile: "billing".to_string(), table: "invoice".to_string(), @@ -139,8 +215,6 @@ mod tests { tables: vec![table("invoice", "dynamic"), table("accounts", "system")], detail: Some(TableDetailView { id: 7, - name: "invoice".to_string(), - table_kind: "dynamic".to_string(), row_display_columns: vec!["number".to_string()], scripts: Vec::new(), columns: vec![DetailColumn { @@ -165,29 +239,96 @@ mod tests { error: None, sql: None, generated: Vec::new(), - permission_object: String::new(), - role_permissions: Vec::new(), + active: "columns", } } - /// The point of the page: every write the service offers is reachable - /// from the one screen, for the one selection. + /// The point of the split: each write is its own page, and the switcher + /// they share is how you get from one to the next. #[test] - fn the_workspace_offers_every_table_definition_write() { - let html = render_workspace(&page()); + fn every_page_links_to_the_others() { + let mut state = page(); + + for (active, html) in [ + ("columns", render_columns_page(&state)), + ("delete", render_delete_page(&state)), + ] { + state.active = active; + assert!(!html.contains("Template error"), "{html}"); + } + + let html = render_columns_page(&state); + for route in [ + "/admin/tables/columns?profile=billing", + "/admin/tables/delete?profile=billing", + "/admin/profiles/copy?profile=billing", + "/admin/tables/from-template?profile=billing", + "/admin/profiles/history?profile=billing", + "/permissions/grants", + ] { + // Escaped, because it is an attribute: `&` is what a browser + // reads back as the `&` separating the two parameters. + let route = route.replace('&', "&"); + assert!(html.contains(&route), "missing the {route} link\n{html}"); + } + // And back to where the table was picked. + assert!(html.contains(r#"href="/admin?profile=billing&table=invoice""#)); + } + + /// Deleting is its own page, so the form is the only thing on it — no + /// scrolling past five other panels to reach it. + #[test] + fn the_delete_page_is_only_the_delete_form() { + let html = render_delete_page(&page()); assert!(!html.contains("Template error"), "{html}"); - for route in [ - "/admin/table-definition/columns", - "/admin/table-definition/rename", - "/admin/table-definition/delete", - "/admin/table-definition/copy", - "/admin/table-definition/invoice-template", - ] { - assert!(html.contains(route), "missing the {route} panel"); - } - // And creating a table, which is the one write that has its own page. - assert!(html.contains("/admin/tables/new?profile=billing")); + assert!(html.contains("/admin/tables/delete")); + assert!(html.contains("Type invoice to confirm")); + // The other writes are links in the switcher, not forms on the page. + assert!(!html.contains("/admin/tables/rename")); + assert!(!html.contains("/admin/profiles/copy?profile=billing\" method")); + } + + /// A system table is the backend's own; every write is refused for it, so + /// none of the write pages offer their form. + #[test] + fn a_system_table_gets_no_write_form() { + let mut state = page(); + state.selection.table = "accounts".to_string(); + + let html = render_delete_page(&state); + assert!(!html.contains("Template error"), "{html}"); + assert!(html.contains("backend's own")); + assert!(!html.contains(r#"name="confirm_table_name""#)); + + let html = render_columns_page(&state); + assert!(!html.contains("/admin/tables/rename")); + } + + /// The profile-wide pages need only a profile, and say so by still + /// rendering with no table selected. + #[test] + fn the_profile_pages_do_not_need_a_table() { + let mut state = page(); + state.selection.table = String::new(); + state.detail = None; + state.active = "copy"; + + let html = render_copy_page(&state); + assert!(!html.contains("Template error"), "{html}"); + assert!(html.contains("/admin/profiles/copy")); + // Table-scoped links are absent, because there is no table. + assert!(!html.contains("/admin/tables/delete")); + + state.active = "template"; + let html = render_template_page(&state); + assert!(!html.contains("Template error"), "{html}"); + assert!(html.contains("/admin/tables/from-template")); + + state.active = "history"; + let html = render_history_page(&state); + assert!(!html.contains("Template error"), "{html}"); + assert!(html.contains("has been renamed")); } /// The append panel posts the selection in its URL, so the column fields @@ -209,8 +350,7 @@ mod tests { let html = render_column_panel(&state); assert!(!html.contains("Template error"), "{html}"); - // Escaped, because it is an attribute: `&` is what a browser reads - // back as the `&` separating the two parameters. + assert!(html.contains("/admin/tables/columns/builder")); // Escaped, because it is an attribute: `&` is what a browser reads // back as the `&` separating the two parameters. assert!(html.contains("?profile=billing&table=invoice")); @@ -254,58 +394,16 @@ mod tests { assert!(!html.contains(r#"