diff --git a/packages/cli-kit/src/public/node/themes/api.test.ts b/packages/cli-kit/src/public/node/themes/api.test.ts index e17a752ef3..5cf276e4f3 100644 --- a/packages/cli-kit/src/public/node/themes/api.test.ts +++ b/packages/cli-kit/src/public/node/themes/api.test.ts @@ -11,6 +11,7 @@ import { AssetParams, deleteThemeAsset, } from './api.js' +import {RemoteBulkUploadResponse} from './factories.js' import {test, vi, expect, describe} from 'vitest' import {restRequest} from '@shopify/cli-kit/node/api/admin' import {AbortError} from '@shopify/cli-kit/node/error' @@ -322,40 +323,25 @@ describe('bulkUploadThemeAssets', async () => { const id = 123 const assets: AssetParams[] = [ {key: 'snippets/product-variant-picker.liquid', value: 'content'}, - {key: 'templates/404.json', value: 'content'}, + {key: 'templates/404.json', value: 'to_be_replaced_with_hash'}, ] - const mockResults = [ + const mockResults: RemoteBulkUploadResponse[] = [ { code: 200, body: { asset: { key: 'assets/test.liquid', - public_url: 'https://cdn.shopify.com/dummy_url', - created_at: '2024-01-24T16:26:13-08:00', - updated_at: '2024-01-24T16:26:13-08:00', - content_type: 'application/x-liquid', - size: 20, checksum: '3f26c8569292ce6f1cc991c5fa7d3fcb', - theme_id: 139503010036, - warnings: [], + attachment: '', + value: '', }, }, }, { - code: 200, + code: 400, body: { - asset: { - key: 'config/settings_data.json', - public_url: null, - created_at: '2024-01-24T16:18:30-08:00', - updated_at: '2024-01-24T16:26:14-08:00', - content_type: 'application/json', - size: 19, - checksum: '336e955222ddd34b61d0f11940208640', - theme_id: 139503010036, - warnings: [], - }, + errors: {asset: ['expected Hash to be a String']}, }, }, ] @@ -377,27 +363,30 @@ describe('bulkUploadThemeAssets', async () => { { assets: [ {key: 'snippets/product-variant-picker.liquid', value: 'content'}, - {key: 'templates/404.json', value: 'content'}, + {key: 'templates/404.json', value: 'to_be_replaced_with_hash'}, ], }, {}, ) expect(bulkUploadresults).toHaveLength(2) expect(bulkUploadresults[0]).toEqual({ - key: 'assets/test.liquid', + key: 'snippets/product-variant-picker.liquid', success: true, - errors: [], + errors: {}, + operation: 'UPLOAD', asset: { + attachment: '', key: 'assets/test.liquid', - public_url: 'https://cdn.shopify.com/dummy_url', - created_at: '2024-01-24T16:26:13-08:00', - updated_at: '2024-01-24T16:26:13-08:00', - content_type: 'application/x-liquid', - size: 20, checksum: '3f26c8569292ce6f1cc991c5fa7d3fcb', - theme_id: 139503010036, - warnings: [], + value: '', }, }) + expect(bulkUploadresults[1]).toEqual({ + key: 'templates/404.json', + operation: 'UPLOAD', + success: false, + errors: {asset: ['expected Hash to be a String']}, + asset: undefined, + }) }) }) diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index b723506c10..30993c522b 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -11,10 +11,10 @@ import { buildTheme, buildThemeAsset, } from '@shopify/cli-kit/node/themes/factories' -import {BulkUploadResult, Checksum, Key, Theme, ThemeAsset} from '@shopify/cli-kit/node/themes/types' +import {Result, Checksum, Key, Theme, ThemeAsset} from '@shopify/cli-kit/node/themes/types' export type ThemeParams = Partial> -export type AssetParams = Partial> +export type AssetParams = Pick & Partial> export async function fetchTheme(id: number, session: AdminSession): Promise { const response = await request('GET', `/themes/${id}`, session, undefined, {fields: 'id,name,role,processing'}) @@ -51,13 +51,9 @@ export async function bulkUploadThemeAssets( id: number, assets: AssetParams[], session: AdminSession, -): Promise { +): Promise { const response = await request('PUT', `/themes/${id}/assets/bulk`, session, {assets}) - const bulkResults = response.json.results - - if (bulkResults?.length > 0) return bulkResults.map(buildBulkUploadResults) - - return [] + return buildBulkUploadResults(response.json.results, assets) } export async function fetchChecksums(id: number, session: AdminSession): Promise { diff --git a/packages/cli-kit/src/public/node/themes/factories.ts b/packages/cli-kit/src/public/node/themes/factories.ts index 8dbe6bdc97..2e01196321 100644 --- a/packages/cli-kit/src/public/node/themes/factories.ts +++ b/packages/cli-kit/src/public/node/themes/factories.ts @@ -1,4 +1,5 @@ -import {BulkUploadResult, Checksum, Theme, ThemeAsset} from '@shopify/cli-kit/node/themes/types' +import {AssetParams} from './api.js' +import {Result, Checksum, Theme, ThemeAsset, Operation} from '@shopify/cli-kit/node/themes/types' interface RemoteThemeResponse { id: number @@ -15,10 +16,9 @@ interface RemoteAssetResponse { value: string } -interface RemoteBulkUploadResponse { - body: {asset: RemoteAssetResponse} +export interface RemoteBulkUploadResponse { + body: {asset?: RemoteAssetResponse; errors?: {asset: string[]}} code: number - errors?: string[] } export function buildTheme(themeJson?: RemoteThemeResponse): Theme | undefined { @@ -52,13 +52,22 @@ export function buildThemeAsset(asset?: RemoteAssetResponse): ThemeAsset | undef return {key, checksum, attachment, value} } -export function buildBulkUploadResults(bulkUpload?: RemoteBulkUploadResponse): BulkUploadResult | undefined { - if (!bulkUpload) return +export function buildBulkUploadResults( + bulkUploadResponse: RemoteBulkUploadResponse[], + assets: AssetParams[], +): Result[] { + const results: Result[] = [] + if (!bulkUploadResponse) return results - return { - key: bulkUpload.body.asset.key, - success: bulkUpload.code === 200, - errors: bulkUpload.errors || [], - asset: bulkUpload.body.asset || {}, - } + bulkUploadResponse.forEach((bulkUpload, index) => { + const asset = assets[index] + results.push({ + key: asset?.key || '', + success: bulkUpload.code === 200, + errors: bulkUpload.body.errors || {}, + asset: bulkUpload.body.asset, + operation: Operation.Upload, + }) + }) + return results } diff --git a/packages/cli-kit/src/public/node/themes/theme-manager.ts b/packages/cli-kit/src/public/node/themes/theme-manager.ts index 6664ba9dba..01672c42f2 100644 --- a/packages/cli-kit/src/public/node/themes/theme-manager.ts +++ b/packages/cli-kit/src/public/node/themes/theme-manager.ts @@ -3,7 +3,7 @@ import {generateThemeName} from '../../../private/node/themes/generate-theme-nam import {AdminSession} from '@shopify/cli-kit/node/session' import {BugError} from '@shopify/cli-kit/node/error' import {Theme} from '@shopify/cli-kit/node/themes/types' -import {DEVELOPMENT_THEME_ROLE} from '@shopify/cli-kit/node/themes/utils' +import {DEVELOPMENT_THEME_ROLE, Role} from '@shopify/cli-kit/node/themes/utils' export abstract class ThemeManager { protected themeId: string | undefined @@ -32,9 +32,9 @@ export abstract class ThemeManager { return theme } - private async create() { - const name = generateThemeName(this.context) - const role = DEVELOPMENT_THEME_ROLE + async create(themeRole?: Role, themeName?: string) { + const name = themeName || generateThemeName(this.context) + const role = themeRole || DEVELOPMENT_THEME_ROLE const theme = await createTheme( { name, diff --git a/packages/cli-kit/src/public/node/themes/types.ts b/packages/cli-kit/src/public/node/themes/types.ts index a0af9a48c0..49f5f795fe 100644 --- a/packages/cli-kit/src/public/node/themes/types.ts +++ b/packages/cli-kit/src/public/node/themes/types.ts @@ -79,28 +79,38 @@ export interface ThemeAsset extends Checksum { } /** - * Represents a single result returned within the response of a bulk upload operation. - * Each result includes the unique identifier for the file being uploaded, - * the status of the upload operation, any errors that occurred, and the asset that was uploaded. + * Represents a single result for a upload or delete operation on a single file + * Each result includes the unique identifier for the file, the type of the operation, + * the sucesss status of the operation, any errors that occurred, and the asset value of the file. */ -export interface BulkUploadResult { +export interface Result { /** * The unique identifier for the file being uploaded. */ key: string /** + * The operation associated with the result. + */ + operation: Operation + + /* * * Indicates whether the upload operation for this file was successful. */ success: boolean /** - * An array of error messages that were generated during the upload operation for this file. + * Error message that was generated during the upload operation for this file. */ - errors?: string[] + errors?: {asset?: string[]} - /** + /* * * The asset that was uploaded as part of the upload operation for this file. */ - asset: ThemeAsset + asset?: ThemeAsset +} + +export enum Operation { + Delete = 'DELETE', + Upload = 'UPLOAD', } diff --git a/packages/cli-kit/src/public/node/themes/utils.ts b/packages/cli-kit/src/public/node/themes/utils.ts index e7dfe55c15..3c562696c9 100644 --- a/packages/cli-kit/src/public/node/themes/utils.ts +++ b/packages/cli-kit/src/public/node/themes/utils.ts @@ -1,6 +1,10 @@ import {Theme} from '@shopify/cli-kit/node/themes/types' export const DEVELOPMENT_THEME_ROLE = 'development' +export const LIVE_THEME_ROLE = 'live' +export const UNPUBLISHED_THEME_ROLE = 'unpublished' + +export type Role = typeof DEVELOPMENT_THEME_ROLE | typeof LIVE_THEME_ROLE | typeof UNPUBLISHED_THEME_ROLE export function isDevelopmentTheme(theme: Theme) { return theme.role === DEVELOPMENT_THEME_ROLE diff --git a/packages/theme/src/cli/commands/theme/push.test.ts b/packages/theme/src/cli/commands/theme/push.test.ts index dc3d07989c..63e64ee273 100644 --- a/packages/theme/src/cli/commands/theme/push.test.ts +++ b/packages/theme/src/cli/commands/theme/push.test.ts @@ -1,76 +1,172 @@ import Push from './push.js' import {DevelopmentThemeManager} from '../../utilities/development-theme-manager.js' import {ensureThemeStore} from '../../utilities/theme-store.js' +import {findOrSelectTheme} from '../../utilities/theme-selector.js' +import {push} from '../../services/push.js' import {describe, vi, expect, test} from 'vitest' import {Config} from '@oclif/core' import {execCLI2} from '@shopify/cli-kit/node/ruby' -import {ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session' +import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session' import {Theme} from '@shopify/cli-kit/node/themes/types' import {buildTheme} from '@shopify/cli-kit/node/themes/factories' +import {renderConfirmationPrompt, renderTextPrompt} from '@shopify/cli-kit/node/ui' +vi.mock('../../services/push.js') vi.mock('../../utilities/development-theme-manager.js') vi.mock('../../utilities/theme-store.js') +vi.mock('../../utilities/theme-selector.js') vi.mock('@shopify/cli-kit/node/ruby') vi.mock('@shopify/cli-kit/node/session') +vi.mock('@shopify/cli-kit/node/themes/api') +vi.mock('@shopify/cli-kit/node/ui') describe('Push', () => { - describe('run', () => { - const adminSession = {token: '', storeFqdn: ''} - const path = '/my-theme' - - async function run(argv: string[], theme?: Theme) { - vi.mocked(ensureThemeStore).mockReturnValue('example.myshopify.com') - vi.mocked(ensureAuthenticatedThemes).mockResolvedValue(adminSession) - if (theme) { - vi.spyOn(DevelopmentThemeManager.prototype, 'findOrCreate').mockResolvedValue(theme) - } - vi.spyOn(DevelopmentThemeManager.prototype, 'fetch').mockResolvedValue(theme) - - const config = {} as Config - const push = new Push([`--path=${path}`, ...argv], config) - - await push.run() - } + const adminSession = {token: '', storeFqdn: ''} + const path = '/my-theme' + + describe('run with CLI 3 implementation', () => { + test('should pass call the CLI 3 command', async () => { + // Given + const theme = buildTheme({id: 1, name: 'Theme', role: 'development'})! + vi.mocked(findOrSelectTheme).mockResolvedValue(theme) + + // When + await runPushCommand([], path, adminSession) + + // Then + expect(execCLI2).not.toHaveBeenCalled() + expect(push).toHaveBeenCalled() + }) + + test('should pass theme selection flags to FindOrSelectTheme', async () => { + // Given + const theme = buildTheme({id: 1, name: 'Theme', role: 'development'})! + vi.mocked(findOrSelectTheme).mockResolvedValue(theme) + + // When + await runPushCommand(['--live', '--development', '-t', '1'], path, adminSession) - function expectCLI2ToHaveBeenCalledWith(command: string) { - expect(execCLI2).toHaveBeenCalledWith(command.split(' '), { - store: 'example.myshopify.com', - adminToken: adminSession.token, + // Then + expect(findOrSelectTheme).toHaveBeenCalledWith(adminSession, { + header: 'Select a theme to open', + filter: { + live: true, + development: true, + theme: '1', + }, }) - } + }) + + test('should create an unpublished theme when the `unpublished` flag is provided', async () => { + // Given + const theme = buildTheme({id: 1, name: 'Theme', role: 'development'})! + vi.mocked(DevelopmentThemeManager.prototype.create).mockResolvedValue(theme) + + // When + await runPushCommand(['--unpublished', '--theme', 'test_theme'], path, adminSession) + + // Then + expect(DevelopmentThemeManager.prototype.create).toHaveBeenCalledWith('unpublished', 'test_theme') + expect(findOrSelectTheme).not.toHaveBeenCalled() + }) + + test("should render confirmation prompt if 'allow-live' flag is not provided and selected theme role is live", async () => { + // Given + const theme = buildTheme({id: 1, name: 'Theme', role: 'live'})! + vi.mocked(findOrSelectTheme).mockResolvedValue(theme) + vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) + // When + await runPushCommand([], path, adminSession) + + // Then + expect(renderConfirmationPrompt).toHaveBeenCalled() + }) + + test('should render text prompt if unpublished flag is provided and theme flag is not provided', async () => { + // Given + const theme = buildTheme({id: 1, name: 'Theme', role: 'development'})! + vi.mocked(renderTextPrompt).mockResolvedValue('test_name') + vi.mocked(DevelopmentThemeManager.prototype.create).mockResolvedValue(theme) + + // When + await runPushCommand(['--unpublished'], path, adminSession) + + // Then + expect(renderTextPrompt).toHaveBeenCalled() + }) + }) + + describe('run with CLI 2 implementation', () => { test('should pass development theme from local storage to CLI 2', async () => { + // Given const theme = buildTheme({id: 1, name: 'Theme', role: 'development'})! await run([], theme) + // Then expect(DevelopmentThemeManager.prototype.findOrCreate).not.toHaveBeenCalled() expect(DevelopmentThemeManager.prototype.fetch).toHaveBeenCalledOnce() - expectCLI2ToHaveBeenCalledWith(`theme push ${path} --development-theme-id ${theme.id}`) + expectCLI2ToHaveBeenCalledWith(`theme push ${path} --stable --development-theme-id ${theme.id}`) }) test('should pass theme and development theme from local storage to CLI 2', async () => { + // Given const themeId = 2 const theme = buildTheme({id: 3, name: 'Theme', role: 'development'})! await run([`--theme=${themeId}`], theme) - expectCLI2ToHaveBeenCalledWith(`theme push ${path} --theme ${themeId} --development-theme-id ${theme.id}`) + // Then + expectCLI2ToHaveBeenCalledWith( + `theme push ${path} --theme ${themeId} --stable --development-theme-id ${theme.id}`, + ) }) test('should not pass development theme to CLI 2 if local storage is empty', async () => { + // When await run([]) + // Then expect(DevelopmentThemeManager.prototype.findOrCreate).not.toHaveBeenCalled() expect(DevelopmentThemeManager.prototype.fetch).toHaveBeenCalledOnce() - expectCLI2ToHaveBeenCalledWith(`theme push ${path}`) + expectCLI2ToHaveBeenCalledWith(`theme push ${path} --stable`) }) test('should pass theme and development theme to CLI 2', async () => { + // Given const theme = buildTheme({id: 4, name: 'Theme', role: 'development'})! await run(['--development'], theme) + // Then expect(DevelopmentThemeManager.prototype.findOrCreate).toHaveBeenCalledOnce() expect(DevelopmentThemeManager.prototype.fetch).not.toHaveBeenCalled() - expectCLI2ToHaveBeenCalledWith(`theme push ${path} --theme ${theme.id} --development-theme-id ${theme.id}`) + expectCLI2ToHaveBeenCalledWith( + `theme push ${path} --stable --theme ${theme.id} --development-theme-id ${theme.id}`, + ) }) }) + + async function run(argv: string[], theme?: Theme) { + await runPushCommand(['--stable', ...argv], path, adminSession, theme) + } + + function expectCLI2ToHaveBeenCalledWith(command: string) { + expect(execCLI2).toHaveBeenCalledWith(command.split(' '), { + store: 'example.myshopify.com', + adminToken: adminSession.token, + }) + } + + async function runPushCommand(argv: string[], path: string, adminSession: AdminSession, theme?: Theme) { + vi.mocked(ensureThemeStore).mockReturnValue('example.myshopify.com') + vi.mocked(ensureAuthenticatedThemes).mockResolvedValue(adminSession) + if (theme) { + vi.spyOn(DevelopmentThemeManager.prototype, 'findOrCreate').mockResolvedValue(theme) + } + vi.spyOn(DevelopmentThemeManager.prototype, 'fetch').mockResolvedValue(theme) + + const config = {} as Config + const push = new Push([`--path=${path}`, ...argv], config) + + await push.run() + } }) diff --git a/packages/theme/src/cli/commands/theme/push.ts b/packages/theme/src/cli/commands/theme/push.ts index 800ed2a77e..367ce37958 100644 --- a/packages/theme/src/cli/commands/theme/push.ts +++ b/packages/theme/src/cli/commands/theme/push.ts @@ -2,10 +2,21 @@ import {themeFlags} from '../../flags.js' import {ensureThemeStore} from '../../utilities/theme-store.js' import ThemeCommand from '../../utilities/theme-command.js' import {DevelopmentThemeManager} from '../../utilities/development-theme-manager.js' +import {findOrSelectTheme} from '../../utilities/theme-selector.js' +import {showEmbeddedCLIWarning} from '../../utilities/embedded-cli-warning.js' +import {push} from '../../services/push.js' +import {hasRequiredThemeDirectories} from '../../utilities/theme-fs.js' +import {currentDirectoryConfirmed} from '../../utilities/theme-ui.js' import {Flags} from '@oclif/core' import {globalFlags} from '@shopify/cli-kit/node/cli' import {execCLI2} from '@shopify/cli-kit/node/ruby' import {ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session' +import {useEmbeddedThemeCLI} from '@shopify/cli-kit/node/context/local' +import {RenderConfirmationPromptOptions, renderConfirmationPrompt, renderTextPrompt} from '@shopify/cli-kit/node/ui' +import {UNPUBLISHED_THEME_ROLE} from '@shopify/cli-kit/node/themes/utils' +import {getRandomName} from '@shopify/cli-kit/common/string' +import {cwd, resolvePath} from '@shopify/cli-kit/node/path' +import {Theme} from '@shopify/cli-kit/node/themes/types' export default class Push extends ThemeCommand { static summary = 'Uploads your local theme files to the connected store, overwriting the remote version if specified.' @@ -130,22 +141,94 @@ export default class Push extends ThemeCommand { async run(): Promise { const {flags} = await this.parse(Push) + const {path, force} = flags const store = ensureThemeStore(flags) const adminSession = await ensureAuthenticatedThemes(store, flags.password) + const workingDirectory = path ? resolvePath(path) : cwd() + if (!(await hasRequiredThemeDirectories(workingDirectory)) && !(await currentDirectoryConfirmed(force))) { + return + } + const developmentThemeManager = new DevelopmentThemeManager(adminSession) - const theme = await (flags.development ? developmentThemeManager.findOrCreate() : developmentThemeManager.fetch()) - if (theme) { + + if (!flags.stable) { + const {live, development, unpublished, path, nodelete, theme, publish, json, force} = flags + + let selectedTheme: Theme + if (flags.unpublished) { + const themeName = theme || (await promptThemeName()) + selectedTheme = await developmentThemeManager.create(UNPUBLISHED_THEME_ROLE, themeName) + } else { + selectedTheme = await findOrSelectTheme(adminSession, { + header: 'Select a theme to open', + filter: { + live, + unpublished, + development, + theme, + }, + }) + } + + if (selectedTheme.role === 'live' && !flags['allow-live']) { + if (!(await confirmPushToLiveTheme(adminSession.storeFqdn))) { + return + } + } + + await push(selectedTheme, adminSession, { + path, + nodelete, + publish, + json, + force, + }) + + return + } + + showEmbeddedCLIWarning() + + const targetTheme = await (flags.development + ? developmentThemeManager.findOrCreate() + : developmentThemeManager.fetch()) + + if (targetTheme) { if (flags.development) { - flags.theme = `${theme.id}` + flags.theme = `${targetTheme.id}` flags.development = false } - flags['development-theme-id'] = theme.id + if (useEmbeddedThemeCLI()) { + flags['development-theme-id'] = targetTheme.id + } } - const flagsToPass = this.passThroughFlags(flags, {allowedFlags: Push.cli2Flags}) + const flagsToPass = this.passThroughFlags(flags, { + allowedFlags: Push.cli2Flags, + }) const command = ['theme', 'push', flags.path, ...flagsToPass] await execCLI2(command, {store, adminToken: adminSession.token}) } } + +async function promptThemeName() { + const defaultName = await getRandomName('creative') + return renderTextPrompt({ + message: 'Name of the new theme', + defaultValue: defaultName, + }) +} + +async function confirmPushToLiveTheme(store: string) { + const message = `Push theme files to the published theme on ${store}?` + + const options: RenderConfirmationPromptOptions = { + message, + confirmationMessage: 'Yes, confirm changes', + cancellationMessage: 'Cancel', + } + + return renderConfirmationPrompt(options) +} diff --git a/packages/theme/src/cli/services/pull.ts b/packages/theme/src/cli/services/pull.ts index 8eea7d5579..dd7b9bdf10 100644 --- a/packages/theme/src/cli/services/pull.ts +++ b/packages/theme/src/cli/services/pull.ts @@ -1,7 +1,7 @@ import {downloadTheme} from '../utilities/theme-downloader.js' import {hasRequiredThemeDirectories, mountThemeFileSystem} from '../utilities/theme-fs.js' import {currentDirectoryConfirmed, themeComponent} from '../utilities/theme-ui.js' -import {rejectLiquidChecksums} from '../utilities/asset-checksum.js' +import {rejectGeneratedStaticAssets} from '../utilities/asset-checksum.js' import {Theme} from '@shopify/cli-kit/node/themes/types' import {AdminSession} from '@shopify/cli-kit/node/session' import {fetchChecksums} from '@shopify/cli-kit/node/themes/api' @@ -36,7 +36,7 @@ export async function pull(theme: Theme, session: AdminSession, options: PullOpt const remoteChecksums = await fetchChecksums(theme.id, session) const themeFileSystem = await mountThemeFileSystem(path) - const themeChecksums = rejectLiquidChecksums(remoteChecksums) + const themeChecksums = rejectGeneratedStaticAssets(remoteChecksums) const store = session.storeFqdn const themeId = theme.id diff --git a/packages/theme/src/cli/services/push.test.ts b/packages/theme/src/cli/services/push.test.ts new file mode 100644 index 0000000000..bf46244e4f --- /dev/null +++ b/packages/theme/src/cli/services/push.test.ts @@ -0,0 +1,41 @@ +import {push} from './push.js' +import {hasRequiredThemeDirectories} from '../utilities/theme-fs.js' +import {uploadTheme} from '../utilities/theme-uploader.js' +import {resolvePath} from '@shopify/cli-kit/node/path' +import {buildTheme} from '@shopify/cli-kit/node/themes/factories' +import {test, describe, vi, expect, beforeEach} from 'vitest' +import {fetchChecksums, publishTheme} from '@shopify/cli-kit/node/themes/api' + +vi.mock('../utilities/theme-fs.js') +vi.mock('@shopify/cli-kit/node/path') +vi.mock('../utilities/theme-uploader.js') +vi.mock('@shopify/cli-kit/node/themes/api') + +describe('push', () => { + const adminSession = {token: '', storeFqdn: ''} + const theme = buildTheme({id: 1, name: 'Theme', role: 'development'})! + + beforeEach(() => { + vi.mocked(uploadTheme).mockResolvedValue(new Map()) + }) + + test('should call publishTheme if publish flag is provided', async () => { + // Given + const theme = buildTheme({id: 1, name: 'Theme', role: 'development'})! + vi.mocked(resolvePath).mockReturnValue('/provided/path') + vi.mocked(hasRequiredThemeDirectories).mockResolvedValue(true) + vi.mocked(fetchChecksums).mockResolvedValue([]) + + // When + await push(theme, adminSession, { + publish: true, + path: '', + nodelete: false, + json: false, + force: false, + }) + + // Then + expect(publishTheme).toHaveBeenCalledWith(theme.id, adminSession) + }) +}) diff --git a/packages/theme/src/cli/services/push.ts b/packages/theme/src/cli/services/push.ts new file mode 100644 index 0000000000..c25fb04ed2 --- /dev/null +++ b/packages/theme/src/cli/services/push.ts @@ -0,0 +1,128 @@ +import {mountThemeFileSystem} from '../utilities/theme-fs.js' +import {uploadTheme} from '../utilities/theme-uploader.js' +import {rejectGeneratedStaticAssets} from '../utilities/asset-checksum.js' +import {themeComponent} from '../utilities/theme-ui.js' +import {AdminSession} from '@shopify/cli-kit/node/session' +import {fetchChecksums, publishTheme} from '@shopify/cli-kit/node/themes/api' +import {Result, Theme} from '@shopify/cli-kit/node/themes/types' +import {outputInfo} from '@shopify/cli-kit/node/output' +import {renderSuccess, renderWarning} from '@shopify/cli-kit/node/ui' +import {themeEditorUrl, themePreviewUrl} from '@shopify/cli-kit/node/themes/urls' + +interface PushOptions { + path: string + nodelete: boolean + json: boolean + publish?: boolean + force: boolean +} + +interface JsonOutput { + id: number + name: string + role: string + shop: string + editor_url: string + preview_url: string + warning?: string +} + +export async function push(theme: Theme, session: AdminSession, options: PushOptions) { + const remoteChecksums = await fetchChecksums(theme.id, session) + const themeFileSystem = await mountThemeFileSystem(options.path) + const themeChecksums = rejectGeneratedStaticAssets(remoteChecksums) + + const results = await uploadTheme(theme, session, themeChecksums, themeFileSystem, { + ...options, + }) + + if (options.publish) { + await publishTheme(theme.id, session) + } + + await handlePushOutput(results, theme, session, options) +} + +function hasUploadErrors(results: Map): boolean { + for (const [_key, result] of results.entries()) { + if (!result.success) { + return true + } + } + return false +} + +async function handlePushOutput( + results: Map, + theme: Theme, + session: AdminSession, + options: PushOptions, +) { + const hasErrors = hasUploadErrors(results) + + if (options.json) { + handleJsonOutput(theme, hasErrors, session) + } else if (options.publish) { + handlePublishOutput(hasErrors, session) + } else { + handleOutput(theme, hasErrors, session) + } +} + +function handleJsonOutput(theme: Theme, hasErrors: boolean, session: AdminSession) { + const output: JsonOutput = { + id: theme.id, + name: theme.name, + role: theme.role, + shop: session.storeFqdn, + editor_url: themeEditorUrl(theme, session), + preview_url: themePreviewUrl(theme, session), + } + + if (hasErrors) { + const message = `The theme ${themeComponent(theme).join(' ')} was pushed with errors` + output.warning = message + } + outputInfo(JSON.stringify(output)) +} + +function handlePublishOutput(hasErrors: boolean, session: AdminSession) { + if (hasErrors) { + renderWarning({body: `Your theme was published with errors and is now live at https://${session.storeFqdn}`}) + } else { + renderSuccess({body: `Your theme is now live at https://${session.storeFqdn}`}) + } +} + +function handleOutput(theme: Theme, hasErrors: boolean, session: AdminSession) { + const nextSteps = [ + [ + { + link: { + label: 'View your theme', + url: themePreviewUrl(theme, session), + }, + }, + ], + [ + { + link: { + label: 'Customize your theme at the theme editor', + url: themeEditorUrl(theme, session), + }, + }, + ], + ] + + if (hasErrors) { + renderWarning({ + body: ['The theme', ...themeComponent(theme), 'was pushed with errors'], + nextSteps, + }) + } else { + renderSuccess({ + body: ['The theme', ...themeComponent(theme), 'was pushed successfully.'], + nextSteps, + }) + } +} diff --git a/packages/theme/src/cli/utilities/asset-checksum.test.ts b/packages/theme/src/cli/utilities/asset-checksum.test.ts index a201be81a9..5dc92e7275 100644 --- a/packages/theme/src/cli/utilities/asset-checksum.test.ts +++ b/packages/theme/src/cli/utilities/asset-checksum.test.ts @@ -1,4 +1,4 @@ -import {checksum, rejectLiquidChecksums} from './asset-checksum.js' +import {checksum, rejectGeneratedStaticAssets} from './asset-checksum.js' import {describe, expect, test} from 'vitest' describe('asset-checksum', () => { @@ -44,7 +44,7 @@ describe('asset-checksum', () => { ] // When - const result = rejectLiquidChecksums(themeChecksums) + const result = rejectGeneratedStaticAssets(themeChecksums) // Then expect(result).toEqual([ diff --git a/packages/theme/src/cli/utilities/asset-checksum.ts b/packages/theme/src/cli/utilities/asset-checksum.ts index c97e66d85c..3deef409ab 100644 --- a/packages/theme/src/cli/utilities/asset-checksum.ts +++ b/packages/theme/src/cli/utilities/asset-checksum.ts @@ -7,6 +7,8 @@ export async function checksum(root: string, path: string) { if (!content) return '' + if (Buffer.isBuffer(content)) return md5(content) + if (isTextFile(path)) content = content.replace(/\r\n/g, '\n') if (isJson(path)) { @@ -49,8 +51,8 @@ export function normalizeJson(jsonStr: string) { return formattedStr } -function md5(content: string) { - const buffer = Buffer.from(content) +function md5(content: string | Buffer) { + const buffer = Buffer.isBuffer(content) ? content : Buffer.from(content) return fileHash(buffer) } @@ -70,14 +72,24 @@ function md5(content: string) { * This function filters out the generated files (like 'assets/basic.css'), * as these are not needed for theme comparison. */ -export function rejectLiquidChecksums(themeChecksums: Checksum[]) { +export function rejectGeneratedStaticAssets(themeChecksums: Checksum[]) { return themeChecksums.filter(({key}) => { const isStaticAsset = key.startsWith('assets/') if (isStaticAsset) { - return !themeChecksums.some((checksum) => checksum.key === `${key}.liquid`) + return !hasLiquidSource(themeChecksums, key) } return true }) } + +/** + * Checks if a given key has a corresponding liquid source in the provided checksums. + * @param checksums - The array of checksums to search through. + * @param key - The key to check for a liquid source. + * @returns True if a liquid source exists for the given key, false otherwise. + */ +function hasLiquidSource(checksums: Checksum[], key: string) { + return checksums.some((checksum) => checksum.key === `${key}.liquid`) +} diff --git a/packages/theme/src/cli/utilities/theme-fs.test.ts b/packages/theme/src/cli/utilities/theme-fs.test.ts index b71364a40e..ac5204b1c9 100644 --- a/packages/theme/src/cli/utilities/theme-fs.test.ts +++ b/packages/theme/src/cli/utilities/theme-fs.test.ts @@ -7,6 +7,8 @@ import { readThemeFile, removeThemeFile, writeThemeFile, + partitionThemeFiles, + readThemeFilesFromDisk, } from './theme-fs.js' import {removeFile, writeFile} from '@shopify/cli-kit/node/fs' import {Checksum} from '@shopify/cli-kit/node/themes/types' @@ -33,14 +35,14 @@ describe('theme-fs', () => { files: new Map([ fsEntry({checksum: 'b7fbe0ecff2a6c1d6e697a13096e2b17', key: 'assets/base.css'}), fsEntry({checksum: '7adcd48a3cc215a81fabd9dafb919507', key: 'assets/sparkle.gif'}), + fsEntry({checksum: '22e69af13b7953914563c60035a831bc', key: 'config/settings_data.json'}), + fsEntry({checksum: '3f6b44e95dbcf0214a0a82627a37cd53', key: 'config/settings_schema.json'}), fsEntry({checksum: '7a92d18f1f58b2396c46f98f9e502c6a', key: 'layout/password.liquid'}), fsEntry({checksum: '2374357fdadd3b4636405e80e21e87fc', key: 'layout/theme.liquid'}), fsEntry({checksum: '94d575574a070397f297a2e9bb32ce7d', key: 'locales/en.default.json'}), - fsEntry({checksum: 'f14a0bd594f4fee47b13fc09543098ff', key: 'templates/404.json'}), - fsEntry({checksum: '3f6b44e95dbcf0214a0a82627a37cd53', key: 'config/settings_schema.json'}), - fsEntry({checksum: '22e69af13b7953914563c60035a831bc', key: 'config/settings_data.json'}), fsEntry({checksum: '3e8fecc3fb5e886f082e12357beb5d56', key: 'sections/announcement-bar.liquid'}), fsEntry({checksum: 'aa0c697b712b22753f73c84ba8a2e35a', key: 'snippets/language-localization.liquid'}), + fsEntry({checksum: 'f14a0bd594f4fee47b13fc09543098ff', key: 'templates/404.json'}), ]), root, }) @@ -69,7 +71,7 @@ describe('theme-fs', () => { // When const content = await readThemeFile(root, key) - const contentJson = JSON.parse(content ?? '') + const contentJson = JSON.parse(content?.toString() || '') // Then expect(contentJson).toEqual({ @@ -94,6 +96,19 @@ describe('theme-fs', () => { // Then expect(content).toBeUndefined() }) + + test('returns Buffer for image files', async () => { + // Given + const root = 'src/cli/utilities/fixtures' + const key = 'assets/sparkle.gif' + + // When + const content = await readThemeFile(root, key) + + // Then + expect(content).toBeDefined() + expect(Buffer.isBuffer(content)).toBe(true) + }) }) describe('writeThemeFile', () => { @@ -198,6 +213,110 @@ describe('theme-fs', () => { }) }) + describe('partitionThemeFiles', () => { + test('should partition theme files correctly', () => { + // Given + const files: Checksum[] = [ + {key: 'assets/base.css', checksum: '1'}, + {key: 'assets/base.css.liquid', checksum: '2'}, + {key: 'assets/sparkle.gif', checksum: '3'}, + {key: 'layout/password.liquid', checksum: '4'}, + {key: 'layout/theme.liquid', checksum: '5'}, + {key: 'locales/en.default.json', checksum: '6'}, + {key: 'templates/404.json', checksum: '7'}, + {key: 'config/settings_schema.json', checksum: '8'}, + {key: 'config/settings_data.json', checksum: '9'}, + {key: 'sections/announcement-bar.liquid', checksum: '10'}, + {key: 'snippets/language-localization.liquid', checksum: '11'}, + {key: 'templates/404.context.uk.json', checksum: '11'}, + ] + // When + const {liquidFiles, jsonFiles, configFiles, staticAssetFiles} = partitionThemeFiles(files) + + // Then + expect(liquidFiles).toEqual([ + {key: 'assets/base.css.liquid', checksum: '2'}, + {key: 'layout/password.liquid', checksum: '4'}, + {key: 'layout/theme.liquid', checksum: '5'}, + {key: 'sections/announcement-bar.liquid', checksum: '10'}, + {key: 'snippets/language-localization.liquid', checksum: '11'}, + ]) + expect(jsonFiles).toEqual([ + {key: 'locales/en.default.json', checksum: '6'}, + {key: 'templates/404.json', checksum: '7'}, + ]) + expect(configFiles).toEqual([ + {key: 'config/settings_schema.json', checksum: '8'}, + {key: 'config/settings_data.json', checksum: '9'}, + ]) + expect(staticAssetFiles).toEqual([ + {key: 'assets/base.css', checksum: '1'}, + {key: 'assets/sparkle.gif', checksum: '3'}, + ]) + }) + + test('should handle empty file array', () => { + // Given + const files: Checksum[] = [] + + // When + const {liquidFiles, jsonFiles, configFiles, staticAssetFiles} = partitionThemeFiles(files) + + // Then + expect(liquidFiles).toEqual([]) + expect(jsonFiles).toEqual([]) + expect(configFiles).toEqual([]) + expect(staticAssetFiles).toEqual([]) + }) + }) + + describe('readThemeFilesFromDisk', () => { + test('should read theme files from disk and update themeFileSystem', async () => { + // Given + const filesToRead = [{key: 'assets/base.css', checksum: '1'}] + const themeFileSystem = await mountThemeFileSystem('src/cli/utilities/fixtures') + + // When + const testFile = themeFileSystem.files.get('assets/base.css') + expect(testFile).toBeDefined() + expect(testFile?.value).toBeUndefined() + await readThemeFilesFromDisk(filesToRead, themeFileSystem) + + // Then + expect(testFile?.value).toBeDefined() + }) + + test('should skip files not present in themeFileSystem', async () => { + // Given + const filesToRead = [{key: 'assets/nonexistent.css', checksum: '1'}] + const themeFileSystem = await mountThemeFileSystem('src/cli/utilities/fixtures') + + // When + const testFile = themeFileSystem.files.get('assets/nonexistent.css') + expect(testFile).toBeUndefined() + await readThemeFilesFromDisk(filesToRead, themeFileSystem) + + // Then + expect(themeFileSystem.files.has('assets/nonexistent.gif')).toBe(false) + }) + + test('should store image data as attachment', async () => { + // Given + const filesToRead = [{key: 'assets/sparkle.gif', checksum: '2'}] + const themeFileSystem = await mountThemeFileSystem('src/cli/utilities/fixtures') + + // When + const testFile = themeFileSystem.files.get('assets/sparkle.gif') + expect(testFile).toBeDefined() + expect(testFile?.attachment).toBeUndefined() + await readThemeFilesFromDisk(filesToRead, themeFileSystem) + + // Then + expect(testFile?.attachment).toBeDefined() + expect(testFile?.value).toBeUndefined() + }) + }) + describe('isTextFile', () => { test(`returns true when it's a text file`, async () => { expect(isTextFile('assets/main.js')).toBeTruthy() diff --git a/packages/theme/src/cli/utilities/theme-fs.ts b/packages/theme/src/cli/utilities/theme-fs.ts index b6dbc4f6b9..eef1c81207 100644 --- a/packages/theme/src/cli/utilities/theme-fs.ts +++ b/packages/theme/src/cli/utilities/theme-fs.ts @@ -34,6 +34,14 @@ const THEME_DIRECTORY_PATTERNS = [ 'templates/customers/**/*.{liquid,json}', ] +const THEME_PARTITION_REGEX = { + liquidRegex: /\.liquid$/, + configRegex: /^config\/(settings_schema|settings_data)\.json$/, + jsonRegex: /^(?!config\/).*\.json$/, + contextualizedJsonRegex: /\.context\.[^.]+\.json$/i, + staticAssetRegex: /^assets\/(?!.*\.liquid$)/, +} + export async function mountThemeFileSystem(root: string): Promise { const filesPaths = await glob(THEME_DIRECTORY_PATTERNS, { cwd: root, @@ -73,7 +81,7 @@ export async function writeThemeFile(root: string, {key, attachment, value}: The } } -export async function readThemeFile(root: string, path: Key) { +export async function readThemeFile(root: string, path: Key): Promise { const options: ReadOptions = isTextFile(path) ? {encoding: 'utf8'} : {} const absolutePath = joinPath(root, path) @@ -106,6 +114,57 @@ export function isJson(path: string) { return lookupMimeType(path) === 'application/json' } +export function partitionThemeFiles(files: ThemeAsset[]) { + const liquidFiles: ThemeAsset[] = [] + const jsonFiles: ThemeAsset[] = [] + const contextualizedJsonFiles: ThemeAsset[] = [] + const configFiles: ThemeAsset[] = [] + const staticAssetFiles: ThemeAsset[] = [] + + files.forEach((file) => { + const fileKey = file.key + if (THEME_PARTITION_REGEX.liquidRegex.test(fileKey)) { + liquidFiles.push(file) + } else if (THEME_PARTITION_REGEX.configRegex.test(fileKey)) { + configFiles.push(file) + } else if (THEME_PARTITION_REGEX.jsonRegex.test(fileKey)) { + if (THEME_PARTITION_REGEX.contextualizedJsonRegex.test(fileKey)) { + contextualizedJsonFiles.push(file) + } else { + jsonFiles.push(file) + } + } else if (THEME_PARTITION_REGEX.staticAssetRegex.test(fileKey)) { + staticAssetFiles.push(file) + } + }) + + return {liquidFiles, jsonFiles, contextualizedJsonFiles, configFiles, staticAssetFiles} +} + +export async function readThemeFilesFromDisk(filesToRead: ThemeAsset[], themeFileSystem: ThemeFileSystem) { + outputDebug(`Reading theme files from disk: ${filesToRead.map((file) => file.key).join(', ')}`) + await Promise.all( + filesToRead.map(async (file) => { + const fileKey = file.key + const themeAsset = themeFileSystem.files.get(fileKey) + if (themeAsset === undefined) { + outputDebug(`File ${fileKey} can't be was not found under directory starting with: ${themeFileSystem.root}`) + return + } + + outputDebug(`Reading theme file: ${fileKey}`) + const fileData = await readThemeFile(themeFileSystem.root, fileKey) + if (Buffer.isBuffer(fileData)) { + themeAsset.attachment = fileData.toString('base64') + } else { + themeAsset.value = fileData + } + themeFileSystem.files.set(fileKey, themeAsset) + }), + ) + outputDebug('All theme files were read from disk') +} + export function isTextFile(path: string) { setMimeTypes({ liquid: 'application/liquid', diff --git a/packages/theme/src/cli/utilities/theme-uploader.test.ts b/packages/theme/src/cli/utilities/theme-uploader.test.ts new file mode 100644 index 0000000000..912a96b241 --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-uploader.test.ts @@ -0,0 +1,436 @@ +import {MAX_BATCH_BYTESIZE, MAX_BATCH_FILE_COUNT, MAX_UPLOAD_RETRY_COUNT, uploadTheme} from './theme-uploader.js' +import {readThemeFilesFromDisk} from './theme-fs.js' +import {fileSize} from '@shopify/cli-kit/node/fs' +import {bulkUploadThemeAssets, deleteThemeAsset} from '@shopify/cli-kit/node/themes/api' +import {Result, Checksum, Key, ThemeAsset, ThemeFileSystem, Operation} from '@shopify/cli-kit/node/themes/types' +import {beforeEach, describe, expect, test, vi} from 'vitest' +import {AdminSession} from '@shopify/cli-kit/node/session' + +vi.mock('@shopify/cli-kit/node/themes/api') +vi.mock('@shopify/cli-kit/node/fs') + +vi.mock('./theme-fs.js', async (realImport) => { + const realModule = await realImport() + const mockModule = {readThemeFilesFromDisk: vi.fn()} + + return {...realModule, ...mockModule} +}) + +beforeEach(() => { + vi.mocked(bulkUploadThemeAssets).mockImplementation( + async ( + _id: number, + assets: Partial>[], + _session: AdminSession, + ): Promise => { + return assets.map((asset) => { + if (asset.key === undefined) { + throw new Error('Asset key is undefined') + } + + return { + key: asset.key, + success: true, + errors: {}, + operation: Operation.Upload, + asset: { + key: asset.key, + value: asset.value, + attachment: asset.attachment, + checksum: 'yourChecksumHere', + }, + } + }) + }, + ) +}) + +describe('theme-uploader', () => { + const remoteTheme = {id: 1, name: '', createdAtRuntime: false, processing: false, role: ''} + const adminSession = {token: '', storeFqdn: ''} + const uploadOptions = {nodelete: false, path: 'tmp'} + + test("should delete files that don't exist locally from remote theme", async () => { + // Given + const remote = [ + {key: 'assets/keepme.liquid', checksum: '1'}, + {key: 'assets/deleteme.liquid', checksum: '2'}, + ] + const local = { + root: 'tmp', + files: new Map([['assets/keepme.liquid', {key: 'assets/keepme.liquid', checksum: '1'}]]), + } as ThemeFileSystem + + // When + await uploadTheme(remoteTheme, adminSession, remote, local, uploadOptions) + + // Then + expect(vi.mocked(deleteThemeAsset)).toHaveBeenCalledOnce() + expect(vi.mocked(deleteThemeAsset)).toHaveBeenCalledWith(remoteTheme.id, 'assets/deleteme.liquid', adminSession) + }) + + test('should not delete files if nodelete is set', async () => { + // Given + const remote = [ + {key: 'assets/keepme.liquid', checksum: '1'}, + {key: 'assets/deleteme.liquid', checksum: '2'}, + ] + const local = { + root: 'tmp', + files: new Map([['assets/keepme.liquid', {key: 'assets/keepme.liquid', checksum: '1'}]]), + } as ThemeFileSystem + + // When + await uploadTheme(remoteTheme, adminSession, remote, local, {...uploadOptions, nodelete: true}) + + // Then + expect(vi.mocked(deleteThemeAsset)).not.toHaveBeenCalled() + }) + + test("should upload local files that don't exist on the remote theme", async () => { + // Given + const remoteChecksums = [{key: 'assets/existing.liquid', checksum: '1'}] + const themeFileSystem = { + root: 'tmp', + files: new Map([ + ['assets/new.liquid', {checksum: '2'}], + ['assets/newer.liquid', {checksum: '3'}], + ]), + } as ThemeFileSystem + + // When + await uploadTheme(remoteTheme, adminSession, remoteChecksums, themeFileSystem, uploadOptions) + + // Then + expect(bulkUploadThemeAssets).toHaveBeenCalledOnce() + expect(bulkUploadThemeAssets).toHaveBeenCalledWith( + remoteTheme.id, + [ + { + key: 'assets/new.liquid', + }, + { + key: 'assets/newer.liquid', + }, + ], + adminSession, + ) + }) + + test('should upload local files that exist on the remote theme if checksums mismatch', async () => { + // Given + const remoteChecksums = [ + {key: 'assets/matching.liquid', checksum: '1'}, + {key: 'assets/conflicting.liquid', checksum: '2'}, + ] + const themeFileSystem = { + root: 'tmp', + files: new Map([ + ['assets/matching.liquid', {checksum: '1'}], + ['assets/conflicting.liquid', {checksum: '3'}], + ]), + } as ThemeFileSystem + + // When + await uploadTheme(remoteTheme, adminSession, remoteChecksums, themeFileSystem, uploadOptions) + + // Then + expect(bulkUploadThemeAssets).toHaveBeenCalledOnce() + expect(bulkUploadThemeAssets).toHaveBeenCalledWith( + remoteTheme.id, + [ + { + key: 'assets/conflicting.liquid', + }, + ], + adminSession, + ) + }) + + test('should separate files by type and upload in correct order', async () => { + // Given + const remoteChecksums: Checksum[] = [] + const themeFileSystem = { + root: 'tmp', + files: new Map([ + ['assets/liquid.liquid', {key: 'assets/liquid.liquid', checksum: '1'}], + ['templates/index.liquid', {key: 'templates/index.liquid', checksum: '4'}], + ['config/settings_data.json', {key: 'config/settings_data.json', checksum: '2'}], + ['config/settings_schema.json', {key: 'config/settings_schema.json', checksum: '3'}], + ['sections/header-group.json', {key: 'sections/header-group.json', checksum: '5'}], + ['templates/product.json', {key: 'templates/product.json', checksum: '6'}], + ['assets/image.png', {key: 'assets/image.png', checksum: '7'}], + ['templates/product.context.uk.json', {key: 'templates/product.context.uk.json', checksum: '8'}], + ]), + } as ThemeFileSystem + + // When + await uploadTheme(remoteTheme, adminSession, remoteChecksums, themeFileSystem, uploadOptions) + + // Then + expect(bulkUploadThemeAssets).toHaveBeenCalledTimes(5) + expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( + 1, + remoteTheme.id, + [ + { + key: 'assets/liquid.liquid', + }, + { + key: 'templates/index.liquid', + }, + ], + adminSession, + ) + expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( + 2, + remoteTheme.id, + [ + { + key: 'sections/header-group.json', + }, + { + key: 'templates/product.json', + }, + ], + adminSession, + ) + expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( + 3, + remoteTheme.id, + [ + { + key: 'templates/product.context.uk.json', + }, + ], + adminSession, + ) + expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( + 4, + remoteTheme.id, + [ + { + key: 'config/settings_data.json', + }, + { + key: 'config/settings_schema.json', + }, + ], + adminSession, + ) + expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( + 5, + remoteTheme.id, + [ + { + key: 'assets/image.png', + }, + ], + adminSession, + ) + }) + + test('should create batches for files when bulk upload file count limit is reached', async () => { + // Given + const remoteChecksums: Checksum[] = [] + const files = new Map() + for (let i = 0; i < MAX_BATCH_FILE_COUNT + 2; i++) { + files.set(`assets/test_${i}.liquid`, { + key: `assets/test_${i}.liquid`, + checksum: i.toString(), + value: `test_${i}`, + }) + } + const themeFileSystem = { + root: 'tmp', + files, + } as ThemeFileSystem + + // When + await uploadTheme(remoteTheme, adminSession, remoteChecksums, themeFileSystem, uploadOptions) + + // Then + expect(bulkUploadThemeAssets).toHaveBeenCalledTimes(2) + }) + + test('should create batches for files when bulk upload request size limit is reached', async () => { + // Given + const remoteChecksums: Checksum[] = [] + const themeFileSystem = { + root: 'tmp', + files: new Map([ + ['config/settings_data.json', {key: 'config/settings_data.json', checksum: '2', value: 'settings_data'}], + ['config/settings_schema.json', {key: 'config/settings_schema.json', checksum: '3', value: 'settings_schema'}], + ]), + } as ThemeFileSystem + + vi.mocked(fileSize).mockResolvedValue(MAX_BATCH_BYTESIZE) + + // When + await uploadTheme(remoteTheme, adminSession, remoteChecksums, themeFileSystem, uploadOptions) + + // Then + expect(bulkUploadThemeAssets).toHaveBeenCalledTimes(2) + }) + + test('should only read values for theme files that will be uploaded', async () => { + // Given + const remoteChecksums = [{key: 'assets/existing.liquid', checksum: '1'}] + const themeFileSystem = { + root: 'tmp', + files: new Map([ + ['assets/new.liquid', {checksum: '2'}], + ['assets/newer.liquid', {checksum: '3'}], + ['assets/existing.liquid', {checksum: '1'}], + ]), + } as ThemeFileSystem + + // When + await uploadTheme(remoteTheme, adminSession, remoteChecksums, themeFileSystem, uploadOptions) + // Then + expect(readThemeFilesFromDisk).toHaveBeenCalledWith( + [ + {checksum: '2', key: 'assets/new.liquid'}, + {checksum: '3', key: 'assets/newer.liquid'}, + ], + themeFileSystem, + ) + }) + + test('should retry failed uploads', async () => { + // Given + const remoteChecksums = [{key: 'assets/existing.liquid', checksum: '1'}] + const themeFileSystem = { + root: 'tmp', + files: new Map([ + ['assets/new.liquid', {checksum: '2'}], + ['assets/newer.liquid', {checksum: '3'}], + ]), + } as ThemeFileSystem + + vi.mocked(bulkUploadThemeAssets) + .mockResolvedValueOnce([ + { + key: 'assets/new.liquid', + success: true, + errors: {}, + operation: Operation.Upload, + asset: {key: 'assets/new.liquid', checksum: '2'}, + }, + { + key: 'assets/newer.liquid', + success: false, + errors: {}, + operation: Operation.Upload, + asset: {key: 'assets/newer.liquid', checksum: '3'}, + }, + ]) + .mockResolvedValue([ + { + key: 'assets/newer.liquid', + success: false, + errors: {}, + operation: Operation.Upload, + asset: {key: 'assets/newer.liquid', checksum: '3'}, + }, + ]) + + // When + await uploadTheme(remoteTheme, adminSession, remoteChecksums, themeFileSystem, uploadOptions) + + // Then + expect(bulkUploadThemeAssets).toHaveBeenCalledTimes(MAX_UPLOAD_RETRY_COUNT + 1) + expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( + 1, + remoteTheme.id, + [ + { + key: 'assets/new.liquid', + }, + { + key: 'assets/newer.liquid', + }, + ], + adminSession, + ) + expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( + 2, + remoteTheme.id, + [ + { + key: 'assets/newer.liquid', + }, + ], + adminSession, + ) + }) + + test('should not delete or upload files specified by the --ignore flag', async () => { + // Given + const remote = [ + {key: 'assets/keepme.liquid', checksum: '1'}, + {key: 'assets/ignore_delete.liquid', checksum: '2'}, + ] + const local = { + root: 'tmp', + files: new Map([ + ['assets/keepme.liquid', {key: 'assets/keepme.liquid', checksum: '3'}], + ['assets/ignore_upload.liquid', {key: 'assets/ignore_upload.liquid', checksum: '4'}], + ]), + } as ThemeFileSystem + + // When + await uploadTheme(remoteTheme, adminSession, remote, local, { + ...uploadOptions, + ignore: ['assets/ignore_delete.liquid', 'assets/ignore_upload.liquid'], + }) + + // Then + expect(vi.mocked(deleteThemeAsset)).not.toHaveBeenCalled() + expect(bulkUploadThemeAssets).toHaveBeenCalledOnce() + expect(bulkUploadThemeAssets).toHaveBeenCalledWith( + remoteTheme.id, + [ + { + key: 'assets/keepme.liquid', + }, + ], + adminSession, + ) + }) + + test('should only delete and upload files specified by --only flag', async () => { + // Given + const remote = [ + {key: 'assets/keepme.liquid', checksum: '1'}, + {key: 'assets/deleteme.liquid', checksum: '2'}, + ] + const local = { + root: 'tmp', + files: new Map([ + ['assets/keepme.liquid', {key: 'assets/keepme.liquid', checksum: '1'}], + ['assets/uploadme.liquid', {key: 'assets/uploadme.liquid', checksum: '3'}], + ]), + } as ThemeFileSystem + + // When + await uploadTheme(remoteTheme, adminSession, remote, local, { + ...uploadOptions, + only: ['assets/keepme.liquid', 'assets/deleteme.liquid', 'assets/uploadme.liquid'], + }) + + // Then + expect(vi.mocked(deleteThemeAsset)).toHaveBeenCalledOnce() + expect(vi.mocked(deleteThemeAsset)).toHaveBeenCalledWith(remoteTheme.id, 'assets/deleteme.liquid', adminSession) + expect(bulkUploadThemeAssets).toHaveBeenCalledOnce() + expect(bulkUploadThemeAssets).toHaveBeenCalledWith( + remoteTheme.id, + [ + { + key: 'assets/uploadme.liquid', + }, + ], + adminSession, + ) + }) +}) diff --git a/packages/theme/src/cli/utilities/theme-uploader.ts b/packages/theme/src/cli/utilities/theme-uploader.ts new file mode 100644 index 0000000000..d5e86c33ca --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-uploader.ts @@ -0,0 +1,393 @@ +import {partitionThemeFiles, readThemeFilesFromDisk} from './theme-fs.js' +import {applyIgnoreFilters} from './asset-ignore.js' +import {AdminSession} from '@shopify/cli-kit/node/session' +import {Result, Checksum, Theme, ThemeFileSystem} from '@shopify/cli-kit/node/themes/types' +import {AssetParams, bulkUploadThemeAssets, deleteThemeAsset} from '@shopify/cli-kit/node/themes/api' +import {fileSize} from '@shopify/cli-kit/node/fs' +import {Task, renderTasks as renderTaskOriginal} from '@shopify/cli-kit/node/ui' +import {outputDebug, outputInfo, outputNewline, outputWarn} from '@shopify/cli-kit/node/output' + +interface UploadOptions { + path: string + nodelete?: boolean + ignore?: string[] + only?: string[] +} +type FileBatch = Checksum[] + +// Limits for Bulk Requests +export const MAX_BATCH_FILE_COUNT = 10 +// 100KB +export const MAX_BATCH_BYTESIZE = 102400 +export const MAX_UPLOAD_RETRY_COUNT = 2 + +export async function uploadTheme( + theme: Theme, + session: AdminSession, + remoteChecksums: Checksum[], + themeFileSystem: ThemeFileSystem, + options: UploadOptions, +) { + const uploadResults: Map = new Map() + const deleteTasks = await buildDeleteTasks(remoteChecksums, themeFileSystem, options, theme, session) + const uploadTasks = await buildUploadTasks(remoteChecksums, themeFileSystem, options, theme, session, uploadResults) + + const {jsonTasks, otherTasks} = deleteTasks + const {liquidUploadTasks, jsonUploadTasks, contextualizedJsonUploadTasks, configUploadTasks, staticUploadTasks} = + uploadTasks + + // The sequence of tasks is important here + await renderTasks([...jsonTasks, ...otherTasks]) + + await renderTasks([ + ...liquidUploadTasks, + ...jsonUploadTasks, + ...contextualizedJsonUploadTasks, + ...configUploadTasks, + ...staticUploadTasks, + ]) + + reportFailedUploads(uploadResults) + return uploadResults +} + +async function buildDeleteTasks( + remoteChecksums: Checksum[], + themeFileSystem: ThemeFileSystem, + options: UploadOptions, + theme: Theme, + session: AdminSession, +) { + if (options.nodelete) { + return {jsonTasks: [], otherTasks: []} + } + + const filteredChecksums = await applyIgnoreFilters(remoteChecksums, themeFileSystem, options) + + const remoteFilesToBeDeleted = await getRemoteFilesToBeDeleted(filteredChecksums, themeFileSystem, options) + const {jsonFiles, liquidFiles, configFiles, staticAssetFiles} = partitionThemeFiles(remoteFilesToBeDeleted) + const otherFiles = [...liquidFiles, ...configFiles, ...staticAssetFiles] + + const jsonTasks = createDeleteTasks(jsonFiles, theme.id, session) + const otherTasks = createDeleteTasks(otherFiles, theme.id, session) + + return {jsonTasks, otherTasks} +} + +async function getRemoteFilesToBeDeleted( + remoteChecksums: Checksum[], + themeFileSystem: ThemeFileSystem, + options: UploadOptions, +): Promise { + const localKeys = new Set(themeFileSystem.files.keys()) + const filteredChecksums = await applyIgnoreFilters(remoteChecksums, themeFileSystem, options) + const filesToBeDeleted = filteredChecksums.filter((checksum) => !localKeys.has(checksum.key)) + outputDebug(`Files to be deleted:\n${filesToBeDeleted.map((file) => `-${file.key}`).join('\n')}`) + return filesToBeDeleted +} + +function createDeleteTasks(files: Checksum[], themeId: number, session: AdminSession): Task[] { + return files.map((file) => ({ + title: `Cleaning your remote theme (removing ${file.key})`, + task: async () => deleteFileFromRemote(themeId, file, session), + })) +} + +async function deleteFileFromRemote(themeId: number, file: Checksum, session: AdminSession) { + await deleteThemeAsset(themeId, file.key, session) +} + +async function buildUploadTasks( + remoteChecksums: Checksum[], + themeFileSystem: ThemeFileSystem, + options: UploadOptions, + theme: Theme, + session: AdminSession, + uploadResults: Map, +): Promise<{ + liquidUploadTasks: Task[] + jsonUploadTasks: Task[] + contextualizedJsonUploadTasks: Task[] + configUploadTasks: Task[] + staticUploadTasks: Task[] +}> { + const filesToUpload = await selectUploadableFiles(themeFileSystem, remoteChecksums, options) + + await readThemeFilesFromDisk(filesToUpload, themeFileSystem) + const {liquidUploadTasks, jsonUploadTasks, contextualizedJsonUploadTasks, configUploadTasks, staticUploadTasks} = + await createUploadTasks(filesToUpload, themeFileSystem, session, theme, uploadResults) + + return {liquidUploadTasks, jsonUploadTasks, contextualizedJsonUploadTasks, configUploadTasks, staticUploadTasks} +} + +async function createUploadTasks( + filesToUpload: Checksum[], + themeFileSystem: ThemeFileSystem, + session: AdminSession, + theme: Theme, + uploadResults: Map, +): Promise<{ + liquidUploadTasks: Task[] + jsonUploadTasks: Task[] + contextualizedJsonUploadTasks: Task[] + configUploadTasks: Task[] + staticUploadTasks: Task[] +}> { + const totalFileCount = filesToUpload.length + const {jsonFiles, contextualizedJsonFiles, liquidFiles, configFiles, staticAssetFiles} = + partitionThemeFiles(filesToUpload) + + const {tasks: liquidUploadTasks, updatedFileCount: liquidCount} = await createUploadTaskForFileType( + liquidFiles, + themeFileSystem, + session, + uploadResults, + theme.id, + totalFileCount, + 0, + ) + const {tasks: jsonUploadTasks, updatedFileCount: jsonCount} = await createUploadTaskForFileType( + jsonFiles, + themeFileSystem, + session, + uploadResults, + theme.id, + totalFileCount, + liquidCount, + ) + const {tasks: contextualizedJsonUploadTasks, updatedFileCount: contextualizedJsonCount} = + await createUploadTaskForFileType( + contextualizedJsonFiles, + themeFileSystem, + session, + uploadResults, + theme.id, + totalFileCount, + jsonCount, + ) + const {tasks: configUploadTasks, updatedFileCount: configCount} = await createUploadTaskForFileType( + configFiles, + themeFileSystem, + session, + uploadResults, + theme.id, + totalFileCount, + contextualizedJsonCount, + ) + const {tasks: staticUploadTasks} = await createUploadTaskForFileType( + staticAssetFiles, + themeFileSystem, + session, + uploadResults, + theme.id, + totalFileCount, + configCount, + ) + + return {liquidUploadTasks, jsonUploadTasks, contextualizedJsonUploadTasks, configUploadTasks, staticUploadTasks} +} + +async function createUploadTaskForFileType( + checksums: Checksum[], + themeFileSystem: ThemeFileSystem, + session: AdminSession, + uploadResults: Map, + themeId: number, + totalFileCount: number, + currentFileCount: number, +): Promise<{tasks: Task[]; updatedFileCount: number}> { + if (checksums.length === 0) { + return {tasks: [], updatedFileCount: currentFileCount} + } + + const batches = await createBatches(checksums, themeFileSystem.root) + const {tasks, updatedFileCount} = await createUploadTaskForBatch( + batches, + themeFileSystem, + session, + uploadResults, + themeId, + totalFileCount, + currentFileCount, + ) + return {tasks, updatedFileCount} +} + +function createUploadTaskForBatch( + batches: FileBatch[], + themeFileSystem: ThemeFileSystem, + session: AdminSession, + uploadResults: Map, + themeId: number, + totalFileCount: number, + currentFileCount: number, +): {tasks: Task[]; updatedFileCount: number} { + let runningFileCount = currentFileCount + const tasks = batches.map((batch) => { + runningFileCount += batch.length + const progress = Math.round((runningFileCount / totalFileCount) * 100) + return { + title: `Uploading files to remote theme [${progress}%]`, + task: async () => uploadBatch(batch, themeFileSystem, session, themeId, uploadResults), + } + }) + return { + tasks, + updatedFileCount: runningFileCount, + } +} + +async function selectUploadableFiles( + themeFileSystem: ThemeFileSystem, + remoteChecksums: Checksum[], + options: UploadOptions, +): Promise { + const localChecksums = calculateLocalChecksums(themeFileSystem) + const filteredLocalChecksums = await applyIgnoreFilters(localChecksums, themeFileSystem, options) + const remoteChecksumsMap = new Map() + remoteChecksums.forEach((remote) => { + remoteChecksumsMap.set(remote.key, remote) + }) + + const filesToUpload = filteredLocalChecksums.filter((local) => { + const remote = remoteChecksumsMap.get(local.key) + return !remote || remote.checksum !== local.checksum + }) + outputDebug(`Files to be uploaded:\n${filesToUpload.map((file) => `-${file.key}`).join('\n')}`) + return filesToUpload +} + +async function createBatches(files: Checksum[], path: string): Promise { + const fileSizes = await Promise.all(files.map((file) => fileSize(`${path}/${file.key}`))) + const batches = [] + + let currentBatch: Checksum[] = [] + let currentBatchSize = 0 + + files.forEach((file, index) => { + const hasEnoughItems = currentBatch.length >= MAX_BATCH_FILE_COUNT + const hasEnoughByteSize = currentBatchSize >= MAX_BATCH_BYTESIZE + + if (hasEnoughItems || hasEnoughByteSize) { + batches.push(currentBatch) + currentBatch = [] + currentBatchSize = 0 + } + + currentBatch.push(file) + currentBatchSize += fileSizes[index] ?? 0 + }) + + if (currentBatch.length > 0) { + batches.push(currentBatch) + } + + return batches +} + +function calculateLocalChecksums(localThemeFileSystem: ThemeFileSystem): Checksum[] { + const checksums: Checksum[] = [] + + localThemeFileSystem.files.forEach((value, key) => { + checksums.push({ + key, + checksum: value.checksum, + }) + }) + + return checksums +} + +async function uploadBatch( + batch: FileBatch, + localThemeFileSystem: ThemeFileSystem, + session: AdminSession, + themeId: number, + uploadResults: Map, +) { + const uploadParams = batch.map((file) => { + const value = localThemeFileSystem.files.get(file.key)?.value + const attachment = localThemeFileSystem.files.get(file.key)?.attachment + return { + key: file.key, + ...(value && {value}), + ...(attachment && {attachment}), + } + }) + outputDebug(`Uploading the following files:\n${batch.map((file) => `-${file.key}`).join('\n')}`) + const results = await handleBulkUpload(uploadParams, themeId, session) + // store the results in uploadResults, overwriting any existing results + results.forEach((result) => { + uploadResults.set(result.key, result) + }) +} + +async function handleBulkUpload( + uploadParams: AssetParams[], + themeId: number, + session: AdminSession, + count = 0, +): Promise { + if (count > 0) { + outputDebug( + `Retry Attempt ${count}/${MAX_UPLOAD_RETRY_COUNT} for the following files: + ${uploadParams.map((param) => `-${param.key}`).join('\n')}`, + ) + } + + const results = await bulkUploadThemeAssets(themeId, uploadParams, session) + outputDebug( + `File Upload Results:\n${results + .map((result) => `-${result.key}: ${result.success ? 'success' : 'failure'}`) + .join('\n')}`, + ) + + const failedUploadResults = results.filter((result) => result.success === false) + if (failedUploadResults.length > 0) { + outputDebug( + `The following files failed to upload:\n${failedUploadResults.map((param) => `-${param.key}`).join('\n')}`, + ) + const failedResults = await handleFailedUploads(failedUploadResults, uploadParams, themeId, session, count) + return results.concat(failedResults) + } + return results +} + +async function handleFailedUploads( + failedUploadResults: Result[], + uploadParams: AssetParams[], + themeId: number, + session: AdminSession, + count: number, +): Promise { + const failedUploadsSet = new Set(failedUploadResults.map((result) => result.key)) + const failedUploadParams = uploadParams.filter((param) => failedUploadsSet.has(param.key)) + + if (count === MAX_UPLOAD_RETRY_COUNT) { + outputDebug( + `Max retry count reached for the following files:\n${failedUploadParams + .map((param) => `-${param.key}`) + .join('\n')}`, + ) + return failedUploadResults + } + + return handleBulkUpload(failedUploadParams, themeId, session, count + 1) +} + +async function renderTasks(tasks: Task[]) { + if (tasks.length > 0) { + await renderTaskOriginal(tasks) + } +} + +function reportFailedUploads(uploadResults: Map) { + for (const [key, result] of uploadResults.entries()) { + if (!result.success) { + const errorMessage = result.errors?.asset?.map((err) => `-${err}`).join('\n') + outputWarn(`Failed to upload file ${key}:`) + outputInfo(`${errorMessage}`) + outputNewline() + } + } +}