From 630c83bebc5cd9f579b6907f227d8f660a428bb0 Mon Sep 17 00:00:00 2001
From: Victor Bustamante <victor@systeminit.com>
Date: Wed, 3 Jul 2024 19:49:23 -0300
Subject: [PATCH] fix(sdf, web): Don't error out UI on asset creation

Also show request status on asset creation modal

Signed-off-by: Victor Bustamante <victor@systeminit.com>
---
 app/web/src/components/AssetListPanel.vue     | 39 ++++++++++---------
 app/web/src/components/AssetNameModal.vue     | 23 +++++++----
 app/web/src/store/asset.store.ts              |  4 +-
 lib/dal/src/schema/variant.rs                 | 12 +++++-
 .../server/service/variant/create_variant.rs  |  9 +++--
 5 files changed, 56 insertions(+), 31 deletions(-)

diff --git a/app/web/src/components/AssetListPanel.vue b/app/web/src/components/AssetListPanel.vue
index 0745519cb4..1a388abced 100644
--- a/app/web/src/components/AssetListPanel.vue
+++ b/app/web/src/components/AssetListPanel.vue
@@ -15,26 +15,26 @@
         </template>
         <div class="flex flex-row gap-xs items-center">
           <IconButton
+            :requestStatus="createAssetReqStatus"
             class="hover:scale-125"
             icon="plus"
-            size="sm"
             loadingIcon="loader"
-            :requestStatus="createAssetReqStatus"
-            variant="simple"
+            loadingTooltip="Creating Asset..."
+            size="sm"
             tooltip="New Asset"
             tooltipPlacement="top"
-            loadingTooltip="Creating Asset..."
+            variant="simple"
             @click="() => newAssetModalRef?.modal?.open()"
           />
           <IconButton
             v-if="canContribute || true"
+            :selected="contributeAssetModalRef?.isOpen || false"
             class="hover:scale-125"
             icon="cloud-upload"
             size="sm"
-            variant="simple"
             tooltip="Contribute All"
             tooltipPlacement="top"
-            :selected="contributeAssetModalRef?.isOpen || false"
+            variant="simple"
             @click="contributeAsset"
           />
           <IconButton
@@ -42,22 +42,23 @@
             class="hover:scale-125"
             icon="code-deployed"
             size="sm"
-            variant="simple"
             tooltip="Update All"
             tooltipPlacement="top"
+            variant="simple"
           />
         </div>
         <AssetNameModal
           ref="newAssetModalRef"
-          title="New Asset"
+          :loading="createAssetReqStatus.isPending"
           buttonLabel="Create Asset"
+          title="New Asset"
           @submit="(name) => newAsset(name)"
         />
       </SidebarSubpanelTitle>
       <SiSearch
         ref="searchRef"
-        placeholder="search assets"
         :filters="searchFiltersWithCounts"
+        placeholder="search assets"
         @search="onSearch"
       />
       <!-- <div
@@ -70,13 +71,13 @@
       <TreeNode
         v-for="category in Object.keys(categorizedAssets)"
         :key="category"
+        :color="categoryColor(category)"
         :label="category"
         :primaryIcon="getAssetIcon(category)"
-        :color="categoryColor(category)"
-        enableDefaultHoverClasses
-        enableGroupToggle
         alwaysShowArrow
         clickLabelToToggle
+        enableDefaultHoverClasses
+        enableGroupToggle
         indentationSize="none"
       >
         <template #icons>
@@ -95,12 +96,12 @@
     </template>
     <ModuleExportModal
       ref="contributeAssetModalRef"
-      title="Contribute Assets"
-      label="Contribute to System Initiative"
       :loadingText="_.sample(contributeLoadingTexts)"
       :preSelectedSchemaVariantId="
         assetStore.selectedSchemaVariant?.schemaVariantId
       "
+      label="Contribute to System Initiative"
+      title="Contribute Assets"
       @export-success="onExport"
     />
     <Modal ref="exportSuccessModalRef" size="sm" title="Contribution sent">
@@ -232,13 +233,15 @@ const categoryColor = (category: string) => {
 const newAsset = async (newAssetName: string) => {
   const result = await assetStore.CREATE_VARIANT(newAssetName);
   if (result.result.success) {
-    assetStore.setSchemaVariantSelection(result.result.data.id, true);
+    assetStore.setSchemaVariantSelection(
+      result.result.data.schemaVariantId,
+      true,
+    );
     newAssetModalRef.value?.modal?.close();
   } else if (result.result.statusCode === 409) {
-    if (newAssetModalRef.value) {
-      newAssetModalRef.value.setError("That name is already in use");
-    }
+    newAssetModalRef.value?.setError("That name is already in use");
   }
+  newAssetModalRef.value?.reset();
 };
 
 const contributeAsset = () => contributeAssetModalRef.value?.open();
diff --git a/app/web/src/components/AssetNameModal.vue b/app/web/src/components/AssetNameModal.vue
index 55f7e474a0..d2cd2027d7 100644
--- a/app/web/src/components/AssetNameModal.vue
+++ b/app/web/src/components/AssetNameModal.vue
@@ -1,15 +1,18 @@
 <template>
-  <Modal ref="modal" size="sm" :title="props.title">
+  <Modal ref="modal" :title="props.title" size="sm">
     <VormInput
       ref="assetNameVorm"
       v-model="assetName"
-      type="text"
+      :disabled="loading"
       label="Asset Name"
       noLabel
       required
+      type="text"
       @enterPressed="submit"
     />
-    <VButton class="mt-sm" @click="submit">{{ props.buttonLabel }}</VButton>
+    <VButton :loading="loading" class="mt-sm" @click="submit"
+      >{{ props.buttonLabel }}
+    </VButton>
   </Modal>
 </template>
 
@@ -20,6 +23,7 @@ import { Modal, VormInput, VButton } from "@si/vue-lib/design-system";
 const props = defineProps<{
   title: string;
   buttonLabel: string;
+  loading?: boolean;
 }>();
 
 const modal = ref<InstanceType<typeof Modal>>();
@@ -27,19 +31,24 @@ const assetName = ref("");
 const assetNameVorm = ref<InstanceType<typeof VormInput>>();
 
 const submit = () => {
-  emit("submit", assetName.value);
+  if (!assetNameVorm.value?.validationState.isError) {
+    emit("submit", assetName.value);
+  }
+};
+
+const reset = () => {
   assetName.value = "";
+  assetNameVorm.value?.validationMethods.reset();
 };
 
 const setError = (msg: string) => {
-  if (assetNameVorm.value) {
-    assetNameVorm.value.setError(msg);
-  }
+  assetNameVorm.value?.setError(msg);
 };
 
 defineExpose({
   modal,
   setError,
+  reset,
 });
 const emit = defineEmits(["submit"]);
 </script>
diff --git a/app/web/src/store/asset.store.ts b/app/web/src/store/asset.store.ts
index 0fa2d606d6..b4035c4139 100644
--- a/app/web/src/store/asset.store.ts
+++ b/app/web/src/store/asset.store.ts
@@ -4,7 +4,7 @@ import * as _ from "lodash-es";
 import { addStoreHooks, ApiRequest } from "@si/vue-lib/pinia";
 import { useWorkspacesStore } from "@/store/workspaces.store";
 import { FuncKind, FuncId } from "@/api/sdf/dal/func";
-import { SchemaVariant, SchemaVariantId } from "@/api/sdf/dal/schema";
+import { SchemaId, SchemaVariant, SchemaVariantId } from "@/api/sdf/dal/schema";
 import { Visibility } from "@/api/sdf/dal/visibility";
 import keyedDebouncer from "@/utils/keyedDebouncer";
 import router from "@/router";
@@ -250,7 +250,7 @@ export const useAssetStore = () => {
           if (changeSetId === changeSetsStore.headChangeSetId)
             changeSetsStore.creatingChangeSet = true;
           return new ApiRequest<
-            { id: SchemaVariantId; success: boolean },
+            { schemaId: SchemaId; schemaVariantId: SchemaVariantId },
             SchemaVariantCreateRequest
           >({
             method: "post",
diff --git a/lib/dal/src/schema/variant.rs b/lib/dal/src/schema/variant.rs
index c5e6981c6a..d5c9c31317 100644
--- a/lib/dal/src/schema/variant.rs
+++ b/lib/dal/src/schema/variant.rs
@@ -102,6 +102,8 @@ pub enum SchemaVariantError {
     FuncArgument(#[from] FuncArgumentError),
     #[error("helper error: {0}")]
     Helper(#[from] HelperError),
+    #[error("{0} exists, but is not a schema variant id")]
+    IdForWrongType(Ulid),
     #[error("input socket error: {0}")]
     InputSocket(#[from] InputSocketError),
     #[error("layer db error: {0}")]
@@ -574,8 +576,16 @@ impl SchemaVariant {
                     err.into()
                 }
             })?;
+
         let node_weight = workspace_snapshot.get_node_weight(node_index).await?;
-        let hash = node_weight.content_hash();
+
+        let NodeWeight::Content(content) = &node_weight else {
+            return Err(SchemaVariantError::IdForWrongType(id.into()));
+        };
+
+        let ContentAddress::SchemaVariant(hash) = content.content_address() else {
+            return Err(SchemaVariantError::IdForWrongType(id.into()));
+        };
 
         let content: SchemaVariantContent = ctx
             .layer_db()
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 f1df2fe607..cc4ee86105 100644
--- a/lib/sdf-server/src/server/service/variant/create_variant.rs
+++ b/lib/sdf-server/src/server/service/variant/create_variant.rs
@@ -3,7 +3,7 @@ use axum::{response::IntoResponse, Json};
 use serde::{Deserialize, Serialize};
 
 use dal::schema::variant::authoring::VariantAuthoringClient;
-use dal::{ChangeSet, Schema, SchemaId, Visibility, WsEvent};
+use dal::{ChangeSet, Schema, SchemaId, SchemaVariantId, Visibility, WsEvent};
 
 use crate::server::extract::{AccessBuilder, HandlerContext, PosthogClient};
 use crate::server::tracking::track;
@@ -21,7 +21,8 @@ pub struct CreateVariantRequest {
 #[derive(Deserialize, Serialize, Debug)]
 #[serde(rename_all = "camelCase")]
 pub struct CreateVariantResponse {
-    pub id: SchemaId,
+    pub schema_id: SchemaId,
+    pub schema_variant_id: SchemaVariantId,
 }
 
 pub async fn create_variant(
@@ -51,6 +52,7 @@ pub async fn create_variant(
     )
     .await?;
 
+    let schema_variant_id = created_schema_variant.id;
     let schema = created_schema_variant.schema(&ctx).await?;
 
     track(
@@ -78,6 +80,7 @@ pub async fn create_variant(
         response = response.header("force_change_set_id", force_change_set_id.to_string());
     }
     Ok(response.body(serde_json::to_string(&CreateVariantResponse {
-        id: schema.id(),
+        schema_id: schema.id(),
+        schema_variant_id,
     })?)?)
 }