diff --git a/.changeset/quiet-crabs-fix.md b/.changeset/quiet-crabs-fix.md new file mode 100644 index 00000000000..81a5318ad3c --- /dev/null +++ b/.changeset/quiet-crabs-fix.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Avoid checking flags for unrelated extension templates when generating a requested template. diff --git a/packages/app/src/cli/services/generate.test.ts b/packages/app/src/cli/services/generate.test.ts index 925550e7003..f8c5e4ab7be 100644 --- a/packages/app/src/cli/services/generate.test.ts +++ b/packages/app/src/cli/services/generate.test.ts @@ -149,6 +149,28 @@ describe('generate', () => { `) }) + test('passes the requested template when fetching extension templates', async () => { + // Given + await mockSuccessfulCommandExecution('cart_checkout_validation') + const templateSpecifications = vi.fn().mockResolvedValue({templates: testRemoteExtensionTemplates, groupOrder: []}) + developerPlatformClient = testDeveloperPlatformClient({templateSpecifications}) + + // When + await generate({ + directory: '/', + reset: false, + app, + project: testProject(), + remoteApp, + specifications, + developerPlatformClient, + template: 'cart_checkout_validation', + }) + + // Then + expect(templateSpecifications).toHaveBeenCalledWith(remoteApp, {requestedTemplate: 'cart_checkout_validation'}) + }) + test('throws error if trying to generate a non existing type', async () => { await mockSuccessfulCommandExecution('subscription_ui') diff --git a/packages/app/src/cli/services/generate.ts b/packages/app/src/cli/services/generate.ts index 866a2e136ca..251f94808b4 100644 --- a/packages/app/src/cli/services/generate.ts +++ b/packages/app/src/cli/services/generate.ts @@ -46,6 +46,7 @@ async function generate(options: GenerateOptions) { developerPlatformClient, remoteApp, availableSpecifications, + {requestedTemplate: template}, ) const promptOptions = await buildPromptOptions(extensionTemplates, groupOrder, specifications, app, options) diff --git a/packages/app/src/cli/services/generate/fetch-template-specifications.test.ts b/packages/app/src/cli/services/generate/fetch-template-specifications.test.ts index da32d07f2a2..58151834742 100644 --- a/packages/app/src/cli/services/generate/fetch-template-specifications.test.ts +++ b/packages/app/src/cli/services/generate/fetch-template-specifications.test.ts @@ -1,7 +1,7 @@ import {fetchExtensionTemplates} from './fetch-template-specifications.js' import {ExtensionFlavorValue} from './extension.js' import {testDeveloperPlatformClient, testOrganizationApp} from '../../models/app/app.test-data.js' -import {describe, expect, test} from 'vitest' +import {describe, expect, test, vi} from 'vitest' describe('fetchTemplateSpecifications', () => { test('returns the remote specs', async () => { @@ -27,6 +27,20 @@ describe('fetchTemplateSpecifications', () => { expect(identifiers).not.toContain('ui_extension') }) + test('passes the requested template to the developer platform client', async () => { + // Given + const orgApp = testOrganizationApp() + const templateSpecifications = vi.fn().mockResolvedValue({templates: [], groupOrder: []}) + + // When + await fetchExtensionTemplates(testDeveloperPlatformClient({templateSpecifications}), orgApp, ['function'], { + requestedTemplate: 'discount', + }) + + // Then + expect(templateSpecifications).toHaveBeenCalledWith(orgApp, {requestedTemplate: 'discount'}) + }) + describe('ui_extension', () => { const allFlavors = [ { diff --git a/packages/app/src/cli/services/generate/fetch-template-specifications.ts b/packages/app/src/cli/services/generate/fetch-template-specifications.ts index 66cfc6c3745..73be3a5a2d6 100644 --- a/packages/app/src/cli/services/generate/fetch-template-specifications.ts +++ b/packages/app/src/cli/services/generate/fetch-template-specifications.ts @@ -1,13 +1,14 @@ import {ExtensionTemplatesResult} from '../../models/app/template.js' import {MinimalAppIdentifiers} from '../../models/organization.js' -import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' +import {DeveloperPlatformClient, TemplateSpecificationsOptions} from '../../utilities/developer-platform-client.js' export async function fetchExtensionTemplates( developerPlatformClient: DeveloperPlatformClient, app: MinimalAppIdentifiers, availableSpecifications: string[], + options: TemplateSpecificationsOptions = {}, ): Promise { - const {templates: remoteTemplates, groupOrder} = await developerPlatformClient.templateSpecifications(app) + const {templates: remoteTemplates, groupOrder} = await developerPlatformClient.templateSpecifications(app, options) const filteredTemplates = remoteTemplates.filter( (template) => diff --git a/packages/app/src/cli/utilities/developer-platform-client.ts b/packages/app/src/cli/utilities/developer-platform-client.ts index e8d792f0de1..b058f1b407f 100644 --- a/packages/app/src/cli/utilities/developer-platform-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client.ts @@ -227,6 +227,10 @@ interface ErrorDetail { [key: string]: unknown } +export interface TemplateSpecificationsOptions { + requestedTemplate?: string +} + export interface DeveloperPlatformClient { readonly clientName: ClientName readonly webUiName: string @@ -250,7 +254,10 @@ export interface DeveloperPlatformClient { orgAndApps: (orgId: string) => Promise> appsForOrg: (orgId: string, term?: string) => Promise> specifications: (app: MinimalAppIdentifiers) => Promise - templateSpecifications: (app: MinimalAppIdentifiers) => Promise + templateSpecifications: ( + app: MinimalAppIdentifiers, + options?: TemplateSpecificationsOptions, + ) => Promise createApp: (org: Organization, options: CreateAppOptions) => Promise devStoresForOrg: (orgId: string, searchTerm?: string) => Promise> storeByDomain: (orgId: string, shopDomain: string, storeTypes: Store[]) => Promise diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts index eb58a9f893b..ce225c38957 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts @@ -257,6 +257,126 @@ describe('templateSpecifications', () => { expect(groupOrder).toEqual(['GroupA', 'GroupB', 'GroupC']) }) + test('does not fetch flags for unrelated templates when a template is requested', async () => { + // Given + const orgApp = testOrganizationApp() + const requestedTemplate: GatedExtensionTemplate = { + ...templateWithoutRules, + identifier: 'discount', + } + const templateWithBetaFlag: GatedExtensionTemplate = { + ...testRemoteExtensionTemplates[1]!, + identifier: 'app_action_link', + organizationBetaFlags: ['unrelated_beta_flag'], + } + const templateWithExpFlag: GatedExtensionTemplate = { + ...testRemoteExtensionTemplates[2]!, + identifier: 'app_action', + organizationExpFlags: ['unrelated_exp_flag'], + } + const templates: GatedExtensionTemplate[] = [requestedTemplate, templateWithBetaFlag, templateWithExpFlag] + const mockedFetch = vi.fn().mockResolvedValueOnce(Response.json(templates)) + vi.mocked(fetch).mockImplementation(mockedFetch) + + // When + const client = AppManagementClient.getInstance() + const {templates: got} = await client.templateSpecifications(orgApp, {requestedTemplate: 'discount'}) + + // Then + expect(vi.mocked(businessPlatformOrganizationsRequest)).not.toHaveBeenCalled() + expect(vi.mocked(businessPlatformOrganizationsRequestDoc)).not.toHaveBeenCalled() + expect(got.map((template) => template.identifier)).toEqual(['discount']) + }) + + test('fetches flags for the requested template when the requested template is gated', async () => { + // Given + const orgApp = testOrganizationApp() + const templateWithBetaFlag: GatedExtensionTemplate = { + ...testRemoteExtensionTemplates[1]!, + identifier: 'app_action_link', + organizationBetaFlags: ['requested_beta_flag'], + } + const templateWithExpFlag: GatedExtensionTemplate = { + ...testRemoteExtensionTemplates[2]!, + identifier: 'app_action', + organizationExpFlags: ['unrelated_exp_flag'], + } + const templates: GatedExtensionTemplate[] = [templateWithoutRules, templateWithBetaFlag, templateWithExpFlag] + const mockedFetch = vi.fn().mockResolvedValueOnce(Response.json(templates)) + vi.mocked(fetch).mockImplementation(mockedFetch) + + const mockedFetchFlagsResponse: OrganizationBetaFlagsQuerySchema = { + organization: { + id: encodedGidFromOrganizationIdForBP(orgApp.organizationId), + flag_requested_beta_flag: true, + }, + } + vi.mocked(businessPlatformOrganizationsRequest).mockResolvedValueOnce(mockedFetchFlagsResponse) + + // When + const client = AppManagementClient.getInstance() + client.businessPlatformToken = () => Promise.resolve('business-platform-token') + const {templates: got} = await client.templateSpecifications(orgApp, {requestedTemplate: 'app_action_link'}) + + // Then + expect(vi.mocked(businessPlatformOrganizationsRequest)).toHaveBeenCalledWith({ + query: expect.stringContaining('flag_requested_beta_flag: hasFeatureFlag(handle: "requested_beta_flag")'), + token: 'business-platform-token', + organizationId: orgApp.organizationId, + variables: { + organizationId: encodedGidFromOrganizationIdForBP(orgApp.organizationId), + }, + unauthorizedHandler: { + type: 'token_refresh', + handler: expect.any(Function), + }, + }) + expect(vi.mocked(businessPlatformOrganizationsRequestDoc)).not.toHaveBeenCalled() + expect(got.map((template) => template.identifier)).toEqual(['app_action_link']) + }) + + test('falls back to the full template list when the requested template is unknown', async () => { + // Given + const orgApp = testOrganizationApp() + const templateWithBetaFlag: GatedExtensionTemplate = { + ...testRemoteExtensionTemplates[1]!, + organizationBetaFlags: ['allowed_flag'], + } + const templateWithDisallowedBetaFlag: GatedExtensionTemplate = { + ...testRemoteExtensionTemplates[2]!, + organizationBetaFlags: ['not_allowed_flag'], + } + const templates: GatedExtensionTemplate[] = [ + templateWithoutRules, + templateWithBetaFlag, + templateWithDisallowedBetaFlag, + ] + const mockedFetch = vi.fn().mockResolvedValueOnce(Response.json(templates)) + vi.mocked(fetch).mockImplementation(mockedFetch) + + const mockedFetchFlagsResponse: OrganizationBetaFlagsQuerySchema = { + organization: { + id: encodedGidFromOrganizationIdForBP(orgApp.organizationId), + flag_allowed_flag: true, + flag_not_allowed_flag: false, + }, + } + vi.mocked(businessPlatformOrganizationsRequest).mockResolvedValueOnce(mockedFetchFlagsResponse) + + // When + const client = AppManagementClient.getInstance() + client.businessPlatformToken = () => Promise.resolve('business-platform-token') + const {templates: got} = await client.templateSpecifications(orgApp, {requestedTemplate: 'unknown_template'}) + + // Then + expect(vi.mocked(businessPlatformOrganizationsRequest)).toHaveBeenCalled() + expect(vi.mocked(businessPlatformOrganizationsRequestDoc)).not.toHaveBeenCalled() + expect(got.map((template) => template.identifier)).toEqual([ + templateWithoutRules.identifier, + templateWithBetaFlag.identifier, + ]) + }) + test('fetches and filters templates by exp flags using enabledFlags', async () => { // Given const orgApp = testOrganizationApp() 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 8e7c79d4992..3bb3ee7528c 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 @@ -12,6 +12,7 @@ import {environmentVariableNames} from '../../constants.js' import {RemoteSpecification} from '../../api/graphql/extension_specifications.js' import { DeveloperPlatformClient, + TemplateSpecificationsOptions, Paginateable, AppVersion, AppVersionWithContext, @@ -464,7 +465,10 @@ export class AppManagementClient implements DeveloperPlatformClient { ) } - async templateSpecifications({organizationId}: MinimalAppIdentifiers): Promise { + async templateSpecifications( + {organizationId}: MinimalAppIdentifiers, + options: TemplateSpecificationsOptions = {}, + ): Promise { let templates: GatedExtensionTemplate[] const {templatesJsonPath} = environmentVariableNames const overrideFile = process.env[templatesJsonPath] @@ -491,9 +495,13 @@ export class AppManagementClient implements DeveloperPlatformClient { // in the static JSON file. This can be removed once PartnersClient, which // uses sortPriority, is gone. let counter = 0 + const requestedTemplates = options.requestedTemplate + ? templates.filter((template) => template.identifier === options.requestedTemplate) + : undefined + const templatesToFilter = requestedTemplates && requestedTemplates.length > 0 ? requestedTemplates : templates const filteredTemplates = ( await allowedTemplates( - templates, + templatesToFilter, async (betaFlags: string[]) => this.organizationBetaFlags(organizationId, betaFlags), async (expFlags: string[]) => this.organizationExpFlags(organizationId, expFlags), ) 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 57e142a35a9..b80967269a5 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 @@ -5,6 +5,7 @@ import { AssetUrlSchema, AppVersionIdentifiers, DeveloperPlatformClient, + TemplateSpecificationsOptions, Paginateable, filterDisabledFlags, ClientName, @@ -352,7 +353,10 @@ export class PartnersClient implements DeveloperPlatformClient { })) } - async templateSpecifications({apiKey}: MinimalAppIdentifiers): Promise { + async templateSpecifications( + {apiKey}: MinimalAppIdentifiers, + _options: TemplateSpecificationsOptions = {}, + ): Promise { const variables: RemoteTemplateSpecificationsVariables = {apiKey} const result: RemoteTemplateSpecificationsSchema = await this.request(RemoteTemplateSpecificationsQuery, variables) const templates = result.templateSpecifications.map((template) => {