Skip to content

Commit 199fa91

Browse files
committed
Exclude default values from multi-env flag requirements
Previously, default flag values were allowed to meet mutli environment command run flag requirements. This commit changes that to only allow requirements to be met by user provided values
1 parent acf3248 commit 199fa91

File tree

2 files changed

+41
-14
lines changed

2 files changed

+41
-14
lines changed

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ class TestThemeCommandWithUnionFlags extends TestThemeCommand {
8484
}),
8585
}
8686
}
87+
class TestThemeCommandWithPath extends TestThemeCommand {
88+
static multiEnvironmentsFlags: RequiredFlags = ['store', 'path']
89+
}
8790

8891
class TestUnauthenticatedThemeCommand extends ThemeCommand {
8992
static flags = {
@@ -390,6 +393,31 @@ describe('ThemeCommand', () => {
390393
expect(renderConcurrentProcesses?.map((process) => process.prefix)).toEqual(['development', 'production'])
391394
})
392395

396+
test('should not execute commands in environments that are missing required flags even if they have a default value', async () => {
397+
// Given
398+
vi.mocked(loadEnvironment)
399+
.mockResolvedValueOnce({store: 'store1.myshopify.com', path: '/a/path'})
400+
.mockResolvedValueOnce({})
401+
.mockResolvedValueOnce({store: 'store3.myshopify.com'})
402+
403+
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
404+
vi.mocked(renderConcurrent).mockResolvedValue(undefined)
405+
406+
await CommandConfig.load()
407+
const command = new TestThemeCommandWithPath(
408+
['--environment', 'development', '--environment', 'env-missing-store', '--environment', 'path-defaults-to-cwd'],
409+
CommandConfig,
410+
)
411+
412+
// When
413+
await command.run()
414+
415+
// Then
416+
const renderConcurrentProcesses = vi.mocked(renderConcurrent).mock.calls[0]?.[0]?.processes
417+
expect(renderConcurrentProcesses).toHaveLength(1)
418+
expect(renderConcurrentProcesses?.map((process) => process.prefix)).toEqual(['development'])
419+
})
420+
393421
test('should not execute commands in environments that are missing required "one of" flags', async () => {
394422
// Given
395423
vi.mocked(loadEnvironment)

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

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,8 @@ export default abstract class ThemeCommand extends Command {
137137
* @param flagsWithoutDefaults - Flags provided via the CLI
138138
* @returns The map of environments
139139
*/
140-
private async loadEnvironments(
141-
environments: EnvironmentName[],
142-
flags: FlagValues,
143-
flagsWithoutDefaults: FlagValues,
144-
): Promise<Map<EnvironmentName, FlagValues>> {
145-
const environmentMap = new Map<EnvironmentName, FlagValues>()
140+
private async loadEnvironments(environments: EnvironmentName[], flags: FlagValues, flagsWithoutDefaults: FlagValues) {
141+
const environmentMap = new Map<EnvironmentName, {flags: FlagValues; validationFlags: FlagValues}>()
146142

147143
for (const environmentName of environments) {
148144
// eslint-disable-next-line no-await-in-loop
@@ -157,10 +153,13 @@ export default abstract class ThemeCommand extends Command {
157153
}
158154

159155
environmentMap.set(environmentName, {
160-
...flags,
161-
...environmentFlags,
162-
...flagsWithoutDefaults,
163-
environment: [environmentName],
156+
flags: {
157+
...flags,
158+
...environmentFlags,
159+
...flagsWithoutDefaults,
160+
environment: [environmentName],
161+
},
162+
validationFlags: {...environmentFlags, ...flagsWithoutDefaults} as FlagValues,
164163
})
165164
}
166165

@@ -175,21 +174,21 @@ export default abstract class ThemeCommand extends Command {
175174
* @returns An object containing valid and invalid environment arrays
176175
*/
177176
private async validateEnvironments(
178-
environmentMap: Map<EnvironmentName, FlagValues>,
177+
environmentMap: Map<EnvironmentName, {flags: FlagValues; validationFlags: FlagValues}>,
179178
requiredFlags: Exclude<RequiredFlags, null>,
180179
requiresAuth: boolean,
181180
) {
182181
const valid: ValidEnvironment[] = []
183182
const invalid: {environment: EnvironmentName; reason: string}[] = []
184183

185-
for (const [environmentName, environmentFlags] of environmentMap) {
186-
const validationResult = this.validConfig(environmentFlags, requiredFlags, environmentName)
184+
for (const [environmentName, {flags, validationFlags}] of environmentMap) {
185+
const validationResult = this.validConfig(validationFlags, requiredFlags, environmentName)
187186
if (validationResult !== true) {
188187
const missingFlagsText = validationResult.join(', ')
189188
invalid.push({environment: environmentName, reason: `Missing flags: ${missingFlagsText}`})
190189
continue
191190
}
192-
valid.push({environment: environmentName, flags: environmentFlags, requiresAuth})
191+
valid.push({environment: environmentName, flags, requiresAuth})
193192
}
194193

195194
return {valid, invalid}

0 commit comments

Comments
 (0)