Skip to content
Merged
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
5 changes: 5 additions & 0 deletions .changeset/quiet-crabs-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/app': patch
---

Avoid checking flags for unrelated extension templates when generating a requested template.
22 changes: 22 additions & 0 deletions packages/app/src/cli/services/generate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down
1 change: 1 addition & 0 deletions packages/app/src/cli/services/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ async function generate(options: GenerateOptions) {
developerPlatformClient,
remoteApp,
availableSpecifications,
{requestedTemplate: template},
)

const promptOptions = await buildPromptOptions(extensionTemplates, groupOrder, specifications, app, options)
Expand Down
Original file line number Diff line number Diff line change
@@ -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 () => {
Expand All @@ -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 = [
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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<ExtensionTemplatesResult> {
const {templates: remoteTemplates, groupOrder} = await developerPlatformClient.templateSpecifications(app)
const {templates: remoteTemplates, groupOrder} = await developerPlatformClient.templateSpecifications(app, options)

const filteredTemplates = remoteTemplates.filter(
(template) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,10 @@ interface ErrorDetail {
[key: string]: unknown
}

export interface TemplateSpecificationsOptions {
requestedTemplate?: string
}

export interface DeveloperPlatformClient {
readonly clientName: ClientName
readonly webUiName: string
Expand All @@ -250,7 +254,10 @@ export interface DeveloperPlatformClient {
orgAndApps: (orgId: string) => Promise<Paginateable<{organization: Organization; apps: MinimalOrganizationApp[]}>>
appsForOrg: (orgId: string, term?: string) => Promise<Paginateable<{apps: MinimalOrganizationApp[]}>>
specifications: (app: MinimalAppIdentifiers) => Promise<RemoteSpecification[]>
templateSpecifications: (app: MinimalAppIdentifiers) => Promise<ExtensionTemplatesResult>
templateSpecifications: (
app: MinimalAppIdentifiers,
options?: TemplateSpecificationsOptions,
) => Promise<ExtensionTemplatesResult>
createApp: (org: Organization, options: CreateAppOptions) => Promise<OrganizationApp>
devStoresForOrg: (orgId: string, searchTerm?: string) => Promise<Paginateable<{stores: OrganizationStore[]}>>
storeByDomain: (orgId: string, shopDomain: string, storeTypes: Store[]) => Promise<OrganizationStore | undefined>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {environmentVariableNames} from '../../constants.js'
import {RemoteSpecification} from '../../api/graphql/extension_specifications.js'
import {
DeveloperPlatformClient,
TemplateSpecificationsOptions,
Paginateable,
AppVersion,
AppVersionWithContext,
Expand Down Expand Up @@ -464,7 +465,10 @@ export class AppManagementClient implements DeveloperPlatformClient {
)
}

async templateSpecifications({organizationId}: MinimalAppIdentifiers): Promise<ExtensionTemplatesResult> {
async templateSpecifications(
{organizationId}: MinimalAppIdentifiers,
options: TemplateSpecificationsOptions = {},
): Promise<ExtensionTemplatesResult> {
let templates: GatedExtensionTemplate[]
const {templatesJsonPath} = environmentVariableNames
const overrideFile = process.env[templatesJsonPath]
Expand All @@ -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),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
AssetUrlSchema,
AppVersionIdentifiers,
DeveloperPlatformClient,
TemplateSpecificationsOptions,
Paginateable,
filterDisabledFlags,
ClientName,
Expand Down Expand Up @@ -352,7 +353,10 @@ export class PartnersClient implements DeveloperPlatformClient {
}))
}

async templateSpecifications({apiKey}: MinimalAppIdentifiers): Promise<ExtensionTemplatesResult> {
async templateSpecifications(
{apiKey}: MinimalAppIdentifiers,
_options: TemplateSpecificationsOptions = {},
): Promise<ExtensionTemplatesResult> {
const variables: RemoteTemplateSpecificationsVariables = {apiKey}
const result: RemoteTemplateSpecificationsSchema = await this.request(RemoteTemplateSpecificationsQuery, variables)
const templates = result.templateSpecifications.map((template) => {
Expand Down
Loading