Skip to content

Commit

Permalink
Port theme file deletes to graphQL
Browse files Browse the repository at this point in the history
  • Loading branch information
catlee committed Dec 3, 2024
1 parent 55b65ec commit fc9d92b
Show file tree
Hide file tree
Showing 11 changed files with 274 additions and 95 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/* 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 ThemeFilesDeleteMutationVariables = Types.Exact<{
themeId: Types.Scalars['ID']['input']
files: Types.Scalars['String']['input'][] | Types.Scalars['String']['input']
}>

export type ThemeFilesDeleteMutation = {
themeFilesDelete?: {
deletedThemeFiles?: {filename: string}[] | null
userErrors: {
filename?: string | null
code?: Types.OnlineStoreThemeFilesUserErrorsCode | null
message: string
}[]
} | null
}

export const ThemeFilesDelete = {
kind: 'Document',
definitions: [
{
kind: 'OperationDefinition',
operation: 'mutation',
name: {kind: 'Name', value: 'themeFilesDelete'},
variableDefinitions: [
{
kind: 'VariableDefinition',
variable: {kind: 'Variable', name: {kind: 'Name', value: 'themeId'}},
type: {kind: 'NonNullType', type: {kind: 'NamedType', name: {kind: 'Name', value: 'ID'}}},
},
{
kind: 'VariableDefinition',
variable: {kind: 'Variable', name: {kind: 'Name', value: 'files'}},
type: {
kind: 'NonNullType',
type: {
kind: 'ListType',
type: {kind: 'NonNullType', type: {kind: 'NamedType', name: {kind: 'Name', value: 'String'}}},
},
},
},
],
selectionSet: {
kind: 'SelectionSet',
selections: [
{
kind: 'Field',
name: {kind: 'Name', value: 'themeFilesDelete'},
arguments: [
{
kind: 'Argument',
name: {kind: 'Name', value: 'themeId'},
value: {kind: 'Variable', name: {kind: 'Name', value: 'themeId'}},
},
{
kind: 'Argument',
name: {kind: 'Name', value: 'files'},
value: {kind: 'Variable', name: {kind: 'Name', value: 'files'}},
},
],
selectionSet: {
kind: 'SelectionSet',
selections: [
{
kind: 'Field',
name: {kind: 'Name', value: 'deletedThemeFiles'},
selectionSet: {
kind: 'SelectionSet',
selections: [
{kind: 'Field', name: {kind: 'Name', value: 'filename'}},
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
],
},
},
{
kind: 'Field',
name: {kind: 'Name', value: 'userErrors'},
selectionSet: {
kind: 'SelectionSet',
selections: [
{kind: 'Field', name: {kind: 'Name', value: 'filename'}},
{kind: 'Field', name: {kind: 'Name', value: 'code'}},
{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<ThemeFilesDeleteMutation, ThemeFilesDeleteMutationVariables>
19 changes: 19 additions & 0 deletions packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,25 @@ export type OnlineStoreThemeFilesUpsertFileInput = {
filename: Scalars['String']['input']
}

/** Possible error codes that can be returned by `OnlineStoreThemeFilesUserErrors`. */
export type OnlineStoreThemeFilesUserErrorsCode =
/** Access denied. */
| 'ACCESS_DENIED'
/** There are files with the same filename. */
| 'DUPLICATE_FILE_INPUT'
/** Error. */
| 'ERROR'
/** The file is invalid. */
| 'FILE_VALIDATION_ERROR'
/** The input value should be less than or equal to the maximum value allowed. */
| 'LESS_THAN_OR_EQUAL_TO'
/** The record with the ID used as the input value couldn't be found. */
| 'NOT_FOUND'
/** There are theme files with conflicts. */
| 'THEME_FILES_CONFLICT'
/** This action is not available on your current plan. Please upgrade to access theme editing features. */
| 'THEME_LIMITED_PLAN'

/** The input fields for Theme attributes to update. */
export type OnlineStoreThemeInput = {
/** The new name of the theme. */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
mutation themeFilesDelete($themeId: ID!, $files: [String!]!) {
themeFilesDelete(themeId: $themeId, files: $files) {
deletedThemeFiles {
filename
}
userErrors {
filename
code
message
}
}
}
62 changes: 25 additions & 37 deletions packages/cli-kit/src/public/node/themes/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import {
fetchChecksums,
bulkUploadThemeAssets,
AssetParams,
deleteThemeAsset,
deleteThemeAssets,
} from './api.js'
import {Operation} from './types.js'
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 {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 {ThemeFilesDelete} from '../../../cli/api/graphql/admin/generated/theme_files_delete.js'
import {OnlineStoreThemeFileBodyInputType} from '../../../cli/api/graphql/admin/generated/types.js'
import {test, vi, expect, describe} from 'vitest'
import {adminRequestDoc, restRequest, supportedApiVersions} from '@shopify/cli-kit/node/api/admin'
Expand Down Expand Up @@ -269,63 +270,51 @@ describe('themePublish', () => {
}
})

describe('deleteThemeAsset', () => {
describe('deleteThemeAssets', () => {
test('deletes a theme asset', async () => {
// Given
const id = 123
const key = 'snippets/product-variant-picker.liquid'

vi.mocked(restRequest).mockResolvedValue({
json: {message: 'snippets/product-variant-picker.liquid was succesfully deleted'},
status: 200,
headers: {},
vi.mocked(adminRequestDoc).mockResolvedValue({
themeFilesDelete: {
deletedThemeFiles: [{filename: key}],
userErrors: [],
},
})

// When
const output = await deleteThemeAsset(id, key, session)
const output = await deleteThemeAssets(id, [key], session)

// Then
expect(restRequest).toHaveBeenCalledWith('DELETE', `/themes/${id}/assets`, session, undefined, {'asset[key]': key})
expect(output).toBe(true)
expect(adminRequestDoc).toHaveBeenCalledWith(ThemeFilesDelete, session, {
themeId: `gid://shopify/OnlineStoreTheme/${id}`,
files: [key],
})
expect(output).toEqual([{key, success: true, operation: 'DELETE'}])
})

test('returns true when attemping to delete an nonexistent asset', async () => {
test('returns success when attempting to delete nonexistent assets', async () => {
// Given
const id = 123
const key = 'snippets/product-variant-picker.liquid'

vi.mocked(restRequest).mockResolvedValue({
json: {},
status: 200,
headers: {},
vi.mocked(adminRequestDoc).mockResolvedValue({
themeFilesDelete: {
deletedThemeFiles: [{filename: key}],
userErrors: [],
},
})

// When
const output = await deleteThemeAsset(id, key, session)
const output = await deleteThemeAssets(id, [key], session)

// Then
expect(restRequest).toHaveBeenCalledWith('DELETE', `/themes/${id}/assets`, session, undefined, {'asset[key]': key})
expect(output).toBe(true)
})

test('throws an AbortError when the server responds with a 403', async () => {
// Given
const id = 123
const key = 'config/settings_data.json'
const message = 'You are not authorized to edit themes on "my-shop.myshopify.com".'

vi.mocked(restRequest).mockResolvedValue({
json: {message},
status: 403,
headers: {},
expect(adminRequestDoc).toHaveBeenCalledWith(ThemeFilesDelete, session, {
themeId: `gid://shopify/OnlineStoreTheme/${id}`,
files: [key],
})

// When
const deletePromise = () => deleteThemeAsset(id, key, session)

// Then
await expect(deletePromise).rejects.toThrow(new AbortError(message))
expect(restRequest).toHaveBeenCalledWith('DELETE', `/themes/${id}/assets`, session, undefined, {'asset[key]': key})
expect(output).toEqual([{key, success: true, operation: 'DELETE'}])
})
})

Expand All @@ -334,7 +323,6 @@ describe('themeDelete', () => {
test(`deletes a theme with graphql with a ${sessionType} session`, async () => {
// Given
const id = 123
const name = 'store theme'

vi.mocked(adminRequestDoc).mockResolvedValue({
themeDelete: {
Expand Down
47 changes: 42 additions & 5 deletions packages/cli-kit/src/public/node/themes/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
ThemeFilesUpsert,
ThemeFilesUpsertMutation,
} from '../../../cli/api/graphql/admin/generated/theme_files_upsert.js'
import {ThemeFilesDelete} from '../../../cli/api/graphql/admin/generated/theme_files_delete.js'
import {
OnlineStoreThemeFileBodyInputType,
OnlineStoreThemeFilesUpsertFileInput,
Expand Down Expand Up @@ -94,11 +95,47 @@ export async function fetchThemeAssets(id: number, filenames: Key[], session: Ad
}
}

export async function deleteThemeAsset(id: number, key: Key, session: AdminSession): Promise<boolean> {
const response = await request('DELETE', `/themes/${id}/assets`, session, undefined, {
'asset[key]': key,
})
return response.status === 200
export async function deleteThemeAssets(id: number, filenames: Key[], session: AdminSession): Promise<Result[]> {
const batchSize = 50
const results: Result[] = []

for (let i = 0; i < filenames.length; i += batchSize) {
const batch = filenames.slice(i, i + batchSize)
// eslint-disable-next-line no-await-in-loop
const {themeFilesDelete} = await adminRequestDoc(ThemeFilesDelete, session, {
themeId: composeThemeGid(id),
files: batch,
})

if (!themeFilesDelete) {
unexpectedGraphQLError('Failed to delete theme assets')
}

const {deletedThemeFiles, userErrors} = themeFilesDelete

if (deletedThemeFiles) {
deletedThemeFiles.forEach((file) => {
results.push({key: file.filename, success: true, operation: Operation.Delete})
})
}

if (userErrors.length > 0) {
userErrors.forEach((error) => {
if (error.filename) {
results.push({
key: error.filename,
success: false,
operation: Operation.Delete,
errors: {asset: [error.message]},
})
} else {
unexpectedGraphQLError(`Failed to delete theme assets: ${error.message}`)
}
})
}
}

return results
}

export async function bulkUploadThemeAssets(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {reconcileJsonFiles} from './theme-reconciliation.js'
import {REMOTE_STRATEGY, LOCAL_STRATEGY} from './remote-theme-watcher.js'
import {fakeThemeFileSystem} from '../theme-fs/theme-fs-mock-factory.js'
import {deleteThemeAsset, fetchThemeAssets} from '@shopify/cli-kit/node/themes/api'
import {deleteThemeAssets, fetchThemeAssets} from '@shopify/cli-kit/node/themes/api'
import {buildTheme} from '@shopify/cli-kit/node/themes/factories'
import {Checksum, ThemeAsset, ThemeFileSystem} from '@shopify/cli-kit/node/themes/types'
import {DEVELOPMENT_THEME_ROLE} from '@shopify/cli-kit/node/themes/utils'
Expand Down Expand Up @@ -143,7 +143,7 @@ describe('reconcileJsonFiles', () => {
)

// Then
expect(deleteThemeAsset).toHaveBeenCalledWith(developmentTheme.id, assetToBeDeleted.key, adminSession)
expect(deleteThemeAssets).toHaveBeenCalledWith(developmentTheme.id, [assetToBeDeleted.key], adminSession)
expect(defaultThemeFileSystem.files.get('templates/asset.json')).toBeUndefined()
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {batchedRequests} from '../batching.js'
import {MAX_GRAPHQL_THEME_FILES} from '../../constants.js'
import {outputDebug} from '@shopify/cli-kit/node/output'
import {AdminSession} from '@shopify/cli-kit/node/session'
import {deleteThemeAsset, fetchThemeAssets} from '@shopify/cli-kit/node/themes/api'
import {deleteThemeAssets, fetchThemeAssets} from '@shopify/cli-kit/node/themes/api'
import {Checksum, ThemeFileSystem, ThemeAsset, Theme} from '@shopify/cli-kit/node/themes/types'
import {renderInfo, renderSelectPrompt} from '@shopify/cli-kit/node/ui'

Expand Down Expand Up @@ -179,9 +179,13 @@ async function performFileReconciliation(
)
})

const deleteRemoteFiles = remoteFilesToDelete.map((file) => deleteThemeAsset(targetTheme.id, file.key, session))
const deleteRemoteFiles = deleteThemeAssets(
targetTheme.id,
remoteFilesToDelete.map((file) => file.key),
session,
)

await Promise.all([...deleteLocalFiles, ...downloadRemoteFiles, ...deleteRemoteFiles])
await Promise.all([...deleteLocalFiles, ...downloadRemoteFiles, deleteRemoteFiles])
}

async function partitionFilesByReconciliationStrategy(
Expand Down
Loading

0 comments on commit fc9d92b

Please sign in to comment.