When using this module, it does not seem like Mailchimp recognizes the site under "Connected Sites". It seems like some custom JavaScript code needs to be added for the site to be recognized: http://kb.mailchimp.com/integrations/connected-sites/about-connected-sit...

If the module cannot generate the script, can we at lease support a field with instructions for manually pasting it in near within the module config?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

firewaller created an issue. See original summary.

firewaller’s picture

Patch attached.

Provides:

  • Directions to Connect Site
  • Textarea for pasting code
  • Include paths
  • Exclude paths
firewaller’s picture

Status: Active » Needs review
Greg Boggs’s picture

I'm realizing looking at this, giving folks the ability to post JavaScript to a Drupal site is the same as giving them user 1. The reason for this is that if you can publish JavaScript, you can publish JavaScript that steals cookies. There's gotta be a way to get the site to show connected without giving folks admin to the Drupal site.

Greg Boggs’s picture

It could be as simple as pulling the exact JS rather than letting users configure which JS to load?

firewaller’s picture

Good call. I was trying to simplify the process to prevent users from having to parse the Mailchimp code, but if we convert the textarea field to a URL field, we can limit any malicious javascript injection.

firewaller’s picture

The "Connected Sites" feature supports multiple sites/multiple lists. I wouldn't know how to pull that via the API, nor can I figure out how to generate the URL. It looks something like this:

<script id="mcjs">!function(c,h,i,m,p){m=c.createElement(h),p=c.getElementsByTagName(h)[0],m.async=1,m.src=i,p.parentNode.insertBefore(m,p)}(document,"script","https://chimpstatic.com/mcjs-connected/js/users/{SOME_ID_1}/{SOME_ID_2}.js");</script>

The options are either to:

  1. Pull script/URL via API
  2. Generate script/URL
  3. Allow user to paste in specific URL
firewaller’s picture

Updated patch attached.

  1. Replace textarea with URL to prevent JS injection
  2. Rename "connected" to "connect"
Greg Boggs’s picture

Unfortunately, a URL field doesn't prevent JS injection if we then output the contents of the URL as JS. The only way to prevent it is to hard code the URL of JavaScript ourselves.

Greg Boggs’s picture

So, instead of letting the user configure what JS to pull, we just pull the JS we know will work.

firewaller’s picture

Ok, then we need to generate the following URL: https://chimpstatic.com/mcjs-connected/js/users/{SOME_ID_1}/{SOME_ID_2}.js

SOME_ID_1 is the account_id
SOME_ID_2 ?

I've opened a support ticket with Mailchimp to determine SOME_ID_2.

firewaller’s picture

Attached is an improved patch that only allows the user to input the 2 IDs outlined above. The form validation restricts the values to alphanumeric strings with exactly 25 characters.

Am still waiting on an automated way to pull the second ID (i.e. "asset_id").

firewaller’s picture

FYI after speaking with Mailchimp support:

Though it's not documented yet, when making a GET request to ecommerce/stores or ecommerce/stores/store_id will return the "connected_sites" object where the "site_script" and the site "foreign_id" will be returned. The site_script object will have the url desired and the entire script to be inserted into a page, if needed. I'll put in a request so that our documentation will include that object.

We still haven't added API functionality for our Connected Sites feature for platforms that aren't listed in our KB article, so for the time, it'll be necessary to use the "Add Another Site" link as explained in the KB, connect the Site to the same list that the store is connected to, and then grab the script URL as needed for implementation/use in Drupal.

I can certainly pass along feedback to our developers that we let Connected Sites be available for stores that aren't listed in the KB article.

So it seems like, for the time being, we'll have to manually grab the code and add it to the site. Meaning the above patch is probably the best option.

kingandy’s picture

Version: 7.x-4.x-dev » 7.x-5.1

I would not use t() to sub variables into javascript, that will get the script entered into the translation system? Aside from cluttering up the translation system with script, it would also expose the javascript to anyone with translation permissions...

Also, I don't honestly think this one-use string really merits a define() function, but that's less of a security issue.

Since none of the params involved need to be sanitised or formatted I'd go direct to strtr(), which would look something like this:

$script = '!function(c,h,i,m,p){m=c.createElement(h),p=c.getElementsByTagName(h)[0],m.async=1,m.src=i,p.parentNode.insertBefore(m,p)}(document,"script","https://chimpstatic.com/mcjs-connected/js/users/!account_id/!asset_id.js");';
$script = strtr($script, array('!account_id' => $account_id, '!asset_id' => $asset_id));
kingandy’s picture

That said - it looks like the connected sites integration has come on a certain distance since this issue was last updated, there seems to be code for picking up connected sites and fetching javascript code via the API. There's still room for improvement though, as the help text doesn't detail how to actually set up a connected site in the first place? Without something at the Drupal end to actually generate the javascript (such as this patch), the only way to connect a site as far as I can see is to

- visit https://us18.admin.mailchimp.com/account/connected-sites/app-selection/
- select "Custom Website" (and "Custom Website")
- enter your site's URL into the appropriate field under "Get custom website code" and select the list to connect to

After this is done, the connection will appear in the MailchimpConnectedSites API object and can be selected on the Drupal config page. The site_script property can then be displayed as normal.

It took me a while to figure this out, I think it should be detailed in the help text because most of the MailChimp Connected Site documentation just takes you to the "Drupal Commerce" module page and waits for it to happen by magic somehow.

firewaller’s picture

Now that 7.x-5x is stable the majority of this patch is out of date. However, I'd like to improve the "mailchimp_connected_paths" to use drupal_match_path() so that we can use wildcards, etc. Also adding an "mailchimp_connected_paths_excluded" will be necessary when using wildcards. Also, yes the documentation should be updated.

firewaller’s picture

Status: Needs review » Needs work
firewaller’s picture

There seems to be official support for "Drupal Commerce" in Mailchimp now. Apter speaking with support, I removed my old manual "connected sites" and connected my sites via the official Drupal Commerce support. This removes the need for the Javascript snippet (i.e. mailchimp_connected_id).

FYI support said:

"It is certainly safe to remove the old manual connections and JS snippet provided by each, and leave the stores connected via the Drupal integration."

samuel.mortenson’s picture

Status: Needs work » Closed (won't fix)

We’re in maintenance-mode for the Drupal 7 releases of Mailchimp, so only bug fixes can be committed going forward. Any new features should be developed for the Drupal 8 releases of Mailchimp. Thanks!