Skip to content

Commit

Permalink
Merge pull request #4874 from Shopify/clean-up-app-management-client-…
Browse files Browse the repository at this point in the history
…module-interpretation-quality-concerns

Clean up app management client module interpretation quality concerns
  • Loading branch information
amcaplan authored Nov 24, 2024
2 parents e56fad4 + ca47e7b commit 734c259
Show file tree
Hide file tree
Showing 11 changed files with 97 additions and 113 deletions.
34 changes: 18 additions & 16 deletions packages/app/src/cli/models/app/app.test-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@ import {
getAppVersionedSchema,
} from './app.js'
import {ExtensionTemplate} from './template.js'
import {Organization, OrganizationStore, MinimalAppIdentifiers, OrganizationApp} from '../organization.js'
import {
Organization,
OrganizationStore,
MinimalAppIdentifiers,
OrganizationApp,
MinimalOrganizationApp,
} from '../organization.js'
import {RemoteSpecification} from '../../api/graphql/extension_specifications.js'
import {ExtensionInstance} from '../extensions/extension-instance.js'
import {loadLocalExtensionsSpecifications} from '../extensions/load-specifications.js'
Expand All @@ -21,8 +27,9 @@ import {PartnersSession} from '../../services/context/partner-account-info.js'
import {WebhooksConfig} from '../extensions/specifications/types/app_config_webhook.js'
import {PaymentsAppExtensionConfigType} from '../extensions/specifications/payments_app_extension.js'
import {
ActiveAppVersion,
AppVersion,
AppVersionIdentifiers,
AppVersionWithContext,
AssetUrlSchema,
CreateAppOptions,
DeveloperPlatformClient,
Expand All @@ -41,7 +48,6 @@ import {SendSampleWebhookSchema, SendSampleWebhookVariables} from '../../service
import {PublicApiVersionsSchema} from '../../services/webhook/request-api-versions.js'
import {WebhookTopicsSchema, WebhookTopicsVariables} from '../../services/webhook/request-topics.js'
import {AppReleaseSchema} from '../../api/graphql/app_release.js'
import {AppVersionByTagSchema, AppVersionByTagVariables} from '../../api/graphql/app_version_by_tag.js'
import {AppVersionsDiffSchema, AppVersionsDiffVariables} from '../../api/graphql/app_versions_diff.js'
import {
MigrateFlowExtensionSchema,
Expand Down Expand Up @@ -1118,21 +1124,17 @@ const emptyAppVersions = {
},
}

const emptyActiveAppVersion: ActiveAppVersion = {
const emptyActiveAppVersion: AppVersion = {
appModuleVersions: [],
}

const appVersionByTagResponse: AppVersionByTagSchema = {
app: {
appVersion: {
id: 1,
uuid: 'uuid',
versionTag: 'version-tag',
location: 'location',
message: 'MESSAGE',
appModuleVersions: [],
},
},
const appVersionByTagResponse: AppVersionWithContext = {
id: 1,
uuid: 'uuid',
versionTag: 'version-tag',
location: 'location',
message: 'MESSAGE',
appModuleVersions: [],
}

const appVersionsDiffResponse: AppVersionsDiffSchema = {
Expand Down Expand Up @@ -1307,7 +1309,7 @@ export function testDeveloperPlatformClient(stubs: Partial<DeveloperPlatformClie
appExtensionRegistrations: (_app: MinimalAppIdentifiers) => Promise.resolve(emptyAppExtensionRegistrations),
appVersions: (_app: MinimalAppIdentifiers) => Promise.resolve(emptyAppVersions),
activeAppVersion: (_app: MinimalAppIdentifiers) => Promise.resolve(emptyActiveAppVersion),
appVersionByTag: (_input: AppVersionByTagVariables) => Promise.resolve(appVersionByTagResponse),
appVersionByTag: (_app: MinimalOrganizationApp, _tag: string) => Promise.resolve(appVersionByTagResponse),
appVersionsDiff: (_input: AppVersionsDiffVariables) => Promise.resolve(appVersionsDiffResponse),
createExtension: (_input: ExtensionCreateVariables) => Promise.resolve(extensionCreateResponse),
updateExtension: (_input: ExtensionUpdateDraftMutationVariables) => Promise.resolve(extensionUpdateResponse),
Expand Down
9 changes: 2 additions & 7 deletions packages/app/src/cli/services/app/select-app.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import {MinimalOrganizationApp} from '../../models/organization.js'
import {
Flag,
AppModuleVersion,
DeveloperPlatformClient,
ActiveAppVersion,
} from '../../utilities/developer-platform-client.js'
import {Flag, AppModuleVersion, DeveloperPlatformClient, AppVersion} from '../../utilities/developer-platform-client.js'
import {ExtensionSpecification} from '../../models/extensions/specification.js'
import {AppConfigurationUsedByCli} from '../../models/extensions/specifications/types/app_config.js'
import {deepMergeObjects} from '@shopify/cli-kit/common/object'
Expand All @@ -30,7 +25,7 @@ export async function fetchAppRemoteConfiguration(
developerPlatformClient: DeveloperPlatformClient,
specifications: ExtensionSpecification[],
flags: Flag[],
activeAppVersion?: ActiveAppVersion,
activeAppVersion?: AppVersion,
) {
const appVersion = activeAppVersion || (await developerPlatformClient.activeAppVersion(remoteApp))
const appModuleVersionsConfig =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {OrganizationApp, MinimalAppIdentifiers} from '../../models/organization.
import {ExtensionInstance} from '../../models/extensions/extension-instance.js'
import {AppVersionsDiffExtensionSchema} from '../../api/graphql/app_versions_diff.js'
import {versionDiffByVersion} from '../release/version-diff.js'
import {ActiveAppVersion, AppModuleVersion, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {AppVersion, AppModuleVersion, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {loadLocalExtensionsSpecifications} from '../../models/extensions/load-specifications.js'
import {describe, vi, test, beforeAll, expect} from 'vitest'
import {setPathValue} from '@shopify/cli-kit/common/object'
Expand Down Expand Up @@ -281,7 +281,7 @@ const options = async (params: {
remoteApp?: OrganizationApp
release?: boolean
developerPlatformClient?: DeveloperPlatformClient
activeAppVersion?: ActiveAppVersion
activeAppVersion?: AppVersion
}) => {
return {
app: await LOCAL_APP(params.uiExtensions),
Expand Down
12 changes: 6 additions & 6 deletions packages/app/src/cli/services/context/breakdown-extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
fetchAppRemoteConfiguration,
remoteAppConfigurationExtensionContent,
} from '../app/select-app.js'
import {ActiveAppVersion, AppModuleVersion, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {AppVersion, AppModuleVersion, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {
AllAppExtensionRegistrationsQuerySchema,
RemoteExtensionRegistrations,
Expand Down Expand Up @@ -136,7 +136,7 @@ export async function configExtensionsIdentifiersBreakdown({
localApp: AppInterface
versionAppModules?: AppModuleVersion[]
release?: boolean
activeAppVersion?: ActiveAppVersion
activeAppVersion?: AppVersion
}) {
if (localApp.allExtensions.filter((extension) => extension.isAppConfigExtension).length === 0) return
if (!release) return loadLocalConfigExtensionIdentifiersBreakdown(localApp)
Expand Down Expand Up @@ -170,7 +170,7 @@ async function resolveRemoteConfigExtensionIdentifiersBreakdown(
remoteApp: MinimalOrganizationApp,
app: AppInterface,
versionAppModules?: AppModuleVersion[],
activeAppVersion?: ActiveAppVersion,
activeAppVersion?: AppVersion,
) {
const remoteConfig: Partial<AppConfigurationUsedByCli> =
(await fetchAppRemoteConfiguration(
Expand Down Expand Up @@ -337,7 +337,7 @@ async function resolveRemoteExtensionIdentifiersBreakdown(
toCreate: LocalSource[],
dashboardOnly: RemoteSource[],
specs: ExtensionSpecification[],
activeAppVersion?: ActiveAppVersion,
activeAppVersion?: AppVersion,
): Promise<ExtensionIdentifiersBreakdown | undefined> {
const version = activeAppVersion || (await developerPlatformClient.activeAppVersion(remoteApp))
if (!version) return
Expand All @@ -359,7 +359,7 @@ async function resolveRemoteExtensionIdentifiersBreakdown(
}

function loadExtensionsIdentifiersBreakdown(
activeAppVersion: ActiveAppVersion,
activeAppVersion: AppVersion,
localRegistration: IdentifiersExtensions,
toCreate: LocalSource[],
specs: ExtensionSpecification[],
Expand Down Expand Up @@ -392,7 +392,7 @@ function loadExtensionsIdentifiersBreakdown(
}
}

function loadDashboardIdentifiersBreakdown(currentRegistrations: RemoteSource[], activeAppVersion: ActiveAppVersion) {
function loadDashboardIdentifiersBreakdown(currentRegistrations: RemoteSource[], activeAppVersion: AppVersion) {
const currentVersions =
activeAppVersion?.appModuleVersions.filter(
(module) => module.specification!.options.managementExperience === 'dashboard',
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/context/identifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {AppInterface} from '../../models/app/app.js'
import {Identifiers} from '../../models/app/identifiers.js'
import {MinimalOrganizationApp} from '../../models/organization.js'
import {deployOrReleaseConfirmationPrompt} from '../../prompts/deploy-release.js'
import {ActiveAppVersion, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {AppVersion, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {AbortSilentError} from '@shopify/cli-kit/node/error'

export type PartnersAppForIdentifierMatching = MinimalOrganizationApp
Expand All @@ -19,7 +19,7 @@ export interface EnsureDeploymentIdsPresenceOptions {
release: boolean
remoteApp: PartnersAppForIdentifierMatching
includeDraftExtensions?: boolean
activeAppVersion?: ActiveAppVersion
activeAppVersion?: AppVersion
}

export interface RemoteSource {
Expand Down
1 change: 0 additions & 1 deletion packages/app/src/cli/services/release.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export async function release(options: ReleaseOptions) {
remoteApp,
versionAppModules: versionDetails.appModuleVersions.map((appModuleVersion) => ({
...appModuleVersion,
...(appModuleVersion.config ? {config: JSON.parse(appModuleVersion.config)} : {}),
})),
release: true,
})
Expand Down
22 changes: 9 additions & 13 deletions packages/app/src/cli/services/release/version-diff.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {versionDiffByVersion} from './version-diff.js'
import {testDeveloperPlatformClient, testOrganizationApp} from '../../models/app/app.test-data.js'
import {AppVersionByTagSchema} from '../../api/graphql/app_version_by_tag.js'
import {AppVersionWithContext} from '../../utilities/developer-platform-client.js'
import {AppVersionsDiffSchema} from '../../api/graphql/app_versions_diff.js'
import {describe, expect, test} from 'vitest'
import {AbortSilentError} from '@shopify/cli-kit/node/error'
Expand Down Expand Up @@ -34,17 +34,13 @@ describe('versionDiffByVersion', () => {

test('returns versionDiff and versionDetails when the version is found', async () => {
// Given
const versionDetails: AppVersionByTagSchema = {
app: {
appVersion: {
id: 1,
uuid: 'uuid',
versionTag: 'versionTag',
location: 'location',
message: 'message',
appModuleVersions: [],
},
},
const versionDetails: AppVersionWithContext = {
id: 1,
uuid: 'uuid',
versionTag: 'versionTag',
location: 'location',
message: 'message',
appModuleVersions: [],
}
const versionsDiff: AppVersionsDiffSchema = {
app: {
Expand Down Expand Up @@ -101,6 +97,6 @@ describe('versionDiffByVersion', () => {
const result = await versionDiffByVersion(testOrganizationApp(), 'version', developerPlatformClient)

// Then
expect(result).toEqual({versionsDiff: versionsDiff.app.versionsDiff, versionDetails: versionDetails.app.appVersion})
expect(result).toEqual({versionsDiff: versionsDiff.app.versionsDiff, versionDetails})
})
})
9 changes: 3 additions & 6 deletions packages/app/src/cli/services/release/version-diff.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {AppVersionsDiffSchema} from '../../api/graphql/app_versions_diff.js'
import {AppVersionByTagSchema} from '../../api/graphql/app_version_by_tag.js'
import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {AppVersionWithContext, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {MinimalOrganizationApp} from '../../models/organization.js'
import {renderError} from '@shopify/cli-kit/node/ui'
import {AbortSilentError} from '@shopify/cli-kit/node/error'
Expand All @@ -11,7 +10,7 @@ export async function versionDiffByVersion(
developerPlatformClient: DeveloperPlatformClient,
): Promise<{
versionsDiff: AppVersionsDiffSchema['app']['versionsDiff']
versionDetails: AppVersionByTagSchema['app']['appVersion']
versionDetails: AppVersionWithContext
}> {
const versionDetails = await versionDetailsByTag(app, versionTag, developerPlatformClient)
const {
Expand All @@ -30,9 +29,7 @@ async function versionDetailsByTag(
developerPlatformClient: DeveloperPlatformClient,
) {
try {
const {
app: {appVersion},
}: AppVersionByTagSchema = await developerPlatformClient.appVersionByTag(app, versionTag)
const appVersion = await developerPlatformClient.appVersionByTag(app, versionTag)
return appVersion
} catch (err) {
renderError({
Expand Down
18 changes: 12 additions & 6 deletions packages/app/src/cli/utilities/developer-platform-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
} from '../api/graphql/development_preview.js'
import {FindAppPreviewModeSchema, FindAppPreviewModeVariables} from '../api/graphql/find_app_preview_mode.js'
import {AppReleaseSchema} from '../api/graphql/app_release.js'
import {AppVersionByTagSchema} from '../api/graphql/app_version_by_tag.js'
import {AppVersionsDiffSchema} from '../api/graphql/app_versions_diff.js'
import {SendSampleWebhookSchema, SendSampleWebhookVariables} from '../services/webhook/request-sample.js'
import {PublicApiVersionsSchema} from '../services/webhook/request-api-versions.js'
Expand Down Expand Up @@ -154,17 +153,24 @@ interface AppModuleVersionSpecification {
export interface AppModuleVersion {
registrationId: string
registrationUuid?: string
registrationUid?: string
registrationTitle: string
config?: object
type: string
specification?: AppModuleVersionSpecification
}

export interface ActiveAppVersion {
export interface AppVersion {
appModuleVersions: AppModuleVersion[]
}

export type AppVersionWithContext = AppVersion & {
id: number
uuid: string
versionTag?: string | null
location: string
message: string
}

export type AppDeployOptions = AppDeployVariables & {
appId: string
organizationId: string
Expand Down Expand Up @@ -219,11 +225,11 @@ export interface DeveloperPlatformClient {
storeByDomain: (orgId: string, shopDomain: string) => Promise<FindStoreByDomainSchema>
appExtensionRegistrations: (
app: MinimalAppIdentifiers,
activeAppVersion?: ActiveAppVersion,
activeAppVersion?: AppVersion,
) => Promise<AllAppExtensionRegistrationsQuerySchema>
appVersions: (app: OrganizationApp) => Promise<AppVersionsQuerySchema>
activeAppVersion: (app: MinimalAppIdentifiers) => Promise<ActiveAppVersion | undefined>
appVersionByTag: (app: MinimalOrganizationApp, tag: string) => Promise<AppVersionByTagSchema>
activeAppVersion: (app: MinimalAppIdentifiers) => Promise<AppVersion | undefined>
appVersionByTag: (app: MinimalOrganizationApp, tag: string) => Promise<AppVersionWithContext>
appVersionsDiff: (app: MinimalOrganizationApp, version: AppVersionIdentifiers) => Promise<AppVersionsDiffSchema>
generateSignedUploadUrl: (app: MinimalAppIdentifiers) => Promise<AssetUrlSchema>
createExtension: (input: ExtensionCreateVariables) => Promise<ExtensionCreateSchema>
Expand Down
Loading

0 comments on commit 734c259

Please sign in to comment.