Skip to content

Conversation

@JeremySkinner
Copy link

These commits begin the work on adding Opt Out features for communication, as well as mailchimp integration for emails.

  • Applies patch to drupal core for fixing an issue with hook entity_field_storage_info not working in D8
  • Adds Communications tab to contacts dashboard
  • Adds Commnucation profile type/bundle
  • Creates/configures the crm_opt_out field
  • Creates a new glue module (contacts_mailchimp):
    • Uses the Mailchimp module (a patch is needed to handle email field on related entities - see https://www.drupal.org/project/mailchimp/issues/2949125)
    • Automatically unsubscribes any mailchimp lists when the crm_opt_out field is set
    • Automatically does the reverse (sets the crm_opt_out field to true when unsubscribe webhook is received from mailchimp)

@JeremySkinner JeremySkinner changed the title Feature/communications and mailing Add Opt Out field and MailChimp integration Mar 5, 2018
Copy link
Collaborator

@yanniboi yanniboi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good so far. A couple of tweaks.

"#2584729: Hook entity_field_storage_info is broken": "https://www.drupal.org/files/issues/core-entity-sql-content-entity-storage-2583111-1_0.patch"
},
"drupal/ctools": {
"#2667652: Option to expose filters in block on views block display": "https://www.drupal.org/files/issues/ctools-option_to_expose-2667652-3.patch",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there not a mailchimp patch as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yanniboi Yes - this is only needed if you install contacts_mailchimp though, so should it go in the composer.json for the contacts module, or would there need to be a new composer.json specifically for contacts_mailchimp?

@@ -0,0 +1,35 @@
uuid: 5800747c-04df-4555-902f-b233214033a7
langcode: en
status: true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't tend to include uuids for config that goes in contributed modules. @andrewbelcher is that still accurate?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the uuid.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the current consensus is that UUIDs should be included in default configuration so you can see if different config entities are being managed (e.g. 2 modules provide the same profile type, UUID can tell you which one you have).

id: 'contacts_entity:profile-crm_communications'
label: 'Communications Profile block'
provider: contacts
label_display: '0'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets change the label to 'Communication preferences'.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied renaming


// TODO: Investigate making this asynchronous.
if (($original === 0 || $original == NULL) && $opt_out === 1 && $entity->uid->entity->hasField('mail')) {
$email = $entity->uid->entity->mail->value;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not many ways of doing asynchronous in Drupal. We could consider moving it to a cron queue for performance, but its probably fine for now.

@yanniboi yanniboi requested a review from andrewbelcher March 6, 2018 08:52
@JeremySkinner JeremySkinner requested a review from yanniboi March 7, 2018 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants