Skip to content

Commit 1f2038b

Browse files
jenstanicakclaude
andcommitted
Address review feedback on group ordering and add tests
- Move groupOrder prop from RenderSelectPromptOptions and RenderAutocompleteOptions to their respective base prompt props interfaces - Simplify SelectInput sorting logic to use single function instead of array with both function and 'group' - Fix group order extraction to use original templates before filtering to preserve intended order - Add comprehensive tests for groupOrder extraction and UI sorting functionality - Remove unused outputDebug imports and debug logging statements 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 00ae388 commit 1f2038b

File tree

6 files changed

+71
-14
lines changed

6 files changed

+71
-14
lines changed

packages/app/src/cli/prompts/generate/extension.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {renderAutocompletePrompt, renderSelectPrompt, renderTextPrompt} from '@s
66
import {AbortError} from '@shopify/cli-kit/node/error'
77
import {joinPath} from '@shopify/cli-kit/node/path'
88
import {slugify} from '@shopify/cli-kit/common/string'
9-
import {outputDebug} from '@shopify/cli-kit/node/output'
109

1110
export interface GenerateExtensionPromptOptions {
1211
name?: string

packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,34 @@ describe('templateSpecifications', () => {
206206
expect(gotLabels).toEqual(expectedAllowedTemplates.map((template) => template.name))
207207
})
208208

209+
test('extracts groupOrder correctly from template order', async () => {
210+
// Given
211+
const orgApp = testOrganizationApp()
212+
const templates: GatedExtensionTemplate[] = [
213+
{...templateWithoutRules, group: 'GroupA'},
214+
{...allowedTemplate, group: 'GroupB'},
215+
{...templateWithoutRules, identifier: 'template3', group: 'GroupA'}, // Same group as first
216+
{...allowedTemplate, identifier: 'template4', group: 'GroupC'},
217+
]
218+
const mockedFetch = vi.fn().mockResolvedValueOnce(Response.json(templates))
219+
vi.mocked(fetch).mockImplementation(mockedFetch)
220+
const mockedFetchFlagsResponse: OrganizationBetaFlagsQuerySchema = {
221+
organization: {
222+
id: encodedGidFromOrganizationId(orgApp.organizationId),
223+
flag_allowedFlag: true,
224+
},
225+
}
226+
vi.mocked(businessPlatformOrganizationsRequest).mockResolvedValueOnce(mockedFetchFlagsResponse)
227+
228+
// When
229+
const client = new AppManagementClient()
230+
client.businessPlatformToken = () => Promise.resolve('business-platform-token')
231+
const {groupOrder} = await client.templateSpecifications(orgApp)
232+
233+
// Then
234+
expect(groupOrder).toEqual(['GroupA', 'GroupB', 'GroupC'])
235+
})
236+
209237
test('fails with an error message when fetching the specifications list fails', async () => {
210238
// Given
211239
vi.mocked(fetch).mockRejectedValueOnce(new Error('Failed to fetch'))

packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,10 +473,10 @@ export class AppManagementClient implements DeveloperPlatformClient {
473473
)
474474
).map((template) => ({...template, sortPriority: counter++}))
475475

476-
// Extract group order from the original template order
476+
// Extract group order from the original template order (before filtering)
477477
const groupOrder: string[] = []
478-
for (const template of filteredTemplates) {
479-
if (!groupOrder.includes(template.group)) {
478+
for (const template of templates) {
479+
if (template.group && !groupOrder.includes(template.group)) {
480480
groupOrder.push(template.group)
481481
}
482482
}

packages/cli-kit/src/private/node/ui/components/SelectInput.test.tsx

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,41 @@ describe('SelectInput', async () => {
230230
expect(onChange).toHaveBeenLastCalledWith(items[2])
231231
})
232232

233+
test.skipIf(runningOnWindows)('respects groupOrder for custom group ordering', async () => {
234+
const onChange = vi.fn()
235+
236+
const items = [
237+
{label: 'first', value: 'first', group: 'GroupA'},
238+
{label: 'second', value: 'second', group: 'GroupA'},
239+
{label: 'third', value: 'third', group: 'GroupB'},
240+
{label: 'fourth', value: 'fourth', group: 'GroupB'},
241+
{label: 'fifth', value: 'fifth', group: 'GroupC'},
242+
{label: 'sixth', value: 'sixth', group: 'GroupC'},
243+
]
244+
245+
// Custom order: GroupC first, then GroupB, then GroupA
246+
const groupOrder = ['GroupC', 'GroupB', 'GroupA']
247+
248+
const renderInstance = render(<SelectInput items={items} onChange={onChange} groupOrder={groupOrder} />)
249+
250+
expect(renderInstance.lastFrame()).toMatchInlineSnapshot(`
251+
" [1mGroupC[22m
252+
[36m>[39m [36mfifth[39m
253+
sixth
254+
255+
[1mGroupB[22m
256+
third
257+
fourth
258+
259+
[1mGroupA[22m
260+
first
261+
second
262+
263+
[2mPress ↑↓ arrows to select, enter to confirm.[22m"
264+
`)
265+
expect(onChange).not.toHaveBeenCalled()
266+
})
267+
233268
test('allows disabling shortcuts', async () => {
234269
const onChange = vi.fn()
235270
const items = [

packages/cli-kit/src/private/node/ui/components/SelectInput.tsx

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,11 @@ function SelectInputInner<T>(
163163
}
164164

165165
const hasAnyGroup = rawItems.some((item) => typeof item.group !== 'undefined')
166-
const items = sortBy(rawItems, [
167-
(item) => {
168-
if (!groupOrder || !item.group) return Number.MAX_SAFE_INTEGER
169-
const index = groupOrder.indexOf(item.group)
170-
return index === -1 ? Number.MAX_SAFE_INTEGER : index
171-
},
172-
'group',
173-
])
166+
const items = sortBy(rawItems, (item) => {
167+
if (!groupOrder || !item.group) return Number.MAX_SAFE_INTEGER
168+
const index = groupOrder.indexOf(item.group)
169+
return index === -1 ? Number.MAX_SAFE_INTEGER : index
170+
})
174171
const itemsHaveKeys = items.some((item) => typeof item.key !== 'undefined' && item.key.length > 0)
175172

176173
if (itemsHaveKeys) validateKeys(items)

packages/cli-kit/src/public/node/ui.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,6 @@ export function renderFatalError(error: Fatal, {renderOptions}: RenderFatalError
242242
export interface RenderSelectPromptOptions<T> extends Omit<SelectPromptProps<T>, 'onSubmit'> {
243243
isConfirmationPrompt?: boolean
244244
renderOptions?: RenderOptions
245-
groupOrder?: string[]
246245
}
247246

248247
/**
@@ -366,7 +365,6 @@ export async function renderConfirmationPrompt({
366365
export interface RenderAutocompleteOptions<T>
367366
extends PartialBy<Omit<AutocompletePromptProps<T>, 'onSubmit'>, 'search'> {
368367
renderOptions?: RenderOptions
369-
groupOrder?: string[]
370368
}
371369

372370
/**

0 commit comments

Comments
 (0)