Remove MauticFactory::getEntityManager. #14611
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 6.x #14611 +/- ##
=========================================
Coverage 64.67% 64.67%
- Complexity 34675 34676 +1
=========================================
Files 2273 2273
Lines 103596 103597 +1
=========================================
+ Hits 66996 66998 +2
+ Misses 36600 36599 -1
|
ed6924a to
0709cf4
Compare
There was a problem hiding this comment.
When I try to trigger the point change I get the following in the logs, the pixel throws a 500 error:
[2025-02-21T17:20:20.265849+00:00] mautic.CRITICAL: Uncaught PHP Exception ReflectionException: "Given object is not an instance of the class this method was declared in" at /var/www/html/app/bundles/PointBundle/Model/PointModel.php line 260 {"exception":"[object] (ReflectionException(code: 0): Given object is not an instance of the class this method was declared in at /var/www/html/app/bundles/PointBundle/Model/PointModel.php:260)
[stacktrace]
#0 /var/www/html/app/bundles/PointBundle/Model/PointModel.php(260): ReflectionMethod->invokeArgs()
#1 /var/www/html/app/bundles/PageBundle/EventListener/PointSubscriber.php(65): Mautic\\PointBundle\\Model\\PointModel->triggerAction()
#2 /var/www/html/vendor/symfony/event-dispatcher/Debug/WrappedListener.php(116): Mautic\\PageBundle\\EventListener\\PointSubscriber->onPageHit()
#3 /var/www/html/vendor/symfony/event-dispatcher/EventDispatcher.php(220): Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener->__invoke()
#4 /var/www/html/vendor/symfony/event-dispatcher/EventDispatcher.php(56): Symfony\\Component\\EventDispatcher\\EventDispatcher->callListeners()
#5 /var/www/html/vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php(139): Symfony\\Component\\EventDispatcher\\EventDispatcher->dispatch()
#6 /var/www/html/app/bundles/PageBundle/Model/PageModel.php(661): Symfony\\Component\\EventDispatcher\\Debug\\TraceableEventDispatcher->dispatch()
#7 /var/www/html/app/bundles/PageBundle/Model/PageModel.php(467): Mautic\\PageBundle\\Model\\PageModel->processPageHit()
#8 /var/www/html/app/bundles/PageBundle/Model/Tracking404Model.php(27): Mautic\\PageBundle\\Model\\PageModel->hitPage()
#9 /var/www/html/app/bundles/PageBundle/Controller/PublicController.php(296): Mautic\\PageBundle\\Model\\Tracking404Model->hitPage()
#10 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(181): Mautic\\PageBundle\\Controller\\PublicController->indexAction()
#11 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(76): Symfony\\Component\\HttpKernel\\HttpKernel->handleRaw()
#12 /var/www/html/vendor/symfony/http-kernel/Kernel.php(197): Symfony\\Component\\HttpKernel\\HttpKernel->handle()
#13 /var/www/html/app/AppKernel.php(101): Symfony\\Component\\HttpKernel\\Kernel->handle()
#14 /var/www/html/app/middlewares/CORSMiddleware.php(76): AppKernel->handle()
#15 /var/www/html/app/middlewares/HSTSMiddleware.php(39): Mautic\\Middleware\\CORSMiddleware->handle()
#16 /var/www/html/app/middlewares/CatchExceptionMiddleware.php(28): Mautic\\Middleware\\HSTSMiddleware->handle()
#17 /var/www/html/app/middlewares/Dev/IpRestrictMiddleware.php(52): Mautic\\Middleware\\CatchExceptionMiddleware->handle()
#18 /var/www/html/app/middlewares/VersionCheckMiddleware.php(58): Mautic\\Middleware\\Dev\\IpRestrictMiddleware->handle()
#19 /var/www/html/app/middlewares/TrustMiddleware.php(42): Mautic\\Middleware\\VersionCheckMiddleware->handle()
#20 /var/www/html/app/middlewares/StackedHttpKernel.php(55): Mautic\\Middleware\\TrustMiddleware->handle()
#21 /var/www/html/index.php(19): Mautic\\Middleware\\StackedHttpKernel->handle()
#22 {main}
"} {"hostname":"mautic6-web","pid":76157}
No points are added as above.
c1318ba to
50d8242
Compare
|
I've bumped this to 7.0-alpha as there are still some things to be resolved in the tests. |
|
@RCheesley tests were green, only a small code style issue is left. |
|
@biozshock not from here: It is untested after fixing the issues I reported, and with pending feedback, and we haven't got time to hold up the release further. 7.0-alpha will be out quite soon so it won't be long before it can be released. |
d2f3151 to
aa47c03
Compare
|
@RCheesley yup, because new test case was added that interfered with the changes. |
|
The failing test case is not a big issue. The issue is that the point handling must be refactored from static callbacks to events. It doesn't work like this. |
|
@escopecz Sure, and i don't like how it's implemented now, but the point of this PR is to remove MauticFactory, not to refactor how PointSubscribers work. You can create a new issue describing this problem with the PointSubscribers. And somebody will get it on track sooner or later. |
|
@biozshock I get that. But I hope we agree that refactoring cannot be merged if it breaks the functionality as Ruth demonstrated. It must be refactored to actually work. This cannot be merged in this state. |
|
@escopecz This was fixed, and even automatically tested: https://github.com/mautic/mautic/pull/14611/files#diff-f3fd1b2c9b2a09517bd18437785e01be6af88ae73c62d2d3f39b57e7bef17219R88 |
|
@biozshock I stand corrected. I was looking for a commit or a comment in the PR history that would suggest that it was fixed and didn't find any. Could you maybe add additional commits instead of updating the original commit? I know that you want to make the git history clean but it's also hard to see what was changed when. This is Ready to test now. |
|
@escopecz , yes, true, in this case when something was fixed - this could use a commit, or at least a comment to indicate the fix. |
aa47c03 to
58cb5ee
Compare
Description
Removal of the
getEntityManagerfrom deprecatedMauticFactory.The only userspace part affected is the points trigger on URL hit.
Other parts please code review.
📋 Steps to test this PR: