Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement Twingle's double opt-in #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

implement Twingle's double opt-in #35

wants to merge 1 commit into from

Conversation

MarcMichalsky
Copy link
Contributor

@MarcMichalsky MarcMichalsky commented Jun 29, 2020

An implementation that permits the use of the Twingle Double-Opt-In procedure.

TwingleManager:
TwingleManager_Double-Opt-In

I added a new action DoubleOptInConfirm to the TwingleDonation entity.

The API call that Twingle would have to make should contain a JSON like this:

{"project_id":"tw5e8b3bd4c2589","user_email":"dummy@contact.com"}

Features

  • If the Double-Opt-In checkbox in the Twingle profile is ticked, a contact will be added to the newsletter groups with a pending state
  • A contact which already is member of the newsletter group, won't get set back to pending

fix #34

Copy link
Collaborator

@jensschuppe jensschuppe left a comment

Choose a reason for hiding this comment

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

@MarcMichalsky Thanks for the PR, I added some thoughts on the implementation.

Have you already talked to Twingle about that new API call?

Comment on lines +70 to +108
if (!empty($params['user_email'])) {
$contacts = civicrm_api3('Email', 'get', array(
'seqential' => 1,
'email' => $params['user_email'],
));

// Get pending group memberships for user
if (!empty($contacts['values'])) {
foreach ($contacts['values'] as $contact) {
$groups = civicrm_api3('GroupContact', 'get', array(
'sequential' => 1,
'contact_id' => $contact['contact_id'],
'status' => "Pending",
));

// Only in newsletter groups: change group membership from pending to added
if (!empty($groups['values'])) {
foreach ($groups['values'] as $group) {
if (in_array($group['group_id'], $newsletter_groups)) {
civicrm_api3('GroupContact', 'create', array(
'group_id' => $group['group_id'],
'contact_id' => $contact['contact_id'],
'status' => "Added",
$result_values['groups'][] = $group['group_id'],
));
// Display message if group membership was confirmed correctly
$result_values['double_opt_in'][$group['group_id']] = "Subscription confirmed";
}
}
// Display message if there is no pending group membership
} else {
$result_values['double_opt_in'][] = "Could not confirm subscription: No pending group membership";
}
}
// Display message if email can't be found
} else {
$result_values['double_opt_in'][] = "Could not confirm subscription: Email not found";
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The contact should be identified using XCM, just as the submit API action does. Otherwise, this might alter group memberships for other contacts with the same e-mail address. We should also only target that one single contact identified by XCM with the same profile.

Therefore, all user_ parameters (and maybe custom_fields also) should be added to this API action.

Copy link
Contributor Author

@MarcMichalsky MarcMichalsky Jun 30, 2020

Choose a reason for hiding this comment

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

Considering that a Double-Opt-In confirmation always refers to an email address, I thought that matching a contact wouldn't be necessary. Especially when the contact has already been matched in the submit step. How should we keep DOI confirmations from different contacts with the same email apart, anyway?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is what XCM takes care of: either match a single contact or create one, according to the XCM profile configuration being used. I.e. contacts with the same e-mail address don't matter, since XCM will only return one contact ID. Since GroupContact is a thing that belongs to a contact (not an e-mail entity) - it's a crosstab with group_id and contact_id - we should really make sure we select the correct one and thus identify the previously matched contact again using XCM.

Comment on lines +493 to +503
if (empty(civicrm_api3('GroupContact', 'get', array(
'group_id' => $group_id,
'contact_id' => $contact_id,
))['values'])) {
civicrm_api3('GroupContact', 'create', array(
'group_id' => $group_id,
'contact_id' => $contact_id,
'status' => "Pending",
));
$result_values['double_opt_in'][] = "Pending";
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make the GroupContact.get a GroupContact.getsingle wrapped with a try and put the GroupContact.create in the catch? This would avoid "Undefined index" warnings when something goes wrong with the API call. There can only be one GroupContact record per contact_id-group_id combination.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

@MarcMichalsky
Copy link
Contributor Author

Have you already talked to Twingle about that new API call?

Hey @jensschuppe,
we just had a conversation with Twingle. The developer told us that he would like to "think bigger" . From his point of view it would make sense to implement a function to update all types of attributes, whereof DOI would only be one.

We talked about a bunch of thinks that we are missing to map all Twingle activities into our CiviCRM. We are going to contact you to talk about possible next steps.

@MarcMichalsky MarcMichalsky changed the title implement Double-Opt-In implement Twingle's double opt-in Jul 21, 2020
@jensschuppe
Copy link
Collaborator

@MarcMichalsky I think I'd prefer not having a one-for-all update API action to keep things easy and maintainable. I can't really imagine what else would need to be updated. Also, this would always involve identifying the contact, which depends on all the user_* data be static for consistent results.

Since Twingle will need to do a REST API call for confirmed Double Opt-Ins anyway, I think it doesn't matter whether it's a specific TwingleDonation.DoubleOptInConfirm or whatever else.

But let's see what we come up with ...

@jensschuppe jensschuppe added the enhancement New feature or request label Aug 14, 2020
@jensschuppe jensschuppe added this to the 1.3 milestone Aug 14, 2020
@jensschuppe jensschuppe added the status:needs review Code needs review and testing label Mar 25, 2021
@jensschuppe jensschuppe added status:needs work There is code, but it needs additional work before it should be reviewed and removed status:needs review Code needs review and testing labels May 2, 2022
@jensschuppe jensschuppe removed this from the 1.3 milestone May 2, 2022
@jensschuppe jensschuppe added this to the 1.4 milestone Feb 22, 2023
@jensschuppe jensschuppe modified the milestones: 1.4, 1.5 May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request status:needs work There is code, but it needs additional work before it should be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Twingle's double-opt-In for e-mail is not getting mapped
2 participants