Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dal, sdf, web): Calculate if an asset can be contributed #4375

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion app/web/src/components/AssetCard.vue
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
<div class="ml-auto flex flex-none gap-xs">
<EditingPill v-if="!asset.isLocked" :color="asset.color" />
<IconButton
v-if="asset.canContribute"
v-if="canContribute"
:selected="contributeAssetModalRef?.isOpen || false"
class="hover:scale-125"
icon="cloud-upload"
Expand Down Expand Up @@ -198,6 +198,14 @@ const canUpdate = computed(
() => !!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) {
Expand Down
9 changes: 8 additions & 1 deletion app/web/src/components/AssetListItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<div class="ml-auto flex flex-none gap-xs shrink-0">
<EditingPill v-if="!a.isLocked" :color="a.color" />
<Icon
v-if="a.canContribute"
v-if="canContribute"
name="cloud-upload"
size="xs"
tone="action"
Expand Down Expand Up @@ -71,6 +71,13 @@ const canUpdate = computed(
() => !!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);
Expand Down
3 changes: 3 additions & 0 deletions app/web/src/store/module.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export const useModuleStore = () => {
state: () => ({
upgradeableModules: {} as Record<SchemaVariantId, LatestModule>,
installableModulesById: {} as Record<ModuleId, LatestModule>,
contributableModules: [] as SchemaVariantId[],
localModulesByName: {} as Record<ModuleName, LocalModuleSummary>,
localModuleDetailsByName: {} as Record<
ModuleName,
Expand Down Expand Up @@ -220,13 +221,15 @@ export const useModuleStore = () => {
{
upgradeable: Record<SchemaVariantId, LatestModule>;
installable: LatestModule[];
contributable: SchemaVariantId[];
},
Visibility
>({
url: API_PREFIX.concat(["sync"]),
params: { ...visibility },
onSuccess: (response) => {
this.upgradeableModules = response.upgradeable;
this.contributableModules = response.contributable;
this.installableModulesById = _.keyBy(
response.installable,
(m) => m.id,
Expand Down
45 changes: 41 additions & 4 deletions lib/dal/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,15 +425,51 @@ impl Module {
pub async fn sync(
ctx: &DalContext,
latest_modules: Vec<frontend_types::LatestModule>,
past_hashes_by_module_id: HashMap<String, HashSet<String>>,
builtin_modules: Vec<frontend_types::ModuleDetails>,
) -> ModuleResult<frontend_types::SyncedModules> {
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::<HashMap<_, _>>();

// For each latest module
// if not for existing schema, is installable
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions lib/dal/tests/integration_test/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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");
Expand Down
16 changes: 1 addition & 15 deletions lib/sdf-server/src/server/service/v2/module/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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::<HashMap<_, _>>();

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,
Expand Down
4 changes: 3 additions & 1 deletion lib/si-frontend-types-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
5 changes: 4 additions & 1 deletion lib/si-frontend-types-rs/src/module.rs
Original file line number Diff line number Diff line change
@@ -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<SchemaVariantId, LatestModule>,
pub installable: Vec<LatestModule>,
pub contributable: Vec<SchemaVariantId>,
}

impl SyncedModules {
Expand Down