An optimistic locking mechanism for campaigns

[fedys]

Q	A
Bug fix? (use the a.b branch)	[❌]
New feature/enhancement? (use the a.x branch)	[✅]
Deprecations?	[❌]
BC breaks? (use the c.x branch)	[❌]
Automated tests included?	[✅]
Related user documentation PR URL
Related developer documentation PR URL
Issue(s) addressed

Description:

This PR adds support for optimistic locking. I tried to implement it via Doctrine functionality. It turned out to be too dangerous as it could cause many bugs (connected with OptimisticLockException). I implemented the optimistic locking the way we can control when an entity version is incremented/checked.

Let's consider the following steps:

1. Navigate to the edit page of an existing campaign.
2. Open a new browser tab and navigate to the edit page of the same campaign.
3. In the latter tab modify the name (append -2 for example) and click Apply.
4. Switch back to the first tab and without refreshing the browser modify the name (append -3 for example) and click Apply.
5. The modification from the third step is lost.

Steps to test this PR:

1. Run the migration with the command ddev exec bin/console doctrine:migrations:migrate if needed
2. Navigate to the edit page of an existing campaign.
3. Open a new tab and navigate to the edit page of the same campaign.
4. In the latter tab modify the name (append -2 for example) and click Apply.
5. Switch back to the first tab and without refreshing the browser modify the name (append -3 for example) and click Apply.
6. You should get an error message that reads The record you are updating has been changed by someone else in the meantime. Please refresh the browser window and re-submit your changes..
7. The modification from the third step is not lost.

[codecov]

Codecov Report

Attention: Patch coverage is 92.53731% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 61.34%. Comparing base (d0fe128) to head (49027ba).
Report is 26 commits behind head on 5.x.

Additional details and impacted files

@@             Coverage Diff              @@
##                5.x   #13659      +/-   ##
============================================
+ Coverage     61.26%   61.34%   +0.07%     
- Complexity    33983    34003      +20     
============================================
  Files          2238     2240       +2     
  Lines        101593   101660      +67     
============================================
+ Hits          62243    62365     +122     
+ Misses        39350    39295      -55     

Files	Coverage Δ
app/bundles/CampaignBundle/Entity/Campaign.php	80.58% <100.00%> (+0.09%)	⬆️
.../bundles/CampaignBundle/Form/Type/CampaignType.php	93.93% <100.00%> (+0.18%)	⬆️
.../bundles/CoreBundle/Entity/OptimisticLockTrait.php	100.00% <100.00%> (ø)
...eBundle/EventListener/OptimisticLockSubscriber.php	92.30% <92.30%> (ø)
...ndle/Controller/AbstractStandardFormController.php	61.89% <86.36%> (+5.94%)	⬆️

... and 4 files with indirect coverage changes

[shinde-rahul]

LGTM!

Thanks @fedys!!

[Mike-Dropsolid]

Followed the steps and works as described!

[RCheesley]

This worked fine if I just changed something like the name.

If I opened the builder, and made a change to the campaign, then I got the following 500 error:

500 Internal Server Error - Key "deleted" for array with keys "canvasSettings, name, triggerMode, triggerDate, triggerInterval, triggerIntervalUnit, triggerHour, triggerRestrictedStartHour, triggerRestrictedStopHour, anchor, properties, type, eventType, anchorEventType, campaignId, _token, buttons, settings, triggerRestrictedDaysOfWeek, tempId, id" does not exist.

Stack trace:

[2024-04-25T13:22:03.155371+00:00] mautic.CRITICAL: Uncaught PHP Exception Twig\Error\RuntimeError: "Key "deleted" for array with keys "canvasSettings, name, triggerMode, triggerDate, triggerInterval, triggerIntervalUnit, triggerHour, triggerRestrictedStartHour, triggerRestrictedStopHour, anchor, properties, type, eventType, anchorEventType, campaignId, _token, buttons, settings, triggerRestrictedDaysOfWeek, tempId, id" does not exist." at /var/www/html/app/bundles/CampaignBundle/Resources/views/Campaign/_builder.html.twig line 36 {"exception":"[object] (Twig\\Error\\RuntimeError(code: 0): Key \"deleted\" for array with keys \"canvasSettings, name, triggerMode, triggerDate, triggerInterval, triggerIntervalUnit, triggerHour, triggerRestrictedStartHour, triggerRestrictedStopHour, anchor, properties, type, eventType, anchorEventType, campaignId, _token, buttons, settings, triggerRestrictedDaysOfWeek, tempId, id\" does not exist. at /var/www/html/app/bundles/CampaignBundle/Resources/views/Campaign/_builder.html.twig:36)
[stacktrace]
#0 /var/www/html/vendor/twig/twig/src/Environment.php(360) : eval()'d code(132): twig_get_attribute(Object(Twig\\Environment), Object(Twig\\Source), Array, 'deleted', Array, 'any', false, false, false, 36)
#1 /var/www/html/vendor/twig/twig/src/Template.php(394): __TwigTemplate_b3b345528e9a52da99d56ed8e00e11da->doDisplay(Array, Array)
#2 /var/www/html/vendor/twig/twig/src/Template.php(367): Twig\\Template->displayWithErrorHandling(Array, Array)
#3 /var/www/html/vendor/twig/twig/src/Template.php(379): Twig\\Template->display(Array)
#4 /var/www/html/vendor/twig/twig/src/TemplateWrapper.php(38): Twig\\Template->render(Array)
#5 /var/www/html/vendor/twig/twig/src/Extension/CoreExtension.php(1347): Twig\\TemplateWrapper->render(Array)
#6 /var/www/html/vendor/twig/twig/src/Environment.php(360) : eval()'d code(170): twig_include(Object(Twig\\Environment), Array, '@MauticCampaign...', Array)
#7 /var/www/html/vendor/twig/twig/src/Template.php(171): __TwigTemplate_38a11f0e36529cc48894b46211a2c936->block_content(Array, Array)
#8 /var/www/html/vendor/twig/twig/src/Template.php(243): Twig\\Template->displayBlock('content', Array, Array, true)
#9 /var/www/html/vendor/twig/twig/src/Environment.php(360) : eval()'d code(66): Twig\\Template->renderBlock('content', Array, Array)
#10 /var/www/html/vendor/twig/twig/src/Template.php(394): __TwigTemplate_274f4d32b0e34e384411d368c6082172->doDisplay(Array, Array)
#11 /var/www/html/vendor/twig/twig/src/Template.php(367): Twig\\Template->displayWithErrorHandling(Array, Array)
#12 /var/www/html/vendor/twig/twig/src/Environment.php(360) : eval()'d code(49): Twig\\Template->display(Array, Array)
#13 /var/www/html/vendor/twig/twig/src/Template.php(394): __TwigTemplate_38a11f0e36529cc48894b46211a2c936->doDisplay(Array, Array)
#14 /var/www/html/vendor/twig/twig/src/Template.php(367): Twig\\Template->displayWithErrorHandling(Array, Array)
#15 /var/www/html/vendor/twig/twig/src/Template.php(379): Twig\\Template->display(Array)
#16 /var/www/html/vendor/twig/twig/src/TemplateWrapper.php(38): Twig\\Template->render(Array)
#17 /var/www/html/vendor/twig/twig/src/Environment.php(280): Twig\\TemplateWrapper->render(Array)
#18 /var/www/html/vendor/symfony/framework-bundle/Controller/AbstractController.php(258): Twig\\Environment->render('@MauticCampaign...', Array)
#19 /var/www/html/app/bundles/CoreBundle/Controller/CommonController.php(334): Symfony\\Bundle\\FrameworkBundle\\Controller\\AbstractController->renderView('@MauticCampaign...', Array)
#20 /var/www/html/app/bundles/CoreBundle/Controller/CommonController.php(157): Mautic\\CoreBundle\\Controller\\CommonController->ajaxAction(Object(Symfony\\Component\\HttpFoundation\\Request), Array)
#21 /var/www/html/app/bundles/CoreBundle/Controller/AbstractStandardFormController.php(490): Mautic\\CoreBundle\\Controller\\CommonController->delegateView(Array)
#22 /var/www/html/app/bundles/CampaignBundle/Controller/CampaignController.php(207): Mautic\\CoreBundle\\Controller\\AbstractStandardFormController->editStandard(Object(Symfony\\Component\\HttpFoundation\\Request), '1', false)
#23 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(163): Mautic\\CampaignBundle\\Controller\\CampaignController->editAction(Object(Symfony\\Component\\HttpFoundation\\Request), '1', false)
#24 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(75): Symfony\\Component\\HttpKernel\\HttpKernel->handleRaw(Object(Symfony\\Component\\HttpFoundation\\Request), 2)
#25 /var/www/html/vendor/symfony/framework-bundle/Controller/AbstractController.php(156): Symfony\\Component\\HttpKernel\\HttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 2)
#26 /var/www/html/app/bundles/CoreBundle/Controller/CommonController.php(415): Symfony\\Bundle\\FrameworkBundle\\Controller\\AbstractController->forward('Mautic\\\\Campaign...', Array, Array)
#27 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(163): Mautic\\CoreBundle\\Controller\\CommonController->executeAction(Object(Symfony\\Component\\HttpFoundation\\Request), 'edit', '1', 0, '')
#28 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(75): Symfony\\Component\\HttpKernel\\HttpKernel->handleRaw(Object(Symfony\\Component\\HttpFoundation\\Request), 1)
#29 /var/www/html/vendor/symfony/http-kernel/Kernel.php(202): Symfony\\Component\\HttpKernel\\HttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#30 /var/www/html/app/AppKernel.php(109): Symfony\\Component\\HttpKernel\\Kernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#31 /var/www/html/app/middlewares/CORSMiddleware.php(76): AppKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#32 /var/www/html/app/middlewares/HSTSMiddleware.php(39): Mautic\\Middleware\\CORSMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#33 /var/www/html/app/middlewares/CatchExceptionMiddleware.php(28): Mautic\\Middleware\\HSTSMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#34 /var/www/html/app/middlewares/Dev/IpRestrictMiddleware.php(52): Mautic\\Middleware\\CatchExceptionMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#35 /var/www/html/app/middlewares/VersionCheckMiddleware.php(58): Mautic\\Middleware\\Dev\\IpRestrictMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#36 /var/www/html/app/middlewares/TrustMiddleware.php(42): Mautic\\Middleware\\VersionCheckMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#37 /var/www/html/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Mautic\\Middleware\\TrustMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#38 /var/www/html/index.php(19): Stack\\StackedHttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request))
#39 {main}
"} {"hostname":"mautic-web","pid":11885}

[fedys]

Thanks @RCheesley! I tried to reproduce it in my environment but with no luck. Please can you provide me with the exact steps you took. Screen recording would help a lot.

[RCheesley]

Sure thing, here's a screen recording: https://app.screencastify.com/v3/watch/oBWSIOnNIxfozpvu2qJP

1. Edit the campaign name with -2 appended in window 2 and save
2. In the first window, edit the campaign, add -3 to the name (for example) and then edit it in the campaign builder and add a new action, for example, then save the campaign
3. Error thrown

[fedys]

@RCheesley thanks a ton! I'm able to reproduce it. I will take a look at what is going wrong.