The interface for setting merge field tokens is confusing and inconsistent with the rest of Drupal.

Rather than setting the token using one of a predefined list of select options we should just provide a text box and a 'replacement patterns' list.

This would improve the user experience no end, and allow me to specify a custom date format e.g. [user:field-date:custom:Y-m-d]. It would also allow removing a big chunk of code I should imagine.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

Component: Lists » General
Category: task » feature

The Mailchimp token system already uses core tokens:

/**
 * Implements hook_mailchimp_lists_merge_tokens().
 */
function mailchimp_lists_mailchimp_lists_merge_tokens() {
  $tokens = array();
  // Grab user tokens. Support nested tokens of one level.
  $token_info = token_info();

just that it provides a dropdown rather than the standard textfield.

So it probably wouldn't be that much work to change the functionality.

We'd need to handle upgrading lists too. But again, doesn't look too hard. My exported list has this:

      "mergefields" : {
        "EMAIL" : "mail",
        "FNAME" : "profile-main:field_common_first_name",
        "LNAME" : "profile-main:field_common_surname"
      },

Those just need a [ and ] around them to be proper tokens.

(Moving to General component as the MC token system is in the core module.)

docans’s picture

Hi Joachim

I am trying to achieve the same functionality with ubercart attribute options. I want the options to accept tokens. I have created an issue at https://drupal.org/node/2180201 to enable me achieve this but no response so far. I am trying to follow the steps at http://clikfocus.com/blog/how-add-token-support-drupal-7 to solve the problem but to no avail.

I have created a repository on github that you can look at and guide me on a possible solution https://github.com/docans/poly_attributes/blob/master/poly_attribute/pol...

Thanks

eric.smith’s picture

Issue summary: View changes

I had a similar issue merging a date field.

hook_mailchimp_lists_mergevars_alter() solved my issue. I added the following to the .module file of my exported feature:

/**
 * Implementation of hook_mailchimp_lists_mergevars_alter
 */
function MYMODULE_mailchimp_lists_mergevars_alter(&$mergevars, $entity, $entity_type) {
  if (isset($mergevars['CHECKIN'])) {
    $mergevars['CHECKIN'] = date('d/m/Y', $mergevars['CHECKIN']);
  }
  if (isset($mergevars['CHECKOUT'])) {
    $mergevars['CHECKOUT'] = date('d/m/Y', $mergevars['CHECKOUT']);
  }
}
maxplus’s picture

Hi eric.smith,

I have an issue that when I try to merge a date field from Drupal that is formatted d/M/YYYY, its appears in the wrong way in the Mailchimp list because it expects to receive a format M/d/YYYY.

Maybe I could also add custom code to convert the date format of my field, before sending it to Mailchimp?

eojthebrave’s picture

I've got a patch in #2423057: Site Roles not merging to MailChimp Custom Text Field that converts the merge var mapping to use tokens, which would resolve this issue if it gets in I believe. At least in the 7.x-3.x branch anyway.

eojthebrave’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev

It looks like #2423057: Site Roles not merging to MailChimp Custom Text Field was closed, but the patch used was not the one that converts to using token syntax. So, I think the thing to do in this issue now will be:

- Change the current select list UX to use a [token] like syntax, and use token_* functions in the background to handle all the replacements
- Add an "advanced" toggle that lets the user switch from the current select list UX to one that uses standard textfields and allows for any token value to be entered

eojthebrave’s picture

Component: General » Lists
Status: Active » Needs review
FileSize
19.03 KB

Okay, here's a patch that converts the select lists to use true token syntax. It was already close, but not quite. So instead of things like mail, we're now doing [entity_type:mail]. This doesn't change the select list based UI at all, but it does allow for the use of token_replace() to figure out what value to use for a mergefield. Which, is far more robust than the current Entity API based solution we have now, which is frankly pretty brittle.

This requires the entity_token.module, which is part of the entity module, which this module already depends on. So I don't think there are any issues there. This is necessary because entity_token handles entity reference and term reference fields in a way that is similar to how we're doing it with the select widget based UI currently. So we can preserve backwards compatibility.

Doing this, then opens up the option to provide an "advanced" mode for mapping merge field values. Which, I've also implemented in this page. Basically, swap the select fields for textfields that allow for standard Drupal token input if someone selects the advanced checkbox. If the token module is installed it will also display a nice token browser. This, allows for orders of magnitude more flexibility when it comes to substituting in values for mergefields. And allows the user to do some pretty complex formatting as well. This solves the problem of multi-value fields not working, and allows you to format values to mean the MailChimp spec for date fields, birthday fields, and even location fields.

Personally, I'm not a huge fan of the extra code required to make the advanced UI possible and think that we should just remove the select field based UI all together. As the original poster points out; Using tokens, and filling them into a field, is a super common pattern used in Drupal. And, while the select list UI might look nicer, I would actually argue that for many Drupal user's it is simply more confusing. It's a pattern that isn't used anywhere else for performing this kind of value substitution, and when used, it makes the MailChimp module far less flexible when it comes to handling value substitution.

However, in order to at least get his functionality in so that we can make use if tokens I've gone ahead and provided for both the basic select field based UI and the token + texfield based UI to co-exist.

I've also included an update hook to enable the now required entity_token module, and to convert all existing configuration to the new standard Drupal 7 token syntax.

  • levelos committed ce86300 on authored by eojthebrave
    Issue #2008898 by eojthebrave: Replace Merge fields select options with...
levelos’s picture

Thanks for the great patch @eojthebrave, applied! Nice job with the advanced toggle. I tested the update on a few of our sites actively using the lists module and everything worked as expected.

We'll bundle a few more issues and roll a release next week.

levelos’s picture

Status: Needs review » Fixed
eojthebrave’s picture

Awesome! Thanks.

I haven't actually verified all of these, but a quick search reveals these other issues that I believe will now be resolved by this issue being fixed. Probably worth going through them to see what we can close.

- #1630810: only authenticated role merges to mailchimp field - this patch allows for all roles to by synced as a coma separated list
- #1400790: Sync Content Profile fields with merge tags - maybe, because we can now use any value that can be retrieved by a token, so any module that extends user's with profile data that supports tokens should work fine
- #1407794: Make it possible to merge with address field type - we can support this now since we can use tokens formatting to match whatever we need to send MailChimp for address fields
- #1325334: content profile widget "select list" don't display for merge variables tokens - we can now support mutli-value fields so I think this can be resolved
- #1999966: User 'date' fields added to user from Fields module don't get properly sent to MailChimp - we can support date fields since we can use tokens to format the Drupal date in whatever way is needed for MailChimp e.g.) [user:created:custom:m]/[user:created:custom:d]/[user:created:custom:Y] for mm/dd/yyyy

eojthebrave’s picture

Status: Fixed » Needs review
FileSize
2.92 KB

I've just discovered 2 different issues that are where introduced by this patch while doing some additional testing.

The first, is that I assumed tokens of the form [user:field_name] would be okay here. However, this isn't true without also making the token module from contrib required. Which, I don't think we want to do. So instead we need to use entity_token style tokens which in this case would be [user:field-name]. The difference is the use of - instead of _. It appears like the token module just globally introduces the ability to use either - or _, but the one's we use currently with the _ are going to fail without token module.

So, the attached patch fixes that issue in both the token replacement code, and in the hook_update_N code.

The other issue happens when you edit mailchimp list field instance that is currently using the select field UI. Then, while editing click the advanced toggle to enable the token based UI, and proceed to enter in some token values before clicking save. Doing so will result in a, "An illegal option ... please contact a site administrator." error message. This happens because of the form validation when submitting the instance settings form where you've just enabled the advanced UI.

When the form is first loaded using the basic UI Drupal places a version of that form in the cache. Then, when you click the ajax button and the select fields are replaced with textfields it becomes possible to enter in new tokens that were not previously possible with the select list. If you do this, and then submit the form, during the validation phase Drupal will compare the form you submitted with the version that's in the cache in order to protect against local modification. Since the version in the cache still thinks the merge field fields are select lists it will attempt to validate their values against the #options for the select list. Which can cause problems since there are far more tokens than those we list in the #options. This results in an error being thrown. This will only happen the FIRST time you toggle the advanced UI on. Since after that, every time the form is generated the cached version is the advanced version.

The attached patch also fixes that issue by ensuring that when the from is rebuilt, and re-cached, during an ajax request, that it uses the field instance data from the just-submitted form, and not that which is in the database, which doesn't reflect the toggle over to advanced mode.

Sorry about missing these the first time around, they where both hidden by the fact that i was testing this all with the token module already enabled. But, we want to make sure things work both when it's on, and when it is off.

  • jami committed 1a7c4a0 on 7.x-3.x authored by eojthebrave
    Issue #2008898 by eojthebrave: Replace Merge fields select options with...
jami’s picture

Version: 7.x-3.x-dev » 7.x-3.4
Status: Needs review » Fixed

Thank you to everyone for their hard work, and thanks eojthebrave for the follow-up patches. Tokens seem to be working well now!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Andreas Radloff’s picture

Version: 7.x-3.4 » 8.x-1.x-dev
Status: Closed (fixed) » Needs work
Related issues: +#2764815: Improve compatabillity with modules expecting an email field.

This needs to be ported to the D8 version.

asrob’s picture

Berdir’s picture

Considering to port this to Drupal 8.

What I specifically need is support for the name module, so I can assign the properties (first name, last name, ..) to Mailchimp merge fields.

I can see two options for that:

a) The simpler solution would be to add support for multi-property fields to the select, something like field_name:first_name => Name: First Name. This is for example how entity reference sort selection does it. But you can't do fancier things like converting a date field to a specific format.

b) Porting this to 8.x, but this would require a dependency on token.module as only that provides field level tokens and it's somewhat tricky with base fields which might be named differently and token only provides base field tokens if there are no default tokens. Also needs an upgrade path.

Some feedback from a maintainer what would be preferred would be useful. I'm considering to implement option a) in a new issue, since that should be considerably less work/smaller patch and is fully backwards compatible.

Berdir’s picture

If anyone is interested, I went with option a) and opened #2908547: Finish port of merge field population logic for that.

ruscoe’s picture

Status: Needs work » Fixed

Berdir's Drupal 8 solution in issue #2908547 has been committed. Thank you for all your work on this and other issues, Berdir.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.