Skip to content

Commit a12a7cc

Browse files
committed
Allow "one of" multi env flag requirements
Previously, the multi environment required flags array was flat and any missing flag would cause validation to fail. However, some commands provide a few different flags that will meet the same req. This commit updates validation logic to take in a nested array to account for this. Example: `['name', ['theme', 'live', 'development']]` To run with multiple environments, `name` flag and one of `theme`, `live`, or `development` are required
1 parent 6357db6 commit a12a7cc

File tree

3 files changed

+108
-8
lines changed

3 files changed

+108
-8
lines changed

.changeset/wet-lions-cross.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@shopify/theme': minor
3+
'@shopify/cli': minor
4+
---
5+
6+
Allow commands run with multiple environments to require "one of" a list of flags

packages/theme/src/cli/utilities/theme-command.test.ts

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import ThemeCommand from './theme-command.js'
1+
import ThemeCommand, {RequiredFlags} from './theme-command.js'
22
import {ensureThemeStore} from './theme-store.js'
33
import {describe, vi, expect, test, beforeEach} from 'vitest'
44
import {Config, Flags} from '@oclif/core'
@@ -29,7 +29,7 @@ class TestThemeCommand extends ThemeCommand {
2929
}),
3030
}
3131

32-
static multiEnvironmentsFlags = ['store']
32+
static multiEnvironmentsFlags: RequiredFlags = ['store']
3333

3434
commandCalls: {flags: any; session: AdminSession; multiEnvironment?: boolean; context?: any}[] = []
3535

@@ -58,6 +58,23 @@ class TestThemeCommandWithForce extends TestThemeCommand {
5858
}
5959
}
6060

61+
class TestThemeCommandWithUnionFlags extends TestThemeCommand {
62+
static multiEnvironmentsFlags: RequiredFlags = ['store', ['live', 'development', 'theme']]
63+
64+
static flags = {
65+
...TestThemeCommand.flags,
66+
development: Flags.boolean({
67+
env: 'SHOPIFY_FLAG_DEVELOPMENT',
68+
}),
69+
theme: Flags.string({
70+
env: 'SHOPIFY_FLAG_THEME_ID',
71+
}),
72+
live: Flags.boolean({
73+
env: 'SHOPIFY_FLAG_LIVE',
74+
}),
75+
}
76+
}
77+
6178
describe('ThemeCommand', () => {
6279
let mockSession: AdminSession
6380

@@ -246,6 +263,31 @@ describe('ThemeCommand', () => {
246263
expect(renderConcurrent).not.toHaveBeenCalled()
247264
})
248265

266+
test('should execute commands in environments with all required flags', async () => {
267+
// Given
268+
vi.mocked(loadEnvironment)
269+
.mockResolvedValueOnce({store: 'store1.myshopify.com', theme: 'theme1.myshopify.com'})
270+
.mockResolvedValueOnce({store: 'store2.myshopify.com', development: true})
271+
.mockResolvedValueOnce({store: 'store3.myshopify.com', live: true})
272+
273+
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
274+
vi.mocked(renderConcurrent).mockResolvedValue(undefined)
275+
276+
await CommandConfig.load()
277+
const command = new TestThemeCommandWithUnionFlags(
278+
['--environment', 'theme', '--environment', 'development', '--environment', 'live'],
279+
CommandConfig,
280+
)
281+
282+
// When
283+
await command.run()
284+
285+
// Then
286+
const renderConcurrentProcesses = vi.mocked(renderConcurrent).mock.calls[0]?.[0]?.processes
287+
expect(renderConcurrentProcesses).toHaveLength(3)
288+
expect(renderConcurrentProcesses?.map((process) => process.prefix)).toEqual(['theme', 'development', 'live'])
289+
})
290+
249291
test('should not execute commands in environments that are missing required flags', async () => {
250292
// Given
251293
vi.mocked(loadEnvironment)
@@ -271,6 +313,31 @@ describe('ThemeCommand', () => {
271313
expect(renderConcurrentProcesses?.map((process) => process.prefix)).toEqual(['development', 'production'])
272314
})
273315

316+
test('should not execute commands in environments that are missing required "one of" flags', async () => {
317+
// Given
318+
vi.mocked(loadEnvironment)
319+
.mockResolvedValueOnce({store: 'store1.myshopify.com', theme: 'theme1.myshopify.com'})
320+
.mockResolvedValueOnce({store: 'store2.myshopify.com'})
321+
.mockResolvedValueOnce({store: 'store3.myshopify.com', live: true})
322+
323+
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
324+
vi.mocked(renderConcurrent).mockResolvedValue(undefined)
325+
326+
await CommandConfig.load()
327+
const command = new TestThemeCommandWithUnionFlags(
328+
['--environment', 'theme', '--environment', 'missing-theme-live-or-development', '--environment', 'live'],
329+
CommandConfig,
330+
)
331+
332+
// When
333+
await command.run()
334+
335+
// Then
336+
const renderConcurrentProcesses = vi.mocked(renderConcurrent).mock.calls[0]?.[0]?.processes
337+
expect(renderConcurrentProcesses).toHaveLength(2)
338+
expect(renderConcurrentProcesses?.map((process) => process.prefix)).toEqual(['theme', 'live'])
339+
})
340+
274341
test('commands error gracefully and continue with other environments', async () => {
275342
// Given
276343
vi.mocked(loadEnvironment).mockResolvedValue({store: 'store.myshopify.com'})

packages/theme/src/cli/utilities/theme-command.ts

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,21 @@ interface PassThroughFlagsOptions {
2222
allowedFlags?: string[]
2323
}
2424
type EnvironmentName = string
25+
/**
26+
* Flags required to run a command in multiple environments
27+
*
28+
* If the command does not support multiple environments, set to null
29+
*
30+
* Otherwise, each element can be:
31+
* - string: A required flag
32+
* - string[]: Multiple flags where at least one is required,
33+
* ordered by precedence
34+
*
35+
* @example
36+
* // store, password, and one of: live, development, or theme
37+
* ['store', 'password', ['live', 'development', 'theme']]
38+
*/
39+
export type RequiredFlags = (string | string[])[] | null
2540

2641
export default abstract class ThemeCommand extends Command {
2742
passThroughFlags(flags: FlagValues, {allowedFlags}: PassThroughFlagsOptions): string[] {
@@ -58,7 +73,7 @@ export default abstract class ThemeCommand extends Command {
5873
>(_opts?: Input<TFlags, TGlobalFlags, TArgs>): Promise<void> {
5974
// Parse command flags using the current command class definitions
6075
const klass = this.constructor as unknown as Input<TFlags, TGlobalFlags, TArgs> & {
61-
multiEnvironmentsFlags: string[] | null
76+
multiEnvironmentsFlags: RequiredFlags
6277
flags: FlagOutput
6378
}
6479
const requiredFlags = klass.multiEnvironmentsFlags
@@ -128,7 +143,10 @@ export default abstract class ThemeCommand extends Command {
128143
* @param requiredFlags - The required flags to check for
129144
* @returns An object containing valid and invalid environment arrays
130145
*/
131-
private async validateEnvironments(environmentMap: Map<EnvironmentName, FlagValues>, requiredFlags: string[]) {
146+
private async validateEnvironments(
147+
environmentMap: Map<EnvironmentName, FlagValues>,
148+
requiredFlags: Exclude<RequiredFlags, null>,
149+
) {
132150
const valid: {environment: EnvironmentName; flags: FlagValues; session: AdminSession}[] = []
133151
const invalid: {environment: EnvironmentName; reason: string}[] = []
134152

@@ -158,7 +176,7 @@ export default abstract class ThemeCommand extends Command {
158176
*/
159177
private async showConfirmation(
160178
commandName: string,
161-
requiredFlags: string[],
179+
requiredFlags: Exclude<RequiredFlags, null>,
162180
validationResults: {
163181
valid: {environment: string; flags: FlagValues}[]
164182
invalid: {environment: string; reason: string}[]
@@ -176,7 +194,10 @@ export default abstract class ThemeCommand extends Command {
176194
const environmentDetails = [
177195
...validationResults.valid.map(({environment, flags}) => {
178196
const flagDetails = requiredFlags
179-
.map((flag) => (flag.includes('password') ? flag : `${flag}: ${String(flags[flag])}`))
197+
.map((flag) => {
198+
const usedFlag = Array.isArray(flag) ? flag.find((flag) => flags[flag]) : flag
199+
return usedFlag && [usedFlag.includes('password') ? usedFlag : `${usedFlag}: ${flags[usedFlag]}`]
200+
})
180201
.join(', ')
181202

182203
return [environment, {subdued: flagDetails || 'No flags required'}]
@@ -241,8 +262,14 @@ export default abstract class ThemeCommand extends Command {
241262
* @param environmentName - The name of the environment
242263
* @returns The missing flags or true if the environment has all required flags
243264
*/
244-
private validConfig(environmentFlags: FlagValues, requiredFlags: string[], environmentName: string): string[] | true {
245-
const missingFlags = requiredFlags.filter((flag) => !environmentFlags[flag])
265+
private validConfig(
266+
environmentFlags: FlagValues,
267+
requiredFlags: Exclude<RequiredFlags, null>,
268+
environmentName: string,
269+
): string[] | true {
270+
const missingFlags = requiredFlags
271+
.filter((flag) => (Array.isArray(flag) ? !flag.some((flag) => environmentFlags[flag]) : !environmentFlags[flag]))
272+
.map((flag) => (Array.isArray(flag) ? flag.join(' or ') : flag))
246273

247274
if (missingFlags.length > 0) {
248275
renderWarning({

0 commit comments

Comments
 (0)