From d0c56333474f667324e935d570b9cb2dae73dfd0 Mon Sep 17 00:00:00 2001 From: Brit Myers Date: Sun, 23 Jun 2024 11:25:52 -0400 Subject: [PATCH] Disambiguate Schema Name, Schema Variant Name, and Schema Variant Display Name across the UI. On import, if there's no display name, hydrate with Schema name. Same thing on regenerate. This is a step towards schema names being immutable. --- app/web/src/api/sdf/dal/diagram.ts | 3 +- app/web/src/components/AssetCard.vue | 2 +- app/web/src/components/AssetDetailsPanel.vue | 16 ++- app/web/src/components/AssetEditorTabs.vue | 6 +- app/web/src/components/AssetListPanel.vue | 2 +- app/web/src/components/AssetPalette.vue | 6 +- .../FuncEditor/AttributeBindings.vue | 9 +- app/web/src/store/asset.store.ts | 9 +- app/web/src/store/components.store.ts | 1 + app/web/src/store/realtime/realtime_events.ts | 1 + lib/dal/src/history_event.rs | 14 ++- lib/dal/src/pkg.rs | 4 +- lib/dal/src/pkg/export.rs | 14 ++- lib/dal/src/pkg/import.rs | 8 +- lib/dal/src/schema/variant/authoring.rs | 115 +++++++++++------- lib/dal/src/schema/variant/json.rs | 27 ++-- lib/dal/src/schema/variant/metadata_view.rs | 4 +- lib/dal/src/user.rs | 5 + lib/dal/tests/integration_test/pkg/mod.rs | 7 +- .../variant/authoring/create_variant.rs | 4 +- .../schema/variant/authoring/save_variant.rs | 4 +- .../server/service/variant/clone_variant.rs | 2 +- .../server/service/variant/create_variant.rs | 2 +- .../src/server/service/variant/get_variant.rs | 6 +- .../server/service/variant/save_variant.rs | 4 +- .../server/service/variant/update_variant.rs | 2 +- lib/si-pkg/src/spec/variant.rs | 3 +- 27 files changed, 186 insertions(+), 94 deletions(-) diff --git a/app/web/src/api/sdf/dal/diagram.ts b/app/web/src/api/sdf/dal/diagram.ts index d57b9b627a..09db44252d 100644 --- a/app/web/src/api/sdf/dal/diagram.ts +++ b/app/web/src/api/sdf/dal/diagram.ts @@ -42,7 +42,8 @@ export interface DiagramSchemaVariant { export interface DiagramSchema { id: string; - name: string; + name: string; // schema name + displayName: string; // variant display name builtin: boolean; variants: DiagramSchemaVariant[]; diff --git a/app/web/src/components/AssetCard.vue b/app/web/src/components/AssetCard.vue index 93df9689f6..7d37c1d2af 100644 --- a/app/web/src/components/AssetCard.vue +++ b/app/web/src/components/AssetCard.vue @@ -24,7 +24,7 @@ {{ asset.displayName }} diff --git a/app/web/src/components/AssetDetailsPanel.vue b/app/web/src/components/AssetDetailsPanel.vue index 9f183ea489..7790a021fd 100644 --- a/app/web/src/components/AssetDetailsPanel.vue +++ b/app/web/src/components/AssetDetailsPanel.vue @@ -61,14 +61,22 @@
- + @@ -78,7 +86,7 @@ type="text" label="Display name" compact - placeholder="(optional) Provide the asset a shorter display name" + placeholder="(optional) Provide the asset version a display name" @blur="updateAsset" /> if (include && searchString.value.length) { include = !!( - asset.name.toLocaleLowerCase().includes(searchString.value) || + asset.schemaName.toLocaleLowerCase().includes(searchString.value) || asset.displayName?.toLocaleLowerCase().includes(searchString.value) ); } diff --git a/app/web/src/components/AssetPalette.vue b/app/web/src/components/AssetPalette.vue index 88b5442fee..e1a0198cb6 100644 --- a/app/web/src/components/AssetPalette.vue +++ b/app/web/src/components/AssetPalette.vue @@ -284,9 +284,11 @@ const schemaDisplayName = ( schemas: DiagramSchemaWithDisplayMetadata[], ) => { const duplicates = schemas.filter((s) => s.name === schema.name); + let displayName = schema.variants[0]?.displayName; + if (!displayName) displayName = schema.name; if (duplicates.length > 1) { - return `${schema.name} (${duplicates.indexOf(schema)})`; - } else return schema.name; + return `${displayName} (${duplicates.indexOf(schema)})`; + } else return displayName; }; const updateMouseNode = (e: MouseEvent) => { diff --git a/app/web/src/components/FuncEditor/AttributeBindings.vue b/app/web/src/components/FuncEditor/AttributeBindings.vue index 0586ed7af0..306b324bcb 100644 --- a/app/web/src/components/FuncEditor/AttributeBindings.vue +++ b/app/web/src/components/FuncEditor/AttributeBindings.vue @@ -19,6 +19,10 @@

Asset:

+

{{ proto.schema }}

+

+ Asset version: +

{{ proto.schemaVariant }}

@@ -227,9 +231,11 @@ const prototypeViews = computed(() => { ]; const schemaVariant = + useComponentsStore().schemaVariantsById[schemaVariantId ?? ""]?.name ?? + "none"; + const schema = useComponentsStore().schemaVariantsById[schemaVariantId ?? ""] ?.schemaName ?? "none"; - const component = funcStore.componentOptions.find((c) => c.value === proto.componentId) ?.label ?? "all"; @@ -256,6 +262,7 @@ const prototypeViews = computed(() => { return { id: proto.id, + schema, schemaVariant, component, outputLocation, diff --git a/app/web/src/store/asset.store.ts b/app/web/src/store/asset.store.ts index 71bc0bf468..4191783a28 100644 --- a/app/web/src/store/asset.store.ts +++ b/app/web/src/store/asset.store.ts @@ -74,6 +74,7 @@ export interface DetachedValidationPrototype { export interface ListedVariant { id: AssetId; defaultSchemaVariantId: string; + schemaName: string; name: string; displayName?: string; category: string; @@ -106,7 +107,7 @@ export type AssetCreateRequest = Omit< export type AssetCloneRequest = Visibility & { id: AssetId; name: string }; export const assetDisplayName = (asset: Asset | AssetListEntry) => - (asset.displayName ?? "").length === 0 ? asset.name : asset.displayName; + (asset.displayName ?? "").length === 0 ? asset.schemaName : asset.displayName; export const useAssetStore = () => { const changeSetsStore = useChangeSetsStore(); @@ -303,7 +304,8 @@ export const useAssetStore = () => { return { id: nilId(), defaultSchemaVariantId: "", - name, + schemaName: name, + name: "v0", displayName: name, code: "", color: this.generateMockColor(), @@ -399,8 +401,6 @@ export const useAssetStore = () => { return () => { if (current) { this.assetsById[asset.id] = current; - } else { - delete this.assetsById[asset.id]; } }; }, @@ -494,6 +494,7 @@ export const useAssetStore = () => { const variant = { id: data.schemaId, defaultSchemaVariantId: data.schemaVariantId, + schemaName: data.schemaName, name: data.name, displayName: "", category: data.category, diff --git a/app/web/src/store/components.store.ts b/app/web/src/store/components.store.ts index 7c89b971c4..ffc70313af 100644 --- a/app/web/src/store/components.store.ts +++ b/app/web/src/store/components.store.ts @@ -570,6 +570,7 @@ export const useComponentsStore = (forceChangeSetId?: ChangeSetId) => { schemasWithAtLeastOneVariant.push({ id: schema.id, name: schema.name, + displayName: schema.displayName, builtin: schema.builtin, variants: schema.variants, category: schema.variants[0].category, diff --git a/app/web/src/store/realtime/realtime_events.ts b/app/web/src/store/realtime/realtime_events.ts index 6a70e47c67..517b2b30cd 100644 --- a/app/web/src/store/realtime/realtime_events.ts +++ b/app/web/src/store/realtime/realtime_events.ts @@ -257,6 +257,7 @@ export type WsEventPayloadMap = { schemaId: string; schemaVariantId: string; name: string; + schemaName: string; category: string; color: string; changeSetId: ChangeSetId; diff --git a/lib/dal/src/history_event.rs b/lib/dal/src/history_event.rs index 622b31e163..f979f57090 100644 --- a/lib/dal/src/history_event.rs +++ b/lib/dal/src/history_event.rs @@ -7,7 +7,7 @@ use telemetry::prelude::*; use thiserror::Error; use crate::actor_view::ActorView; -use crate::{pk, DalContext, Timestamp, UserPk}; +use crate::{pk, DalContext, Timestamp, User, UserPk}; use crate::{Tenancy, TransactionsError}; #[remain::sorted] @@ -23,6 +23,8 @@ pub enum HistoryEventError { StandardModel(String), #[error("transactions error: {0}")] Transactions(#[from] TransactionsError), + #[error("user error: {0}")] + User(String), } pub type HistoryEventResult = Result; @@ -41,6 +43,16 @@ impl HistoryActor { HistoryActor::SystemInit => "unknown-backend".to_string(), } } + pub async fn email(&self, ctx: &DalContext) -> HistoryEventResult { + Ok(match self { + HistoryActor::SystemInit => "sally@systeminit.com".to_string(), + HistoryActor::User(user_pk) => User::get_by_pk_or_error(ctx, *user_pk) + .await + .map_err(|e| HistoryEventError::User(e.to_string()))? + .email() + .clone(), + }) + } } impl From for HistoryActor { diff --git a/lib/dal/src/pkg.rs b/lib/dal/src/pkg.rs index 656c8f0752..4c311aee27 100644 --- a/lib/dal/src/pkg.rs +++ b/lib/dal/src/pkg.rs @@ -24,7 +24,7 @@ use crate::{ SchemaVariantId, TransactionsError, UserPk, WorkspaceError, WorkspacePk, WsEvent, WsEventResult, WsPayload, }; -use crate::{AttributePrototypeId, FuncId, PropId, PropKind}; +use crate::{AttributePrototypeId, FuncId, HistoryEventError, PropId, PropKind}; use crate::module::ModuleError; use crate::socket::connection_annotation::ConnectionAnnotationError; @@ -64,6 +64,8 @@ pub enum PkgError { FuncArgumentNotFoundByName(FuncId, String), #[error("func {0} could not be found by name")] FuncNotFoundByName(String), + #[error("history event error: {0}")] + HistoryEvent(#[from] HistoryEventError), #[error("input socket error: {0}")] InputSocket(#[from] InputSocketError), #[error("Missing Func {1} for AttributePrototype {0}")] diff --git a/lib/dal/src/pkg/export.rs b/lib/dal/src/pkg/export.rs index 16620860e9..cc7b5b378d 100644 --- a/lib/dal/src/pkg/export.rs +++ b/lib/dal/src/pkg/export.rs @@ -64,8 +64,8 @@ impl PkgExporter { } } - fn new_standalone_variant_exporter() -> Self { - Self::new_module_exporter("", "", None::, "", vec![]) + fn new_standalone_variant_exporter(schema_name: &str) -> Self { + Self::new_module_exporter(schema_name, "", None::, "", vec![]) } pub async fn export_as_bytes(&mut self, ctx: &DalContext) -> PkgResult> { @@ -144,9 +144,11 @@ impl PkgExporter { pub async fn export_variant_standalone( ctx: &DalContext, variant: &SchemaVariant, + schema_name: &str, ) -> PkgResult<(SchemaVariantSpec, Vec)> { - let mut exporter = Self::new_standalone_variant_exporter(); - + let mut exporter = Self::new_standalone_variant_exporter(schema_name); + let email = ctx.history_actor().email(ctx).await?; + exporter.created_by = email; exporter.export_funcs_for_variant(ctx, variant.id()).await?; exporter.export_intrinsics(ctx).await?; let variant_spec = exporter.export_variant(ctx, variant, false).await?; @@ -171,7 +173,6 @@ impl PkgExporter { variant_spec_builder.name(variant.name()); variant_spec_builder.is_builtin(variant_is_builtin); variant_spec_builder.unique_id(variant.id().to_string()); - let mut data_builder = SchemaVariantSpecData::builder(); data_builder.name(variant.name()); @@ -180,6 +181,9 @@ impl PkgExporter { if let Some(link) = variant.link() { data_builder.try_link(link.to_string().deref())?; } + if let Some(display_name) = variant.display_name() { + data_builder.display_name(display_name); + } data_builder.component_type(get_component_type(ctx, variant).await?); diff --git a/lib/dal/src/pkg/import.rs b/lib/dal/src/pkg/import.rs index c5c9c28556..825c3b752f 100644 --- a/lib/dal/src/pkg/import.rs +++ b/lib/dal/src/pkg/import.rs @@ -971,6 +971,7 @@ pub(crate) async fn import_schema_variant( let (variant, created) = match existing_schema_variant { None => { let spec = schema_spec.to_spec().await?; + let schema_name = &spec.name.clone(); let metadata = SchemaVariantJson::metadata_from_spec(spec)?; let mut asset_func_id: Option = None; @@ -987,12 +988,17 @@ pub(crate) async fn import_schema_variant( asset_func_id = Some(asset_func.id) } } + let display_name = match metadata.display_name { + Some(name) => name, + None => schema_name.into(), + }; + info!(%display_name, "display_name"); ( SchemaVariant::new( ctx, schema.id(), variant_spec.name(), - metadata.menu_name, + display_name, metadata.category, metadata.color, metadata.component_type, diff --git a/lib/dal/src/schema/variant/authoring.rs b/lib/dal/src/schema/variant/authoring.rs index 1f94d73402..8e49a2d634 100644 --- a/lib/dal/src/schema/variant/authoring.rs +++ b/lib/dal/src/schema/variant/authoring.rs @@ -21,7 +21,8 @@ use crate::pkg::{import_pkg_from_pkg, PkgError}; use crate::schema::variant::{SchemaVariantJson, SchemaVariantMetadataJson}; use crate::{ pkg, ComponentType, DalContext, Func, FuncBackendKind, FuncBackendResponseType, FuncError, - FuncId, Schema, SchemaError, SchemaVariant, SchemaVariantError, SchemaVariantId, + FuncId, HistoryEventError, Schema, SchemaError, SchemaVariant, SchemaVariantError, + SchemaVariantId, }; #[allow(missing_docs)] @@ -38,6 +39,8 @@ pub enum VariantAuthoringError { FuncRun(#[from] FuncRunnerError), #[error("func run value sender has terminated without sending")] FuncRunGone, + #[error("history event error: {0}")] + HistoryEvent(#[from] HistoryEventError), #[error("layer db error: {0}")] LayerDb(#[from] LayerDbError), #[error("no new asset was created")] @@ -111,22 +114,19 @@ impl VariantAuthoringClient { let definition = execute_asset_func(ctx, &asset_func).await?; let metadata = SchemaVariantMetadataJson { - schema_name: name, - menu_name: display_name.clone(), + schema_name: name.clone(), + name: "v0".into(), + display_name: display_name.clone(), category: category.into(), color: color.into(), component_type: ComponentType::Component, link: link.clone(), description: description.clone(), }; + let email = ctx.history_actor().email(ctx).await?; - //TODO @stack72 - figure out how we get the current user in this! - let pkg_spec = build_pkg_spec_for_variant( - definition, - &asset_func_spec, - &metadata, - "sally@systeminit.com", - )?; + let pkg_spec = + build_pkg_spec_for_variant(&name, definition, &asset_func_spec, &metadata, &email)?; let pkg = SiPkg::load_from_spec(pkg_spec.clone())?; @@ -163,7 +163,7 @@ impl VariantAuthoringClient { let variant = SchemaVariant::get_by_id(ctx, schema_variant_id).await?; let schema = variant.schema(ctx).await?; - let display_name = variant.display_name().map(|dn| format!("{dn} Clone")); + let display_name = variant.display_name().map(|mn| format!("{mn} Clone")); if let Some(asset_func_id) = variant.asset_func_id() { let old_func = Func::get_by_id_or_error(ctx, asset_func_id).await?; @@ -172,20 +172,22 @@ impl VariantAuthoringClient { let cloned_func_spec = build_asset_func_spec(&cloned_func)?; let definition = execute_asset_func(ctx, &cloned_func).await?; let metadata = SchemaVariantMetadataJson { - schema_name: name, - menu_name: display_name.clone(), + schema_name: name.clone(), + name: "v0".into(), + display_name: display_name.clone(), category: variant.category().to_string(), color: variant.get_color(ctx).await?, component_type: variant.component_type(), link: variant.link().clone(), description: variant.description().clone(), }; - //TODO @stack72 - figure out how we get the current user in this! + let email = ctx.history_actor().email(ctx).await?; let pkg_spec = build_pkg_spec_for_variant( + &schema.name, definition, &cloned_func_spec, &metadata, - "sally@systeminit.com", + &email, )?; let pkg = SiPkg::load_from_spec(pkg_spec.clone())?; @@ -227,7 +229,7 @@ impl VariantAuthoringClient { ctx: &DalContext, current_sv_id: SchemaVariantId, name: impl Into, - menu_name: Option, + display_name: Option, category: impl Into, color: impl Into, link: Option, @@ -236,6 +238,7 @@ impl VariantAuthoringClient { component_type: ComponentType, ) -> VariantAuthoringResult { let sv = SchemaVariant::get_by_id(ctx, current_sv_id).await?; + let schema = sv.schema(ctx).await?; let asset_func_id = sv.asset_func_id() .ok_or(VariantAuthoringError::SchemaVariantAssetNotFound( @@ -249,8 +252,9 @@ impl VariantAuthoringClient { Self::update_existing_variant_and_regenerate( ctx, current_sv_id, + schema.name, name, - menu_name.clone(), + display_name.clone(), category, color, link.clone(), @@ -261,12 +265,15 @@ impl VariantAuthoringClient { .await?; Ok(current_sv_id) } else { + let name = name.into(); + let name = increment_version(&name); Self::update_and_generate_variant_with_new_version( ctx, &asset_func, current_sv_id, + schema.name, name, - menu_name.clone(), + display_name.clone(), category, color, link.clone(), @@ -287,8 +294,9 @@ impl VariantAuthoringClient { async fn update_existing_variant_and_regenerate( ctx: &DalContext, current_schema_variant_id: SchemaVariantId, + schema_name: impl Into, name: impl Into, - menu_name: Option, + display_name: Option, category: impl Into, color: impl Into, link: Option, @@ -308,14 +316,14 @@ impl VariantAuthoringClient { let name = name.into(); let category = category.into(); let color = color.into(); - + let schema_name = schema_name.into(); let mut asset_func = Func::get_by_id_or_error(ctx, asset_func_id).await?; asset_func = asset_func .modify(ctx, |func| { func.name.clone_from(&name); func.backend_kind = FuncBackendKind::JsSchemaVariantDefinition; func.backend_response_type = FuncBackendResponseType::SchemaVariantDefinition; - func.display_name = menu_name + func.display_name = display_name .clone() .map(|display_name| display_name.to_owned()); func.code_base64 = Some(code_base64); @@ -329,8 +337,9 @@ impl VariantAuthoringClient { let asset_func_spec = build_asset_func_spec(&asset_func)?; let definition = execute_asset_func(ctx, &asset_func).await?; let metadata = SchemaVariantMetadataJson { - schema_name: name.clone(), - menu_name: menu_name.clone(), + schema_name: schema_name.clone(), + name: name.clone(), + display_name: display_name.clone(), category: category.clone(), color: color.clone(), component_type, @@ -349,10 +358,10 @@ impl VariantAuthoringClient { .await?; let schema_spec = metadata.to_schema_spec(new_variant_spec)?; - //TODO @stack72 - figure out how we get the current user in this! + let email = ctx.history_actor().email(ctx).await?; let pkg_spec = PkgSpec::builder() - .name(&metadata.schema_name) - .created_by("sally@systeminit.com") + .name(&schema_name.clone()) + .created_by(&email) .funcs(variant_funcs.clone()) .func(asset_func_spec) .schema(schema_spec) @@ -381,7 +390,7 @@ impl VariantAuthoringClient { schema .clone() .modify(ctx, |s| { - s.name.clone_from(&name); + s.name.clone_from(&schema_name.clone()); Ok(()) }) .await?; @@ -418,7 +427,7 @@ impl VariantAuthoringClient { sv.category.clone_from(&category); sv.component_type = component_type; sv.color.clone_from(&color); - sv.display_name = menu_name; + sv.display_name = Some(display_name.unwrap_or(schema_name.clone())); Ok(()) }) .await?; @@ -437,8 +446,9 @@ impl VariantAuthoringClient { ctx: &DalContext, old_asset_func: &Func, current_sv_id: SchemaVariantId, + schema_name: impl Into, name: impl Into, - menu_name: Option, + display_name: Option, category: impl Into, color: impl Into, link: Option, @@ -447,6 +457,7 @@ impl VariantAuthoringClient { component_type: ComponentType, ) -> VariantAuthoringResult { let name = name.into(); + let schema_name = schema_name.into(); let mut new_asset_func = old_asset_func .duplicate(ctx, generate_scaffold_func_name(&name)) .await?; @@ -457,7 +468,7 @@ impl VariantAuthoringClient { .modify(ctx, |func| { func.backend_kind = FuncBackendKind::JsSchemaVariantDefinition; func.backend_response_type = FuncBackendResponseType::SchemaVariantDefinition; - func.display_name = menu_name + func.display_name = display_name .clone() .map(|display_name| display_name.to_owned()); func.code_base64 = Some(code_base64); @@ -473,8 +484,9 @@ impl VariantAuthoringClient { let definition = execute_asset_func(ctx, &new_asset_func).await?; let metadata = SchemaVariantMetadataJson { - schema_name: name.clone(), - menu_name: menu_name.clone(), + schema_name: schema_name.clone(), + name: name.clone(), + display_name: display_name.clone(), category: category.into(), color: color.into(), component_type, @@ -494,10 +506,11 @@ impl VariantAuthoringClient { let schema_spec = metadata.to_schema_spec(new_variant_spec)?; - //TODO @stack72 - figure out how we get the current user in this! + let email = ctx.history_actor().email(ctx).await?; + let pkg_spec = PkgSpec::builder() - .name(&metadata.schema_name) - .created_by("sally@systeminit.com") + .name(&schema_name.clone()) + .created_by(&email) .funcs(variant_funcs.clone()) .func(asset_func_spec) .schema(schema_spec) @@ -526,7 +539,7 @@ impl VariantAuthoringClient { schema .clone() .modify(ctx, |s| { - s.name.clone_from(&name); + s.name = schema_name; Ok(()) }) .await?; @@ -561,8 +574,9 @@ impl VariantAuthoringClient { pub async fn save_variant_content( ctx: &DalContext, current_schema_variant_id: SchemaVariantId, - content_name: impl Into, - menu_name: Option, + schema_name: impl Into, + name: impl Into, + display_name: Option, link: Option, code: impl Into, description: Option, @@ -579,22 +593,23 @@ impl VariantAuthoringClient { VariantAuthoringError::SchemaVariantAssetNotFound(current_schema_variant_id), )?; - let name: String = content_name.into(); + let name: String = name.into(); let name = &name; current_schema .modify(ctx, |s| { - s.name = name.to_string(); + s.name = schema_name.into(); Ok(()) }) .await?; let variant_description = description.clone(); let variant_link = link.clone(); - let variant_display_name = menu_name.clone(); + let variant_display_name = display_name.clone(); current_schema_variant .modify(ctx, |sv| { + sv.name.clone_from(name); sv.description = variant_description; sv.link = variant_link; sv.category.clone_from(&category.into()); @@ -612,7 +627,7 @@ impl VariantAuthoringClient { func.name = name.to_string(); func.backend_kind = FuncBackendKind::JsSchemaVariantDefinition; func.backend_response_type = FuncBackendResponseType::SchemaVariantDefinition; - func.display_name = menu_name + func.display_name = display_name .clone() .map(|display_name| display_name.to_owned()); func.code_base64 = Some(code_base64); @@ -637,7 +652,7 @@ async fn build_variant_spec_based_on_existing_variant( let identity_func_spec = IntrinsicFunc::Identity.to_spec()?; let existing_variant = SchemaVariant::get_by_id(ctx, existing_variant_id).await?; - + let schema = existing_variant.schema(ctx).await?; let variant_spec = definition.to_spec( metadata.clone(), &identity_func_spec.unique_id, @@ -645,7 +660,7 @@ async fn build_variant_spec_based_on_existing_variant( )?; let (existing_variant_spec, variant_funcs) = - PkgExporter::export_variant_standalone(ctx, &existing_variant).await?; + PkgExporter::export_variant_standalone(ctx, &existing_variant, schema.name()).await?; let identity_name = IntrinsicFunc::Identity.name(); let identity_func = variant_funcs @@ -732,6 +747,7 @@ async fn execute_asset_func( #[allow(clippy::result_large_err)] fn build_pkg_spec_for_variant( + schema_name: &str, definition: SchemaVariantJson, asset_func_spec: &FuncSpec, metadata: &SchemaVariantMetadataJson, @@ -746,7 +762,7 @@ fn build_pkg_spec_for_variant( )?; let schema_spec = metadata.to_schema_spec(variant_spec)?; Ok(PkgSpec::builder() - .name(metadata.clone().schema_name) + .name(schema_name) .created_by(user_email) .func(identity_func_spec) .func(asset_func_spec.clone()) @@ -760,3 +776,12 @@ fn generate_scaffold_func_name(name: impl AsRef) -> String { let generated_name = format!("{}Scaffold_{}", name.as_ref().to_case(Case::Camel), version); generated_name } +/// temporarily increment the version so we can differentiate between schema variants when viewing attribute func bindings. This is a holdover until we start using versions for real. +fn increment_version(input: &str) -> String { + if let Some(stripped) = input.strip_prefix('v') { + if let Ok(num) = stripped.parse::() { + return format!("v{}", num + 1); + } + } + input.to_string() +} diff --git a/lib/dal/src/schema/variant/json.rs b/lib/dal/src/schema/variant/json.rs index b56629e33a..0a0a8506d7 100644 --- a/lib/dal/src/schema/variant/json.rs +++ b/lib/dal/src/schema/variant/json.rs @@ -14,12 +14,13 @@ use crate::{ComponentType, PropKind, SchemaVariantError, SocketArity}; #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct SchemaVariantMetadataJson { - /// Name for this variant. Actually, this is the name for this [`Schema`](crate::Schema), we're - /// punting on the issue of multiple variants for the moment. + /// Name for the schema. pub schema_name: String, - /// Override for the UI name for this schema - #[serde(alias = "menu_name")] - pub menu_name: Option, + /// Name for this variant. + pub name: String, + /// Override for the UI name for this schema variant + #[serde(alias = "menu_name", alias = "display_name")] + pub display_name: Option, /// The category this schema variant belongs to pub category: String, /// The color for the component on the component diagram as a hex string @@ -37,8 +38,8 @@ impl SchemaVariantMetadataJson { let mut data_builder = SchemaSpecData::builder(); data_builder.name(&self.schema_name); data_builder.category(&self.category); - if let Some(menu_name) = &self.menu_name { - data_builder.category_name(menu_name.as_str()); + if let Some(display_name) = &self.display_name { + data_builder.category_name(display_name.as_str()); } builder.data(data_builder.build()?); builder.variant(variant); @@ -88,12 +89,14 @@ impl SchemaVariantJson { asset_func_spec_unique_id: &str, ) -> SchemaVariantResult { let mut builder = SchemaVariantSpec::builder(); - let name = "v0"; - builder.name(name); + builder.name(metadata.name.clone()); let mut data_builder = SchemaVariantSpecData::builder(); - data_builder.name(name); + data_builder.name(metadata.name.clone()); + if let Some(display_name) = metadata.display_name { + data_builder.display_name(display_name); + } data_builder.color(metadata.color); data_builder.component_type(metadata.component_type); if let Some(link) = metadata.link { @@ -161,6 +164,7 @@ impl SchemaVariantJson { .to_owned() .unwrap_or(SchemaVariantSpecData { name: "v0".into(), + display_name: None, color: None, link: None, component_type: si_pkg::SchemaVariantSpecComponentType::Component, @@ -168,8 +172,9 @@ impl SchemaVariantJson { }); let metadata = SchemaVariantMetadataJson { + name: variant_spec_data.name, schema_name: schema_spec.name, - menu_name: schema_data.category_name, + display_name: schema_data.category_name, category: schema_data.category, color: variant_spec_data .color diff --git a/lib/dal/src/schema/variant/metadata_view.rs b/lib/dal/src/schema/variant/metadata_view.rs index d4e3066df1..940c08d694 100644 --- a/lib/dal/src/schema/variant/metadata_view.rs +++ b/lib/dal/src/schema/variant/metadata_view.rs @@ -10,6 +10,7 @@ use crate::{ pub struct SchemaVariantMetadataView { id: SchemaId, default_schema_variant_id: SchemaVariantId, + schema_name: String, name: String, category: String, #[serde(alias = "display_name")] @@ -33,7 +34,8 @@ impl SchemaVariantMetadataView { views.push(SchemaVariantMetadataView { id: schema.id, default_schema_variant_id: default_schema_variant.id, - name: schema.name.to_owned(), + schema_name: schema.name.to_owned(), + name: default_schema_variant.name.to_owned(), category: default_schema_variant.category.to_owned(), color: default_schema_variant.get_color(ctx).await?, timestamp: default_schema_variant.timestamp.to_owned(), diff --git a/lib/dal/src/user.rs b/lib/dal/src/user.rs index b3de77706b..73bc563c26 100644 --- a/lib/dal/src/user.rs +++ b/lib/dal/src/user.rs @@ -123,6 +123,11 @@ impl User { Ok(None) } } + pub async fn get_by_pk_or_error(ctx: &DalContext, pk: UserPk) -> UserResult { + Self::get_by_pk(ctx, pk) + .await? + .ok_or_else(|| UserError::NotFoundInTenancy(pk, *ctx.tenancy())) + } pub async fn authorize( ctx: &DalContext, diff --git a/lib/dal/tests/integration_test/pkg/mod.rs b/lib/dal/tests/integration_test/pkg/mod.rs index 9a0396e724..7374a616e6 100644 --- a/lib/dal/tests/integration_test/pkg/mod.rs +++ b/lib/dal/tests/integration_test/pkg/mod.rs @@ -40,9 +40,10 @@ async fn import_pkg_from_pkg_set_latest_default(ctx: &mut DalContext) { assert_eq!(default_schema_variant, Some(variant.id())); // now lets create a pkg from the asset and import it - let (variant_spec, variant_funcs) = PkgExporter::export_variant_standalone(ctx, &variant) - .await - .expect("should go to spec"); + let (variant_spec, variant_funcs) = + PkgExporter::export_variant_standalone(ctx, &variant, schema.name()) + .await + .expect("should go to spec"); let schema_spec = SchemaSpec::builder() .name(schema.name()) diff --git a/lib/dal/tests/integration_test/schema/variant/authoring/create_variant.rs b/lib/dal/tests/integration_test/schema/variant/authoring/create_variant.rs index b448ff4cbc..0c0adc466c 100644 --- a/lib/dal/tests/integration_test/schema/variant/authoring/create_variant.rs +++ b/lib/dal/tests/integration_test/schema/variant/authoring/create_variant.rs @@ -37,7 +37,9 @@ async fn create_variant(ctx: &mut DalContext) { assert_eq!(variant.category(), category.clone()); assert_eq!(new_schema.name(), asset_name.clone()); - assert_eq!(variant.display_name(), display_name.clone()); + // we update the display_name to match the schema name if display_name is none + + assert_eq!(variant.display_name(), Some(asset_name.clone())); assert_eq!( variant.get_color(ctx).await.expect("unable to get color"), color.clone() diff --git a/lib/dal/tests/integration_test/schema/variant/authoring/save_variant.rs b/lib/dal/tests/integration_test/schema/variant/authoring/save_variant.rs index a769a57fac..4f300e153b 100644 --- a/lib/dal/tests/integration_test/schema/variant/authoring/save_variant.rs +++ b/lib/dal/tests/integration_test/schema/variant/authoring/save_variant.rs @@ -37,7 +37,8 @@ async fn save_variant(ctx: &mut DalContext) { assert_eq!(variant.category(), category.clone()); assert_eq!(new_schema.name(), asset_name.clone()); - assert_eq!(variant.display_name(), display_name.clone()); + // we update the display_name to match the schema name if display_name is none + assert_eq!(variant.display_name(), Some(asset_name.clone())); assert_eq!( variant.get_color(ctx).await.expect("unable to get color"), color.clone() @@ -78,6 +79,7 @@ async fn save_variant(ctx: &mut DalContext) { ctx, variant.id(), updated_func_name.clone(), + updated_func_name.clone(), display_name.clone(), link.clone(), updated_func_content.clone(), diff --git a/lib/sdf-server/src/server/service/variant/clone_variant.rs b/lib/sdf-server/src/server/service/variant/clone_variant.rs index 429d1f8d91..957b054005 100644 --- a/lib/sdf-server/src/server/service/variant/clone_variant.rs +++ b/lib/sdf-server/src/server/service/variant/clone_variant.rs @@ -60,7 +60,7 @@ pub async fn clone_variant( serde_json::json!({ "variant_name": request.name, "variant_category": cloned_schema_variant.category(), - "variant_menu_name": cloned_schema_variant.display_name(), + "variant_display_name": cloned_schema_variant.display_name(), "variant_id": cloned_schema_variant.id(), "variant_component_type": cloned_schema_variant.component_type(), }), diff --git a/lib/sdf-server/src/server/service/variant/create_variant.rs b/lib/sdf-server/src/server/service/variant/create_variant.rs index 29ee1af6c2..f7739376fb 100644 --- a/lib/sdf-server/src/server/service/variant/create_variant.rs +++ b/lib/sdf-server/src/server/service/variant/create_variant.rs @@ -64,7 +64,7 @@ pub async fn create_variant( serde_json::json!({ "variant_name": request.name.clone(), "variant_category": request.category.clone(), - "variant_menu_name": request.display_name.clone(), + "variant_display_name": request.display_name.clone(), "variant_id": created_schema_variant.id().clone(), }), ); diff --git a/lib/sdf-server/src/server/service/variant/get_variant.rs b/lib/sdf-server/src/server/service/variant/get_variant.rs index f50f92e3a7..b61d7505f7 100644 --- a/lib/sdf-server/src/server/service/variant/get_variant.rs +++ b/lib/sdf-server/src/server/service/variant/get_variant.rs @@ -24,6 +24,7 @@ pub struct GetVariantRequest { pub struct GetVariantResponse { pub id: SchemaId, pub default_schema_variant_id: SchemaVariantId, + pub schema_name: String, pub name: String, pub display_name: Option, pub category: String, @@ -64,7 +65,8 @@ pub async fn get_variant( let mut response: GetVariantResponse = GetVariantResponse { id: request.id, default_schema_variant_id, - name: schema.name().into(), + schema_name: schema.name().into(), + name: variant.name().into(), display_name: variant.display_name(), category: variant.category().into(), color: variant.get_color(&ctx).await?, @@ -101,7 +103,7 @@ pub async fn get_variant( serde_json::json!({ "variant_name": variant.name(), "variant_category": variant.category(), - "variant_menu_name": variant.display_name(), + "variant_display_name": variant.display_name(), "variant_id": variant.id(), "schema_id": schema.id(), "variant_component_type": variant.component_type(), diff --git a/lib/sdf-server/src/server/service/variant/save_variant.rs b/lib/sdf-server/src/server/service/variant/save_variant.rs index 2de1ed7caa..2bb242b5c9 100644 --- a/lib/sdf-server/src/server/service/variant/save_variant.rs +++ b/lib/sdf-server/src/server/service/variant/save_variant.rs @@ -12,6 +12,7 @@ use serde::{Deserialize, Serialize}; pub struct SaveVariantRequest { pub id: SchemaId, pub default_schema_variant_id: SchemaVariantId, + pub schema_name: String, pub name: String, pub display_name: Option, pub category: String, @@ -44,6 +45,7 @@ pub async fn save_variant( VariantAuthoringClient::save_variant_content( &ctx, request.default_schema_variant_id, + request.schema_name.clone(), request.name.clone(), request.display_name.clone(), request.link.clone(), @@ -64,7 +66,7 @@ pub async fn save_variant( "variant_id": request.id, "variant_category": request.category, "variant_name": request.name, - "variant_menu_name": request.display_name, + "variant_display_name": request.display_name, }), ); diff --git a/lib/sdf-server/src/server/service/variant/update_variant.rs b/lib/sdf-server/src/server/service/variant/update_variant.rs index cf8702a589..6194837b75 100644 --- a/lib/sdf-server/src/server/service/variant/update_variant.rs +++ b/lib/sdf-server/src/server/service/variant/update_variant.rs @@ -59,7 +59,7 @@ pub async fn update_variant( serde_json::json!({ "variant_name": request.name.clone(), "variant_category": request.category.clone(), - "variant_menu_name": request.display_name.clone(), + "variant_display_name": request.display_name.clone(), "variant_id": updated_schema_variant_id, }), ); diff --git a/lib/si-pkg/src/spec/variant.rs b/lib/si-pkg/src/spec/variant.rs index 43184b1a5a..78abffbff1 100644 --- a/lib/si-pkg/src/spec/variant.rs +++ b/lib/si-pkg/src/spec/variant.rs @@ -112,7 +112,8 @@ pub struct SchemaVariantSpecData { pub link: Option, #[builder(setter(into, strip_option), default)] pub color: Option, - + #[builder(setter(into, strip_option), default)] + pub display_name: Option, #[builder(setter(into), default)] pub component_type: SchemaVariantSpecComponentType, #[builder(setter(into))]