fix: address CodeRabbit review comments

- Remove data URL content from error messages (security)
- Replace 'as any' casts with proper type assertions in tests
- Use uploadMediaBatch in WidgetSelectDropdown for single store update
- Localize error strings with i18n (uploadFailed, tempUploadFailed)

Amp-Thread-ID: https://ampcode.com/threads/T-019c0daa-eb18-72eb-bc87-90f09afc3d3a
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
bymyself
2026-01-29 22:59:34 -08:00
parent 2a8cc540f7
commit 226fc7e840
5 changed files with 44 additions and 27 deletions

View File

@@ -42,7 +42,9 @@ class Load3dUtils {
)
if (!result.success || !result.response) {
const err = `Error uploading temp file: ${result.error}`
const err = t('toastMessages.tempUploadFailed', {
error: result.error || ''
})
useToastStore().addAlert(err)
throw new Error(err)
}

View File

@@ -1974,6 +1974,8 @@
"pleaseSelectNodesToGroup": "Please select the nodes (or other groups) to create a group for",
"emptyCanvas": "Empty canvas",
"fileUploadFailed": "File upload failed",
"uploadFailed": "Upload failed",
"tempUploadFailed": "Error uploading temp file: {error}",
"fileTooLarge": "File too large ({size} MB). Maximum supported size is {maxSize} MB",
"unableToGetModelFilePath": "Unable to get model file path",
"couldNotDetermineFileType": "Could not determine file type",

View File

@@ -26,7 +26,9 @@ describe('uploadService', () => {
})
}
vi.mocked(api.fetchApi).mockResolvedValue(mockResponse as any)
vi.mocked(api.fetchApi).mockResolvedValue(
mockResponse as Partial<Response> as Response
)
const result = await uploadMedia({ source: mockFile })
@@ -46,7 +48,9 @@ describe('uploadService', () => {
})
}
vi.mocked(api.fetchApi).mockResolvedValue(mockResponse as any)
vi.mocked(api.fetchApi).mockResolvedValue(
mockResponse as Partial<Response> as Response
)
const result = await uploadMedia({ source: mockBlob })
@@ -68,7 +72,9 @@ describe('uploadService', () => {
})
}
vi.mocked(api.fetchApi).mockResolvedValue(mockResponse as any)
vi.mocked(api.fetchApi).mockResolvedValue(
mockResponse as Partial<Response> as Response
)
try {
const result = await uploadMedia({ source: dataURL })
@@ -94,7 +100,9 @@ describe('uploadService', () => {
json: vi.fn().mockResolvedValue({ name: 'test.png' })
}
vi.mocked(api.fetchApi).mockResolvedValue(mockResponse as any)
vi.mocked(api.fetchApi).mockResolvedValue(
mockResponse as Partial<Response> as Response
)
await uploadMedia(
{ source: mockFile },
@@ -131,7 +139,9 @@ describe('uploadService', () => {
statusText: 'Internal Server Error'
}
vi.mocked(api.fetchApi).mockResolvedValue(mockResponse as any)
vi.mocked(api.fetchApi).mockResolvedValue(
mockResponse as Partial<Response> as Response
)
const result = await uploadMedia({ source: mockFile })
@@ -157,7 +167,9 @@ describe('uploadService', () => {
json: vi.fn().mockResolvedValue({ name: 'mask.png' })
}
vi.mocked(api.fetchApi).mockResolvedValue(mockResponse as any)
vi.mocked(api.fetchApi).mockResolvedValue(
mockResponse as Partial<Response> as Response
)
const originalRef = {
filename: 'original.png',
@@ -194,8 +206,8 @@ describe('uploadService', () => {
}
vi.mocked(api.fetchApi)
.mockResolvedValueOnce(mockResponse1 as any)
.mockResolvedValueOnce(mockResponse2 as any)
.mockResolvedValueOnce(mockResponse1 as Partial<Response> as Response)
.mockResolvedValueOnce(mockResponse2 as Partial<Response> as Response)
const results = await uploadMediaBatch(
mockFiles.map((source) => ({ source }))

View File

@@ -51,7 +51,7 @@ async function convertToFile(
// dataURL string
if (!isDataURL(source)) {
throw new Error(`Invalid data URL: ${source.substring(0, 50)}...`)
throw new Error('Invalid data URL')
}
try {

View File

@@ -8,7 +8,7 @@ import { SUPPORTED_EXTENSIONS_ACCEPT } from '@/extensions/core/load3d/constants'
import { t } from '@/i18n'
import { useAssetFilterOptions } from '@/platform/assets/composables/useAssetFilterOptions'
import { useMediaAssets } from '@/platform/assets/composables/media/useMediaAssets'
import { uploadMedia } from '@/platform/assets/services/uploadService'
import { uploadMediaBatch } from '@/platform/assets/services/uploadService'
import {
filterItemByBaseModels,
filterItemByOwnership
@@ -375,29 +375,30 @@ function updateSelectedItems(selectedItems: Set<string>) {
useWorkflowStore().activeWorkflow?.changeTracker?.checkState()
}
// Handle multiple file uploads using shared uploadMedia service
// Handle multiple file uploads using shared uploadMediaBatch service
const uploadFiles = async (files: File[]): Promise<string[]> => {
const folder = props.uploadFolder ?? 'input'
const assetsStore = useAssetsStore()
const uploadPromises = files.map(async (file) => {
const result = await uploadMedia({ source: file }, { type: folder })
const results = await uploadMediaBatch(
files.map((file) => ({ source: file })),
{ type: folder }
)
if (!result.success) {
toastStore.addAlert(result.error || 'Upload failed')
return null
}
// Report failed uploads
const failedUploads = results.filter((r) => !r.success)
for (const failed of failedUploads) {
toastStore.addAlert(failed.error || t('toastMessages.uploadFailed'))
}
// Update AssetsStore when uploading to input folder
if (folder === 'input') {
await assetsStore.updateInputs()
}
// Update AssetsStore once after all uploads complete (not per-file)
const successfulPaths = results.filter((r) => r.success).map((r) => r.path)
return result.path
})
if (folder === 'input' && successfulPaths.length > 0) {
await assetsStore.updateInputs()
}
const results = await Promise.all(uploadPromises)
return results.filter((path): path is string => path !== null)
return successfulPaths
}
async function handleFilesUpdate(files: File[]) {
@@ -408,7 +409,7 @@ async function handleFilesUpdate(files: File[]) {
const uploadedPaths = await uploadFiles(files)
if (uploadedPaths.length === 0) {
toastStore.addAlert('File upload failed')
toastStore.addAlert(t('toastMessages.uploadFailed'))
return
}