Skip to content

Commit

Permalink
Move action required message to the spec (#4879)
Browse files Browse the repository at this point in the history
<!--
  ☝️How to write a good PR title:
  - Prefix it with [Feature] (if applicable)
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Use a draft PR while it’s a work in progress
-->

### WHY are these changes introduced?
Move the responsibility of the "Action Required" message to the specification instead of hardcoding the message in the `dev-session`

<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?
Add a new `getDevSessionActionUpdateMessage` to the extension-instance and the extension-specification. And define it for the app access module
<!--
  Summary of the changes committed.
  Before / after screenshots appreciated for UI changes.
-->

### How to test your changes?
Run consistent-dev, make a change in the scopes, you should see the action required message.
<!--
  Please, provide steps for the reviewer to test your changes locally.
-->

### Post-release steps

<!--
  If changes require post-release steps, for example merging and publishing some documentation changes,
  specify it in this section and add the label "includes-post-release-steps".
  If it doesn't, feel free to remove this section.
-->

### Measuring impact

How do we know this change was effective? Please choose one:

- [ ] n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
- [ ] Existing analytics will cater for this addition
- [ ] PR includes analytics changes to measure impact

### Checklist

- [ ] I've considered possible cross-platform impacts (Mac, Linux, Windows)
- [ ] I've considered possible [documentation](https://shopify.dev) changes
  • Loading branch information
isaacroldan authored Nov 25, 2024
2 parents 330ec0b + 3ef6f86 commit 8a78cb3
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 11 deletions.
10 changes: 9 additions & 1 deletion packages/app/src/cli/models/extensions/extension-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
import {bundleThemeExtension} from '../../services/extensions/bundle.js'
import {Identifiers} from '../app/identifiers.js'
import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {AppConfigurationWithoutPath} from '../app/app.js'
import {AppConfigurationWithoutPath, CurrentAppConfiguration} from '../app/app.js'
import {ok} from '@shopify/cli-kit/node/result'
import {constantize, slugify} from '@shopify/cli-kit/common/string'
import {hashString, randomUUID} from '@shopify/cli-kit/node/crypto'
Expand Down Expand Up @@ -418,6 +418,14 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
}
}

async getDevSessionActionUpdateMessage(
appConfig: CurrentAppConfiguration,
storeFqdn: string,
): Promise<string | undefined> {
if (!this.specification.getDevSessionActionUpdateMessage) return undefined
return this.specification.getDevSessionActionUpdateMessage(this.configuration, appConfig, storeFqdn)
}

private buildHandle() {
switch (this.specification.uidStrategy) {
case 'single':
Expand Down
14 changes: 13 additions & 1 deletion packages/app/src/cli/models/extensions/specification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {ExtensionInstance} from './extension-instance.js'
import {blocks} from '../../constants.js'

import {Flag} from '../../utilities/developer-platform-client.js'
import {AppConfigurationWithoutPath} from '../app/app.js'
import {AppConfigurationWithoutPath, CurrentAppConfiguration} from '../app/app.js'
import {loadLocalesConfig} from '../../utilities/extensions/locales-configuration.js'
import {Result} from '@shopify/cli-kit/node/result'
import {capitalize} from '@shopify/cli-kit/common/string'
Expand Down Expand Up @@ -62,6 +62,11 @@ export interface ExtensionSpecification<TConfiguration extends BaseConfigType =
buildValidation?: (extension: ExtensionInstance<TConfiguration>) => Promise<void>
hasExtensionPointTarget?(config: TConfiguration, target: string): boolean
appModuleFeatures: (config?: TConfiguration) => ExtensionFeature[]
getDevSessionActionUpdateMessage?: (
config: TConfiguration,
appConfig: CurrentAppConfiguration,
storeFqdn: string,
) => Promise<string>

/**
* If required, convert configuration from the format used in the local filesystem to that expected by the platform.
Expand Down Expand Up @@ -167,6 +172,7 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
reverseTransform: spec.transformRemoteToLocal,
experience: spec.experience ?? 'extension',
uidStrategy: spec.uidStrategy ?? (spec.experience === 'configuration' ? 'single' : 'uuid'),
getDevSessionActionUpdateMessage: spec.getDevSessionActionUpdateMessage,
}
const merged = {...defaults, ...spec}

Expand Down Expand Up @@ -212,6 +218,11 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
appModuleFeatures?: (config?: TConfiguration) => ExtensionFeature[]
transformConfig?: TransformationConfig | CustomTransformationConfig
uidStrategy?: UidStrategy
getDevSessionActionUpdateMessage?: (
config: TConfiguration,
appConfig: CurrentAppConfiguration,
storeFqdn: string,
) => Promise<string>
}): ExtensionSpecification<TConfiguration> {
const appModuleFeatures = spec.appModuleFeatures ?? (() => [])
return createExtensionSpecification({
Expand All @@ -224,6 +235,7 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
transformRemoteToLocal: resolveReverseAppConfigTransform(spec.schema, spec.transformConfig),
experience: 'configuration',
uidStrategy: spec.uidStrategy ?? 'single',
getDevSessionActionUpdateMessage: spec.getDevSessionActionUpdateMessage,
})
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {buildAppURLForWeb} from '../../../utilities/app/app-url.js'
import {validateUrl} from '../../app/validation/common.js'
import {TransformationConfig, createConfigExtensionSpecification} from '../specification.js'
import {outputContent, outputToken} from '@shopify/cli-kit/node/output'
import {normalizeDelimitedString} from '@shopify/cli-kit/common/string'
import {zod} from '@shopify/cli-kit/node/schema'

Expand Down Expand Up @@ -45,6 +47,10 @@ const appAccessSpec = createConfigExtensionSpecification({
identifier: AppAccessSpecIdentifier,
schema: AppAccessSchema,
transformConfig: AppAccessTransformConfig,
getDevSessionActionUpdateMessage: async (_, appConfig, storeFqdn) => {
const scopesURL = await buildAppURLForWeb(storeFqdn, appConfig.client_id)
return outputContent`Scopes updated. ${outputToken.link('Open app to accept scopes.', scopesURL)}`.value
},
})

export default appAccessSpec
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {AppLinkedInterface} from '../../../models/app/app.js'
import {AppEventWatcher} from '../app-events/app-event-watcher.js'
import {buildAppURLForWeb} from '../../../utilities/app/app-url.js'
import {
testAppAccessConfigExtension,
testAppLinked,
testDeveloperPlatformClient,
testUIExtension,
Expand Down Expand Up @@ -165,7 +166,8 @@ describe('pushUpdatesForDevSession', () => {
test('handles scope changes and displays action required message', async () => {
// Given
vi.mocked(buildAppURLForWeb).mockResolvedValue('https://test.myshopify.com/admin/apps/test')
const event = {extensionEvents: [{type: 'updated', extension: {handle: 'app-access'}}], app}
const appAccess = await testAppAccessConfigExtension()
const event = {extensionEvents: [{type: 'updated', extension: appAccess}], app}
const contextSpy = vi.spyOn(outputContext, 'useConcurrentOutputContext')

// When
Expand Down
34 changes: 26 additions & 8 deletions packages/app/src/cli/services/dev/processes/dev-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {DeveloperPlatformClient} from '../../../utilities/developer-platform-cli
import {AppLinkedInterface} from '../../../models/app/app.js'
import {getExtensionUploadURL} from '../../deploy/upload.js'
import {AppEvent, AppEventWatcher} from '../app-events/app-event-watcher.js'
import {buildAppURLForWeb} from '../../../utilities/app/app-url.js'
import {readFileSync, writeFile} from '@shopify/cli-kit/node/fs'
import {dirname, joinPath} from '@shopify/cli-kit/node/path'
import {AbortSignal} from '@shopify/cli-kit/node/abort'
Expand All @@ -16,6 +15,7 @@ import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components'
import {JsonMapType} from '@shopify/cli-kit/node/toml'
import {AbortError} from '@shopify/cli-kit/node/error'
import {isUnitTest} from '@shopify/cli-kit/node/context/local'
import {getArrayRejectingUndefined} from '@shopify/cli-kit/common/array'
import {Writable} from 'stream'

interface DevSessionOptions {
Expand Down Expand Up @@ -141,13 +141,7 @@ async function handleDevSessionResult(
) {
if (result.status === 'updated') {
await printSuccess(`✅ Updated`, processOptions.stdout)
const scopeChanges = event?.extensionEvents.find((eve) => eve.extension.handle === 'app-access')
if (scopeChanges) {
await printWarning(`🔄 Action required`, processOptions.stdout)
const scopesURL = await buildAppURLForWeb(processOptions.storeFqdn, processOptions.apiKey)
const message = outputContent`└ Scopes updated. ${outputToken.link('Open app to accept scopes.', scopesURL)}`
await printWarning(message.value, processOptions.stdout)
}
await printActionRequiredMessages(processOptions, event)
} else if (result.status === 'created') {
isDevSessionReady = true
await printSuccess(`✅ Ready, watching for changes in your app `, processOptions.stdout)
Expand All @@ -162,6 +156,30 @@ async function handleDevSessionResult(
if (!isDevSessionReady && !isUnitTest()) throw new AbortError('Failed to create dev session')
}

/**
* Some extensions may require the user to take some action after an update in the dev session.
* This function will print those action messages to the terminal.
*/
async function printActionRequiredMessages(processOptions: DevSessionProcessOptions, event?: AppEvent) {
if (!event) return
const extensionEvents = event.extensionEvents ?? []
const warningMessages = getArrayRejectingUndefined(
await Promise.all(
extensionEvents.map((eve) =>
eve.extension.getDevSessionActionUpdateMessage(event.app.configuration, processOptions.storeFqdn),
),
),
)

if (warningMessages.length) {
await printWarning(`🔄 Action required`, processOptions.stdout)
// eslint-disable-next-line @typescript-eslint/no-misused-promises
warningMessages.forEach(async (message) => {
await printWarning(outputContent`└ ${message}`.value, processOptions.stdout)
})
}
}

/**
* Bundle all extensions and upload them to the developer platform
* Generate a new manifest in the bundle folder, zip it and upload it to GCS.
Expand Down

0 comments on commit 8a78cb3

Please sign in to comment.