Real-time campaign events triggering

[kuzmany]

Please be sure you are submitting this against the staging branch.

Q	A
Bug fix?
New feature?	y
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

• add migrations

This PR allow trigger events immediately after contact is added to campaign.

Steps to test this PR:

1. Load up this PR.
   If use own test enviroment run php bin/console doctrine:migrations:migrate
2. Create campaign with some events (actions/conditions) and enable option Trigger event's realtime
3. Add contact to campaign
4. See If events are triggered immediately after contact was added to campaign
5. Then disable option. Add another contact to campaign and see If events are not triggered immediately. I would by wait for cron

[codecov]

Codecov Report

Merging #10587 (93c8020) into 4.x (988ddd0) will increase coverage by 0.16%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##                4.x   #10587      +/-   ##
============================================
+ Coverage     42.95%   43.12%   +0.16%     
- Complexity    34532    34602      +70     
============================================
  Files          2062     2070       +8     
  Lines        115638   115961     +323     
============================================
+ Hits          49669    50003     +334     
+ Misses        65969    65958      -11     

Impacted Files	Coverage Δ
...paignBundle/Command/UpdateLeadCampaignsCommand.php	56.07% <0.00%> (ø)
...ecutioner/ContactFinder/Limiter/ContactLimiter.php	69.49% <ø> (ø)
.../bundles/CampaignBundle/Form/Type/CampaignType.php	0.00% <0.00%> (ø)
.../CampaignBundle/Command/TriggerCampaignCommand.php	85.88% <100.00%> (ø)
app/bundles/CampaignBundle/Entity/Campaign.php	60.37% <100.00%> (+1.75%)	⬆️
...ventListener/CampaignRealtimeTriggerSubscriber.php	100.00% <100.00%> (ø)
app/bundles/EmailBundle/Entity/EmailRepository.php	48.52% <0.00%> (-8.09%)	⬇️
...bhookBundle/Notificator/WebhookKillNotificator.php	95.45% <0.00%> (-4.55%)	⬇️
...undles/CoreBundle/Templating/Helper/FormHelper.php	57.14% <0.00%> (-3.58%)	⬇️
app/bundles/LeadBundle/Form/Type/FilterTrait.php	31.57% <0.00%> (-3.24%)	⬇️
... and 41 more

[kuzmany]

@escopecz can you check that error from PHPStan level 6

There is related article, but I was not able to set return type for iterator and I tried a lot of variants?
https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type

Can we disable that checks?

[kuzmany]

@escopecz I am talking about this error

Method                                                                                    
         Mautic\CampaignBundle\Tests\EventListener\CampaignRealtimeTriggerSubs                     
         criberTest::campaignRealtimeProvider() return type has no value type                      
         specified in iterable type iterable.                                                      
         💡 See:                                                                                    
         phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type            

I've tried a lot of variants and PHPDoc return type but does not work

[escopecz]

@kuzmany it looks like you figured it out. Thanks!

I know it is annoying and PHP developers are not used to specifying types inside an array. But it is common in other languages and there is a good chance it will come to PHP as well.

There is a bunch of bugs in Mautic that are coming from developers expecting an array in certain format but in some special cases there are other array structures coming in. Also, it will speed up the development time if the developers can clearly see what is in the array that is being returned or passed as a param.

[escopecz]

@kuzmany I'm looking at the new functionality here. Can you be more verbose about why is this needed? If the community will decide to merge it there should be some good documentation about this. I would vote against it as we try to decouple features to improve performance. This is coupling campaign membership build with campaign execution.

See #10550. It is actually working this way and my PR is removing that feature because it overloads the database if we build the membership and execute the events at the same time. One database table is heavily hit with reads and writes at the same time. CPU is running on 100% for hours and other Mautic features are getting unstable or even data loss can happen.

[kuzmany]

@escopecz It's 3 years old feature what I remember we use it for forgot password and some unlock feature, what we need immediately. It's not build to run mass events, we can add note or something. But If you need provide action immediately, for example after add contact to campaign by API it's useful.

@npracht @jos0405 as marketing guy do you have a use cases for this feature?

[escopecz]

I'm sure that users of the Mautic instances we maintain would abuse it and cause troubles for themselves which would end with more support tickets, angry users and wasted hours on investigations. If we decide to go forward with this, can we wrap the checkbox with an IF statement that would check for a config param? (Feature flag) so we could enable/disable it per instance?

[kuzmany]

@escopecz ok, that make sense.
Let's decide marketers If they need it, then we can add it as hidden feature by default to prevent your concerns

[jos0405]

This feature can be used for faster double opt-in processing, even if the cron would fire in lower intervalls.
I think it's a good feature.

[jos0405]

I would love to see this feature.
But gitpod hates me:

[ghost]

So the error is since July... today tested and I receive same error described also on another Issue that I also could not test: Label support for contactfield token #10626

I would be happy to test again :)
Greetings from Ionut

[kuzmany]

Let's close it