diff --git a/packages/app/src/cli/api/graphql/business-platform-organizations/generated/fetch_store_by_domain.ts b/packages/app/src/cli/api/graphql/business-platform-organizations/generated/fetch_store_by_domain.ts new file mode 100644 index 00000000000..09a5a6f5ea4 --- /dev/null +++ b/packages/app/src/cli/api/graphql/business-platform-organizations/generated/fetch_store_by_domain.ts @@ -0,0 +1,119 @@ +/* eslint-disable @typescript-eslint/consistent-type-definitions, @typescript-eslint/no-duplicate-type-constituents */ +import * as Types from './types.js' + +import {TypedDocumentNode as DocumentNode} from '@graphql-typed-document-node/core' + +export type FetchStoreByDomainQueryVariables = Types.Exact<{ + domain?: Types.InputMaybe +}> + +export type FetchStoreByDomainQuery = { + organization?: { + id: string + name: string + accessibleShops?: { + edges: { + node: { + id: string + externalId?: string | null + name: string + storeType?: Types.Store | null + primaryDomain?: string | null + shortName?: string | null + url?: string | null + } + }[] + } | null + currentUser?: {organizationPermissions: string[]} | {organizationPermissions: string[]} | null + } | null +} + +export const FetchStoreByDomain = { + kind: 'Document', + definitions: [ + { + kind: 'OperationDefinition', + operation: 'query', + name: {kind: 'Name', value: 'FetchStoreByDomain'}, + variableDefinitions: [ + { + kind: 'VariableDefinition', + variable: {kind: 'Variable', name: {kind: 'Name', value: 'domain'}}, + type: {kind: 'NamedType', name: {kind: 'Name', value: 'String'}}, + }, + ], + selectionSet: { + kind: 'SelectionSet', + selections: [ + { + kind: 'Field', + name: {kind: 'Name', value: 'organization'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'id'}}, + {kind: 'Field', name: {kind: 'Name', value: 'name'}}, + { + kind: 'Field', + name: {kind: 'Name', value: 'accessibleShops'}, + arguments: [ + { + kind: 'Argument', + name: {kind: 'Name', value: 'search'}, + value: {kind: 'Variable', name: {kind: 'Name', value: 'domain'}}, + }, + ], + selectionSet: { + kind: 'SelectionSet', + selections: [ + { + kind: 'Field', + name: {kind: 'Name', value: 'edges'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + { + kind: 'Field', + name: {kind: 'Name', value: 'node'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'id'}}, + {kind: 'Field', name: {kind: 'Name', value: 'externalId'}}, + {kind: 'Field', name: {kind: 'Name', value: 'name'}}, + {kind: 'Field', name: {kind: 'Name', value: 'storeType'}}, + {kind: 'Field', name: {kind: 'Name', value: 'primaryDomain'}}, + {kind: 'Field', name: {kind: 'Name', value: 'shortName'}}, + {kind: 'Field', name: {kind: 'Name', value: 'url'}}, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + { + kind: 'Field', + name: {kind: 'Name', value: 'currentUser'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'organizationPermissions'}}, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + ], + }, + }, + ], +} as unknown as DocumentNode diff --git a/packages/app/src/cli/api/graphql/business-platform-organizations/queries/fetch_dev_store_by_domain.graphql b/packages/app/src/cli/api/graphql/business-platform-organizations/queries/fetch_dev_store_by_domain.graphql index d6529e1728f..692aabe6376 100644 --- a/packages/app/src/cli/api/graphql/business-platform-organizations/queries/fetch_dev_store_by_domain.graphql +++ b/packages/app/src/cli/api/graphql/business-platform-organizations/queries/fetch_dev_store_by_domain.graphql @@ -1,22 +1,22 @@ - query FetchDevStoreByDomain($domain: String) { - organization { - id - name - accessibleShops(filters: {field: STORE_TYPE, operator: EQUALS, value: "app_development"}, search: $domain) { - edges { - node { - id - externalId - name - storeType - primaryDomain - shortName - url - } +query FetchDevStoreByDomain($domain: String) { + organization { + id + name + accessibleShops(filters: {field: STORE_TYPE, operator: EQUALS, value: "app_development"}, search: $domain) { + edges { + node { + id + externalId + name + storeType + primaryDomain + shortName + url } } - currentUser { - organizationPermissions - } + } + currentUser { + organizationPermissions } } +} diff --git a/packages/app/src/cli/api/graphql/business-platform-organizations/queries/fetch_store_by_domain.graphql b/packages/app/src/cli/api/graphql/business-platform-organizations/queries/fetch_store_by_domain.graphql new file mode 100644 index 00000000000..0613ebc3a23 --- /dev/null +++ b/packages/app/src/cli/api/graphql/business-platform-organizations/queries/fetch_store_by_domain.graphql @@ -0,0 +1,22 @@ +query FetchStoreByDomain($domain: String) { + organization { + id + name + accessibleShops(search: $domain) { + edges { + node { + id + externalId + name + storeType + primaryDomain + shortName + url + } + } + } + currentUser { + organizationPermissions + } + } +} diff --git a/packages/app/src/cli/commands/app/bulk/execute.ts b/packages/app/src/cli/commands/app/bulk/execute.ts index 8ea3f21dafd..6b7be8a28ad 100644 --- a/packages/app/src/cli/commands/app/bulk/execute.ts +++ b/packages/app/src/cli/commands/app/bulk/execute.ts @@ -26,7 +26,7 @@ export default class BulkExecute extends AppLinkedCommand { await executeBulkOperation({ organization: appContextResult.organization, remoteApp: appContextResult.remoteApp, - storeFqdn: store.shopDomain, + store, query, variables: flags.variables, variableFile: flags['variable-file'], diff --git a/packages/app/src/cli/commands/app/execute.ts b/packages/app/src/cli/commands/app/execute.ts index 743e7cf565c..79b3e89a261 100644 --- a/packages/app/src/cli/commands/app/execute.ts +++ b/packages/app/src/cli/commands/app/execute.ts @@ -25,7 +25,7 @@ export default class Execute extends AppLinkedCommand { await executeOperation({ organization: appContextResult.organization, remoteApp: appContextResult.remoteApp, - storeFqdn: store.shopDomain, + store, query, variables: flags.variables, variableFile: flags['variable-file'], diff --git a/packages/app/src/cli/models/organization.ts b/packages/app/src/cli/models/organization.ts index 48b1b950fe7..c723b62bc82 100644 --- a/packages/app/src/cli/models/organization.ts +++ b/packages/app/src/cli/models/organization.ts @@ -60,4 +60,5 @@ export interface OrganizationStore { transferDisabled: boolean convertableToPartnerTest: boolean provisionable: boolean + storeType?: string } diff --git a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts index 3b5900b8096..fff16e7493d 100644 --- a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts +++ b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts @@ -203,6 +203,19 @@ describe('getBulkOperationStatus', () => { ) }) + test('uses resolved API version in admin request', async () => { + vi.mocked(resolveApiVersion).mockResolvedValue('test-api-version') + vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperation({status: 'RUNNING'})) + + await getBulkOperationStatus({organization: mockOrganization, storeFqdn, operationId, remoteApp}) + + expect(adminRequestDoc).toHaveBeenCalledWith( + expect.objectContaining({ + version: 'test-api-version', + }), + ) + }) + describe('time formatting', () => { test('uses "Started" for running operations', async () => { vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperation({status: 'RUNNING'})) @@ -386,4 +399,17 @@ describe('listBulkOperations', () => { }), ) }) + + test('uses resolved API version in admin request', async () => { + vi.mocked(resolveApiVersion).mockResolvedValue('test-api-version') + vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperationsList([])) + + await listBulkOperations({organization: mockOrganization, storeFqdn, remoteApp}) + + expect(adminRequestDoc).toHaveBeenCalledWith( + expect.objectContaining({ + version: 'test-api-version', + }), + ) + }) }) diff --git a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts index 8d450a17d22..11d221e0cf1 100644 --- a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts +++ b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts @@ -7,7 +7,7 @@ import {BULK_OPERATIONS_MIN_API_VERSION} from './constants.js' import {resolveApiVersion} from '../graphql/common.js' import {BulkOperationRunQueryMutation} from '../../api/graphql/bulk-operations/generated/bulk-operation-run-query.js' import {BulkOperationRunMutationMutation} from '../../api/graphql/bulk-operations/generated/bulk-operation-run-mutation.js' -import {OrganizationApp, OrganizationSource} from '../../models/organization.js' +import {OrganizationApp, OrganizationSource, OrganizationStore} from '../../models/organization.js' import {renderSuccess, renderWarning, renderError, renderInfo} from '@shopify/cli-kit/node/ui' import {ensureAuthenticatedAdminAsApp} from '@shopify/cli-kit/node/session' import {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs' @@ -24,6 +24,7 @@ vi.mock('../graphql/common.js', async () => { return { ...actual, resolveApiVersion: vi.fn(), + validateMutationStore: vi.fn(), } }) vi.mock('@shopify/cli-kit/node/ui') @@ -50,6 +51,15 @@ describe('executeBulkOperation', () => { } as OrganizationApp const storeFqdn = 'test-store.myshopify.com' + const mockStore: OrganizationStore = { + shopId: '123', + link: 'https://test-store.myshopify.com/admin', + shopDomain: storeFqdn, + shopName: 'Test Store', + transferDisabled: false, + convertableToPartnerTest: false, + provisionable: true, + } const mockAdminSession = {token: 'test-token', storeFqdn} const createdBulkOperation: NonNullable< @@ -87,7 +97,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) @@ -110,7 +120,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) @@ -133,7 +143,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query: mutation, }) @@ -158,7 +168,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query: mutation, variables, }) @@ -181,7 +191,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) @@ -206,7 +216,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) @@ -242,7 +252,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query: mutation, variableFile: variableFilePath, }) @@ -265,7 +275,7 @@ describe('executeBulkOperation', () => { executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query: mutation, variableFile: nonExistentPath, }), @@ -284,7 +294,7 @@ describe('executeBulkOperation', () => { executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, variables, }), @@ -305,7 +315,7 @@ describe('executeBulkOperation', () => { executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, variableFile: variableFilePath, }), @@ -338,7 +348,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, watch: true, }) @@ -378,7 +388,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, watch: true, }) @@ -403,7 +413,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, watch: false, }) @@ -430,7 +440,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, watch: false, }) @@ -467,7 +477,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, watch: true, outputFile, @@ -501,7 +511,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, watch: true, }) @@ -530,7 +540,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, watch: true, }) @@ -555,7 +565,7 @@ describe('executeBulkOperation', () => { executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }), ).rejects.toThrow('Bulk operation response returned null with no error message.') @@ -590,7 +600,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, watch: true, }) @@ -626,7 +636,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, watch: true, }) @@ -662,7 +672,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, watch: true, outputFile, @@ -688,7 +698,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) @@ -711,7 +721,7 @@ describe('executeBulkOperation', () => { await executeBulkOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) diff --git a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts index 1ac3a313962..eecac70803d 100644 --- a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts +++ b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts @@ -5,19 +5,24 @@ import {formatBulkOperationStatus} from './format-bulk-operation-status.js' import {downloadBulkOperationResults} from './download-bulk-operation-results.js' import {extractBulkOperationId} from './bulk-operation-status.js' import {BULK_OPERATIONS_MIN_API_VERSION} from './constants.js' -import {createAdminSessionAsApp, formatOperationInfo, resolveApiVersion} from '../graphql/common.js' -import {OrganizationApp, Organization} from '../../models/organization.js' +import { + createAdminSessionAsApp, + formatOperationInfo, + resolveApiVersion, + validateMutationStore, + isMutation, +} from '../graphql/common.js' +import {OrganizationApp, Organization, OrganizationStore} from '../../models/organization.js' import {renderSuccess, renderInfo, renderError, renderWarning, TokenItem} from '@shopify/cli-kit/node/ui' import {outputContent, outputToken, outputResult} from '@shopify/cli-kit/node/output' import {AbortError, BugError} from '@shopify/cli-kit/node/error' import {AbortController} from '@shopify/cli-kit/node/abort' -import {parse} from 'graphql' import {readFile, writeFile, fileExists} from '@shopify/cli-kit/node/fs' interface ExecuteBulkOperationInput { organization: Organization remoteApp: OrganizationApp - storeFqdn: string + store: OrganizationStore query: string variables?: string[] variableFile?: string @@ -47,7 +52,7 @@ export async function executeBulkOperation(input: ExecuteBulkOperationInput): Pr const { organization, remoteApp, - storeFqdn, + store, query, variables, variableFile, @@ -56,7 +61,7 @@ export async function executeBulkOperation(input: ExecuteBulkOperationInput): Pr version: userSpecifiedVersion, } = input - const adminSession = await createAdminSessionAsApp(remoteApp, storeFqdn) + const adminSession = await createAdminSessionAsApp(remoteApp, store.shopDomain) const version = await resolveApiVersion({ adminSession, @@ -67,13 +72,14 @@ export async function executeBulkOperation(input: ExecuteBulkOperationInput): Pr const variablesJsonl = await parseVariablesToJsonl(variables, variableFile) validateBulkOperationVariables(query, variablesJsonl) + validateMutationStore(query, store) renderInfo({ headline: 'Starting bulk operation.', body: [ { list: { - items: formatOperationInfo({organization, remoteApp, storeFqdn, version}), + items: formatOperationInfo({organization, remoteApp, storeFqdn: store.shopDomain, version}), }, }, ], @@ -220,9 +226,3 @@ function statusCommandHelpMessage(operationId: string): TokenItem { {command: `shopify app bulk status --id=${extractBulkOperationId(operationId)}`}, ] } - -function isMutation(graphqlOperation: string): boolean { - const document = parse(graphqlOperation) - const operation = document.definitions.find((def) => def.kind === 'OperationDefinition') - return operation?.kind === 'OperationDefinition' && operation.operation === 'mutation' -} diff --git a/packages/app/src/cli/services/dev/fetch.test.ts b/packages/app/src/cli/services/dev/fetch.test.ts index 6d1401f744e..35fc3990114 100644 --- a/packages/app/src/cli/services/dev/fetch.test.ts +++ b/packages/app/src/cli/services/dev/fetch.test.ts @@ -113,7 +113,7 @@ describe('fetchStore', () => { // Then expect(got).toEqual(STORE1) - expect(developerPlatformClient.storeByDomain).toHaveBeenCalledWith(ORG1.id, 'domain1') + expect(developerPlatformClient.storeByDomain).toHaveBeenCalledWith(ORG1.id, 'domain1', false) }) test('throws error if store not found', async () => { @@ -129,7 +129,7 @@ describe('fetchStore', () => { await expect(got).rejects.toThrow( new AbortError( `Could not find store for domain domain1 in organization org1.`, - `Ensure you've provided the correct store domain, that the store is a dev store, and that you have access to the store.`, + `Ensure you have provided the correct store domain, that the store is a dev store, and that you have access to the store.`, ), ) }) diff --git a/packages/app/src/cli/services/dev/fetch.ts b/packages/app/src/cli/services/dev/fetch.ts index ea07d3b40f0..3d654708963 100644 --- a/packages/app/src/cli/services/dev/fetch.ts +++ b/packages/app/src/cli/services/dev/fetch.ts @@ -117,19 +117,25 @@ export async function fetchOrgFromId( * @param org - Organization * @param storeFqdn - store domain fqdn * @param developerPlatformClient - The client to access the platform API + * @param includeAllStores - Whether to include all store types or only dev stores */ export async function fetchStore( org: Organization, storeFqdn: string, developerPlatformClient: DeveloperPlatformClient, + includeAllStores = false, ): Promise { - const store = await developerPlatformClient.storeByDomain(org.id, storeFqdn) + const store = await developerPlatformClient.storeByDomain(org.id, storeFqdn, includeAllStores) - if (!store) + if (!store) { + const storeTypeMessage = includeAllStores + ? 'Ensure you have provided the correct store domain and that you have access to the store.' + : 'Ensure you have provided the correct store domain, that the store is a dev store, and that you have access to the store.' throw new AbortError( `Could not find store for domain ${storeFqdn} in organization ${org.businessName}.`, - `Ensure you've provided the correct store domain, that the store is a dev store, and that you have access to the store.`, + storeTypeMessage, ) + } return store } diff --git a/packages/app/src/cli/services/execute-operation.test.ts b/packages/app/src/cli/services/execute-operation.test.ts index 889919fe304..eb503d1f82d 100644 --- a/packages/app/src/cli/services/execute-operation.test.ts +++ b/packages/app/src/cli/services/execute-operation.test.ts @@ -1,6 +1,6 @@ import {executeOperation} from './execute-operation.js' -import {createAdminSessionAsApp, resolveApiVersion} from './graphql/common.js' -import {OrganizationApp, OrganizationSource} from '../models/organization.js' +import {createAdminSessionAsApp, resolveApiVersion, validateMutationStore} from './graphql/common.js' +import {OrganizationApp, OrganizationSource, OrganizationStore} from '../models/organization.js' import {renderSuccess, renderError, renderSingleTask} from '@shopify/cli-kit/node/ui' import {adminRequestDoc} from '@shopify/cli-kit/node/api/admin' import {ClientError} from 'graphql-request' @@ -28,6 +28,16 @@ describe('executeOperation', () => { } as OrganizationApp const storeFqdn = 'test-store.myshopify.com' + const mockStore: OrganizationStore = { + shopId: '123', + link: 'https://test-store.myshopify.com/admin', + shopDomain: storeFqdn, + shopName: 'Test Store', + transferDisabled: false, + convertableToPartnerTest: false, + provisionable: true, + storeType: 'APP_DEVELOPMENT', + } const mockAdminSession = {token: 'test-token', storeFqdn} beforeEach(() => { @@ -50,7 +60,7 @@ describe('executeOperation', () => { await executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) @@ -75,7 +85,7 @@ describe('executeOperation', () => { await executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, variables, }) @@ -95,7 +105,7 @@ describe('executeOperation', () => { executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, variables: invalidVariables, }), @@ -117,7 +127,7 @@ describe('executeOperation', () => { await executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, variableFile, }) @@ -139,7 +149,7 @@ describe('executeOperation', () => { executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, variableFile: nonExistentFile, }), @@ -160,7 +170,7 @@ describe('executeOperation', () => { executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, variableFile, }), @@ -180,7 +190,7 @@ describe('executeOperation', () => { await executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, version, }) @@ -203,7 +213,7 @@ describe('executeOperation', () => { await executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) @@ -222,7 +232,7 @@ describe('executeOperation', () => { await executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, outputFile, }) @@ -245,7 +255,7 @@ describe('executeOperation', () => { await executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) @@ -265,7 +275,7 @@ describe('executeOperation', () => { executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }), ).rejects.toThrow('API request failed') @@ -282,7 +292,7 @@ describe('executeOperation', () => { await executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) @@ -306,7 +316,7 @@ describe('executeOperation', () => { await executeOperation({ organization: mockOrganization, remoteApp: mockRemoteApp, - storeFqdn, + store: mockStore, query, }) @@ -317,4 +327,37 @@ describe('executeOperation', () => { }), ) }) + + test('throws AbortError when attempting mutation on non-dev store', async () => { + const nonDevStore: OrganizationStore = { + shopId: '456', + link: 'https://prod-store.myshopify.com/admin', + shopDomain: 'prod-store.myshopify.com', + shopName: 'Production Store', + transferDisabled: false, + convertableToPartnerTest: false, + provisionable: false, + storeType: 'PRODUCTION', + } + + const mutation = 'mutation { productUpdate(input: {}) { product { id } } }' + + // Import the real validateMutationStore to test end-to-end validation + const {validateMutationStore: realValidateMutationStore} = await vi.importActual< + typeof import('./graphql/common.js') + >('./graphql/common.js') + vi.mocked(validateMutationStore).mockImplementation((query, store) => realValidateMutationStore(query, store)) + + await expect( + executeOperation({ + organization: mockOrganization, + remoteApp: mockRemoteApp, + store: nonDevStore, + query: mutation, + }), + ).rejects.toThrow('Mutations can only be executed on dev stores') + + // Verify no API call was made - validation should fail before reaching the API + expect(adminRequestDoc).not.toHaveBeenCalled() + }) }) diff --git a/packages/app/src/cli/services/execute-operation.ts b/packages/app/src/cli/services/execute-operation.ts index 70ba3c7d631..01bd946a4d4 100644 --- a/packages/app/src/cli/services/execute-operation.ts +++ b/packages/app/src/cli/services/execute-operation.ts @@ -1,5 +1,11 @@ -import {createAdminSessionAsApp, resolveApiVersion, formatOperationInfo} from './graphql/common.js' -import {OrganizationApp, Organization} from '../models/organization.js' +import { + createAdminSessionAsApp, + validateSingleOperation, + resolveApiVersion, + formatOperationInfo, + validateMutationStore, +} from './graphql/common.js' +import {OrganizationApp, Organization, OrganizationStore} from '../models/organization.js' import {renderSuccess, renderError, renderInfo, renderSingleTask} from '@shopify/cli-kit/node/ui' import {outputContent, outputToken, outputResult} from '@shopify/cli-kit/node/output' import {AbortError} from '@shopify/cli-kit/node/error' @@ -11,7 +17,7 @@ import {writeFile, readFile, fileExists} from '@shopify/cli-kit/node/fs' interface ExecuteOperationInput { organization: Organization remoteApp: OrganizationApp - storeFqdn: string + store: OrganizationStore query: string variables?: string variableFile?: string @@ -59,7 +65,7 @@ export async function executeOperation(input: ExecuteOperationInput): Promise { expect(result).not.toContain(expect.stringContaining('API version')) }) }) + +describe('isMutation', () => { + test('returns true for mutation operation', () => { + const mutation = 'mutation { productUpdate(input: {}) { product { id } } }' + + expect(isMutation(mutation)).toBe(true) + }) + + test('returns false for query operation', () => { + const query = 'query { shop { name } }' + + expect(isMutation(query)).toBe(false) + }) + + test('returns false for shorthand query syntax', () => { + const query = '{ shop { name } }' + + expect(isMutation(query)).toBe(false) + }) +}) + +describe('validateMutationStore', () => { + const devStore: OrganizationStore = { + shopId: '123', + link: 'link', + shopDomain: 'dev-store.myshopify.com', + shopName: 'Dev Store', + transferDisabled: true, + convertableToPartnerTest: false, + provisionable: true, + storeType: 'APP_DEVELOPMENT', + } + + const nonDevStore: OrganizationStore = { + shopId: '456', + link: 'link', + shopDomain: 'prod-store.myshopify.com', + shopName: 'Production Store', + transferDisabled: false, + convertableToPartnerTest: false, + provisionable: false, + storeType: 'PRODUCTION', + } + + test('allows queries on dev stores', () => { + const query = 'query { shop { name } }' + + expect(() => validateMutationStore(query, devStore)).not.toThrow() + }) + + test('allows queries on non-dev stores', () => { + const query = 'query { shop { name } }' + + expect(() => validateMutationStore(query, nonDevStore)).not.toThrow() + }) + + test('allows mutations on dev stores', () => { + const mutation = 'mutation { productUpdate(input: {}) { product { id } } }' + + expect(() => validateMutationStore(mutation, devStore)).not.toThrow() + }) + + test('throws when attempting mutation on non-dev store', () => { + const mutation = 'mutation { productUpdate(input: {}) { product { id } } }' + + expect(() => validateMutationStore(mutation, nonDevStore)).toThrow('Mutations can only be executed on dev stores') + }) + + test('includes store domain in error message for non-dev store mutations', () => { + const mutation = 'mutation { productUpdate(input: {}) { product { id } } }' + + expect(() => validateMutationStore(mutation, nonDevStore)).toThrow('dev stores') + }) +}) diff --git a/packages/app/src/cli/services/graphql/common.ts b/packages/app/src/cli/services/graphql/common.ts index c13a0ab42f9..48127580370 100644 --- a/packages/app/src/cli/services/graphql/common.ts +++ b/packages/app/src/cli/services/graphql/common.ts @@ -1,4 +1,4 @@ -import {OrganizationApp} from '../../models/organization.js' +import {OrganizationApp, OrganizationStore} from '../../models/organization.js' import {ensureAuthenticatedAdminAsApp, AdminSession} from '@shopify/cli-kit/node/session' import {AbortError, BugError} from '@shopify/cli-kit/node/error' import {outputContent} from '@shopify/cli-kit/node/output' @@ -115,3 +115,29 @@ export function formatOperationInfo(options: { return items } + +/** + * Checks if a GraphQL operation is a mutation. + * + * @param graphqlOperation - The GraphQL query or mutation string to check. + * @returns True if the operation is a mutation, false otherwise. + */ +export function isMutation(graphqlOperation: string): boolean { + const document = parse(graphqlOperation) + const operationDefinition = document.definitions.find((def) => def.kind === 'OperationDefinition') + + return operationDefinition?.operation === 'mutation' +} + +/** + * Validates that mutations can only be executed on dev stores. + * + * @param graphqlOperation - The GraphQL operation to validate. + * @param store - The store where the operation will be executed. + * @throws AbortError if attempting to run a mutation on a non-dev store. + */ +export function validateMutationStore(graphqlOperation: string, store: OrganizationStore): void { + if (isMutation(graphqlOperation) && store.storeType !== 'APP_DEVELOPMENT') { + throw new AbortError(`Mutations can only be executed on dev stores.`) + } +} diff --git a/packages/app/src/cli/services/store-context.test.ts b/packages/app/src/cli/services/store-context.test.ts index 5d8cd3f45fa..95558267ad2 100644 --- a/packages/app/src/cli/services/store-context.test.ts +++ b/packages/app/src/cli/services/store-context.test.ts @@ -60,6 +60,7 @@ describe('storeContext', () => { mockOrganization, 'explicit-store.myshopify.com', mockDeveloperPlatformClient, + false, ) expect(convertToTransferDisabledStoreIfNeeded).toHaveBeenCalledWith( mockStore, @@ -85,6 +86,7 @@ describe('storeContext', () => { mockOrganization, 'cached-store.myshopify.com', mockDeveloperPlatformClient, + false, ) expect(result).toEqual(mockStore) }) diff --git a/packages/app/src/cli/services/store-context.ts b/packages/app/src/cli/services/store-context.ts index 42b9d1a5dfe..98c800f49b8 100644 --- a/packages/app/src/cli/services/store-context.ts +++ b/packages/app/src/cli/services/store-context.ts @@ -12,11 +12,13 @@ import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' * @param appContextResult - The result of the app context function. * @param forceReselectStore - Whether to force reselecting the store. * @param storeFqdn - a store FQDN, optional, when explicitly provided it has preference over anything else. + * @param includeAllStores - Whether to include all store types or only Dev Stores. */ interface StoreContextOptions { appContextResult: LoadedAppContextOutput forceReselectStore: boolean storeFqdn?: string + includeAllStores?: boolean } /** @@ -30,6 +32,7 @@ export async function storeContext({ appContextResult, storeFqdn, forceReselectStore, + includeAllStores = false, }: StoreContextOptions): Promise { const {app, organization, developerPlatformClient} = appContextResult let selectedStore: OrganizationStore @@ -46,9 +49,11 @@ export async function storeContext({ const storeFqdnToUse = storeFqdn ?? cachedStoreInToml if (storeFqdnToUse) { - selectedStore = await fetchStore(organization, storeFqdnToUse, developerPlatformClient) + selectedStore = await fetchStore(organization, storeFqdnToUse, developerPlatformClient, includeAllStores) // never automatically convert a store provided via the command line - await convertToTransferDisabledStoreIfNeeded(selectedStore, organization.id, developerPlatformClient, 'never') + if (!includeAllStores) { + await convertToTransferDisabledStoreIfNeeded(selectedStore, organization.id, developerPlatformClient, 'never') + } } else { // If no storeFqdn is provided, fetch all stores for the organization and let the user select one. const allStores = await developerPlatformClient.devStoresForOrg(organization.id) diff --git a/packages/app/src/cli/utilities/developer-platform-client.ts b/packages/app/src/cli/utilities/developer-platform-client.ts index d3b16261ea2..0b92c12354b 100644 --- a/packages/app/src/cli/utilities/developer-platform-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client.ts @@ -250,7 +250,11 @@ export interface DeveloperPlatformClient { templateSpecifications: (app: MinimalAppIdentifiers) => Promise createApp: (org: Organization, options: CreateAppOptions) => Promise devStoresForOrg: (orgId: string, searchTerm?: string) => Promise> - storeByDomain: (orgId: string, shopDomain: string) => Promise + storeByDomain: ( + orgId: string, + shopDomain: string, + includeAllStores?: boolean, + ) => Promise ensureUserAccessToStore: (orgId: string, store: OrganizationStore) => Promise appExtensionRegistrations: ( app: MinimalAppIdentifiers, diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts index f74ceebd240..b1c38ab8518 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts @@ -86,10 +86,11 @@ import { DevSessionUpdateMutationVariables, } from '../../api/graphql/app-dev/generated/dev-session-update.js' import {DevSessionDelete, DevSessionDeleteMutation} from '../../api/graphql/app-dev/generated/dev-session-delete.js' +import {FetchDevStoreByDomain} from '../../api/graphql/business-platform-organizations/generated/fetch_dev_store_by_domain.js' import { - FetchDevStoreByDomain, - FetchDevStoreByDomainQueryVariables, -} from '../../api/graphql/business-platform-organizations/generated/fetch_dev_store_by_domain.js' + FetchStoreByDomain, + FetchStoreByDomainQueryVariables, +} from '../../api/graphql/business-platform-organizations/generated/fetch_store_by_domain.js' import { ListAppDevStores, ListAppDevStoresQuery, @@ -834,10 +835,14 @@ export class AppManagementClient implements DeveloperPlatformClient { } } - async storeByDomain(orgId: string, shopDomain: string): Promise { - const queryVariables: FetchDevStoreByDomainQueryVariables = {domain: shopDomain} + async storeByDomain( + orgId: string, + shopDomain: string, + includeAllStores = false, + ): Promise { + const queryVariables: FetchStoreByDomainQueryVariables = {domain: shopDomain} const storesResult = await this.businessPlatformOrganizationsRequest({ - query: FetchDevStoreByDomain, + query: includeAllStores ? FetchStoreByDomain : FetchDevStoreByDomain, organizationId: String(numberFromGid(orgId)), variables: queryVariables, }) @@ -1303,7 +1308,7 @@ function mapBusinessPlatformStoresToOrganizationStores( provisionable: boolean, ): OrganizationStore[] { return storesArray.map((store: ShopNode) => { - const {externalId, primaryDomain, name, url} = store + const {externalId, primaryDomain, name, url, storeType} = store const shopDomain = url ?? primaryDomain if (!shopDomain) throw new BugError('The selected store does not have a valid URL') return { @@ -1314,6 +1319,7 @@ function mapBusinessPlatformStoresToOrganizationStores( transferDisabled: true, convertableToPartnerTest: true, provisionable, + storeType, } as OrganizationStore }) } diff --git a/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts b/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts index 59a680465da..2804414b372 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts @@ -513,7 +513,12 @@ export class PartnersClient implements DeveloperPlatformClient { return this.request(ConvertDevToTransferDisabledStoreQuery, input) } - async storeByDomain(orgId: string, shopDomain: string): Promise { + async storeByDomain( + orgId: string, + shopDomain: string, + _includeAllStores = false, + ): Promise { + // Note: includeAllStores parameter not implemented for PartnersClient const variables: FindStoreByDomainQueryVariables = {orgId, shopDomain} const result: FindStoreByDomainSchema = await this.request(FindStoreByDomainQuery, variables) diff --git a/packages/app/src/cli/utilities/execute-command-helpers.test.ts b/packages/app/src/cli/utilities/execute-command-helpers.test.ts index d4bf76e4ee5..c8d3f233908 100644 --- a/packages/app/src/cli/utilities/execute-command-helpers.test.ts +++ b/packages/app/src/cli/utilities/execute-command-helpers.test.ts @@ -59,6 +59,7 @@ describe('prepareAppStoreContext', () => { appContextResult: mockAppContextResult, storeFqdn: mockFlags.store, forceReselectStore: mockFlags.reset, + includeAllStores: true, }) }) diff --git a/packages/app/src/cli/utilities/execute-command-helpers.ts b/packages/app/src/cli/utilities/execute-command-helpers.ts index 1d6780efb22..8a9acd073f8 100644 --- a/packages/app/src/cli/utilities/execute-command-helpers.ts +++ b/packages/app/src/cli/utilities/execute-command-helpers.ts @@ -47,6 +47,7 @@ export async function prepareAppStoreContext(flags: AppStoreContextFlags): Promi appContextResult, storeFqdn: flags.store, forceReselectStore: flags.reset, + includeAllStores: true, }) return {appContextResult, store}