From 991c54084fa9c2de3dee4bfae0852705f0cb5000 Mon Sep 17 00:00:00 2001 From: Panat Taranat Date: Tue, 8 Sep 2026 15:56:26 -0400 Subject: [PATCH] fix(properties): include team Deal Stage values in the bulk loader The loader behind soup and search only returned system properties and tags. A customized team stores stage values on its own Deal Stage definition, so every board reload lost them. Return that definition too. --- ...e79f932f8668dd7dce998e2a885bcaf0d8bbf.json | 15 --- ...dcaf7352f2c8e7409d8eb3565cdeabb4fe6a.json} | 25 ++++- crates/crm/src/domain/stages.rs | 2 +- crates/properties/fixtures/team_stage.sql | 23 +++++ crates/properties/src/domain/model.rs | 3 + crates/properties/src/lib.rs | 5 +- .../outbound/entity_properties_get_query.rs | 29 ++++-- .../outbound/entity_properties_values_test.rs | 93 +++++++++++++++++++ crates/soup/src/outbound/pg_soup_repo.rs | 5 +- 9 files changed, 169 insertions(+), 31 deletions(-) delete mode 100644 .sqlx/query-9d1892f8bb8ba65131fa035ffe3e79f932f8668dd7dce998e2a885bcaf0d8bbf.json rename .sqlx/{query-4d75ecd9c8d6f98881d44d88d10ebc20de426c8aad90b680cc8ae351118e5495.json => query-b68293cb0a18d2274fade4859e0ddcaf7352f2c8e7409d8eb3565cdeabb4fe6a.json} (87%) create mode 100644 crates/properties/fixtures/team_stage.sql diff --git a/.sqlx/query-9d1892f8bb8ba65131fa035ffe3e79f932f8668dd7dce998e2a885bcaf0d8bbf.json b/.sqlx/query-9d1892f8bb8ba65131fa035ffe3e79f932f8668dd7dce998e2a885bcaf0d8bbf.json deleted file mode 100644 index 785d0addfd6..00000000000 --- a/.sqlx/query-9d1892f8bb8ba65131fa035ffe3e79f932f8668dd7dce998e2a885bcaf0d8bbf.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n UPDATE entity_properties\n SET\n values = jsonb_set(\n values,\n '{value}',\n COALESCE(\n (\n SELECT jsonb_agg(elem)\n FROM jsonb_array_elements(values -> 'value') AS elem\n WHERE elem <> to_jsonb($2::text)\n ),\n '[]'::jsonb\n )\n ),\n updated_at = NOW()\n WHERE property_definition_id = $1\n AND values @> jsonb_build_object('value', jsonb_build_array($2::text))\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Uuid", - "Text" - ] - }, - "nullable": [] - }, - "hash": "9d1892f8bb8ba65131fa035ffe3e79f932f8668dd7dce998e2a885bcaf0d8bbf" -} diff --git a/.sqlx/query-4d75ecd9c8d6f98881d44d88d10ebc20de426c8aad90b680cc8ae351118e5495.json b/.sqlx/query-b68293cb0a18d2274fade4859e0ddcaf7352f2c8e7409d8eb3565cdeabb4fe6a.json similarity index 87% rename from .sqlx/query-4d75ecd9c8d6f98881d44d88d10ebc20de426c8aad90b680cc8ae351118e5495.json rename to .sqlx/query-b68293cb0a18d2274fade4859e0ddcaf7352f2c8e7409d8eb3565cdeabb4fe6a.json index 5ed75dc2063..949b6cc0072 100644 --- a/.sqlx/query-4d75ecd9c8d6f98881d44d88d10ebc20de426c8aad90b680cc8ae351118e5495.json +++ b/.sqlx/query-b68293cb0a18d2274fade4859e0ddcaf7352f2c8e7409d8eb3565cdeabb4fe6a.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\nSELECT\n ep.id as entity_property_id,\n ep.entity_id,\n ep.entity_type as \"entity_type: EntityType\",\n ep.property_definition_id,\n ep.values as \"values: sqlx::types::JsonValue\",\n ep.created_at as entity_property_created_at,\n ep.updated_at as entity_property_updated_at,\n pd.team_id as definition_team_id,\n pd.user_id as definition_user_id,\n pd.display_name,\n pd.data_type as \"data_type: DataType\",\n pd.is_multi_select,\n pd.specific_entity_type as \"specific_entity_type: Option\",\n pd.created_at as definition_created_at,\n pd.updated_at as definition_updated_at,\n pd.is_system as definition_is_system\nFROM entity_properties ep\nINNER JOIN property_definitions pd ON ep.property_definition_id = pd.id\nWHERE (ep.entity_id, ep.entity_type) IN (\n SELECT * FROM UNNEST($1::TEXT[], $2::property_entity_type[])\n)\nAND (\n pd.id = ANY($3::UUID[])\n OR (\n $4::text IS NOT NULL\n AND pd.data_type = $5\n AND (\n pd.user_id = $4\n OR pd.team_id IN (SELECT tu.team_id FROM team_user tu WHERE tu.user_id = $4)\n )\n )\n)\n ", + "query": "\nSELECT\n ep.id as entity_property_id,\n ep.entity_id,\n ep.entity_type as \"entity_type: EntityType\",\n ep.property_definition_id,\n ep.values as \"values: sqlx::types::JsonValue\",\n ep.created_at as entity_property_created_at,\n ep.updated_at as entity_property_updated_at,\n pd.team_id as definition_team_id,\n pd.user_id as definition_user_id,\n pd.display_name,\n pd.data_type as \"data_type: DataType\",\n pd.is_multi_select,\n pd.specific_entity_type as \"specific_entity_type: Option\",\n pd.created_at as definition_created_at,\n pd.updated_at as definition_updated_at,\n pd.is_system as definition_is_system\nFROM entity_properties ep\nINNER JOIN property_definitions pd ON ep.property_definition_id = pd.id\nWHERE (ep.entity_id, ep.entity_type) IN (\n SELECT * FROM UNNEST($1::TEXT[], $2::property_entity_type[])\n)\nAND (\n pd.id = ANY($3::UUID[])\n OR (\n $4::text IS NOT NULL\n AND pd.data_type = $5\n AND (\n pd.user_id = $4\n OR pd.team_id IN (SELECT tu.team_id FROM team_user tu WHERE tu.user_id = $4)\n )\n )\n OR (\n $4::text IS NOT NULL\n AND pd.is_system = FALSE\n AND pd.is_multi_select = FALSE\n AND pd.data_type = $6\n AND pd.display_name = $7\n AND pd.team_id IN (SELECT tu.team_id FROM team_user tu WHERE tu.user_id = $4)\n )\n)\n ", "describe": { "columns": [ { @@ -185,7 +185,26 @@ ] } } - } + }, + { + "Custom": { + "name": "property_data_type", + "kind": { + "Enum": [ + "BOOLEAN", + "DATE", + "NUMBER", + "STRING", + "SELECT_NUMBER", + "SELECT_STRING", + "ENTITY", + "LINK", + "TAG" + ] + } + } + }, + "Text" ] }, "nullable": [ @@ -207,5 +226,5 @@ false ] }, - "hash": "4d75ecd9c8d6f98881d44d88d10ebc20de426c8aad90b680cc8ae351118e5495" + "hash": "b68293cb0a18d2274fade4859e0ddcaf7352f2c8e7409d8eb3565cdeabb4fe6a" } diff --git a/crates/crm/src/domain/stages.rs b/crates/crm/src/domain/stages.rs index 7f75e147ac6..e9be03a1694 100644 --- a/crates/crm/src/domain/stages.rs +++ b/crates/crm/src/domain/stages.rs @@ -14,7 +14,7 @@ use crate::domain::{ mod test; /// Name of the team-scoped stage definition; `Stage` is reserved by a trigger. -pub const CRM_TEAM_STAGE_DEFINITION_NAME: &str = "Deal Stage"; +pub use properties::CRM_TEAM_STAGE_DEFINITION_NAME; /// Maximum stages in one pipeline. pub const MAX_STAGES: usize = 50; diff --git a/crates/properties/fixtures/team_stage.sql b/crates/properties/fixtures/team_stage.sql new file mode 100644 index 00000000000..653ffefcc4d --- /dev/null +++ b/crates/properties/fixtures/team_stage.sql @@ -0,0 +1,23 @@ +-- One Deal Stage definition per team, shaped like the CRM stage service creates them. +-- company1 has a value on both, so a viewer must only see their own team's. +INSERT INTO property_definitions (id, team_id, user_id, display_name, data_type, is_multi_select, specific_entity_type) +VALUES + ('dd111111-1111-1111-1111-111111111111', '0e000000-0000-0000-0000-000000000001', NULL, 'Deal Stage', 'SELECT_STRING', false, NULL), + ('dd222222-2222-2222-2222-222222222222', '0e000000-0000-0000-0000-000000000002', NULL, 'Deal Stage', 'SELECT_STRING', false, NULL), + -- Same shape, different name: must not be picked up. + ('dd333333-3333-3333-3333-333333333333', '0e000000-0000-0000-0000-000000000001', NULL, 'Region', 'SELECT_STRING', false, NULL) +ON CONFLICT (id) DO NOTHING; + +INSERT INTO property_options (id, property_definition_id, display_order, number_value, string_value, color) +VALUES + ('0dd11111-1111-1111-1111-111111111111', 'dd111111-1111-1111-1111-111111111111', 0, NULL, 'Lead', NULL), + ('0dd11111-1111-1111-1111-111111111112', 'dd111111-1111-1111-1111-111111111111', 1, NULL, 'Customer', NULL), + ('0dd22222-2222-2222-2222-222222222222', 'dd222222-2222-2222-2222-222222222222', 0, NULL, 'Prospect', NULL), + ('0dd33333-3333-3333-3333-333333333333', 'dd333333-3333-3333-3333-333333333333', 0, NULL, 'EMEA', NULL) +ON CONFLICT (id) DO NOTHING; + +INSERT INTO entity_properties (id, entity_id, entity_type, property_definition_id, values) +VALUES + ('e0888888-8888-8888-8888-888888888881', 'company1', 'COMPANY', 'dd111111-1111-1111-1111-111111111111', '{"type": "SelectOption", "value": ["0dd11111-1111-1111-1111-111111111112"]}'), + ('e0888888-8888-8888-8888-888888888882', 'company1', 'COMPANY', 'dd222222-2222-2222-2222-222222222222', '{"type": "SelectOption", "value": ["0dd22222-2222-2222-2222-222222222222"]}'), + ('e0888888-8888-8888-8888-888888888883', 'company1', 'COMPANY', 'dd333333-3333-3333-3333-333333333333', '{"type": "SelectOption", "value": ["0dd33333-3333-3333-3333-333333333333"]}'); diff --git a/crates/properties/src/domain/model.rs b/crates/properties/src/domain/model.rs index 8a757947fba..0bfcf5ce9b1 100644 --- a/crates/properties/src/domain/model.rs +++ b/crates/properties/src/domain/model.rs @@ -15,6 +15,9 @@ use models_properties::service::property_value::PropertyValue; use models_properties::{DataType, EntityReference, EntityType, PropertyOwner}; use uuid::Uuid; +/// Name of a team's CRM stage definition. Written by the CRM crate, read by the loaders here. +pub const CRM_TEAM_STAGE_DEFINITION_NAME: &str = "Deal Stage"; + /// Map an internal properties storage type to its canonical entity type. pub fn canonical_entity_type(entity_type: EntityType) -> AccessEntityType { match entity_type { diff --git a/crates/properties/src/lib.rs b/crates/properties/src/lib.rs index 640882676b4..df54c872e55 100644 --- a/crates/properties/src/lib.rs +++ b/crates/properties/src/lib.rs @@ -18,8 +18,9 @@ pub mod outbound; pub use domain::error::PropertiesErr; pub use domain::model::{ - EditReceipt, EntityPropertiesKey, EntityPropertyInfo, PropertyAccessReceiptExt, - PropertyOptionInfo, PropertyTargetKey, ViewReceipt, canonical_entity_type, + CRM_TEAM_STAGE_DEFINITION_NAME, EditReceipt, EntityPropertiesKey, EntityPropertyInfo, + PropertyAccessReceiptExt, PropertyOptionInfo, PropertyTargetKey, ViewReceipt, + canonical_entity_type, }; pub use domain::ports::{NotificationService, PermissionService, PropertiesRepo}; pub use domain::service::{PropertiesService, TeamReceipt}; diff --git a/crates/properties/src/outbound/entity_properties_get_query.rs b/crates/properties/src/outbound/entity_properties_get_query.rs index a2bbcfea663..6f4fdfb2113 100644 --- a/crates/properties/src/outbound/entity_properties_get_query.rs +++ b/crates/properties/src/outbound/entity_properties_get_query.rs @@ -12,7 +12,9 @@ use sqlx::{Pool, Postgres}; use uuid::Uuid; use super::property_option_queries; -use crate::domain::model::{EntityPropertyInfo, PropertyOptionInfo}; +use crate::domain::model::{ + CRM_TEAM_STAGE_DEFINITION_NAME, EntityPropertyInfo, PropertyOptionInfo, +}; /// Database row from the joined query. struct PropertyRow { @@ -627,17 +629,18 @@ WHERE (ep.entity_id, ep.entity_type) IN ( /// Gets entity properties with their definitions and values for multiple entities, filtered by property definition IDs. /// Returns a HashMap where the key is the entity_id and the value is Vec. -/// Only returns properties matching the specified property_ids. When `tag_viewer_user_id` is set, -/// also returns TAG properties whose definition is owned by that user or their team. +/// Only returns properties matching the specified property_ids. When `viewer_user_id` is set, +/// also returns TAG properties owned by that user or their team, and the team's CRM stage +/// definition ([`CRM_TEAM_STAGE_DEFINITION_NAME`]). #[tracing::instrument(skip(pool))] pub async fn get_bulk_entity_properties_values_filtered( pool: &Pool, entity_refs: &[EntityReference], property_ids: &[Uuid], - tag_viewer_user_id: Option<¯o_user_id::user_id::MacroUserIdStr<'_>>, + viewer_user_id: Option<¯o_user_id::user_id::MacroUserIdStr<'_>>, ) -> anyhow::Result>> { - let tag_viewer_user_id: Option<&str> = tag_viewer_user_id.map(|u| u.as_ref()); - if entity_refs.is_empty() || (property_ids.is_empty() && tag_viewer_user_id.is_none()) { + let viewer_user_id: Option<&str> = viewer_user_id.map(|u| u.as_ref()); + if entity_refs.is_empty() || (property_ids.is_empty() && viewer_user_id.is_none()) { // If no property_ids specified, return empty map for each entity let mut result = HashMap::new(); for entity_ref in entity_refs { @@ -683,13 +686,23 @@ AND ( OR pd.team_id IN (SELECT tu.team_id FROM team_user tu WHERE tu.user_id = $4) ) ) + OR ( + $4::text IS NOT NULL + AND pd.is_system = FALSE + AND pd.is_multi_select = FALSE + AND pd.data_type = $6 + AND pd.display_name = $7 + AND pd.team_id IN (SELECT tu.team_id FROM team_user tu WHERE tu.user_id = $4) + ) ) "#, &entity_ids, &entity_types as &[EntityType], &property_ids, - tag_viewer_user_id, - DataType::Tag as DataType + viewer_user_id, + DataType::Tag as DataType, + DataType::SelectString as DataType, + CRM_TEAM_STAGE_DEFINITION_NAME ) .fetch_all(pool) .await?; diff --git a/crates/properties/src/outbound/entity_properties_values_test.rs b/crates/properties/src/outbound/entity_properties_values_test.rs index 763b7fd07e3..930d95817ab 100644 --- a/crates/properties/src/outbound/entity_properties_values_test.rs +++ b/crates/properties/src/outbound/entity_properties_values_test.rs @@ -579,3 +579,96 @@ async fn get_property_options_batch(pool: Pool) -> anyhow::Result<()> Ok(()) } + +/// Definition ids returned for company1 for the given ids and viewer. +async fn company1_definition_ids( + pool: &Pool, + property_ids: &[Uuid], + viewer: Option<¯o_user_id::user_id::MacroUserIdStr<'_>>, +) -> anyhow::Result> { + let entity_refs = vec![EntityReference { + entity_id: "company1".to_string(), + entity_type: EntityType::Company, + specific_message_id: None, + }]; + let map = entity_properties_get_query::get_bulk_entity_properties_values_filtered( + pool, + &entity_refs, + property_ids, + viewer, + ) + .await?; + let mut ids: Vec = map["company1"].iter().map(|p| p.definition.id).collect(); + ids.sort(); + Ok(ids) +} + +#[sqlx::test( + migrator = "MACRO_DB_MIGRATIONS", + fixtures(path = "../../fixtures", scripts("properties", "team_stage")) +)] +async fn get_bulk_filtered_includes_viewer_team_deal_stage( + pool: Pool, +) -> anyhow::Result<()> { + let team1_stage = Uuid::parse_str("dd111111-1111-1111-1111-111111111111")?; + let team2_stage = Uuid::parse_str("dd222222-2222-2222-2222-222222222222")?; + let team1_region = Uuid::parse_str("dd333333-3333-3333-3333-333333333333")?; + let customer_option = Uuid::parse_str("0dd11111-1111-1111-1111-111111111112")?; + let user1 = + macro_user_id::user_id::MacroUserIdStr::parse_from_str("macro|user1@test.com").unwrap(); + let user2 = + macro_user_id::user_id::MacroUserIdStr::parse_from_str("macro|user2@test.com").unwrap(); + let user3 = + macro_user_id::user_id::MacroUserIdStr::parse_from_str("macro|user3@test.com").unwrap(); + + // Each viewer sees only their own team's Deal Stage. + assert_eq!( + company1_definition_ids(&pool, &[], Some(&user1)).await?, + vec![team1_stage] + ); + assert_eq!( + company1_definition_ids(&pool, &[], Some(&user3)).await?, + vec![team1_stage] + ); + assert_eq!( + company1_definition_ids(&pool, &[], Some(&user2)).await?, + vec![team2_stage] + ); + + // Options come with it. + let entity_refs = vec![EntityReference { + entity_id: "company1".to_string(), + entity_type: EntityType::Company, + specific_message_id: None, + }]; + let map = entity_properties_get_query::get_bulk_entity_properties_values_filtered( + &pool, + &entity_refs, + &[], + Some(&user1), + ) + .await?; + let stage = &map["company1"][0]; + assert_eq!(stage.definition.display_name, "Deal Stage"); + assert!( + stage + .options + .as_deref() + .unwrap_or_default() + .iter() + .any(|option| option.id == customer_option) + ); + + // Requested ids still come back too. + let mut expected = vec![team1_stage, team1_region]; + expected.sort(); + assert_eq!( + company1_definition_ids(&pool, &[team1_region], Some(&user1)).await?, + expected + ); + + // No viewer, nothing extra. + assert!(company1_definition_ids(&pool, &[], None).await?.is_empty()); + + Ok(()) +} diff --git a/crates/soup/src/outbound/pg_soup_repo.rs b/crates/soup/src/outbound/pg_soup_repo.rs index 9006021a446..b1e2899993b 100644 --- a/crates/soup/src/outbound/pg_soup_repo.rs +++ b/crates/soup/src/outbound/pg_soup_repo.rs @@ -300,8 +300,9 @@ fn type_err(e: E) -> sqlx::Error { /// /// This helper collects entity references from items that support properties /// and performs one bulk lookup. System properties are always included, plus -/// the caller's own and team tag properties. Tasks use `EntityType::Task` while -/// regular documents use `EntityType::Document`. +/// the caller's own and team tag properties and the team's CRM stage +/// definition. Tasks use `EntityType::Task` while regular documents use +/// `EntityType::Document`. #[tracing::instrument(err, skip(db, items))] pub(crate) async fn populate_properties( db: &sqlx::PgPool,