Skip to content

Commit 2932221

Browse files
shopify-riveregaodd
andcommitted
Route every store Organizations API request through one helper
Review follow-up. The access guidance moves from tryMessage to nextSteps. `store create dev --json` and `store delete --json` only serialize message and nextSteps, so the guidance on which access to grant and where was lost from their JSON output. Command tests now cover both JSON paths. withOrganizationStoreAccess, which each call site had to remember to wrap around its request, is replaced by organizationStoresRequestDoc in @Shopify/organizations. It sends a Business Platform Organizations API request for an organization and turns a refused one into OrganizationStoreAccessError. Every Organizations API request in the store and organizations packages now goes through it: store list, store create dev and its creation poll, store delete and its confirmation poll, and the shop lookup behind store info and store delete. A new request gets the same handling by using the helper. Co-authored-by: Eddie Gao <eddie.gao@shopify.com>
1 parent ef7faa3 commit 2932221

12 files changed

Lines changed: 207 additions & 115 deletions

File tree

‎packages/organizations/src/cli/services/business-platform.test.ts‎

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
import {
22
isOrganizationAccessError,
33
OrganizationStoreAccessError,
4-
withOrganizationStoreAccess,
4+
organizationStoresRequestDoc,
55
} from './business-platform.js'
66
import {describe, expect, test, vi, beforeEach} from 'vitest'
7+
import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform'
78
import {getAppAutomationToken} from '@shopify/cli-kit/node/environment'
89
import {isServiceAccountTokenSet} from '@shopify/cli-kit/node/session'
910
import {AbortError} from '@shopify/cli-kit/node/error'
1011

1112
vi.mock('@shopify/cli-kit/node/environment')
13+
vi.mock('@shopify/cli-kit/node/api/business-platform')
1214
vi.mock('@shopify/cli-kit/node/session')
1315

1416
const organization = {id: '299069', businessName: 'A Better Looking Shop'}
@@ -56,8 +58,12 @@ describe('OrganizationStoreAccessError', () => {
5658

5759
expect(error).toBeInstanceOf(AbortError)
5860
expect(error.message).toBe("This service account can't manage dev stores in A Better Looking Shop.")
59-
expect(JSON.stringify(error.tryMessage)).toContain('Dev store management')
60-
expect(JSON.stringify(error.tryMessage)).toContain('Settings > Users > Security')
61+
// The guidance lives in nextSteps, the field the store commands' --json output carries.
62+
expect(error.tryMessage).toBeNull()
63+
expect(error.nextSteps).toEqual([
64+
'Give the service account Dev store management access in the Shopify admin under Settings > Users > Security.',
65+
'Or use a token from a service account that has that access.',
66+
])
6167
})
6268

6369
test('points a legacy automation token at service accounts', () => {
@@ -66,38 +72,54 @@ describe('OrganizationStoreAccessError', () => {
6672
const error = new OrganizationStoreAccessError(organization)
6773

6874
expect(error.message).toBe("This automation token can't manage dev stores in A Better Looking Shop.")
69-
expect(JSON.stringify(error.tryMessage)).toContain('SHOPIFY_SERVICE_ACCOUNT_TOKEN')
75+
expect(error.nextSteps?.join(' ')).toContain('SHOPIFY_SERVICE_ACCOUNT_TOKEN')
7076
})
7177

7278
test('talks about permission for a logged-in user', () => {
7379
const error = new OrganizationStoreAccessError(organization)
7480

7581
expect(error.message).toBe("You don't have permission to manage dev stores in A Better Looking Shop.")
82+
expect(error.nextSteps).toEqual([
83+
"Check that you're logged in to the right account.",
84+
'Or ask an organization admin for access.',
85+
])
86+
})
87+
88+
test('falls back to the organization ID when the name is unknown', () => {
89+
const error = new OrganizationStoreAccessError({id: '299069'})
90+
91+
expect(error.message).toBe("You don't have permission to manage dev stores in organization 299069.")
7692
})
7793
})
7894

79-
describe('withOrganizationStoreAccess', () => {
80-
test('returns the request result', async () => {
81-
await expect(withOrganizationStoreAccess(organization, async () => 'ok')).resolves.toBe('ok')
95+
describe('organizationStoresRequestDoc', () => {
96+
const request = {
97+
query: {} as never,
98+
token: 'token',
99+
variables: {first: 1},
100+
unauthorizedHandler: {type: 'token_refresh' as const, handler: async () => ({token: 'refreshed'})},
101+
}
102+
103+
test("sends the request to the organization's Organizations API and returns the result", async () => {
104+
vi.mocked(businessPlatformOrganizationsRequestDoc).mockResolvedValue({ok: true})
105+
106+
await expect(organizationStoresRequestDoc(organization, request)).resolves.toEqual({ok: true})
107+
expect(businessPlatformOrganizationsRequestDoc).toHaveBeenCalledWith({...request, organizationId: '299069'})
82108
})
83109

84110
test('replaces a refused request with the access error', async () => {
85111
vi.mocked(isServiceAccountTokenSet).mockReturnValue(true)
112+
vi.mocked(businessPlatformOrganizationsRequestDoc).mockRejectedValue(handledClientError(401))
86113

87-
const result = withOrganizationStoreAccess(organization, async () => {
88-
throw handledClientError(401)
89-
})
90-
91-
await expect(result).rejects.toThrow("This service account can't manage dev stores in A Better Looking Shop.")
114+
await expect(organizationStoresRequestDoc(organization, request)).rejects.toThrow(
115+
"This service account can't manage dev stores in A Better Looking Shop.",
116+
)
92117
})
93118

94119
test('rethrows every other error unchanged', async () => {
95-
const boom = new Error('boom')
120+
const notFound = handledClientError(404)
121+
vi.mocked(businessPlatformOrganizationsRequestDoc).mockRejectedValue(notFound)
96122

97-
await expect(
98-
withOrganizationStoreAccess(organization, async () => {
99-
throw boom
100-
}),
101-
).rejects.toBe(boom)
123+
await expect(organizationStoresRequestDoc(organization, request)).rejects.toBe(notFound)
102124
})
103125
})

‎packages/organizations/src/cli/services/business-platform.ts‎

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
import {type Organization} from '../models/organization.js'
2-
import {type UnauthorizedHandler} from '@shopify/cli-kit/node/api/graphql'
1+
import {
2+
businessPlatformOrganizationsRequestDoc,
3+
type BusinessPlatformOrganizationsRequestOptions,
4+
} from '@shopify/cli-kit/node/api/business-platform'
5+
import {type GraphQLVariables, type UnauthorizedHandler} from '@shopify/cli-kit/node/api/graphql'
36
import {getAppAutomationToken} from '@shopify/cli-kit/node/environment'
47
import {AbortError} from '@shopify/cli-kit/node/error'
58
import {ensureAuthenticatedBusinessPlatform, isServiceAccountTokenSet} from '@shopify/cli-kit/node/session'
@@ -19,6 +22,14 @@ export function businessPlatformTokenRefreshHandler(
1922
}
2023
}
2124

25+
/**
26+
* The organization a request addresses. The name is optional because some callers only know the ID.
27+
*/
28+
export interface OrganizationReference {
29+
id: string
30+
businessName?: string
31+
}
32+
2233
/**
2334
* Whether the Organizations API refused a request because the account can't act in the organization.
2435
*
@@ -40,54 +51,50 @@ export function isOrganizationAccessError(error: unknown): boolean {
4051
/**
4152
* The error to show when the Organizations API refused access to an organization's stores.
4253
*
43-
* Names the credential that was refused: the service account, a legacy automation token, or the logged-in user.
54+
* Names the credential that was refused (the service account, a legacy automation token, or the logged-in user) and
55+
* puts the way to fix it in `nextSteps`, which both the terminal banner and the store commands' `--json` output show.
4456
*/
4557
export class OrganizationStoreAccessError extends AbortError {
46-
constructor(organization: Organization) {
47-
const where = ['in the Shopify admin under', {subdued: 'Settings > Users > Security'}]
58+
constructor(organization: OrganizationReference) {
59+
const name = organization.businessName ?? `organization ${organization.id}`
60+
const where = 'in the Shopify admin under Settings > Users > Security'
4861

4962
if (isServiceAccountTokenSet()) {
50-
super(`This service account can't manage dev stores in ${organization.businessName}.`, [
51-
'Give it',
52-
{bold: 'Dev store management'},
53-
'access',
54-
...where,
55-
{char: ','},
56-
'or use a token from a service account that has it.',
63+
super(`This service account can't manage dev stores in ${name}.`, null, [
64+
`Give the service account Dev store management access ${where}.`,
65+
'Or use a token from a service account that has that access.',
5766
])
5867
} else if (getAppAutomationToken()) {
59-
super(`This automation token can't manage dev stores in ${organization.businessName}.`, [
60-
'Store commands need a service account token with',
61-
{bold: 'Dev store management'},
62-
'access. Create one',
63-
...where,
64-
'and set it in',
65-
{command: 'SHOPIFY_SERVICE_ACCOUNT_TOKEN'},
66-
{char: '.'},
68+
super(`This automation token can't manage dev stores in ${name}.`, null, [
69+
`Create a service account with Dev store management access ${where}.`,
70+
'Set its token in the SHOPIFY_SERVICE_ACCOUNT_TOKEN environment variable.',
6771
])
6872
} else {
69-
super(
70-
`You don't have permission to manage dev stores in ${organization.businessName}.`,
71-
"Check that you're logged in to the right account, or ask an organization admin for access.",
72-
)
73+
super(`You don't have permission to manage dev stores in ${name}.`, null, [
74+
"Check that you're logged in to the right account.",
75+
'Or ask an organization admin for access.',
76+
])
7377
}
7478
}
7579
}
7680

7781
/**
78-
* Runs an Organizations API request for the organization, turning a refused request into an
79-
* `OrganizationStoreAccessError` so the user learns which access is missing rather than seeing the HTTP status.
82+
* Runs a Business Platform Organizations API request against an organization's stores.
83+
*
84+
* Every Organizations API request the store commands make goes through here, so a token that can't act in the
85+
* organization gets the same `OrganizationStoreAccessError` from every command, instead of each call site having to
86+
* remember to translate the HTTP status.
8087
*
8188
* @param organization - The organization the request addresses.
82-
* @param request - The request to run.
83-
* @returns The request's result.
89+
* @param options - The request, without the organization ID, which comes from `organization`.
90+
* @returns The response of the query.
8491
*/
85-
export async function withOrganizationStoreAccess<T>(
86-
organization: Organization,
87-
request: () => Promise<T>,
88-
): Promise<T> {
92+
export async function organizationStoresRequestDoc<TResult, TVariables extends GraphQLVariables>(
93+
organization: OrganizationReference,
94+
options: Omit<BusinessPlatformOrganizationsRequestOptions<TResult, TVariables>, 'organizationId'>,
95+
): Promise<TResult> {
8996
try {
90-
return await request()
97+
return await businessPlatformOrganizationsRequestDoc({...options, organizationId: organization.id})
9198
} catch (error) {
9299
if (isOrganizationAccessError(error)) throw new OrganizationStoreAccessError(organization)
93100
throw error

‎packages/organizations/src/cli/services/dev/create-dev-store.ts‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ import {
66
PollStoreCreationQuery,
77
} from '../../api/graphql/business-platform-organizations/generated/poll_store_creation.js'
88
import {Organization} from '../../models/organization.js'
9-
import {businessPlatformTokenRefreshHandler, withOrganizationStoreAccess} from '../business-platform.js'
10-
import {businessPlatformOrganizationsRequestDoc} from '@shopify/cli-kit/node/api/business-platform'
9+
import {businessPlatformTokenRefreshHandler, organizationStoresRequestDoc} from '../business-platform.js'
1110
import {AbortError} from '@shopify/cli-kit/node/error'
1211
import {outputContent, outputResult} from '@shopify/cli-kit/node/output'
1312
import {sleep} from '@shopify/cli-kit/node/system'
@@ -60,21 +59,18 @@ export async function createDevStore(options: CreateDevStoreOptions): Promise<st
6059
const token = await ensureAuthenticatedBusinessPlatform([], {allowAutomationToken: true})
6160
const unauthorizedHandler = businessPlatformTokenRefreshHandler()
6261

63-
const mutationResult = await withOrganizationStoreAccess(org, () =>
64-
businessPlatformOrganizationsRequestDoc({
65-
query: CreateAppDevelopmentStore,
66-
token,
67-
organizationId: org.id,
68-
variables: {
69-
shopName: name,
70-
priceLookupKey: DEV_STORE_PLANS[plan],
71-
prepopulateTestData: options.withDemoData ?? false,
72-
developerPreviewHandle: options.featurePreview,
73-
country: options.country,
74-
},
75-
unauthorizedHandler,
76-
}),
77-
)
62+
const mutationResult = await organizationStoresRequestDoc(org, {
63+
query: CreateAppDevelopmentStore,
64+
token,
65+
variables: {
66+
shopName: name,
67+
priceLookupKey: DEV_STORE_PLANS[plan],
68+
prepopulateTestData: options.withDemoData ?? false,
69+
developerPreviewHandle: options.featurePreview,
70+
country: options.country,
71+
},
72+
unauthorizedHandler,
73+
})
7874

7975
const createAppDevelopmentStore = mutationResult.createAppDevelopmentStore
8076
if (!createAppDevelopmentStore) {
@@ -101,10 +97,9 @@ export async function createDevStore(options: CreateDevStoreOptions): Promise<st
10197
}
10298

10399
// eslint-disable-next-line no-await-in-loop
104-
const pollResult = await businessPlatformOrganizationsRequestDoc({
100+
const pollResult = await organizationStoresRequestDoc(org, {
105101
query: PollStoreCreation,
106102
token,
107-
organizationId: org.id,
108103
variables: {shopDomain},
109104
unauthorizedHandler,
110105
})

‎packages/organizations/src/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ export type {Organization, OrganizationWithDetails, OrganizationStatus} from './
55
export {organizationStatusValues} from './cli/models/organization.js'
66
export {
77
businessPlatformTokenRefreshHandler,
8-
isOrganizationAccessError,
98
OrganizationStoreAccessError,
10-
withOrganizationStoreAccess,
9+
organizationStoresRequestDoc,
1110
} from './cli/services/business-platform.js'
11+
export type {OrganizationReference} from './cli/services/business-platform.js'
1212
export {createDevStore, devStorePlanHandles} from './cli/services/dev/create-dev-store.js'
1313
export type {CreateDevStoreOptions, DevStorePlan} from './cli/services/dev/create-dev-store.js'
1414
export {devStoreNamePrompt, devStorePlanPrompt, devStoreDemoDataPrompt} from './cli/prompts/dev.js'

‎packages/store/src/cli/commands/store/create/dev.test.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import StoreCreateDev from './dev.js'
22
import {createDevStore, createDevStoreJsonOutputSchema} from '../../../services/store/create/dev.js'
33
import {storeNamePrompt, storePlanPrompt, storeDemoDataPrompt} from '../../../prompts/store.js'
4-
import {selectOrg} from '@shopify/organizations'
4+
import {OrganizationStoreAccessError, selectOrg} from '@shopify/organizations'
55
import {AbortError} from '@shopify/cli-kit/node/error'
66
import {outputResult} from '@shopify/cli-kit/node/output'
77
import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system'
@@ -279,6 +279,32 @@ describe('store create dev command', () => {
279279
mockExit.mockRestore()
280280
})
281281

282+
test('includes how to grant a service account dev store access in the --json error', async () => {
283+
vi.stubEnv('SHOPIFY_SERVICE_ACCOUNT_TOKEN', 'service-account-token')
284+
vi.mocked(createDevStore).mockRejectedValueOnce(new OrganizationStoreAccessError(defaultOrg))
285+
const mockExit = vi.spyOn(process, 'exit').mockImplementation((() => {
286+
throw new Error('process.exit')
287+
}) as never)
288+
289+
await expect(
290+
StoreCreateDev.run(['--name', 'my-test-store', '--plan', 'plus', '--organization-id', '12345', '--json']),
291+
).rejects.toThrow('process.exit')
292+
293+
const parsed = JSON.parse(vi.mocked(outputResult).mock.calls[0]![0] as string)
294+
expect(parsed).toEqual({
295+
error: true,
296+
message: "This service account can't manage dev stores in Test Org.",
297+
nextSteps: [
298+
'Give the service account Dev store management access in the Shopify admin under Settings > Users > Security.',
299+
'Or use a token from a service account that has that access.',
300+
],
301+
exitCode: 1,
302+
})
303+
304+
mockExit.mockRestore()
305+
vi.unstubAllEnvs()
306+
})
307+
282308
test('outputs one standard bug error for unexpected failures when --json is active', async () => {
283309
vi.mocked(createDevStore).mockRejectedValueOnce(new Error('unexpected'))
284310

‎packages/store/src/cli/commands/store/delete.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import StoreDelete from './delete.js'
22
import {deleteDevStoreJsonOutputSchema} from '../../services/store/delete/types.js'
33
import {deleteDevStore} from '../../services/store/delete/dev.js'
44
import {resolveOrganizationForStore} from '../../utilities/store-lookup/organization.js'
5+
import {OrganizationStoreAccessError} from '@shopify/organizations'
56
import {AbortError} from '@shopify/cli-kit/node/error'
67
import {outputResult} from '@shopify/cli-kit/node/output'
78
import {isTTY, renderDangerousConfirmationPrompt} from '@shopify/cli-kit/node/ui'
@@ -172,6 +173,33 @@ describe('store delete command', () => {
172173
mockExit.mockRestore()
173174
})
174175

176+
test('includes how to grant a service account dev store access in the --json error', async () => {
177+
vi.stubEnv('SHOPIFY_SERVICE_ACCOUNT_TOKEN', 'service-account-token')
178+
vi.mocked(deleteDevStore).mockRejectedValueOnce(new OrganizationStoreAccessError(defaultOrg))
179+
const mockExit = vi.spyOn(process, 'exit').mockImplementation((() => {
180+
throw new Error('process.exit')
181+
}) as never)
182+
183+
await expect(
184+
StoreDelete.run(['--store', 'my-store.myshopify.com', '--organization-id', '12345', '--json', '--force']),
185+
).rejects.toThrow('process.exit')
186+
187+
const parsed = JSON.parse(vi.mocked(outputResult).mock.calls[0]![0] as string)
188+
expect(deleteDevStoreJsonOutputSchema.validate(parsed)).toEqual(parsed)
189+
expect(parsed).toEqual({
190+
error: true,
191+
message: "This service account can't manage dev stores in Test Org.",
192+
nextSteps: [
193+
'Give the service account Dev store management access in the Shopify admin under Settings > Users > Security.',
194+
'Or use a token from a service account that has that access.',
195+
],
196+
exitCode: 1,
197+
})
198+
199+
mockExit.mockRestore()
200+
vi.unstubAllEnvs()
201+
})
202+
175203
test('outputs structured JSON error when --json is active and organization resolution throws AbortError', async () => {
176204
vi.mocked(resolveOrganizationForStore).mockRejectedValueOnce(new AbortError('Could not resolve organization'))
177205
const mockExit = vi.spyOn(process, 'exit').mockImplementation((() => {
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
export {businessPlatformTokenRefreshHandler, withOrganizationStoreAccess} from '@shopify/organizations'
1+
export {businessPlatformTokenRefreshHandler, organizationStoresRequestDoc} from '@shopify/organizations'

0 commit comments

Comments
 (0)