Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/puny-plants-allow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@shopify/cli-kit': minor
'@shopify/theme': minor
'@shopify/cli': minor
---

Add `--name` flag to `theme push`
221 changes: 221 additions & 0 deletions packages/cli-kit/src/public/node/themes/theme-manager.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
import {ThemeManager} from './theme-manager.js'
import {Theme} from './types.js'
import {fetchTheme, themeCreate} from './api.js'
import {DEVELOPMENT_THEME_ROLE, UNPUBLISHED_THEME_ROLE} from './utils.js'
import {BugError} from '../error.js'
import {test, describe, expect, vi, beforeEach} from 'vitest'

vi.mock('./api.js')
vi.mock('../../../private/node/themes/generate-theme-name.js', () => ({
generateThemeName: vi.fn((context: string) => `${context} (test-123-hostname)`),
}))

const session = {token: 'token', storeFqdn: 'my-shop.myshopify.com', refresh: async () => {}}

class TestThemeManager extends ThemeManager {
protected context = 'test-context'
private storedThemeId: string | undefined

constructor(adminSession: any) {
super(adminSession)
this.storedThemeId = undefined
}

getStoredThemeId(): string | undefined {
return this.storedThemeId
}

setThemeId(themeId: string | undefined): void {
this.themeId = themeId
}

protected setTheme(themeId: string): void {
this.storedThemeId = themeId
this.themeId = themeId
}

protected removeTheme(): void {
this.storedThemeId = undefined
this.themeId = undefined
}
}

const mockTheme: Theme = {
id: 123,
name: 'Test Theme',
role: DEVELOPMENT_THEME_ROLE,
processing: false,
createdAtRuntime: false,
}

describe('ThemeManager', () => {
let manager: TestThemeManager

beforeEach(() => {
manager = new TestThemeManager(session)
})

describe('findOrCreate', () => {
test('returns an existing theme when one exists', async () => {
// Given
manager.setThemeId('123')
vi.mocked(fetchTheme).mockResolvedValue(mockTheme)

// When
const result = await manager.findOrCreate()

// Then
expect(fetchTheme).toHaveBeenCalledWith(123, session)
expect(result).toEqual(mockTheme)
expect(themeCreate).not.toHaveBeenCalled()
})

test('creates a new theme when one does not exist', async () => {
// Given
manager.setThemeId(undefined)
vi.mocked(themeCreate).mockResolvedValue(mockTheme)

expect(manager.getStoredThemeId()).toBeUndefined()

// When
const result = await manager.findOrCreate()

// Then
expect(fetchTheme).not.toHaveBeenCalled()
expect(themeCreate).toHaveBeenCalledWith(
{
name: 'test-context (test-123-hostname)',
role: DEVELOPMENT_THEME_ROLE,
},
session,
)
expect(result).toEqual(mockTheme)
expect(manager.getStoredThemeId()).toBe('123')
})

test('always creates a new theme when name is provided', async () => {
// Given
manager.setThemeId('123')
const customTheme = {...mockTheme, name: 'Custom name'}
vi.mocked(themeCreate).mockResolvedValue(customTheme)

// When
const result = await manager.findOrCreate('Custom name')

// Then
expect(fetchTheme).not.toHaveBeenCalled()
expect(themeCreate).toHaveBeenCalledWith(
{
name: 'Custom name',
role: DEVELOPMENT_THEME_ROLE,
},
session,
)
expect(result).toEqual(customTheme)
expect(manager.getStoredThemeId()).toBe('123')
})
})

describe('fetch', () => {
test('returns undefined when no themeId is set', async () => {
// Given
manager.setThemeId(undefined)

// When
const result = await manager.fetch()

// Then
expect(result).toBeUndefined()
expect(fetchTheme).not.toHaveBeenCalled()
})

test('fetches and returns a theme when themeId is set', async () => {
// Given
manager.setThemeId('123')
vi.mocked(fetchTheme).mockResolvedValue(mockTheme)

// When
const result = await manager.fetch()

// Then
expect(fetchTheme).toHaveBeenCalledWith(123, session)
expect(result).toEqual(mockTheme)
})

test('removes theme when fetch returns undefined', async () => {
// Given
manager.setThemeId('123')
vi.mocked(fetchTheme).mockResolvedValue(undefined)

// When
const result = await manager.fetch()

// Then
expect(fetchTheme).toHaveBeenCalledWith(123, session)
expect(result).toBeUndefined()
expect(manager.getStoredThemeId()).toBeUndefined()
})
})

describe('generateThemeName', () => {
test('generates a theme name with the provided context', () => {
// When
const result = manager.generateThemeName('my-app')

// Then
expect(result).toBe('my-app (test-123-hostname)')
})
})

describe('create', () => {
test('creates a new theme with default role and generated name', async () => {
// Given
vi.mocked(themeCreate).mockResolvedValue(mockTheme)

// When
const result = await manager.create()

// Then
expect(themeCreate).toHaveBeenCalledWith(
{
name: 'test-context (test-123-hostname)',
role: DEVELOPMENT_THEME_ROLE,
},
session,
)
expect(result).toEqual(mockTheme)
expect(manager.getStoredThemeId()).toBe('123')
})

test('creates a new theme with specified role and name', async () => {
// Given
const customTheme = {...mockTheme, name: 'Custom name', role: UNPUBLISHED_THEME_ROLE}
vi.mocked(themeCreate).mockResolvedValue(customTheme)

// When
const result = await manager.create(UNPUBLISHED_THEME_ROLE, 'Custom name')

// Then
expect(themeCreate).toHaveBeenCalledWith(
{
name: 'Custom name',
role: UNPUBLISHED_THEME_ROLE,
},
session,
)
expect(result).toEqual(customTheme)
expect(manager.getStoredThemeId()).toBe('123')
})

test('throws BugError when theme creation fails', async () => {
// Given
vi.mocked(themeCreate).mockResolvedValue(undefined)

// When/Then
await expect(manager.create()).rejects.toThrow(BugError)
await expect(manager.create()).rejects.toThrow(
'Could not create theme with name "test-context (test-123-hostname)" and role "development"',
)
})
})
})
6 changes: 3 additions & 3 deletions packages/cli-kit/src/public/node/themes/theme-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ export abstract class ThemeManager {

constructor(protected adminSession: AdminSession) {}

async findOrCreate(): Promise<Theme> {
let theme = await this.fetch()
async findOrCreate(name?: string): Promise<Theme> {
let theme = name ? undefined : await this.fetch()
if (!theme) {
theme = await this.create()
theme = await this.create(undefined, name)
}
return theme
}
Expand Down
1 change: 1 addition & 0 deletions packages/cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2330,6 +2330,7 @@ FLAGS
-u, --unpublished Create a new unpublished theme and push to it.
-x, --ignore=<value>... Skip uploading the specified files (Multiple flags allowed). Wrap the value in double
quotes if you're using wildcards.
--name=<value> The name for the theme. Will always create a new theme.
--no-color Disable color output.
--password=<value> Password generated from the Theme Access app.
--path=<value> The path where you want to run the command. Defaults to the current working directory.
Expand Down
11 changes: 11 additions & 0 deletions packages/cli/oclif.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -6527,6 +6527,17 @@
"name": "live",
"type": "boolean"
},
"name": {
"description": "The name for the theme. Will always create a new theme.",
"env": "SHOPIFY_FLAG_NAME",
"exclusive": [
"theme"
],
"hasDynamicHelp": false,
"multiple": false,
"name": "name",
"type": "option"
},
"no-color": {
"allowNo": false,
"description": "Disable color output.",
Expand Down
5 changes: 5 additions & 0 deletions packages/theme/src/cli/commands/theme/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ export default class Push extends ThemeCommand {
description: 'Require theme check to pass without errors before pushing. Warnings are allowed.',
env: 'SHOPIFY_FLAG_STRICT_PUSH',
}),
name: Flags.string({
description: 'The name for the theme. Will always create a new theme.',
env: 'SHOPIFY_FLAG_NAME',
exclusive: ['theme'],
}),
environment: themeFlags.environment,
}

Expand Down
29 changes: 29 additions & 0 deletions packages/theme/src/cli/services/push.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,21 @@ describe('createOrSelectTheme', async () => {
expect(setDevelopmentTheme).not.toHaveBeenCalled()
})

test('creates unpublished theme when name flag is provided', async () => {
// Given
vi.mocked(themeCreate).mockResolvedValue(buildTheme({id: 2, name: 'Custom name', role: UNPUBLISHED_THEME_ROLE}))
vi.mocked(fetchTheme).mockResolvedValue(undefined)

const flags: PushFlags = {name: 'Custom name'}

// When
const theme = await createOrSelectTheme(adminSession, flags)

// Then
expect(theme).toMatchObject({name: 'Custom name'})
expect(setDevelopmentTheme).not.toHaveBeenCalled()
})

test('creates development theme when development flag is provided', async () => {
// Given
vi.mocked(themeCreate).mockResolvedValue(buildTheme({id: 1, name: 'Theme', role: DEVELOPMENT_THEME_ROLE}))
Expand All @@ -272,6 +287,20 @@ describe('createOrSelectTheme', async () => {
expect(setDevelopmentTheme).toHaveBeenCalled()
})

test('creates development theme when development and name flags are provided', async () => {
// Given
vi.mocked(themeCreate).mockResolvedValue(buildTheme({id: 1, name: 'Custom name', role: DEVELOPMENT_THEME_ROLE}))
vi.mocked(fetchTheme).mockResolvedValue(undefined)
const flags: PushFlags = {development: true, name: 'Custom name'}

// When
const theme = await createOrSelectTheme(adminSession, flags)

// Then
expect(theme).toMatchObject({role: DEVELOPMENT_THEME_ROLE, name: 'Custom name'})
expect(setDevelopmentTheme).toHaveBeenCalled()
})

test('creates development theme when development and unpublished flags are provided', async () => {
// Given
vi.mocked(themeCreate).mockResolvedValue(buildTheme({id: 1, name: 'Theme', role: DEVELOPMENT_THEME_ROLE}))
Expand Down
11 changes: 7 additions & 4 deletions packages/theme/src/cli/services/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ export interface PushFlags {

/** The environment to push the theme to. */
environment?: string[]

/** The name of the theme. Will always create a new theme. */
name?: string
}

/**
Expand Down Expand Up @@ -366,13 +369,13 @@ export async function createOrSelectTheme(
flags: PushFlags,
multiEnvironment?: boolean,
): Promise<Theme | undefined> {
const {live, development, unpublished, theme, environment} = flags
const {live, development, unpublished, theme, environment, name} = flags

if (development) {
const themeManager = new DevelopmentThemeManager(session)
return themeManager.findOrCreate()
} else if (unpublished) {
const themeName = theme ?? (await promptThemeName('Name of the new theme'))
return themeManager.findOrCreate(name)
} else if (unpublished ?? name) {
const themeName = name ?? theme ?? (await promptThemeName('Name of the new theme'))
return themeCreate(
{
name: themeName,
Expand Down
Loading