Fix Atomatewoo opt-in Sync

[puntope]

Description

This PR fixes/implements the Opt-in sync between MailPoet and AW

Currently, the Checkout implements already a compatibility between the two plugins. Also, we do full sync when the user is Unsubscribed.

However, when it subscribes via newsletter it doesn't get Opted In under AutomateWoo

Code review notes

How to test this PR:

1. Checkout this PR
2. Install AutomateWoo Plugin
3. Create a Mail Poet signup form somewhere.
4. Sign up a user and accept the invitation
5. Go to AutomateWoo-Opt-in
6. See if the user is Opted-in
7. GO to communication preferences. Receive marketing promotions is checked.
8. Make this user Opted-out in AutomateWoo
9. Check in Mailpoet user is set as Unsubscribed.
10. Repeat step 4
11. Unsubscribe the user using Mailpoet
12. Go to AutomateWoo-Opt-in and see the user is not in Opt-in list

Tasks

• I followed best practices for translations
• I added sufficient test coverage
• I embraced TypeScript by either creating new files in TypeScript or converting existing JavaScript files when making changes

[JanJakes]

@puntope Thanks for the pull request! The code looks perfectly fine to me, simply from the implementation side, I could approve the changes.

However, I'm wondering about the product side of this. Could you, please, clarify the reason for these changes? I'm only asking because I'm not sure whether we're generally allowed to sync subscription status for AutomateWoo when someone subscribes on the MailPoet side.

Legally, we're required to ask for user consent for each type of email, and sending something different from what the user agreed to could be considered spam.

Since we're listening to SubscriberEntity::HOOK_SUBSCRIBER_STATUS_CHANGED here, it appears that this would result in users subscribing to any list would also be given consent on the AutomateWoo side. I think that might be problematic. Maybe we'll need to check for subscription status in the WooCommerce list on our side, or something like that, although we don't seem to sync unsubscribes from that list (only the global ones).

I think @brezocordero and @websupporter might know more about the original implementation and the desired behavior, so I will check with them.

[puntope]

However, I'm wondering about the product side of this. Could you, please, clarify the reason for these changes? I'm only asking because I'm not sure whether we're generally allowed to sync subscription status for AutomateWoo when someone subscribes on the MailPoet side.

The context for this is a ticket. For what I see in the ticket 8195586-zen

Basically when the user subscribes in Mailpoet and confirms the email. The AW opt-in is not updated directly. The user needs to go to communication preferences and then the sync is happening.

Notice we do the sync in the status in previous implementations when checking if a user is unsubscribed. So In practice. The result in this PR is the same. But without the need to go to communication preferences.

This is what I can give from my side. As I'm not familiar with the product. I will be waiting for Mailpoet discussion about it. Let me know if I can help with something.

[JanJakes]

@puntope Thanks for the details. If we're currently syncing $subscriber->getStatus() in any direction (opt-in, opt-out), then that could be problematic, and we may need to review that to sync a WooCommerce list + global status combination instead, so that what is a "WooCommerce opt-in" in MailPoet equals to WooCommerce opt-in in AutomateWoo. MailPoet can also serve lists that are unrelated to WooCommerce and syncing that status could cause issues for some users.

[puntope]

@puntope Thanks for the details. If we're currently syncing $subscriber->getStatus() in any direction (opt-in, opt-out), then that could be problematic, and we may need to review that to sync a WooCommerce list + global status combination instead, so that what is a "WooCommerce opt-in" in MailPoet equals to WooCommerce opt-in in AutomateWoo. MailPoet can also serve lists that are unrelated to WooCommerce and syncing that status could cause issues for some users.

Can you elaborate this? Not super familiar with mailpoet so maybe I'm missing some bits.

[JanJakes]

@puntope What we need to sync with AutomateWoo is "WooCommerce checkout opt-in/opt-out", a feature for customers of a WooCommerce shop. If we'd sync any different opt-in/opt-out flags, that would be wrong. For instance, if you subscribe to receive my blog articles via email, then we are not supposed (and allowed) to send WooCommerce related emails from either of MailPoet and AutomateWoo.

[puntope]

What we need to sync with AutomateWoo is "WooCommerce checkout opt-in/opt-out"

This seems working. But also the subscriber can opt-in (to WooCommerce list) via the subscribe form. Which seems not working. (see in the ticket 8195586-zd-a8c)

I guess the part missing is that I need to check that besides Subscribed status is subscribed. The subscribed list is WooCommerce?

[JanJakes]

@puntope

I guess the part missing is that I need to check that besides Subscribed status is subscribed. The subscribed list is WooCommerce?

If my understanding is correct product-wise (I will verify with the team), then yes, this is correct, and in addition to that, we'll need to listen to mailpoet_segment_subscribed hook as well to cover list-level-only status changes as well. So, in summary, I think it should be:

1. Add a check that the subscriber is subscribed to the WooCommerce list (you can use $shouldOptIn = $subscriber->getStatus() === SubscriberEntity::STATUS_SUBSCRIBED && $this->subscribersRepository->getWooCommerceSegmentSubscriber($subscriber->getEmail());, for instance.
2. Listen also to mailpoet_segment_subscribed hook. In this case, you will get a SubscriberSegmentEntity, so you'll need to check that $subscriberSegment->getSegment()->getType() === SegmentEntity::TYPE_WC_USERS, and if so, then you can fetch the subscriber ($subscriberSegment->getSubscriber()) and proceed the same code path as in point 1.

I hope that makes sense.

[puntope]

HI @JanJakes I updated the code with your suggestions. However I have 2 issues I don't know how to solve:

• Some Circle CI tasks failed. but not sure why
• I tried to create a new WooCommerce subscriber from scratch using the Form but I wasn't able to. Is this not anymore possible?

Thanks in advance

[JanJakes]

Hi @puntope, sorry for the delays, we sorted out the desired behavior and I finally got to this. I rebased this on latest trunk, fixed a failing test, and added some small improvements — some just stylistic (we don't add PHP doc annotations that don't add more info on top of typehints and method names), some small fixes (the if/else opt-in/out condition would miss cases when someone opts-out from the WooCommerce list, but not globally).

I'll move it on for another code review, given that I touched the code as well, and then we will proceed merging & releasing this.

One important thing to note here is that if AutomateWoo itself synchronizes the opt-in/opt-out in any way, it will need to respect the WooCommerce list subscription in the same way as implemented here. That is:

if ($subscriber->getStatus() === SubscriberEntity::STATUS_SUBSCRIBED && $subscribedInWooCommerceList) {
  // opt-in
} else {
  // opt-out
}

Thanks a lot for your contribution! 👏

[brezocordero]

Looks good, nothing to add on my end.
I haven't tested it manually, I will let QA do it.

[Aschepikov]

GJ!