Skip to content

Commit

Permalink
[Themes] Add error tracking for theme upload results
Browse files Browse the repository at this point in the history
  • Loading branch information
jamesmengo committed Jan 16, 2025
1 parent 0971b54 commit 91d164c
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 0 deletions.
81 changes: 81 additions & 0 deletions packages/theme/src/cli/utilities/theme-uploader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
MAX_UPLOAD_RETRY_COUNT,
MINIMUM_THEME_ASSETS,
uploadTheme,
updateUploadErrors,
} from './theme-uploader.js'
import {fakeThemeFileSystem} from './theme-fs/theme-fs-mock-factory.js'
import {bulkUploadThemeAssets, deleteThemeAsset} from '@shopify/cli-kit/node/themes/api'
Expand Down Expand Up @@ -602,3 +603,83 @@ describe('theme-uploader', () => {
)
})
})

describe('updateUploadErrors', () => {
test('should clear error for successful upload', () => {
// Given
const themeFileSystem = fakeThemeFileSystem('tmp', new Map())
const result: Result = {
key: 'file1.liquid',
success: true,
errors: {},
operation: Operation.Upload,
asset: {key: 'file1.liquid', checksum: 'abc'},
}

// Pre-existing error that should be cleared
themeFileSystem.uploadErrors.set('file1.liquid', ['Old error'])

// When
updateUploadErrors(result, themeFileSystem)

// Then
expect(themeFileSystem.uploadErrors.has('file1.liquid')).toBe(false)
})

test('should set error for failed upload', () => {
// Given
const themeFileSystem = fakeThemeFileSystem('tmp', new Map())
const result: Result = {
key: 'file1.liquid',
success: false,
errors: {asset: ['Upload failed']},
operation: Operation.Upload,
asset: {key: 'file1.liquid', checksum: 'abc'},
}

// When
updateUploadErrors(result, themeFileSystem)

// Then
expect(themeFileSystem.uploadErrors.get('file1.liquid')).toEqual(['Upload failed'])
})

test('should set default error when no specific error provided', () => {
// Given
const themeFileSystem = fakeThemeFileSystem('tmp', new Map())
const result: Result = {
key: 'file1.liquid',
success: false,
errors: {},
operation: Operation.Upload,
asset: {key: 'file1.liquid', checksum: 'abc'},
}

// When
updateUploadErrors(result, themeFileSystem)

// Then
expect(themeFileSystem.uploadErrors.get('file1.liquid')).toEqual(['Response was not successful.'])
})

test('should update existing error', () => {
// Given
const themeFileSystem = fakeThemeFileSystem('tmp', new Map())
const result: Result = {
key: 'file1.liquid',
success: false,
errors: {asset: ['New error']},
operation: Operation.Upload,
asset: {key: 'file1.liquid', checksum: 'abc'},
}

// Pre-existing error that should be updated
themeFileSystem.uploadErrors.set('file1.liquid', ['Old error'])

// When
updateUploadErrors(result, themeFileSystem)

// Then
expect(themeFileSystem.uploadErrors.get('file1.liquid')).toEqual(['New error'])
})
})
10 changes: 10 additions & 0 deletions packages/theme/src/cli/utilities/theme-uploader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,19 @@ async function uploadBatch(
// store the results in uploadResults, overwriting any existing results
results.forEach((result) => {
uploadResults.set(result.key, result)
updateUploadErrors(result, localThemeFileSystem)
})
}

export function updateUploadErrors(result: Result, localThemeFileSystem: ThemeFileSystem) {
if (result.success) {
localThemeFileSystem.uploadErrors.delete(result.key)
} else {
const errors = result.errors?.asset ?? ['Response was not successful.']
localThemeFileSystem.uploadErrors.set(result.key, errors)
}
}

async function handleBulkUpload(
uploadParams: AssetParams[],
themeId: number,
Expand Down

0 comments on commit 91d164c

Please sign in to comment.