From 9c6e341f14dabb4ca2965e5a74f195e9bd5f48ef Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:32:37 +0200 Subject: [PATCH 01/16] Add scoped Clients page readers Bind the Clients list and detail readers to the requesting session and the current permission policy. Policy one requires the client catalog grant and, for the default rate, the global billable-rate grant. Legacy workflow readers are unchanged and listed in the client permission contract. --- crates/horae/src/models/client.rs | 23 + crates/horae/src/server_fns/clients.rs | 211 ++++++++- .../src/server_fns/clients/page_tests.rs | 402 ++++++++++++++++++ .../importers/authorization_tests.rs | 2 + .../authorization_tests/client_page.rs | 86 ++++ .../contracts/client-permissions.md | 36 ++ 6 files changed, 754 insertions(+), 6 deletions(-) create mode 100644 crates/horae/src/server_fns/clients/page_tests.rs create mode 100644 crates/horae/src/server_fns/importers/authorization_tests/client_page.rs diff --git a/crates/horae/src/models/client.rs b/crates/horae/src/models/client.rs index 204dd912..1e9e18ad 100644 --- a/crates/horae/src/models/client.rs +++ b/crates/horae/src/models/client.rs @@ -2,6 +2,8 @@ use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use uuid::Uuid; +use super::permission_editor::PermissionRequester; + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[cfg_attr(feature = "server", derive(sqlx::FromRow))] pub struct Client { @@ -31,6 +33,27 @@ pub struct ClientDetails { pub billing: Option, } +/// Clients page rows bound to one authenticated requester and policy mode. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ClientOverview { + pub requester: PermissionRequester, + pub canonical_permissions: bool, + /// False means the catalog is withheld, distinct from an empty catalog. + pub can_read: bool, + /// Display affordance for the legacy client writes; every write rechecks. + pub can_manage: bool, + pub clients: Vec, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ClientDetailView { + pub requester: PermissionRequester, + pub canonical_permissions: bool, + /// Display affordance for the legacy client writes; every write rechecks. + pub can_manage: bool, + pub details: ClientDetails, +} + /// An absent section means restricted access; an absent rate means not set. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct ClientBilling { diff --git a/crates/horae/src/server_fns/clients.rs b/crates/horae/src/server_fns/clients.rs index d9a08006..327feade 100644 --- a/crates/horae/src/server_fns/clients.rs +++ b/crates/horae/src/server_fns/clients.rs @@ -5,7 +5,11 @@ use super::*; use crate::models::client::ClientBilling; #[cfg(feature = "server")] use crate::models::client::ClientRateChange; -use crate::models::client::{ClientDetails, ClientProfile, ClientProfileEdit, ClientSummary}; +use crate::models::client::{ + ClientDetailView, ClientDetails, ClientOverview, ClientProfile, ClientProfileEdit, + ClientSummary, +}; +use crate::models::permission_editor::PermissionRequester; #[server] pub async fn create_client_profile( @@ -225,19 +229,72 @@ async fn client_details_for_viewer<'e>( org_id: uuid::Uuid, viewer_id: uuid::Uuid, client_id: uuid::Uuid, +) -> Result { + client_details_in_scope(db, org_id, viewer_id, client_id, ClientReadScope::LEGACY).await +} + +/// Current grants for the canonical Clients readers; `legacy` keeps policy zero. +#[cfg(feature = "server")] +#[derive(Clone, Copy)] +struct ClientReadScope { + legacy: bool, + catalog: bool, + rates: bool, + projects_all: bool, + projects_managed: bool, +} + +#[cfg(feature = "server")] +impl ClientReadScope { + const LEGACY: Self = Self { + legacy: true, + catalog: false, + rates: false, + projects_all: false, + projects_managed: false, + }; + + fn current(access: &ProjectReadAccess<'_>) -> Self { + use horae_core::permissions::catalog::Permission; + Self { + legacy: access.legacy(), + catalog: access.has(Permission::ClientReadAll), + // FR-035: the client default is a global rate; managed grants never apply. + rates: access.has(Permission::BillableRateReadAll), + projects_all: access.has(Permission::ProjectReadAll), + projects_managed: access.has(Permission::ProjectReadManaged), + } + } + + fn can_read(self) -> bool { + self.legacy || self.catalog + } +} + +#[cfg(feature = "server")] +async fn client_details_in_scope<'e>( + db: impl sqlx::Executor<'e, Database = sqlx::Postgres>, + org_id: uuid::Uuid, + viewer_id: uuid::Uuid, + client_id: uuid::Uuid, + scope: ClientReadScope, ) -> Result { let row = sqlx::query!( r#"SELECT c.id, c.org_id, c.name, c.currency, c.address, c.tax_id, c.active, c.created_at as "created_at: chrono::DateTime", - u.org_role IN ('admin', 'manager') as "can_view_billing!", - CASE WHEN u.org_role IN ('admin', 'manager') + (CASE WHEN $4 THEN u.org_role IN ('admin', 'manager') ELSE $5 AND $6 END) + as "can_view_billing!", + CASE WHEN (CASE WHEN $4 THEN u.org_role IN ('admin', 'manager') ELSE $5 AND $6 END) THEN c.default_rate_cents END as default_rate_cents FROM clients c JOIN users u ON u.org_id = c.org_id AND u.id = $2 AND u.active - WHERE c.org_id = $1 AND c.id = $3"#, + WHERE c.org_id = $1 AND c.id = $3 AND ($4 OR $5)"#, org_id, viewer_id, client_id, + scope.legacy, + scope.catalog, + scope.rates, ) .fetch_optional(db) .await @@ -300,6 +357,20 @@ async fn client_summaries_for_viewer( org_id: uuid::Uuid, user_id: uuid::Uuid, ) -> Result, ServerFnError> { + client_summaries_in_scope(db, org_id, user_id, ClientReadScope::LEGACY).await +} + +/// Policy one counts the projects `list_projects` would show the same viewer. +#[cfg(feature = "server")] +async fn client_summaries_in_scope<'e>( + db: impl sqlx::Executor<'e, Database = sqlx::Postgres>, + org_id: uuid::Uuid, + user_id: uuid::Uuid, + scope: ClientReadScope, +) -> Result, ServerFnError> { + if !scope.can_read() { + return Ok(Vec::new()); + } let rows = sqlx::query!( r#"SELECT c.id, c.org_id, c.name, c.currency, c.address, c.tax_id, c.active, c.created_at as "created_at: chrono::DateTime", @@ -310,16 +381,28 @@ async fn client_summaries_for_viewer( FROM clients c JOIN users u ON u.org_id = c.org_id AND u.id = $2 AND u.active LEFT JOIN projects p ON p.client_id = c.id AND p.org_id = c.org_id - AND EXISTS ( + AND CASE WHEN $3 THEN EXISTS ( SELECT 1 FROM project_read_access a WHERE a.project_id = p.id AND a.org_id = c.org_id AND a.user_id = u.id AND a.can_view_progress - ) + ) ELSE $4 OR ($5 AND EXISTS ( + SELECT 1 FROM project_management_assignments m + WHERE m.org_id = p.org_id AND m.project_id = p.id AND m.manager_id = u.id + )) OR (EXISTS ( + SELECT 1 FROM assignments member + WHERE member.project_id = p.id AND member.user_id = u.id + ) AND COALESCE(( + SELECT s.report_visibility FROM project_settings s + WHERE s.project_id = p.id AND s.org_id = p.org_id + ), 'project_members') = 'project_members') END WHERE c.org_id = $1 GROUP BY c.id ORDER BY c.name, c.id"#, org_id, user_id, + scope.legacy, + scope.projects_all, + scope.projects_managed, ) .fetch_all(db) .await @@ -345,6 +428,120 @@ async fn client_summaries_for_viewer( .collect()) } +/// The canonical Clients page. Workflow pickers keep their legacy readers. +#[server] +pub async fn client_overview( + expected_requester: Option, +) -> Result { + let viewer = require_user().await?; + let state = crate::state::global_state().await; + client_overview_for(&state.db, viewer.org_id, viewer.id, expected_requester).await +} + +#[server] +pub async fn client_detail_view( + client_id: String, + expected_requester: Option, +) -> Result { + let viewer = require_user().await?; + let client_id = client_id + .parse() + .map_err(|_| err(BAD_REQUEST, "Invalid client ID"))?; + let state = crate::state::global_state().await; + client_detail_view_for( + &state.db, + viewer.org_id, + viewer.id, + client_id, + expected_requester, + ) + .await +} + +#[cfg(feature = "server")] +fn client_requester( + org_id: uuid::Uuid, + user_id: uuid::Uuid, + expected_requester: Option, +) -> Result { + let requester = PermissionRequester { org_id, user_id }; + if expected_requester.is_some_and(|expected| expected != requester) { + return Err(forbidden("Client requester has changed")); + } + Ok(requester) +} + +/// The client writes still check only the legacy role. Until they use current +/// grants, policy one offers no client mutations rather than role-derived ones. +#[cfg(feature = "server")] +async fn offers_legacy_client_writes( + access: &mut ProjectReadAccess<'_>, + org_id: uuid::Uuid, + user_id: uuid::Uuid, +) -> Result { + if !access.legacy() { + return Ok(false); + } + let role = sqlx::query_scalar!( + r#"SELECT org_role as "org_role: OrgRole" FROM users WHERE org_id=$1 AND id=$2"#, + org_id, + user_id, + ) + .fetch_one(&mut *access.tx) + .await + .map_err(server_err)?; + Ok(role.is_manager_or_above()) +} + +#[cfg(feature = "server")] +async fn client_overview_for( + db: &sqlx::PgPool, + org_id: uuid::Uuid, + viewer_id: uuid::Uuid, + expected_requester: Option, +) -> Result { + let requester = client_requester(org_id, viewer_id, expected_requester)?; + let mut access = ProjectReadAccess::begin(db, org_id, viewer_id) + .await? + .ok_or_else(|| forbidden("Current client access is required"))?; + let scope = ClientReadScope::current(&access); + let can_manage = offers_legacy_client_writes(&mut access, org_id, viewer_id).await?; + let clients = client_summaries_in_scope(&mut *access.tx, org_id, viewer_id, scope).await?; + access.tx.commit().await.map_err(server_err)?; + Ok(ClientOverview { + requester, + canonical_permissions: !scope.legacy, + can_read: scope.can_read(), + can_manage, + clients, + }) +} + +#[cfg(feature = "server")] +async fn client_detail_view_for( + db: &sqlx::PgPool, + org_id: uuid::Uuid, + viewer_id: uuid::Uuid, + client_id: uuid::Uuid, + expected_requester: Option, +) -> Result { + let requester = client_requester(org_id, viewer_id, expected_requester)?; + let mut access = ProjectReadAccess::begin(db, org_id, viewer_id) + .await? + .ok_or_else(|| not_found("Client not found"))?; + let scope = ClientReadScope::current(&access); + let details = + client_details_in_scope(&mut *access.tx, org_id, viewer_id, client_id, scope).await?; + let can_manage = offers_legacy_client_writes(&mut access, org_id, viewer_id).await?; + access.tx.commit().await.map_err(server_err)?; + Ok(ClientDetailView { + requester, + canonical_permissions: !scope.legacy, + can_manage, + details, + }) +} + /// Explicit client creation from the project form, including its default rate. #[server] pub async fn create_project_client( @@ -605,5 +802,7 @@ async fn lock_client( .ok_or_else(|| not_found("Client not found")) } +#[cfg(all(test, feature = "server"))] +mod page_tests; #[cfg(all(test, feature = "server"))] mod tests; diff --git a/crates/horae/src/server_fns/clients/page_tests.rs b/crates/horae/src/server_fns/clients/page_tests.rs new file mode 100644 index 00000000..b84805c1 --- /dev/null +++ b/crates/horae/src/server_fns/clients/page_tests.rs @@ -0,0 +1,402 @@ +//! Canonical Clients page readers under both permission policies. + +use super::*; +use crate::server_fns::test_seed::{SeedIds, seed, wait_for_blocked}; +use horae_core::permissions::catalog::{Permission, PermissionSelection}; +use sqlx::PgPool; +use uuid::Uuid; + +const ADDRESS: &str = "Hidden Street 1"; +const TAX_ID: &str = "HIDDEN-VAT"; + +async fn set_grants(pool: &PgPool, user_id: Uuid, permissions: &[Permission]) { + let grants: Vec = + serde_json::from_value(serde_json::json!(PermissionSelection::new(permissions))).unwrap(); + sqlx::query!( + "UPDATE person_permission_states SET grants=$2 WHERE user_id=$1", + user_id, + &grants + ) + .execute(pool) + .await + .unwrap(); +} + +/// A disposable organization on policy one, with a client carrying every +/// protected field so payload checks can look for leaks. +async fn policy_one(pool: &PgPool, role: OrgRole, permissions: &[Permission]) -> SeedIds { + let ids = seed(pool, role).await; + sqlx::query!( + "UPDATE clients SET address=$2, tax_id=$3, default_rate_cents=12345 WHERE id=$1", + ids.client_id, + ADDRESS, + TAX_ID, + ) + .execute(pool) + .await + .unwrap(); + sqlx::query!( + "INSERT INTO person_permission_states (id,org_id,user_id,catalog_version,grants,is_administrator,source) + VALUES ($1,$2,$3,1,'{}',false,'individual')", + Uuid::now_v7(), ids.org_id, ids.user_id, + ) + .execute(pool) + .await + .unwrap(); + set_grants(pool, ids.user_id, permissions).await; + sqlx::query!( + "UPDATE organizations SET permission_policy_version=1 WHERE id=$1", + ids.org_id + ) + .execute(pool) + .await + .unwrap(); + ids +} + +async fn overview(pool: &PgPool, ids: &SeedIds) -> Result { + client_overview_for(pool, ids.org_id, ids.user_id, None).await +} + +async fn detail(pool: &PgPool, ids: &SeedIds) -> Result { + client_detail_view_for(pool, ids.org_id, ids.user_id, ids.client_id, None).await +} + +fn assert_code(error: ServerFnError, expected: u16) { + assert!( + matches!(error, ServerFnError::ServerError { code, .. } if code == expected), + "expected {expected}, got {error:?}" + ); +} + +#[sqlx::test(migrations = "./migrations")] +async fn read_only_client_grant_shows_catalog_without_writes_rates_or_projects(pool: PgPool) { + let ids = policy_one(&pool, OrgRole::Member, &[Permission::ClientReadAll]).await; + let page = overview(&pool, &ids).await.unwrap(); + assert!(page.canonical_permissions && page.can_read && !page.can_manage); + assert_eq!(page.clients.len(), 1); + let summary = &page.clients[0]; + assert_eq!(summary.client.id, ids.client_id); + assert_eq!(summary.client.address.as_deref(), Some(ADDRESS)); + assert_eq!((summary.active_projects, summary.total_projects), (0, 0)); + assert!(summary.project_currencies.is_empty()); + + let view = detail(&pool, &ids).await.unwrap(); + assert!(view.canonical_permissions && !view.can_manage); + assert_eq!(view.details.client.tax_id.as_deref(), Some(TAX_ID)); + assert!(view.details.billing.is_none()); + assert!(!serde_json::to_string(&view).unwrap().contains("12345")); +} + +#[sqlx::test(migrations = "./migrations")] +async fn unrelated_grants_and_legacy_roles_do_not_open_the_catalog(pool: PgPool) { + for (role, permissions) in [ + (OrgRole::Admin, vec![]), + (OrgRole::Manager, vec![Permission::ProjectReadAll]), + (OrgRole::Member, vec![Permission::InvoiceReadAll]), + (OrgRole::Member, vec![Permission::BillableRateReadAll]), + (OrgRole::Member, vec![Permission::ReportProfitabilityRead]), + ] { + let ids = policy_one(&pool, role, &permissions).await; + let page = overview(&pool, &ids).await.unwrap(); + assert!( + page.canonical_permissions && !page.can_read && !page.can_manage, + "{permissions:?}" + ); + let payload = serde_json::to_string(&page).unwrap(); + for hidden in ["Acme", ADDRESS, TAX_ID, "12345"] { + assert!(!payload.contains(hidden), "{hidden} in {payload}"); + } + assert_code(detail(&pool, &ids).await.unwrap_err(), NOT_FOUND); + } +} + +#[sqlx::test(migrations = "./migrations")] +async fn default_rate_requires_the_global_billable_rate_grant(pool: PgPool) { + for (rate_grant, visible) in [ + (Permission::BillableRateReadAll, true), + (Permission::BillableRateReadManaged, false), + ] { + let ids = policy_one( + &pool, + OrgRole::Manager, + &[Permission::ClientReadAll, rate_grant], + ) + .await; + let billing = detail(&pool, &ids).await.unwrap().details.billing; + assert_eq!( + billing.map(|billing| billing.default_rate_cents), + visible.then_some(Some(12345)), + "{rate_grant:?}" + ); + } +} + +#[sqlx::test(migrations = "./migrations")] +async fn project_counts_follow_the_policy_one_project_reader(pool: PgPool) { + let ids = policy_one( + &pool, + OrgRole::Member, + &[Permission::ClientReadAll, Permission::ProjectReadAll], + ) + .await; + let counts = |page: ClientOverview| { + let summary = &page.clients[0]; + (summary.total_projects, summary.project_currencies.clone()) + }; + assert_eq!( + counts(overview(&pool, &ids).await.unwrap()), + (1, vec!["EUR".to_owned()]) + ); + + set_grants(&pool, ids.user_id, &[Permission::ClientReadAll]).await; + assert_eq!(counts(overview(&pool, &ids).await.unwrap()), (0, vec![])); + sqlx::query!( + "INSERT INTO assignments (id, project_id, user_id) VALUES ($1,$2,$3)", + Uuid::now_v7(), + ids.project_id, + ids.user_id, + ) + .execute(&pool) + .await + .unwrap(); + assert_eq!(counts(overview(&pool, &ids).await.unwrap()).0, 1); + sqlx::query!( + "INSERT INTO project_settings (id, org_id, project_id, creator_id, rate_mode, report_visibility) + VALUES ($1,$2,$3,$4,'project','managers')", + Uuid::now_v7(), ids.org_id, ids.project_id, ids.user_id, + ).execute(&pool).await.unwrap(); + assert_eq!(counts(overview(&pool, &ids).await.unwrap()).0, 0); +} + +#[sqlx::test(migrations = "./migrations")] +async fn managed_project_counts_require_an_actual_management_link(pool: PgPool) { + let ids = policy_one( + &pool, + OrgRole::Member, + &[Permission::ClientReadAll, Permission::ProjectReadManaged], + ) + .await; + let total = |page: ClientOverview| page.clients[0].total_projects; + assert_eq!(total(overview(&pool, &ids).await.unwrap()), 0); + sqlx::query!( + "INSERT INTO project_management_assignments (id,org_id,manager_id,project_id) VALUES ($1,$2,$3,$4)", + Uuid::now_v7(), ids.org_id, ids.user_id, ids.project_id, + ).execute(&pool).await.unwrap(); + assert_eq!(total(overview(&pool, &ids).await.unwrap()), 1); +} + +#[sqlx::test(migrations = "./migrations")] +async fn visible_default_rate_distinguishes_unset_from_zero(pool: PgPool) { + let ids = policy_one( + &pool, + OrgRole::Member, + &[Permission::ClientReadAll, Permission::BillableRateReadAll], + ) + .await; + for (rate, serialized) in [ + (None, r#""billing":{"default_rate_cents":null}"#), + (Some(0), r#""billing":{"default_rate_cents":0}"#), + ] { + sqlx::query!( + "UPDATE clients SET default_rate_cents = $2 WHERE id = $1", + ids.client_id, + rate + ) + .execute(&pool) + .await + .unwrap(); + let payload = serde_json::to_string(&detail(&pool, &ids).await.unwrap()).unwrap(); + assert!(payload.contains(serialized), "{payload}"); + } +} + +#[sqlx::test(migrations = "./migrations")] +async fn foreign_missing_and_rebound_requests_fail_closed(pool: PgPool) { + let ids = policy_one(&pool, OrgRole::Member, &[Permission::ClientReadAll]).await; + let foreign = seed(&pool, OrgRole::Admin).await; + for client_id in [foreign.client_id, Uuid::now_v7()] { + let error = client_detail_view_for(&pool, ids.org_id, ids.user_id, client_id, None) + .await + .unwrap_err(); + assert_code(error, NOT_FOUND); + } + // A known organization does not admit an actor from another one. + let error = client_overview_for(&pool, ids.org_id, foreign.user_id, None) + .await + .unwrap_err(); + assert_code(error, FORBIDDEN); + let error = client_detail_view_for(&pool, ids.org_id, foreign.user_id, ids.client_id, None) + .await + .unwrap_err(); + assert_code(error, NOT_FOUND); + + let page = overview(&pool, &ids).await.unwrap(); + let bound = Some(page.requester); + assert_eq!( + client_overview_for(&pool, ids.org_id, ids.user_id, bound) + .await + .unwrap(), + page + ); + let error = client_overview_for(&pool, foreign.org_id, foreign.user_id, bound) + .await + .unwrap_err(); + assert_code(error, FORBIDDEN); + let error = client_detail_view_for( + &pool, + foreign.org_id, + foreign.user_id, + foreign.client_id, + bound, + ) + .await + .unwrap_err(); + assert_code(error, FORBIDDEN); +} + +#[sqlx::test(migrations = "./migrations")] +async fn unavailable_permission_state_and_inactive_actors_fail_closed(pool: PgPool) { + for state in ["unknown_policy", "missing", "malformed", "inactive"] { + let ids = policy_one(&pool, OrgRole::Admin, &[Permission::ClientReadAll]).await; + match state { + "unknown_policy" => sqlx::query!( + "UPDATE organizations SET permission_policy_version=99 WHERE id=$1", + ids.org_id + ) + .execute(&pool) + .await + .unwrap(), + "missing" => sqlx::query!( + "DELETE FROM person_permission_states WHERE user_id=$1", + ids.user_id + ) + .execute(&pool) + .await + .unwrap(), + "malformed" => sqlx::query!( + "UPDATE person_permission_states SET grants=ARRAY['unknown_permission'] WHERE user_id=$1", + ids.user_id + ) + .execute(&pool) + .await + .unwrap(), + "inactive" => sqlx::query!("UPDATE users SET active=false WHERE id=$1", ids.user_id) + .execute(&pool) + .await + .unwrap(), + _ => unreachable!(), + }; + assert_code(overview(&pool, &ids).await.unwrap_err(), FORBIDDEN); + let expected = if state == "inactive" { + NOT_FOUND + } else { + FORBIDDEN + }; + assert_code(detail(&pool, &ids).await.unwrap_err(), expected); + } +} + +#[sqlx::test(migrations = "./migrations")] +async fn readers_observe_revocation_after_waiting_for_current_authority(pool: PgPool) { + for read_detail in [false, true] { + let ids = policy_one(&pool, OrgRole::Admin, &[Permission::ClientReadAll]).await; + let mut hold = pool.begin().await.unwrap(); + sqlx::query!( + "UPDATE organizations SET access_revision=access_revision+1 WHERE id=$1", + ids.org_id + ) + .execute(&mut *hold) + .await + .unwrap(); + let blocker = sqlx::query_scalar!("SELECT pg_backend_pid() AS \"pid!\"") + .fetch_one(&mut *hold) + .await + .unwrap(); + let run_pool = pool.clone(); + let (org_id, user_id, client_id) = (ids.org_id, ids.user_id, ids.client_id); + let pending = tokio::spawn(async move { + if read_detail { + client_detail_view_for(&run_pool, org_id, user_id, client_id, None) + .await + .map(|_| ()) + } else { + let page = client_overview_for(&run_pool, org_id, user_id, None).await?; + assert!(!page.can_read && page.clients.is_empty()); + Ok(()) + } + }); + wait_for_blocked(&pool, blocker).await; + let grants: Vec = + serde_json::from_value(serde_json::json!(PermissionSelection::new(&[]))).unwrap(); + sqlx::query!( + "UPDATE person_permission_states SET grants=$2 WHERE user_id=$1", + ids.user_id, + &grants + ) + .execute(&mut *hold) + .await + .unwrap(); + hold.commit().await.unwrap(); + let result = pending.await.unwrap(); + if read_detail { + assert_code(result.unwrap_err(), NOT_FOUND); + } else { + result.unwrap(); + } + } +} + +#[sqlx::test(migrations = "./migrations")] +async fn policy_zero_keeps_legacy_catalog_billing_and_manager_affordance(pool: PgPool) { + for (role, manager) in [ + (OrgRole::Admin, true), + (OrgRole::Manager, true), + (OrgRole::Member, false), + ] { + let ids = seed(&pool, role).await; + sqlx::query!( + "UPDATE clients SET default_rate_cents=12345 WHERE id=$1", + ids.client_id + ) + .execute(&pool) + .await + .unwrap(); + let page = overview(&pool, &ids).await.unwrap(); + assert!(!page.canonical_permissions && page.can_read); + assert_eq!(page.can_manage, manager, "{role:?}"); + assert_eq!( + page.clients, + client_summaries_for_viewer(&pool, ids.org_id, ids.user_id) + .await + .unwrap() + ); + let view = detail(&pool, &ids).await.unwrap(); + assert_eq!(view.can_manage, manager); + assert_eq!( + view.details, + client_details_for_viewer(&pool, ids.org_id, ids.user_id, ids.client_id) + .await + .unwrap() + ); + assert_eq!(view.details.billing.is_some(), manager); + } +} + +/// Deferred: the shared legacy readers are not cut over by this page slice. +#[sqlx::test(migrations = "./migrations")] +async fn legacy_workflow_readers_are_unchanged_under_policy_one(pool: PgPool) { + let ids = policy_one(&pool, OrgRole::Manager, &[]).await; + let legacy = client_details_for_viewer(&pool, ids.org_id, ids.user_id, ids.client_id) + .await + .unwrap(); + assert_eq!(legacy.client.tax_id.as_deref(), Some(TAX_ID)); + assert!(legacy.billing.is_some()); + assert_eq!( + client_summaries_for_viewer(&pool, ids.org_id, ids.user_id) + .await + .unwrap() + .len(), + 1 + ); +} diff --git a/crates/horae/src/server_fns/importers/authorization_tests.rs b/crates/horae/src/server_fns/importers/authorization_tests.rs index cf89db93..6dda6050 100644 --- a/crates/horae/src/server_fns/importers/authorization_tests.rs +++ b/crates/horae/src/server_fns/importers/authorization_tests.rs @@ -25,6 +25,7 @@ use super::*; mod approval_labels; mod cli; +mod client_page; #[path = "../time_entries/commands/tests/http.rs"] mod delegated_time; mod exports; @@ -391,6 +392,7 @@ async fn job_endpoints_enforce_session_role_and_organization(pool: PgPool) { project_people::check(&pool, &api).await; project_reads::check(&pool, &api).await; task_reads::check(&pool, &api).await; + client_page::check(&pool, &api).await; task_writes::check(&pool, &api).await; task_edits::check(&pool, &api).await; task_activity::check(&pool, &api).await; diff --git a/crates/horae/src/server_fns/importers/authorization_tests/client_page.rs b/crates/horae/src/server_fns/importers/authorization_tests/client_page.rs new file mode 100644 index 00000000..84e78c13 --- /dev/null +++ b/crates/horae/src/server_fns/importers/authorization_tests/client_page.rs @@ -0,0 +1,86 @@ +//! Canonical Clients page context through registered functions and real session cookies. + +use super::*; +use horae_core::permissions::catalog::{Permission, PermissionSelection}; + +pub(super) async fn check(pool: &PgPool, api: &Api) { + let ids = crate::server_fns::test_seed::seed(pool, OrgRole::Member).await; + let restricted = user(pool, ids.org_id, OrgRole::Admin).await; + for (actor, permissions) in [ + (ids.user_id, vec![Permission::ClientReadAll]), + (restricted, vec![]), + ] { + let grants: Vec = + serde_json::from_value(json!(PermissionSelection::new(&permissions))).unwrap(); + sqlx::query!("INSERT INTO person_permission_states (id,org_id,user_id,catalog_version,grants,is_administrator,source) + VALUES ($1,$2,$3,1,$4,false,'individual')", + Uuid::now_v7(), ids.org_id, actor, &grants).execute(pool).await.unwrap(); + } + sqlx::query!( + "UPDATE organizations SET permission_policy_version=1 WHERE id=$1", + ids.org_id + ) + .execute(pool) + .await + .unwrap(); + let cookie = api.cookie(ids.user_id).await; + let restricted_cookie = api.cookie(restricted).await; + let unbound_detail = json!({"client_id": ids.client_id, "expected_requester": null}); + for (endpoint, body) in [ + ("client_overview", json!({"expected_requester": null})), + ("client_detail_view", unbound_detail.clone()), + ] { + assert_eq!( + api.call(endpoint, body, None, false).await.status(), + StatusCode::UNAUTHORIZED + ); + } + + let page = api + .json("client_overview", json!({"expected_requester": null}), &cookie) + .await; + let requester = json!({"org_id": ids.org_id, "user_id": ids.user_id}); + assert_eq!(page["requester"], requester); + assert_eq!(page["can_read"], true); + assert_eq!(page["can_manage"], false); + assert_eq!(page["clients"][0]["client"]["id"], json!(ids.client_id)); + let bound = json!({"expected_requester": requester}); + assert_eq!( + api.json("client_overview", bound.clone(), &cookie).await, + page + ); + let bound_detail = json!({"client_id": ids.client_id, "expected_requester": requester}); + let view = api + .json("client_detail_view", bound_detail.clone(), &cookie) + .await; + assert_eq!(view["details"]["client"]["id"], json!(ids.client_id)); + assert_eq!(view["can_manage"], false); + + for (endpoint, body) in [ + ("client_overview", bound), + ("client_detail_view", bound_detail), + ] { + assert_eq!( + api.call(endpoint, body, Some(&restricted_cookie), false) + .await + .status(), + StatusCode::FORBIDDEN, + "{endpoint} must not substitute another authenticated requester", + ); + } + let withheld = api + .json( + "client_overview", + json!({"expected_requester": null}), + &restricted_cookie, + ) + .await; + assert_eq!(withheld["can_read"], false); + assert_eq!(withheld["clients"], json!([])); + assert_eq!( + api.call("client_detail_view", unbound_detail, Some(&restricted_cookie), false) + .await + .status(), + StatusCode::NOT_FOUND + ); +} diff --git a/specs/015-scoped-permissions/contracts/client-permissions.md b/specs/015-scoped-permissions/contracts/client-permissions.md index 170c69ad..6eb4c8ae 100644 --- a/specs/015-scoped-permissions/contracts/client-permissions.md +++ b/specs/015-scoped-permissions/contracts/client-permissions.md @@ -83,3 +83,39 @@ project creation/reactivation and current client authority under the existing organization-first gate; preserve history and independent state constraints. Policy-zero compatibility and the reviewed activation delta remain explicit: the existing legacy operation changes only the client without this prerequisite. + +## Clients page read slice (partial T238) + +Scope: OP17 for the canonical Clients list and detail pages only; FR-006/008/ +010/018 and FR-035's read half. This is **not** catalog-wide enforcement and is +not sufficient for policy activation. T238 and OP17/18 remain open. + +- `client_overview(expected_requester)` returns `ClientOverview` and + `client_detail_view(client_id, expected_requester)` returns + `ClientDetailView`. Both carry the bound `PermissionRequester`; a mismatched + expected requester fails with forbidden. Each holds `ProjectReadAccess` + through materialization. +- Policy zero keeps the legacy catalog, project-progress counts and + administrator/manager billing. Policy one requires `ClientReadAll`; without + it the overview reports `can_read: false` (withheld, not an empty catalog) + and direct IDs are not found. Counts use the `list_projects` reader. The + default rate requires `BillableRateReadAll` as well; managed rate grants never + apply. Unknown policy and missing/malformed state are forbidden; inactive or + foreign actors fail closed. +- `can_manage` is the legacy manager role under policy zero and always false + under policy one. This is temporary UI state while the client writes still + check only the legacy role, not a product rule. Invoice controls must not be + inferred from billing visibility. + +Registered legacy readers left unchanged, which still expose catalog fields or +rates under policy one without `ClientReadAll`: + +| Server function | Exposure under policy one | Consumers | +| --- | --- | --- | +| `list_clients` | All organization clients with address and tax ID, for any active user | Legacy Reports filter, invoice list and invoice detail | +| `get_client_details` | Address and tax ID for any active user; default rate for the legacy administrator/manager role | `ProjectsForClient` context, client editor | +| `list_client_summaries` | Full catalog with legacy progress counts | Clients page until it adopts `client_overview` | +| `list_client_invoices` | Gated by the legacy manager role, not invoice or client grants | Client detail invoices | + +Each needs a scoped workflow-identity replacement (OP17/21/25) before +activation. From 70df7806e97dcff89e66d9993ed5416bb512bf20 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:17:11 +0200 Subject: [PATCH 02/16] Bind Clients pages to account-checked client readers Load the list and detail through client_overview and client_detail_view, rejecting responses for a changed account, organization or policy. Show management actions, the invoice panel and the editor only from a settled legacy-only can_manage, never from billing visibility, and close the editor whenever authority reloads. Distinguish denied from empty lists and hide the Clients link unless the current policy grants client read. --- crates/horae/src/components/sidebar.rs | 70 ++++++++++++++++++++++-- crates/horae/src/pages/clients.rs | 64 +++++++++++++++++++--- crates/horae/src/pages/clients/detail.rs | 24 ++++++-- 3 files changed, 139 insertions(+), 19 deletions(-) diff --git a/crates/horae/src/components/sidebar.rs b/crates/horae/src/components/sidebar.rs index d8932258..add6aa28 100644 --- a/crates/horae/src/components/sidebar.rs +++ b/crates/horae/src/components/sidebar.rs @@ -5,6 +5,7 @@ use crate::components::avatar::Avatar; use crate::components::icons::NavIcon; use crate::components::logo::HoraeMark; use crate::components::timer_widget::TimerWidget; +use crate::models::own_permissions::OwnPermissions; use crate::pages::timesheet::{Anchor, CalSpan, ViewMode}; use crate::route::Route; use crate::server_fns; @@ -14,6 +15,8 @@ use crate::server_fns; /// `AppLayout` so the shell can narrow the content area in step with the rail. #[component] pub fn Sidebar(collapsed: Signal, on_navigate: EventHandler<()>) -> Element { + let own = use_resource(server_fns::get_my_permissions); + let clients = own.state()() == UseResourceState::Ready && shows_clients(own.read().as_ref()); rsx! { aside { id: "app-navigation", class: "app-sidebar", aria_label: "Navigation", div { class: "sidebar-brand", @@ -38,7 +41,9 @@ pub fn Sidebar(collapsed: Signal, on_navigate: EventHandler<()>) -> Elemen div { class: "sidebar-section", "Organize" } div { class: "sidebar-group", - SideLink { to: Route::ClientList {}, icon: "clients", label: "Clients", on_navigate } + if clients { + SideLink { to: Route::ClientList {}, icon: "clients", label: "Clients", on_navigate } + } SideLink { to: Route::ProjectList {}, icon: "projects", label: "Projects", on_navigate } SideLink { to: Route::InvoiceList {}, icon: "invoices", label: "Invoices", on_navigate } } @@ -51,7 +56,7 @@ pub fn Sidebar(collapsed: Signal, on_navigate: EventHandler<()>) -> Elemen div { class: "sidebar-spacer" } - SidebarUser { on_navigate } + SidebarUser { own, on_navigate } } } } @@ -77,9 +82,11 @@ fn SideLink(to: Route, icon: String, label: String, on_navigate: EventHandler<() /// (settings, an admin section for admins, and sign out). Falls back to a /// placeholder until `get_me` resolves (or when not authenticated). #[component] -fn SidebarUser(on_navigate: EventHandler<()>) -> Element { +fn SidebarUser( + mut own: Resource, ServerFnError>>, + on_navigate: EventHandler<()>, +) -> Element { let me = use_resource(|| async move { server_fns::get_me().await }); - let mut own = use_resource(server_fns::get_my_permissions); let mut open = use_signal(|| false); let user = me.read(); @@ -195,6 +202,18 @@ fn SidebarUser(on_navigate: EventHandler<()>) -> Element { } } +/// Policy zero keeps Clients for everyone; failed or unknown permission state hides it. +fn shows_clients(own: Option<&Result, ServerFnError>>) -> bool { + match own { + Some(Ok(None)) => true, + Some(Ok(Some(access))) => { + access.catalog_version == PERMISSION_CATALOG_VERSION + && access.grants.contains(&Permission::ClientReadAll) + } + _ => false, + } +} + /// Capitalize the first character (roles are stored lower-case: "admin" → "Admin"). fn capitalize(s: &str) -> String { let mut chars = s.chars(); @@ -212,3 +231,46 @@ fn initials(name: &str) -> String { .collect::() .to_uppercase() } + +#[cfg(test)] +mod tests { + use super::*; + + fn canonical(grants: Vec, catalog_version: u32) -> OwnPermissions { + OwnPermissions { + catalog_version, + grants, + is_administrator: false, + access_revision: 1, + person_revision: 1, + managed_person_ids: Vec::new(), + managed_project_ids: Vec::new(), + } + } + + #[test] + fn clients_link_follows_client_read_and_fails_closed() { + let shows = |own: Result, ServerFnError>| shows_clients(Some(&own)); + assert!(!shows_clients(None), "loading"); + assert!(shows(Ok(None)), "policy zero"); + assert!(!shows(Err(ServerFnError::new("unavailable")))); + assert!(shows(Ok(Some(canonical( + vec![Permission::ClientReadAll], + PERMISSION_CATALOG_VERSION + ))))); + assert!(!shows(Ok(Some(canonical( + vec![Permission::ClientReadAll], + PERMISSION_CATALOG_VERSION + 1 + ))))); + let unrelated = vec![ + Permission::ProjectReadAll, + Permission::InvoiceReadAll, + Permission::BillableRateReadAll, + Permission::ReportProfitabilityRead, + ]; + assert!(!shows(Ok(Some(canonical( + unrelated, + PERMISSION_CATALOG_VERSION + ))))); + } +} diff --git a/crates/horae/src/pages/clients.rs b/crates/horae/src/pages/clients.rs index 6863bef6..27a9e46a 100644 --- a/crates/horae/src/pages/clients.rs +++ b/crates/horae/src/pages/clients.rs @@ -2,10 +2,11 @@ use dioxus::prelude::*; use std::collections::BTreeSet; use uuid::Uuid; -use super::{is_admin, is_manager, run_action}; +use super::{is_admin, run_action}; use crate::components::badge::Badge; use crate::components::icons::NavIcon; use crate::components::menu::{Menu, MenuItem}; +use crate::models::permission_editor::PermissionRequester; use crate::route::Route; use crate::server_fns; @@ -20,12 +21,24 @@ pub use detail::ClientDetail; #[path = "clients/form.rs"] mod form; +/// The account and permission policy a Clients page first loaded under. +type Binding = (PermissionRequester, bool); + +/// Rejects a response for another account, organization or policy than the +/// page is bound to, so a refresh never shows the previous identity's data. +fn bind(mut binding: Signal>, current: Binding) -> Result<(), ServerFnError> { + if binding.peek().is_some_and(|expected| expected != current) { + return Err(ServerFnError::new( + "Your session or client policy changed. Reload this page to continue.", + )); + } + binding.set(Some(current)); + Ok(()) +} + #[component] pub fn ClientList() -> Element { // Management view: include inactive clients so managers can reactivate them. - let mut clients = use_resource(|| async move { server_fns::list_client_summaries().await }); - let me = use_resource(|| async move { server_fns::get_me().await }); - let mut query = use_signal(String::new); let mut scope = use_signal(ClientScope::default); let mut currency_filter = use_signal(String::new); @@ -35,11 +48,35 @@ pub fn ClientList() -> Element { let mut editing_id = use_signal(|| None::); let action_error = use_signal(|| None::); - let is_manager = is_manager(&me); - let can_import = is_admin(&me); + let binding = use_signal(|| None::); + let mut clients = use_resource(move || async move { + // An editor opened under the previous authority must not survive a refresh. + editing_id.set(None); + show_form.set(false); + let expected = binding.peek().map(|(requester, _)| requester); + let overview = server_fns::client_overview(expected).await?; + bind( + binding, + (overview.requester, overview.canonical_permissions), + )?; + Ok::<_, ServerFnError>(overview) + }); + let me = use_resource(|| async move { server_fns::get_me().await }); + let ready = clients.state()() == UseResourceState::Ready; let data = clients.read(); - let list = data.as_ref().and_then(|result| result.as_ref().ok()); + // A reloading resource keeps its previous value; never act on it. + let overview = ready + .then(|| data.as_ref().and_then(|result| result.as_ref().ok())) + .flatten(); + // Client writes are not yet integrated with canonical permissions, so the + // server grants `can_manage` only to legacy-policy managers. + let is_manager = overview.is_some_and(|overview| overview.can_manage); + let can_import = is_manager && is_admin(&me); + let denied = overview.is_some_and(|overview| !overview.can_read); + let list = overview + .filter(|overview| overview.can_read) + .map(|overview| &overview.clients); let matching: Vec<_> = list .into_iter() .flatten() @@ -83,6 +120,7 @@ pub fn ClientList() -> Element { div { class: "page-actions items-center gap-4", if is_manager { button { + id: "clients-new", class: "btn btn-primary", onclick: move |_| { editing_id.set(None); show_form.set(true); }, "New client" @@ -139,10 +177,16 @@ pub fn ClientList() -> Element { p { class: "text-sm text-secondary", role: "status", aria_busy: "true", "Loading clients…" } } else if let Some(Err(_)) = &*data { div { class: "alert alert-danger", role: "alert", - "Could not load clients. Retry to refresh the list. " - button { r#type: "button", class: "btn btn-secondary btn-sm", + "Could not load clients. Retry, or reload this page if your account changed. " + button { id: "clients-retry", r#type: "button", class: "btn btn-secondary btn-sm", onclick: move |_| clients.restart(), "Retry clients" } } + } else if denied { + div { class: "empty-state rounded-xl", + div { class: "empty-state-icon-tile", aria_hidden: "true", NavIcon { name: "clients" } } + h2 { class: "empty-state-title text-xl m-0", "Client access is unavailable" } + p { class: "empty-state-text m-0", "Your permissions do not include the client list." } + } } else if let Some(list) = list { if visible.is_empty() { div { class: "empty-state rounded-xl", @@ -205,6 +249,7 @@ pub fn ClientList() -> Element { td { div { class: "flex items-center gap-2", button { + id: "client-edit-{c.id}", class: "btn btn-secondary btn-sm", onclick: { let c = c.clone(); @@ -216,6 +261,7 @@ pub fn ClientList() -> Element { "Edit" } button { + id: "client-activity-{c.id}", class: "btn btn-secondary btn-sm", onclick: { let id = c.id; diff --git a/crates/horae/src/pages/clients/detail.rs b/crates/horae/src/pages/clients/detail.rs index b62b1b75..9aec7fd3 100644 --- a/crates/horae/src/pages/clients/detail.rs +++ b/crates/horae/src/pages/clients/detail.rs @@ -3,6 +3,7 @@ use horae_core::{duration::format_hhmm, money::format_cents}; use uuid::Uuid; use crate::components::{badge::Badge, icons::NavIcon}; +use crate::models::client::ClientDetailView; use crate::route::Route; use crate::server_fns; @@ -15,10 +16,21 @@ pub fn ClientDetail(id: Uuid) -> Element { #[component] fn ClientDetailContent(id: Uuid) -> Element { let mut show_editor = use_signal(|| false); - let mut details = - use_resource(move || async move { server_fns::get_client_details(id.to_string()).await }); + let binding = use_signal(|| None::); + let mut details = use_resource(move || async move { + // An editor opened under the previous authority must not survive a refresh. + show_editor.set(false); + let expected = binding.peek().map(|(requester, _)| requester); + let view = server_fns::client_detail_view(id.to_string(), expected).await?; + super::bind(binding, (view.requester, view.canonical_permissions))?; + Ok::<_, ServerFnError>(view) + }); let ready = details.state()() == UseResourceState::Ready; let data = details.read(); + // Legacy-only until client writes and invoices use canonical permissions; + // billing visibility is a separate grant and never implies management. + // A reloading resource keeps its previous value, so require a settled one. + let can_manage = ready && matches!(&*data, Some(Ok(view)) if view.can_manage); rsx! { div { class: "client-detail min-w-0", @@ -28,7 +40,7 @@ fn ClientDetailContent(id: Uuid) -> Element { } if !ready { p { role: "status", aria_busy: "true", "Loading client…" } - } else if let Some(Ok(detail)) = &*data { + } else if let Some(Ok(ClientDetailView { details: detail, .. })) = &*data { div { class: "page-header border-b pb-6 mb-4", div { class: "min-w-0", p { class: "text-xs text-secondary m-0 mb-2", @@ -44,7 +56,7 @@ fn ClientDetailContent(id: Uuid) -> Element { if !detail.client.active { Badge { variant: "neutral", "Inactive" } } } } - if detail.billing.is_some() { + if can_manage { div { class: "page-actions", if detail.client.active { Link { to: Route::NewProjectForClient { client: id.to_string() }, class: "btn btn-secondary", "New project" } @@ -81,7 +93,7 @@ fn ClientDetailContent(id: Uuid) -> Element { } } } - if detail.billing.is_some() { ClientInvoices { client_id: id } } + if can_manage { ClientInvoices { client_id: id } } } } } else { @@ -91,7 +103,7 @@ fn ClientDetailContent(id: Uuid) -> Element { onclick: move |_| details.restart(), "Retry client" } } } - super::form::ClientEditor { open: show_editor(), client_id: Some(id), focus_fallback: "client-back", + super::form::ClientEditor { open: show_editor() && can_manage, client_id: Some(id), focus_fallback: "client-back", on_dismiss: move |_| show_editor.set(false), on_saved: move |_| { show_editor.set(false); details.restart(); }, } From f3016be642397494974c094ea70596fa30d2914d Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:17:11 +0200 Subject: [PATCH 03/16] Cover Clients read authority, denial and stale identity Add SSR cases for legacy and policy-one readers, denied versus empty lists, restricted versus unset default rates, non-disclosing detail errors, pending refreshes and changed requesters. Point Clients browser suites at the new reader endpoints and assert their legacy flags. --- crates/horae/tests/browser/action-errors.cjs | 2 +- crates/horae/tests/browser/clients-access.cjs | 10 +- crates/horae/tests/browser/clients-errors.cjs | 24 +- .../horae/tests/browser/mobile-navigation.cjs | 2 +- .../horae/tests/browser/responsive-layout.cjs | 2 +- .../tests/browser/shared-style-audit.cjs | 2 +- crates/horae/tests/detail_navigation.rs | 69 +++- .../horae/tests/detail_navigation/clients.rs | 338 ++++++++++++++++++ 8 files changed, 427 insertions(+), 22 deletions(-) create mode 100644 crates/horae/tests/detail_navigation/clients.rs diff --git a/crates/horae/tests/browser/action-errors.cjs b/crates/horae/tests/browser/action-errors.cjs index 8fd0f44f..5e1739cd 100644 --- a/crates/horae/tests/browser/action-errors.cjs +++ b/crates/horae/tests/browser/action-errors.cjs @@ -19,7 +19,7 @@ assert.ok(base, 'Set HORAE_TEST_URL to an isolated, seeded test instance'); page.on('pageerror', error => pageErrors.push(error.message)); const cases = [ { - name: 'client activation', path: '/clients', resource: 'list_client_summaries', + name: 'client activation', path: '/clients', resource: 'client_overview', endpoint: 'set_client_active', form: 'New client', formEndpoint: 'create_client_profile', submit: 'Create client', action: async () => page.getByRole('button', { name: 'Deactivate', exact: true }).first().click(), diff --git a/crates/horae/tests/browser/clients-access.cjs b/crates/horae/tests/browser/clients-access.cjs index 10398bfc..389a1598 100644 --- a/crates/horae/tests/browser/clients-access.cjs +++ b/crates/horae/tests/browser/clients-access.cjs @@ -87,7 +87,7 @@ const name = 'Access scope client'; for (const patch of [{ default_rate: '-1' }, { profile: { ...createPayload.profile, currency: 'XYZ' } }]) { assert.equal((await post('create_client_profile', { ...createPayload, ...patch })).status(), 400); } - const reads = [['get_client_details', { client_id: client }], ['list_client_summaries', {}], ['list_client_invoices', { client_id: client }]]; + const reads = [['get_client_details', { client_id: client }], ['client_overview', null], ['client_detail_view', null], ['list_client_invoices', { client_id: client }]]; for (const scenario of [ { role: 'admin', assignment: 'freelancer', count: 2, billing: true }, { role: 'manager', assignment: 'freelancer', count: 2, billing: true }, @@ -102,7 +102,10 @@ const name = 'Access scope client'; const row = page.getByRole('row').filter({ has: page.getByRole('link', { name, exact: true }) }); await expect(row.getByLabel(`${scenario.count} active of ${scenario.count} visible projects`)).toBeVisible(); await expect(page.getByRole('button', { name: 'New client', exact: true })).toHaveCount(scenario.billing ? 1 : 0); - const summaries = await json('list_client_summaries', {}); + const overview = await json('client_overview', null); + assert.deepEqual([overview.canonical_permissions, overview.can_read, overview.can_manage], [false, true, scenario.billing]); + assert.deepEqual(overview.requester, { org_id: actor.org_id, user_id: actor.id }); + const summaries = overview.clients; assert.equal(summaries.some(item => item.client.id === foreignClient), false); const summary = summaries.find(item => item.client.id === client); assert.equal(summary.total_projects, scenario.count); @@ -115,6 +118,9 @@ const name = 'Access scope client'; await expect(page.getByRole('link', { name: 'Access hidden project', exact: true })).toHaveCount(scenario.count === 2 ? 1 : 0); await expect(page.getByRole('button', { name: 'Edit client', exact: true })).toHaveCount(scenario.billing ? 1 : 0); await expect(page.getByRole('link', { name: 'New invoice', exact: true })).toHaveCount(scenario.billing ? 1 : 0); + const view = await json('client_detail_view', null); + assert.deepEqual([view.canonical_permissions, view.can_manage, view.details.client.id], [false, scenario.billing, client]); + assert.equal(Object.hasOwn(view.details, 'billing'), scenario.billing); const detail = await json('get_client_details', { client_id: client }); assert.equal(Object.hasOwn(detail, 'billing'), scenario.billing); const projectRows = await json('list_projects', { client_id: client, include_inactive: true }); diff --git a/crates/horae/tests/browser/clients-errors.cjs b/crates/horae/tests/browser/clients-errors.cjs index ed3a4cd4..5547c06c 100644 --- a/crates/horae/tests/browser/clients-errors.cjs +++ b/crates/horae/tests/browser/clients-errors.cjs @@ -23,13 +23,13 @@ const sql = query => execFileSync('psql', [process.env.DATABASE_URL, '-X', '-v', await page.waitForURL(`${base}/`); let release; const gate = new Promise(resolve => { release = resolve; }); - await page.route('**/api/list_client_summaries*', async route => { await gate; await route.abort('failed'); }); + await page.route('**/api/client_overview*', async route => { await gate; await route.abort('failed'); }); await page.goto(`${base}/clients`); await expect(page.getByRole('status')).toContainText('Loading clients'); await expect(page.getByText('No clients yet', { exact: true })).toHaveCount(0); release(); await expect(page.getByRole('alert')).toContainText('Could not load clients'); - await page.unroute('**/api/list_client_summaries*'); + await page.unroute('**/api/client_overview*'); await page.getByRole('button', { name: 'Retry clients', exact: true }).click(); await expect(page.getByRole('link', { name: 'Acme Corp', exact: true })).toBeVisible(); const search = page.getByRole('searchbox', { name: 'Search clients by name' }); @@ -66,14 +66,24 @@ const sql = query => execFileSync('psql', [process.env.DATABASE_URL, '-X', '-v', await expect(page.locator('#client-currency-menu-trigger')).toContainText('All currencies'); await expect(page.getByRole('link', { name: 'State filter inactive', exact: true })).toBeVisible(); // Explicitly injected empty payload checks presentation, not database emptiness. - await page.route('**/api/list_client_summaries*', route => route.fulfill({ status: 200, contentType: 'application/json', body: '[]' })); + const overview = async patch => page.route('**/api/client_overview*', async route => { + const response = await route.fetch(); + await route.fulfill({ response, json: { ...(await response.json()), clients: [], ...patch } }); + }); + await overview({}); await page.reload(); await expect(page.getByRole('heading', { name: 'No clients yet', exact: true })).toBeVisible(); - await page.unroute('**/api/list_client_summaries*'); - console.log('PASS: clients loading/error/retry and distinct empty/no-match states'); + await page.unroute('**/api/client_overview*'); + await overview({ can_read: false, can_manage: false }); + await page.reload(); + await expect(page.getByRole('heading', { name: 'Client access is unavailable', exact: true })).toBeVisible(); + await expect(page.getByText('No clients yet', { exact: true })).toHaveCount(0); + await expect(page.getByRole('button', { name: 'New client', exact: true })).toHaveCount(0); + await page.unroute('**/api/client_overview*'); + console.log('PASS: clients loading/error/retry and distinct empty/denied/no-match states'); for (const scenario of [ - { endpoint: 'get_client_details', retry: 'Retry client', message: 'Could not load this client' }, + { endpoint: 'client_detail_view', retry: 'Retry client', message: 'Could not load this client' }, { endpoint: 'list_projects', retry: 'Retry projects', message: 'Could not load projects' }, { endpoint: 'list_project_spend', retry: 'Retry totals', message: 'Could not load project totals' }, { endpoint: 'list_client_invoices', retry: 'Retry invoices', message: 'Could not load invoices' }, @@ -82,7 +92,7 @@ const sql = query => execFileSync('psql', [process.env.DATABASE_URL, '-X', '-v', await page.route(pattern, route => route.abort('failed')); await page.goto(`${base}/clients/${client}`); await expect(page.getByRole('alert')).toContainText(scenario.message); - if (scenario.endpoint !== 'get_client_details') { + if (scenario.endpoint !== 'client_detail_view') { await expect(page.getByRole('heading', { name: 'Acme Corp', exact: true })).toBeVisible(); await expect(page.getByRole('region', { name: 'Billing', exact: true })).toBeVisible(); } diff --git a/crates/horae/tests/browser/mobile-navigation.cjs b/crates/horae/tests/browser/mobile-navigation.cjs index b4823c6c..49cf9a61 100644 --- a/crates/horae/tests/browser/mobile-navigation.cjs +++ b/crates/horae/tests/browser/mobile-navigation.cjs @@ -16,7 +16,7 @@ assert.ok(base, 'Set HORAE_TEST_URL to an isolated test instance'); await page.goto(`${base}/auth/login`); await page.getByRole('button', { name: 'Sign in as Admin' }).click(); await page.waitForURL(`${base}/`); - const ready = page.waitForResponse(r => r.url().includes('/api/list_client_summaries') && r.status() === 200); + const ready = page.waitForResponse(r => r.url().includes('/api/client_overview') && r.status() === 200); await page.goto(`${base}/clients`); await (await ready).finished(); const sidebar = page.locator('.app-sidebar'); diff --git a/crates/horae/tests/browser/responsive-layout.cjs b/crates/horae/tests/browser/responsive-layout.cjs index 45a35f9d..412967cc 100644 --- a/crates/horae/tests/browser/responsive-layout.cjs +++ b/crates/horae/tests/browser/responsive-layout.cjs @@ -65,7 +65,7 @@ const sunday = new Date(monday.getTime() + 6 * 86400000).toISOString().slice(0, await page.getByRole('button', { name: 'Sign in as Admin' }).click(); await page.waitForURL(`${base}/`); for (const [path, resource] of [ - ['/clients', 'list_client_summaries'], ['/projects', 'get_project_overview'], + ['/clients', 'client_overview'], ['/projects', 'get_project_overview'], ['/invoices', 'list_invoices'], ['/reports', 'report_time'], ['/admin/users', 'list_users'], ['/approvals', 'list_approvals'], ['/settings', 'get_me'], ['/admin/importers', 'get_me'], diff --git a/crates/horae/tests/browser/shared-style-audit.cjs b/crates/horae/tests/browser/shared-style-audit.cjs index 700556bf..2d3295c5 100644 --- a/crates/horae/tests/browser/shared-style-audit.cjs +++ b/crates/horae/tests/browser/shared-style-audit.cjs @@ -24,7 +24,7 @@ assert.ok(['localhost', '127.0.0.1'].includes(target.hostname) && target.port != await page.waitForURL(`${base}/`); const result = {}; for (const [path, resource] of [ - ['/clients', 'list_client_summaries'], ['/invoices', 'list_invoices'], + ['/clients', 'client_overview'], ['/invoices', 'list_invoices'], ['/reports', 'report_time'], ['/admin/users', 'list_users'], ['/settings', 'get_me'], ['/admin/importers', 'import/harvest/connection'], ['/components', 'get_me'], [`/timesheet/week/${week}`, 'load_timesheet_page'], diff --git a/crates/horae/tests/detail_navigation.rs b/crates/horae/tests/detail_navigation.rs index 107e33a0..30689a83 100644 --- a/crates/horae/tests/detail_navigation.rs +++ b/crates/horae/tests/detail_navigation.rs @@ -75,6 +75,8 @@ type AssignmentResponse = Result, ServerFnError>; type ProjectDetailsResponse = Result; type DeferredResponse = Rc>>>>; +#[path = "detail_navigation/clients.rs"] +mod client_tests; #[path = "detail_navigation/detail.rs"] mod detail_tests; #[path = "detail_navigation/overview.rs"] @@ -87,6 +89,12 @@ type ProjectSpendResponse = Result, ServerFnError> #[derive(Clone, Default)] struct Probe { member: bool, + canonical: bool, + client_overview_requests: Rc>>>, + client_overview_response: DeferredResponse, + client_view_requests: Rc>>>, + client_view_response: DeferredResponse, + client_status_changes: Rc>>, client_requests: Rc>>, client_invoice_requests: Rc>>, client_response: Rc>>>, @@ -823,13 +831,23 @@ fn loaded( } } +// Mirrors the production helper; endpoint doubles decide whether a mutation is allowed. fn run_action( - _future: impl std::future::Future> + 'static, - _resource: Resource, - _error: Signal>, - _on_ok: impl FnOnce() + 'static, + future: impl std::future::Future> + 'static, + mut resource: Resource, + mut error: Signal>, + on_ok: impl FnOnce() + 'static, ) { - panic!("navigation must not trigger a mutation"); + spawn(async move { + match future.await { + Ok(_) => { + error.set(None); + on_ok(); + resource.restart(); + } + Err(problem) => error.set(Some(problem.to_string())), + } + }); } mod server_fns { @@ -908,8 +926,34 @@ mod server_fns { Ok(vec![invoice]) } - pub async fn list_client_summaries() -> Result, ServerFnError> { - Ok(Vec::new()) + pub async fn client_overview( + expected: Option, + ) -> Result { + let probe = consume_context::(); + probe.client_overview_requests.borrow_mut().push(expected); + let response = probe.client_overview_response.borrow_mut().take(); + if let Some(response) = response { + return response.await.unwrap(); + } + Ok(super::client_tests::overview(&probe)) + } + + pub async fn client_detail_view( + id: String, + expected: Option, + ) -> Result { + let probe = consume_context::(); + probe.client_view_requests.borrow_mut().push(expected); + let response = probe.client_view_response.borrow_mut().take(); + if let Some(response) = response { + return response.await.unwrap(); + } + Ok(client::ClientDetailView { + requester: super::client_tests::requester(), + canonical_permissions: probe.canonical, + can_manage: !probe.member && !probe.canonical, + details: get_client_details(id).await?, + }) } pub async fn create_client_profile( _profile: client::ClientProfile, @@ -923,8 +967,15 @@ mod server_fns { ) -> ClientDetailsResponse { panic!("navigation must not edit clients"); } - pub async fn set_client_active(_id: String, _active: bool) -> Result { - panic!("navigation must not change client status"); + pub async fn set_client_active(id: String, active: bool) -> Result { + let id = Uuid::parse_str(&id).unwrap(); + let probe = consume_context::(); + assert!( + !probe.member && !probe.canonical, + "only legacy managers may change client status" + ); + probe.client_status_changes.borrow_mut().push((id, active)); + Ok(super::client_tests::client_record(id)) } pub async fn list_assignments( diff --git a/crates/horae/tests/detail_navigation/clients.rs b/crates/horae/tests/detail_navigation/clients.rs new file mode 100644 index 00000000..68ff1357 --- /dev/null +++ b/crates/horae/tests/detail_navigation/clients.rs @@ -0,0 +1,338 @@ +use super::*; +use overview_tests::click; + +const NOT_FOUND: u16 = 404; +/// Markers for the legacy client mutation controls on the list. +const LIST_ACTIONS: [&str; 3] = [ + "id=\"clients-new\"", + "id=\"client-edit-", + "id=\"client-activity-", +]; + +pub(super) fn requester() -> permission_editor::PermissionRequester { + permission_editor::PermissionRequester { + org_id: Uuid::from_u128(10), + user_id: Uuid::from_u128(20), + } +} + +pub(super) fn client_record(id: Uuid) -> client::Client { + client::Client { + id, + org_id: Uuid::nil(), + name: format!("Client-{}", id.as_u128()), + currency: "EUR".into(), + address: Some(format!("Address-{}", id.as_u128())), + // Set, so "Not set" can only refer to the default rate. + tax_id: Some(format!("Tax-{}", id.as_u128())), + active: true, + created_at: chrono::DateTime::UNIX_EPOCH, + } +} + +/// The backend's legacy-only management rule: policy one never manages yet. +pub(super) fn overview(probe: &Probe) -> client::ClientOverview { + client::ClientOverview { + requester: requester(), + canonical_permissions: probe.canonical, + can_read: true, + can_manage: !probe.member && !probe.canonical, + clients: vec![client::ClientSummary { + client: client_record(Uuid::from_u128(1)), + active_projects: 1, + total_projects: 1, + project_currencies: vec!["USD".into()], + }], + } +} + +fn respond(slot: &DeferredResponse, value: Result) { + let (send, receive) = oneshot::channel(); + assert!(send.send(value).is_ok()); + *slot.borrow_mut() = Some(receive); +} + +fn mount(probe: &Probe) -> VirtualDom { + let mut dom = VirtualDom::new_with_props(app, probe.clone()); + dom.rebuild_in_place(); + settle(&mut dom); + dom +} + +fn list(probe: Probe) -> Probe { + Probe { + initial_path: Some("/clients".into()), + ..probe + } +} + +fn detail(probe: Probe) -> Probe { + Probe { + initial_path: Some(format!("/clients/{}", Uuid::from_u128(1))), + ..probe + } +} + +fn has_list_actions(html: &str) -> bool { + LIST_ACTIONS.iter().any(|marker| html.contains(marker)) +} + +#[tokio::test] +async fn legacy_manager_list_keeps_rows_and_management_actions() { + let dom = mount(&list(Probe::default())); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("Client-1"), "{html}"); + for marker in LIST_ACTIONS { + assert!(html.contains(marker), "missing {marker}: {html}"); + } + assert!(html.contains("href=\"/admin/importers\""), "{html}"); +} + +#[tokio::test] +async fn legacy_member_and_policy_one_readers_get_rows_without_legacy_writes() { + for probe in [ + Probe { + member: true, + ..Probe::default() + }, + // `get_me` still reports a legacy Admin role here; it must not grant writes. + Probe { + canonical: true, + ..Probe::default() + }, + ] { + let probe = list(probe); + let dom = mount(&probe); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("Client-1"), "{html}"); + assert!(!has_list_actions(&html), "{html}"); + assert!(!html.contains("href=\"/admin/importers\""), "{html}"); + assert!(probe.client_status_changes.borrow().is_empty()); + } +} + +#[tokio::test] +async fn denied_list_is_neither_empty_nor_populated() { + let probe = list(Probe { + canonical: true, + ..Probe::default() + }); + let mut denied = overview(&probe); + // Fail closed even if a denied response were to carry rows. + denied.can_read = false; + respond(&probe.client_overview_response, Ok(denied)); + let dom = mount(&probe); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("Client access is unavailable"), "{html}"); + for hidden in ["Client-1", "No clients yet", "Active clients"] { + assert!(!html.contains(hidden), "shown {hidden}: {html}"); + } + assert!(!has_list_actions(&html), "{html}"); +} + +#[tokio::test] +async fn readable_empty_list_is_a_genuine_empty_state() { + let probe = list(Probe { + canonical: true, + ..Probe::default() + }); + let mut empty = overview(&probe); + empty.clients.clear(); + respond(&probe.client_overview_response, Ok(empty)); + let dom = mount(&probe); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("No clients yet"), "{html}"); + assert!(!html.contains("Client access is unavailable"), "{html}"); + assert!(!has_list_actions(&html), "{html}"); +} + +#[tokio::test] +async fn list_loading_and_errors_never_claim_empty_or_denied() { + let probe = list(Probe::default()); + let (send, receive) = oneshot::channel(); + *probe.client_overview_response.borrow_mut() = Some(receive); + let mut dom = mount(&probe); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("Loading clients"), "{html}"); + assert!(!has_list_actions(&html), "{html}"); + send.send(Err(ServerFnError::new("Clients unavailable"))) + .unwrap(); + settle(&mut dom); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("Could not load clients"), "{html}"); + for hidden in ["No clients yet", "Client access is unavailable", "Client-1"] { + assert!(!html.contains(hidden), "shown {hidden}: {html}"); + } + assert!(!has_list_actions(&html), "{html}"); + click(&mut dom, "clients-retry"); + assert!(dioxus::ssr::render(&dom).contains("Client-1")); + // A failed load binds nothing, so the retry is still unbound. + assert_eq!(*probe.client_overview_requests.borrow(), [None, None]); +} + +#[tokio::test] +async fn pending_refresh_withholds_previous_manager_controls_and_editor() { + let probe = list(Probe::default()); + let id = Uuid::from_u128(1); + let mut dom = mount(&probe); + click(&mut dom, &format!("client-edit-{id}")); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("Save changes"), "{html}"); + let (send, receive) = oneshot::channel(); + *probe.client_overview_response.borrow_mut() = Some(receive); + click(&mut dom, &format!("client-activity-{id}")); + assert_eq!(*probe.client_status_changes.borrow(), [(id, false)]); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("Loading clients"), "{html}"); + for hidden in [ + "Client-1", + "Address-1", + "Save changes", + "href=\"/admin/importers\"", + ] { + assert!(!html.contains(hidden), "stale {hidden}: {html}"); + } + assert!(!has_list_actions(&html), "{html}"); + assert_eq!( + *probe.client_overview_requests.borrow(), + [None, Some(requester())] + ); + send.send(Ok(overview(&probe))).unwrap(); + settle(&mut dom); + let html = dioxus::ssr::render(&dom); + assert!( + html.contains("Client-1") && has_list_actions(&html), + "{html}" + ); + assert!(!html.contains("Save changes"), "editor reopened: {html}"); +} + +#[tokio::test] +async fn list_refresh_rejects_changed_account_organization_or_policy() { + for change in ["user", "organization", "policy"] { + let probe = list(Probe::default()); + let id = Uuid::from_u128(1); + let mut dom = mount(&probe); + let (send, receive) = oneshot::channel(); + *probe.client_overview_response.borrow_mut() = Some(receive); + click(&mut dom, &format!("client-activity-{id}")); + let mut changed = overview(&probe); + match change { + "user" => changed.requester.user_id = Uuid::now_v7(), + "organization" => changed.requester.org_id = Uuid::now_v7(), + _ => { + changed.canonical_permissions = true; + changed.can_manage = false; + } + } + changed.clients[0].client.name = "Other account client".into(); + send.send(Ok(changed)).unwrap(); + settle(&mut dom); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("Could not load clients"), "{change}: {html}"); + for hidden in ["Other account client", "Client-1"] { + assert!(!html.contains(hidden), "{change} shown {hidden}: {html}"); + } + assert!(!has_list_actions(&html), "{change}: {html}"); + } +} + +#[tokio::test] +async fn policy_one_rate_reader_sees_rate_without_management_or_invoices() { + let probe = detail(Probe { + canonical: true, + ..Probe::default() + }); + let dom = mount(&probe); + let html = dioxus::ssr::render(&dom); + for expected in ["Client-1", "Address-1", "Default rate", "EUR 0.00 / h"] { + assert!(html.contains(expected), "missing {expected}: {html}"); + } + // The editor's dialog title is always mounted; the button is the second mention. + assert_eq!(html.matches("Edit client").count(), 1, "{html}"); + for prefix in ["/projects/new/client/", "/invoices/new/client/"] { + assert!(!html.contains(prefix), "{html}"); + } + assert!(!html.contains("Invoices"), "{html}"); + assert!(probe.client_invoice_requests.borrow().is_empty()); + assert_eq!(*probe.client_view_requests.borrow(), [None]); + // Project context stays reachable without client management. + assert!(html.contains("href=\"/projects/client/"), "{html}"); +} + +#[tokio::test] +async fn policy_one_detail_distinguishes_restricted_from_unset_rate() { + for (billing, rate_shown, unset_shown) in [ + (None, false, false), + ( + Some(client::ClientBilling { + default_rate_cents: None, + }), + true, + true, + ), + ] { + let probe = detail(Probe { + canonical: true, + ..Probe::default() + }); + let id = Uuid::from_u128(1); + respond( + &probe.client_view_response, + Ok(client::ClientDetailView { + requester: requester(), + canonical_permissions: true, + can_manage: false, + details: client::ClientDetails { + client: client_record(id), + billing, + }, + }), + ); + let dom = mount(&probe); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("Address-1"), "{html}"); + assert_eq!(html.contains("Default rate"), rate_shown, "{html}"); + assert_eq!(html.contains("Not set"), unset_shown, "{html}"); + assert!(!html.contains("/ h"), "{html}"); + assert!(!html.contains("Invoices"), "{html}"); + assert!(probe.client_invoice_requests.borrow().is_empty()); + } +} + +#[tokio::test] +async fn hidden_or_missing_client_detail_reveals_no_identity() { + let probe = detail(Probe { + canonical: true, + ..Probe::default() + }); + respond( + &probe.client_view_response, + Err(ServerFnError::ServerError { + code: NOT_FOUND, + message: "Client not found".into(), + details: None, + }), + ); + let dom = mount(&probe); + let html = dioxus::ssr::render(&dom); + assert!(html.contains("Could not load this client"), "{html}"); + for hidden in ["Client-1", "Address-1", "Default rate", "Invoices"] { + assert!(!html.contains(hidden), "shown {hidden}: {html}"); + } + assert!(probe.client_invoice_requests.borrow().is_empty()); +} + +#[tokio::test] +async fn client_project_context_keeps_its_existing_reader() { + let id = Uuid::from_u128(1); + let probe = Probe { + canonical: true, + initial_path: Some(format!("/projects/client/{id}")), + ..Probe::default() + }; + mount(&probe); + assert!(probe.client_requests.borrow().contains(&id)); + assert!(probe.client_view_requests.borrow().is_empty()); + assert!(probe.client_overview_requests.borrow().is_empty()); +} From 4f4fba4b882ba3da690725c6aea0210df15fe770 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:36:53 +0200 Subject: [PATCH 04/16] Keep Clients navigation mutations opt-in in tests Gate the run_action double and the set_client_active double on an explicit Probe flag, enabled only by the two refresh tests that change client status, so other navigation tests still fail fast on any mutation. --- crates/horae/tests/detail_navigation.rs | 13 ++++++++++++- crates/horae/tests/detail_navigation/clients.rs | 10 ++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/crates/horae/tests/detail_navigation.rs b/crates/horae/tests/detail_navigation.rs index 30689a83..87ef59aa 100644 --- a/crates/horae/tests/detail_navigation.rs +++ b/crates/horae/tests/detail_navigation.rs @@ -90,6 +90,9 @@ type ProjectSpendResponse = Result, ServerFnError> struct Probe { member: bool, canonical: bool, + /// Opt-in for tests that refresh Clients through a status change; every + /// other navigation mutation stays fail-fast. + allow_client_activity: bool, client_overview_requests: Rc>>>, client_overview_response: DeferredResponse, client_view_requests: Rc>>>, @@ -831,13 +834,17 @@ fn loaded( } } -// Mirrors the production helper; endpoint doubles decide whether a mutation is allowed. +// Mirrors the production helper only for tests that opt into client activity. fn run_action( future: impl std::future::Future> + 'static, mut resource: Resource, mut error: Signal>, on_ok: impl FnOnce() + 'static, ) { + assert!( + consume_context::().allow_client_activity, + "navigation must not trigger a mutation" + ); spawn(async move { match future.await { Ok(_) => { @@ -970,6 +977,10 @@ mod server_fns { pub async fn set_client_active(id: String, active: bool) -> Result { let id = Uuid::parse_str(&id).unwrap(); let probe = consume_context::(); + assert!( + probe.allow_client_activity, + "navigation must not change client status" + ); assert!( !probe.member && !probe.canonical, "only legacy managers may change client status" diff --git a/crates/horae/tests/detail_navigation/clients.rs b/crates/horae/tests/detail_navigation/clients.rs index 68ff1357..3c5c4e50 100644 --- a/crates/horae/tests/detail_navigation/clients.rs +++ b/crates/horae/tests/detail_navigation/clients.rs @@ -172,7 +172,10 @@ async fn list_loading_and_errors_never_claim_empty_or_denied() { #[tokio::test] async fn pending_refresh_withholds_previous_manager_controls_and_editor() { - let probe = list(Probe::default()); + let probe = list(Probe { + allow_client_activity: true, + ..Probe::default() + }); let id = Uuid::from_u128(1); let mut dom = mount(&probe); click(&mut dom, &format!("client-edit-{id}")); @@ -210,7 +213,10 @@ async fn pending_refresh_withholds_previous_manager_controls_and_editor() { #[tokio::test] async fn list_refresh_rejects_changed_account_organization_or_policy() { for change in ["user", "organization", "policy"] { - let probe = list(Probe::default()); + let probe = list(Probe { + allow_client_activity: true, + ..Probe::default() + }); let id = Uuid::from_u128(1); let mut dom = mount(&probe); let (send, receive) = oneshot::channel(); From 4ed3ca54586a58f9790fa4e73346cc49f0149f2b Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:39:01 +0200 Subject: [PATCH 05/16] Read the bound Clients requester through Option Signal::peek returns a guard whose own map method shadows Option::map; borrow the Option explicitly before copying the requester out. --- crates/horae/src/pages/clients.rs | 2 +- crates/horae/src/pages/clients/detail.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/horae/src/pages/clients.rs b/crates/horae/src/pages/clients.rs index 27a9e46a..edd1f7ed 100644 --- a/crates/horae/src/pages/clients.rs +++ b/crates/horae/src/pages/clients.rs @@ -53,7 +53,7 @@ pub fn ClientList() -> Element { // An editor opened under the previous authority must not survive a refresh. editing_id.set(None); show_form.set(false); - let expected = binding.peek().map(|(requester, _)| requester); + let expected = binding.peek().as_ref().map(|(requester, _)| *requester); let overview = server_fns::client_overview(expected).await?; bind( binding, diff --git a/crates/horae/src/pages/clients/detail.rs b/crates/horae/src/pages/clients/detail.rs index 9aec7fd3..950e3525 100644 --- a/crates/horae/src/pages/clients/detail.rs +++ b/crates/horae/src/pages/clients/detail.rs @@ -20,7 +20,7 @@ fn ClientDetailContent(id: Uuid) -> Element { let mut details = use_resource(move || async move { // An editor opened under the previous authority must not survive a refresh. show_editor.set(false); - let expected = binding.peek().map(|(requester, _)| requester); + let expected = binding.peek().as_ref().map(|(requester, _)| *requester); let view = server_fns::client_detail_view(id.to_string(), expected).await?; super::bind(binding, (view.requester, view.canonical_permissions))?; Ok::<_, ServerFnError>(view) From a7e5495cb34410c5f8b85a3e757f71e8522e4d48 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:40:25 +0200 Subject: [PATCH 06/16] Remove the unused client summaries endpoint The Clients page reads client_overview, so the legacy summaries endpoint only remained as an unscoped catalog reader. Keep its query for the policy-zero parity tests. --- crates/horae/src/server_fns/clients.rs | 11 ++--------- crates/horae/src/server_fns/clients/page_tests.rs | 7 ------- .../contracts/client-permissions.md | 4 ++-- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/crates/horae/src/server_fns/clients.rs b/crates/horae/src/server_fns/clients.rs index 327feade..97634171 100644 --- a/crates/horae/src/server_fns/clients.rs +++ b/crates/horae/src/server_fns/clients.rs @@ -343,15 +343,8 @@ async fn client_invoices_for_viewer( Ok(invoices) } -/// Project counts and currency filters use the same progress access as Projects. -#[server] -pub async fn list_client_summaries() -> Result, ServerFnError> { - let user = require_user().await?; - let state = crate::state::global_state().await; - client_summaries_for_viewer(&state.db, user.org_id, user.id).await -} - -#[cfg(feature = "server")] +/// Policy-zero summaries, kept for parity tests of the canonical overview. +#[cfg(all(test, feature = "server"))] async fn client_summaries_for_viewer( db: &sqlx::PgPool, org_id: uuid::Uuid, diff --git a/crates/horae/src/server_fns/clients/page_tests.rs b/crates/horae/src/server_fns/clients/page_tests.rs index b84805c1..d462f52a 100644 --- a/crates/horae/src/server_fns/clients/page_tests.rs +++ b/crates/horae/src/server_fns/clients/page_tests.rs @@ -392,11 +392,4 @@ async fn legacy_workflow_readers_are_unchanged_under_policy_one(pool: PgPool) { .unwrap(); assert_eq!(legacy.client.tax_id.as_deref(), Some(TAX_ID)); assert!(legacy.billing.is_some()); - assert_eq!( - client_summaries_for_viewer(&pool, ids.org_id, ids.user_id) - .await - .unwrap() - .len(), - 1 - ); } diff --git a/specs/015-scoped-permissions/contracts/client-permissions.md b/specs/015-scoped-permissions/contracts/client-permissions.md index 6eb4c8ae..84f305b8 100644 --- a/specs/015-scoped-permissions/contracts/client-permissions.md +++ b/specs/015-scoped-permissions/contracts/client-permissions.md @@ -114,8 +114,8 @@ rates under policy one without `ClientReadAll`: | --- | --- | --- | | `list_clients` | All organization clients with address and tax ID, for any active user | Legacy Reports filter, invoice list and invoice detail | | `get_client_details` | Address and tax ID for any active user; default rate for the legacy administrator/manager role | `ProjectsForClient` context, client editor | -| `list_client_summaries` | Full catalog with legacy progress counts | Clients page until it adopts `client_overview` | | `list_client_invoices` | Gated by the legacy manager role, not invoice or client grants | Client detail invoices | Each needs a scoped workflow-identity replacement (OP17/21/25) before -activation. +activation. The former `list_client_summaries` endpoint is removed; the Clients +page reads `client_overview`. From f9823f844628e4200793f691cfeb2409a40f4806 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:53:58 +0200 Subject: [PATCH 07/16] Refresh the SQLx cache for the scoped Clients readers Also drops nine descriptors that no compiled target references. --- ...6a523d64a9d62aa5da7d0b579ec90e9b19126.json | 48 ------- ...f97c92e5a4d233dfa4715226420ab8c33b909.json | 122 ------------------ ...b2ddf66b0f8004ccc26b69089af011dd380aa.json | 50 ------- ...6f8dc56eb74054cd3b77f0de6d7b68d38ffb.json} | 9 +- ...cac11e4941f4da3048cfbdd9485ddcf364e23.json | 99 -------------- ...a24637e8e16dc7c2122dbc1be5972851c43ae.json | 14 ++ ...f5547aa7b8cecca91bb22c51066effe76b7bf.json | 16 +++ ...b97956ba006a265f0ce9ae6c9ee096595d24f.json | 24 ---- ...fbe703129dbd8b1506224ceb829a9f92784d.json} | 9 +- ...74630e630dfd46237e294b30c0e6888cf10c8.json | 20 --- ...586a87ad11aa7ac6bad170b52b60371cbb5d2.json | 48 ------- ...4bb91900af08ae3c43f81add437090941f754.json | 16 --- ...435f9dd95ae6195210a5880e3e284b814b473.json | 56 -------- 13 files changed, 42 insertions(+), 489 deletions(-) delete mode 100644 .sqlx/query-028534bd9ba324fefb979232b5b6a523d64a9d62aa5da7d0b579ec90e9b19126.json delete mode 100644 .sqlx/query-32fc50677ceb32135d0253a1a30f97c92e5a4d233dfa4715226420ab8c33b909.json delete mode 100644 .sqlx/query-3702ee7cf5a0ddf13a8ba74e5bcb2ddf66b0f8004ccc26b69089af011dd380aa.json rename .sqlx/{query-d5bf3533b2ddc547eacee873aaa9fce611662cfa2d962f81483227d09a7f248e.json => query-48a87d8b1c912b6e2bff1e6af66c6f8dc56eb74054cd3b77f0de6d7b68d38ffb.json} (68%) delete mode 100644 .sqlx/query-65e674e65f4c67c5e5216275900cac11e4941f4da3048cfbdd9485ddcf364e23.json create mode 100644 .sqlx/query-741fbefe9d661f97013981e684ba24637e8e16dc7c2122dbc1be5972851c43ae.json create mode 100644 .sqlx/query-89b7e2f75c2a6b9fa115713d4c6f5547aa7b8cecca91bb22c51066effe76b7bf.json delete mode 100644 .sqlx/query-901934d5e79fe984b18d1756169b97956ba006a265f0ce9ae6c9ee096595d24f.json rename .sqlx/{query-7045efef9482fd9a58f8ecde0b48bc710f56afcea528b24ff276cae326a72cf6.json => query-9b208a807c5f7fdb108b873212e9fbe703129dbd8b1506224ceb829a9f92784d.json} (68%) delete mode 100644 .sqlx/query-bf3004fb39949ba0c698368729374630e630dfd46237e294b30c0e6888cf10c8.json delete mode 100644 .sqlx/query-d98c597fa0ff3a77afa1cd1ffe9586a87ad11aa7ac6bad170b52b60371cbb5d2.json delete mode 100644 .sqlx/query-e8b8fc8b20f9b1a37e0644b8ffb4bb91900af08ae3c43f81add437090941f754.json delete mode 100644 .sqlx/query-f0f900fb3ec82bfeb2b6310d8f0435f9dd95ae6195210a5880e3e284b814b473.json diff --git a/.sqlx/query-028534bd9ba324fefb979232b5b6a523d64a9d62aa5da7d0b579ec90e9b19126.json b/.sqlx/query-028534bd9ba324fefb979232b5b6a523d64a9d62aa5da7d0b579ec90e9b19126.json deleted file mode 100644 index b83d3849..00000000 --- a/.sqlx/query-028534bd9ba324fefb979232b5b6a523d64a9d62aa5da7d0b579ec90e9b19126.json +++ /dev/null @@ -1,48 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "UPDATE projects SET client_id = $3, name = $4, code = $5, starts_on = $6, ends_on = $7,\n project_type = $8, rate_cents = $9, budget_kind = $10, budget_amount_cents = $11, budget_minutes = $12\n WHERE id = $1 AND org_id = $2\n AND (client_id, name, code, starts_on, ends_on, project_type, rate_cents, budget_kind, budget_amount_cents, budget_minutes)\n IS DISTINCT FROM ($3, $4, $5, $6, $7, $8, $9, $10, $11, $12)", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Uuid", - "Uuid", - "Uuid", - "Text", - "Text", - "Date", - "Date", - { - "Custom": { - "name": "project_type", - "kind": { - "Enum": [ - "time_and_materials", - "fixed_fee", - "non_billable", - "retainer" - ] - } - } - }, - "Int8", - { - "Custom": { - "name": "budget_kind", - "kind": { - "Enum": [ - "none", - "amount", - "hours" - ] - } - } - }, - "Int8", - "Int8" - ] - }, - "nullable": [] - }, - "hash": "028534bd9ba324fefb979232b5b6a523d64a9d62aa5da7d0b579ec90e9b19126" -} diff --git a/.sqlx/query-32fc50677ceb32135d0253a1a30f97c92e5a4d233dfa4715226420ab8c33b909.json b/.sqlx/query-32fc50677ceb32135d0253a1a30f97c92e5a4d233dfa4715226420ab8c33b909.json deleted file mode 100644 index b06a0403..00000000 --- a/.sqlx/query-32fc50677ceb32135d0253a1a30f97c92e5a4d233dfa4715226420ab8c33b909.json +++ /dev/null @@ -1,122 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "WITH bounded AS MATERIALIZED (\n SELECT p.id, c.name client_name, p.code, p.name, p.project_type,\n p.currency, p.budget_kind, p.budget_amount_cents, p.budget_minutes, p.active\n FROM projects p JOIN clients c ON c.id=p.client_id\n JOIN project_read_access a ON a.project_id=p.id AND a.org_id=p.org_id\n WHERE p.org_id=$1 AND a.user_id=$2 AND a.can_view_progress AND CASE $3\n WHEN 'budgeted' THEN p.active AND p.budget_kind <> 'none'\n WHEN 'archived' THEN NOT p.active ELSE p.active END\n LIMIT $4::bigint + 1\n ), stats AS (\n SELECT COUNT(*) rows,\n COALESCE(SUM(octet_length(client_name)::bigint + COALESCE(octet_length(code),0)\n + octet_length(name) + octet_length(currency)),0)::bigint bytes,\n COALESCE(MAX(GREATEST(octet_length(client_name), COALESCE(octet_length(code),0),\n octet_length(name), octet_length(currency))),0) field_bytes\n FROM bounded\n )\n SELECT s.rows AS \"rows!\", s.bytes AS \"bytes!\", s.field_bytes AS \"field_bytes!\",\n b.id AS \"id?\", b.client_name AS \"client_name?\", b.code, b.name AS \"name?\",\n b.project_type AS \"project_type?: horae_core::types::ProjectType\",\n b.currency AS \"currency?\", b.budget_kind AS \"budget_kind?: horae_core::types::BudgetKind\",\n b.budget_amount_cents, b.budget_minutes, b.active AS \"active?\"\n FROM stats s LEFT JOIN bounded b ON s.rows <= $4 AND s.bytes <= $5 AND s.field_bytes <= $6\n ORDER BY b.client_name, b.name, b.id", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "rows!", - "type_info": "Int8" - }, - { - "ordinal": 1, - "name": "bytes!", - "type_info": "Int8" - }, - { - "ordinal": 2, - "name": "field_bytes!", - "type_info": "Int4" - }, - { - "ordinal": 3, - "name": "id?", - "type_info": "Uuid" - }, - { - "ordinal": 4, - "name": "client_name?", - "type_info": "Text" - }, - { - "ordinal": 5, - "name": "code", - "type_info": "Text" - }, - { - "ordinal": 6, - "name": "name?", - "type_info": "Text" - }, - { - "ordinal": 7, - "name": "project_type?: horae_core::types::ProjectType", - "type_info": { - "Custom": { - "name": "project_type", - "kind": { - "Enum": [ - "time_and_materials", - "fixed_fee", - "non_billable", - "retainer" - ] - } - } - } - }, - { - "ordinal": 8, - "name": "currency?", - "type_info": "Bpchar" - }, - { - "ordinal": 9, - "name": "budget_kind?: horae_core::types::BudgetKind", - "type_info": { - "Custom": { - "name": "budget_kind", - "kind": { - "Enum": [ - "none", - "amount", - "hours" - ] - } - } - } - }, - { - "ordinal": 10, - "name": "budget_amount_cents", - "type_info": "Int8" - }, - { - "ordinal": 11, - "name": "budget_minutes", - "type_info": "Int8" - }, - { - "ordinal": 12, - "name": "active?", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Uuid", - "Uuid", - "Text", - "Int8", - "Int8", - "Int4" - ] - }, - "nullable": [ - null, - null, - null, - false, - false, - true, - false, - false, - false, - false, - true, - true, - false - ] - }, - "hash": "32fc50677ceb32135d0253a1a30f97c92e5a4d233dfa4715226420ab8c33b909" -} diff --git a/.sqlx/query-3702ee7cf5a0ddf13a8ba74e5bcb2ddf66b0f8004ccc26b69089af011dd380aa.json b/.sqlx/query-3702ee7cf5a0ddf13a8ba74e5bcb2ddf66b0f8004ccc26b69089af011dd380aa.json deleted file mode 100644 index 24456249..00000000 --- a/.sqlx/query-3702ee7cf5a0ddf13a8ba74e5bcb2ddf66b0f8004ccc26b69089af011dd380aa.json +++ /dev/null @@ -1,50 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT t.id, t.name, t.active, t.billable_default,\n CASE WHEN access.can_view_rates THEN t.default_rate_cents END AS default_rate_cents\n FROM tasks t JOIN task_read_access access ON access.task_id = t.id AND access.org_id = t.org_id\n WHERE t.org_id = $1 AND access.user_id = $5\n AND ($2::bool IS NULL OR t.active = $2)\n ORDER BY t.name, t.id\n LIMIT $3 OFFSET $4", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Uuid" - }, - { - "ordinal": 1, - "name": "name", - "type_info": "Text" - }, - { - "ordinal": 2, - "name": "active", - "type_info": "Bool" - }, - { - "ordinal": 3, - "name": "billable_default", - "type_info": "Bool" - }, - { - "ordinal": 4, - "name": "default_rate_cents", - "type_info": "Int8" - } - ], - "parameters": { - "Left": [ - "Uuid", - "Bool", - "Int8", - "Int8", - "Uuid" - ] - }, - "nullable": [ - false, - false, - false, - false, - null - ] - }, - "hash": "3702ee7cf5a0ddf13a8ba74e5bcb2ddf66b0f8004ccc26b69089af011dd380aa" -} diff --git a/.sqlx/query-d5bf3533b2ddc547eacee873aaa9fce611662cfa2d962f81483227d09a7f248e.json b/.sqlx/query-48a87d8b1c912b6e2bff1e6af66c6f8dc56eb74054cd3b77f0de6d7b68d38ffb.json similarity index 68% rename from .sqlx/query-d5bf3533b2ddc547eacee873aaa9fce611662cfa2d962f81483227d09a7f248e.json rename to .sqlx/query-48a87d8b1c912b6e2bff1e6af66c6f8dc56eb74054cd3b77f0de6d7b68d38ffb.json index 30d34366..bf0ad0dd 100644 --- a/.sqlx/query-d5bf3533b2ddc547eacee873aaa9fce611662cfa2d962f81483227d09a7f248e.json +++ b/.sqlx/query-48a87d8b1c912b6e2bff1e6af66c6f8dc56eb74054cd3b77f0de6d7b68d38ffb.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT c.id, c.org_id, c.name, c.currency, c.address, c.tax_id, c.active,\n c.created_at as \"created_at: chrono::DateTime\",\n COUNT(p.id) FILTER (WHERE p.active) as \"active_projects!\",\n COUNT(p.id) as \"total_projects!\",\n COALESCE(ARRAY_AGG(DISTINCT p.currency ORDER BY p.currency)\n FILTER (WHERE p.id IS NOT NULL), ARRAY[]::text[]) as \"project_currencies!\"\n FROM clients c\n JOIN users u ON u.org_id = c.org_id AND u.id = $2 AND u.active\n LEFT JOIN projects p ON p.client_id = c.id AND p.org_id = c.org_id\n AND EXISTS (\n SELECT 1 FROM project_read_access a\n WHERE a.project_id = p.id AND a.org_id = c.org_id\n AND a.user_id = u.id AND a.can_view_progress\n )\n WHERE c.org_id = $1\n GROUP BY c.id\n ORDER BY c.name, c.id", + "query": "SELECT c.id, c.org_id, c.name, c.currency, c.address, c.tax_id, c.active,\n c.created_at as \"created_at: chrono::DateTime\",\n COUNT(p.id) FILTER (WHERE p.active) as \"active_projects!\",\n COUNT(p.id) as \"total_projects!\",\n COALESCE(ARRAY_AGG(DISTINCT p.currency ORDER BY p.currency)\n FILTER (WHERE p.id IS NOT NULL), ARRAY[]::text[]) as \"project_currencies!\"\n FROM clients c\n JOIN users u ON u.org_id = c.org_id AND u.id = $2 AND u.active\n LEFT JOIN projects p ON p.client_id = c.id AND p.org_id = c.org_id\n AND CASE WHEN $3 THEN EXISTS (\n SELECT 1 FROM project_read_access a\n WHERE a.project_id = p.id AND a.org_id = c.org_id\n AND a.user_id = u.id AND a.can_view_progress\n ) ELSE $4 OR ($5 AND EXISTS (\n SELECT 1 FROM project_management_assignments m\n WHERE m.org_id = p.org_id AND m.project_id = p.id AND m.manager_id = u.id\n )) OR (EXISTS (\n SELECT 1 FROM assignments member\n WHERE member.project_id = p.id AND member.user_id = u.id\n ) AND COALESCE((\n SELECT s.report_visibility FROM project_settings s\n WHERE s.project_id = p.id AND s.org_id = p.org_id\n ), 'project_members') = 'project_members') END\n WHERE c.org_id = $1\n GROUP BY c.id\n ORDER BY c.name, c.id", "describe": { "columns": [ { @@ -62,7 +62,10 @@ "parameters": { "Left": [ "Uuid", - "Uuid" + "Uuid", + "Bool", + "Bool", + "Bool" ] }, "nullable": [ @@ -79,5 +82,5 @@ null ] }, - "hash": "d5bf3533b2ddc547eacee873aaa9fce611662cfa2d962f81483227d09a7f248e" + "hash": "48a87d8b1c912b6e2bff1e6af66c6f8dc56eb74054cd3b77f0de6d7b68d38ffb" } diff --git a/.sqlx/query-65e674e65f4c67c5e5216275900cac11e4941f4da3048cfbdd9485ddcf364e23.json b/.sqlx/query-65e674e65f4c67c5e5216275900cac11e4941f4da3048cfbdd9485ddcf364e23.json deleted file mode 100644 index 97b3c2da..00000000 --- a/.sqlx/query-65e674e65f4c67c5e5216275900cac11e4941f4da3048cfbdd9485ddcf364e23.json +++ /dev/null @@ -1,99 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT id AS \"id!\",client_name AS \"client_name!\",code,name AS \"name!\",\n project_type AS \"project_type!: horae_core::types::ProjectType\",currency AS \"currency!\",\n budget_kind AS \"budget_kind!: horae_core::types::BudgetKind\",budget_amount_cents,\n budget_minutes,active AS \"active!\"\n FROM fetch_csv_export_rows($1) AS source(id uuid,client_name text,code text,name text,\n project_type project_type,currency text,budget_kind budget_kind,budget_amount_cents bigint,\n budget_minutes bigint,active boolean,export_bytes bigint)", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id!", - "type_info": "Uuid" - }, - { - "ordinal": 1, - "name": "client_name!", - "type_info": "Text" - }, - { - "ordinal": 2, - "name": "code", - "type_info": "Text" - }, - { - "ordinal": 3, - "name": "name!", - "type_info": "Text" - }, - { - "ordinal": 4, - "name": "project_type!: horae_core::types::ProjectType", - "type_info": { - "Custom": { - "name": "project_type", - "kind": { - "Enum": [ - "time_and_materials", - "fixed_fee", - "non_billable", - "retainer" - ] - } - } - } - }, - { - "ordinal": 5, - "name": "currency!", - "type_info": "Text" - }, - { - "ordinal": 6, - "name": "budget_kind!: horae_core::types::BudgetKind", - "type_info": { - "Custom": { - "name": "budget_kind", - "kind": { - "Enum": [ - "none", - "amount", - "hours" - ] - } - } - } - }, - { - "ordinal": 7, - "name": "budget_amount_cents", - "type_info": "Int8" - }, - { - "ordinal": 8, - "name": "budget_minutes", - "type_info": "Int8" - }, - { - "ordinal": 9, - "name": "active!", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Int4" - ] - }, - "nullable": [ - null, - null, - null, - null, - null, - null, - null, - null, - null, - null - ] - }, - "hash": "65e674e65f4c67c5e5216275900cac11e4941f4da3048cfbdd9485ddcf364e23" -} diff --git a/.sqlx/query-741fbefe9d661f97013981e684ba24637e8e16dc7c2122dbc1be5972851c43ae.json b/.sqlx/query-741fbefe9d661f97013981e684ba24637e8e16dc7c2122dbc1be5972851c43ae.json new file mode 100644 index 00000000..06926e40 --- /dev/null +++ b/.sqlx/query-741fbefe9d661f97013981e684ba24637e8e16dc7c2122dbc1be5972851c43ae.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE clients SET default_rate_cents=12345 WHERE id=$1", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Uuid" + ] + }, + "nullable": [] + }, + "hash": "741fbefe9d661f97013981e684ba24637e8e16dc7c2122dbc1be5972851c43ae" +} diff --git a/.sqlx/query-89b7e2f75c2a6b9fa115713d4c6f5547aa7b8cecca91bb22c51066effe76b7bf.json b/.sqlx/query-89b7e2f75c2a6b9fa115713d4c6f5547aa7b8cecca91bb22c51066effe76b7bf.json new file mode 100644 index 00000000..d1124e7e --- /dev/null +++ b/.sqlx/query-89b7e2f75c2a6b9fa115713d4c6f5547aa7b8cecca91bb22c51066effe76b7bf.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE clients SET address=$2, tax_id=$3, default_rate_cents=12345 WHERE id=$1", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Uuid", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "89b7e2f75c2a6b9fa115713d4c6f5547aa7b8cecca91bb22c51066effe76b7bf" +} diff --git a/.sqlx/query-901934d5e79fe984b18d1756169b97956ba006a265f0ce9ae6c9ee096595d24f.json b/.sqlx/query-901934d5e79fe984b18d1756169b97956ba006a265f0ce9ae6c9ee096595d24f.json deleted file mode 100644 index 9908fe61..00000000 --- a/.sqlx/query-901934d5e79fe984b18d1756169b97956ba006a265f0ce9ae6c9ee096595d24f.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT COUNT(*) AS \"count!\" FROM project_read_access\n WHERE org_id=$1 AND user_id=$2 AND project_id=ANY($3) AND can_view_progress", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "count!", - "type_info": "Int8" - } - ], - "parameters": { - "Left": [ - "Uuid", - "Uuid", - "UuidArray" - ] - }, - "nullable": [ - null - ] - }, - "hash": "901934d5e79fe984b18d1756169b97956ba006a265f0ce9ae6c9ee096595d24f" -} diff --git a/.sqlx/query-7045efef9482fd9a58f8ecde0b48bc710f56afcea528b24ff276cae326a72cf6.json b/.sqlx/query-9b208a807c5f7fdb108b873212e9fbe703129dbd8b1506224ceb829a9f92784d.json similarity index 68% rename from .sqlx/query-7045efef9482fd9a58f8ecde0b48bc710f56afcea528b24ff276cae326a72cf6.json rename to .sqlx/query-9b208a807c5f7fdb108b873212e9fbe703129dbd8b1506224ceb829a9f92784d.json index 4b0523f4..351ab18b 100644 --- a/.sqlx/query-7045efef9482fd9a58f8ecde0b48bc710f56afcea528b24ff276cae326a72cf6.json +++ b/.sqlx/query-9b208a807c5f7fdb108b873212e9fbe703129dbd8b1506224ceb829a9f92784d.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT c.id, c.org_id, c.name, c.currency, c.address, c.tax_id, c.active,\n c.created_at as \"created_at: chrono::DateTime\",\n u.org_role IN ('admin', 'manager') as \"can_view_billing!\",\n CASE WHEN u.org_role IN ('admin', 'manager')\n THEN c.default_rate_cents END as default_rate_cents\n FROM clients c\n JOIN users u ON u.org_id = c.org_id AND u.id = $2 AND u.active\n WHERE c.org_id = $1 AND c.id = $3", + "query": "SELECT c.id, c.org_id, c.name, c.currency, c.address, c.tax_id, c.active,\n c.created_at as \"created_at: chrono::DateTime\",\n (CASE WHEN $4 THEN u.org_role IN ('admin', 'manager') ELSE $5 AND $6 END)\n as \"can_view_billing!\",\n CASE WHEN (CASE WHEN $4 THEN u.org_role IN ('admin', 'manager') ELSE $5 AND $6 END)\n THEN c.default_rate_cents END as default_rate_cents\n FROM clients c\n JOIN users u ON u.org_id = c.org_id AND u.id = $2 AND u.active\n WHERE c.org_id = $1 AND c.id = $3 AND ($4 OR $5)", "describe": { "columns": [ { @@ -58,7 +58,10 @@ "Left": [ "Uuid", "Uuid", - "Uuid" + "Uuid", + "Bool", + "Bool", + "Bool" ] }, "nullable": [ @@ -74,5 +77,5 @@ null ] }, - "hash": "7045efef9482fd9a58f8ecde0b48bc710f56afcea528b24ff276cae326a72cf6" + "hash": "9b208a807c5f7fdb108b873212e9fbe703129dbd8b1506224ceb829a9f92784d" } diff --git a/.sqlx/query-bf3004fb39949ba0c698368729374630e630dfd46237e294b30c0e6888cf10c8.json b/.sqlx/query-bf3004fb39949ba0c698368729374630e630dfd46237e294b30c0e6888cf10c8.json deleted file mode 100644 index 67a7a6ad..00000000 --- a/.sqlx/query-bf3004fb39949ba0c698368729374630e630dfd46237e294b30c0e6888cf10c8.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO project_task_settings (id,org_id,project_id,task_id,restricted,budget_minutes,budget_cents) VALUES ($1,$2,$3,$4,$5,$6,$7)\n ON CONFLICT (project_id,task_id) DO UPDATE SET restricted=EXCLUDED.restricted, budget_minutes=EXCLUDED.budget_minutes, budget_cents=EXCLUDED.budget_cents", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Uuid", - "Uuid", - "Uuid", - "Uuid", - "Bool", - "Int8", - "Int8" - ] - }, - "nullable": [] - }, - "hash": "bf3004fb39949ba0c698368729374630e630dfd46237e294b30c0e6888cf10c8" -} diff --git a/.sqlx/query-d98c597fa0ff3a77afa1cd1ffe9586a87ad11aa7ac6bad170b52b60371cbb5d2.json b/.sqlx/query-d98c597fa0ff3a77afa1cd1ffe9586a87ad11aa7ac6bad170b52b60371cbb5d2.json deleted file mode 100644 index ed7f4132..00000000 --- a/.sqlx/query-d98c597fa0ff3a77afa1cd1ffe9586a87ad11aa7ac6bad170b52b60371cbb5d2.json +++ /dev/null @@ -1,48 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT t.id, t.name, t.active, t.billable_default,\n CASE WHEN access.can_view_rates THEN t.default_rate_cents END AS default_rate_cents\n FROM tasks t JOIN task_read_access access ON access.task_id = t.id AND access.org_id = t.org_id\n WHERE t.id = $1 AND t.org_id = $2 AND access.user_id = $3", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Uuid" - }, - { - "ordinal": 1, - "name": "name", - "type_info": "Text" - }, - { - "ordinal": 2, - "name": "active", - "type_info": "Bool" - }, - { - "ordinal": 3, - "name": "billable_default", - "type_info": "Bool" - }, - { - "ordinal": 4, - "name": "default_rate_cents", - "type_info": "Int8" - } - ], - "parameters": { - "Left": [ - "Uuid", - "Uuid", - "Uuid" - ] - }, - "nullable": [ - false, - false, - false, - false, - null - ] - }, - "hash": "d98c597fa0ff3a77afa1cd1ffe9586a87ad11aa7ac6bad170b52b60371cbb5d2" -} diff --git a/.sqlx/query-e8b8fc8b20f9b1a37e0644b8ffb4bb91900af08ae3c43f81add437090941f754.json b/.sqlx/query-e8b8fc8b20f9b1a37e0644b8ffb4bb91900af08ae3c43f81add437090941f754.json deleted file mode 100644 index e65f183c..00000000 --- a/.sqlx/query-e8b8fc8b20f9b1a37e0644b8ffb4bb91900af08ae3c43f81add437090941f754.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "DECLARE horae_csv NO SCROLL CURSOR FOR\n SELECT p.id,c.name,p.code,p.name,p.project_type,p.currency::text,\n p.budget_kind,p.budget_amount_cents,p.budget_minutes,p.active,\n 128::bigint + octet_length(c.name)::bigint + COALESCE(octet_length(p.code),0)::bigint\n + octet_length(p.name)::bigint + octet_length(p.currency::text)::bigint AS export_bytes\n FROM projects p JOIN clients c ON c.id=p.client_id\n JOIN project_read_access a ON a.project_id=p.id AND a.org_id=p.org_id\n WHERE p.org_id=$1 AND a.user_id=$2 AND a.can_view_progress AND CASE $3\n WHEN 'budgeted' THEN p.active AND p.budget_kind <> 'none'\n WHEN 'archived' THEN NOT p.active ELSE p.active END\n ORDER BY c.name,p.name,p.id", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Uuid", - "Uuid", - "Text" - ] - }, - "nullable": [] - }, - "hash": "e8b8fc8b20f9b1a37e0644b8ffb4bb91900af08ae3c43f81add437090941f754" -} diff --git a/.sqlx/query-f0f900fb3ec82bfeb2b6310d8f0435f9dd95ae6195210a5880e3e284b814b473.json b/.sqlx/query-f0f900fb3ec82bfeb2b6310d8f0435f9dd95ae6195210a5880e3e284b814b473.json deleted file mode 100644 index 4dd1eeaa..00000000 --- a/.sqlx/query-f0f900fb3ec82bfeb2b6310d8f0435f9dd95ae6195210a5880e3e284b814b473.json +++ /dev/null @@ -1,56 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "UPDATE tasks\n SET name = $3, billable_default = $4, default_rate_cents = $5,\n default_rate_currency = CASE\n WHEN default_rate_cents IS NOT DISTINCT FROM $5 THEN default_rate_currency\n WHEN $5::bigint IS NULL THEN NULL\n ELSE (SELECT upper(btrim(default_currency)) FROM organizations WHERE id = $2) END\n WHERE id = $1 AND org_id = $2\n AND (name, billable_default, default_rate_cents) IS DISTINCT FROM ($3, $4, $5)\n RETURNING id, org_id, name, billable_default, default_rate_cents, active", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Uuid" - }, - { - "ordinal": 1, - "name": "org_id", - "type_info": "Uuid" - }, - { - "ordinal": 2, - "name": "name", - "type_info": "Text" - }, - { - "ordinal": 3, - "name": "billable_default", - "type_info": "Bool" - }, - { - "ordinal": 4, - "name": "default_rate_cents", - "type_info": "Int8" - }, - { - "ordinal": 5, - "name": "active", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Uuid", - "Uuid", - "Text", - "Bool", - "Int8" - ] - }, - "nullable": [ - false, - false, - false, - false, - true, - false - ] - }, - "hash": "f0f900fb3ec82bfeb2b6310d8f0435f9dd95ae6195210a5880e3e284b814b473" -} From 01eefb777a989c0e683c138f7f19514c688bd105 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:57:15 +0200 Subject: [PATCH 08/16] Drop the unused is_manager navigation test double Clients now gate actions on server-reported affordances, so no page in the navigation harness calls the legacy role helper. --- crates/horae/tests/detail_navigation.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/horae/tests/detail_navigation.rs b/crates/horae/tests/detail_navigation.rs index 87ef59aa..db27de1a 100644 --- a/crates/horae/tests/detail_navigation.rs +++ b/crates/horae/tests/detail_navigation.rs @@ -819,10 +819,6 @@ fn is_admin(me: &Resource>) -> bool { matches!(&*me.read(), Some(Ok(user)) if user.is_admin()) } -fn is_manager(me: &Resource>) -> bool { - matches!(&*me.read(), Some(Ok(user)) if user.is_manager_or_above()) -} - fn loaded( state: &Option>, render: impl FnOnce(&T) -> Element, From 03bd422f5225a59d792036b3a2a9cf4bbaa4e8f9 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:00:56 +0200 Subject: [PATCH 09/16] Format the Clients authorization test --- .../authorization_tests/client_page.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/crates/horae/src/server_fns/importers/authorization_tests/client_page.rs b/crates/horae/src/server_fns/importers/authorization_tests/client_page.rs index 84e78c13..82502f7f 100644 --- a/crates/horae/src/server_fns/importers/authorization_tests/client_page.rs +++ b/crates/horae/src/server_fns/importers/authorization_tests/client_page.rs @@ -37,7 +37,11 @@ pub(super) async fn check(pool: &PgPool, api: &Api) { } let page = api - .json("client_overview", json!({"expected_requester": null}), &cookie) + .json( + "client_overview", + json!({"expected_requester": null}), + &cookie, + ) .await; let requester = json!({"org_id": ids.org_id, "user_id": ids.user_id}); assert_eq!(page["requester"], requester); @@ -78,9 +82,14 @@ pub(super) async fn check(pool: &PgPool, api: &Api) { assert_eq!(withheld["can_read"], false); assert_eq!(withheld["clients"], json!([])); assert_eq!( - api.call("client_detail_view", unbound_detail, Some(&restricted_cookie), false) - .await - .status(), + api.call( + "client_detail_view", + unbound_detail, + Some(&restricted_cookie), + false + ) + .await + .status(), StatusCode::NOT_FOUND ); } From af22e2fcfbf779f690c8f7269074b212d8940279 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:24:42 +0200 Subject: [PATCH 10/16] Import ClientSummary only for the server build --- crates/horae/src/server_fns/clients.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/horae/src/server_fns/clients.rs b/crates/horae/src/server_fns/clients.rs index 97634171..52530be6 100644 --- a/crates/horae/src/server_fns/clients.rs +++ b/crates/horae/src/server_fns/clients.rs @@ -5,9 +5,10 @@ use super::*; use crate::models::client::ClientBilling; #[cfg(feature = "server")] use crate::models::client::ClientRateChange; +#[cfg(feature = "server")] +use crate::models::client::ClientSummary; use crate::models::client::{ ClientDetailView, ClientDetails, ClientOverview, ClientProfile, ClientProfileEdit, - ClientSummary, }; use crate::models::permission_editor::PermissionRequester; From 5a6e6486bd7906c33795d6956185ebe1bfd1b228 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:18:55 +0200 Subject: [PATCH 11/16] Return focus to the client editor's opener after a save reload Saving a client restarts the bound Clients reader, which unmounts the New client or Edit button while the dialog closes, so the native dialog could only fall back to the page heading. Once the reload settles with management still offered, focus the rebuilt opener. The request is one-shot: a failed, denied or rebound reload consumes it without moving focus, and focus the user has already moved is left alone. --- crates/horae/src/pages/clients.rs | 38 +++++++++++++++++++++++- crates/horae/src/pages/clients/detail.rs | 18 +++++++++-- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/crates/horae/src/pages/clients.rs b/crates/horae/src/pages/clients.rs index edd1f7ed..6e65b514 100644 --- a/crates/horae/src/pages/clients.rs +++ b/crates/horae/src/pages/clients.rs @@ -36,6 +36,22 @@ fn bind(mut binding: Signal>, current: Binding) -> Result<(), Se Ok(()) } +/// A save reloads the page, unmounting the editor's opener before the dialog +/// can return focus to it. Focus the rebuilt opener, unless the user has +/// already moved focus beyond the dialog's fallback during the reload. +fn restore_opener_focus(opener: &str, fallback: &str) { + let opener = serde_json::json!(opener); + let fallback = serde_json::json!(fallback); + document::eval(&format!( + "requestAnimationFrame(() => {{ + const active = document.activeElement; + if (!active || active === document.body || active.id === {fallback}) {{ + document.getElementById({opener})?.focus(); + }} + }});" + )); +} + #[component] pub fn ClientList() -> Element { // Management view: include inactive clients so managers can reactivate them. @@ -47,6 +63,8 @@ pub fn ClientList() -> Element { // `Some(id)` while editing an existing client, `None` while creating. let mut editing_id = use_signal(|| None::); let action_error = use_signal(|| None::); + // The opener to refocus after a save's reload; see `restore_opener_focus`. + let mut refocus = use_signal(|| None::); let binding = use_signal(|| None::); let mut clients = use_resource(move || async move { @@ -62,6 +80,17 @@ pub fn ClientList() -> Element { Ok::<_, ServerFnError>(overview) }); let me = use_resource(|| async move { server_fns::get_me().await }); + use_effect(move || { + if clients.state()() != UseResourceState::Ready { + return; + } + // One-shot: a failed, denied or rebound reload consumes it without focusing. + if let Some(opener) = refocus.take() { + if matches!(&*clients.peek(), Some(Ok(overview)) if overview.can_manage) { + restore_opener_focus(&opener, "clients-heading"); + } + } + }); let ready = clients.state()() == UseResourceState::Ready; let data = clients.read(); @@ -166,7 +195,14 @@ pub fn ClientList() -> Element { form::ClientEditor { open: show_form() && is_manager, client_id: editing_id(), focus_fallback: "clients-heading", on_dismiss: move |_| reset_form(), - on_saved: move |_| { reset_form(); clients.restart(); }, + on_saved: move |_| { + refocus.set(Some(match *editing_id.peek() { + Some(id) => format!("client-edit-{id}"), + None => "clients-new".to_owned(), + })); + reset_form(); + clients.restart(); + }, } if let Some(message) = action_error() { diff --git a/crates/horae/src/pages/clients/detail.rs b/crates/horae/src/pages/clients/detail.rs index 950e3525..1fdc2c2a 100644 --- a/crates/horae/src/pages/clients/detail.rs +++ b/crates/horae/src/pages/clients/detail.rs @@ -25,6 +25,16 @@ fn ClientDetailContent(id: Uuid) -> Element { super::bind(binding, (view.requester, view.canonical_permissions))?; Ok::<_, ServerFnError>(view) }); + let mut refocus_edit = use_signal(|| false); + use_effect(move || { + if details.state()() != UseResourceState::Ready { + return; + } + // One-shot: a failed, denied or rebound reload consumes it without focusing. + if refocus_edit.take() && matches!(&*details.peek(), Some(Ok(view)) if view.can_manage) { + super::restore_opener_focus("client-edit", "client-back"); + } + }); let ready = details.state()() == UseResourceState::Ready; let data = details.read(); // Legacy-only until client writes and invoices use canonical permissions; @@ -62,7 +72,7 @@ fn ClientDetailContent(id: Uuid) -> Element { Link { to: Route::NewProjectForClient { client: id.to_string() }, class: "btn btn-secondary", "New project" } Link { to: Route::NewInvoiceForClient { client: id.to_string() }, class: "btn btn-primary", "New invoice" } } - button { r#type: "button", class: "btn btn-secondary", onclick: move |_| show_editor.set(true), "Edit client" } + button { id: "client-edit", r#type: "button", class: "btn btn-secondary", onclick: move |_| show_editor.set(true), "Edit client" } } } } @@ -105,7 +115,11 @@ fn ClientDetailContent(id: Uuid) -> Element { } super::form::ClientEditor { open: show_editor() && can_manage, client_id: Some(id), focus_fallback: "client-back", on_dismiss: move |_| show_editor.set(false), - on_saved: move |_| { show_editor.set(false); details.restart(); }, + on_saved: move |_| { + refocus_edit.set(true); + show_editor.set(false); + details.restart(); + }, } } } From 1531cb8dc63178620b561401c0ace6ec84c99ade Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:18:55 +0200 Subject: [PATCH 12/16] Check focus returns to the opener after successful client edits --- crates/horae/tests/browser/clients.cjs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/horae/tests/browser/clients.cjs b/crates/horae/tests/browser/clients.cjs index f837a1b2..82c11e0b 100644 --- a/crates/horae/tests/browser/clients.cjs +++ b/crates/horae/tests/browser/clients.cjs @@ -52,6 +52,13 @@ const sql = query => execFileSync('psql', [process.env.DATABASE_URL, '-X', '-v', await page.getByRole('searchbox', { name: 'Search clients by name' }).fill(name); const link = page.getByRole('link', { name, exact: true }); await expect(link).toHaveCount(1); + const rowEdit = page.getByRole('row').filter({ has: link }).getByRole('button', { name: 'Edit', exact: true }); + await rowEdit.click(); + await expect(field('Tax ID (optional)')).toHaveValue('TEST-VAT'); + await field('Tax ID (optional)').fill('TEST-VAT-2'); + await dialog.getByRole('button', { name: 'Save changes', exact: true }).click(); + await expect(dialog).toBeHidden(); + await expect(rowEdit).toBeFocused(); const detailPath = await link.getAttribute('href'); assert.match(detailPath, /^\/clients\/[0-9a-f-]+$/); await page.reload(); @@ -76,6 +83,7 @@ const sql = query => execFileSync('psql', [process.env.DATABASE_URL, '-X', '-v', await dialog.getByRole('button', { name: 'Use this rate in USD', exact: true }).click(); await dialog.getByRole('button', { name: 'Save changes', exact: true }).click(); await expect(dialog).toBeHidden(); + await expect(edit).toBeFocused(); await expect(page.getByRole('region', { name: 'Billing', exact: true })).toContainText('USD 0.00 / h'); await page.reload(); await expect(page.getByRole('region', { name: 'Billing', exact: true })).toContainText('USD 0.00 / h'); From b3c0fc7a0b4953e663a4b009e3ce808db6c407c0 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:18:56 +0200 Subject: [PATCH 13/16] Cover canonical Clients reads and refresh fences in the browser Exercise ClientReadAll without management for a legacy Administrator, managed versus global default-rate visibility with unset and zero rates, denied versus empty catalogs, unknown policy, and account or policy changes during a reload, on a disposable database only. --- .../tests/browser/clients-permissions.cjs | 202 ++++++++++++++++++ .../horae/tests/browser/run-design-checks.sh | 2 +- 2 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 crates/horae/tests/browser/clients-permissions.cjs diff --git a/crates/horae/tests/browser/clients-permissions.cjs b/crates/horae/tests/browser/clients-permissions.cjs new file mode 100644 index 00000000..c9892715 --- /dev/null +++ b/crates/horae/tests/browser/clients-permissions.cjs @@ -0,0 +1,202 @@ +// Canonical Clients reads and refresh fences use only the design runner's disposable database. +const { chromium, expect: baseExpect } = require(process.env.PLAYWRIGHT_MODULE || 'playwright/test'); +const expect = baseExpect.configure({ timeout: 15000 }); +const { execFileSync } = require('node:child_process'); +const assert = require('node:assert/strict'); +const base = process.env.HORAE_TEST_URL; +const target = new URL(base), database = new URL(process.env.DATABASE_URL); +assert.ok(['localhost', '127.0.0.1'].includes(target.hostname) && target.port === '8093'); +assert.equal(database.pathname, '/horae'); +assert.match(database.searchParams.get('host') || '', /^\/tmp\/horae-browser\.[A-Za-z0-9]+$/); +const sql = query => execFileSync('psql', [process.env.DATABASE_URL, '-X', '-v', 'ON_ERROR_STOP=1', '-qAt', '-c', query], { encoding: 'utf8' }).trim(); +const output = process.env.HORAE_TEST_SCREENSHOT_DIR; +if (output) require('node:fs').mkdirSync(output, { recursive: true }); +const id = n => `019f4200-0000-7000-8000-${String(n).padStart(12, '0')}`; +const unset = id(1), zero = id(2), project = id(3), state = id(4), assignment = id(5), stranger = id(6); +const floor = ['time_read_own', 'time_write_own', 'expense_read_own', 'expense_write_own']; +const catalog = [...floor, 'client_read_all']; +const grantsSql = grants => `ARRAY[${grants.map(grant => `'${grant}'`).join(',')}]`; +const setGrants = grants => sql(`UPDATE person_permission_states SET grants=${grantsSql(grants)} WHERE id='${state}'`); +let actor, org, owned = false; +const setPolicy = version => sql(`UPDATE organizations SET permission_policy_version=${version} WHERE id='${org}'`); + +(async () => { + const browser = await chromium.launch({ executablePath: process.env.CHROMIUM_PATH || undefined }); + const context = await browser.newContext({ viewport: { width: 1440, height: 900 } }); + const page = await context.newPage(); + const errors = []; + page.on('pageerror', error => errors.push(error.stack || error.message)); + const dialog = page.getByRole('dialog'); + const sidebarClients = page.locator('.app-sidebar').getByRole('link', { name: 'Clients', exact: true }); + const billing = page.getByRole('region', { name: 'Billing', exact: true }); + // Focus restoration runs on the next animation frame after a reload settles. + const activeId = () => page.evaluate(() => new Promise(resolve => + requestAnimationFrame(() => requestAnimationFrame(() => resolve(document.activeElement?.id || ''))))); + const capture = async name => { + if (output) await page.screenshot({ path: `${output}/clients-permissions-${name}.png` }); + }; + const loadList = async () => { + const response = page.waitForResponse(r => r.url().includes('/api/client_overview')); + await page.goto(`${base}/clients`); + return (await response).json(); + }; + const loadDetail = async client => { + const response = page.waitForResponse(r => r.url().includes('/api/client_detail_view')); + await page.goto(`${base}/clients/${client}`); + return response; + }; + const editTaxId = async (opener, value) => { + await opener.click(); + await expect(dialog.getByLabel('Client name', { exact: true })).toBeFocused(); + await dialog.getByLabel('Tax ID (optional)', { exact: true }).fill(value); + await dialog.getByRole('button', { name: 'Save changes', exact: true }).click(); + await expect(dialog).toBeHidden(); + }; + try { + const login = await context.request.post(`${base}/auth/dev-login`, { maxRedirects: 0 }); + assert.equal(login.status(), 303); + // Dev login can select another admin created by a preceding suite. + const me = page.waitForResponse(r => r.url().includes('/api/get_me') && r.status() === 200); + await page.goto(`${base}/clients`); + actor = await (await me).json(); + org = actor.org_id; + assert.equal(actor.org_role, 'admin'); + // Only this suite's rows are created and removed; the policy is restored to its observed value. + assert.equal(sql(`SELECT permission_policy_version FROM organizations WHERE id='${org}'`), '0'); + assert.equal(sql(`SELECT count(*) FROM person_permission_states WHERE user_id='${actor.id}'`), '0'); + assert.equal(sql(`SELECT count(*) FROM clients WHERE id IN ('${unset}','${zero}')`), '0'); + owned = true; + sql(`BEGIN; + INSERT INTO clients (id,org_id,name,currency) VALUES ('${unset}','${org}','Permission unset client','EUR'); + INSERT INTO clients (id,org_id,name,currency,default_rate_cents) VALUES ('${zero}','${org}','Permission zero client','EUR',0); + INSERT INTO projects (id,org_id,client_id,name,currency) VALUES ('${project}','${org}','${unset}','Permission managed project','EUR'); + INSERT INTO project_management_assignments (id,org_id,project_id,manager_id) VALUES ('${assignment}','${org}','${project}','${actor.id}'); + COMMIT;`); + + // Policy zero: a save's reload that fails, is rebound or changes policy + // consumes the opener refocus instead of moving focus later. + await loadList(); + await page.getByRole('searchbox', { name: 'Search clients by name' }).fill('Permission unset client'); + const rowEdit = page.locator(`#client-edit-${unset}`); + await page.route('**/api/client_overview*', route => route.abort('failed'), { times: 1 }); + await editTaxId(rowEdit, 'PERM-1'); + await expect(page.getByRole('alert')).toContainText('Could not load clients'); + await expect(page.locator('#clients-heading')).toBeFocused(); + await page.getByRole('button', { name: 'Retry clients', exact: true }).click(); + await expect(rowEdit).toBeVisible(); + assert.notEqual(await activeId(), `client-edit-${unset}`); + + let refresh; + await page.route('**/api/client_overview*', async route => { + refresh = route.request().postDataJSON(); + const response = await route.fetch(); + const body = await response.json(); + await route.fulfill({ response, json: { ...body, requester: { ...body.requester, user_id: stranger } } }); + }, { times: 1 }); + await editTaxId(rowEdit, 'PERM-2'); + await expect(page.getByRole('alert')).toContainText('Could not load clients'); + assert.deepEqual(refresh.expected_requester, { org_id: org, user_id: actor.id }, 'the reload asks the server to check the bound account'); + await expect(page.getByRole('button', { name: 'New client', exact: true })).toHaveCount(0); + await page.getByRole('button', { name: 'Retry clients', exact: true }).click(); + await expect(rowEdit).toBeVisible(); + assert.notEqual(await activeId(), `client-edit-${unset}`); + await editTaxId(rowEdit, 'PERM-3'); + await expect(rowEdit).toBeFocused(); + console.log('PASS: list refocus is one-shot across failed and rebound reloads; a successful edit returns to its row'); + + await loadDetail(unset); + const edit = page.locator('#client-edit'); + await page.route('**/api/client_detail_view*', async route => { + sql(`BEGIN; + INSERT INTO person_permission_states (id,org_id,user_id,catalog_version,grants,is_administrator,source) + VALUES ('${state}','${org}','${actor.id}',1,${grantsSql(catalog)},false,'individual'); + UPDATE organizations SET permission_policy_version=1 WHERE id='${org}'; COMMIT;`); + await route.continue(); + }, { times: 1 }); + await editTaxId(edit, 'PERM-4'); + await expect(page.getByRole('alert')).toContainText('Could not load this client'); + await expect(page.getByRole('heading', { name: 'Permission unset client', exact: true })).toHaveCount(0); + await expect(edit).toHaveCount(0); + await page.getByRole('button', { name: 'Retry client', exact: true }).click(); + await expect(page.getByRole('alert')).toContainText('Could not load this client'); + setPolicy(0); + await page.getByRole('button', { name: 'Retry client', exact: true }).click(); + await expect(edit).toBeVisible(); + assert.notEqual(await activeId(), 'client-edit'); + await editTaxId(edit, 'PERM-5'); + await expect(edit).toBeFocused(); + console.log('PASS: a policy change during a detail reload is rejected until the original policy returns'); + + // Policy one: catalog reads without management, even for a legacy Administrator. + setPolicy(1); + let overview = await loadList(); + assert.deepEqual([overview.canonical_permissions, overview.can_read, overview.can_manage], [true, true, false]); + assert.ok([unset, zero].every(client => overview.clients.some(row => row.client.id === client))); + await expect(page.getByRole('link', { name: 'Permission zero client', exact: true })).toBeVisible(); + await expect(sidebarClients).toBeVisible(); + for (const control of ['New client', 'Edit', 'Deactivate', 'Activate']) { + await expect(page.getByRole('button', { name: control, exact: true })).toHaveCount(0); + } + await expect(page.getByRole('link', { name: 'Import', exact: true })).toHaveCount(0); + await capture('read-only-list'); + for (const client of [unset, zero]) { + const view = await (await loadDetail(client)).json(); + assert.deepEqual([view.canonical_permissions, view.can_manage, Object.hasOwn(view.details, 'billing')], [true, false, false]); + await expect(billing).toBeVisible(); + await expect(billing).not.toContainText('Default rate'); + } + for (const control of ['Edit client', 'New project', 'New invoice']) { + await expect(page.getByRole(control === 'Edit client' ? 'button' : 'link', { name: control, exact: true })).toHaveCount(0); + } + console.log('PASS: ClientReadAll reads the catalog without client management or default rates'); + + setGrants([...catalog, 'project_read_managed', 'billable_rate_read_managed']); + const managed = await (await loadDetail(unset)).json(); + assert.equal(Object.hasOwn(managed.details, 'billing'), false); + await expect(billing).not.toContainText('Default rate'); + + setGrants([...catalog, 'billable_rate_read_all']); + for (const [client, rate, text] of [[unset, null, 'Not set'], [zero, 0, 'EUR 0.00 / h']]) { + const view = await (await loadDetail(client)).json(); + assert.deepEqual(view.details.billing, { default_rate_cents: rate }); + await expect(billing.locator('dt:text-is("Default rate") + dd')).toHaveText(text); + await expect(page.getByRole('button', { name: 'Edit client', exact: true })).toHaveCount(0); + } + await capture('global-rate-detail'); + console.log('PASS: only global billable-rate read reveals the client default, keeping unset distinct from zero'); + + setGrants([...floor, 'project_read_all', 'invoice_read_all', 'billable_rate_read_all', 'report_profitability_read']); + overview = await loadList(); + assert.deepEqual([overview.can_read, overview.can_manage, overview.clients], [false, false, []]); + await expect(page.getByRole('heading', { name: 'Client access is unavailable', exact: true })).toBeVisible(); + await expect(page.getByText('No clients yet', { exact: true })).toHaveCount(0); + await expect(page.getByRole('button', { name: 'New client', exact: true })).toHaveCount(0); + await expect(sidebarClients).toHaveCount(0); + await capture('denied-list'); + const denied = await loadDetail(zero); + assert.equal(denied.status(), 404); + await expect(page.getByRole('alert')).toContainText('Could not load this client'); + await expect(page.getByRole('heading', { name: 'Permission zero client', exact: true })).toHaveCount(0); + console.log('PASS: unrelated grants withhold the catalog as denied, not empty, and direct IDs are not found'); + + setGrants(catalog); + setPolicy(2); + await loadList(); + await expect(page.getByRole('alert')).toContainText('Could not load clients'); + await expect(page.getByRole('link', { name: 'Permission zero client', exact: true })).toHaveCount(0); + assert.deepEqual(errors, []); + console.log('PASS: an unknown policy fails closed on the Clients page'); + } catch (error) { + console.error(error); + console.error({ url: page.url(), errors, page: await page.locator('body').ariaSnapshot({ timeout: 3000 }).catch(failure => failure.message) }); + throw error; + } finally { + await browser.close(); + if (owned) sql(`BEGIN; + UPDATE organizations SET permission_policy_version=0 WHERE id='${org}'; + DELETE FROM person_permission_states WHERE id='${state}'; + DELETE FROM project_management_assignments WHERE id='${assignment}'; + DELETE FROM projects WHERE id='${project}'; + DELETE FROM clients WHERE id IN ('${unset}','${zero}'); COMMIT;`); + } +})().catch(error => { console.error(error); process.exitCode = 1; }); diff --git a/crates/horae/tests/browser/run-design-checks.sh b/crates/horae/tests/browser/run-design-checks.sh index 901627d9..56308eaa 100644 --- a/crates/horae/tests/browser/run-design-checks.sh +++ b/crates/horae/tests/browser/run-design-checks.sh @@ -65,7 +65,7 @@ fi # Explicit filenames allow focused iteration without bypassing database isolation. suites=("$@") if [[ ${#suites[@]} == 0 ]]; then - suites=(task-read-permissions project-read-permissions project-editor-permissions reports-permissions editor-navigation permission-recovery-storage permission-editor-recovery modals timesheet-errors timesheet-permissions projects-design responsive-layout menu-popovers mobile-navigation project-bulk-actions project-bulk-recovery action-errors new-project invoice-preparation project-task-rates new-project-permissions new-project-task-errors new-project-keyboard new-project-transport project-edit new-project-navigation clients client-context clients-access clients-errors clients-visual own-permissions permission-history task-catalog) + suites=(task-read-permissions project-read-permissions project-editor-permissions reports-permissions editor-navigation permission-recovery-storage permission-editor-recovery modals timesheet-errors timesheet-permissions projects-design responsive-layout menu-popovers mobile-navigation project-bulk-actions project-bulk-recovery action-errors new-project invoice-preparation project-task-rates new-project-permissions new-project-task-errors new-project-keyboard new-project-transport project-edit new-project-navigation clients client-context clients-access clients-permissions clients-errors clients-visual own-permissions permission-history task-catalog) fi for suite in "${suites[@]}"; do if [[ ! $suite =~ ^[a-z][a-z-]*$ || ! -f "$browser_tests/$suite.cjs" ]]; then From dbfc36ba37e168cddf9dfcc8922ef9430bc1e08c Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:22:45 +0200 Subject: [PATCH 14/16] Collapse the Clients refocus condition into a let chain --- crates/horae/src/pages/clients.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/horae/src/pages/clients.rs b/crates/horae/src/pages/clients.rs index 6e65b514..31af0022 100644 --- a/crates/horae/src/pages/clients.rs +++ b/crates/horae/src/pages/clients.rs @@ -85,10 +85,10 @@ pub fn ClientList() -> Element { return; } // One-shot: a failed, denied or rebound reload consumes it without focusing. - if let Some(opener) = refocus.take() { - if matches!(&*clients.peek(), Some(Ok(overview)) if overview.can_manage) { - restore_opener_focus(&opener, "clients-heading"); - } + if let Some(opener) = refocus.take() + && matches!(&*clients.peek(), Some(Ok(overview)) if overview.can_manage) + { + restore_opener_focus(&opener, "clients-heading"); } }); From efa3539e1b6cc3ddddf78bb6aba716a8459ce5b9 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:32:45 +0200 Subject: [PATCH 15/16] Store canonical grant sets in the Clients permission fixtures Global billable-rate, project and invoice reads require their managed prerequisites, so the fixtures stored non-canonical selections that the server correctly rejected. Assert success status before reading views. --- .../horae/tests/browser/clients-permissions.cjs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/horae/tests/browser/clients-permissions.cjs b/crates/horae/tests/browser/clients-permissions.cjs index c9892715..ef314247 100644 --- a/crates/horae/tests/browser/clients-permissions.cjs +++ b/crates/horae/tests/browser/clients-permissions.cjs @@ -38,7 +38,11 @@ const setPolicy = version => sql(`UPDATE organizations SET permission_policy_ver const loadList = async () => { const response = page.waitForResponse(r => r.url().includes('/api/client_overview')); await page.goto(`${base}/clients`); - return (await response).json(); + return success(await response); + }; + const success = async response => { + assert.equal(response.status(), 200, `${response.url()}: ${await response.text()}`); + return response.json(); }; const loadDetail = async client => { const response = page.waitForResponse(r => r.url().includes('/api/client_detail_view')); @@ -140,7 +144,7 @@ const setPolicy = version => sql(`UPDATE organizations SET permission_policy_ver await expect(page.getByRole('link', { name: 'Import', exact: true })).toHaveCount(0); await capture('read-only-list'); for (const client of [unset, zero]) { - const view = await (await loadDetail(client)).json(); + const view = await success(await loadDetail(client)); assert.deepEqual([view.canonical_permissions, view.can_manage, Object.hasOwn(view.details, 'billing')], [true, false, false]); await expect(billing).toBeVisible(); await expect(billing).not.toContainText('Default rate'); @@ -151,13 +155,13 @@ const setPolicy = version => sql(`UPDATE organizations SET permission_policy_ver console.log('PASS: ClientReadAll reads the catalog without client management or default rates'); setGrants([...catalog, 'project_read_managed', 'billable_rate_read_managed']); - const managed = await (await loadDetail(unset)).json(); + const managed = await success(await loadDetail(unset)); assert.equal(Object.hasOwn(managed.details, 'billing'), false); await expect(billing).not.toContainText('Default rate'); - setGrants([...catalog, 'billable_rate_read_all']); + setGrants([...catalog, 'billable_rate_read_managed', 'billable_rate_read_all']); for (const [client, rate, text] of [[unset, null, 'Not set'], [zero, 0, 'EUR 0.00 / h']]) { - const view = await (await loadDetail(client)).json(); + const view = await success(await loadDetail(client)); assert.deepEqual(view.details.billing, { default_rate_cents: rate }); await expect(billing.locator('dt:text-is("Default rate") + dd')).toHaveText(text); await expect(page.getByRole('button', { name: 'Edit client', exact: true })).toHaveCount(0); @@ -165,7 +169,8 @@ const setPolicy = version => sql(`UPDATE organizations SET permission_policy_ver await capture('global-rate-detail'); console.log('PASS: only global billable-rate read reveals the client default, keeping unset distinct from zero'); - setGrants([...floor, 'project_read_all', 'invoice_read_all', 'billable_rate_read_all', 'report_profitability_read']); + setGrants([...floor, 'project_read_managed', 'project_read_all', 'invoice_read_managed', 'invoice_read_all', + 'billable_rate_read_managed', 'billable_rate_read_all', 'report_profitability_read']); overview = await loadList(); assert.deepEqual([overview.can_read, overview.can_manage, overview.clients], [false, false, []]); await expect(page.getByRole('heading', { name: 'Client access is unavailable', exact: true })).toBeVisible(); From 270065691df46518f4185217938346d1c07fc282 Mon Sep 17 00:00:00 2001 From: Aldo Borrero <82811+aldoborrero@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:33:17 +0200 Subject: [PATCH 16/16] Expect the unknown-policy Clients read to be forbidden --- crates/horae/tests/browser/clients-permissions.cjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/horae/tests/browser/clients-permissions.cjs b/crates/horae/tests/browser/clients-permissions.cjs index ef314247..cd666735 100644 --- a/crates/horae/tests/browser/clients-permissions.cjs +++ b/crates/horae/tests/browser/clients-permissions.cjs @@ -38,7 +38,7 @@ const setPolicy = version => sql(`UPDATE organizations SET permission_policy_ver const loadList = async () => { const response = page.waitForResponse(r => r.url().includes('/api/client_overview')); await page.goto(`${base}/clients`); - return success(await response); + return response; }; const success = async response => { assert.equal(response.status(), 200, `${response.url()}: ${await response.text()}`); @@ -133,7 +133,7 @@ const setPolicy = version => sql(`UPDATE organizations SET permission_policy_ver // Policy one: catalog reads without management, even for a legacy Administrator. setPolicy(1); - let overview = await loadList(); + let overview = await success(await loadList()); assert.deepEqual([overview.canonical_permissions, overview.can_read, overview.can_manage], [true, true, false]); assert.ok([unset, zero].every(client => overview.clients.some(row => row.client.id === client))); await expect(page.getByRole('link', { name: 'Permission zero client', exact: true })).toBeVisible(); @@ -171,7 +171,7 @@ const setPolicy = version => sql(`UPDATE organizations SET permission_policy_ver setGrants([...floor, 'project_read_managed', 'project_read_all', 'invoice_read_managed', 'invoice_read_all', 'billable_rate_read_managed', 'billable_rate_read_all', 'report_profitability_read']); - overview = await loadList(); + overview = await success(await loadList()); assert.deepEqual([overview.can_read, overview.can_manage, overview.clients], [false, false, []]); await expect(page.getByRole('heading', { name: 'Client access is unavailable', exact: true })).toBeVisible(); await expect(page.getByText('No clients yet', { exact: true })).toHaveCount(0); @@ -186,7 +186,7 @@ const setPolicy = version => sql(`UPDATE organizations SET permission_policy_ver setGrants(catalog); setPolicy(2); - await loadList(); + assert.equal((await loadList()).status(), 403); await expect(page.getByRole('alert')).toContainText('Could not load clients'); await expect(page.getByRole('link', { name: 'Permission zero client', exact: true })).toHaveCount(0); assert.deepEqual(errors, []);