From d0d12870b49e2f60a696e721b8ef36ea9ad20b28 Mon Sep 17 00:00:00 2001 From: stack72 Date: Sun, 18 Aug 2024 02:27:46 +0100 Subject: [PATCH] fix(dal, sdf, web): Calculate if an asset can be contributed We used to depend on checking that a Module doesn't exist to understand if an asset can be contributed or not We are now checking the module index to see if it can be contributed or not --- app/web/src/components/AssetCard.vue | 10 ++++- app/web/src/components/AssetListItem.vue | 9 +++- app/web/src/store/module.store.ts | 3 ++ lib/dal/src/module.rs | 45 +++++++++++++++++-- lib/dal/tests/integration_test/module.rs | 3 +- .../src/server/service/v2/module/sync.rs | 16 +------ lib/si-frontend-types-rs/src/lib.rs | 4 +- lib/si-frontend-types-rs/src/module.rs | 5 ++- 8 files changed, 70 insertions(+), 25 deletions(-) diff --git a/app/web/src/components/AssetCard.vue b/app/web/src/components/AssetCard.vue index 696669c62a..e849a9ba57 100644 --- a/app/web/src/components/AssetCard.vue +++ b/app/web/src/components/AssetCard.vue @@ -31,7 +31,7 @@
!!moduleStore.upgradeableModules[props.assetId], ); +const canContribute = computed(() => { + return ( + !!moduleStore.contributableModules.includes( + asset.value?.schemaVariantId ?? "", + ) || asset.value?.canContribute + ); +}); + const updateAsset = () => { const schemaVariantId = asset.value?.schemaVariantId; if (!schemaVariantId) { diff --git a/app/web/src/components/AssetListItem.vue b/app/web/src/components/AssetListItem.vue index 1fa4eed894..6578d8375d 100644 --- a/app/web/src/components/AssetListItem.vue +++ b/app/web/src/components/AssetListItem.vue @@ -24,7 +24,7 @@
!!moduleStore.upgradeableModules[props.a.schemaVariantId], ); +const canContribute = computed(() => { + return ( + !!moduleStore.contributableModules.includes(props.a.schemaVariantId) || + props.a.canContribute + ); +}); + const onClick = (e: MouseEvent) => { if (e.shiftKey) assetStore.addSchemaVariantSelection(props.a.schemaVariantId); else assetStore.setSchemaVariantSelection(props.a.schemaVariantId); diff --git a/app/web/src/store/module.store.ts b/app/web/src/store/module.store.ts index 351624211b..559e52349a 100644 --- a/app/web/src/store/module.store.ts +++ b/app/web/src/store/module.store.ts @@ -144,6 +144,7 @@ export const useModuleStore = () => { state: () => ({ upgradeableModules: {} as Record, installableModulesById: {} as Record, + contributableModules: [] as SchemaVariantId[], localModulesByName: {} as Record, localModuleDetailsByName: {} as Record< ModuleName, @@ -220,6 +221,7 @@ export const useModuleStore = () => { { upgradeable: Record; installable: LatestModule[]; + contributable: SchemaVariantId[]; }, Visibility >({ @@ -227,6 +229,7 @@ export const useModuleStore = () => { params: { ...visibility }, onSuccess: (response) => { this.upgradeableModules = response.upgradeable; + this.contributableModules = response.contributable; this.installableModulesById = _.keyBy( response.installable, (m) => m.id, diff --git a/lib/dal/src/module.rs b/lib/dal/src/module.rs index dec9d2fa9a..4e40e0fff7 100644 --- a/lib/dal/src/module.rs +++ b/lib/dal/src/module.rs @@ -425,15 +425,51 @@ impl Module { pub async fn sync( ctx: &DalContext, latest_modules: Vec, - past_hashes_by_module_id: HashMap>, + builtin_modules: Vec, ) -> ModuleResult { let start = Instant::now(); + // Initialize result struct + let mut synced_modules = frontend_types::SyncedModules::new(); + // Collect all user facing schema variants. We need to see what can be upgraded. let schema_variants = SchemaVariant::list_user_facing(ctx).await?; - // Initialize result struct - let mut synced_modules = frontend_types::SyncedModules::new(); + // Check the locally found schema_variant_ids to see if it's contributable + // Contributable means that it's not avilable in the module index NOR is it a builtin + // we check it's a builtin because it's hash would be in the past_hashes_by_module_id + for schema_variant in &schema_variants { + let schema_variant_id: SchemaVariantId = schema_variant.schema_variant_id.into(); + let schema_id: SchemaId = schema_variant.schema_id.into(); + let is_default = SchemaVariant::is_default_by_id(ctx, schema_variant_id).await?; + let is_locked = SchemaVariant::is_locked_by_id(ctx, schema_variant_id).await?; + + let module = Module::find_for_module_schema_id(ctx, schema_id.into()) + .await + .map_err(|e| SchemaVariantError::Module(e.to_string()))?; + if let Some(m) = module { + if is_default + && is_locked + && !builtin_modules + .iter() + .any(|md| md.latest_hash == m.root_hash) + { + synced_modules.contributable.push(schema_variant_id.into()) + } + } + } + + // Build a list of the past hashes for all of the modules + let past_hashes_for_module_id = builtin_modules + .into_iter() + .filter_map(|m| { + if let Some(past_hashes) = m.past_hashes { + Some((m.id, HashSet::from_iter(past_hashes.into_iter()))) + } else { + None + } + }) + .collect::>(); // For each latest module // if not for existing schema, is installable @@ -469,7 +505,7 @@ impl Module { // Due to rust lifetimes, we need to actually create a set here to reference in unwrap. // The Alternative would be to clone the existing hashset every loop, which is way worse. let empty_set = HashSet::new(); - let past_hashes = past_hashes_by_module_id + let past_hashes = past_hashes_for_module_id .get(&latest_module.id) .unwrap_or(&empty_set); @@ -508,6 +544,7 @@ impl Module { debug!(?synced_modules.installable, "collected installable modules"); debug!(?synced_modules.upgradeable, "collected upgradeable modules"); + debug!(?synced_modules.contributable, "collected contributable modules"); debug!("syncing modules took: {:?}", start.elapsed()); Ok(synced_modules) diff --git a/lib/dal/tests/integration_test/module.rs b/lib/dal/tests/integration_test/module.rs index d424e92dec..8685627a98 100644 --- a/lib/dal/tests/integration_test/module.rs +++ b/lib/dal/tests/integration_test/module.rs @@ -5,7 +5,6 @@ use dal::{DalContext, Schema}; use dal_test::test; use pretty_assertions_sorted::assert_eq; use si_pkg::{SocketSpecArity, SocketSpecKind}; -use std::collections::HashMap; use ulid::Ulid; #[test] @@ -227,7 +226,7 @@ async fn dummy_sync(ctx: &DalContext) { dummy_latest_module_upgradeable, dummy_latest_module_installable, ], - HashMap::new(), + vec![], ) .await .expect("could not sync"); diff --git a/lib/sdf-server/src/server/service/v2/module/sync.rs b/lib/sdf-server/src/server/service/v2/module/sync.rs index 8529f97e83..a85d8a6f79 100644 --- a/lib/sdf-server/src/server/service/v2/module/sync.rs +++ b/lib/sdf-server/src/server/service/v2/module/sync.rs @@ -5,7 +5,6 @@ use axum::{ use dal::{module::Module, ChangeSetId, WorkspacePk}; use module_index_client::ModuleIndexClient; use si_frontend_types as frontend_types; -use std::collections::{HashMap, HashSet}; use crate::server::{ extract::{AccessBuilder, HandlerContext, PosthogClient, RawAccessToken}, @@ -39,20 +38,7 @@ pub async fn sync( ) }; - let past_hashes_for_module_id = module_details - .modules - .into_iter() - .filter_map(|m| { - if let Some(past_hashes) = m.past_hashes { - Some((m.id, HashSet::from_iter(past_hashes.into_iter()))) - } else { - None - } - }) - .collect::>(); - - let synced_modules = - Module::sync(&ctx, latest_modules.modules, past_hashes_for_module_id).await?; + let synced_modules = Module::sync(&ctx, latest_modules.modules, module_details.modules).await?; track( &posthog_client, diff --git a/lib/si-frontend-types-rs/src/lib.rs b/lib/si-frontend-types-rs/src/lib.rs index 6c91d8bd13..acb5933e69 100644 --- a/lib/si-frontend-types-rs/src/lib.rs +++ b/lib/si-frontend-types-rs/src/lib.rs @@ -8,7 +8,9 @@ pub use crate::func::{ AttributeArgumentBinding, FuncArgument, FuncArgumentKind, FuncBinding, FuncBindings, FuncCode, FuncSummary, LeafInputLocation, }; -pub use crate::module::{LatestModule, ModuleContributeRequest, SyncedModules}; +pub use crate::module::{ + BuiltinModules, LatestModule, ModuleContributeRequest, ModuleDetails, SyncedModules, +}; pub use crate::schema_variant::{ ComponentType, InputSocket, OutputSocket, Prop, PropKind, SchemaVariant, }; diff --git a/lib/si-frontend-types-rs/src/module.rs b/lib/si-frontend-types-rs/src/module.rs index 95d9e332c8..cfd8ac1fd9 100644 --- a/lib/si-frontend-types-rs/src/module.rs +++ b/lib/si-frontend-types-rs/src/module.rs @@ -1,15 +1,18 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; -use si_events::SchemaVariantId; +pub use module_index_types::BuiltinsDetailsResponse as BuiltinModules; pub use module_index_types::LatestModuleResponse as LatestModule; +pub use module_index_types::ModuleDetailsResponse as ModuleDetails; +use si_events::SchemaVariantId; #[derive(Clone, Debug, Deserialize, Eq, Serialize, PartialEq, Default)] #[serde(rename_all = "camelCase")] pub struct SyncedModules { pub upgradeable: HashMap, pub installable: Vec, + pub contributable: Vec, } impl SyncedModules {