code review comments

Author: luke-mino-altherr

src/locales/en/main.json

@@ -2078,6 +2078,7 @@
     "uploadModel": "Upload model",
     "uploadModelFromCivitai": "Upload a model from Civitai",
     "uploadModelFailedToRetrieveMetadata": "Failed to retrieve metadata. Please check the link and try again.",
+    "onlyCivitaiUrlsSupported": "Only Civitai URLs are supported",
     "uploadModelDescription1": "Paste a Civitai model download link to add it to your library.",
     "uploadModelDescription2": "Only links from https://civitai.com are supported at the moment",
     "uploadModelDescription3": "Max file size: 1 GB",

src/platform/assets/components/UploadModelDialog.vue

@@ -31,169 +31,55 @@
       :can-fetch-metadata="canFetchMetadata"
       :can-upload-model="canUploadModel"
       :upload-status="uploadStatus"
-      :on-back="goToPreviousStep"
-      :on-fetch-metadata="handleFetchMetadata"
-      :on-upload="handleUploadModel"
-      :on-close="handleClose"
+      @back="goToPreviousStep"
+      @fetch-metadata="handleFetchMetadata"
+      @upload="handleUploadModel"
+      @close="handleClose"
     />
   </div>
 </template>
 
 <script setup lang="ts">
-import { computed, onMounted, ref, watch } from 'vue'
+import { onMounted } from 'vue'
 
-import { st } from '@/i18n'
 import UploadModelConfirmation from '@/platform/assets/components/UploadModelConfirmation.vue'
 import UploadModelFooter from '@/platform/assets/components/UploadModelFooter.vue'
 import UploadModelProgress from '@/platform/assets/components/UploadModelProgress.vue'
 import UploadModelUrlInput from '@/platform/assets/components/UploadModelUrlInput.vue'
 import { useModelTypes } from '@/platform/assets/composables/useModelTypes'
-import type { AssetMetadata } from '@/platform/assets/schemas/assetSchema'
-import { assetService } from '@/platform/assets/services/assetService'
+import { useUploadModelWizard } from '@/platform/assets/composables/useUploadModelWizard'
 import { useDialogStore } from '@/stores/dialogStore'
 
 const dialogStore = useDialogStore()
+const { modelTypes, fetchModelTypes } = useModelTypes()
 
 const emit = defineEmits<{
   'upload-success': []
 }>()
 
-const currentStep = ref(1)
-const isFetchingMetadata = ref(false)
-const isUploading = ref(false)
-const uploadStatus = ref<'idle' | 'uploading' | 'success' | 'error'>('idle')
-const uploadError = ref('')
-
-const wizardData = ref<{
-  url: string
-  metadata: AssetMetadata | null
-  name: string
-  tags: string[]
-}>({
-  url: '',
-  metadata: null,
-  name: '',
-  tags: []
-})
-
-const selectedModelType = ref<string | undefined>(undefined)
-
-const { modelTypes, fetchModelTypes } = useModelTypes()
-
-// Clear error when URL changes
-watch(
-  () => wizardData.value.url,
-  () => {
-    uploadError.value = ''
-  }
-)
-
-// Validation
-const canFetchMetadata = computed(() => {
-  return wizardData.value.url.trim().length > 0
-})
-
-const canUploadModel = computed(() => {
-  return !!selectedModelType.value
-})
+const {
+  currentStep,
+  isFetchingMetadata,
+  isUploading,
+  uploadStatus,
+  uploadError,
+  wizardData,
+  selectedModelType,
+  canFetchMetadata,
+  canUploadModel,
+  fetchMetadata,
+  uploadModel,
+  goToPreviousStep
+} = useUploadModelWizard(modelTypes)
 
 async function handleFetchMetadata() {
-  if (!canFetchMetadata.value) return
-
-  // Validate that URL is from Civitai domain
-  const isCivitaiUrl = (url: string): boolean => {
-    try {
-      const hostname = new URL(url).hostname.toLowerCase()
-      return hostname === 'civitai.com' || hostname.endsWith('.civitai.com')
-    } catch {
-      return false
-    }
-  }
-
-  if (!isCivitaiUrl(wizardData.value.url)) {
-    uploadError.value = 'Only Civitai URLs are supported'
-    return
-  }
-
-  isFetchingMetadata.value = true
-  try {
-    const metadata = await assetService.getAssetMetadata(wizardData.value.url)
-    wizardData.value.metadata = metadata
-
-    // Pre-fill name from metadata
-    wizardData.value.name = metadata.filename || metadata.name || ''
-
-    // Pre-fill model type from metadata tags if available
-    if (metadata.tags && metadata.tags.length > 0) {
-      wizardData.value.tags = metadata.tags
-      // Try to detect model type from tags
-      const typeTag = metadata.tags.find((tag) =>
-        modelTypes.value.some((type) => type.value === tag)
-      )
-      if (typeTag) {
-        selectedModelType.value = typeTag
-      }
-    }
-
-    currentStep.value = 2
-  } catch (error) {
-    console.error('Failed to retrieve metadata:', error)
-    uploadError.value =
-      error instanceof Error
-        ? error.message
-        : st(
-            'assetBrowser.uploadModelFailedToRetrieveMetadata',
-            'Failed to retrieve metadata. Please check the link and try again.'
-          )
-    currentStep.value = 1
-  } finally {
-    isFetchingMetadata.value = false
-  }
+  await fetchMetadata()
 }
 
 async function handleUploadModel() {
-  if (!canUploadModel.value) return
-
-  isUploading.value = true
-  uploadStatus.value = 'uploading'
-
-  try {
-    const tags = selectedModelType.value
-      ? ['models', selectedModelType.value]
-      : ['models']
-    const filename =
-      wizardData.value.metadata?.filename ||
-      wizardData.value.metadata?.name ||
-      'model'
-
-    await assetService.uploadAssetFromUrl({
-      url: wizardData.value.url,
-      name: filename,
-      tags,
-      user_metadata: {
-        source: 'civitai',
-        source_url: wizardData.value.url,
-        model_type: selectedModelType.value
-      }
-    })
-
-    uploadStatus.value = 'success'
-    currentStep.value = 3
+  const success = await uploadModel()
+  if (success) {
     emit('upload-success')
-  } catch (error) {
-    console.error('Failed to upload asset:', error)
-    uploadStatus.value = 'error'
-    uploadError.value =
-      error instanceof Error ? error.message : 'Failed to upload model'
-    currentStep.value = 3
-  } finally {
-    isUploading.value = false
-  }
-}
-
-function goToPreviousStep() {
-  if (currentStep.value > 1) {
-    currentStep.value = currentStep.value - 1
   }
 }
 

src/platform/assets/components/UploadModelFooter.vue

@@ -6,7 +6,7 @@
       type="secondary"
       size="md"
       :disabled="isFetchingMetadata || isUploading"
-      :on-click="onBack"
+      @click="emit('back')"
     />
     <span v-else />
 
@@ -16,7 +16,7 @@
       type="primary"
       size="md"
       :disabled="!canFetchMetadata || isFetchingMetadata"
-      :on-click="onFetchMetadata"
+      @click="emit('fetchMetadata')"
     >
       <template #icon>
         <i
@@ -31,7 +31,7 @@
       type="primary"
       size="md"
       :disabled="!canUploadModel || isUploading"
-      :on-click="onUpload"
+      @click="emit('upload')"
     >
       <template #icon>
         <i
@@ -45,7 +45,7 @@
       :label="$t('assetBrowser.finish')"
       type="primary"
       size="md"
-      :on-click="onClose"
+      @click="emit('close')"
     />
   </div>
 </template>
@@ -61,9 +61,12 @@ defineProps<{
   canFetchMetadata: boolean
   canUploadModel: boolean
   uploadStatus: 'idle' | 'uploading' | 'success' | 'error'
-  onBack: () => void
-  onFetchMetadata: () => void
-  onUpload: () => void
-  onClose: () => void
+}>()
+
+const emit = defineEmits<{
+  (e: 'back'): void
+  (e: 'fetchMetadata'): void
+  (e: 'upload'): void
+  (e: 'close'): void
 }>()
 </script>

src/platform/assets/components/UploadModelProgress.vue

@@ -26,9 +26,7 @@
         </p>
       </div>
 
-      <div
-        class="flex flex-row items-start p-8 bg-node-component-surface rounded-lg"
-      >
+      <div class="flex flex-row items-start p-8 bg-neutral-800 rounded-lg">
         <div class="flex flex-col justify-center items-start gap-1 flex-1">
           <p class="text-sm m-0">
             {{ metadata?.name || metadata?.filename }}

src/platform/assets/composables/useModelTypes.ts

@@ -1,5 +1,4 @@
-import { ref } from 'vue'
-import { createSharedComposable } from '@vueuse/core'
+import { createSharedComposable, useAsyncState } from '@vueuse/core'
 
 import { api } from '@/scripts/api'
 
@@ -43,47 +42,27 @@ interface ModelTypeOption {
  * Uses shared state to ensure data is only fetched once
  */
 export const useModelTypes = createSharedComposable(() => {
-  const modelTypes = ref<ModelTypeOption[]>([])
-  const isLoading = ref(false)
-  const error = ref<string | null>(null)
-  let fetchPromise: Promise<void> | null = null
-
-  /**
-   * Fetch model types from the API (only fetches once, subsequent calls reuse the same promise)
-   */
-  async function fetchModelTypes() {
-    // If already loaded, return immediately
-    if (modelTypes.value.length > 0) {
-      return
-    }
-
-    // If currently loading, return the existing promise
-    if (fetchPromise) {
-      return fetchPromise
-    }
-
-    isLoading.value = true
-    error.value = null
-
-    fetchPromise = (async () => {
-      try {
-        const response = await api.getModelFolders()
-        modelTypes.value = response.map((folder) => ({
-          name: formatDisplayName(folder.name),
-          value: folder.name
-        }))
-      } catch (err) {
-        error.value =
-          err instanceof Error ? err.message : 'Failed to fetch model types'
+  const {
+    state: modelTypes,
+    isLoading,
+    error,
+    execute: fetchModelTypes
+  } = useAsyncState(
+    async (): Promise<ModelTypeOption[]> => {
+      const response = await api.getModelFolders()
+      return response.map((folder) => ({
+        name: formatDisplayName(folder.name),
+        value: folder.name
+      }))
+    },
+    [] as ModelTypeOption[],
+    {
+      immediate: false,
+      onError: (err) => {
         console.error('Failed to fetch model types:', err)
-      } finally {
-        isLoading.value = false
-        fetchPromise = null
       }
-    })()
-
-    return fetchPromise
-  }
+    }
+  )
 
   return {
     modelTypes,