From ba40cca6bb8ce79774602d6cc263fa096765dc0b Mon Sep 17 00:00:00 2001 From: Gordon Hirsch Date: Wed, 30 Oct 2024 17:16:38 -0400 Subject: [PATCH 1/4] get module uid from user_identifier field --- .../graphql/app-management/generated/active-app-release.ts | 4 ++++ .../api/graphql/app-management/generated/app-version-by-id.ts | 2 ++ .../app/src/cli/api/graphql/app-management/generated/apps.ts | 2 ++ .../graphql/app-management/generated/create-app-version.ts | 2 ++ .../graphql/app-management/queries/active-app-release.graphql | 1 + .../developer-platform-client/app-management-client.ts | 4 ++-- 6 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/app/src/cli/api/graphql/app-management/generated/active-app-release.ts b/packages/app/src/cli/api/graphql/app-management/generated/active-app-release.ts index 4dfdaafd176..3f6ed301e94 100644 --- a/packages/app/src/cli/api/graphql/app-management/generated/active-app-release.ts +++ b/packages/app/src/cli/api/graphql/app-management/generated/active-app-release.ts @@ -19,6 +19,7 @@ export type ActiveAppReleaseQuery = { name: string appModules: { uuid: string + userIdentifier: string handle: string config: JsonMapType specification: {identifier: string; externalIdentifier: string; name: string} @@ -30,6 +31,7 @@ export type ActiveAppReleaseQuery = { export type ReleasedAppModuleFragment = { uuid: string + userIdentifier: string handle: string config: JsonMapType specification: {identifier: string; externalIdentifier: string; name: string} @@ -46,6 +48,7 @@ export const ReleasedAppModuleFragmentDoc = { kind: 'SelectionSet', selections: [ {kind: 'Field', name: {kind: 'Name', value: 'uuid'}}, + {kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}}, {kind: 'Field', name: {kind: 'Name', value: 'handle'}}, {kind: 'Field', name: {kind: 'Name', value: 'config'}}, { @@ -176,6 +179,7 @@ export const ActiveAppRelease = { kind: 'SelectionSet', selections: [ {kind: 'Field', name: {kind: 'Name', value: 'uuid'}}, + {kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}}, {kind: 'Field', name: {kind: 'Name', value: 'handle'}}, {kind: 'Field', name: {kind: 'Name', value: 'config'}}, { diff --git a/packages/app/src/cli/api/graphql/app-management/generated/app-version-by-id.ts b/packages/app/src/cli/api/graphql/app-management/generated/app-version-by-id.ts index e42d417dd42..e2c2fd6978c 100644 --- a/packages/app/src/cli/api/graphql/app-management/generated/app-version-by-id.ts +++ b/packages/app/src/cli/api/graphql/app-management/generated/app-version-by-id.ts @@ -14,6 +14,7 @@ export type AppVersionByIdQuery = { metadata: {versionTag?: string | null} appModules: { uuid: string + userIdentifier: string handle: string config: JsonMapType specification: {identifier: string; externalIdentifier: string; name: string} @@ -89,6 +90,7 @@ export const AppVersionById = { kind: 'SelectionSet', selections: [ {kind: 'Field', name: {kind: 'Name', value: 'uuid'}}, + {kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}}, {kind: 'Field', name: {kind: 'Name', value: 'handle'}}, {kind: 'Field', name: {kind: 'Name', value: 'config'}}, { diff --git a/packages/app/src/cli/api/graphql/app-management/generated/apps.ts b/packages/app/src/cli/api/graphql/app-management/generated/apps.ts index c80651dda1f..ee8ec25c3bf 100644 --- a/packages/app/src/cli/api/graphql/app-management/generated/apps.ts +++ b/packages/app/src/cli/api/graphql/app-management/generated/apps.ts @@ -20,6 +20,7 @@ export type ListAppsQuery = { name: string appModules: { uuid: string + userIdentifier: string handle: string config: JsonMapType specification: {identifier: string; externalIdentifier: string; name: string} @@ -147,6 +148,7 @@ export const ListApps = { kind: 'SelectionSet', selections: [ {kind: 'Field', name: {kind: 'Name', value: 'uuid'}}, + {kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}}, {kind: 'Field', name: {kind: 'Name', value: 'handle'}}, {kind: 'Field', name: {kind: 'Name', value: 'config'}}, { diff --git a/packages/app/src/cli/api/graphql/app-management/generated/create-app-version.ts b/packages/app/src/cli/api/graphql/app-management/generated/create-app-version.ts index 8e6fa636d93..e1c061b2ba4 100644 --- a/packages/app/src/cli/api/graphql/app-management/generated/create-app-version.ts +++ b/packages/app/src/cli/api/graphql/app-management/generated/create-app-version.ts @@ -17,6 +17,7 @@ export type CreateAppVersionMutation = { id: string appModules: { uuid: string + userIdentifier: string handle: string config: JsonMapType specification: {identifier: string; externalIdentifier: string; name: string} @@ -157,6 +158,7 @@ export const CreateAppVersion = { kind: 'SelectionSet', selections: [ {kind: 'Field', name: {kind: 'Name', value: 'uuid'}}, + {kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}}, {kind: 'Field', name: {kind: 'Name', value: 'handle'}}, {kind: 'Field', name: {kind: 'Name', value: 'config'}}, { diff --git a/packages/app/src/cli/api/graphql/app-management/queries/active-app-release.graphql b/packages/app/src/cli/api/graphql/app-management/queries/active-app-release.graphql index 008511125c8..3408adc4edc 100644 --- a/packages/app/src/cli/api/graphql/app-management/queries/active-app-release.graphql +++ b/packages/app/src/cli/api/graphql/app-management/queries/active-app-release.graphql @@ -23,6 +23,7 @@ query activeAppRelease($appId: ID!) { fragment ReleasedAppModule on AppModule { uuid + userIdentifier handle config specification { diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts index 0882f12c247..b87463fe59b 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts @@ -946,8 +946,8 @@ function mapBusinessPlatformStoresToOrganizationStores(storesArray: ShopNode[]): function appModuleVersion(mod: ReleasedAppModuleFragment): Required { return { - registrationId: mod.uuid, - registrationUuid: mod.uuid, + registrationId: mod.userIdentifier, + registrationUuid: mod.userIdentifier, registrationTitle: mod.handle, type: mod.specification.externalIdentifier, config: mod.config, From 9fe739b81229bd6b5b7564642a1d6dd295e26e1f Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Fri, 8 Nov 2024 13:08:56 +0100 Subject: [PATCH 2/4] Fix type-check --- .../developer-platform-client/app-management-client.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts index eef4777b28d..db22e2eb65e 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts @@ -47,6 +47,7 @@ const templateDisallowedByBetaFlag: GatedExtensionTemplate = { function moduleFromExtension(extension: ExtensionInstance) { return { uuid: extension.uid, + userIdentifier: extension.uid, handle: extension.handle, config: extension.configuration, specification: { From becf4a2491de9355b75c264bea575f511c722de5 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Sun, 17 Nov 2024 22:18:50 +0200 Subject: [PATCH 3/4] Use userIdentifier to diff modules --- .../app-management-client.test.ts | 31 +++++++++++++++++++ .../app-management-client.ts | 12 +++---- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts index db22e2eb65e..c49525391d7 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts @@ -16,6 +16,7 @@ import {fetch} from '@shopify/cli-kit/node/http' import {businessPlatformOrganizationsRequest} from '@shopify/cli-kit/node/api/business-platform' import {appManagementRequestDoc} from '@shopify/cli-kit/node/api/app-management' import {BugError} from '@shopify/cli-kit/node/error' +import {randomUUID} from '@shopify/cli-kit/node/crypto' vi.mock('@shopify/cli-kit/node/http') vi.mock('@shopify/cli-kit/node/api/business-platform') @@ -77,6 +78,36 @@ describe('diffAppModules', () => { expect(removed).toEqual([moduleA]) expect(updated).toEqual([moduleB]) }) + + // This test considers the case where there are local and remote modules, which may have slightly different properties + test('extracts the added, removed and updated modules before deployment', async () => { + // Given + const [remoteModuleA, remoteModuleB] = [moduleFromExtension(extensionA), moduleFromExtension(extensionB)] + // Under some circumstances, local UUID may differ from remote. + // So we are testing that diffing happens based on the shared userIdentifier + // property, not the UUID. + const localModuleB = { + ...remoteModuleB, + uuid: randomUUID(), + } + const localModuleC = { + ...moduleFromExtension(extensionC), + uuid: randomUUID(), + } + + const before = [remoteModuleA, remoteModuleB] + const after = [localModuleB, localModuleC] + + // When + const {added, removed, updated} = diffAppModules({currentModules: before, selectedVersionModules: after}) + + // Then + expect(added).toEqual([localModuleC]) + expect(removed).toEqual([remoteModuleA]) + // Updated returns the remote module, not the local one. This shouldn't matter + // as the module identifiers are the same. + expect(updated).toEqual([remoteModuleB]) + }) }) describe('templateSpecifications', () => { diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts index b87463fe59b..ab875512679 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts @@ -902,12 +902,12 @@ interface DiffAppModulesOutput { } export function diffAppModules({currentModules, selectedVersionModules}: DiffAppModulesInput): DiffAppModulesOutput { - const currentModuleUids = currentModules.map((mod) => mod.uuid) - const selectedVersionModuleUids = selectedVersionModules.map((mod) => mod.uuid) - const removed = currentModules.filter((mod) => !selectedVersionModuleUids.includes(mod.uuid)) - const added = selectedVersionModules.filter((mod) => !currentModuleUids.includes(mod.uuid)) - const addedUids = added.map((mod) => mod.uuid) - const updated = selectedVersionModules.filter((mod) => !addedUids.includes(mod.uuid)) + const currentModuleUids = currentModules.map((mod) => mod.userIdentifier) + const selectedVersionModuleUids = selectedVersionModules.map((mod) => mod.userIdentifier) + const added = selectedVersionModules.filter((mod) => !currentModuleUids.includes(mod.userIdentifier)) + const removed = currentModules.filter((mod) => !selectedVersionModuleUids.includes(mod.userIdentifier)) + const removedUids = removed.map((mod) => mod.userIdentifier) + const updated = currentModules.filter((mod) => !removedUids.includes(mod.userIdentifier)) return {added, removed, updated} } From e510e9027407270931d1049d1cd0d9fa21453cfa Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Sun, 24 Nov 2024 18:18:03 +0200 Subject: [PATCH 4/4] Compensate for uid being removed --- .../cli/services/context/id-matching.test.ts | 21 +++++++------------ .../src/cli/services/context/id-matching.ts | 15 +++++++------ .../context/identifiers-extensions.ts | 1 - .../src/cli/services/context/identifiers.ts | 1 - 4 files changed, 14 insertions(+), 24 deletions(-) diff --git a/packages/app/src/cli/services/context/id-matching.test.ts b/packages/app/src/cli/services/context/id-matching.test.ts index edc80f251d9..dc5217bb1bd 100644 --- a/packages/app/src/cli/services/context/id-matching.test.ts +++ b/packages/app/src/cli/services/context/id-matching.test.ts @@ -10,7 +10,6 @@ vi.mock('../dev/create-extension') const REGISTRATION_A: RemoteSource = { uuid: 'UUID_A', - uid: 'UID_A', id: 'A', title: 'EXTENSION_A', type: 'CHECKOUT_POST_PURCHASE', @@ -18,7 +17,6 @@ const REGISTRATION_A: RemoteSource = { const REGISTRATION_A_2: RemoteSource = { uuid: 'UUID_A_2', - uid: 'UID_A_2', id: 'A_2', title: 'EXTENSION_A_2', type: 'CHECKOUT_POST_PURCHASE', @@ -26,7 +24,6 @@ const REGISTRATION_A_2: RemoteSource = { const REGISTRATION_A_3: RemoteSource = { uuid: 'UUID_A_3', - uid: 'UID_A_3', id: 'A_3', title: 'EXTENSION_A_3', type: 'CHECKOUT_POST_PURCHASE', @@ -41,7 +38,6 @@ const REGISTRATION_A_4: RemoteSource = { const REGISTRATION_B: RemoteSource = { uuid: 'UUID_B', - uid: 'UID_B', id: 'B', title: 'EXTENSION_B', type: 'SUBSCRIPTION_MANAGEMENT', @@ -49,7 +45,6 @@ const REGISTRATION_B: RemoteSource = { const REGISTRATION_C: RemoteSource = { uuid: 'UUID_C', - uid: 'UID_C', id: 'C', title: 'EXTENSION_C', type: 'THEME_APP_EXTENSION', @@ -57,7 +52,6 @@ const REGISTRATION_C: RemoteSource = { const REGISTRATION_D: RemoteSource = { uuid: 'UUID_D', - uid: 'UID_D', id: 'D', title: 'EXTENSION_D', type: 'WEB_PIXEL_EXTENSION', @@ -65,7 +59,6 @@ const REGISTRATION_D: RemoteSource = { const REGISTRATION_FUNCTION_A: RemoteSource = { uuid: 'FUNCTION_UUID_A', - uid: 'FUNCTION_UID_A', id: 'FUNCTION_A', title: 'FUNCTION A', type: 'FUNCTION', @@ -107,7 +100,7 @@ beforeAll(async () => { }, entrySourceFilePath: '', devUUID: 'devUUID', - uid: 'UID_A', + uid: 'UUID_A', }) EXTENSION_A_2 = await testUIExtension({ @@ -131,7 +124,7 @@ beforeAll(async () => { }, entrySourceFilePath: '', devUUID: 'devUUID', - uid: 'UID_A_2', + uid: 'UIUD_A_2', }) EXTENSION_B = await testUIExtension({ @@ -155,7 +148,7 @@ beforeAll(async () => { }, entrySourceFilePath: '', devUUID: 'devUUID', - uid: 'UID_B', + uid: 'UUID_B', }) EXTENSION_B_2 = await testUIExtension({ @@ -179,7 +172,7 @@ beforeAll(async () => { }, entrySourceFilePath: '', devUUID: 'devUUID', - uid: 'UID_B_2', + uid: 'UUID_B_2', }) EXTENSION_C = await testUIExtension({ @@ -203,7 +196,7 @@ beforeAll(async () => { }, entrySourceFilePath: '', devUUID: 'devUUID', - uid: 'UID_C', + uid: 'UUID_C', }) EXTENSION_D = await testUIExtension({ @@ -228,7 +221,7 @@ beforeAll(async () => { outputPath: '', entrySourceFilePath: '', devUUID: 'devUUID', - uid: 'UID_D', + uid: 'UUID_D', }) FUNCTION_A = await testFunctionExtension({ @@ -794,7 +787,7 @@ describe('automaticMatchmaking: with Atomic Deployments enabled', () => { // Then const expected = { - identifiers: {'extension-a': 'UID_A'}, + identifiers: {'extension-a': 'UUID_A'}, toConfirm: [], toCreate: [EXTENSION_A_2], toManualMatch: {local: [], remote: []}, diff --git a/packages/app/src/cli/services/context/id-matching.ts b/packages/app/src/cli/services/context/id-matching.ts index 4baf7c76178..1827c78b5ec 100644 --- a/packages/app/src/cli/services/context/id-matching.ts +++ b/packages/app/src/cli/services/context/id-matching.ts @@ -67,7 +67,7 @@ function matchByNameAndType( /** * Automatically match local and remote sources if they have the same UID */ -function matchByUID( +function matchByUUID( local: LocalSource[], remote: RemoteSource[], ): { @@ -79,9 +79,9 @@ function matchByUID( const matched: IdentifiersExtensions = {} local.forEach((localSource) => { - const possibleMatch = remote.find((remoteSource) => remoteSource.uid === localSource.uid) + const possibleMatch = remote.find((remoteSource) => remoteSource.uuid === localSource.uid) // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - if (possibleMatch) matched[localSource.localIdentifier] = possibleMatch.uid! + if (possibleMatch) matched[localSource.localIdentifier] = possibleMatch.uuid! }) const toCreate = local.filter((elem) => !matched[elem.localIdentifier]) @@ -207,15 +207,14 @@ export async function automaticMatchmaking( identifiers: IdentifiersExtensions, developerPlatformClient: DeveloperPlatformClient, ): Promise { - const useUidMatching = developerPlatformClient.supportsAtomicDeployments + const useUuidMatching = developerPlatformClient.supportsAtomicDeployments const ids = getExtensionIds(localSources, identifiers) const localIds = Object.values(ids) const existsRemotely = (local: LocalSource) => remoteSources.some((remote) => { if (remote.type !== developerPlatformClient.toExtensionGraphQLType(local.graphQLType)) return false - const remoteIdField = useUidMatching ? 'uid' : 'uuid' - return ids[local.localIdentifier] === remote[remoteIdField] + return ids[local.localIdentifier] === remote.uuid }) const {migrated: migratedFunctions, pending: pendingAfterMigratingFunctions} = migrateLegacyFunctions( @@ -225,8 +224,8 @@ export async function automaticMatchmaking( ) const {local, remote} = pendingAfterMigratingFunctions - const {matched, toCreate, toConfirm, toManualMatch} = useUidMatching - ? matchByUID(local, remote) + const {matched, toCreate, toConfirm, toManualMatch} = useUuidMatching + ? matchByUUID(local, remote) : matchByNameAndType(local, remote) return { diff --git a/packages/app/src/cli/services/context/identifiers-extensions.ts b/packages/app/src/cli/services/context/identifiers-extensions.ts index 613bd0c2c12..c8b04cf7e75 100644 --- a/packages/app/src/cli/services/context/identifiers-extensions.ts +++ b/packages/app/src/cli/services/context/identifiers-extensions.ts @@ -311,7 +311,6 @@ async function createExtensions( result[extension.localIdentifier] = { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion id: extension.uid!, - uid: extension.uid, // eslint-disable-next-line @typescript-eslint/no-non-null-assertion uuid: extension.uid!, type: extension.type, diff --git a/packages/app/src/cli/services/context/identifiers.ts b/packages/app/src/cli/services/context/identifiers.ts index 3407fa88771..cdcb57b7484 100644 --- a/packages/app/src/cli/services/context/identifiers.ts +++ b/packages/app/src/cli/services/context/identifiers.ts @@ -24,7 +24,6 @@ export interface EnsureDeploymentIdsPresenceOptions { export interface RemoteSource { uuid: string - uid?: string type: string id: string title: string