fix [properties-be] - bunch of fixes

rust/cloud-storage/models_properties/src/lib.rs

@@ -13,4 +13,4 @@ pub mod service;
 pub mod shared;
 
 // Re-export commonly used shared types for convenience
-pub use shared::{DataType, EntityReference, EntityType, PropertyOwner};
+pub use shared::{DataType, EntityPropertyReference, EntityReference, EntityType, PropertyOwner};

rust/cloud-storage/models_properties/src/shared/entity_reference.rs

@@ -2,12 +2,22 @@
 
 use serde::{Deserialize, Serialize};
 use utoipa::ToSchema;
+use uuid::Uuid;
 
 use super::EntityType;
 
 /// Entity reference for entity-type property values.
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, ToSchema)]
 pub struct EntityReference {
+    pub entity_id: String,
     pub entity_type: EntityType,
+}
+
+/// Reference to an entity property, including the property definition ID.
+/// Used for property lookups and deletion validation.
+#[derive(Debug, Clone)]
+pub struct EntityPropertyReference {
     pub entity_id: String,
+    pub entity_type: EntityType,
+    pub property_definition_id: Uuid,
 }

rust/cloud-storage/models_properties/src/shared/mod.rs

@@ -4,6 +4,6 @@ pub mod entity_type;
 pub mod property_owner;
 
 pub use data_type::DataType;
-pub use entity_reference::EntityReference;
+pub use entity_reference::{EntityPropertyReference, EntityReference};
 pub use entity_type::EntityType;
 pub use property_owner::PropertyOwner;

rust/cloud-storage/properties_db_client/src/entity_properties/get.rs

@@ -9,7 +9,7 @@ use models_properties::service::entity_property::EntityProperty;
 use models_properties::service::entity_property_with_definition::EntityPropertyWithDefinition;
 use models_properties::service::property_definition::PropertyDefinition;
 use models_properties::service::property_value::PropertyValue;
-use models_properties::{DataType, EntityReference, EntityType};
+use models_properties::{DataType, EntityPropertyReference, EntityReference, EntityType};
 
 type Result<T> = std::result::Result<T, PropertiesDatabaseError>;
 
@@ -228,34 +228,31 @@ async fn attach_property_options(
     Ok(())
 }
 
-/// Gets the entity_id and entity_type for a given entity_property_id.
-/// Used for permission checking before deletion.
-///
-/// Note: This excludes system properties (is_system = TRUE). System properties
-/// cannot be deleted, so this function will return None for them.
+/// Looks up an entity property by its ID.
+/// Returns entity reference (for permissions) and definition ID (for required property check).
 #[tracing::instrument(skip(db))]
-pub async fn get_entity_type_from_entity_property(
+pub async fn lookup_entity_property(
     db: &Pool<Postgres>,
     entity_property_id: Uuid,
-) -> Result<Option<EntityReference>> {
+) -> Result<Option<EntityPropertyReference>> {
     let row = sqlx::query!(
         r#"
         SELECT 
             ep.entity_id,
-            ep.entity_type as "entity_type: EntityType"
+            ep.entity_type as "entity_type: EntityType",
+            ep.property_definition_id
         FROM entity_properties ep
-        JOIN property_definitions pd ON ep.property_definition_id = pd.id
         WHERE ep.id = $1
-          AND pd.is_system = FALSE
         "#,
         entity_property_id
     )
     .fetch_optional(db)
     .await?;
 
-    Ok(row.map(|r| EntityReference {
+    Ok(row.map(|r| EntityPropertyReference {
         entity_id: r.entity_id,
         entity_type: r.entity_type,
+        property_definition_id: r.property_definition_id,
     }))
 }
 
@@ -658,18 +655,24 @@ mod tests {
         migrator = "MACRO_DB_MIGRATIONS",
         fixtures(path = "../../fixtures", scripts("properties"))
     )]
-    async fn test_get_entity_type_from_entity_property(pool: Pool<Postgres>) -> anyhow::Result<()> {
+    async fn test_lookup_entity_property(pool: Pool<Postgres>) -> anyhow::Result<()> {
         const _: &sqlx::migrate::Migrator = &MACRO_DB_MIGRATIONS;
 
         let entity_property_id = "e0111111-1111-1111-1111-111111111111"
             .parse::<Uuid>()
             .unwrap();
-        let entity_ref = get_entity_type_from_entity_property(&pool, entity_property_id).await?;
+        let info = lookup_entity_property(&pool, entity_property_id).await?;
 
-        assert!(entity_ref.is_some());
-        let entity_ref = entity_ref.unwrap();
-        assert_eq!(entity_ref.entity_id, "doc1");
-        assert_eq!(entity_ref.entity_type, EntityType::Document);
+        assert!(info.is_some());
+        let info = info.unwrap();
+        assert_eq!(info.entity_id, "doc1");
+        assert_eq!(info.entity_type, EntityType::Document);
+        assert_eq!(
+            info.property_definition_id,
+            "11111111-1111-1111-1111-111111111111"
+                .parse::<Uuid>()
+                .unwrap()
+        );
 
         Ok(())
     }
@@ -678,17 +681,15 @@ mod tests {
         migrator = "MACRO_DB_MIGRATIONS",
         fixtures(path = "../../fixtures", scripts("properties"))
     )]
-    async fn test_get_entity_type_from_entity_property_not_found(
-        pool: Pool<Postgres>,
-    ) -> anyhow::Result<()> {
+    async fn test_lookup_entity_property_not_found(pool: Pool<Postgres>) -> anyhow::Result<()> {
         const _: &sqlx::migrate::Migrator = &MACRO_DB_MIGRATIONS;
 
         let entity_property_id = "00000000-0000-0000-0000-000000000000"
             .parse::<Uuid>()
             .unwrap();
-        let entity_ref = get_entity_type_from_entity_property(&pool, entity_property_id).await?;
+        let info = lookup_entity_property(&pool, entity_property_id).await?;
 
-        assert!(entity_ref.is_none());
+        assert!(info.is_none());
 
         Ok(())
     }

rust/cloud-storage/properties_service/Cargo.toml

@@ -38,6 +38,7 @@ models_permissions = { path = "../models_permissions" }
 models_properties = { path = "../models_properties" }
 properties_db_client = { path = "../properties_db_client" }
 secretsmanager_client = { path = "../secretsmanager_client" }
+system_properties = { path = "../system_properties" }
 serde = { workspace = true }
 serde_json = { workspace = true }
 serde_with = { workspace = true }

rust/cloud-storage/properties_service/src/api/permissions.rs

@@ -76,7 +76,8 @@ async fn get_access_level(
         EntityType::Project => "project",
         EntityType::Thread => "thread",
         EntityType::Channel => "channel",
-        EntityType::Company | EntityType::Task | EntityType::User => {
+        EntityType::Task => "document",
+        EntityType::Company | EntityType::User => {
             tracing::warn!("property operations not supported for this entity type");
             return Err(PermissionError::UnsupportedEntityType);
         }

rust/cloud-storage/properties_service/src/api/properties/entities/delete_property.rs

@@ -3,15 +3,15 @@ use axum::{
     http::StatusCode,
     response::{IntoResponse, Response},
 };
+use models_properties::EntityReference;
+use system_properties::SystemPropertyKey;
 use thiserror::Error;
 use uuid::Uuid;
 
 use crate::api::context::ApiContext;
 use model::user::UserContext;
 use properties_db_client::{
-    entity_properties::{
-        delete as entity_properties_delete, get::get_entity_type_from_entity_property,
-    },
+    entity_properties::{delete as entity_properties_delete, get::lookup_entity_property},
     error::PropertiesDatabaseError,
 };
 
@@ -25,6 +25,8 @@ pub enum DeleteEntityPropertyErr {
     Permission(#[from] crate::api::permissions::PermissionError),
     #[error("Entity property not found")]
     NotFound,
+    #[error("This property is required and cannot be removed from this entity")]
+    RequiredProperty,
 }
 
 impl IntoResponse for DeleteEntityPropertyErr {
@@ -34,6 +36,7 @@ impl IntoResponse for DeleteEntityPropertyErr {
             | DeleteEntityPropertyErr::DatabaseError(_) => StatusCode::INTERNAL_SERVER_ERROR,
             DeleteEntityPropertyErr::Permission(e) => e.status_code(),
             DeleteEntityPropertyErr::NotFound => StatusCode::NOT_FOUND,
+            DeleteEntityPropertyErr::RequiredProperty => StatusCode::FORBIDDEN,
         };
 
         if status_code.is_server_error() {
@@ -57,6 +60,7 @@ impl IntoResponse for DeleteEntityPropertyErr {
     ),
     responses(
         (status = 204, description = "Entity property removed successfully"),
+        (status = 403, description = "Property is required and cannot be removed"),
         (status = 404, description = "Entity property not found"),
         (status = 500, description = "Internal server error")
     ),
@@ -70,10 +74,8 @@ pub async fn delete_entity_property(
 ) -> Result<StatusCode, DeleteEntityPropertyErr> {
     tracing::info!("removing entity property");
 
-    // Get entity property metadata to check permissions
-    // Note: get_entity_type_from_entity_property excludes system properties,
-    // so system properties will return NotFound here.
-    let entity_ref = get_entity_type_from_entity_property(&context.db, entity_property_uuid)
+    // Lookup entity property
+    let property_info = lookup_entity_property(&context.db, entity_property_uuid)
         .await
         .inspect_err(|e| {
             tracing::error!(
@@ -84,11 +86,30 @@ pub async fn delete_entity_property(
         .ok_or(DeleteEntityPropertyErr::NotFound)?;
 
     tracing::debug!(
-        entity_id = %entity_ref.entity_id,
-        entity_type = ?entity_ref.entity_type,
-        "fetched entity property metadata"
+        entity_id = %property_info.entity_id,
+        entity_type = ?property_info.entity_type,
+        property_definition_id = %property_info.property_definition_id,
+        "fetched entity property info"
     );
 
+    // Check if this property is required for the entity type (e.g., Task properties)
+    if SystemPropertyKey::is_required_for_entity(
+        property_info.property_definition_id,
+        property_info.entity_type,
+    ) {
+        tracing::warn!(
+            entity_type = ?property_info.entity_type,
+            property_definition_id = %property_info.property_definition_id,
+            "attempted to remove required property"
+        );
+        return Err(DeleteEntityPropertyErr::RequiredProperty);
+    }
+
+    let entity_ref = EntityReference {
+        entity_id: property_info.entity_id,
+        entity_type: property_info.entity_type,
+    };
+
     crate::api::permissions::check_entity_edit_permission(
         &context,
         &user_context.user_id,

rust/cloud-storage/properties_service/src/api/properties/entities/get.rs

@@ -102,10 +102,11 @@ pub async fn get_entity_properties(
             ),
             async {
                 match entity_type {
-                    EntityType::Document => {
+                    EntityType::Document | EntityType::Task => {
                         super::super::metadata::get_document_metadata_properties(
                             &context.db,
                             &entity_id,
+                            entity_type,
                         )
                         .await
                     }