feat: enhance model plugin workflow checks and model provider management UX (#33289)

Signed-off-by: yyh <yuanyouhuilyz@gmail.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: CodingOnStar <hanxujiang@dify.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Coding On Star <447357187@qq.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: -LAN- <laipz8200@outlook.com>
Co-authored-by: statxc <tyleradams93226@gmail.com>

web/app/components/plugins/update-plugin/__tests__/index.spec.tsx

@@ -104,36 +104,6 @@ vi.mock('../../install-plugin/install-from-github', () => ({
   ),
 }))
 
-// Mock Portal components for PluginVersionPicker
-let mockPortalOpen = false
-vi.mock('@/app/components/base/portal-to-follow-elem', () => ({
-  PortalToFollowElem: ({ children, open, onOpenChange: _onOpenChange }: {
-    children: React.ReactNode
-    open: boolean
-    onOpenChange: (open: boolean) => void
-  }) => {
-    mockPortalOpen = open
-    return <div data-testid="portal-elem" data-open={open}>{children}</div>
-  },
-  PortalToFollowElemTrigger: ({ children, onClick, className }: {
-    children: React.ReactNode
-    onClick: () => void
-    className?: string
-  }) => (
-    <div data-testid="portal-trigger" onClick={onClick} className={className}>
-      {children}
-    </div>
-  ),
-  PortalToFollowElemContent: ({ children, className }: {
-    children: React.ReactNode
-    className?: string
-  }) => {
-    if (!mockPortalOpen)
-      return null
-    return <div data-testid="portal-content" className={className}>{children}</div>
-  },
-}))
-
 // Mock semver
 vi.mock('semver', () => ({
   lt: (v1: string, v2: string) => {
@@ -247,7 +217,6 @@ const renderWithQueryClient = (ui: React.ReactElement) => {
 describe('update-plugin', () => {
   beforeEach(() => {
     vi.clearAllMocks()
-    mockPortalOpen = false
     mockCheck.mockResolvedValue({ status: TaskStatus.success })
   })
 
@@ -732,8 +701,8 @@ describe('update-plugin', () => {
         })
       })
 
-      it('should show error toast when task status is failed', async () => {
-        // Arrange - covers lines 99-100
+      it('should reset loading state when task status check fails', async () => {
+        // Arrange
         const mockToastNotify = vi.fn()
         vi.mocked(await import('../../../base/toast')).default.notify = mockToastNotify
 
@@ -770,6 +739,53 @@ describe('update-plugin', () => {
         })
         // onSave should NOT be called when task fails
         expect(onSave).not.toHaveBeenCalled()
+        await waitFor(() => {
+          expect(screen.getByRole('button', { name: 'plugin.upgrade.upgrade' })).toBeInTheDocument()
+        })
+        expect(screen.getByRole('button', { name: 'common.operation.cancel' })).toBeInTheDocument()
+      })
+
+      it('should stop loading when upgrade API returns failed task directly', async () => {
+        // Arrange
+        const mockToastNotify = vi.fn()
+        vi.mocked(await import('../../../base/toast')).default.notify = mockToastNotify
+
+        mockUpdateFromMarketPlace.mockResolvedValue({
+          task: {
+            status: TaskStatus.failed,
+            plugins: [{
+              plugin_unique_identifier: 'test-target-id',
+              status: TaskStatus.failed,
+              message: 'failed to init environment',
+            }],
+          },
+        })
+        const onSave = vi.fn()
+        const payload = createMockMarketPlacePayload()
+
+        // Act
+        renderWithQueryClient(
+          <UpdateFromMarketplace
+            payload={payload}
+            onSave={onSave}
+            onCancel={vi.fn()}
+          />,
+        )
+        fireEvent.click(screen.getByRole('button', { name: 'plugin.upgrade.upgrade' }))
+
+        // Assert
+        await waitFor(() => {
+          expect(mockToastNotify).toHaveBeenCalledWith({
+            type: 'error',
+            message: 'failed to init environment',
+          })
+        })
+        expect(mockCheck).not.toHaveBeenCalled()
+        expect(onSave).not.toHaveBeenCalled()
+        await waitFor(() => {
+          expect(screen.getByRole('button', { name: 'plugin.upgrade.upgrade' })).toBeInTheDocument()
+        })
+        expect(screen.getByRole('button', { name: 'common.operation.cancel' })).toBeInTheDocument()
       })
     })
 
@@ -946,7 +962,7 @@ describe('update-plugin', () => {
       onShowChange: vi.fn(),
       pluginID: 'test-plugin-id',
       currentVersion: '1.0.0',
-      trigger: <button>Select Version</button>,
+      trigger: <span>Select Version</span>,
       onSelect: vi.fn(),
     }
 
@@ -964,7 +980,7 @@ describe('update-plugin', () => {
         render(<PluginVersionPicker {...defaultProps} isShow={false} />)
 
         // Assert
-        expect(screen.queryByTestId('portal-content')).not.toBeInTheDocument()
+        expect(screen.queryByText('plugin.detailPanel.switchVersion')).not.toBeInTheDocument()
       })
 
       it('should render version list when isShow is true', () => {
@@ -972,7 +988,6 @@ describe('update-plugin', () => {
         render(<PluginVersionPicker {...defaultProps} isShow={true} />)
 
         // Assert
-        expect(screen.getByTestId('portal-content')).toBeInTheDocument()
         expect(screen.getByText('plugin.detailPanel.switchVersion')).toBeInTheDocument()
       })
 
@@ -1002,7 +1017,7 @@ describe('update-plugin', () => {
 
         // Act
         render(<PluginVersionPicker {...defaultProps} onShowChange={onShowChange} />)
-        fireEvent.click(screen.getByTestId('portal-trigger'))
+        fireEvent.click(screen.getByText('Select Version'))
 
         // Assert
         expect(onShowChange).toHaveBeenCalledWith(true)
@@ -1014,7 +1029,7 @@ describe('update-plugin', () => {
 
         // Act
         render(<PluginVersionPicker {...defaultProps} disabled={true} onShowChange={onShowChange} />)
-        fireEvent.click(screen.getByTestId('portal-trigger'))
+        fireEvent.click(screen.getByText('Select Version'))
 
         // Assert
         expect(onShowChange).not.toHaveBeenCalled()
@@ -1116,7 +1131,7 @@ describe('update-plugin', () => {
         )
 
         // Assert
-        expect(screen.getByTestId('portal-elem')).toBeInTheDocument()
+        expect(screen.getByText('plugin.detailPanel.switchVersion')).toBeInTheDocument()
       })
 
       it('should support custom offset', () => {
@@ -1125,12 +1140,13 @@ describe('update-plugin', () => {
           <PluginVersionPicker
             {...defaultProps}
             isShow={true}
-            offset={{ mainAxis: 10, crossAxis: 20 }}
+            sideOffset={10}
+            alignOffset={20}
           />,
         )
 
         // Assert
-        expect(screen.getByTestId('portal-elem')).toBeInTheDocument()
+        expect(screen.getByText('plugin.detailPanel.switchVersion')).toBeInTheDocument()
       })
     })
 
@@ -1190,7 +1206,7 @@ describe('update-plugin', () => {
           onShowChange: vi.fn(),
           pluginID: 'test',
           currentVersion: '1.0.0',
-          trigger: <button>Select</button>,
+          trigger: <span>Select</span>,
           onSelect: vi.fn(),
         }}
         />,

web/app/components/plugins/update-plugin/downgrade-warning.tsx

@@ -18,8 +18,8 @@ const DowngradeWarningModal = ({
   return (
     <>
       <div className="flex flex-col items-start gap-2 self-stretch">
-        <div className="title-2xl-semi-bold text-text-primary">{t(`${i18nPrefix}.title`, { ns: 'plugin' })}</div>
-        <div className="system-md-regular text-text-secondary">
+        <div className="text-text-primary title-2xl-semi-bold">{t(`${i18nPrefix}.title`, { ns: 'plugin' })}</div>
+        <div className="text-text-secondary system-md-regular">
           {t(`${i18nPrefix}.description`, { ns: 'plugin' })}
         </div>
       </div>

web/app/components/plugins/update-plugin/from-market-place.tsx

@@ -6,7 +6,12 @@ import { useCallback, useEffect, useMemo, useState } from 'react'
 import { useTranslation } from 'react-i18next'
 import Badge, { BadgeState } from '@/app/components/base/badge/index'
 import Button from '@/app/components/base/button'
-import Modal from '@/app/components/base/modal'
+import {
+  Dialog,
+  DialogCloseButton,
+  DialogContent,
+  DialogTitle,
+} from '@/app/components/base/ui/dialog'
 import Card from '@/app/components/plugins/card'
 import checkTaskStatus from '@/app/components/plugins/install-plugin/base/check-task-status'
 import { pluginManifestToCardPluginProps } from '@/app/components/plugins/install-plugin/utils'
@@ -28,6 +33,16 @@ type Props = {
   isShowDowngradeWarningModal?: boolean
 }
 
+type FailedUpgradeResponse = {
+  task?: {
+    status?: TaskStatus
+    plugins?: Array<{
+      plugin_unique_identifier: string
+      message: string
+    }>
+  }
+}
+
 enum UploadStep {
   notStarted = 'notStarted',
   upgrading = 'upgrading',
@@ -78,13 +93,26 @@ const UpdatePluginModal: FC<Props> = ({
     if (uploadStep === UploadStep.notStarted) {
       setUploadStep(UploadStep.upgrading)
       try {
+        const response = await updateFromMarketPlace({
+          original_plugin_unique_identifier: originalPackageInfo.id,
+          new_plugin_unique_identifier: targetPackageInfo.id,
+        }) as Awaited<ReturnType<typeof updateFromMarketPlace>> & FailedUpgradeResponse
+
+        if (response.task?.status === TaskStatus.failed) {
+          const failedPlugin = response.task.plugins?.find(plugin => plugin.plugin_unique_identifier === targetPackageInfo.id)
+            ?? response.task.plugins?.[0]
+          Toast.notify({
+            type: 'error',
+            message: failedPlugin?.message || t('error', { ns: 'common' }),
+          })
+          setUploadStep(UploadStep.notStarted)
+          return
+        }
+
         const {
           all_installed: isInstalled,
           task_id: taskId,
-        } = await updateFromMarketPlace({
-          original_plugin_unique_identifier: originalPackageInfo.id,
-          new_plugin_unique_identifier: targetPackageInfo.id,
-        })
+        } = response
 
         if (isInstalled) {
           onSave()
@@ -97,6 +125,7 @@ const UpdatePluginModal: FC<Props> = ({
         })
         if (status === TaskStatus.failed) {
           Toast.notify({ type: 'error', message: error! })
+          setUploadStep(UploadStep.notStarted)
           return
         }
         onSave()
@@ -109,7 +138,7 @@ const UpdatePluginModal: FC<Props> = ({
     }
     if (uploadStep === UploadStep.installed)
       onSave()
-  }, [onSave, uploadStep, check, originalPackageInfo.id, handleRefetch, targetPackageInfo.id])
+  }, [onSave, uploadStep, check, originalPackageInfo.id, handleRefetch, t, targetPackageInfo.id])
 
   const { mutateAsync } = useRemoveAutoUpgrade()
   const invalidateReferenceSettings = useInvalidateReferenceSettings()
@@ -125,63 +154,65 @@ const UpdatePluginModal: FC<Props> = ({
   const doShowDowngradeWarningModal = isShowDowngradeWarningModal && uploadStep === UploadStep.notStarted
 
   return (
-    <Modal
-      isShow={true}
-      onClose={onCancel}
-      className={cn('min-w-[560px]', doShowDowngradeWarningModal && 'min-w-[640px]')}
-      closable
-      title={!doShowDowngradeWarningModal && t(`${i18nPrefix}.${uploadStep === UploadStep.installed ? 'successfulTitle' : 'title'}`, { ns: 'plugin' })}
-    >
-      {doShowDowngradeWarningModal && (
-        <DowngradeWarningModal
-          onCancel={onCancel}
-          onJustDowngrade={handleConfirm}
-          onExcludeAndDowngrade={handleExcludeAndDownload}
-        />
-      )}
-      {!doShowDowngradeWarningModal && (
-        <>
-          <div className="system-md-regular mb-2 mt-3 text-text-secondary">
-            {t(`${i18nPrefix}.description`, { ns: 'plugin' })}
-          </div>
-          <div className="flex flex-wrap content-start items-start gap-1 self-stretch rounded-2xl bg-background-section-burn p-2">
-            <Card
-              installed={uploadStep === UploadStep.installed}
-              payload={pluginManifestToCardPluginProps({
-                ...originalPackageInfo.payload,
-                icon: icon!,
-              })}
-              className="w-full"
-              titleLeft={(
-                <>
-                  <Badge className="mx-1" size="s" state={BadgeState.Warning}>
-                    {`${originalPackageInfo.payload.version} -> ${targetPackageInfo.version}`}
-                  </Badge>
-                </>
+    <Dialog open onOpenChange={() => onCancel()}>
+      <DialogContent
+        backdropProps={{ forceRender: true }}
+        className={cn('min-w-[560px]', doShowDowngradeWarningModal && 'min-w-[640px]')}
+      >
+        <DialogCloseButton />
+        {doShowDowngradeWarningModal && (
+          <DowngradeWarningModal
+            onCancel={onCancel}
+            onJustDowngrade={handleConfirm}
+            onExcludeAndDowngrade={handleExcludeAndDownload}
+          />
+        )}
+        {!doShowDowngradeWarningModal && (
+          <>
+            <DialogTitle className="text-text-primary title-2xl-semi-bold">
+              {t(`${i18nPrefix}.${uploadStep === UploadStep.installed ? 'successfulTitle' : 'title'}`, { ns: 'plugin' })}
+            </DialogTitle>
+            <div className="mb-2 mt-3 text-text-secondary system-md-regular">
+              {t(`${i18nPrefix}.description`, { ns: 'plugin' })}
+            </div>
+            <div className="flex flex-wrap content-start items-start gap-1 self-stretch rounded-2xl bg-background-section-burn p-2">
+              <Card
+                installed={uploadStep === UploadStep.installed}
+                payload={pluginManifestToCardPluginProps({
+                  ...originalPackageInfo.payload,
+                  icon: icon!,
+                })}
+                className="w-full"
+                titleLeft={(
+                  <>
+                    <Badge className="mx-1" size="s" state={BadgeState.Warning}>
+                      {`${originalPackageInfo.payload.version} -> ${targetPackageInfo.version}`}
+                    </Badge>
+                  </>
+                )}
+              />
+            </div>
+            <div className="flex items-center justify-end gap-2 self-stretch pt-5">
+              {uploadStep === UploadStep.notStarted && (
+                <Button
+                  onClick={handleCancel}
+                >
+                  {t('operation.cancel', { ns: 'common' })}
+                </Button>
               )}
-            />
-          </div>
-          <div className="flex items-center justify-end gap-2 self-stretch pt-5">
-            {uploadStep === UploadStep.notStarted && (
               <Button
-                onClick={handleCancel}
+                variant="primary"
+                loading={uploadStep === UploadStep.upgrading}
+                onClick={handleConfirm}
+                disabled={uploadStep === UploadStep.upgrading}
               >
-                {t('operation.cancel', { ns: 'common' })}
+                {configBtnText}
               </Button>
-            )}
-            <Button
-              variant="primary"
-              loading={uploadStep === UploadStep.upgrading}
-              onClick={handleConfirm}
-              disabled={uploadStep === UploadStep.upgrading}
-            >
-              {configBtnText}
-            </Button>
-          </div>
-        </>
-      )}
-
-    </Modal>
+            </div>
+          </>
+        )}
+      </DialogContent>
+    </Dialog>
   )
 }
 export default React.memo(UpdatePluginModal)

web/app/components/plugins/update-plugin/plugin-version-picker.tsx

@@ -1,19 +1,16 @@
 'use client'
-import type {
-  OffsetOptions,
-  Placement,
-} from '@floating-ui/react'
 import type { FC } from 'react'
+import type { Placement } from '@/app/components/base/ui/placement'
 import * as React from 'react'
 import { useCallback } from 'react'
 import { useTranslation } from 'react-i18next'
 import { lt } from 'semver'
 import Badge from '@/app/components/base/badge'
 import {
-  PortalToFollowElem,
-  PortalToFollowElemContent,
-  PortalToFollowElemTrigger,
-} from '@/app/components/base/portal-to-follow-elem'
+  Popover,
+  PopoverContent,
+  PopoverTrigger,
+} from '@/app/components/base/ui/popover'
 import useTimestamp from '@/hooks/use-timestamp'
 import { useVersionListOfPlugin } from '@/service/use-plugins'
 import { cn } from '@/utils/classnames'
@@ -26,7 +23,8 @@ type Props = {
   currentVersion: string
   trigger: React.ReactNode
   placement?: Placement
-  offset?: OffsetOptions
+  sideOffset?: number
+  alignOffset?: number
   onSelect: ({
     version,
     unique_identifier,
@@ -46,22 +44,14 @@ const PluginVersionPicker: FC<Props> = ({
   currentVersion,
   trigger,
   placement = 'bottom-start',
-  offset = {
-    mainAxis: 4,
-    crossAxis: -16,
-  },
+  sideOffset = 4,
+  alignOffset = -16,
   onSelect,
 }) => {
   const { t } = useTranslation()
   const format = t('dateTimeFormat', { ns: 'appLog' }).split(' ')[0]
   const { formatDate } = useTimestamp()
 
-  const handleTriggerClick = () => {
-    if (disabled)
-      return
-    onShowChange(true)
-  }
-
   const { data: res } = useVersionListOfPlugin(pluginID)
 
   const handleSelect = useCallback(({ version, unique_identifier, isDowngrade }: {
@@ -76,49 +66,53 @@ const PluginVersionPicker: FC<Props> = ({
   }, [currentVersion, onSelect, onShowChange])
 
   return (
-    <PortalToFollowElem
-      placement={placement}
-      offset={offset}
+    <Popover
       open={isShow}
-      onOpenChange={onShowChange}
+      onOpenChange={(open) => {
+        if (!disabled)
+          onShowChange(open)
+      }}
     >
-      <PortalToFollowElemTrigger
+      <PopoverTrigger
+        disabled={disabled}
         className={cn('inline-flex cursor-pointer items-center', disabled && 'cursor-default')}
-        onClick={handleTriggerClick}
       >
         {trigger}
-      </PortalToFollowElemTrigger>
+      </PopoverTrigger>
 
-      <PortalToFollowElemContent className="z-[1000]">
-        <div className="relative w-[209px] rounded-xl border-[0.5px] border-components-panel-border bg-components-panel-bg-blur p-1 shadow-lg backdrop-blur-sm">
-          <div className="system-xs-medium-uppercase px-3 pb-0.5 pt-1 text-text-tertiary">
-            {t('detailPanel.switchVersion', { ns: 'plugin' })}
-          </div>
-          <div className="relative">
-            {res?.data.versions.map(version => (
-              <div
-                key={version.unique_identifier}
-                className={cn(
-                  'flex h-7 cursor-pointer items-center gap-1 rounded-lg px-3 py-1 hover:bg-state-base-hover',
-                  currentVersion === version.version && 'cursor-default opacity-30 hover:bg-transparent',
-                )}
-                onClick={() => handleSelect({
-                  version: version.version,
-                  unique_identifier: version.unique_identifier,
-                  isDowngrade: lt(version.version, currentVersion),
-                })}
-              >
-                <div className="flex grow items-center">
-                  <div className="system-sm-medium text-text-secondary">{version.version}</div>
-                  {currentVersion === version.version && <Badge className="ml-1" text="CURRENT" />}
-                </div>
-                <div className="system-xs-regular shrink-0 text-text-tertiary">{formatDate(version.created_at, format)}</div>
-              </div>
-            ))}
-          </div>
+      <PopoverContent
+        placement={placement}
+        sideOffset={sideOffset}
+        alignOffset={alignOffset}
+        popupClassName="relative w-[209px] bg-components-panel-bg-blur p-1 backdrop-blur-sm"
+      >
+        <div className="px-3 pb-0.5 pt-1 text-text-tertiary system-xs-medium-uppercase">
+          {t('detailPanel.switchVersion', { ns: 'plugin' })}
         </div>
-      </PortalToFollowElemContent>
-    </PortalToFollowElem>
+        <div className="relative max-h-[224px] overflow-y-auto">
+          {res?.data.versions.map(version => (
+            <div
+              key={version.unique_identifier}
+              className={cn(
+                'flex h-7 cursor-pointer items-center gap-1 rounded-lg px-3 py-1 hover:bg-state-base-hover',
+                currentVersion === version.version && 'cursor-default opacity-30 hover:bg-transparent',
+              )}
+              onClick={() => handleSelect({
+                version: version.version,
+                unique_identifier: version.unique_identifier,
+                isDowngrade: lt(version.version, currentVersion),
+              })}
+            >
+              <div className="flex grow items-center">
+                <div className="text-text-secondary system-sm-medium">{version.version}</div>
+                {currentVersion === version.version && <Badge className="ml-1" text="CURRENT" />}
+              </div>
+              <div className="shrink-0 text-text-tertiary system-xs-regular">{formatDate(version.created_at, format)}</div>
+            </div>
+          ))}
+        </div>
+      </PopoverContent>
+    </Popover>
   )
 }