Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate themeCreate to GraphQL #5249

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {waitForThemeToBeProcessed} from './host-theme-watcher.js'
import {HostThemeManager, DEFAULT_THEME_ZIP, FALLBACK_THEME_ZIP} from './host-theme-manager.js'
import {createTheme} from '@shopify/cli-kit/node/themes/api'
import {themeCreate} from '@shopify/cli-kit/node/themes/api'
import {beforeEach, describe, expect, test, vi} from 'vitest'
import {DEVELOPMENT_THEME_ROLE} from '@shopify/cli-kit/node/themes/utils'
import {AdminSession} from '@shopify/cli-kit/node/session'
Expand All @@ -18,8 +18,8 @@ describe('HostThemeManager', () => {
vi.spyOn(ThemeManager.prototype, 'generateThemeName').mockImplementation(() => 'App Ext. Host Name')
})

test('should call createTheme with the provided name and src param', async () => {
vi.mocked(createTheme).mockResolvedValue({
test('should call themeCreate with the provided name and src param', async () => {
vi.mocked(themeCreate).mockResolvedValue({
id: 12345,
name: 'Theme',
role: 'development',
Expand All @@ -31,7 +31,7 @@ describe('HostThemeManager', () => {
await themeManager.findOrCreate()

// Then
expect(createTheme).toHaveBeenCalledWith(
expect(themeCreate).toHaveBeenCalledWith(
{
name: 'App Ext. Host Name',
role: DEVELOPMENT_THEME_ROLE,
Expand All @@ -42,9 +42,9 @@ describe('HostThemeManager', () => {
})

describe('dev preview', () => {
test('should call createTheme with the provided name and src param', async () => {
test('should call themeCreate with the provided name and src param', async () => {
// Given
vi.mocked(createTheme).mockResolvedValue({
vi.mocked(themeCreate).mockResolvedValue({
id: 12345,
name: 'Theme',
role: 'development',
Expand All @@ -56,7 +56,7 @@ describe('HostThemeManager', () => {
await themeManager.findOrCreate()

// Then
expect(createTheme).toHaveBeenCalledWith(
expect(themeCreate).toHaveBeenCalledWith(
{
name: 'App Ext. Host Name',
role: DEVELOPMENT_THEME_ROLE,
Expand All @@ -68,7 +68,7 @@ describe('HostThemeManager', () => {

test('should wait for the theme to be processed', async () => {
// Given
vi.mocked(createTheme).mockResolvedValue({
vi.mocked(themeCreate).mockResolvedValue({
id: 12345,
name: 'Theme',
role: 'development',
Expand All @@ -86,7 +86,7 @@ describe('HostThemeManager', () => {

test('should retry creating the theme if the first attempt fails', async () => {
// Given
vi.mocked(createTheme).mockResolvedValueOnce(undefined).mockResolvedValueOnce({
vi.mocked(themeCreate).mockResolvedValueOnce(undefined).mockResolvedValueOnce({
id: 12345,
name: 'Theme',
role: 'development',
Expand All @@ -98,8 +98,8 @@ describe('HostThemeManager', () => {
await themeManager.findOrCreate()

// Then
expect(createTheme).toHaveBeenCalledTimes(2)
expect(createTheme).toHaveBeenNthCalledWith(
expect(themeCreate).toHaveBeenCalledTimes(2)
expect(themeCreate).toHaveBeenNthCalledWith(
1,
{
role: DEVELOPMENT_THEME_ROLE,
Expand All @@ -108,7 +108,7 @@ describe('HostThemeManager', () => {
},
adminSession,
)
expect(createTheme).toHaveBeenNthCalledWith(
expect(themeCreate).toHaveBeenNthCalledWith(
2,
{
role: DEVELOPMENT_THEME_ROLE,
Expand All @@ -121,7 +121,7 @@ describe('HostThemeManager', () => {

test('should gracefully handle a 422 from the server during theme creation', async () => {
// Given
vi.mocked(createTheme)
vi.mocked(themeCreate)
.mockRejectedValueOnce(new Error('API request unprocessable content: {"src":["is empty"]}'))
.mockRejectedValueOnce(new Error('API request unprocessable content: {"src":["is empty"]}'))
.mockResolvedValueOnce({
Expand All @@ -136,12 +136,12 @@ describe('HostThemeManager', () => {
await themeManager.findOrCreate()

// Then
expect(createTheme).toHaveBeenCalledTimes(3)
expect(themeCreate).toHaveBeenCalledTimes(3)
})

test('should retry creating the theme with the Fallback theme zip after 3 failed retry attempts', async () => {
// Given
vi.mocked(createTheme)
vi.mocked(themeCreate)
.mockResolvedValueOnce(undefined)
.mockResolvedValueOnce(undefined)
.mockResolvedValueOnce(undefined)
Expand All @@ -157,8 +157,8 @@ describe('HostThemeManager', () => {
await themeManager.findOrCreate()

// Then
expect(createTheme).toHaveBeenCalledTimes(4)
expect(createTheme).toHaveBeenLastCalledWith(
expect(themeCreate).toHaveBeenCalledTimes(4)
expect(themeCreate).toHaveBeenLastCalledWith(
{
role: DEVELOPMENT_THEME_ROLE,
name: 'App Ext. Host Name',
Expand All @@ -170,14 +170,14 @@ describe('HostThemeManager', () => {

test('should throw a BugError if the theme cannot be created', async () => {
// Given
vi.mocked(createTheme).mockResolvedValue(undefined)
vi.mocked(themeCreate).mockResolvedValue(undefined)

// When
// Then
await expect(themeManager.findOrCreate()).rejects.toThrow(
'Could not create theme with name "App Ext. Host Name" and role "development"',
)
expect(createTheme).toHaveBeenCalledTimes(4)
expect(themeCreate).toHaveBeenCalledTimes(4)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {getHostTheme, removeHostTheme, setHostTheme} from '@shopify/cli-kit/node
import {ThemeManager} from '@shopify/cli-kit/node/themes/theme-manager'
import {AdminSession} from '@shopify/cli-kit/node/session'
import {Theme} from '@shopify/cli-kit/node/themes/types'
import {createTheme} from '@shopify/cli-kit/node/themes/api'
import {themeCreate} from '@shopify/cli-kit/node/themes/api'
import {DEVELOPMENT_THEME_ROLE} from '@shopify/cli-kit/node/themes/utils'
import {BugError} from '@shopify/cli-kit/node/error'
import {outputDebug} from '@shopify/cli-kit/node/output'
Expand Down Expand Up @@ -52,7 +52,7 @@ export class HostThemeManager extends ThemeManager {

try {
// eslint-disable-next-line no-await-in-loop
const theme = await createTheme(options, this.adminSession)
const theme = await themeCreate(options, this.adminSession)
if (theme) {
this.setTheme(theme.id.toString())
outputDebug(`Waiting for theme with id "${theme.id}" to be processed`)
Expand All @@ -69,7 +69,7 @@ export class HostThemeManager extends ThemeManager {
}

outputDebug(`Theme creation failed after ${retryAttemps} retries. Creating theme using fallback theme zip`)
const theme = await createTheme({...options, src: FALLBACK_THEME_ZIP}, this.adminSession)
const theme = await themeCreate({...options, src: FALLBACK_THEME_ZIP}, this.adminSession)
if (!theme) {
outputDebug(`Theme creation failed. Exiting process.`)
throw new BugError(`Could not create theme with name "${options.name}" and role "${options.role}"`)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/* eslint-disable @typescript-eslint/consistent-type-definitions */
import * as Types from './types.js'

import {TypedDocumentNode as DocumentNode} from '@graphql-typed-document-node/core'

export type ThemeCreateMutationVariables = Types.Exact<{
name: Types.Scalars['String']['input']
source: Types.Scalars['URL']['input']
}>

export type ThemeCreateMutation = {
themeCreate?: {
theme?: {id: string; name: string; role: Types.ThemeRole} | null
userErrors: {field?: string[] | null; message: string}[]
} | null
}

export const ThemeCreate = {
kind: 'Document',
definitions: [
{
kind: 'OperationDefinition',
operation: 'mutation',
name: {kind: 'Name', value: 'themeCreate'},
variableDefinitions: [
{
kind: 'VariableDefinition',
variable: {kind: 'Variable', name: {kind: 'Name', value: 'name'}},
type: {kind: 'NonNullType', type: {kind: 'NamedType', name: {kind: 'Name', value: 'String'}}},
},
{
kind: 'VariableDefinition',
variable: {kind: 'Variable', name: {kind: 'Name', value: 'source'}},
type: {kind: 'NonNullType', type: {kind: 'NamedType', name: {kind: 'Name', value: 'URL'}}},
},
],
selectionSet: {
kind: 'SelectionSet',
selections: [
{
kind: 'Field',
name: {kind: 'Name', value: 'themeCreate'},
arguments: [
{
kind: 'Argument',
name: {kind: 'Name', value: 'name'},
value: {kind: 'Variable', name: {kind: 'Name', value: 'name'}},
},
{
kind: 'Argument',
name: {kind: 'Name', value: 'source'},
value: {kind: 'Variable', name: {kind: 'Name', value: 'source'}},
},
],
selectionSet: {
kind: 'SelectionSet',
selections: [
{
kind: 'Field',
name: {kind: 'Name', value: 'theme'},
selectionSet: {
kind: 'SelectionSet',
selections: [
{kind: 'Field', name: {kind: 'Name', value: 'id'}},
{kind: 'Field', name: {kind: 'Name', value: 'name'}},
{kind: 'Field', name: {kind: 'Name', value: 'role'}},
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
],
},
},
{
kind: 'Field',
name: {kind: 'Name', value: 'userErrors'},
selectionSet: {
kind: 'SelectionSet',
selections: [
{kind: 'Field', name: {kind: 'Name', value: 'field'}},
{kind: 'Field', name: {kind: 'Name', value: 'message'}},
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
],
},
},
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
],
},
},
],
},
},
],
} as unknown as DocumentNode<ThemeCreateMutation, ThemeCreateMutationVariables>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
mutation themeCreate($name: String!, $source: URL!) {
themeCreate(name: $name, source: $source) {
theme {
id
name
role
}
userErrors {
field
message
}
}
}
60 changes: 24 additions & 36 deletions packages/cli-kit/src/public/node/themes/api.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
createTheme,
themeCreate,
themeDelete,
fetchTheme,
fetchThemes,
Expand All @@ -15,6 +15,7 @@
import {ThemeDelete} from '../../../cli/api/graphql/admin/generated/theme_delete.js'
import {ThemeUpdate} from '../../../cli/api/graphql/admin/generated/theme_update.js'
import {ThemePublish} from '../../../cli/api/graphql/admin/generated/theme_publish.js'
import {ThemeCreate} from '../../../cli/api/graphql/admin/generated/theme_create.js'
import {GetThemeFileChecksums} from '../../../cli/api/graphql/admin/generated/get_theme_file_checksums.js'
import {ThemeFilesUpsert} from '../../../cli/api/graphql/admin/generated/theme_files_upsert.js'
import {OnlineStoreThemeFileBodyInputType} from '../../../cli/api/graphql/admin/generated/types.js'
Expand Down Expand Up @@ -154,68 +155,55 @@
})
})

describe('createTheme', () => {
describe('themeCreate', () => {
const id = 123
const name = 'new theme'
const role = 'unpublished'
const processing = false
const params: ThemeParams = {name, role}

test('creates a theme', async () => {
// Given
vi.mocked(restRequest).mockResolvedValueOnce({
json: {theme: {id, name, role, processing}},
status: 200,
headers: {},
})

vi.mocked(adminRequestDoc).mockResolvedValue({
themeFilesUpsert: {
upsertedThemeFiles: [],
vi.mocked(adminRequestDoc).mockResolvedValueOnce({
themeCreate: {
theme: {
id: `gid://shopify/OnlineStoreTheme/${id}`,
name,
role,
},
userErrors: [],
},
})

// When
const theme = await createTheme(params, session)
const theme = await themeCreate(params, session)

// Then
expect(restRequest).toHaveBeenCalledWith('POST', '/themes', session, {theme: params}, {})
expect(adminRequestDoc).toHaveBeenCalledWith(ThemeCreate, session, {name: params.name, source: "cdn.shopify.com/static/online-store/theme-skeleton.zip"})

Check failure on line 181 in packages/cli-kit/src/public/node/themes/api.test.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/cli-kit/src/public/node/themes/api.test.ts#L181

[prettier/prettier] Replace `name:·params.name,·source:·"cdn.shopify.com/static/online-store/theme-skeleton.zip"` with `⏎······name:·params.name,⏎······source:·'cdn.shopify.com/static/online-store/theme-skeleton.zip',⏎····`
expect(theme).not.toBeNull()
expect(theme!.id).toEqual(id)
expect(theme!.name).toEqual(name)
expect(theme!.role).toEqual(role)
expect(theme!.processing).toBeFalsy()
})

test('does not upload minimum theme assets when src is provided', async () => {
test('does not use skeletonThemeCdn when src is provided', async () => {
// Given
vi.mocked(restRequest)
.mockResolvedValueOnce({
json: {theme: {id, name, role, processing}},
status: 200,
headers: {},
})
.mockResolvedValueOnce({
json: {
results: [],
vi.mocked(adminRequestDoc).mockResolvedValueOnce({
themeCreate: {
theme: {
id: `gid://shopify/OnlineStoreTheme/${id}`,
name,
role,
},
status: 207,
headers: {},
})
userErrors: [],
},
})

// When
const theme = await createTheme({...params, src: 'https://example.com/theme.zip'}, session)
const theme = await themeCreate({...params, src: 'https://example.com/theme.zip'}, session)

// Then
expect(restRequest).toHaveBeenCalledWith(
'POST',
'/themes',
session,
{theme: {...params, src: 'https://example.com/theme.zip'}},
{},
)
expect(restRequest).not.toHaveBeenCalledWith('PUT', `/themes/${id}/assets/bulk`, session, undefined, {})
expect(adminRequestDoc).toHaveBeenCalledWith(ThemeCreate, session, {name: params.name, source: "https://example.com/theme.zip"})

Check failure on line 206 in packages/cli-kit/src/public/node/themes/api.test.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/cli-kit/src/public/node/themes/api.test.ts#L206

[prettier/prettier] Replace `name:·params.name,·source:·"https://example.com/theme.zip"` with `⏎······name:·params.name,⏎······source:·'https://example.com/theme.zip',⏎····`
})
})

Expand Down
Loading
Loading