[STRATCONN-354] Marketo-V2 associateLead() deprecation #630

marinhero · 2021-09-10T00:55:45Z

What does this PR do?

Deprecates the Marketo associateLead and uses the recommended method of MktoForms2 instead.

Are there breaking changes in this PR?

Marketo will be removing associateLead in October. If customers don't take preemptive actions to start using forms, their calls to identify will be showing a message in the console:

'Invalid settings for identify method. Please review your Marketo V2 destination settings.'

Customer comms and doc updates are in progress to mitigate the hard cutover.

Testing

Testing completed successfully using the analytics.js-integrations compiler connected to stage.

Does this require a new integration setting? If so, please explain how the new setting works

Yes, two:

marketoFormId: every form in Marketo has an ID. We need customers to provide their form ID so we can pull it and use the form in calls to identify.
marketoHostUrl: the forms we will be using are relative to a domain. We need customers to provide their host URL so we can fetch the form from the right host.

Both settings are strings, available via CDN, and not required since track can work without them.

Links to helpful docs and other external resources

brennan · 2021-09-10T16:16:50Z

integrations/marketo-v2/lib/index.js

 */

 Marketo.prototype.initialize = function() {
  var self = this;


I don't think you need to reassign this to self here. When passed as an argument to a method (as on line 108), this should still refer to the outer context (in this case, the initialize prototype method).

Wouldn't that only apply if you are trying to access this itself inside of the function? All I'm doing on 108 is to pass a reference to the value inside the options object. Options become available via the scope of the function (this).

I'll move the reasign below the variable definitions but please let me know what you think.

brennan · 2021-09-10T16:17:33Z

integrations/marketo-v2/lib/index.js

  .option('host', 'https://api.segment.io')
  .option('accountId', '')
  .option('projectId', '')
  .option('traits', [])


Do we need to add the marketoFormId option somewhere here?

Good call, I'm not sure but I will learn about this. There's also marketoHostURL and it should be added in here as well

brennan · 2021-09-10T16:29:19Z

integrations/marketo-v2/lib/index.js

 var url = require('component-url');
 var when = require('do-when');
 var each = require('@ndhoule/each');
+var onBody = require('on-body-ready');


I wonder if we simply write our own helper method rather than install a third-party package. The relevant code looks to be about ~10 lines or so. Can we copy/paste?

Good idea! I was just following a pattern I saw in another integration but I'd rather not use a dependency. Thanks!

Turns out load will be enough to make sure the form library is ready to work! we don't need to bring anything in 😎

lcampos · 2021-09-13T15:42:49Z

integrations/marketo-v2/lib/index.js

 };

+Marketo.prototype.setupAndSubmitForm = function(traits, form) {
+  console.log('PRIME')


Do we need to keep this console.log ?

No no, this was for testing purposes. My bad!

lcampos · 2021-09-13T15:47:41Z

integrations/marketo-v2/lib/index.js

  });
+
+  this.load('forms', { marketoHostUrl: marketoHostUrl }, function() {
+    var marketoForm = document.createElement('form');


Should we only create the form id marketoHostUrl is defined ?

There's a validation for that already. The code won't reach this point if marketoHostUrl is undefined

integrations/marketo-v2/lib/index.js

integrations/marketo-v2/HISTORY.md

lcampos · 2021-09-13T16:00:07Z

compiler/package.json

    "@segment/analytics.js-integration-gtag": "file:../integrations/gtag",
    "@segment/analytics.js-integration-gosquared": "file:../integrations/gosquared",
    "@segment/analytics.js-integration-heap": "file:../integrations/heap",
-    "@segment/analytics.js-integration-hellobar": "file:../integrations/hellobar",


These deletes seem unrelated to your changes, do we need them in this PR ?

Indeed it's cleanup work but I think we should keep them. Otherwise, people will always have to troubleshoot the compiler before being able to start working.

brennan · 2021-09-13T21:14:26Z

@marinhero Is it possible to share, either directly in the PR comment or in a linked paper doc, the manual testing you did using the compiler tool?

marinhero · 2021-09-13T21:16:30Z

integrations/marketo-v2/lib/index.js

+Marketo.prototype.setupAndSubmitForm = function(traits, form) {
+  form.addHiddenFields(traits, form);
+  // Do not remove this callback. This ensures there are no page refreshes after the form is submitted.
+  form.onSuccess(function() {


Add an onFailure callback to not refresh in that scenario either

No callback available https://developers.marketo.com/javascript-api/forms/api-reference/ :|

lcampos

Approved pending the couple of changes we talked about earlier

…effects

brennan reviewed Sep 10, 2021

View reviewed changes

marinhero marked this pull request as ready for review September 11, 2021 00:32

marinhero requested review from lcampos, mericsson, nick-Ag and rvadera12 September 11, 2021 00:32

lcampos reviewed Sep 13, 2021

View reviewed changes

integrations/marketo-v2/lib/index.js Show resolved Hide resolved

lcampos reviewed Sep 13, 2021

View reviewed changes

integrations/marketo-v2/HISTORY.md Show resolved Hide resolved

lcampos reviewed Sep 13, 2021

View reviewed changes

marinhero force-pushed the marin/marketo-associateLead-deprecation branch from 1b66d5e to 4542428 Compare September 13, 2021 20:31

marinhero commented Sep 13, 2021

View reviewed changes

lcampos approved these changes Sep 13, 2021

View reviewed changes

marinhero force-pushed the marin/marketo-associateLead-deprecation branch from a0bb739 to 62c770c Compare September 17, 2021 19:32

kdaswani mentioned this pull request Sep 20, 2021

Update marketo V2 identify docs segmentio/segment-docs#1861

Merged

marinhero force-pushed the marin/marketo-associateLead-deprecation branch 3 times, most recently from 25b973e to 630b43c Compare September 27, 2021 16:51

marinhero added 8 commits October 5, 2021 14:12

Fix compiler

d5c5dff

Add onBody dependency to guarantee DOM readyness when inserting the form

c785e13

Add new tags

a101b83

Initialize and submit form

564bb0c

Add form settings validation

9b594a1

Remove unused code

f5a92fc

Update HISTORY

b06b48e

Remove unused dependencies

9d8d30e

marinhero added 17 commits October 5, 2021 14:12

WIP - TESTS

8ff8084

Add new options to integration export

21d9ff9

Remove onbody dependency

3620e0c

Move this reasign

b9ab9fa

Fix tests

0f60cc5

Fix unit test

febec39

Address comments

439f773

Source stable script

e3a7b99

Add form test

89350f0

Reset compiler changes

4202c39

Remove beta file from test suite

85fb4ee

Flag as beta

d6d4ac6

Update URL schema

03fa58f

Target an specific form so Marketo does not submit all forms

8e03e41

Dont use this inside a function because the scope changes

8782ebf

Setup no-redirects on form initialize to mitigate multiple form side …

492ed6e

…effects

Consider whenReady runs N times. N being the number of forms in a page

c62b13e

marinhero force-pushed the marin/marketo-associateLead-deprecation branch from 630b43c to c62b13e Compare October 5, 2021 19:12

update to 4.0.1 to prevent npm publish conflicts

ace7938

marinhero merged commit 9d2ab0a into master Oct 5, 2021

marinhero deleted the marin/marketo-associateLead-deprecation branch October 5, 2021 19:20

[STRATCONN-354] Marketo-V2 associateLead() deprecation #630

[STRATCONN-354] Marketo-V2 associateLead() deprecation #630

Uh oh!

Conversation

marinhero commented Sep 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brennan commented Sep 13, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcampos left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

marinhero commented Sep 10, 2021 •

edited

Loading