Fix custom nodes license parsing logic (#2975)

src/components/dialog/content/manager/infoPanel/tabs/DescriptionTabPanel.vue

@@ -18,22 +18,14 @@ import InfoTextSection, {
   type TextSection
 } from '@/components/dialog/content/manager/infoPanel/InfoTextSection.vue'
 import { components } from '@/types/comfyRegistryTypes'
+import { isValidUrl } from '@/utils/formatUtil'
 
 const { t } = useI18n()
 
-const props = defineProps<{
+const { nodePack } = defineProps<{
   nodePack: components['schemas']['Node']
 }>()
 
-const isValidUrl = (url: string): boolean => {
-  try {
-    new URL(url)
-    return true
-  } catch {
-    return false
-  }
-}
-
 const isLicenseFile = (filename: string): boolean => {
   // Match LICENSE, LICENSE.md, LICENSE.txt (case insensitive)
   const licensePattern = /^license(\.md|\.txt)?$/i
@@ -41,7 +33,6 @@ const isLicenseFile = (filename: string): boolean => {
 }
 
 const extractBaseRepoUrl = (repoUrl: string): string => {
-  // Match GitHub repository URL and extract the base URL
   const githubRepoPattern = /^(https?:\/\/github\.com\/[^/]+\/[^/]+)/i
   const match = repoUrl.match(githubRepoPattern)
   return match ? match[1] : repoUrl
@@ -50,60 +41,65 @@ const extractBaseRepoUrl = (repoUrl: string): string => {
 const createLicenseUrl = (filename: string, repoUrl: string): string => {
   if (!repoUrl || !filename) return ''
 
-  // Use the filename if it's a license file, otherwise use LICENSE
   const licenseFile = isLicenseFile(filename) ? filename : 'LICENSE'
-
-  // Get the base repository URL
   const baseRepoUrl = extractBaseRepoUrl(repoUrl)
-
   return `${baseRepoUrl}/blob/main/${licenseFile}`
 }
 
 const parseLicenseObject = (
   licenseObj: any
 ): { text: string; isUrl: boolean } => {
-  // Get the license file or text
   const licenseFile = licenseObj.file || licenseObj.text
 
-  // If it's a string and a license file, create a URL
-  if (typeof licenseFile === 'string' && isLicenseFile(licenseFile)) {
-    const url = createLicenseUrl(licenseFile, props.nodePack.repository)
+  if (
+    typeof licenseFile === 'string' &&
+    isLicenseFile(licenseFile) &&
+    nodePack.repository
+  ) {
+    const url = createLicenseUrl(licenseFile, nodePack.repository)
     return {
       text: url,
       isUrl: !!url && isValidUrl(url)
     }
-  }
-  // Otherwise use the text directly
-  else if (licenseObj.text) {
+  } else if (licenseObj.text) {
     return {
       text: licenseObj.text,
       isUrl: false
     }
+  } else if (typeof licenseFile === 'string') {
+    // Return the license file name if repository is missing
+    return {
+      text: licenseFile,
+      isUrl: false
+    }
   }
-
-  // Default fallback
   return {
     text: JSON.stringify(licenseObj),
     isUrl: false
   }
 }
 
-const formatLicense = (license: string): { text: string; isUrl: boolean } => {
+const formatLicense = (
+  license: string
+): { text: string; isUrl: boolean } | null => {
+  // Treat "{}" JSON string as undefined
+  if (license === '{}') return null
+
   try {
     const licenseObj = JSON.parse(license)
+    // Handle empty object case
+    if (Object.keys(licenseObj).length === 0) {
+      return null
+    }
     return parseLicenseObject(licenseObj)
   } catch (e) {
-    // Not JSON, handle as plain string
-    // If it's a license file, create a URL
-    if (isLicenseFile(license)) {
-      const url = createLicenseUrl(license, props.nodePack.repository)
+    if (isLicenseFile(license) && nodePack.repository) {
+      const url = createLicenseUrl(license, nodePack.repository)
       return {
         text: url,
         isUrl: !!url && isValidUrl(url)
       }
     }
-
-    // Otherwise return as is
     return {
       text: license,
       isUrl: false
@@ -115,25 +111,25 @@ const descriptionSections = computed<TextSection[]>(() => {
   const sections: TextSection[] = [
     {
       title: t('g.description'),
-      text: props.nodePack.description || t('manager.noDescription')
+      text: nodePack.description || t('manager.noDescription')
     }
   ]
 
-  if (props.nodePack.repository) {
+  if (nodePack.repository) {
     sections.push({
       title: t('manager.repository'),
-      text: props.nodePack.repository,
-      isUrl: isValidUrl(props.nodePack.repository)
+      text: nodePack.repository,
+      isUrl: isValidUrl(nodePack.repository)
     })
   }
 
-  if (props.nodePack.license) {
-    const { text, isUrl } = formatLicense(props.nodePack.license)
-    if (text) {
+  if (nodePack.license) {
+    const licenseInfo = formatLicense(nodePack.license)
+    if (licenseInfo && licenseInfo.text) {
       sections.push({
         title: t('manager.license'),
-        text,
-        isUrl
+        text: licenseInfo.text,
+        isUrl: licenseInfo.isUrl
       })
     }
   }

src/components/dialog/content/manager/infoPanel/tabs/__tests__/DescriptionTabPanel.test.ts

@@ -0,0 +1,179 @@
+import { mount } from '@vue/test-utils'
+import { describe, expect, it } from 'vitest'
+import { createI18n } from 'vue-i18n'
+
+import enMessages from '@/locales/en/main.json'
+import { components } from '@/types/comfyRegistryTypes'
+
+import DescriptionTabPanel from '../DescriptionTabPanel.vue'
+
+const i18n = createI18n({
+  legacy: false,
+  locale: 'en',
+  messages: {
+    en: enMessages
+  }
+})
+
+const TRANSLATIONS = {
+  description: 'Description',
+  repository: 'Repository',
+  license: 'License',
+  noDescription: 'No description available'
+}
+
+describe('DescriptionTabPanel', () => {
+  const mountComponent = (props: {
+    nodePack: Partial<components['schemas']['Node']>
+  }) => {
+    return mount(DescriptionTabPanel, {
+      props,
+      global: {
+        plugins: [i18n]
+      }
+    })
+  }
+
+  const getSectionByTitle = (
+    wrapper: ReturnType<typeof mountComponent>,
+    title: string
+  ) => {
+    const sections = wrapper
+      .findComponent({ name: 'InfoTextSection' })
+      .props('sections')
+    return sections.find((s: any) => s.title === title)
+  }
+
+  const createNodePack = (
+    overrides: Partial<components['schemas']['Node']> = {}
+  ) => ({
+    description: 'Test description',
+    ...overrides
+  })
+
+  const licenseTests = [
+    {
+      name: 'handles plain text license',
+      nodePack: createNodePack({
+        license: 'MIT License',
+        repository: 'https://github.com/user/repo'
+      }),
+      expected: {
+        text: 'MIT License',
+        isUrl: false
+      }
+    },
+    {
+      name: 'handles license file names',
+      nodePack: createNodePack({
+        license: 'LICENSE',
+        repository: 'https://github.com/user/repo'
+      }),
+      expected: {
+        text: 'https://github.com/user/repo/blob/main/LICENSE',
+        isUrl: true
+      }
+    },
+    {
+      name: 'handles license.md file names',
+      nodePack: createNodePack({
+        license: 'license.md',
+        repository: 'https://github.com/user/repo'
+      }),
+      expected: {
+        text: 'https://github.com/user/repo/blob/main/license.md',
+        isUrl: true
+      }
+    },
+    {
+      name: 'handles JSON license objects with text property',
+      nodePack: createNodePack({
+        license: JSON.stringify({ text: 'GPL-3.0' }),
+        repository: 'https://github.com/user/repo'
+      }),
+      expected: {
+        text: 'GPL-3.0',
+        isUrl: false
+      }
+    },
+    {
+      name: 'handles JSON license objects with file property',
+      nodePack: createNodePack({
+        license: JSON.stringify({ file: 'LICENSE.md' }),
+        repository: 'https://github.com/user/repo'
+      }),
+      expected: {
+        text: 'https://github.com/user/repo/blob/main/LICENSE.md',
+        isUrl: true
+      }
+    },
+    {
+      name: 'handles missing repository URL',
+      nodePack: createNodePack({
+        license: 'LICENSE'
+      }),
+      expected: {
+        text: 'LICENSE',
+        isUrl: false
+      }
+    },
+    {
+      name: 'handles non-GitHub repository URLs',
+      nodePack: createNodePack({
+        license: 'LICENSE',
+        repository: 'https://gitlab.com/user/repo'
+      }),
+      expected: {
+        text: 'https://gitlab.com/user/repo/blob/main/LICENSE',
+        isUrl: true
+      }
+    }
+  ]
+
+  describe('license formatting', () => {
+    licenseTests.forEach((test) => {
+      it(test.name, () => {
+        const wrapper = mountComponent({ nodePack: test.nodePack })
+        const licenseSection = getSectionByTitle(wrapper, TRANSLATIONS.license)
+        expect(licenseSection).toBeDefined()
+        expect(licenseSection.text).toBe(test.expected.text)
+        expect(licenseSection.isUrl).toBe(test.expected.isUrl)
+      })
+    })
+  })
+
+  describe('description sections', () => {
+    it('shows description section', () => {
+      const wrapper = mountComponent({
+        nodePack: createNodePack()
+      })
+      const descriptionSection = getSectionByTitle(
+        wrapper,
+        TRANSLATIONS.description
+      )
+      expect(descriptionSection).toBeDefined()
+      expect(descriptionSection.text).toBe('Test description')
+    })
+
+    it('shows repository section when available', () => {
+      const wrapper = mountComponent({
+        nodePack: createNodePack({
+          repository: 'https://github.com/user/repo'
+        })
+      })
+      const repoSection = getSectionByTitle(wrapper, TRANSLATIONS.repository)
+      expect(repoSection).toBeDefined()
+      expect(repoSection.text).toBe('https://github.com/user/repo')
+      expect(repoSection.isUrl).toBe(true)
+    })
+
+    it('shows fallback text when description is missing', () => {
+      const wrapper = mountComponent({
+        nodePack: {
+          description: undefined
+        }
+      })
+      expect(wrapper.find('p').text()).toBe(TRANSLATIONS.noDescription)
+    })
+  })
+})