fix: Make updated schemas variants take the display name from the schema

Signed-off-by: Victor Bustamante <[email protected]>

app/web/src/components/AssetDetailsPanel.vue

@@ -30,17 +30,17 @@
             @click="executeAsset"
           />
           <VButton
-            label="Clone"
-            tone="neutral"
             icon="clipboard-copy"
+            label="Clone"
             size="md"
+            tone="neutral"
             @click="() => cloneAssetModalRef?.modal?.open()"
           />
         </div>
         <AssetNameModal
           ref="cloneAssetModalRef"
-          title="Asset Name"
           buttonLabel="Clone Asset"
+          title="Asset Name"
           @submit="cloneAsset"
         />
 
@@ -73,29 +73,29 @@
         <VormInput
           id="schemaName"
           v-model="editingAsset.schemaName"
-          type="text"
-          label="Asset Name"
           compact
+          label="Asset Name"
           placeholder="(mandatory) Provide the asset a name"
+          type="text"
           @blur="updateAsset"
         />
         <VormInput
           id="name"
           v-model="editingAsset.name"
-          type="text"
-          label="Asset Version Name"
           compact
+          disabled
+          label="Asset Version Name"
           placeholder="(mandatory) Provide the asset version a name"
-          @blur="updateAsset"
+          type="text"
         />
 
         <VormInput
           id="displayName"
           v-model="editingAsset.displayName"
-          type="text"
-          label="Display name"
           compact
+          label="Display name"
           placeholder="(optional) Provide the asset version a display name"
+          type="text"
           @blur="updateAsset"
         />
         <VormInput

lib/dal/src/layer_db_types/content_types.rs

@@ -2,17 +2,28 @@ use chrono::{DateTime, Utc};
 use serde::{Deserialize, Serialize};
 use si_events::{CasValue, ContentHash};
 use strum::EnumDiscriminants;
+use thiserror::Error;
 
 use crate::action::prototype::ActionKind;
 use crate::validation::ValidationStatus;
 use crate::{
     action::ActionCompletionStatus, func::argument::FuncArgumentKind, prop::WidgetOptions,
     property_editor::schema::WidgetKind, socket::connection_annotation::ConnectionAnnotation,
-    ActionPrototypeId, ComponentId, ComponentType, FuncBackendKind, FuncBackendResponseType,
-    FuncId, PropId, PropKind, SocketArity, SocketKind, Timestamp, UserPk,
+    ActionPrototypeId, ComponentId, ComponentType, DalContext, FuncBackendKind,
+    FuncBackendResponseType, FuncId, PropId, PropKind, SchemaVariant, SchemaVariantId, SocketArity,
+    SocketKind, Timestamp, UserPk,
 };
 
-/// This type gathers up all the kinds of things we will store in the
+#[remain::sorted]
+#[derive(Error, Debug)]
+pub enum ContentTypeError {
+    #[error("error extracting schema variant content : {0}")]
+    SchemaVariantContent(String),
+}
+
+pub type ContentTypeResult<T> = Result<T, ContentTypeError>;
+
+/// This type gathers all the kinds of things we will store in the
 /// content-store portion of the layered database. Anything we want to read or
 /// write from there should be added here. Then the impl_into_content_types!
 /// macro should be used to provide from/into implementations between the inner
@@ -350,32 +361,49 @@ pub enum SchemaVariantContent {
 }
 
 impl SchemaVariantContent {
-    pub fn extract(self) -> SchemaVariantContentV2 {
+    pub async fn extract(
+        self,
+        ctx: &DalContext,
+        id: SchemaVariantId,
+    ) -> ContentTypeResult<SchemaVariantContentV2> {
         // update progressively
         let at_least_v2 = match self {
-            SchemaVariantContent::V1(v1) => SchemaVariantContent::V2(SchemaVariantContentV2 {
-                timestamp: v1.timestamp,
-                ui_hidden: v1.ui_hidden,
-                version: v1.name.to_owned(),
-                display_name: v1.display_name.unwrap_or(v1.name),
-                category: v1.category,
-                color: v1.color,
-                component_type: v1.component_type,
-                link: v1.link,
-                description: v1.description,
-                asset_func_id: v1.asset_func_id,
-                finalized_once: v1.finalized_once,
-                is_builtin: v1.is_builtin,
-                is_locked: true,
-            }),
+            SchemaVariantContent::V1(v1) => {
+                let display_name = if let Some(display_name) = v1.display_name {
+                    display_name
+                } else {
+                    let schema = SchemaVariant::schema_for_schema_variant_id(ctx, id)
+                        .await
+                        .map_err(|e| ContentTypeError::SchemaVariantContent(e.to_string()))?;
+                    schema.name
+                };
+
+                SchemaVariantContent::V2(SchemaVariantContentV2 {
+                    timestamp: v1.timestamp,
+                    ui_hidden: v1.ui_hidden,
+                    version: v1.name.to_owned(),
+                    display_name,
+                    category: v1.category,
+                    color: v1.color,
+                    component_type: v1.component_type,
+                    link: v1.link,
+                    description: v1.description,
+                    asset_func_id: v1.asset_func_id,
+                    finalized_once: v1.finalized_once,
+                    is_builtin: v1.is_builtin,
+                    is_locked: true,
+                })
+            }
             later => later,
         };
 
         // extract latest
-        match at_least_v2 {
+        let latest = match at_least_v2 {
             SchemaVariantContent::V2(v2) => v2,
             _ => unreachable!(),
-        }
+        };
+
+        Ok(latest)
     }
 }
 

lib/dal/src/pkg/import.rs

@@ -975,7 +975,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 schema_name = spec.name.clone();
                 let metadata = SchemaVariantJson::metadata_from_spec(spec)?;
 
                 let mut asset_func_id: Option<FuncId> = None;
@@ -997,7 +997,7 @@ pub(crate) async fn import_schema_variant(
                     ctx,
                     schema.id(),
                     variant_spec.name(),
-                    metadata.menu_name.unwrap_or(schema.name.to_string()),
+                    metadata.display_name.unwrap_or(schema_name),
                     metadata.category,
                     metadata.color,
                     metadata.component_type,

lib/dal/src/schema/variant.rs

@@ -21,7 +21,7 @@ use crate::func::argument::{FuncArgument, FuncArgumentError};
 use crate::func::intrinsics::IntrinsicFunc;
 use crate::func::{FuncError, FuncKind};
 use crate::layer_db_types::{
-    FuncContent, InputSocketContent, OutputSocketContent, SchemaVariantContent,
+    ContentTypeError, FuncContent, InputSocketContent, OutputSocketContent, SchemaVariantContent,
     SchemaVariantContentV2,
 };
 use crate::prop::{PropError, PropPath};
@@ -84,6 +84,8 @@ pub enum SchemaVariantError {
     ChangeSet(#[from] ChangeSetError),
     #[error("component error: {0}")]
     Component(#[from] Box<ComponentError>),
+    #[error("content error: {0}")]
+    ContentType(#[from] ContentTypeError),
     #[error("default schema variant not found for schema: {0}")]
     DefaultSchemaVariantNotFound(SchemaId),
     #[error("default variant not found: {0}")]
@@ -318,10 +320,14 @@ impl From<SchemaVariant> for SchemaVariantContent {
 }
 
 impl SchemaVariant {
-    pub fn assemble(id: SchemaVariantId, content: SchemaVariantContent) -> Self {
-        let inner = content.extract();
+    pub async fn assemble(
+        ctx: &DalContext,
+        id: SchemaVariantId,
+        content: SchemaVariantContent,
+    ) -> SchemaVariantResult<Self> {
+        let inner = content.extract(ctx, id).await?;
 
-        Self {
+        Ok(Self {
             id,
             timestamp: inner.timestamp,
             version: inner.version,
@@ -336,7 +342,7 @@ impl SchemaVariant {
             finalized_once: inner.finalized_once,
             is_builtin: inner.is_builtin,
             is_locked: inner.is_locked,
-        }
+        })
     }
 
     #[allow(clippy::too_many_arguments)]
@@ -396,7 +402,8 @@ impl SchemaVariant {
         let root_prop = RootProp::new(ctx, schema_variant_id).await?;
         let _func_id = Func::find_intrinsic(ctx, IntrinsicFunc::Identity).await?;
 
-        let schema_variant = Self::assemble(id.into(), SchemaVariantContent::V2(content));
+        let schema_variant =
+            Self::assemble(ctx, id.into(), SchemaVariantContent::V2(content)).await?;
         Ok((schema_variant, root_prop))
     }
 
@@ -508,7 +515,7 @@ impl SchemaVariant {
             .await?
             .ok_or(WorkspaceSnapshotError::MissingContentFromStore(id.into()))?;
 
-        Ok(Self::assemble(id, content))
+        Self::assemble(ctx, id, content).await
     }
 
     /// Lists all [`Components`](Component) that are using the provided [`SchemaVariantId`](SchemaVariant).
@@ -642,7 +649,8 @@ impl SchemaVariant {
         for node_weight in node_weights {
             match content_map.get(&node_weight.content_hash()) {
                 Some(content) => {
-                    schema_variants.push(Self::assemble(node_weight.id().into(), content.clone()));
+                    schema_variants
+                        .push(Self::assemble(ctx, node_weight.id().into(), content.clone()).await?);
                 }
                 None => Err(WorkspaceSnapshotError::MissingContentFromStore(
                     node_weight.id(),
@@ -973,7 +981,7 @@ impl SchemaVariant {
             .await?
             .ok_or(WorkspaceSnapshotError::MissingContentFromStore(id))?;
 
-        Ok((hash, content.extract()))
+        Ok((hash, content.extract(ctx, schema_variant_id).await?))
     }
 
     /// This _idempotent_ function "finalizes" a [`SchemaVariant`].

lib/dal/src/schema/variant/authoring.rs

@@ -179,7 +179,7 @@ impl VariantAuthoringClient {
     pub async fn clone_variant(
         ctx: &DalContext,
         schema_variant_id: SchemaVariantId,
-        name: String,
+        schema_name: String,
     ) -> VariantAuthoringResult<(SchemaVariant, Schema)> {
         let variant = SchemaVariant::get_by_id(ctx, schema_variant_id).await?;
         let schema = variant.schema(ctx).await?;
@@ -189,11 +189,11 @@ impl VariantAuthoringClient {
         if let Some(asset_func_id) = variant.asset_func_id() {
             let old_func = Func::get_by_id_or_error(ctx, asset_func_id).await?;
 
-            let cloned_func = old_func.duplicate(ctx, name.clone()).await?;
+            let cloned_func = old_func.duplicate(ctx, schema_name.clone()).await?;
             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.clone(),
+                schema_name: schema_name.clone(),
                 name: "v0".into(),
                 display_name: display_name.clone(),
                 category: variant.category().to_string(),
@@ -618,8 +618,9 @@ impl VariantAuthoringClient {
         let definition = execute_asset_func(ctx, &unlocked_asset_func).await?;
 
         let metadata = SchemaVariantMetadataJson {
+            schema_name: schema.name.clone(),
             name: "VERSION GOES HERE".to_string(),
-            menu_name: Some(format!("{}, unlocked", schema.name)),
+            display_name: Some(format!("{}, unlocked", locked_variant.display_name.clone())),
             category: locked_variant.category().to_string(),
             color: locked_variant.color().to_string(),
             component_type: locked_variant.component_type(),
@@ -637,7 +638,7 @@ impl VariantAuthoringClient {
             )
             .await?;
 
-        let schema_spec = metadata.to_spec(unlocked_variant_spec)?;
+        let schema_spec = metadata.to_schema_spec(unlocked_variant_spec)?;
 
         let creator_email = ctx.history_actor().email(ctx).await?;
         let pkg_spec = PkgSpec::builder()
@@ -689,7 +690,7 @@ impl VariantAuthoringClient {
         ctx: &DalContext,
         current_schema_variant_id: SchemaVariantId,
         schema_name: impl Into<String>,
-        name: impl Into<String>,
+        version: impl Into<String>,
         display_name: Option<String>,
         link: Option<String>,
         code: impl Into<String>,
@@ -707,7 +708,8 @@ impl VariantAuthoringClient {
             VariantAuthoringError::SchemaVariantAssetNotFound(current_schema_variant_id),
         )?;
 
-        let name: String = name.into();
+        // TODO rename this to version without breaking frontend
+        let name: String = version.into();
         let name = &name;
 
         current_schema
@@ -723,7 +725,7 @@ impl VariantAuthoringClient {
 
         current_schema_variant
             .modify(ctx, |sv| {
-                sv.name.clone_from(name);
+                sv.version.clone_from(name);
                 sv.description = variant_description;
                 sv.link = variant_link;
                 sv.category.clone_from(&category.into());

lib/dal/src/schema/variant/metadata_view.rs

@@ -11,6 +11,7 @@ pub struct SchemaVariantMetadataView {
     id: SchemaId,
     default_schema_variant_id: SchemaVariantId,
     schema_name: String,
+    // TODO rename this to version without breaking the frontend
     name: String,
     category: String,
     #[serde(alias = "display_name")]
@@ -35,7 +36,7 @@ impl SchemaVariantMetadataView {
                 id: schema.id,
                 default_schema_variant_id: default_schema_variant.id,
                 schema_name: schema.name.to_owned(),
-                name: default_schema_variant.name.to_owned(),
+                name: default_schema_variant.version.to_owned(),
                 category: default_schema_variant.category.to_owned(),
                 color: default_schema_variant.get_color(ctx).await?,
                 timestamp: default_schema_variant.timestamp.to_owned(),

lib/dal/src/user.rs

@@ -129,12 +129,6 @@ impl User {
             .ok_or_else(|| UserError::NotFoundInTenancy(pk, *ctx.tenancy()))
     }
 
-    pub async fn get_by_pk_or_error(ctx: &DalContext, pk: UserPk) -> UserResult<Self> {
-        Self::get_by_pk(ctx, pk)
-            .await?
-            .ok_or_else(|| UserError::NotFoundInTenancy(pk, *ctx.tenancy()))
-    }
-
     pub async fn authorize(
         ctx: &DalContext,
         user_pk: &UserPk,

lib/dal/tests/integration_test/schema/variant/authoring/clone_variant.rs

@@ -20,7 +20,7 @@ async fn clone_variant(ctx: &mut DalContext) {
         .get_default_schema_variant_id(ctx)
         .await
         .expect("Unable to find the default schema variant id");
-    let existing_schema = SchemaVariant::get_by_id(
+    let existing_variant = SchemaVariant::get_by_id(
         ctx,
         default_schema_variant.expect("unable to unwrap schema variant id"),
     )
@@ -32,12 +32,12 @@ async fn clone_variant(ctx: &mut DalContext) {
     let (new_schema_variant, _) = VariantAuthoringClient::clone_variant(
         ctx,
         default_schema_variant.expect("unable to get the schema variant id from the option"),
-        existing_schema.name().to_string(),
+        schema.name().to_string(),
     )
     .await
     .expect("unable to clone the schema variant");
 
-    assert_eq!(new_schema_variant.category(), existing_schema.category());
+    assert_eq!(new_schema_variant.category(), existing_variant.category());
     assert_eq!(
         new_schema_variant.display_name(),
         Some("dummy-secret Clone".to_string())

lib/sdf-server/src/server/service/variant/get_variant.rs

@@ -66,7 +66,7 @@ pub async fn get_variant(
         id: request.id,
         default_schema_variant_id,
         schema_name: schema.name().into(),
-        name: variant.name().into(),
+        name: variant.version().into(),
         display_name: variant.display_name(),
         category: variant.category().into(),
         color: variant.get_color(&ctx).await?,