putyourlightson / craft-campaign

Send and manage email campaigns, contacts and mailing lists in Craft CMS.
https://putyourlightson.com/plugins/campaign
Other
63 stars 25 forks source link

Create and Subscribe Contact not working on a Synced List #389

Closed shifuma closed 1 year ago

shifuma commented 1 year ago

I'm creating a new user and then creating their contact and subscribing them to a list like this:

$mailingList = MailingListElement::find()->slug('users')->one();
Campaign::$plugin->forms->createAndSubscribeContact($email, null, $mailingList, 'Module', 'Module');

This works fine. However, after setting up a sync between the 'Users' group and the 'Users' mailing list the above no longer works. Is that expected behaviour?

bencroker commented 1 year ago

After syncing a mailing list and user group, every time a user in the selected user group is created, a contact in the synced mailing list will also be created. Since this is a one-way sync, you should probably be creating a user rather than creating a contact. Does that make sense?

shifuma commented 1 year ago

Ah, that does make sense; I am creating the user in my module so obviously don't need the createAndSubscribeContact because the group -> list sync handles that. Thanks for clarifying that.

However, I'm finding the sync to be a little bit hit-and-miss in my case. My module creates the user and adds them to the group. I see the contact is created but their fields aren't synced and they're not added to the synced list. At what point would the sync happen in that case? Is there a job I could trigger?

If I delete and re-add the sync, I see the fields are synced but they're not added to the list. Possibly just a timing thing?

bencroker commented 1 year ago

The sync should happen when the newly created user is assigned to a synced user group. Does the contact have the same fields as the user and are these not being synced? If that’s the case then this may be a bug, but I’d need more details to replicate and troubleshoot.

shifuma commented 1 year ago

That's right; it syncs the fields but doesn't add them to the mailing list. I'm using this to create a new user on a Commerce checkout using EVENT_AFTER_COMPLETE_ORDER. Here is the main part of my module:

$options = $item->options;

foreach ($options['email'] as $index => $email) {
    $name = $options['name'][$index];
    $location = $options['location'][$index];
    $subscribe = $options['mailingLists'][$index];

    $user = User::find()->email($email)->one();

    if ($user === null && $email) {
        $user = new User();
        $user->email = $email;
        $user->username = $email;
        $user->fullName = $name;
        $user->password = null;
        Craft::$app->getElements()->saveElement($user);
    }

    if ($location) {
        $locationEntry = Entry::find()->section('locations')->slug($location)->one();
        $user->location = $locationEntry->id;
    }

    Craft::$app->getElements()->saveElement($user);

    $usersGroup = Craft::$app->getUserGroups()->getGroupByHandle('allUsers')->id;
    if ($subscribe && !$user->isInGroup($usersGroup)) {
        Craft::$app->getUsers()->assignUserToGroups($user->id, [$usersGroup]);
    }
}

You can see I'm adding the location to the user; this plus the name are being synced to the contact. However, they're not subscribed to the mailing list synced with the allUsers user group.

I'm definitely not ruling out this being an error in my code.

bencroker commented 1 year ago

I tested your code and it works fine for me. My guess is that your contacts have fields that are required and that are not receiving valid values from the user. I added logging of sync errors in https://github.com/putyourlightson/craft-campaign/commit/c9c2ff15d544839c6bfa611a04429d5098ded503 and just released in version 2.7.2. Please update, retest the module and check the campaign.log file for errors, then let me know what you find.

shifuma commented 1 year ago

Thanks Ben. Unfortunately, nothing is written; that makes sense because the contact is synced properly, it's just that they're not added to the Mailing List.

But I think I've realised it's because of this:

https://github.com/putyourlightson/craft-campaign/blob/c9c2ff15d544839c6bfa611a04429d5098ded503/src/services/SyncService.php#L181-L197

I'm specifically creating inactive Users. I can understand why this might not be ideal for email subscriptions (spam) but the use case is members of our group purchasing on behalf of a family member, a friend etc. They must opt in on the front-end and also verify their email too.

Assuming this is the cause, could this perhaps be a setting? 'Sync only active users' - 'Sync all users' etc?

shifuma commented 1 year ago

Actually, no I don't think that's it. Adding $user->active = true; to new User() creation doesn't help.

I do wonder if there's something in my ordering. If I do this:

if ($user === null && $email) {
  $user = new User();
  $user->email = $email;
  $user->username = $email;
  $user->fullName = $name;
  $user->password = null;
  $user->active = true;
  Craft::$app->getElements()->saveElement($user);
}

if ($location) {
  $locationEntry = Entry::find()->section('locations')->slug($location)->one();
  $user->location = $locationEntry->id;
}

$usersGroup = Craft::$app->getUserGroups()->getGroupByHandle('allUsers')->id;
if ($subscribe && !$user->isInGroup($usersGroup)) {
  Craft::$app->getUsers()->assignUserToGroups($user->id, [$usersGroup]);
}

Craft::$app->getElements()->saveElement($user);

The contact is created, but the fields and mailing lists aren't added/synced properly. But if do this:

if ($user === null && $email) {
  $user = new User();
  $user->email = $email;
  $user->username = $email;
  $user->fullName = $name;
  $user->password = null;
  $user->active = true;
}

if ($location) {
  $locationEntry = Entry::find()->section('locations')->slug($location)->one();
  $user->location = $locationEntry->id;
}

Craft::$app->getElements()->saveElement($user);

$usersGroup = Craft::$app->getUserGroups()->getGroupByHandle('allUsers')->id;
if ($subscribe && !$user->isInGroup($usersGroup)) {
  Craft::$app->getUsers()->assignUserToGroups($user->id, [$usersGroup]);
}

Craft::$app->getElements()->saveElement($user);

The $location is synced properly, but the contact isn't subscribed to the mailing lists (the key difference being where Craft::$app->getElements()->saveElement($user); is run. If I save the user manually in the control panel after this, the user is synced/subscribed successfully.

So...perhaps the final saveElement($user) is wrong? It's not saving properly? I'm kind of thinking out loud as I go here, apologies, but if you have any thoughts on that I'd be interested. You mentioned my previous code worked for you so I'm confused.

bencroker commented 1 year ago

I was testing without the location field so my code wasn’t one-to-one with yours. Here’s how I would write it, there’s no need for saving the user a second time.

if ($user === null && $email) {
  $user = new User();
  $user->email = $email;
  $user->username = $email;
  $user->fullName = $name;
  $user->password = null;
  $user->active = true;
}

if ($location) {
  $locationEntry = Entry::find()->section('locations')->slug($location)->one();
  $user->location = $locationEntry->id;
}

Craft::$app->getElements()->saveElement($user);

$usersGroup = Craft::$app->getUserGroups()->getGroupByHandle('allUsers')->id;
if ($subscribe && !$user->isInGroup($usersGroup)) {
  Craft::$app->getUsers()->assignUserToGroups($user->id, [$usersGroup]);
}

If you want to deactivate the user after assigning to the user group then of course you can.

$user->active = false;
Craft::$app->getElements()->saveElement($user);
shifuma commented 1 year ago

Thanks, Ben. So for whatever reason, my module isn't subscribing them properly...that's not a huge problem for me. However, going back to the inactive/active thing, any syncing with mailing lists after the initial creation is based on that status. This part:

https://github.com/putyourlightson/craft-campaign/blob/c9c2ff15d544839c6bfa611a04429d5098ded503/src/services/SyncService.php#L181-L197

Is that something you could add a setting for?

bencroker commented 1 year ago

Yes I’ll consider it but let’s first resolve your issue. Can you try the following to rule out the case in which users are already in the allUsers user group?


$usersGroup = Craft::$app->getUserGroups()->getGroupByHandle('allUsers')->id;
Craft::$app->getUsers()->assignUserToGroups($user->id, [$usersGroup]);
shifuma commented 1 year ago

Thanks Ben, I appreciate your commitment! Unfortunately, no difference there.

I've tried this on a fresh install, with a simplified module/controller, and I can reproduce it there. I've packed those files up, in case it might help: campaign-issue.zip

Some steps to reproduce:

  1. Go to site homepage, create a user
  2. User and Contact are created, Contact is not added to Mailing List
  3. Remove $user->status == User::STATUS_ACTIVE from syncUserMailingList in the SyncService
  4. Save the User in the control panel, Contact is subscribed to the Mailing List
bencroker commented 1 year ago

Yes, you’re right that users are being subscribed to the mailing list only when they are “Active” users. This still makes sense to me, to help avoid spam users ending up in the mailing list.

So where are we in terms of your module code? The following code works perfectly for me, so you should be able to adapt it to your needs.

$email = 'sync-test@domain.com';
$user = User::find()->email($email)->one();
if ($user === null) {
    $user = new User();
    $user->email = $email;
    $user->username = $email;
    Craft::$app->getElements()->saveElement($user);
}

Craft::$app->getUsers()->activateUser($user);
Craft::$app->getUsers()->assignUserToGroups($user->id, [1]);
Craft::$app->getUsers()->deactivateUser($user);
shifuma commented 1 year ago

Ok that's got it working! Sorry, I was a bit slow to catch on to the activate/deactivate part. Thanks for sticking with it!